From 6be383da34046384a4a5fbd029f238bf119ca21b Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Oct 2021 18:04:11 -0700 Subject: [PATCH] Refactor pkcs#11 extractable certs and keys. --- kms/pkcs11/other_test.go | 40 +++++++++++++++++++++++-- kms/pkcs11/pkcs11.go | 65 +++++++++++++++++++++++----------------- 2 files changed, 75 insertions(+), 30 deletions(-) diff --git a/kms/pkcs11/other_test.go b/kms/pkcs11/other_test.go index 835587f7..51732853 100644 --- a/kms/pkcs11/other_test.go +++ b/kms/pkcs11/other_test.go @@ -1,3 +1,4 @@ +//go:build cgo && !softhsm2 && !yubihsm2 && !opensc // +build cgo,!softhsm2,!yubihsm2,!opensc package pkcs11 @@ -79,10 +80,23 @@ func (s *stubPKCS11) FindCertificate(id, label []byte, serial *big.Int) (*x509.C } +func (s *stubPKCS11) ImportCertificateWithAttributes(template crypto11.AttributeSet, cert *x509.Certificate) error { + var id, label []byte + if v := template[crypto11.CkaId]; v != nil { + id = v.Value + } + if v := template[crypto11.CkaLabel]; v != nil { + label = v.Value + } + return s.ImportCertificateWithLabel(id, label, cert) +} + func (s *stubPKCS11) ImportCertificateWithLabel(id, label []byte, cert *x509.Certificate) error { switch { - case id == nil && label == nil: - return errors.New("id and label cannot both be nil") + case id == nil: + return errors.New("id cannot both be nil") + case label == nil: + return errors.New("label cannot both be nil") case cert == nil: return errors.New("certificate cannot be nil") } @@ -110,6 +124,17 @@ func (s *stubPKCS11) DeleteCertificate(id, label []byte, serial *big.Int) error return nil } +func (s *stubPKCS11) GenerateRSAKeyPairWithAttributes(public, private crypto11.AttributeSet, bits int) (crypto11.SignerDecrypter, error) { + var id, label []byte + if v := public[crypto11.CkaId]; v != nil { + id = v.Value + } + if v := public[crypto11.CkaLabel]; v != nil { + label = v.Value + } + return s.GenerateRSAKeyPairWithLabel(id, label, bits) +} + func (s *stubPKCS11) GenerateRSAKeyPairWithLabel(id, label []byte, bits int) (crypto11.SignerDecrypter, error) { if id == nil && label == nil { return nil, errors.New("id and label cannot both be nil") @@ -130,6 +155,17 @@ func (s *stubPKCS11) GenerateRSAKeyPairWithLabel(id, label []byte, bits int) (cr return k, nil } +func (s *stubPKCS11) GenerateECDSAKeyPairWithAttributes(public, private crypto11.AttributeSet, curve elliptic.Curve) (crypto11.Signer, error) { + var id, label []byte + if v := public[crypto11.CkaId]; v != nil { + id = v.Value + } + if v := public[crypto11.CkaLabel]; v != nil { + label = v.Value + } + return s.GenerateECDSAKeyPairWithLabel(id, label, curve) +} + func (s *stubPKCS11) GenerateECDSAKeyPairWithLabel(id, label []byte, curve elliptic.Curve) (crypto11.Signer, error) { if id == nil && label == nil { return nil, errors.New("id and label cannot both be nil") diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 98b08c58..7a418e19 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -1,3 +1,4 @@ +//go:build cgo // +build cgo package pkcs11 @@ -32,11 +33,9 @@ const DefaultRSASize = 3072 type P11 interface { FindKeyPair(id, label []byte) (crypto11.Signer, error) FindCertificate(id, label []byte, serial *big.Int) (*x509.Certificate, error) - ImportCertificateWithLabel(id, label []byte, cert *x509.Certificate) error ImportCertificateWithAttributes(template crypto11.AttributeSet, certificate *x509.Certificate) error DeleteCertificate(id, label []byte, serial *big.Int) error - GenerateRSAKeyPairWithLabel(id, label []byte, bits int) (crypto11.SignerDecrypter, error) - GenerateECDSAKeyPairWithLabel(id, label []byte, curve elliptic.Curve) (crypto11.Signer, error) + GenerateRSAKeyPairWithAttributes(public, private crypto11.AttributeSet, bits int) (crypto11.SignerDecrypter, error) GenerateECDSAKeyPairWithAttributes(public, private crypto11.AttributeSet, curve elliptic.Curve) (crypto11.Signer, error) Close() error } @@ -188,6 +187,12 @@ func (k *PKCS11) StoreCertificate(req *apiv1.StoreCertificateRequest) error { return errors.Wrap(err, "storeCertificate failed") } + // Enforce the use of both id and labels. This is not strictly necessary in + // PKCS #11, but it's a good practice. + if len(id) == 0 || len(object) == 0 { + return errors.Errorf("key with uri %s is not valid, id and object are required", req.Name) + } + cert, err := k.p11.FindCertificate(id, object, nil) if err != nil { return errors.Wrap(err, "storeCertificate failed") @@ -198,31 +203,23 @@ func (k *PKCS11) StoreCertificate(req *apiv1.StoreCertificateRequest) error { }, "storeCertificate failed") } - if err := ImportCertificateWithLabel(k.p11, id, object, req.Certificate, req.Extractable); err != nil { + // Import certificate with the necessary attributes. + template, err := crypto11.NewAttributeSetWithIDAndLabel(id, object) + if err != nil { + return errors.Wrap(err, "storeCertificate failed") + } + if req.Extractable { + template.AddIfNotPresent([]*pkcs11.Attribute{ + pkcs11.NewAttribute(pkcs11.CKA_EXTRACTABLE, true), + }) + } + if err := k.p11.ImportCertificateWithAttributes(template, cert); err != nil { return errors.Wrap(err, "storeCertificate failed") } return nil } -func ImportCertificateWithLabel(ctx P11, id []byte, label []byte, certificate *x509.Certificate, extractable bool) error { - if id == nil { - return errors.New("id cannot be nil") - } - if label == nil { - return errors.New("label cannot be nil") - } - - template, err := crypto11.NewAttributeSetWithIDAndLabel(id, label) - if err != nil { - return err - } - template.AddIfNotPresent([]*pkcs11.Attribute{ - pkcs11.NewAttribute(pkcs11.CKA_EXTRACTABLE, extractable), - }) - return ctx.ImportCertificateWithAttributes(template, certificate) -} - // DeleteKey is a utility function to delete a key given an uri. func (k *PKCS11) DeleteKey(uri string) error { id, object, err := parseObject(uri) @@ -305,6 +302,18 @@ func generateKey(ctx P11, req *apiv1.CreateKeyRequest) (crypto11.Signer, error) return nil, errors.Errorf("key with uri %s is not valid, id and object are required", req.Name) } + // Create template for public and private keys + public, err := crypto11.NewAttributeSetWithIDAndLabel(id, object) + if err != nil { + return nil, err + } + private := public.Copy() + if req.Extractable { + private.AddIfNotPresent([]*pkcs11.Attribute{ + pkcs11.NewAttribute(pkcs11.CKA_EXTRACTABLE, true), + }) + } + bits := req.Bits if bits == 0 { bits = DefaultRSASize @@ -312,17 +321,17 @@ func generateKey(ctx P11, req *apiv1.CreateKeyRequest) (crypto11.Signer, error) switch req.SignatureAlgorithm { case apiv1.UnspecifiedSignAlgorithm: - return GenerateECDSAKeyPairWithLabel(ctx, id, object, elliptic.P256(), req.Extractable) + return ctx.GenerateECDSAKeyPairWithAttributes(public, private, elliptic.P256()) case apiv1.SHA256WithRSA, apiv1.SHA384WithRSA, apiv1.SHA512WithRSA: - return ctx.GenerateRSAKeyPairWithLabel(id, object, bits) + return ctx.GenerateRSAKeyPairWithAttributes(public, private, bits) case apiv1.SHA256WithRSAPSS, apiv1.SHA384WithRSAPSS, apiv1.SHA512WithRSAPSS: - return ctx.GenerateRSAKeyPairWithLabel(id, object, bits) + return ctx.GenerateRSAKeyPairWithAttributes(public, private, bits) case apiv1.ECDSAWithSHA256: - return GenerateECDSAKeyPairWithLabel(ctx, id, object, elliptic.P256(), req.Extractable) + return ctx.GenerateECDSAKeyPairWithAttributes(public, private, elliptic.P256()) case apiv1.ECDSAWithSHA384: - return GenerateECDSAKeyPairWithLabel(ctx, id, object, elliptic.P384(), req.Extractable) + return ctx.GenerateECDSAKeyPairWithAttributes(public, private, elliptic.P384()) case apiv1.ECDSAWithSHA512: - return GenerateECDSAKeyPairWithLabel(ctx, id, object, elliptic.P521(), req.Extractable) + return ctx.GenerateECDSAKeyPairWithAttributes(public, private, elliptic.P521()) case apiv1.PureEd25519: return nil, fmt.Errorf("signature algorithm %s is not supported", req.SignatureAlgorithm) default: