Skip to content

Commit

Permalink
feat: remove GetPrivKeyHandle (#992)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bhearsum authored Sep 20, 2024
1 parent 3062de6 commit 4f4674e
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 27 deletions.
6 changes: 4 additions & 2 deletions database/queries.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion database/queries_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
5 changes: 2 additions & 3 deletions signer/contentsignaturepki/contentsignature.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 1 addition & 17 deletions signer/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 1 addition & 4 deletions signer/signer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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))
}
}
}

Expand Down

0 comments on commit 4f4674e

Please sign in to comment.