Refactor pkcs#11 extractable certs and keys.

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:
+	case id == nil:
-		return errors.New("id and label cannot both be 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")

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)
+	GenerateRSAKeyPairWithAttributes(public, private crypto11.AttributeSet, bits int) (crypto11.SignerDecrypter, error)
-	GenerateECDSAKeyPairWithLabel(id, label []byte, curve elliptic.Curve) (crypto11.Signer, 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: