Skip to content

Commit

Permalink
chore: store public key fingerprint
Browse files Browse the repository at this point in the history
Store the public key fingerprint in the DB rather than calculating it at runtime.
  • Loading branch information
kian99 committed Jan 13, 2025
1 parent e04e29d commit 763cbd4
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 42 deletions.
43 changes: 11 additions & 32 deletions internal/db/sshkeys.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ package db

import (
"context"
"fmt"

gossh "golang.org/x/crypto/ssh"

"github.com/canonical/jimm/v3/internal/dbmodel"
"github.com/canonical/jimm/v3/internal/errors"
Expand Down Expand Up @@ -42,27 +39,19 @@ func (d *Database) RemoveSSHKeyByFingerprint(ctx context.Context, identityName s
defer durationObserver()
defer servermon.ErrorCounter(servermon.DBQueryErrorCount, &err, string(op))

var keys []dbmodel.SSHKey
if err := d.DB.Where("identity_name = ?", identityName).Find(&keys).Error; err != nil {
query := d.DB.Where("identity_name = ?", identityName).
Where("md5_fingerprint = ?", fingerprint).
Delete(&dbmodel.SSHKey{})

if err := query.Error; err != nil {
return errors.E(op, dbError(err))
}

// It is expected that we only have 1 key that matches this fingerprint
// because of the unique constraint between users and public keys.
for _, key := range keys {
fp, err := calculateSSHFingerprint(key.PublicKey)
if err != nil {
return errors.E(op, err)
}
if fp == fingerprint {
if err := d.DB.WithContext(ctx).Delete(key).Error; err != nil {
return errors.E(op, dbError(err))
}
return nil
}
if query.RowsAffected == 0 {
return errors.E(op, errors.CodeNotFound, "key not found")
}

return errors.E(op, errors.CodeNotFound, "key not found")
return nil
}

// RemoveSSHKeyByComment removes a user's ssh key identified by its comment.
Expand All @@ -89,9 +78,9 @@ func (d *Database) RemoveSSHKeyByComment(ctx context.Context, identityName strin
return nil
}

// ListSSHKeys all a user's SSH keys.
func (d *Database) ListSSHKeys(ctx context.Context, identityName string) (keys []dbmodel.SSHKey, err error) {
const op = errors.Op("db.ListSSHKeys")
// ListSSHKeysForUser lists all user's SSH keys.
func (d *Database) ListSSHKeysForUser(ctx context.Context, identityName string) (keys []dbmodel.SSHKey, err error) {
const op = errors.Op("db.ListSSHKeysForUser")

if err := d.ready(); err != nil {
return nil, errors.E(op, err)
Expand All @@ -107,13 +96,3 @@ func (d *Database) ListSSHKeys(ctx context.Context, identityName string) (keys [

return keys, nil
}

// calculateSSHFingerprint parses an SSH public key and returns its MD5 fingerprint
func calculateSSHFingerprint(publicKey []byte) (string, error) {
parsedKey, err := gossh.ParsePublicKey(publicKey)
if err != nil {
return "", fmt.Errorf("invalid SSH key: %v", err)
}

return gossh.FingerprintLegacyMD5(parsedKey), nil
}
23 changes: 13 additions & 10 deletions internal/db/sshkeys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ func (s *dbSuite) TestCreateSSHKey(c *qt.C) {
c.Assert(s.Database.DB.Create(u).Error, qt.IsNil)

key := dbmodel.SSHKey{
Identity: *u,
PublicKey: []byte("foo"),
KeyComment: "bar",
Identity: *u,
PublicKey: []byte("foo"),
KeyComment: "bar",
MD5Fingerprint: "fake-fingerprint",
}
err = s.Database.AddSSHKey(context.Background(), &key)
c.Assert(err, qt.IsNil)
Expand All @@ -40,7 +41,7 @@ func (s *dbSuite) TestCreateSSHKey(c *qt.C) {
c.Assert(err, qt.ErrorMatches, `.*duplicate key value violates unique constraint.*`)
}

func (s *dbSuite) TestListSSHKeys(c *qt.C) {
func (s *dbSuite) TestListSSHKeysForUser(c *qt.C) {
err := s.Database.Migrate(context.Background())
c.Assert(err, qt.Equals, nil)

Expand All @@ -52,24 +53,26 @@ func (s *dbSuite) TestListSSHKeys(c *qt.C) {
c.Assert(err, qt.IsNil)
c.Assert(s.Database.DB.Create(u2).Error, qt.IsNil)

key := dbmodel.SSHKey{Identity: *u, PublicKey: []byte("foo"), KeyComment: "bar"}
key := dbmodel.SSHKey{Identity: *u, PublicKey: []byte("foo"), KeyComment: "bar", MD5Fingerprint: "fake-fingerprint"}
c.Assert(s.Database.DB.Create(&key).Error, qt.IsNil)

key2 := dbmodel.SSHKey{Identity: *u, PublicKey: []byte("foo2"), KeyComment: "bar2"}
key2 := dbmodel.SSHKey{Identity: *u, PublicKey: []byte("foo2"), KeyComment: "bar2", MD5Fingerprint: "fake-fingerprint"}
c.Assert(s.Database.DB.Create(&key2).Error, qt.IsNil)

// Key3 is owned by Alice and should not be returned.
key3 := dbmodel.SSHKey{Identity: *u2, PublicKey: []byte("foo3"), KeyComment: "bar3"}
key3 := dbmodel.SSHKey{Identity: *u2, PublicKey: []byte("foo3"), KeyComment: "bar3", MD5Fingerprint: "fake-fingerprint"}
c.Assert(s.Database.DB.Create(&key3).Error, qt.IsNil)

gotKeys, err := s.Database.ListSSHKeys(context.Background(), "bob@canonical.com")
gotKeys, err := s.Database.ListSSHKeysForUser(context.Background(), "bob@canonical.com")
c.Assert(err, qt.IsNil)
c.Assert(gotKeys, qt.HasLen, 2)
c.Assert(gotKeys[0].IdentityName, qt.Equals, "bob@canonical.com")
c.Assert(gotKeys[0].KeyComment, qt.Equals, "bar")
c.Assert(gotKeys[0].MD5Fingerprint, qt.Equals, "fake-fingerprint")
c.Assert(string(gotKeys[0].PublicKey), qt.Equals, "foo")
c.Assert(gotKeys[1].IdentityName, qt.Equals, "bob@canonical.com")
c.Assert(gotKeys[1].KeyComment, qt.Equals, "bar2")
c.Assert(gotKeys[1].MD5Fingerprint, qt.Equals, "fake-fingerprint")
c.Assert(string(gotKeys[1].PublicKey), qt.Equals, "foo2")
}

Expand All @@ -91,11 +94,11 @@ func (s *dbSuite) TestRemoveSSHKeyByFingerprint(c *qt.C) {
publicKey, err := gossh.NewPublicKey(&rsaKey.PublicKey)
c.Assert(err, qt.IsNil)

key := dbmodel.SSHKey{Identity: *u, PublicKey: publicKey.Marshal(), KeyComment: "bar"}
key := dbmodel.SSHKey{Identity: *u, PublicKey: publicKey.Marshal(), KeyComment: "bar", MD5Fingerprint: gossh.FingerprintLegacyMD5(publicKey)}
c.Assert(s.Database.DB.Create(&key).Error, qt.IsNil)

// key2 with the same public-key but different owner should not be deleted.
key2 := dbmodel.SSHKey{Identity: *u2, PublicKey: publicKey.Marshal(), KeyComment: "bar"}
key2 := dbmodel.SSHKey{Identity: *u2, PublicKey: publicKey.Marshal(), KeyComment: "bar", MD5Fingerprint: gossh.FingerprintLegacyMD5(publicKey)}
c.Assert(s.Database.DB.Create(&key2).Error, qt.IsNil)

var keyCount int64
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
-- store public keys fingerprints

ALTER TABLE ssh_keys ADD COLUMN md5_fingerprint VARCHAR(50) NOT NULL;
2 changes: 2 additions & 0 deletions internal/dbmodel/sshkeys.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ type SSHKey struct {

// PublicKey holds the user's public SSH key.
PublicKey []byte `gorm:"uniqueIndex:unique_identity_ssh_key"`
// MD5Fingerprint is the MD5 fingerprint of the public key.
MD5Fingerprint string
// KeyComment holds a user provided comment.
KeyComment string
}

0 comments on commit 763cbd4

Please sign in to comment.