From 4f4674eb755b7cae80d0416decf957366005f7c2 Mon Sep 17 00:00:00 2001 From: "Ben Hearsum (he/him)" Date: Fri, 20 Sep 2024 14:07:46 -0400 Subject: [PATCH] feat: remove GetPrivKeyHandle (#992) This function is used, but the only use in production code is in `contentsignaturepki/contentsignature.go`. The only thing we do with it _there_ is insert it into the database. The only time it is consumed from the database is to be printed out as general information as part of cleanup script. That is to say: it is unused for all practical purposes. This change removes GetPrivKeyHandle and its uses, but does not go as far as removing the database column itself. This is in part because a schema change is less trivial than code removal, but also to guard against the remote chance that I'm misinterpreting something, and we need to roll this back. We can remove the column itself at some future date when we're absolutely certain it is unused. --- database/queries.go | 6 ++++-- database/queries_test.go | 2 +- signer/contentsignaturepki/contentsignature.go | 5 ++--- signer/signer.go | 18 +----------------- signer/signer_test.go | 5 +---- 5 files changed, 9 insertions(+), 27 deletions(-) diff --git a/database/queries.go b/database/queries.go index 62872f97a..3a84a6c6d 100755 --- a/database/queries.go +++ b/database/queries.go @@ -66,9 +66,11 @@ func (db *Handler) GetLabelOfLatestEE(signerID string, youngerThan time.Duration } // InsertEE uses an existing transaction to insert an end-entity in database -func (tx *Transaction) InsertEE(x5u, label, signerID string, hsmHandle uint) (err error) { +func (tx *Transaction) InsertEE(x5u, label, signerID string) (err error) { + // hsm_handle is unused, but required to be not null. We should remove this + // column at some point; in the meantime, we always set it to -1 _, err = tx.Exec(`INSERT INTO endentities(x5u, label, signer_id, hsm_handle, is_current) - VALUES ($1, $2, $3, $4, $5)`, x5u, label, signerID, hsmHandle, true) + VALUES ($1, $2, $3, $4, $5)`, x5u, label, signerID, -1, true) if err != nil { tx.Rollback() err = fmt.Errorf("failed to insert new key in database: %w", err) diff --git a/database/queries_test.go b/database/queries_test.go index 7c5cb48c7..80a84de75 100644 --- a/database/queries_test.go +++ b/database/queries_test.go @@ -85,7 +85,7 @@ func waitAndMakeEE(j int, db *Handler, wg *sync.WaitGroup, t *testing.T, signerI label = fmt.Sprintf("%d", time.Now().UnixNano()) t.Logf("TestConcurrentEndEntityOperations: routine %d is making an end-entity", j) err = tx.InsertEE("http://example.com/TestConcurrentEndEntityOperations", - label, signerID, uint(j)) + label, signerID) if err != nil { t.Fatalf("failed to insert end-entity into db: %v", err) } diff --git a/signer/contentsignaturepki/contentsignature.go b/signer/contentsignaturepki/contentsignature.go index 993b3d394..087c563d4 100755 --- a/signer/contentsignaturepki/contentsignature.go +++ b/signer/contentsignaturepki/contentsignature.go @@ -169,12 +169,11 @@ func (s *ContentSigner) initEE(conf signer.Configuration) error { } if tx != nil { // insert it in database - hsmHandle := signer.GetPrivKeyHandle(s.eePriv) - err = tx.InsertEE(s.X5U, s.eeLabel, s.ID, hsmHandle) + err = tx.InsertEE(s.X5U, s.eeLabel, s.ID) if err != nil { return fmt.Errorf("contentsignaturepki %q: failed to insert EE into database: %w", s.ID, err) } - log.Printf("contentsignaturepki %q: generated private key labeled %q with hsm handle %d and x5u %q", s.ID, s.eeLabel, hsmHandle, s.X5U) + log.Printf("contentsignaturepki %q: generated private key labeled %q with x5u %q", s.ID, s.eeLabel, s.X5U) } releaseLock: if tx != nil { diff --git a/signer/signer.go b/signer/signer.go index f7dbd4ce6..069b3915e 100644 --- a/signer/signer.go +++ b/signer/signer.go @@ -403,14 +403,10 @@ func (cfg *Configuration) CheckHSMConnection() error { return fmt.Errorf("HSM is not available for signer %s", cfg.ID) } - privKey, err := cfg.GetPrivateKey() + _, err := cfg.GetPrivateKey() if err != nil { return fmt.Errorf("error fetching private key for signer %s: %w", cfg.ID, err) } - // returns 0 if the key is not stored in the hsm - if GetPrivKeyHandle(privKey) != 0 { - return nil - } return fmt.Errorf("unable to check HSM connection for signer %s private key is not stored in the HSM", cfg.ID) } @@ -478,18 +474,6 @@ func (cfg *Configuration) MakeKey(keyTpl interface{}, keyName string) (priv cryp } } -// GetPrivKeyHandle returns the hsm handler object id of a key stored in the hsm, -// or 0 if the key is not stored in the hsm -func GetPrivKeyHandle(priv crypto.PrivateKey) uint { - switch key := priv.(type) { - case *crypto11.PKCS11PrivateKeyECDSA: - return uint(key.Handle) - case *crypto11.PKCS11PrivateKeyRSA: - return uint(key.Handle) - } - return 0 -} - // StatsClient is a helper for sending statsd stats with the relevant // tags for the signer and error handling type StatsClient struct { diff --git a/signer/signer_test.go b/signer/signer_test.go index d451e68dc..45d3bfe90 100644 --- a/signer/signer_test.go +++ b/signer/signer_test.go @@ -204,7 +204,7 @@ func TestMakeKey(t *testing.T) { if err != nil { t.Fatalf("testcase %d failed to load signer configuration: %v", i, err) } - priv, pub, err := testcase.cfg.MakeKey(keyTpl, "test") + _, pub, err := testcase.cfg.MakeKey(keyTpl, "test") if err != nil { t.Fatalf("testcase %d failed to make %T key from signer configuration: %v", i, keyTpl, err) } @@ -213,9 +213,6 @@ func TestMakeKey(t *testing.T) { if keyTplType != pubType { t.Fatalf("testcase %d failed, expected public key of type %q but got %q", i, keyTplType, keyTplType) } - if GetPrivKeyHandle(priv) != 0 { - t.Fatalf("testcase %d failed, expected public key handle 0 but got %d", i, GetPrivKeyHandle(priv)) - } } }