From be89459524a4d74edd7e2375ae98d00095bd55f5 Mon Sep 17 00:00:00 2001 From: Gary Belvin Date: Thu, 17 Jun 2021 09:06:35 -0400 Subject: [PATCH 1/9] Set key export bit --- cmd/step-pkcs11-init/main.go | 4 ++++ go.mod | 1 + kms/apiv1/requests.go | 4 ++++ kms/pkcs11/pkcs11.go | 24 ++++++++++++++++++++---- 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/cmd/step-pkcs11-init/main.go b/cmd/step-pkcs11-init/main.go index 34f9f8f8..b190c261 100644 --- a/cmd/step-pkcs11-init/main.go +++ b/cmd/step-pkcs11-init/main.go @@ -50,6 +50,7 @@ type Config struct { NoCerts bool EnableSSH bool Force bool + Extractable bool } // Validate checks the flags in the config. @@ -117,6 +118,7 @@ func main() { flag.BoolVar(&c.EnableSSH, "ssh", false, "Enable the creation of ssh keys.") flag.BoolVar(&c.NoCerts, "no-certs", false, "Do not store certificates in the module.") flag.BoolVar(&c.Force, "force", false, "Force the delete of previous keys.") + flag.BoolVar(&c.Extractable, "extractable", false, "Allow export of private keys under wrap.") flag.Usage = usage flag.Parse() @@ -286,6 +288,7 @@ func createPKI(k kms.KeyManager, c Config) error { resp, err := k.CreateKey(&apiv1.CreateKeyRequest{ Name: c.RootKeyObject, SignatureAlgorithm: apiv1.ECDSAWithSHA256, + Extractable: c.Extractable, }) if err != nil { return err @@ -366,6 +369,7 @@ func createPKI(k kms.KeyManager, c Config) error { resp, err := k.CreateKey(&apiv1.CreateKeyRequest{ Name: c.CrtKeyObject, SignatureAlgorithm: apiv1.ECDSAWithSHA256, + Extractable: c.Extractable, }) if err != nil { return err diff --git a/go.mod b/go.mod index ac3d54fb..74aa56e9 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/googleapis/gax-go/v2 v2.0.5 github.com/konsorten/go-windows-terminal-sequences v1.0.2 // indirect github.com/micromdm/scep/v2 v2.0.0 + github.com/miekg/pkcs11 v1.0.3-0.20190429190417-a667d056470f github.com/newrelic/go-agent v2.15.0+incompatible github.com/pkg/errors v0.9.1 github.com/rs/xid v1.2.1 diff --git a/kms/apiv1/requests.go b/kms/apiv1/requests.go index f6fe7dd2..321b308b 100644 --- a/kms/apiv1/requests.go +++ b/kms/apiv1/requests.go @@ -112,6 +112,10 @@ type CreateKeyRequest struct { // ProtectionLevel specifies how cryptographic operations are performed. // Used by: cloudkms ProtectionLevel ProtectionLevel + + // Whether the key may be exported from the HSM under a wrap key. + // Sets the CKA_EXTRACTABLE bit. + Extractable bool } // CreateKeyResponse is the response value of the kms.CreateKey method. diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 47c298a5..f6a86a0d 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -14,6 +14,7 @@ import ( "sync" "github.com/ThalesIgnite/crypto11" + "github.com/miekg/pkcs11" "github.com/pkg/errors" "github.com/smallstep/certificates/kms/apiv1" "github.com/smallstep/certificates/kms/uri" @@ -35,6 +36,7 @@ type P11 interface { 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) + GenerateECDSAKeyPairWithAttributes(public, private crypto11.AttributeSet, curve elliptic.Curve) (crypto11.Signer, error) Close() error } @@ -291,17 +293,17 @@ func generateKey(ctx P11, req *apiv1.CreateKeyRequest) (crypto11.Signer, error) switch req.SignatureAlgorithm { case apiv1.UnspecifiedSignAlgorithm: - return ctx.GenerateECDSAKeyPairWithLabel(id, object, elliptic.P256()) + return GenerateECDSAKeyPairWithLabel(ctx, id, object, elliptic.P256(), req.Extractable) case apiv1.SHA256WithRSA, apiv1.SHA384WithRSA, apiv1.SHA512WithRSA: return ctx.GenerateRSAKeyPairWithLabel(id, object, bits) case apiv1.SHA256WithRSAPSS, apiv1.SHA384WithRSAPSS, apiv1.SHA512WithRSAPSS: return ctx.GenerateRSAKeyPairWithLabel(id, object, bits) case apiv1.ECDSAWithSHA256: - return ctx.GenerateECDSAKeyPairWithLabel(id, object, elliptic.P256()) + return GenerateECDSAKeyPairWithLabel(ctx, id, object, elliptic.P256(), req.Extractable) case apiv1.ECDSAWithSHA384: - return ctx.GenerateECDSAKeyPairWithLabel(id, object, elliptic.P384()) + return GenerateECDSAKeyPairWithLabel(ctx, id, object, elliptic.P384(), req.Extractable) case apiv1.ECDSAWithSHA512: - return ctx.GenerateECDSAKeyPairWithLabel(id, object, elliptic.P521()) + return GenerateECDSAKeyPairWithLabel(ctx, id, object, elliptic.P521(), req.Extractable) case apiv1.PureEd25519: return nil, fmt.Errorf("signature algorithm %s is not supported", req.SignatureAlgorithm) default: @@ -309,6 +311,20 @@ func generateKey(ctx P11, req *apiv1.CreateKeyRequest) (crypto11.Signer, error) } } +func GenerateECDSAKeyPairWithLabel(ctx P11, id, label []byte, curve elliptic.Curve, extractable bool) (crypto11.Signer, error) { + public, err := crypto11.NewAttributeSetWithIDAndLabel(id, label) + if err != nil { + return nil, err + } + // Copy the AttributeSet to allow modifications. + private := public.Copy() + private.AddIfNotPresent([]*pkcs11.Attribute{ + pkcs11.NewAttribute(pkcs11.CKA_EXTRACTABLE, extractable), + }) + + return ctx.GenerateECDSAKeyPairWithAttributes(public, private, curve) +} + func findSigner(ctx P11, rawuri string) (crypto11.Signer, error) { id, object, err := parseObject(rawuri) if err != nil { From 22b471acf92035180fedb98a144c3db3261659ae Mon Sep 17 00:00:00 2001 From: Gary Belvin Date: Thu, 17 Jun 2021 09:23:22 -0400 Subject: [PATCH 2/9] Extractable certs --- cmd/step-pkcs11-init/main.go | 2 ++ kms/apiv1/requests.go | 4 ++++ kms/pkcs11/pkcs11.go | 21 ++++++++++++++++++++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/cmd/step-pkcs11-init/main.go b/cmd/step-pkcs11-init/main.go index b190c261..c7ac9b0f 100644 --- a/cmd/step-pkcs11-init/main.go +++ b/cmd/step-pkcs11-init/main.go @@ -328,6 +328,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 } @@ -406,6 +407,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 321b308b..94d832f9 100644 --- a/kms/apiv1/requests.go +++ b/kms/apiv1/requests.go @@ -156,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 f6a86a0d..98b08c58 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -33,6 +33,7 @@ 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) @@ -197,13 +198,31 @@ func (k *PKCS11) StoreCertificate(req *apiv1.StoreCertificateRequest) error { }, "storeCertificate failed") } - if err := k.p11.ImportCertificateWithLabel(id, object, req.Certificate); err != nil { + if err := ImportCertificateWithLabel(k.p11, id, object, req.Certificate, req.Extractable); 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) From 6be383da34046384a4a5fbd029f238bf119ca21b Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Oct 2021 18:04:11 -0700 Subject: [PATCH 3/9] 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: From 886b9a1d8db24970c5b0ca0a386229579785d4af Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Oct 2021 18:16:16 -0700 Subject: [PATCH 4/9] Store the certificate passed. --- kms/pkcs11/pkcs11.go | 2 +- kms/pkcs11/setup_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index a0c8cea6..cec05d33 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -209,7 +209,7 @@ func (k *PKCS11) StoreCertificate(req *apiv1.StoreCertificateRequest) error { if req.Extractable { template.Set(crypto11.CkaExtractable, true) } - if err := k.p11.ImportCertificateWithAttributes(template, cert); err != nil { + if err := k.p11.ImportCertificateWithAttributes(template, req.Certificate); err != nil { return errors.Wrap(err, "storeCertificate failed") } diff --git a/kms/pkcs11/setup_test.go b/kms/pkcs11/setup_test.go index 52dc5207..8aba2aaa 100644 --- a/kms/pkcs11/setup_test.go +++ b/kms/pkcs11/setup_test.go @@ -105,7 +105,7 @@ func setup(t TBTesting, k *PKCS11) { }); err != nil && !errors.Is(errors.Cause(err), apiv1.ErrAlreadyExists{ Message: c.Name + " already exists", }) { - t.Errorf("PKCS1.StoreCertificate() error = %+v", err) + t.Errorf("PKCS1.StoreCertificate() error = %v", err) continue } testCerts[i].Certificates = append(testCerts[i].Certificates, cert) From fa11e82b67378f8044d4a235e5fbe85934983e1f Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Oct 2021 19:45:19 -0700 Subject: [PATCH 5/9] Add tests with extractable property. --- kms/pkcs11/pkcs11_test.go | 29 +++++++++++++++++++++++++++-- kms/pkcs11/setup_test.go | 1 + 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index 6df9b92a..409cfb3f 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -208,6 +208,16 @@ func TestPKCS11_CreateKey(t *testing.T) { SigningKey: testObject, }, }, false}, + {"default extractable", args{&apiv1.CreateKeyRequest{ + Name: testObject, + Extractable: true, + }}, &apiv1.CreateKeyResponse{ + Name: testObject, + PublicKey: &ecdsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: testObject, + }, + }, false}, {"RSA SHA256WithRSA", args{&apiv1.CreateKeyRequest{ Name: testObject, SignatureAlgorithm: apiv1.SHA256WithRSA, @@ -563,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 { @@ -577,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, @@ -593,13 +609,22 @@ func TestPKCS11_StoreCertificate(t *testing.T) { Name: "http:id=7770;object=create-cert", Certificate: cert, }}, true}, - {"fail ImportCertificateWithLabel", args{&apiv1.StoreCertificateRequest{ - Name: "pkcs11:foo=bar", + {"fail missing id", args{&apiv1.StoreCertificateRequest{ + Name: "pkcs11:object=create-cert", + Certificate: cert, + }}, true}, + {"fail missing object", args{&apiv1.StoreCertificateRequest{ + Name: "pkcs11:id=7770;object=", Certificate: cert, }}, true}, } 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) } diff --git a/kms/pkcs11/setup_test.go b/kms/pkcs11/setup_test.go index 8aba2aaa..902d89ac 100644 --- a/kms/pkcs11/setup_test.go +++ b/kms/pkcs11/setup_test.go @@ -18,6 +18,7 @@ import ( var ( testModule = "" testObject = "pkcs11:id=7370;object=test-name" + testObjectAlt = "pkcs11:id=7377;object=alt-test-name" testObjectByID = "pkcs11:id=7370" testObjectByLabel = "pkcs11:object=test-name" testKeys = []struct { From 614ee794896a46c89bebb5734f744fbc6729c0cc Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 29 Oct 2021 12:02:24 -0700 Subject: [PATCH 6/9] Remove extractable from StoreCertificate. --- 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, 11 insertions(+), 34 deletions(-) diff --git a/cmd/step-pkcs11-init/main.go b/cmd/step-pkcs11-init/main.go index 0db1e4d9..df98aa8c 100644 --- a/cmd/step-pkcs11-init/main.go +++ b/cmd/step-pkcs11-init/main.go @@ -335,7 +335,6 @@ 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 } @@ -414,7 +413,6 @@ 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 94d832f9..c2d1d49a 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, pkcs11, yubikey. + // Used by: awskms, cloudkms, azurekms, pkcs11, yubikey. Name string // SignatureAlgorithm represents the type of key to create. @@ -110,11 +110,13 @@ type CreateKeyRequest struct { Bits int // ProtectionLevel specifies how cryptographic operations are performed. - // Used by: cloudkms + // Used by: cloudkms, azurekms ProtectionLevel ProtectionLevel - // Whether the key may be exported from the HSM under a wrap key. - // Sets the CKA_EXTRACTABLE bit. + // 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 Extractable bool } @@ -156,8 +158,4 @@ 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 cec05d33..3e14debe 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) - ImportCertificateWithAttributes(template crypto11.AttributeSet, certificate *x509.Certificate) error + ImportCertificateWithLabel(id, label []byte, 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,15 +201,7 @@ func (k *PKCS11) StoreCertificate(req *apiv1.StoreCertificateRequest) error { }, "storeCertificate failed") } - // 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 { + if err := k.p11.ImportCertificateWithLabel(id, object, 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 409cfb3f..dfb63753 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: testObject, + Name: testObjectAlt, Extractable: true, }}, &apiv1.CreateKeyResponse{ - Name: testObject, + Name: testObjectAlt, PublicKey: &ecdsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: testObject, + SigningKey: testObjectAlt, }, }, false}, {"RSA SHA256WithRSA", args{&apiv1.CreateKeyRequest{ @@ -573,7 +573,6 @@ 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 { @@ -588,11 +587,6 @@ 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, @@ -620,11 +614,6 @@ 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) } From 8366b7ddf197f741c783b1809afa7c2b76b8bd21 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 29 Oct 2021 14:45:10 -0700 Subject: [PATCH 7/9] 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) } From 7ec1424cb60e9afb5461a76638d28186a8a07a4e Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 29 Oct 2021 14:47:57 -0700 Subject: [PATCH 8/9] Fix help. --- kms/apiv1/requests.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/kms/apiv1/requests.go b/kms/apiv1/requests.go index 94d832f9..762d9dd8 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, pkcs11, yubikey. + // Used by: awskms, cloudkms, azurekms, pkcs11, yubikey. Name string // SignatureAlgorithm represents the type of key to create. @@ -110,11 +110,13 @@ type CreateKeyRequest struct { Bits int // ProtectionLevel specifies how cryptographic operations are performed. - // Used by: cloudkms + // Used by: cloudkms, azurekms. ProtectionLevel ProtectionLevel - // Whether the key may be exported from the HSM under a wrap key. - // Sets the CKA_EXTRACTABLE bit. + // 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 Extractable bool } @@ -157,7 +159,9 @@ 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 defines if the new certificate may be exported from the HSM + // under a wrap key. On pkcs11 sets the CKA_EXTRACTABLE bit. + // + // Used by: pkcs11 Extractable bool } From 91fb57e8aac9ce8057eb5894180653e64ae098e7 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 29 Oct 2021 15:09:53 -0700 Subject: [PATCH 9/9] Add entry to changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca792f55..a902e001 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased - 0.17.7] - DATE ### Added +- Support for generate extractable keys and certificates on a pkcs#11 module. ### Changed ### Deprecated ### Removed