From 8366b7ddf197f741c783b1809afa7c2b76b8bd21 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 29 Oct 2021 14:45:10 -0700 Subject: [PATCH] Revert "Remove extractable from StoreCertificate." This reverts commit 614ee794896a46c89bebb5734f744fbc6729c0cc. --- cmd/step-pkcs11-init/main.go | 2 ++ kms/apiv1/requests.go | 14 ++++++++------ kms/pkcs11/pkcs11.go | 12 ++++++++++-- kms/pkcs11/pkcs11_test.go | 17 ++++++++++++++--- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/cmd/step-pkcs11-init/main.go b/cmd/step-pkcs11-init/main.go index df98aa8c..0db1e4d9 100644 --- a/cmd/step-pkcs11-init/main.go +++ b/cmd/step-pkcs11-init/main.go @@ -335,6 +335,7 @@ func createPKI(k kms.KeyManager, c Config) error { if err := cm.StoreCertificate(&apiv1.StoreCertificateRequest{ Name: c.RootObject, Certificate: root, + Extractable: c.Extractable, }); err != nil { return err } @@ -413,6 +414,7 @@ func createPKI(k kms.KeyManager, c Config) error { if err := cm.StoreCertificate(&apiv1.StoreCertificateRequest{ Name: c.CrtObject, Certificate: intermediate, + Extractable: c.Extractable, }); err != nil { return err } diff --git a/kms/apiv1/requests.go b/kms/apiv1/requests.go index c2d1d49a..94d832f9 100644 --- a/kms/apiv1/requests.go +++ b/kms/apiv1/requests.go @@ -100,7 +100,7 @@ type GetPublicKeyRequest struct { type CreateKeyRequest struct { // Name represents the key name or label used to identify a key. // - // Used by: awskms, cloudkms, azurekms, pkcs11, yubikey. + // Used by: awskms, cloudkms, pkcs11, yubikey. Name string // SignatureAlgorithm represents the type of key to create. @@ -110,13 +110,11 @@ type CreateKeyRequest struct { Bits int // ProtectionLevel specifies how cryptographic operations are performed. - // Used by: cloudkms, azurekms + // Used by: cloudkms ProtectionLevel ProtectionLevel - // Extractable defines if the new key may be exported from the HSM under a - // wrap key. On pkcs11 sets the CKA_EXTRACTABLE bit. - // - // Used by: pkcs11 + // Whether the key may be exported from the HSM under a wrap key. + // Sets the CKA_EXTRACTABLE bit. Extractable bool } @@ -158,4 +156,8 @@ type LoadCertificateRequest struct { type StoreCertificateRequest struct { Name string Certificate *x509.Certificate + + // Whether the key may be exported from the HSM under a wrap key. + // Sets the CKA_EXTRACTABLE bit. + Extractable bool } diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 3e14debe..cec05d33 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -32,7 +32,7 @@ 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, certificate *x509.Certificate) error + ImportCertificateWithAttributes(template crypto11.AttributeSet, certificate *x509.Certificate) error DeleteCertificate(id, label []byte, serial *big.Int) error GenerateRSAKeyPairWithAttributes(public, private crypto11.AttributeSet, bits int) (crypto11.SignerDecrypter, error) GenerateECDSAKeyPairWithAttributes(public, private crypto11.AttributeSet, curve elliptic.Curve) (crypto11.Signer, error) @@ -201,7 +201,15 @@ func (k *PKCS11) StoreCertificate(req *apiv1.StoreCertificateRequest) error { }, "storeCertificate failed") } - if err := k.p11.ImportCertificateWithLabel(id, object, req.Certificate); 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.Set(crypto11.CkaExtractable, true) + } + if err := k.p11.ImportCertificateWithAttributes(template, req.Certificate); err != nil { return errors.Wrap(err, "storeCertificate failed") } diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index dfb63753..409cfb3f 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -209,13 +209,13 @@ func TestPKCS11_CreateKey(t *testing.T) { }, }, false}, {"default extractable", args{&apiv1.CreateKeyRequest{ - Name: testObjectAlt, + Name: testObject, Extractable: true, }}, &apiv1.CreateKeyResponse{ - Name: testObjectAlt, + Name: testObject, PublicKey: &ecdsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: testObjectAlt, + SigningKey: testObject, }, }, false}, {"RSA SHA256WithRSA", args{&apiv1.CreateKeyRequest{ @@ -573,6 +573,7 @@ func TestPKCS11_StoreCertificate(t *testing.T) { // Make sure to delete the created certificate t.Cleanup(func() { k.DeleteCertificate(testObject) + k.DeleteCertificate(testObjectAlt) }) type args struct { @@ -587,6 +588,11 @@ func TestPKCS11_StoreCertificate(t *testing.T) { Name: testObject, Certificate: cert, }}, false}, + {"ok extractable", args{&apiv1.StoreCertificateRequest{ + Name: testObjectAlt, + Certificate: cert, + Extractable: true, + }}, false}, {"fail already exists", args{&apiv1.StoreCertificateRequest{ Name: testObject, Certificate: cert, @@ -614,6 +620,11 @@ func TestPKCS11_StoreCertificate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + if tt.args.req.Extractable { + if testModule == "SoftHSM2" { + t.Skip("Extractable certificates are not supported on SoftHSM2") + } + } if err := k.StoreCertificate(tt.args.req); (err != nil) != tt.wantErr { t.Errorf("PKCS11.StoreCertificate() error = %v, wantErr %v", err, tt.wantErr) }