Skip to content

Commit

Permalink
refactor: isolate HSM interactions and crypto11 use in a more generic…
Browse files Browse the repository at this point in the history
… interface (#993)

The main driver behind this is to enable us to add a `GCPHSM` with its own MakeKey implementation to avoid cluttering up `Configuration.MakeKey` further. While I was doing this, I moved the other HSM and `crypto11`` related things elsewhere as well. Notably, two things are still present in `signer.Configuration` that would be part of a `signer.HSM` implementation in an ideal world:
* The pub key casting in `Configuration.GetKeys`
* `Configuration.CheckHSMConnection` entirely

Both of these things depend on `Configuration.GetPrivateKey` which needs a `label` of a key to fetch. I don't think it's appropriate to move that label to the `HSM` interface (because it would tie an instance to a single key). I couldn't find a way to adjust the usage of these methods that didn't end up making things messier, or involved refactoring other things, so I'm punting on it for now.
  • Loading branch information
bhearsum authored Sep 23, 2024
1 parent 48489a1 commit f531e44
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 44 deletions.
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ func (a *autographer) initHSM(conf configuration) error {
// if we successfully initialized the crypto11 context,
// tell the signers they can try using the HSM
for i := range conf.Signers {
conf.Signers[i].InitHSM(tmpCtx)
conf.Signers[i].InitHSM(signer.NewAWSHSM(tmpCtx))
signerConf := &conf.Signers[i]

// save the first signer with an HSM label as
Expand Down
75 changes: 75 additions & 0 deletions signer/hsm.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package signer

import (
"crypto"
"crypto/ecdsa"
"crypto/rsa"
"fmt"
"io"

"github.com/miekg/pkcs11"
"github.com/mozilla-services/autograph/crypto11"
)

type HSM interface {
GetPrivateKey(label []byte) (crypto.PrivateKey, error)
MakeKey(keyTpl interface{}, keyName string) (crypto.PrivateKey, crypto.PublicKey, error)
GetRand() io.Reader
}

type GenericHSM struct {
ctx *pkcs11.Ctx
}

// GetPrivateKey locates the keypair given by `label` in the HSM.
func (hsm *GenericHSM) GetPrivateKey(label []byte) (crypto.PrivateKey, error) {
return crypto11.FindKeyPair(nil, label)
}

func (hsm *GenericHSM) GetRand() io.Reader {
return new(crypto11.PKCS11RandReader)
}

type AWSHSM struct {
GenericHSM
}

// MakeKey generates a new keypair of type `keyTpl` and returns the new key structs.
func (hsm *AWSHSM) MakeKey(keyTpl interface{}, keyName string) (crypto.PrivateKey, crypto.PublicKey, error) {
var slots []uint
slots, err := hsm.ctx.GetSlotList(true)
if err != nil {
return nil, nil, fmt.Errorf("failed to list PKCS#11 Slots: %w", err)
}
if len(slots) < 1 {
return nil, nil, fmt.Errorf("failed to find a usable slot in hsm context")
}
keyNameBytes := []byte(keyName)
switch keyTplType := keyTpl.(type) {
case *ecdsa.PublicKey:
priv, err := crypto11.GenerateECDSAKeyPairOnSlot(slots[0], keyNameBytes, keyNameBytes, keyTplType)
if err != nil {
return nil, nil, fmt.Errorf("failed to generate ecdsa key in hsm: %w", err)
}
pub := priv.PubKey.(*ecdsa.PublicKey)
return priv, pub, nil
case *rsa.PublicKey:
keySize := keyTplType.Size()
priv, err := crypto11.GenerateRSAKeyPairOnSlot(slots[0], keyNameBytes, keyNameBytes, keySize)
if err != nil {
return nil, nil, fmt.Errorf("failed to generate rsa key in hsm: %w", err)
}
pub := priv.PubKey.(*rsa.PublicKey)
return priv, pub, nil
default:
return nil, nil, fmt.Errorf("making key of type %T is not supported", keyTpl)
}
}

func NewAWSHSM(ctx *pkcs11.Ctx) *AWSHSM {
return &AWSHSM{
GenericHSM{
ctx,
},
}
}
58 changes: 15 additions & 43 deletions signer/signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/mozilla-services/autograph/formats"

"github.com/DataDog/datadog-go/statsd"
"github.com/miekg/pkcs11"
"github.com/mozilla-services/autograph/crypto11"

log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -130,13 +129,13 @@ type Configuration struct {
SignerOpts crypto.SignerOpts `json:"signer_opts,omitempty" yaml:"signeropts,omitempty"`

isHsmAvailable bool
hsmCtx *pkcs11.Ctx
Hsm HSM
}

// InitHSM indicates that an HSM has been initialized
func (cfg *Configuration) InitHSM(ctx *pkcs11.Ctx) {
func (cfg *Configuration) InitHSM(hsm HSM) {
cfg.isHsmAvailable = true
cfg.hsmCtx = ctx
cfg.Hsm = hsm
}

// Signer is an interface to a configurable issuer of digital signatures
Expand Down Expand Up @@ -251,7 +250,7 @@ type TestFileGetter interface {
// HSM if available and otherwise rand.Reader
func (cfg *Configuration) GetRand() io.Reader {
if cfg.isHsmAvailable {
return new(crypto11.PKCS11RandReader)
return cfg.Hsm.GetRand()
}
return rand.Reader
}
Expand Down Expand Up @@ -307,20 +306,18 @@ func (cfg *Configuration) GetKeys() (priv crypto.PrivateKey, pub crypto.PublicKe

// GetPrivateKey uses a signer configuration to determine where a private
// key should be accessed from. If it is in local configuration, it will
// be parsed and loaded in the signer. If it is in an HSM, it will be
// used via a PKCS11 interface. This is completely transparent to the
// caller, who should simply assume that the privatekey implements a
// crypto.Sign interface
//
// Note that we assume the PKCS11 library has been previously initialized
// be parsed and loaded in the signer. If it is in an HSM, this will be
// outsourced to `cfg.Hsm`, which knows how to locate a private key handle
// in an HSM. Either way, the returned value implements the crypto.Sign
// interface.
func (cfg *Configuration) GetPrivateKey() (crypto.PrivateKey, error) {
cfg.PrivateKey = removePrivateKeyNewlines(cfg.PrivateKey)
if cfg.PrivateKeyHasPEMPrefix() {
return ParsePrivateKey([]byte(cfg.PrivateKey))
}
// otherwise, we assume the privatekey represents a label in the HSM
if cfg.isHsmAvailable {
key, err := crypto11.FindKeyPair(nil, []byte(cfg.PrivateKey))
key, err := cfg.Hsm.GetPrivateKey([]byte(cfg.PrivateKey))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -395,6 +392,8 @@ func (cfg *Configuration) PrivateKeyHasPEMPrefix() bool {
// CheckHSMConnection (exposed via the signer.Configuration
// interface). It tried to fetch the signer private key and errors if
// that fails or the private key is not an HSM key handle.
// Ideally this would be part of the HSM interface, but the check requires
// the label of a key on the HSM, which is part of the Configuration
func (cfg *Configuration) CheckHSMConnection() error {
if cfg.PrivateKeyHasPEMPrefix() {
return fmt.Errorf("private key for signer %s has a PEM prefix and is not an HSM key label", cfg.ID)
Expand All @@ -410,39 +409,12 @@ func (cfg *Configuration) CheckHSMConnection() error {
return nil
}

// MakeKey generates a new key of type keyTpl and returns the priv and public interfaces.
// If an HSM is available, it is used to generate and store the key, in which case 'priv'
// just points to the HSM handler and must be used via the crypto.Signer interface.
// MakeKey generates a new key of type keyTpl and returns the private and
// public interfaces. If an HSM is available, this is outsourced to `cfg.Hsm`,
// which will generate them in an HSM instead of in memory.
func (cfg *Configuration) MakeKey(keyTpl interface{}, keyName string) (priv crypto.PrivateKey, pub crypto.PublicKey, err error) {
if cfg.isHsmAvailable {
var slots []uint
slots, err = cfg.hsmCtx.GetSlotList(true)
if err != nil {
return nil, nil, fmt.Errorf("failed to list PKCS#11 Slots: %w", err)
}
if len(slots) < 1 {
return nil, nil, fmt.Errorf("failed to find a usable slot in hsm context")
}
keyNameBytes := []byte(keyName)
switch keyTplType := keyTpl.(type) {
case *ecdsa.PublicKey:
priv, err = crypto11.GenerateECDSAKeyPairOnSlot(slots[0], keyNameBytes, keyNameBytes, keyTplType)
if err != nil {
return nil, nil, fmt.Errorf("failed to generate ecdsa key in hsm: %w", err)
}
pub = priv.(*crypto11.PKCS11PrivateKeyECDSA).PubKey.(*ecdsa.PublicKey)
return
case *rsa.PublicKey:
keySize := keyTplType.Size()
priv, err = crypto11.GenerateRSAKeyPairOnSlot(slots[0], keyNameBytes, keyNameBytes, keySize)
if err != nil {
return nil, nil, fmt.Errorf("failed to generate rsa key in hsm: %w", err)
}
pub = priv.(*crypto11.PKCS11PrivateKeyRSA).PubKey.(*rsa.PublicKey)
return
default:
return nil, nil, fmt.Errorf("making key of type %T is not supported", keyTpl)
}
return cfg.Hsm.MakeKey(keyTpl, keyName)
}
// no hsm, make keys in memory
switch keyTplType := keyTpl.(type) {
Expand Down

0 comments on commit f531e44

Please sign in to comment.