From 8dca652bc72cbca1b9203ea49ad97db5e99b6161 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 26 Jan 2021 20:03:53 -0800 Subject: [PATCH 01/33] Add support for PKCS #11 KMS. The implementation works with YubiHSM2. Unit tests are still pending. Fixes #301 --- Makefile | 11 +- cmd/step-ca/main.go | 1 + cmd/step-pkcs11-init/main.go | 430 +++++++++++++++++++++++++++++++++++ go.mod | 2 + go.sum | 7 + kms/apiv1/options.go | 18 +- kms/apiv1/options_test.go | 2 +- kms/apiv1/requests.go | 11 +- kms/uri/uri.go | 130 ++++++++++- kms/uri/uri_test.go | 3 +- 10 files changed, 602 insertions(+), 13 deletions(-) create mode 100644 cmd/step-pkcs11-init/main.go diff --git a/Makefile b/Makefile index 9fb552d1..6f37e121 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,8 @@ AWSKMS_BINNAME?=step-awskms-init AWSKMS_PKG?=github.com/smallstep/certificates/cmd/step-awskms-init YUBIKEY_BINNAME?=step-yubikey-init YUBIKEY_PKG?=github.com/smallstep/certificates/cmd/step-yubikey-init +PKCS11_BINNAME?=step-pkcs11-init +PKCS11_PKG?=github.com/smallstep/certificates/cmd/step-pkcs11-init # Set V to 1 for verbose output from the Makefile Q=$(if $V,,@) @@ -76,7 +78,7 @@ GOFLAGS := CGO_ENABLED=0 download: $Q go mod download -build: $(PREFIX)bin/$(BINNAME) $(PREFIX)bin/$(CLOUDKMS_BINNAME) $(PREFIX)bin/$(AWSKMS_BINNAME) $(PREFIX)bin/$(YUBIKEY_BINNAME) +build: $(PREFIX)bin/$(BINNAME) $(PREFIX)bin/$(CLOUDKMS_BINNAME) $(PREFIX)bin/$(AWSKMS_BINNAME) $(PREFIX)bin/$(YUBIKEY_BINNAME) $(PREFIX)bin/$(PKCS11_BINNAME) @echo "Build Complete!" $(PREFIX)bin/$(BINNAME): download $(call rwildcard,*.go) @@ -95,6 +97,10 @@ $(PREFIX)bin/$(YUBIKEY_BINNAME): download $(call rwildcard,*.go) $Q mkdir -p $(@D) $Q $(GOOS_OVERRIDE) $(GOFLAGS) go build -v -o $(PREFIX)bin/$(YUBIKEY_BINNAME) $(LDFLAGS) $(YUBIKEY_PKG) +$(PREFIX)bin/$(PKCS11_BINNAME): download $(call rwildcard,*.go) + $Q mkdir -p $(@D) + $Q $(GOOS_OVERRIDE) $(GOFLAGS) go build -v -o $(PREFIX)bin/$(PKCS11_BINNAME) $(LDFLAGS) $(PKCS11_PKG) + # Target to force a build of step-ca without running tests simple: build @@ -171,6 +177,9 @@ endif ifneq ($(YUBIKEY_BINNAME),"") $Q rm -f bin/$(YUBIKEY_BINNAME) endif +ifneq ($(PKCS11_BINNAME),"") + $Q rm -f bin/$(PKCS11_BINNAME) +endif .PHONY: clean diff --git a/cmd/step-ca/main.go b/cmd/step-ca/main.go index 096935af..dad9cdbe 100644 --- a/cmd/step-ca/main.go +++ b/cmd/step-ca/main.go @@ -31,6 +31,7 @@ import ( _ "github.com/smallstep/certificates/kms/sshagentkms" // Experimental kms interfaces. + _ "github.com/smallstep/certificates/kms/pkcs11" _ "github.com/smallstep/certificates/kms/yubikey" // Enabled cas interfaces. diff --git a/cmd/step-pkcs11-init/main.go b/cmd/step-pkcs11-init/main.go new file mode 100644 index 00000000..174eed78 --- /dev/null +++ b/cmd/step-pkcs11-init/main.go @@ -0,0 +1,430 @@ +package main + +import ( + "context" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/sha1" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "flag" + "fmt" + "math/big" + "os" + "runtime" + "time" + + "github.com/pkg/errors" + "github.com/smallstep/certificates/kms" + "github.com/smallstep/certificates/kms/apiv1" + "github.com/smallstep/certificates/kms/uri" + "go.step.sm/cli-utils/fileutil" + "go.step.sm/cli-utils/ui" + "go.step.sm/crypto/pemutil" + + // Enable pkcs11. + _ "github.com/smallstep/certificates/kms/pkcs11" +) + +// Config is a mapping of the cli flags. +type Config struct { + KMS string + RootOnly bool + RootObject string + RootKeyObject string + CrtObject string + CrtKeyObject string + SSHHostKeyObject string + SSHUserKeyObject string + RootFile string + KeyFile string + Pin string + EnableSSH bool + Force bool +} + +// Validate checks the flags in the config. +func (c *Config) Validate() error { + switch { + case c.KMS == "": + return errors.New("one of flag `--kms` is required") + case c.RootFile != "" && c.KeyFile == "": + return errors.New("flag `--root` requires flag `--key`") + case c.KeyFile != "" && c.RootFile == "": + return errors.New("flag `--key` requires flag `--root`") + case c.RootOnly && c.RootFile != "": + return errors.New("flag `--root-only` is incompatible with flag `--root`") + case c.RootFile == "" && c.RootObject == "": + return errors.New("one of flag `--root` or `--root-cert` is required") + case c.RootFile == "" && c.RootKeyObject == "": + return errors.New("one of flag `--root` or `--root-key` is required") + default: + if c.RootFile != "" { + c.RootObject = "" + c.RootKeyObject = "" + } + if c.RootOnly { + c.CrtObject = "" + c.CrtKeyObject = "" + } + if !c.EnableSSH { + c.SSHHostKeyObject = "" + c.SSHUserKeyObject = "" + } + return nil + } +} + +func main() { + var kmsuri string + switch runtime.GOOS { + case "darwin": + kmsuri = "pkcs11:module-path=/usr/local/lib/pkcs11/yubihsm_pkcs11.dylib;token=YubiHSM" + case "linux": + kmsuri = "pkcs11:module-path=/usr/lib/x86_64-linux-gnu/pkcs11/yubihsm_pkcs11.so;token=YubiHSM" + case "windows": + if home, err := os.UserHomeDir(); err == nil { + kmsuri = "pkcs11:module-path=" + home + "\\yubihsm2-sdk\\bin\\yubihsm_pkcs11.dll" + ";token=YubiHSM" + } + default: + + } + + var c Config + flag.StringVar(&c.KMS, "kms", kmsuri, "PKCS #11 URI with the module-path and token to connect to the module.") + flag.StringVar(&c.RootObject, "root-cert", "pkcs11:id=7330;object=root-cert", "PKCS #11 URI with object id and label to store the root certificate.") + flag.StringVar(&c.RootKeyObject, "root-key", "pkcs11:id=7330;object=root-key", "PKCS #11 URI with object id and label to store the root key.") + flag.StringVar(&c.CrtObject, "crt-cert", "pkcs11:id=7331;object=intermediate-cert", "PKCS #11 URI with object id and label to store the intermediate certificate.") + flag.StringVar(&c.CrtKeyObject, "crt-key", "pkcs11:id=7331;object=intermediate-key", "PKCS #11 URI with object id and label to store the intermediate certificate.") + flag.StringVar(&c.SSHHostKeyObject, "ssh-host-key", "pkcs11:id=7332;object=ssh-host-key", "PKCS #11 URI with object id and label to store the key used to sign SSH host certificates.") + flag.StringVar(&c.SSHUserKeyObject, "ssh-user-key", "pkcs11:id=7333;object=ssh-user-key", "PKCS #11 URI with object id and label to store the key used to sign SSH user certificates.") + flag.BoolVar(&c.RootOnly, "root-only", false, "Store only only the root certificate and sign and intermediate.") + flag.StringVar(&c.RootFile, "root", "", "Path to the root certificate to use.") + flag.StringVar(&c.KeyFile, "key", "", "Path to the root key to use.") + flag.BoolVar(&c.EnableSSH, "ssh", false, "Enable the creation of ssh keys.") + flag.BoolVar(&c.Force, "force", false, "Force the delete of previous keys.") + flag.Usage = usage + flag.Parse() + + if err := c.Validate(); err != nil { + fatal(err) + } + + u, err := uri.ParseWithScheme("pkcs11", c.KMS) + if err != nil { + fatal(err) + } + + if u.Pin() == "" { + pin, err := ui.PromptPassword("What is the PKCS#11 PIN?") + if err != nil { + fatal(err) + } + c.Pin = string(pin) + } + + k, err := kms.New(context.Background(), apiv1.Options{ + Type: string(apiv1.PKCS11), + URI: c.KMS, + Pin: c.Pin, + }) + if err != nil { + fatal(err) + } + + // Check if the slots are empty, fail if they are not + certUris := []string{ + c.RootObject, c.CrtObject, + } + keyUris := []string{ + c.RootKeyObject, c.CrtKeyObject, + c.SSHHostKeyObject, c.SSHUserKeyObject, + } + if !c.Force { + for _, u := range certUris { + if u != "" { + checkObject(k, u) + } + } + for _, u := range keyUris { + if u != "" { + checkObject(k, u) + } + } + } else { + deleter, ok := k.(interface { + DeleteKey(uri string) error + DeleteCertificate(uri string) error + }) + if ok { + for _, u := range certUris { + if u != "" { + if err := deleter.DeleteCertificate(u); err != nil { + fatal(err) + } + } + } + for _, u := range keyUris { + if u != "" { + if err := deleter.DeleteKey(u); err != nil { + fatal(err) + } + } + } + } + } + + if err := createPKI(k, c); err != nil { + fatal(err) + } + + defer func() { + _ = k.Close() + }() +} + +func fatal(err error) { + if os.Getenv("STEPDEBUG") == "1" { + fmt.Fprintf(os.Stderr, "%+v\n", err) + } else { + fmt.Fprintln(os.Stderr, err) + } + os.Exit(1) +} + +func usage() { + fmt.Fprintln(os.Stderr, "Usage: step-pkcs11-init") + fmt.Fprintln(os.Stderr, ` +The step-pkcs11-init command initializes a public key infrastructure (PKI) +to be used by step-ca. + +This tool is experimental and in the future it will be integrated in step cli. + +OPTIONS`) + fmt.Fprintln(os.Stderr) + flag.PrintDefaults() + fmt.Fprintln(os.Stderr, ` +COPYRIGHT + + (c) 2018-2021 Smallstep Labs, Inc.`) + os.Exit(1) +} + +func checkObject(k kms.KeyManager, rawuri string) { + if _, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ + Name: rawuri, + }); err == nil { + fmt.Fprintf(os.Stderr, "⚠️ Your PKCS #11 module already has a key on %s.\n", rawuri) + fmt.Fprintln(os.Stderr, " If you want to delete it and start fresh, use `--force`.") + os.Exit(1) + } +} + +func createPKI(k kms.KeyManager, c Config) error { + var err error + ui.Println("Creating PKI ...") + now := time.Now() + + // Root Certificate + var signer crypto.Signer + var root *x509.Certificate + if c.RootFile != "" && c.KeyFile != "" { + root, err = pemutil.ReadCertificate(c.RootFile) + if err != nil { + return err + } + + key, err := pemutil.Read(c.KeyFile) + if err != nil { + return err + } + + var ok bool + if signer, ok = key.(crypto.Signer); !ok { + return errors.Errorf("key type '%T' does not implement a signer", key) + } + } else { + resp, err := k.CreateKey(&apiv1.CreateKeyRequest{ + Name: c.RootKeyObject, + SignatureAlgorithm: apiv1.ECDSAWithSHA256, + }) + if err != nil { + return err + } + + signer, err = k.CreateSigner(&resp.CreateSignerRequest) + if err != nil { + return err + } + + template := &x509.Certificate{ + IsCA: true, + NotBefore: now, + NotAfter: now.Add(time.Hour * 24 * 365 * 10), + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + BasicConstraintsValid: true, + MaxPathLen: 1, + MaxPathLenZero: false, + Issuer: pkix.Name{CommonName: "PKCS #11 Smallstep Root"}, + Subject: pkix.Name{CommonName: "PKCS #11 Smallstep Root"}, + SerialNumber: mustSerialNumber(), + SubjectKeyId: mustSubjectKeyID(resp.PublicKey), + AuthorityKeyId: mustSubjectKeyID(resp.PublicKey), + } + + b, err := x509.CreateCertificate(rand.Reader, template, template, resp.PublicKey, signer) + if err != nil { + return err + } + + root, err = x509.ParseCertificate(b) + if err != nil { + return errors.Wrap(err, "error parsing root certificate") + } + + if cm, ok := k.(kms.CertificateManager); ok { + if err = cm.StoreCertificate(&apiv1.StoreCertificateRequest{ + Name: c.RootObject, + Certificate: root, + }); err != nil { + return err + } + } + + if err = fileutil.WriteFile("root_ca.crt", pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: b, + }), 0600); err != nil { + return err + } + + ui.PrintSelected("Root Key", resp.Name) + ui.PrintSelected("Root Certificate", "root_ca.crt") + } + + // Intermediate Certificate + var keyName string + var publicKey crypto.PublicKey + if c.RootOnly { + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + return errors.Wrap(err, "error creating intermediate key") + } + + pass, err := ui.PromptPasswordGenerate("What do you want your password to be? [leave empty and we'll generate one]", + ui.WithRichPrompt()) + if err != nil { + return err + } + + _, err = pemutil.Serialize(priv, pemutil.WithPassword(pass), pemutil.ToFile("intermediate_ca_key", 0600)) + if err != nil { + return err + } + + publicKey = priv.Public() + } else { + resp, err := k.CreateKey(&apiv1.CreateKeyRequest{ + Name: c.CrtKeyObject, + SignatureAlgorithm: apiv1.ECDSAWithSHA256, + }) + if err != nil { + return err + } + publicKey = resp.PublicKey + keyName = resp.Name + } + + template := &x509.Certificate{ + IsCA: true, + NotBefore: now, + NotAfter: now.Add(time.Hour * 24 * 365 * 10), + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + BasicConstraintsValid: true, + MaxPathLen: 0, + MaxPathLenZero: true, + Issuer: root.Subject, + Subject: pkix.Name{CommonName: "YubiKey Smallstep Intermediate"}, + SerialNumber: mustSerialNumber(), + SubjectKeyId: mustSubjectKeyID(publicKey), + } + + b, err := x509.CreateCertificate(rand.Reader, template, root, publicKey, signer) + if err != nil { + return err + } + + intermediate, err := x509.ParseCertificate(b) + if err != nil { + return errors.Wrap(err, "error parsing intermediate certificate") + } + + if cm, ok := k.(kms.CertificateManager); ok { + if err = cm.StoreCertificate(&apiv1.StoreCertificateRequest{ + Name: c.CrtObject, + Certificate: intermediate, + }); err != nil { + return err + } + } + + if err = fileutil.WriteFile("intermediate_ca.crt", pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: b, + }), 0600); err != nil { + return err + } + + if c.RootOnly { + ui.PrintSelected("Intermediate Key", "intermediate_ca_key") + } else { + ui.PrintSelected("Intermediate Key", keyName) + } + + ui.PrintSelected("Intermediate Certificate", "intermediate_ca.crt") + + if c.SSHHostKeyObject != "" { + resp, err := k.CreateKey(&apiv1.CreateKeyRequest{ + Name: c.SSHHostKeyObject, + SignatureAlgorithm: apiv1.ECDSAWithSHA256, + }) + if err != nil { + return err + } + ui.PrintSelected("SSH Host Key", resp.Name) + } + + if c.SSHUserKeyObject != "" { + resp, err := k.CreateKey(&apiv1.CreateKeyRequest{ + Name: c.SSHUserKeyObject, + SignatureAlgorithm: apiv1.ECDSAWithSHA256, + }) + if err != nil { + return err + } + ui.PrintSelected("SSH User Key", resp.Name) + } + + return nil +} + +func mustSerialNumber() *big.Int { + serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) + sn, err := rand.Int(rand.Reader, serialNumberLimit) + if err != nil { + panic(err) + } + return sn +} + +func mustSubjectKeyID(key crypto.PublicKey) []byte { + b, err := x509.MarshalPKIXPublicKey(key) + if err != nil { + panic(err) + } + hash := sha1.Sum(b) + return hash[:] +} diff --git a/go.mod b/go.mod index 74873ea8..33782eb9 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.14 require ( cloud.google.com/go v0.70.0 github.com/Masterminds/sprig/v3 v3.1.0 + github.com/ThalesIgnite/crypto11 v1.2.3 github.com/aws/aws-sdk-go v1.30.29 github.com/go-chi/chi v4.0.2+incompatible github.com/go-piv/piv-go v1.7.0 @@ -33,3 +34,4 @@ require ( // replace github.com/smallstep/nosql => ../nosql // replace go.step.sm/crypto => ../crypto // replace github.com/smallstep/nosql => ../nosql +replace github.com/ThalesIgnite/crypto11 => github.com/maraino/crypto11 v1.2.4-0.20210127032225-7ed5319b45a1 diff --git a/go.sum b/go.sum index f4a18362..ae64c5f4 100644 --- a/go.sum +++ b/go.sum @@ -214,6 +214,8 @@ github.com/lunixbochs/vtclean v1.0.0/go.mod h1:pHhQNgMf3btfWnGBVipUOjRYhoOsdGqdm github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/manifoldco/promptui v0.8.0 h1:R95mMF+McvXZQ7j1g8ucVZE1gLP3Sv6j9vlF9kyRqQo= github.com/manifoldco/promptui v0.8.0/go.mod h1:n4zTdgP0vr0S3w7/O/g98U+e0gwLScEXGwov2nIKuGQ= +github.com/maraino/crypto11 v1.2.4-0.20210127032225-7ed5319b45a1 h1:aj2ASiF6u9p576GdvGsRW5SiwiE/Hp5BeU2c/ldlYTI= +github.com/maraino/crypto11 v1.2.4-0.20210127032225-7ed5319b45a1/go.mod h1:ILDKtnCKiQ7zRoNxcp36Y1ZR8LBPmR2E23+wTQe/MlE= github.com/mattn/go-colorable v0.0.9 h1:UVL0vNpWh04HeJXV0KLcaT7r06gOH2l4OW6ddYRUIY4= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.4 h1:snbPLB8fVfU9iwbbo30TPtbLRzwWu6aJS6Xh4eaaviA= @@ -222,6 +224,8 @@ github.com/mattn/go-isatty v0.0.4/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNx github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= github.com/mattn/go-isatty v0.0.11 h1:FxPOTFNqGkuDUGi3H/qkUbQO4ZiBa2brKq5r0l8TGeM= github.com/mattn/go-isatty v0.0.11/go.mod h1:PhnuNfih5lzO57/f3n+odYbM4JtupLOxQOAqxQCu2WE= +github.com/miekg/pkcs11 v1.0.3-0.20190429190417-a667d056470f h1:eVB9ELsoq5ouItQBr5Tj334bhPJG/MX+m7rTchmzVUQ= +github.com/miekg/pkcs11 v1.0.3-0.20190429190417-a667d056470f/go.mod h1:XsNlhZGX73bx86s2hdc/FuaLm2CPZJemRLMA+WTFxgs= github.com/mitchellh/copystructure v1.0.0 h1:Laisrj+bAB6b/yJwB5Bt3ITZhGJdqmxquMKeZ+mmkFQ= github.com/mitchellh/copystructure v1.0.0/go.mod h1:SNtv71yrdKgLRyLFxmLdkAbkKEFWgYaq1OVrnRcwhnw= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= @@ -273,9 +277,12 @@ github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DM github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1 h1:nOGnQDM7FYENwehXlg/kFVnos3rEvtKTjRvOWSzb6H4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +github.com/thales-e-security/pool v0.0.2 h1:RAPs4q2EbWsTit6tpzuvTFlgFRJ3S8Evf5gtvVDbmPg= +github.com/thales-e-security/pool v0.0.2/go.mod h1:qtpMm2+thHtqhLzTwgDBj/OuNnMpupY8mv0Phz0gjhU= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/urfave/cli v1.22.2 h1:gsqYFH8bb9ekPA12kRo0hfjngWQjkJPlN9R0N78BoUo= github.com/urfave/cli v1.22.2/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0= diff --git a/kms/apiv1/options.go b/kms/apiv1/options.go index b7845109..07d7df0d 100644 --- a/kms/apiv1/options.go +++ b/kms/apiv1/options.go @@ -64,10 +64,15 @@ type Options struct { // Path to the credentials file used in CloudKMS and AmazonKMS. CredentialsFile string `json:"credentialsFile"` - // Path to the module used with PKCS11 KMS. - Module string `json:"module"` + // URI is based on the PKCS #11 URI Scheme defined in + // https://tools.ietf.org/html/rfc7512 and represents the configuration used + // to connect to the KMS. + // + // Used by: pkcs11 + URI string `json:"uri"` - // Pin used to access the PKCS11 module. + // Pin used to access the PKCS11 module. It can be defined in the URI using + // the pin-value or pin-source properties. Pin string `json:"pin"` // ManagementKey used in YubiKeys. Default management key is the hexadecimal @@ -93,10 +98,9 @@ func (o *Options) Validate() error { } switch Type(strings.ToLower(o.Type)) { - case DefaultKMS, SoftKMS, CloudKMS, AmazonKMS, SSHAgentKMS: - case YubiKey: - case PKCS11: - return ErrNotImplemented{"support for PKCS11 is not yet implemented"} + case DefaultKMS, SoftKMS: // Go crypto based kms. + case CloudKMS, AmazonKMS, SSHAgentKMS: // Cloud based kms. + case YubiKey, PKCS11: // Hardware based kms. default: return errors.Errorf("unsupported kms type %s", o.Type) } diff --git a/kms/apiv1/options_test.go b/kms/apiv1/options_test.go index 150dd17b..5a8307eb 100644 --- a/kms/apiv1/options_test.go +++ b/kms/apiv1/options_test.go @@ -15,7 +15,7 @@ func TestOptions_Validate(t *testing.T) { {"cloudkms", &Options{Type: "cloudkms"}, false}, {"awskms", &Options{Type: "awskms"}, false}, {"sshagentkms", &Options{Type: "sshagentkms"}, false}, - {"pkcs11", &Options{Type: "pkcs11"}, true}, + {"pkcs11", &Options{Type: "pkcs11"}, false}, {"unsupported", &Options{Type: "unsupported"}, true}, } for _, tt := range tests { diff --git a/kms/apiv1/requests.go b/kms/apiv1/requests.go index bbee4cfc..e58c4546 100644 --- a/kms/apiv1/requests.go +++ b/kms/apiv1/requests.go @@ -98,9 +98,16 @@ type GetPublicKeyRequest struct { // CreateKeyRequest is the parameter used in the kms.CreateKey method. type CreateKeyRequest struct { - Name string + // Name represents the key name or label used to identify a key. + // + // Used by: awskms, cloudkms, pkcs11, yubikey. + Name string + + // SignatureAlgorithm represents the type of key to create. SignatureAlgorithm SignatureAlgorithm - Bits int + + // Bits is the number of bits on RSA keys. + Bits int // ProtectionLevel specifies how cryptographic operations are performed. // Used by: cloudkms diff --git a/kms/uri/uri.go b/kms/uri/uri.go index 02bec42c..85d512db 100644 --- a/kms/uri/uri.go +++ b/kms/uri/uri.go @@ -1,8 +1,12 @@ package uri import ( + "bytes" + "encoding/hex" + "io/ioutil" "net/url" "strings" + "unicode" "github.com/pkg/errors" ) @@ -82,5 +86,129 @@ func ParseWithScheme(scheme, rawuri string) (*URI, error) { // Get returns the first value in the uri with the give n key, it will return // empty string if that field is not present. func (u *URI) Get(key string) string { - return u.Values.Get(key) + v := u.Values.Get(key) + if v == "" { + v = u.URL.Query().Get(key) + } + return StringDecode(v) +} + +// GetHex returns the first value in the uri with the give n key, it will return +// empty nil if that field is not present. +func (u *URI) GetHex(key string) ([]byte, error) { + v := u.Values.Get(key) + if v == "" { + v = u.URL.Query().Get(key) + } + return HexDecode(v) +} + +// Pin returns the pin encoded in the url. It will read the pin from the +// pin-value or the pin-source attributes. +func (u *URI) Pin() string { + if value := u.Get("pin-value"); value != "" { + return StringDecode(value) + } + if path := u.Get("pin-source"); path != "" { + if b, err := readFile(path); err == nil { + return string(bytes.TrimRightFunc(b, unicode.IsSpace)) + } + } + return "" +} + +func readFile(path string) ([]byte, error) { + u, err := url.Parse(path) + if err == nil && (u.Scheme == "" || u.Scheme == "file") && u.Path != "" { + path = u.Path + } + b, err := ioutil.ReadFile(path) + if err != nil { + return nil, errors.Wrapf(err, "error reading %s", path) + } + return b, nil +} + +// PercentEncode encodes the given bytes using the percent encoding described in +// RFC3986 (https://tools.ietf.org/html/rfc3986). +func PercentEncode(b []byte) string { + buf := new(strings.Builder) + for _, v := range b { + buf.WriteString("%" + hex.EncodeToString([]byte{v})) + } + return buf.String() +} + +// PercentDecode decodes the given string using the percent encoding described +// in RFC3986 (https://tools.ietf.org/html/rfc3986). +func PercentDecode(s string) ([]byte, error) { + if len(s)%3 != 0 { + return nil, errors.Errorf("error parsing %s: wrong length", s) + } + + var first string + buf := new(bytes.Buffer) + for i, r := range s { + mod := i % 3 + rr := string(r) + switch mod { + case 0: + if r != '%' { + return nil, errors.Errorf("error parsing %s: expected %% and found %s in position %d", s, rr, i) + } + case 1: + if !isHex(r) { + return nil, errors.Errorf("error parsing %s: %s in position %d is not an hexadecimal number", s, rr, i) + } + first = string(r) + case 2: + if !isHex(r) { + return nil, errors.Errorf("error parsing %s: %s in position %d is not an hexadecimal number", s, rr, i) + } + b, err := hex.DecodeString(first + rr) + if err != nil { + return nil, errors.Wrapf(err, "error parsing %s", s) + } + buf.Write(b) + } + } + return buf.Bytes(), nil +} + +// StringDecode returns the string given, but it will use Percent-Encoding if +// the string is percent encoded. +func StringDecode(s string) string { + if strings.HasPrefix(s, "%") { + if b, err := PercentDecode(s); err == nil { + return string(b) + } + } + return s +} + +// HexDecode deocdes the string s using Percent-Encoding or regular hex +// encoding. +func HexDecode(s string) ([]byte, error) { + if strings.HasPrefix(s, "%") { + return PercentDecode(s) + } + + b, err := hex.DecodeString(s) + if err != nil { + return nil, errors.Wrapf(err, "error parsing %s", s) + } + return b, nil +} + +func isHex(r rune) bool { + switch { + case r >= '0' && r <= '9': + return true + case r >= 'a' && r <= 'f': + return true + case r >= 'A' && r <= 'F': + return true + default: + return false + } } diff --git a/kms/uri/uri_test.go b/kms/uri/uri_test.go index a2b69b65..3f001afc 100644 --- a/kms/uri/uri_test.go +++ b/kms/uri/uri_test.go @@ -189,8 +189,9 @@ func TestURI_Get(t *testing.T) { {"ok", mustParse("yubikey:slot-id=9a"), args{"slot-id"}, "9a"}, {"ok first", mustParse("yubikey:slot-id=9a;slot-id=9b"), args{"slot-id"}, "9a"}, {"ok multiple", mustParse("yubikey:slot-id=9a;foo=bar"), args{"foo"}, "bar"}, + {"ok in query", mustParse("yubikey:slot-id=9a?foo=bar"), args{"foo"}, "bar"}, {"fail missing", mustParse("yubikey:slot-id=9a"), args{"foo"}, ""}, - {"fail in query", mustParse("yubikey:slot-id=9a?foo=bar"), args{"foo"}, ""}, + {"fail missing query", mustParse("yubikey:slot-id=9a?bar=zar"), args{"foo"}, ""}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From a6c784d5dd2251fe750036db5b17dd2d6a9e2867 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 26 Jan 2021 20:15:57 -0800 Subject: [PATCH 02/33] Add missing pkcs11 package. --- kms/pkcs11/pkcs11.go | 305 ++++++++++++++++++++++++++++++++++++ kms/pkcs11/pkcs11_no_cgo.go | 19 +++ kms/pkcs11/pkcs11_test.go | 3 + 3 files changed, 327 insertions(+) create mode 100644 kms/pkcs11/pkcs11.go create mode 100644 kms/pkcs11/pkcs11_no_cgo.go create mode 100644 kms/pkcs11/pkcs11_test.go diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go new file mode 100644 index 00000000..d0a429f8 --- /dev/null +++ b/kms/pkcs11/pkcs11.go @@ -0,0 +1,305 @@ +// +build cgo + +package pkcs11 + +import ( + "context" + "crypto" + "crypto/elliptic" + "crypto/x509" + "encoding/hex" + "fmt" + "math/big" + + "github.com/ThalesIgnite/crypto11" + "github.com/pkg/errors" + "github.com/smallstep/certificates/kms/apiv1" + "github.com/smallstep/certificates/kms/uri" +) + +// DefaultRSASize is the number of bits of a new RSA key if not bitsize has been +// specified. +const DefaultRSASize = 3072 + +// PKCS11 is the implementation of a KMS using the PKCS #11 standard. +type PKCS11 struct { + context *crypto11.Context +} + +// New returns a new PKCS11 KMS. +func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { + var config crypto11.Config + if opts.URI != "" { + u, err := uri.ParseWithScheme("pkcs11", opts.URI) + if err != nil { + return nil, err + } + config.Path = u.Get("module-path") + config.TokenLabel = u.Get("token") + config.TokenSerial = u.Get("serial") + config.Pin = u.Pin() + } + if config.Pin == "" && opts.Pin != "" { + config.Pin = opts.Pin + } + + switch { + case config.Path == "": + return nil, errors.New("kms uri 'module-path' are required") + case config.TokenLabel == "" && config.TokenSerial == "": + return nil, errors.New("kms uri 'token' or 'serial' are required") + case config.Pin == "": + return nil, errors.New("kms 'pin' cannot be empty") + case config.TokenLabel != "" && config.TokenSerial != "": + return nil, errors.New("kms uri 'token' or 'serial' are mutually exclusive") + } + + p11Ctx, err := crypto11.Configure(&config) + if err != nil { + return nil, errors.Wrap(err, "error initializing PKCS#11") + } + + return &PKCS11{ + context: p11Ctx, + }, nil +} + +func init() { + apiv1.Register(apiv1.PKCS11, func(ctx context.Context, opts apiv1.Options) (apiv1.KeyManager, error) { + return New(ctx, opts) + }) +} + +// GetPublicKey returns the public key .... +func (k *PKCS11) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKey, error) { + if req.Name == "" { + return nil, errors.New("getPublicKeyRequest 'name' cannot be empty") + } + + signer, err := findSigner(k.context, req.Name) + if err != nil { + return nil, errors.Wrap(err, "getPublicKey failed") + } + + return signer.Public(), nil +} + +// CreateKey generates a new key in the PKCS#11 module and returns the public key. +func (k *PKCS11) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyResponse, error) { + switch { + case req.Name == "": + return nil, errors.New("createKeyRequest 'name' cannot be empty") + case req.Bits < 0: + return nil, errors.New("createKeyRequest 'bits' cannot be negative") + } + + signer, err := generateKey(k.context, req) + if err != nil { + return nil, errors.Wrap(err, "createKey failed") + } + + return &apiv1.CreateKeyResponse{ + Name: req.Name, + PublicKey: signer.Public(), + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: req.Name, + }, + }, nil +} + +// CreateSigner creates a signer using the key present in the PKCS#11 MODULE signature +// slot. +func (k *PKCS11) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, error) { + switch { + case req.SigningKey == "": + return nil, errors.New("createSignerRequest 'signingKey' cannot be empty") + } + + signer, err := findSigner(k.context, req.SigningKey) + if err != nil { + return nil, errors.Wrap(err, "createSigner failed") + } + + return signer, nil +} + +// LoadCertificate implements kms.CertificateManager and loads a certificate +// from the YubiKey. +func (k *PKCS11) LoadCertificate(req *apiv1.LoadCertificateRequest) (*x509.Certificate, error) { + if req.Name == "" { + return nil, errors.New("loadCertificateRequest 'name' cannot be nil") + } + cert, err := findCertificate(k.context, req.Name) + if err != nil { + return nil, errors.Wrap(err, "loadCertificate failed") + } + return cert, nil +} + +// StoreCertificate implements kms.CertificateManager and stores a certificate +// in the YubiKey. +func (k *PKCS11) StoreCertificate(req *apiv1.StoreCertificateRequest) error { + switch { + case req.Name == "": + return errors.New("storeCertificateRequest 'name' cannot be empty") + case req.Certificate == nil: + return errors.New("storeCertificateRequest 'Certificate' cannot be nil") + } + + id, object, err := parseObject(req.Name) + if err != nil { + return errors.Wrap(err, "storeCertificate failed") + } + + if err := k.context.ImportCertificateWithLabel(id, object, req.Certificate); err != nil { + return errors.Wrap(err, "storeCertificate failed") + } + + return nil +} + +// DeleteKey is a utility function to delete a key given an uri. +func (k *PKCS11) DeleteKey(uri string) error { + id, object, err := parseObject(uri) + if err != nil { + return errors.Wrap(err, "deleteKey failed") + } + signer, err := k.context.FindKeyPair(id, object) + if err != nil { + return errors.Wrap(err, "deleteKey failed") + } + if signer == nil { + return nil + } + if err := signer.Delete(); err != nil { + return errors.Wrap(err, "deleteKey failed") + } + return nil +} + +// DeleteCertificate is a utility function to delete a certificate given an uri. +func (k *PKCS11) DeleteCertificate(uri string) error { + id, object, err := parseObject(uri) + if err != nil { + return errors.Wrap(err, "deleteCertificate failed") + } + if err := k.context.DeleteCertificate(id, object, nil); err != nil { + return errors.Wrap(err, "deleteCertificate failed") + } + return nil +} + +// Close releases the connection to the PKCS#11 module. +func (k *PKCS11) Close() error { + return errors.Wrap(k.context.Close(), "error closing pkcs#11 context") +} + +func toByte(s string) []byte { + if s == "" { + return nil + } + return []byte(s) +} + +func generateKey(ctx *crypto11.Context, req *apiv1.CreateKeyRequest) (crypto11.Signer, error) { + id, object, err := parseObject(req.Name) + if err != nil { + return nil, err + } + signer, err := ctx.FindKeyPair(id, object) + if err != nil { + return nil, err + } + if signer != nil { + return nil, errors.Errorf("%s already exists", req.Name) + } + + bits := req.Bits + if bits == 0 { + bits = DefaultRSASize + } + + switch req.SignatureAlgorithm { + case apiv1.UnspecifiedSignAlgorithm: + return ctx.GenerateECDSAKeyPairWithLabel(id, object, elliptic.P256()) + 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()) + case apiv1.ECDSAWithSHA384: + return ctx.GenerateECDSAKeyPairWithLabel(id, object, elliptic.P384()) + case apiv1.ECDSAWithSHA512: + return ctx.GenerateECDSAKeyPairWithLabel(id, object, elliptic.P521()) + case apiv1.PureEd25519: + return nil, fmt.Errorf("signature algorithm %s is not supported", req.SignatureAlgorithm) + default: + return nil, fmt.Errorf("signature algorithm %s is not supported", req.SignatureAlgorithm) + } +} + +func parseObject(rawuri string) ([]byte, []byte, error) { + u, err := uri.ParseWithScheme("pkcs11", rawuri) + if err != nil { + return nil, nil, err + } + id, err := u.GetHex("id") + if err != nil { + return nil, nil, err + } + object := u.Get("object") + if len(id) == 0 && object == "" { + return nil, nil, errors.Errorf("key with uri %s is not valid, id or object are required", rawuri) + } + + return id, toByte(object), nil +} + +func findSigner(ctx *crypto11.Context, rawuri string) (crypto11.Signer, error) { + id, object, err := parseObject(rawuri) + if err != nil { + return nil, err + } + signer, err := ctx.FindKeyPair(id, object) + if err != nil { + return nil, errors.Wrapf(err, "error finding key with uri %s", rawuri) + } + if signer == nil { + return nil, errors.Errorf("key with uri %s not found", rawuri) + } + return signer, nil +} + +func findCertificate(ctx *crypto11.Context, rawuri string) (*x509.Certificate, error) { + u, err := uri.ParseWithScheme("pkcs11", rawuri) + if err != nil { + return nil, err + } + id, err := u.GetHex("id") + if err != nil { + return nil, err + } + object, serial := u.Get("object"), u.Get("serial") + if len(id) == 0 && object == "" && serial == "" { + return nil, errors.Errorf("key with uri %s is not valid, id, object or serial are required", rawuri) + } + + var serialNumber *big.Int + if serial != "" { + b, err := hex.DecodeString(serial) + if err != nil { + return nil, errors.Errorf("key with uri %s is not valid, failed to decode serial", rawuri) + } + serialNumber = new(big.Int).SetBytes(b) + } + + cert, err := ctx.FindCertificate(id, toByte(object), serialNumber) + if err != nil { + return nil, errors.Wrapf(err, "error finding certificate with uri %s", rawuri) + } + if cert == nil { + return nil, errors.Errorf("certificate with uri %s not found", rawuri) + } + return cert, nil +} diff --git a/kms/pkcs11/pkcs11_no_cgo.go b/kms/pkcs11/pkcs11_no_cgo.go new file mode 100644 index 00000000..e60a7563 --- /dev/null +++ b/kms/pkcs11/pkcs11_no_cgo.go @@ -0,0 +1,19 @@ +// +build !cgo + +package pkcs11 + +import ( + "context" + "os" + "path/filepath" + + "github.com/pkg/errors" + "github.com/smallstep/certificates/kms/apiv1" +) + +func init() { + apiv1.Register(apiv1.PKCS11, func(ctx context.Context, opts apiv1.Options) (apiv1.KeyManager, error) { + name := filepath.Base(os.Args[0]) + return nil, errors.Errorf("unsupported kms type 'pkcs11': %s is compiled without cgo support", name) + }) +} diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go new file mode 100644 index 00000000..835fd47d --- /dev/null +++ b/kms/pkcs11/pkcs11_test.go @@ -0,0 +1,3 @@ +// +build cgo + +package pkcs11 From 294f84b8d43fcf5897beb79a844a30b447e35101 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 27 Jan 2021 20:17:14 -0800 Subject: [PATCH 03/33] Add initial set of unit tests for pkcs11 kms. --- kms/apiv1/options.go | 19 +- kms/pkcs11/pkcs11.go | 82 +++-- kms/pkcs11/pkcs11_test.go | 641 ++++++++++++++++++++++++++++++++++++++ kms/pkcs11/setup_test.go | 235 ++++++++++++++ kms/uri/uri.go | 4 +- 5 files changed, 945 insertions(+), 36 deletions(-) create mode 100644 kms/pkcs11/setup_test.go diff --git a/kms/apiv1/options.go b/kms/apiv1/options.go index 07d7df0d..705f3633 100644 --- a/kms/apiv1/options.go +++ b/kms/apiv1/options.go @@ -26,16 +26,29 @@ type CertificateManager interface { // ErrNotImplemented is the type of error returned if an operation is not // implemented. type ErrNotImplemented struct { - msg string + Message string } func (e ErrNotImplemented) Error() string { - if e.msg != "" { - return e.msg + if e.Message != "" { + return e.Message } return "not implemented" } +// ErrAlreadyExists is the type of error returned if a key already exists. This +// is currently only implmented on pkcs11. +type ErrAlreadyExists struct { + Message string +} + +func (e ErrAlreadyExists) Error() string { + if e.Message != "" { + return e.Message + } + return "key already exists" +} + // Type represents the KMS type used. type Type string diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index d0a429f8..d45f8045 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -21,9 +21,25 @@ import ( // specified. const DefaultRSASize = 3072 +// P11 defines the methods on crypto11.Context that this package will use. This +// interface will be used for unit testing. +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 + 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) + Close() error +} + +var p11Configure = func(config *crypto11.Config) (P11, error) { + return crypto11.Configure(config) +} + // PKCS11 is the implementation of a KMS using the PKCS #11 standard. type PKCS11 struct { - context *crypto11.Context + p11 P11 } // New returns a new PKCS11 KMS. @@ -54,13 +70,13 @@ func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { return nil, errors.New("kms uri 'token' or 'serial' are mutually exclusive") } - p11Ctx, err := crypto11.Configure(&config) + p11, err := p11Configure(&config) if err != nil { return nil, errors.Wrap(err, "error initializing PKCS#11") } return &PKCS11{ - context: p11Ctx, + p11: p11, }, nil } @@ -76,7 +92,7 @@ func (k *PKCS11) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKey, return nil, errors.New("getPublicKeyRequest 'name' cannot be empty") } - signer, err := findSigner(k.context, req.Name) + signer, err := findSigner(k.p11, req.Name) if err != nil { return nil, errors.Wrap(err, "getPublicKey failed") } @@ -93,7 +109,7 @@ func (k *PKCS11) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyRespons return nil, errors.New("createKeyRequest 'bits' cannot be negative") } - signer, err := generateKey(k.context, req) + signer, err := generateKey(k.p11, req) if err != nil { return nil, errors.Wrap(err, "createKey failed") } @@ -115,7 +131,7 @@ func (k *PKCS11) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, er return nil, errors.New("createSignerRequest 'signingKey' cannot be empty") } - signer, err := findSigner(k.context, req.SigningKey) + signer, err := findSigner(k.p11, req.SigningKey) if err != nil { return nil, errors.Wrap(err, "createSigner failed") } @@ -129,7 +145,7 @@ func (k *PKCS11) LoadCertificate(req *apiv1.LoadCertificateRequest) (*x509.Certi if req.Name == "" { return nil, errors.New("loadCertificateRequest 'name' cannot be nil") } - cert, err := findCertificate(k.context, req.Name) + cert, err := findCertificate(k.p11, req.Name) if err != nil { return nil, errors.Wrap(err, "loadCertificate failed") } @@ -151,7 +167,7 @@ func (k *PKCS11) StoreCertificate(req *apiv1.StoreCertificateRequest) error { return errors.Wrap(err, "storeCertificate failed") } - if err := k.context.ImportCertificateWithLabel(id, object, req.Certificate); err != nil { + if err := k.p11.ImportCertificateWithLabel(id, object, req.Certificate); err != nil { return errors.Wrap(err, "storeCertificate failed") } @@ -164,7 +180,7 @@ func (k *PKCS11) DeleteKey(uri string) error { if err != nil { return errors.Wrap(err, "deleteKey failed") } - signer, err := k.context.FindKeyPair(id, object) + signer, err := k.p11.FindKeyPair(id, object) if err != nil { return errors.Wrap(err, "deleteKey failed") } @@ -183,7 +199,7 @@ func (k *PKCS11) DeleteCertificate(uri string) error { if err != nil { return errors.Wrap(err, "deleteCertificate failed") } - if err := k.context.DeleteCertificate(id, object, nil); err != nil { + if err := k.p11.DeleteCertificate(id, object, nil); err != nil { return errors.Wrap(err, "deleteCertificate failed") } return nil @@ -191,7 +207,7 @@ func (k *PKCS11) DeleteCertificate(uri string) error { // Close releases the connection to the PKCS#11 module. func (k *PKCS11) Close() error { - return errors.Wrap(k.context.Close(), "error closing pkcs#11 context") + return errors.Wrap(k.p11.Close(), "error closing pkcs#11 context") } func toByte(s string) []byte { @@ -201,7 +217,24 @@ func toByte(s string) []byte { return []byte(s) } -func generateKey(ctx *crypto11.Context, req *apiv1.CreateKeyRequest) (crypto11.Signer, error) { +func parseObject(rawuri string) ([]byte, []byte, error) { + u, err := uri.ParseWithScheme("pkcs11", rawuri) + if err != nil { + return nil, nil, err + } + id, err := u.GetHex("id") + if err != nil { + return nil, nil, err + } + object := u.Get("object") + if len(id) == 0 && object == "" { + return nil, nil, errors.Errorf("key with uri %s is not valid, id or object are required", rawuri) + } + + return id, toByte(object), nil +} + +func generateKey(ctx P11, req *apiv1.CreateKeyRequest) (crypto11.Signer, error) { id, object, err := parseObject(req.Name) if err != nil { return nil, err @@ -211,7 +244,9 @@ func generateKey(ctx *crypto11.Context, req *apiv1.CreateKeyRequest) (crypto11.S return nil, err } if signer != nil { - return nil, errors.Errorf("%s already exists", req.Name) + return nil, apiv1.ErrAlreadyExists{ + Message: req.Name + " already exists", + } } bits := req.Bits @@ -239,24 +274,7 @@ func generateKey(ctx *crypto11.Context, req *apiv1.CreateKeyRequest) (crypto11.S } } -func parseObject(rawuri string) ([]byte, []byte, error) { - u, err := uri.ParseWithScheme("pkcs11", rawuri) - if err != nil { - return nil, nil, err - } - id, err := u.GetHex("id") - if err != nil { - return nil, nil, err - } - object := u.Get("object") - if len(id) == 0 && object == "" { - return nil, nil, errors.Errorf("key with uri %s is not valid, id or object are required", rawuri) - } - - return id, toByte(object), nil -} - -func findSigner(ctx *crypto11.Context, rawuri string) (crypto11.Signer, error) { +func findSigner(ctx P11, rawuri string) (crypto11.Signer, error) { id, object, err := parseObject(rawuri) if err != nil { return nil, err @@ -271,7 +289,7 @@ func findSigner(ctx *crypto11.Context, rawuri string) (crypto11.Signer, error) { return signer, nil } -func findCertificate(ctx *crypto11.Context, rawuri string) (*x509.Certificate, error) { +func findCertificate(ctx P11, rawuri string) (*x509.Certificate, error) { u, err := uri.ParseWithScheme("pkcs11", rawuri) if err != nil { return nil, err diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index 835fd47d..f0e8bafc 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -1,3 +1,644 @@ // +build cgo package pkcs11 + +import ( + "context" + "crypto" + "crypto/ecdsa" + "crypto/ed25519" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "reflect" + "testing" + + "github.com/smallstep/certificates/kms/apiv1" +) + +func TestNew(t *testing.T) { + type args struct { + ctx context.Context + opts apiv1.Options + } + tests := []struct { + name string + args args + want *PKCS11 + wantErr bool + }{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := New(tt.args.ctx, tt.args.opts) + if (err != nil) != tt.wantErr { + t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("New() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestPKCS11_GetPublicKey(t *testing.T) { + setupSoftHSM2, setupYubiHSM2 := setupFuncs(t) + type args struct { + req *apiv1.GetPublicKeyRequest + } + tests := []struct { + name string + setup func(t *testing.T) *PKCS11 + args args + want crypto.PublicKey + wantErr bool + }{ + // SoftHSM2 + {"softhsm RSA", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:id=7371;object=rsa-key", + }}, &rsa.PublicKey{}, false}, + {"softhsm RSA by id", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:id=7371", + }}, &rsa.PublicKey{}, false}, + {"softhsm RSA by label", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:object=rsa-key", + }}, &rsa.PublicKey{}, false}, + {"softhsm ECDSA", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:id=7373;object=ecdsa-p256-key", + }}, &ecdsa.PublicKey{}, false}, + {"softhsm ECDSA by id", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:id=7373", + }}, &ecdsa.PublicKey{}, false}, + {"softhsm ECDSA by label", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:object=ecdsa-p256-key", + }}, &ecdsa.PublicKey{}, false}, + // YubiHSM2 + {"yubiHSM2 RSA", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:id=7371;object=rsa-key", + }}, &rsa.PublicKey{}, false}, + {"yubiHSM2 RSA by id", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:id=7371", + }}, &rsa.PublicKey{}, false}, + {"yubiHSM2 RSA by label", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:object=rsa-key", + }}, &rsa.PublicKey{}, false}, + {"yubiHSM2 ECDSA", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:id=7373;object=ecdsa-p256-key", + }}, &ecdsa.PublicKey{}, false}, + {"yubiHSM2 ECDSA by id", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:id=7373", + }}, &ecdsa.PublicKey{}, false}, + {"yubiHSM2 ECDSA by label", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:object=ecdsa-p256-key", + }}, &ecdsa.PublicKey{}, false}, + // Errors + {"fail name", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "", + }}, nil, true}, + {"fail uri", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "https:id=9999;object=https", + }}, nil, true}, + {"fail softhsm missing", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:id=9999;object=rsa-key", + }}, nil, true}, + {"fail yubiHSM2 missing", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:id=9999;object=ecdsa-p256-key", + }}, nil, true}, + {"fail softhsm FindKeyPair", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:foo=bar", + }}, nil, true}, + {"fail yubiHSM2 FindKeyPair", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ + Name: "pkcs11:foo=bar", + }}, nil, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k := tt.setup(t) + got, err := k.GetPublicKey(tt.args.req) + if (err != nil) != tt.wantErr { + t.Errorf("PKCS11.GetPublicKey() error = %v, wantErr %v", err, tt.wantErr) + return + } + if reflect.TypeOf(got) != reflect.TypeOf(tt.want) { + t.Errorf("PKCS11.GetPublicKey() = %T, want %T", got, tt.want) + } + }) + } +} + +func TestPKCS11_CreateKey(t *testing.T) { + setupSoftHSM2, setupYubiHSM2 := setupFuncs(t) + type args struct { + req *apiv1.CreateKeyRequest + } + tests := []struct { + name string + setup func(t *testing.T) *PKCS11 + args args + want *apiv1.CreateKeyResponse + wantErr bool + }{ + // SoftHSM2 + {"softhsm Default", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=ecdsa-create-key", + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=ecdsa-create-key", + PublicKey: &ecdsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=ecdsa-create-key", + }, + }, false}, + {"softhsm RSA SHA256WithRSA", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.SHA256WithRSA, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &rsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"softhsm RSA SHA384WithRSA", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.SHA384WithRSA, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &rsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"softhsm RSA SHA512WithRSA", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.SHA512WithRSA, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &rsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"softhsm RSA SHA256WithRSAPSS", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.SHA256WithRSAPSS, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &rsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"softhsm RSA SHA384WithRSAPSS", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.SHA384WithRSAPSS, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &rsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"softhsm RSA SHA512WithRSAPSS", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.SHA512WithRSAPSS, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &rsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"softhsm RSA 2048", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.SHA256WithRSA, + Bits: 2048, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &rsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"softhsm RSA 4096", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.SHA256WithRSA, + Bits: 4096, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &rsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"softhsm ECDSA P256", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.ECDSAWithSHA256, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &ecdsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"softhsm ECDSA P384", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.ECDSAWithSHA384, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &ecdsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"softhsm ECDSA P521", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.ECDSAWithSHA512, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &ecdsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + // YubiHSM2 + {"yubihsm RSA", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.SHA256WithRSA, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &rsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"yubihsm RSA 2048", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.SHA256WithRSA, + Bits: 2048, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &rsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"yubihsm RSA 4096", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.SHA256WithRSA, + Bits: 4096, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &rsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"yubihsm Default", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=ecdsa-create-key", + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=ecdsa-create-key", + PublicKey: &ecdsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=ecdsa-create-key", + }, + }, false}, + {"yubihsm ECDSA P256", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.ECDSAWithSHA256, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &ecdsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"yubihsm ECDSA P384", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.ECDSAWithSHA384, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &ecdsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + {"yubihsm ECDSA P521", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7771;object=rsa-create-key", + SignatureAlgorithm: apiv1.ECDSAWithSHA512, + }}, &apiv1.CreateKeyResponse{ + Name: "pkcs11:id=7771;object=rsa-create-key", + PublicKey: &ecdsa.PublicKey{}, + CreateSignerRequest: apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7771;object=rsa-create-key", + }, + }, false}, + // Errors + {"fail name", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "", + }}, nil, true}, + {"fail bits", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=9999;object=rsa-create-key", + Bits: -1, + SignatureAlgorithm: apiv1.SHA256WithRSAPSS, + }}, nil, true}, + {"fail ed25519", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=9999;object=rsa-create-key", + SignatureAlgorithm: apiv1.PureEd25519, + }}, nil, true}, + {"fail unknown", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=9999;object=rsa-create-key", + SignatureAlgorithm: apiv1.SignatureAlgorithm(100), + }}, nil, true}, + {"fail uri", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=xxxx;object=https", + SignatureAlgorithm: apiv1.SHA256WithRSAPSS, + }}, nil, true}, + {"fail softhsm FindKeyPair", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:foo=bar", + SignatureAlgorithm: apiv1.SHA256WithRSAPSS, + }}, nil, true}, + {"fail yubihsm FindKeyPair", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:foo=bar", + SignatureAlgorithm: apiv1.SHA256WithRSAPSS, + }}, nil, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k := tt.setup(t) + got, err := k.CreateKey(tt.args.req) + if (err != nil) != tt.wantErr { + t.Errorf("PKCS11.CreateKey() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != nil { + got.PublicKey = tt.want.PublicKey + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("PKCS11.CreateKey() = %v, want %v", got, tt.want) + } + if got != nil { + if err := k.DeleteKey(got.Name); err != nil { + t.Errorf("PKCS11.DeleteKey() error = %v", err) + } + } + }) + } +} + +func TestPKCS11_CreateSigner(t *testing.T) { + data := []byte("buggy-coheir-RUBRIC-rabbet-liberal-eaglet-khartoum-stagger") + setupSoftHSM2, setupYubiHSM2 := setupFuncs(t) + + type args struct { + req *apiv1.CreateSignerRequest + } + tests := []struct { + name string + setup func(t *testing.T) *PKCS11 + args args + algorithm apiv1.SignatureAlgorithm + signerOpts crypto.SignerOpts + wantErr bool + }{ + // SoftHSM2 + {"softhsm RSA", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7371;object=rsa-key", + }}, apiv1.SHA256WithRSA, crypto.SHA256, false}, + {"softhsm RSA PSS", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7371;object=rsa-key", + }}, apiv1.SHA256WithRSA, crypto.SHA256, false}, + {"softhsm ECDSA P256", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7373;object=ecdsa-p256-key", + }}, apiv1.ECDSAWithSHA256, crypto.SHA256, false}, + {"softhsm ECDSA P384", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7374;object=ecdsa-p384-key", + }}, apiv1.ECDSAWithSHA384, crypto.SHA384, false}, + {"softhsm ECDSA P521", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7375;object=ecdsa-p521-key", + }}, apiv1.ECDSAWithSHA512, crypto.SHA512, false}, + // YubiHSM2 + {"yubihsm RSA", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7371;object=rsa-key", + }}, apiv1.SHA256WithRSA, crypto.SHA256, false}, + {"yubihsm RSA PSS", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7371;object=rsa-key", + }}, apiv1.SHA256WithRSA, crypto.SHA256, false}, + {"yubihsm ECDSA P256", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7373;object=ecdsa-p256-key", + }}, apiv1.ECDSAWithSHA256, crypto.SHA256, false}, + {"yubihsm ECDSA P384", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7374;object=ecdsa-p384-key", + }}, apiv1.ECDSAWithSHA384, crypto.SHA384, false}, + {"yubihsm ECDSA P521", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7375;object=ecdsa-p521-key", + }}, apiv1.ECDSAWithSHA512, crypto.SHA512, false}, + // Errors + {"fail SigningKey", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "", + }}, 0, nil, true}, + {"fail uri", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "https:id=7375;object=ecdsa-p521-key", + }}, 0, nil, true}, + {"fail softhsm FindKeyPair", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:foo=bar", + }}, 0, nil, true}, + {"fail yubihsm FindKeyPair", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:foo=bar", + }}, 0, nil, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k := tt.setup(t) + got, err := k.CreateSigner(tt.args.req) + if (err != nil) != tt.wantErr { + t.Errorf("PKCS11.CreateSigner() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if got != nil { + hash := tt.signerOpts.HashFunc() + h := hash.New() + h.Write(data) + digest := h.Sum(nil) + sig, err := got.Sign(rand.Reader, digest, tt.signerOpts) + if err != nil { + t.Errorf("cyrpto.Signer.Sign() error = %v", err) + } + + switch tt.algorithm { + case apiv1.SHA256WithRSA, apiv1.SHA384WithRSA, apiv1.SHA512WithRSA: + pub := got.Public().(*rsa.PublicKey) + if err := rsa.VerifyPKCS1v15(pub, hash, digest, sig); err != nil { + t.Errorf("rsa.VerifyPKCS1v15() error = %v", err) + } + case apiv1.UnspecifiedSignAlgorithm, apiv1.SHA256WithRSAPSS, apiv1.SHA384WithRSAPSS, apiv1.SHA512WithRSAPSS: + pub := got.Public().(*rsa.PublicKey) + if err := rsa.VerifyPSS(pub, hash, digest, sig, tt.signerOpts.(*rsa.PSSOptions)); err != nil { + t.Errorf("rsa.VerifyPSS() error = %v", err) + } + case apiv1.ECDSAWithSHA256, apiv1.ECDSAWithSHA384, apiv1.ECDSAWithSHA512: + pub := got.Public().(*ecdsa.PublicKey) + if !ecdsa.VerifyASN1(pub, digest, sig) { + t.Error("ecdsa.VerifyASN1() failed") + } + default: + t.Errorf("signature algorithm %s is not supported", tt.algorithm) + } + + } + + }) + } +} + +func TestPKCS11_LoadCertificate(t *testing.T) { + setupSoftHSM2, setupYubiHSM2 := setupFuncs(t) + + getCertFn := func(i, j int) func() *x509.Certificate { + return func() *x509.Certificate { + return testCerts[i].Certificates[j] + } + } + + type args struct { + req *apiv1.LoadCertificateRequest + } + tests := []struct { + name string + setup func(t *testing.T) *PKCS11 + args args + wantFn func() *x509.Certificate + wantErr bool + }{ + {"softhsm", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:id=7370;object=root", + }}, getCertFn(0, 0), false}, + {"softhsm by id", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:id=7370", + }}, getCertFn(0, 0), false}, + {"softhsm by label", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:object=root", + }}, getCertFn(0, 0), false}, + {"yubihsm", setupYubiHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:id=7370;object=root", + }}, getCertFn(0, 1), false}, + {"yubihsm by id", setupYubiHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:id=7370", + }}, getCertFn(0, 1), false}, + {"yubihsm by label", setupYubiHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:object=root", + }}, getCertFn(0, 1), false}, + {"fail softhsm missing", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:id=9999;object=root", + }}, nil, true}, + {"fail yubihsm missing", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:id=9999;object=root", + }}, nil, true}, + {"fail name", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "", + }}, nil, true}, + {"fail uri", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:id=xxxx;object=root", + }}, nil, true}, + {"fail softhsm FindCertificate", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:foo=bar", + }}, nil, true}, + {"fail yubihsm FindCertificate", setupYubiHSM2, args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:foo=bar", + }}, nil, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k := tt.setup(t) + got, err := k.LoadCertificate(tt.args.req) + if (err != nil) != tt.wantErr { + t.Errorf("PKCS11.LoadCertificate() error = %v, wantErr %v", err, tt.wantErr) + return + } + var want *x509.Certificate + if tt.wantFn != nil { + want = tt.wantFn() + got.Raw, got.RawIssuer, got.RawSubject, got.RawTBSCertificate, got.RawSubjectPublicKeyInfo = nil, nil, nil, nil, nil + want.Raw, want.RawIssuer, want.RawSubject, want.RawTBSCertificate, want.RawSubjectPublicKeyInfo = nil, nil, nil, nil, nil + } + if !reflect.DeepEqual(got, want) { + t.Errorf("PKCS11.LoadCertificate() = %v, want %v", got, want) + } + }) + } +} + +func TestPKCS11_StoreCertificate(t *testing.T) { + setupSoftHSM2, setupYubiHSM2 := setupFuncs(t) + + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("ed25519.GenerateKey() error = %v", err) + } + + cert, err := generateCertificate(pub, priv) + if err != nil { + t.Fatalf("x509.CreateCertificate() error = %v", err) + } + + type args struct { + req *apiv1.StoreCertificateRequest + } + tests := []struct { + name string + setup func(t *testing.T) *PKCS11 + args args + wantErr bool + }{ + {"softhsm", setupSoftHSM2, args{&apiv1.StoreCertificateRequest{ + Name: "pkcs11:id=7771;object=root", + Certificate: cert, + }}, false}, + {"yubihsm", setupYubiHSM2, args{&apiv1.StoreCertificateRequest{ + Name: "pkcs11:id=7771;object=root", + Certificate: cert, + }}, false}, + {"fail name", setupSoftHSM2, args{&apiv1.StoreCertificateRequest{ + Name: "", + Certificate: cert, + }}, true}, + {"fail certificate", setupSoftHSM2, args{&apiv1.StoreCertificateRequest{ + Name: "pkcs11:id=7771;object=root", + Certificate: nil, + }}, true}, + {"fail uri", setupSoftHSM2, args{&apiv1.StoreCertificateRequest{ + Name: "http:id=7771;object=root", + Certificate: cert, + }}, true}, + {"fail softhsm ImportCertificateWithLabel", setupSoftHSM2, args{&apiv1.StoreCertificateRequest{ + Name: "pkcs11:foo=bar", + Certificate: cert, + }}, true}, + {"fail yubihsm ImportCertificateWithLabel", setupYubiHSM2, args{&apiv1.StoreCertificateRequest{ + Name: "pkcs11:foo=bar", + Certificate: cert, + }}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + k := tt.setup(t) + if err := k.StoreCertificate(tt.args.req); (err != nil) != tt.wantErr { + t.Errorf("PKCS11.StoreCertificate() error = %v, wantErr %v", err, tt.wantErr) + } + if !tt.wantErr { + got, err := k.LoadCertificate(&apiv1.LoadCertificateRequest{ + Name: tt.args.req.Name, + }) + if err != nil { + t.Errorf("PKCS11.LoadCertificate() error = %v", err) + } + if !reflect.DeepEqual(got, cert) { + t.Errorf("PKCS11.LoadCertificate() = %v, want %v", got, cert) + } + if err := k.DeleteCertificate(tt.args.req.Name); err != nil { + t.Errorf("PKCS11.DeleteCertificate() error = %v", err) + } + } + }) + } +} diff --git a/kms/pkcs11/setup_test.go b/kms/pkcs11/setup_test.go new file mode 100644 index 00000000..47c6ab3b --- /dev/null +++ b/kms/pkcs11/setup_test.go @@ -0,0 +1,235 @@ +// +build cgo + +package pkcs11 + +import ( + "crypto" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "runtime" + "sync" + "testing" + "time" + + "github.com/pkg/errors" + + "github.com/ThalesIgnite/crypto11" + "github.com/smallstep/certificates/kms/apiv1" +) + +var ( + softHSM2Once sync.Once + yubiHSM2Once sync.Once +) + +var ( + testKeys = []struct { + Name string + SignatureAlgorithm apiv1.SignatureAlgorithm + Bits int + }{ + {"pkcs11:id=7371;object=rsa-key", apiv1.SHA256WithRSA, 2048}, + {"pkcs11:id=7372;object=rsa-pss-key", apiv1.SHA256WithRSAPSS, DefaultRSASize}, + {"pkcs11:id=7373;object=ecdsa-p256-key", apiv1.ECDSAWithSHA256, 0}, + {"pkcs11:id=7374;object=ecdsa-p384-key", apiv1.ECDSAWithSHA384, 0}, + {"pkcs11:id=7375;object=ecdsa-p521-key", apiv1.ECDSAWithSHA512, 0}, + } + + testCerts = []struct { + Name string + Key string + Certificates []*x509.Certificate + }{ + {"pkcs11:id=7370;object=root", "pkcs11:id=7373;object=ecdsa-p256-key", nil}, + } +) + +func generateCertificate(pub crypto.PublicKey, signer crypto.Signer) (*x509.Certificate, error) { + now := time.Now() + template := &x509.Certificate{ + Subject: pkix.Name{CommonName: "Test Root Certificate"}, + Issuer: pkix.Name{CommonName: "Test Root Certificate"}, + IsCA: true, + MaxPathLen: 1, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + NotBefore: now, + NotAfter: now.Add(time.Hour), + SerialNumber: big.NewInt(100), + } + + b, err := x509.CreateCertificate(rand.Reader, template, template, pub, signer) + if err != nil { + return nil, err + } + + return x509.ParseCertificate(b) +} + +func setup(t *testing.T, k *PKCS11) { + for _, tk := range testKeys { + _, err := k.CreateKey(&apiv1.CreateKeyRequest{ + Name: tk.Name, + SignatureAlgorithm: tk.SignatureAlgorithm, + Bits: tk.Bits, + }) + if err != nil && !errors.Is(errors.Cause(err), apiv1.ErrAlreadyExists{ + Message: tk.Name + " already exists", + }) { + t.Errorf("PKCS11.GetPublicKey() error = %v", err) + } + } + + for i, c := range testCerts { + signer, err := k.CreateSigner(&apiv1.CreateSignerRequest{ + SigningKey: c.Key, + }) + if err != nil { + t.Errorf("PKCS11.CreateSigner() error = %v", err) + continue + } + cert, err := generateCertificate(signer.Public(), signer) + if err != nil { + t.Errorf("x509.CreateCertificate() error = %v", err) + continue + } + if err := k.StoreCertificate(&apiv1.StoreCertificateRequest{ + Name: c.Name, + Certificate: cert, + }); err != nil { + t.Errorf("PKCS1.StoreCertificate() error = %v", err) + continue + } + testCerts[i].Certificates = append(testCerts[i].Certificates, cert) + } +} + +func teardown(t *testing.T, k *PKCS11) { + for _, tk := range testKeys { + if err := k.DeleteKey(tk.Name); err != nil { + t.Errorf("PKCS11.DeleteKey() error = %v", err) + } + } + for _, tc := range testCerts { + if err := k.DeleteCertificate(tc.Name); err != nil { + t.Errorf("PKCS11.DeleteCertificate() error = %v", err) + } + } +} + +type setupFunc func(t *testing.T) *PKCS11 + +func setupFuncs(t *testing.T) (setupFunc, setupFunc) { + var sh2, yh2 *PKCS11 + t.Cleanup(func() { + if sh2 != nil { + sh2.Close() + } + if yh2 != nil { + yh2.Close() + } + }) + setupSoftHSM2 := func(t *testing.T) *PKCS11 { + if sh2 != nil { + return sh2 + } + sh2 = softHSM2(t) + return sh2 + } + setupYubiHSM2 := func(t *testing.T) *PKCS11 { + if yh2 != nil { + return yh2 + } + yh2 = yubiHSM2(t) + return yh2 + } + return setupSoftHSM2, setupYubiHSM2 +} + +// softHSM2 configures a *PKCS11 KMS to be used with softHSM2. To initialize +// this tests, we should run: +// softhsm2-util --init-token --free \ +// --token pkcs11-test --label pkcs11-test \ +// --so-pin password --pin password +// +// To delete we should run: +// softhsm2-util --delete-token --token pkcs11-test +func softHSM2(t *testing.T) *PKCS11 { + t.Helper() + if runtime.GOARCH != "amd64" { + t.Skipf("softHSM2 test skipped on %s:%s", runtime.GOOS, runtime.GOARCH) + } + + var path string + switch runtime.GOOS { + case "darwin": + path = "/usr/local/lib/softhsm/libsofthsm2.so" + case "linux": + path = "/usr/lib/softhsm/libsofthsm2.so" + default: + t.Skipf("softHSM2 test skipped on %s", runtime.GOOS) + return nil + } + p11, err := crypto11.Configure(&crypto11.Config{ + Path: path, + TokenLabel: "pkcs11-test", + Pin: "password", + }) + if err != nil { + t.Skipf("softHSM test skipped on %s: %v", runtime.GOOS, err) + } + + k := &PKCS11{ + p11: p11, + } + + // Setup + softHSM2Once.Do(func() { + teardown(t, k) + setup(t, k) + }) + + return k +} + +// yubiHSM2 configures a *PKCS11 KMS to be used with YubiHSM2. To initialize +// this tests, we should run: +// yubihsm-connector -d +func yubiHSM2(t *testing.T) *PKCS11 { + t.Helper() + if runtime.GOARCH != "amd64" { + t.Skipf("yubiHSM2 test skipped on %s:%s", runtime.GOOS, runtime.GOARCH) + } + + var path string + switch runtime.GOOS { + case "darwin": + path = "/usr/local/lib/pkcs11/yubihsm_pkcs11.dylib" + case "linux": + path = "/usr/lib/x86_64-linux-gnu/pkcs11/yubihsm_pkcs11.so" + default: + t.Skipf("yubiHSM2 test skipped on %s", runtime.GOOS) + return nil + } + p11, err := crypto11.Configure(&crypto11.Config{ + Path: path, + TokenLabel: "YubiHSM", + Pin: "0001password", + }) + if err != nil { + t.Skipf("yubiHSM2 test skipped on %s: %v", runtime.GOOS, err) + } + + k := &PKCS11{ + p11: p11, + } + + // Setup + yubiHSM2Once.Do(func() { + teardown(t, k) + setup(t, k) + }) + + return k +} diff --git a/kms/uri/uri.go b/kms/uri/uri.go index 85d512db..a5a4c55b 100644 --- a/kms/uri/uri.go +++ b/kms/uri/uri.go @@ -189,7 +189,9 @@ func StringDecode(s string) string { // HexDecode deocdes the string s using Percent-Encoding or regular hex // encoding. func HexDecode(s string) ([]byte, error) { - if strings.HasPrefix(s, "%") { + if s == "" { + return nil, nil + } else if strings.HasPrefix(s, "%") { return PercentDecode(s) } From d9da150a5fe10882187c9a79b96189261f52a315 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 27 Jan 2021 20:23:45 -0800 Subject: [PATCH 04/33] Fix test. --- kms/apiv1/options_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kms/apiv1/options_test.go b/kms/apiv1/options_test.go index 5a8307eb..72d9ba24 100644 --- a/kms/apiv1/options_test.go +++ b/kms/apiv1/options_test.go @@ -42,7 +42,7 @@ func TestErrNotImplemented_Error(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { e := ErrNotImplemented{ - msg: tt.fields.msg, + Message: tt.fields.msg, } if got := e.Error(); got != tt.want { t.Errorf("ErrNotImplemented.Error() = %v, want %v", got, tt.want) From 35bf9b787ec16da29e8b47827f0caa4e56a9f658 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 27 Jan 2021 20:35:42 -0800 Subject: [PATCH 05/33] Implement ecdsa.VerifyASN1 to be compatible with go < 1.15 --- kms/pkcs11/pkcs11_test.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index f0e8bafc..62497549 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -10,10 +10,13 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" + "math/big" "reflect" "testing" "github.com/smallstep/certificates/kms/apiv1" + "golang.org/x/crypto/cryptobyte" + "golang.org/x/crypto/cryptobyte/asn1" ) func TestNew(t *testing.T) { @@ -478,7 +481,7 @@ func TestPKCS11_CreateSigner(t *testing.T) { } case apiv1.ECDSAWithSHA256, apiv1.ECDSAWithSHA384, apiv1.ECDSAWithSHA512: pub := got.Public().(*ecdsa.PublicKey) - if !ecdsa.VerifyASN1(pub, digest, sig) { + if !VerifyASN1(pub, digest, sig) { t.Error("ecdsa.VerifyASN1() failed") } default: @@ -642,3 +645,21 @@ func TestPKCS11_StoreCertificate(t *testing.T) { }) } } + +// VerifyASN1 verifies the ASN.1 encoded signature, sig, of hash using the +// public key, pub. Its return value records whether the signature is valid. +func VerifyASN1(pub *ecdsa.PublicKey, hash, sig []byte) bool { + var ( + r, s = &big.Int{}, &big.Int{} + inner cryptobyte.String + ) + input := cryptobyte.String(sig) + if !input.ReadASN1(&inner, asn1.SEQUENCE) || + input.Empty() || + inner.ReadASN1Integer(r) || + inner.ReadASN1Integer(s) || + inner.Empty() { + return false + } + return ecdsa.Verify(pub, hash, r, s) +} From 6c113542c882254c6a9988eaed7145352f288542 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Jan 2021 11:38:21 -0800 Subject: [PATCH 06/33] Fix ecdsa signature verification test. --- kms/pkcs11/pkcs11_test.go | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index 62497549..619d9f23 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -392,6 +392,24 @@ func TestPKCS11_CreateSigner(t *testing.T) { data := []byte("buggy-coheir-RUBRIC-rabbet-liberal-eaglet-khartoum-stagger") setupSoftHSM2, setupYubiHSM2 := setupFuncs(t) + // VerifyASN1 verifies the ASN.1 encoded signature, sig, of hash using the + // public key, pub. Its return value records whether the signature is valid. + verifyASN1 := func(pub *ecdsa.PublicKey, hash, sig []byte) bool { + var ( + r, s = &big.Int{}, &big.Int{} + inner cryptobyte.String + ) + input := cryptobyte.String(sig) + if !input.ReadASN1(&inner, asn1.SEQUENCE) || + !input.Empty() || + !inner.ReadASN1Integer(r) || + !inner.ReadASN1Integer(s) || + !inner.Empty() { + return false + } + return ecdsa.Verify(pub, hash, r, s) + } + type args struct { req *apiv1.CreateSignerRequest } @@ -481,7 +499,7 @@ func TestPKCS11_CreateSigner(t *testing.T) { } case apiv1.ECDSAWithSHA256, apiv1.ECDSAWithSHA384, apiv1.ECDSAWithSHA512: pub := got.Public().(*ecdsa.PublicKey) - if !VerifyASN1(pub, digest, sig) { + if !verifyASN1(pub, digest, sig) { t.Error("ecdsa.VerifyASN1() failed") } default: @@ -645,21 +663,3 @@ func TestPKCS11_StoreCertificate(t *testing.T) { }) } } - -// VerifyASN1 verifies the ASN.1 encoded signature, sig, of hash using the -// public key, pub. Its return value records whether the signature is valid. -func VerifyASN1(pub *ecdsa.PublicKey, hash, sig []byte) bool { - var ( - r, s = &big.Int{}, &big.Int{} - inner cryptobyte.String - ) - input := cryptobyte.String(sig) - if !input.ReadASN1(&inner, asn1.SEQUENCE) || - input.Empty() || - inner.ReadASN1Integer(r) || - inner.ReadASN1Integer(s) || - inner.Empty() { - return false - } - return ecdsa.Verify(pub, hash, r, s) -} From 673675fa89c758e9d5cf6094e27f27c0024c9c70 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Jan 2021 14:43:22 -0800 Subject: [PATCH 07/33] Convert pkcs11 tests to use tags. --- kms/pkcs11/other_test.go | 178 +++++++++++++++++++ kms/pkcs11/pkcs11_test.go | 340 ++++++++++-------------------------- kms/pkcs11/setup_test.go | 126 +------------ kms/pkcs11/softhsm2_test.go | 60 +++++++ kms/pkcs11/yubihsm2_test.go | 55 ++++++ 5 files changed, 393 insertions(+), 366 deletions(-) create mode 100644 kms/pkcs11/other_test.go create mode 100644 kms/pkcs11/softhsm2_test.go create mode 100644 kms/pkcs11/yubihsm2_test.go diff --git a/kms/pkcs11/other_test.go b/kms/pkcs11/other_test.go new file mode 100644 index 00000000..602e101c --- /dev/null +++ b/kms/pkcs11/other_test.go @@ -0,0 +1,178 @@ +// +build !softhsm2,!yubihsm2 + +package pkcs11 + +import ( + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "fmt" + "io" + "math/big" + "testing" + + "github.com/ThalesIgnite/crypto11" + "github.com/pkg/errors" +) + +func mustPKCS11(t *testing.T) *PKCS11 { + t.Helper() + testModule = "Golang crypto" + k := &PKCS11{ + p11: &stubPKCS11{ + signerIndex: make(map[keyType]int), + certIndex: make(map[keyType]int), + }, + } + for i := range testCerts { + testCerts[i].Certificates = nil + } + setup(t, k) + return k +} + +type keyType struct { + id string + label string + serial string +} + +func newKey(id, label []byte, serial *big.Int) keyType { + var serialString string + if serial != nil { + serialString = serial.String() + } + return keyType{ + id: string(id), + label: string(label), + serial: serialString, + } +} + +type stubPKCS11 struct { + signers []crypto11.Signer + certs []*x509.Certificate + signerIndex map[keyType]int + certIndex map[keyType]int +} + +func (s *stubPKCS11) FindKeyPair(id, label []byte) (crypto11.Signer, error) { + if id == nil && label == nil { + return nil, errors.New("id and label cannot both be nil") + } + i, ok := s.signerIndex[newKey(id, label, nil)] + fmt.Println(i, ok) + if ok { + + return s.signers[i], nil + } + return nil, nil +} + +func (s *stubPKCS11) FindCertificate(id, label []byte, serial *big.Int) (*x509.Certificate, error) { + if id == nil && label == nil && serial == nil { + return nil, errors.New("id, label and serial cannot both be nil") + } + if i, ok := s.certIndex[newKey(id, label, serial)]; ok { + return s.certs[i], nil + } + return nil, nil + +} + +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 cert == nil: + return errors.New("certificate cannot be nil") + } + + i := len(s.certs) + s.certs = append(s.certs, cert) + s.certIndex[newKey(id, label, cert.SerialNumber)] = i + s.certIndex[newKey(id, nil, nil)] = i + s.certIndex[newKey(nil, label, nil)] = i + s.certIndex[newKey(nil, nil, cert.SerialNumber)] = i + s.certIndex[newKey(id, label, nil)] = i + s.certIndex[newKey(id, nil, cert.SerialNumber)] = i + s.certIndex[newKey(nil, label, cert.SerialNumber)] = i + + return nil +} + +func (s *stubPKCS11) DeleteCertificate(id, label []byte, serial *big.Int) error { + if id == nil && label == nil && serial == nil { + return errors.New("id, label and serial cannot both be nil") + } + if i, ok := s.certIndex[newKey(id, label, serial)]; ok { + s.certs[i] = nil + } + return nil +} + +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") + } + p, err := rsa.GenerateKey(rand.Reader, bits) + if err != nil { + return nil, err + } + k := &privateKey{ + Signer: p, + index: len(s.signers), + stub: s, + } + s.signers = append(s.signers, k) + s.signerIndex[newKey(id, label, nil)] = k.index + s.signerIndex[newKey(id, nil, nil)] = k.index + s.signerIndex[newKey(nil, label, nil)] = k.index + return k, nil +} + +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") + } + p, err := ecdsa.GenerateKey(curve, rand.Reader) + if err != nil { + return nil, err + } + k := &privateKey{ + Signer: p, + index: len(s.signers), + stub: s, + } + s.signers = append(s.signers, k) + s.signerIndex[newKey(id, label, nil)] = k.index + s.signerIndex[newKey(id, nil, nil)] = k.index + s.signerIndex[newKey(nil, label, nil)] = k.index + return k, nil +} + +func (s *stubPKCS11) Close() error { + return nil +} + +type privateKey struct { + crypto.Signer + index int + stub *stubPKCS11 +} + +func (s *privateKey) Delete() error { + s.stub.signers[s.index] = nil + return nil +} + +func (s *privateKey) Decrypt(rand io.Reader, msg []byte, opts crypto.DecrypterOpts) (plaintext []byte, err error) { + k, ok := s.Signer.(*rsa.PrivateKey) + if !ok { + return nil, errors.New("key is not an rsa key") + } + return k.Decrypt(rand, msg, opts) +} diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index 619d9f23..3c4d0bc3 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -45,78 +45,49 @@ func TestNew(t *testing.T) { } func TestPKCS11_GetPublicKey(t *testing.T) { - setupSoftHSM2, setupYubiHSM2 := setupFuncs(t) + k := setupPKCS11(t) type args struct { req *apiv1.GetPublicKeyRequest } tests := []struct { name string - setup func(t *testing.T) *PKCS11 args args want crypto.PublicKey wantErr bool }{ - // SoftHSM2 - {"softhsm RSA", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + {"RSA", args{&apiv1.GetPublicKeyRequest{ Name: "pkcs11:id=7371;object=rsa-key", }}, &rsa.PublicKey{}, false}, - {"softhsm RSA by id", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + {"RSA by id", args{&apiv1.GetPublicKeyRequest{ Name: "pkcs11:id=7371", }}, &rsa.PublicKey{}, false}, - {"softhsm RSA by label", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + {"RSA by label", args{&apiv1.GetPublicKeyRequest{ Name: "pkcs11:object=rsa-key", }}, &rsa.PublicKey{}, false}, - {"softhsm ECDSA", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + {"ECDSA", args{&apiv1.GetPublicKeyRequest{ Name: "pkcs11:id=7373;object=ecdsa-p256-key", }}, &ecdsa.PublicKey{}, false}, - {"softhsm ECDSA by id", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + {"ECDSA by id", args{&apiv1.GetPublicKeyRequest{ Name: "pkcs11:id=7373", }}, &ecdsa.PublicKey{}, false}, - {"softhsm ECDSA by label", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + {"ECDSA by label", args{&apiv1.GetPublicKeyRequest{ Name: "pkcs11:object=ecdsa-p256-key", }}, &ecdsa.PublicKey{}, false}, - // YubiHSM2 - {"yubiHSM2 RSA", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ - Name: "pkcs11:id=7371;object=rsa-key", - }}, &rsa.PublicKey{}, false}, - {"yubiHSM2 RSA by id", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ - Name: "pkcs11:id=7371", - }}, &rsa.PublicKey{}, false}, - {"yubiHSM2 RSA by label", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ - Name: "pkcs11:object=rsa-key", - }}, &rsa.PublicKey{}, false}, - {"yubiHSM2 ECDSA", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ - Name: "pkcs11:id=7373;object=ecdsa-p256-key", - }}, &ecdsa.PublicKey{}, false}, - {"yubiHSM2 ECDSA by id", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ - Name: "pkcs11:id=7373", - }}, &ecdsa.PublicKey{}, false}, - {"yubiHSM2 ECDSA by label", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ - Name: "pkcs11:object=ecdsa-p256-key", - }}, &ecdsa.PublicKey{}, false}, - // Errors - {"fail name", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + {"fail name", args{&apiv1.GetPublicKeyRequest{ Name: "", }}, nil, true}, - {"fail uri", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + {"fail uri", args{&apiv1.GetPublicKeyRequest{ Name: "https:id=9999;object=https", }}, nil, true}, - {"fail softhsm missing", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ + {"fail missing", args{&apiv1.GetPublicKeyRequest{ Name: "pkcs11:id=9999;object=rsa-key", }}, nil, true}, - {"fail yubiHSM2 missing", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ - Name: "pkcs11:id=9999;object=ecdsa-p256-key", - }}, nil, true}, - {"fail softhsm FindKeyPair", setupSoftHSM2, args{&apiv1.GetPublicKeyRequest{ - Name: "pkcs11:foo=bar", - }}, nil, true}, - {"fail yubiHSM2 FindKeyPair", setupYubiHSM2, args{&apiv1.GetPublicKeyRequest{ + {"fail FindKeyPair", args{&apiv1.GetPublicKeyRequest{ Name: "pkcs11:foo=bar", }}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - k := tt.setup(t) got, err := k.GetPublicKey(tt.args.req) if (err != nil) != tt.wantErr { t.Errorf("PKCS11.GetPublicKey() error = %v, wantErr %v", err, tt.wantErr) @@ -130,244 +101,170 @@ func TestPKCS11_GetPublicKey(t *testing.T) { } func TestPKCS11_CreateKey(t *testing.T) { - setupSoftHSM2, setupYubiHSM2 := setupFuncs(t) + k := setupPKCS11(t) + + // Make sure to delete the created key + keyName := "pkcs11:id=7771;object=create-key" + k.DeleteKey(keyName) + type args struct { req *apiv1.CreateKeyRequest } tests := []struct { name string - setup func(t *testing.T) *PKCS11 args args want *apiv1.CreateKeyResponse wantErr bool }{ // SoftHSM2 - {"softhsm Default", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=ecdsa-create-key", + {"default", args{&apiv1.CreateKeyRequest{ + Name: keyName, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=ecdsa-create-key", + Name: keyName, PublicKey: &ecdsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=ecdsa-create-key", + SigningKey: keyName, }, }, false}, - {"softhsm RSA SHA256WithRSA", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", + {"RSA SHA256WithRSA", args{&apiv1.CreateKeyRequest{ + Name: keyName, SignatureAlgorithm: apiv1.SHA256WithRSA, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", + Name: keyName, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", + SigningKey: keyName, }, }, false}, - {"softhsm RSA SHA384WithRSA", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", + {"RSA SHA384WithRSA", args{&apiv1.CreateKeyRequest{ + Name: keyName, SignatureAlgorithm: apiv1.SHA384WithRSA, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", + Name: keyName, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", + SigningKey: keyName, }, }, false}, - {"softhsm RSA SHA512WithRSA", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", + {"RSA SHA512WithRSA", args{&apiv1.CreateKeyRequest{ + Name: keyName, SignatureAlgorithm: apiv1.SHA512WithRSA, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", + Name: keyName, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", + SigningKey: keyName, }, }, false}, - {"softhsm RSA SHA256WithRSAPSS", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", + {"RSA SHA256WithRSAPSS", args{&apiv1.CreateKeyRequest{ + Name: keyName, SignatureAlgorithm: apiv1.SHA256WithRSAPSS, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", + Name: keyName, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", + SigningKey: keyName, }, }, false}, - {"softhsm RSA SHA384WithRSAPSS", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", + {"RSA SHA384WithRSAPSS", args{&apiv1.CreateKeyRequest{ + Name: keyName, SignatureAlgorithm: apiv1.SHA384WithRSAPSS, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", + Name: keyName, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", + SigningKey: keyName, }, }, false}, - {"softhsm RSA SHA512WithRSAPSS", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", + {"RSA SHA512WithRSAPSS", args{&apiv1.CreateKeyRequest{ + Name: keyName, SignatureAlgorithm: apiv1.SHA512WithRSAPSS, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", + Name: keyName, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", + SigningKey: keyName, }, }, false}, - {"softhsm RSA 2048", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", + {"RSA 2048", args{&apiv1.CreateKeyRequest{ + Name: keyName, SignatureAlgorithm: apiv1.SHA256WithRSA, Bits: 2048, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", + Name: keyName, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", + SigningKey: keyName, }, }, false}, - {"softhsm RSA 4096", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", + {"RSA 4096", args{&apiv1.CreateKeyRequest{ + Name: keyName, SignatureAlgorithm: apiv1.SHA256WithRSA, Bits: 4096, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", + Name: keyName, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", + SigningKey: keyName, }, }, false}, - {"softhsm ECDSA P256", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", + {"ECDSA P256", args{&apiv1.CreateKeyRequest{ + Name: keyName, SignatureAlgorithm: apiv1.ECDSAWithSHA256, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", + Name: keyName, PublicKey: &ecdsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", + SigningKey: keyName, }, }, false}, - {"softhsm ECDSA P384", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", + {"ECDSA P384", args{&apiv1.CreateKeyRequest{ + Name: keyName, SignatureAlgorithm: apiv1.ECDSAWithSHA384, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", + Name: keyName, PublicKey: &ecdsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", + SigningKey: keyName, }, }, false}, - {"softhsm ECDSA P521", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", + {"ECDSA P521", args{&apiv1.CreateKeyRequest{ + Name: keyName, SignatureAlgorithm: apiv1.ECDSAWithSHA512, }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", + Name: keyName, PublicKey: &ecdsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", + SigningKey: keyName, }, }, false}, - // YubiHSM2 - {"yubihsm RSA", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", - SignatureAlgorithm: apiv1.SHA256WithRSA, - }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", - PublicKey: &rsa.PublicKey{}, - CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", - }, - }, false}, - {"yubihsm RSA 2048", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", - SignatureAlgorithm: apiv1.SHA256WithRSA, - Bits: 2048, - }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", - PublicKey: &rsa.PublicKey{}, - CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", - }, - }, false}, - {"yubihsm RSA 4096", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", - SignatureAlgorithm: apiv1.SHA256WithRSA, - Bits: 4096, - }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", - PublicKey: &rsa.PublicKey{}, - CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", - }, - }, false}, - {"yubihsm Default", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=ecdsa-create-key", - }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=ecdsa-create-key", - PublicKey: &ecdsa.PublicKey{}, - CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=ecdsa-create-key", - }, - }, false}, - {"yubihsm ECDSA P256", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", - SignatureAlgorithm: apiv1.ECDSAWithSHA256, - }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", - PublicKey: &ecdsa.PublicKey{}, - CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", - }, - }, false}, - {"yubihsm ECDSA P384", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", - SignatureAlgorithm: apiv1.ECDSAWithSHA384, - }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", - PublicKey: &ecdsa.PublicKey{}, - CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", - }, - }, false}, - {"yubihsm ECDSA P521", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=7771;object=rsa-create-key", - SignatureAlgorithm: apiv1.ECDSAWithSHA512, - }}, &apiv1.CreateKeyResponse{ - Name: "pkcs11:id=7771;object=rsa-create-key", - PublicKey: &ecdsa.PublicKey{}, - CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7771;object=rsa-create-key", - }, - }, false}, - // Errors - {"fail name", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + {"fail name", args{&apiv1.CreateKeyRequest{ Name: "", }}, nil, true}, - {"fail bits", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + {"fail bits", args{&apiv1.CreateKeyRequest{ Name: "pkcs11:id=9999;object=rsa-create-key", Bits: -1, SignatureAlgorithm: apiv1.SHA256WithRSAPSS, }}, nil, true}, - {"fail ed25519", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + {"fail ed25519", args{&apiv1.CreateKeyRequest{ Name: "pkcs11:id=9999;object=rsa-create-key", SignatureAlgorithm: apiv1.PureEd25519, }}, nil, true}, - {"fail unknown", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + {"fail unknown", args{&apiv1.CreateKeyRequest{ Name: "pkcs11:id=9999;object=rsa-create-key", SignatureAlgorithm: apiv1.SignatureAlgorithm(100), }}, nil, true}, - {"fail uri", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ + {"fail uri", args{&apiv1.CreateKeyRequest{ Name: "pkcs11:id=xxxx;object=https", SignatureAlgorithm: apiv1.SHA256WithRSAPSS, }}, nil, true}, - {"fail softhsm FindKeyPair", setupSoftHSM2, args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:foo=bar", - SignatureAlgorithm: apiv1.SHA256WithRSAPSS, - }}, nil, true}, - {"fail yubihsm FindKeyPair", setupYubiHSM2, args{&apiv1.CreateKeyRequest{ + {"fail FindKeyPair", args{&apiv1.CreateKeyRequest{ Name: "pkcs11:foo=bar", SignatureAlgorithm: apiv1.SHA256WithRSAPSS, }}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - k := tt.setup(t) got, err := k.CreateKey(tt.args.req) if (err != nil) != tt.wantErr { t.Errorf("PKCS11.CreateKey() error = %v, wantErr %v", err, tt.wantErr) @@ -389,8 +286,8 @@ func TestPKCS11_CreateKey(t *testing.T) { } func TestPKCS11_CreateSigner(t *testing.T) { + k := setupPKCS11(t) data := []byte("buggy-coheir-RUBRIC-rabbet-liberal-eaglet-khartoum-stagger") - setupSoftHSM2, setupYubiHSM2 := setupFuncs(t) // VerifyASN1 verifies the ASN.1 encoded signature, sig, of hash using the // public key, pub. Its return value records whether the signature is valid. @@ -415,61 +312,39 @@ func TestPKCS11_CreateSigner(t *testing.T) { } tests := []struct { name string - setup func(t *testing.T) *PKCS11 args args algorithm apiv1.SignatureAlgorithm signerOpts crypto.SignerOpts wantErr bool }{ // SoftHSM2 - {"softhsm RSA", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + {"RSA", args{&apiv1.CreateSignerRequest{ SigningKey: "pkcs11:id=7371;object=rsa-key", }}, apiv1.SHA256WithRSA, crypto.SHA256, false}, - {"softhsm RSA PSS", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + {"RSA PSS", args{&apiv1.CreateSignerRequest{ SigningKey: "pkcs11:id=7371;object=rsa-key", }}, apiv1.SHA256WithRSA, crypto.SHA256, false}, - {"softhsm ECDSA P256", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + {"ECDSA P256", args{&apiv1.CreateSignerRequest{ SigningKey: "pkcs11:id=7373;object=ecdsa-p256-key", }}, apiv1.ECDSAWithSHA256, crypto.SHA256, false}, - {"softhsm ECDSA P384", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + {"ECDSA P384", args{&apiv1.CreateSignerRequest{ SigningKey: "pkcs11:id=7374;object=ecdsa-p384-key", }}, apiv1.ECDSAWithSHA384, crypto.SHA384, false}, - {"softhsm ECDSA P521", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + {"ECDSA P521", args{&apiv1.CreateSignerRequest{ SigningKey: "pkcs11:id=7375;object=ecdsa-p521-key", }}, apiv1.ECDSAWithSHA512, crypto.SHA512, false}, - // YubiHSM2 - {"yubihsm RSA", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7371;object=rsa-key", - }}, apiv1.SHA256WithRSA, crypto.SHA256, false}, - {"yubihsm RSA PSS", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7371;object=rsa-key", - }}, apiv1.SHA256WithRSA, crypto.SHA256, false}, - {"yubihsm ECDSA P256", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7373;object=ecdsa-p256-key", - }}, apiv1.ECDSAWithSHA256, crypto.SHA256, false}, - {"yubihsm ECDSA P384", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7374;object=ecdsa-p384-key", - }}, apiv1.ECDSAWithSHA384, crypto.SHA384, false}, - {"yubihsm ECDSA P521", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7375;object=ecdsa-p521-key", - }}, apiv1.ECDSAWithSHA512, crypto.SHA512, false}, - // Errors - {"fail SigningKey", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + {"fail SigningKey", args{&apiv1.CreateSignerRequest{ SigningKey: "", }}, 0, nil, true}, - {"fail uri", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ + {"fail uri", args{&apiv1.CreateSignerRequest{ SigningKey: "https:id=7375;object=ecdsa-p521-key", }}, 0, nil, true}, - {"fail softhsm FindKeyPair", setupSoftHSM2, args{&apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:foo=bar", - }}, 0, nil, true}, - {"fail yubihsm FindKeyPair", setupYubiHSM2, args{&apiv1.CreateSignerRequest{ + {"fail FindKeyPair", args{&apiv1.CreateSignerRequest{ SigningKey: "pkcs11:foo=bar", }}, 0, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - k := tt.setup(t) got, err := k.CreateSigner(tt.args.req) if (err != nil) != tt.wantErr { t.Errorf("PKCS11.CreateSigner() error = %v, wantErr %v", err, tt.wantErr) @@ -513,7 +388,7 @@ func TestPKCS11_CreateSigner(t *testing.T) { } func TestPKCS11_LoadCertificate(t *testing.T) { - setupSoftHSM2, setupYubiHSM2 := setupFuncs(t) + k := setupPKCS11(t) getCertFn := func(i, j int) func() *x509.Certificate { return func() *x509.Certificate { @@ -526,51 +401,34 @@ func TestPKCS11_LoadCertificate(t *testing.T) { } tests := []struct { name string - setup func(t *testing.T) *PKCS11 args args wantFn func() *x509.Certificate wantErr bool }{ - {"softhsm", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + {"load", args{&apiv1.LoadCertificateRequest{ Name: "pkcs11:id=7370;object=root", }}, getCertFn(0, 0), false}, - {"softhsm by id", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + {"load by id", args{&apiv1.LoadCertificateRequest{ Name: "pkcs11:id=7370", }}, getCertFn(0, 0), false}, - {"softhsm by label", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + {"load by label", args{&apiv1.LoadCertificateRequest{ Name: "pkcs11:object=root", }}, getCertFn(0, 0), false}, - {"yubihsm", setupYubiHSM2, args{&apiv1.LoadCertificateRequest{ - Name: "pkcs11:id=7370;object=root", - }}, getCertFn(0, 1), false}, - {"yubihsm by id", setupYubiHSM2, args{&apiv1.LoadCertificateRequest{ - Name: "pkcs11:id=7370", - }}, getCertFn(0, 1), false}, - {"yubihsm by label", setupYubiHSM2, args{&apiv1.LoadCertificateRequest{ - Name: "pkcs11:object=root", - }}, getCertFn(0, 1), false}, - {"fail softhsm missing", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + {"fail missing", args{&apiv1.LoadCertificateRequest{ Name: "pkcs11:id=9999;object=root", }}, nil, true}, - {"fail yubihsm missing", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ - Name: "pkcs11:id=9999;object=root", - }}, nil, true}, - {"fail name", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + {"fail name", args{&apiv1.LoadCertificateRequest{ Name: "", }}, nil, true}, - {"fail uri", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ + {"fail uri", args{&apiv1.LoadCertificateRequest{ Name: "pkcs11:id=xxxx;object=root", }}, nil, true}, - {"fail softhsm FindCertificate", setupSoftHSM2, args{&apiv1.LoadCertificateRequest{ - Name: "pkcs11:foo=bar", - }}, nil, true}, - {"fail yubihsm FindCertificate", setupYubiHSM2, args{&apiv1.LoadCertificateRequest{ + {"fail FindCertificate", args{&apiv1.LoadCertificateRequest{ Name: "pkcs11:foo=bar", }}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - k := tt.setup(t) got, err := k.LoadCertificate(tt.args.req) if (err != nil) != tt.wantErr { t.Errorf("PKCS11.LoadCertificate() error = %v, wantErr %v", err, tt.wantErr) @@ -590,7 +448,7 @@ func TestPKCS11_LoadCertificate(t *testing.T) { } func TestPKCS11_StoreCertificate(t *testing.T) { - setupSoftHSM2, setupYubiHSM2 := setupFuncs(t) + k := setupPKCS11(t) pub, priv, err := ed25519.GenerateKey(rand.Reader) if err != nil { @@ -607,42 +465,32 @@ func TestPKCS11_StoreCertificate(t *testing.T) { } tests := []struct { name string - setup func(t *testing.T) *PKCS11 args args wantErr bool }{ - {"softhsm", setupSoftHSM2, args{&apiv1.StoreCertificateRequest{ + {"ok", args{&apiv1.StoreCertificateRequest{ Name: "pkcs11:id=7771;object=root", Certificate: cert, }}, false}, - {"yubihsm", setupYubiHSM2, args{&apiv1.StoreCertificateRequest{ - Name: "pkcs11:id=7771;object=root", - Certificate: cert, - }}, false}, - {"fail name", setupSoftHSM2, args{&apiv1.StoreCertificateRequest{ + {"fail name", args{&apiv1.StoreCertificateRequest{ Name: "", Certificate: cert, }}, true}, - {"fail certificate", setupSoftHSM2, args{&apiv1.StoreCertificateRequest{ + {"fail certificate", args{&apiv1.StoreCertificateRequest{ Name: "pkcs11:id=7771;object=root", Certificate: nil, }}, true}, - {"fail uri", setupSoftHSM2, args{&apiv1.StoreCertificateRequest{ + {"fail uri", args{&apiv1.StoreCertificateRequest{ Name: "http:id=7771;object=root", Certificate: cert, }}, true}, - {"fail softhsm ImportCertificateWithLabel", setupSoftHSM2, args{&apiv1.StoreCertificateRequest{ - Name: "pkcs11:foo=bar", - Certificate: cert, - }}, true}, - {"fail yubihsm ImportCertificateWithLabel", setupYubiHSM2, args{&apiv1.StoreCertificateRequest{ + {"fail ImportCertificateWithLabel", args{&apiv1.StoreCertificateRequest{ Name: "pkcs11:foo=bar", Certificate: cert, }}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - k := tt.setup(t) 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 47c6ab3b..b912f79c 100644 --- a/kms/pkcs11/setup_test.go +++ b/kms/pkcs11/setup_test.go @@ -8,24 +8,17 @@ import ( "crypto/x509" "crypto/x509/pkix" "math/big" - "runtime" - "sync" "testing" "time" "github.com/pkg/errors" - "github.com/ThalesIgnite/crypto11" "github.com/smallstep/certificates/kms/apiv1" ) var ( - softHSM2Once sync.Once - yubiHSM2Once sync.Once -) - -var ( - testKeys = []struct { + testModule = "" + testKeys = []struct { Name string SignatureAlgorithm apiv1.SignatureAlgorithm Bits int @@ -68,6 +61,7 @@ func generateCertificate(pub crypto.PublicKey, signer crypto.Signer) (*x509.Cert } func setup(t *testing.T, k *PKCS11) { + t.Log("Running using", testModule) for _, tk := range testKeys { _, err := k.CreateKey(&apiv1.CreateKeyRequest{ Name: tk.Name, @@ -118,118 +112,10 @@ func teardown(t *testing.T, k *PKCS11) { } } -type setupFunc func(t *testing.T) *PKCS11 - -func setupFuncs(t *testing.T) (setupFunc, setupFunc) { - var sh2, yh2 *PKCS11 +func setupPKCS11(t *testing.T) *PKCS11 { + k := mustPKCS11(t) t.Cleanup(func() { - if sh2 != nil { - sh2.Close() - } - if yh2 != nil { - yh2.Close() - } + k.Close() }) - setupSoftHSM2 := func(t *testing.T) *PKCS11 { - if sh2 != nil { - return sh2 - } - sh2 = softHSM2(t) - return sh2 - } - setupYubiHSM2 := func(t *testing.T) *PKCS11 { - if yh2 != nil { - return yh2 - } - yh2 = yubiHSM2(t) - return yh2 - } - return setupSoftHSM2, setupYubiHSM2 -} - -// softHSM2 configures a *PKCS11 KMS to be used with softHSM2. To initialize -// this tests, we should run: -// softhsm2-util --init-token --free \ -// --token pkcs11-test --label pkcs11-test \ -// --so-pin password --pin password -// -// To delete we should run: -// softhsm2-util --delete-token --token pkcs11-test -func softHSM2(t *testing.T) *PKCS11 { - t.Helper() - if runtime.GOARCH != "amd64" { - t.Skipf("softHSM2 test skipped on %s:%s", runtime.GOOS, runtime.GOARCH) - } - - var path string - switch runtime.GOOS { - case "darwin": - path = "/usr/local/lib/softhsm/libsofthsm2.so" - case "linux": - path = "/usr/lib/softhsm/libsofthsm2.so" - default: - t.Skipf("softHSM2 test skipped on %s", runtime.GOOS) - return nil - } - p11, err := crypto11.Configure(&crypto11.Config{ - Path: path, - TokenLabel: "pkcs11-test", - Pin: "password", - }) - if err != nil { - t.Skipf("softHSM test skipped on %s: %v", runtime.GOOS, err) - } - - k := &PKCS11{ - p11: p11, - } - - // Setup - softHSM2Once.Do(func() { - teardown(t, k) - setup(t, k) - }) - - return k -} - -// yubiHSM2 configures a *PKCS11 KMS to be used with YubiHSM2. To initialize -// this tests, we should run: -// yubihsm-connector -d -func yubiHSM2(t *testing.T) *PKCS11 { - t.Helper() - if runtime.GOARCH != "amd64" { - t.Skipf("yubiHSM2 test skipped on %s:%s", runtime.GOOS, runtime.GOARCH) - } - - var path string - switch runtime.GOOS { - case "darwin": - path = "/usr/local/lib/pkcs11/yubihsm_pkcs11.dylib" - case "linux": - path = "/usr/lib/x86_64-linux-gnu/pkcs11/yubihsm_pkcs11.so" - default: - t.Skipf("yubiHSM2 test skipped on %s", runtime.GOOS) - return nil - } - p11, err := crypto11.Configure(&crypto11.Config{ - Path: path, - TokenLabel: "YubiHSM", - Pin: "0001password", - }) - if err != nil { - t.Skipf("yubiHSM2 test skipped on %s: %v", runtime.GOOS, err) - } - - k := &PKCS11{ - p11: p11, - } - - // Setup - yubiHSM2Once.Do(func() { - teardown(t, k) - setup(t, k) - }) - return k } diff --git a/kms/pkcs11/softhsm2_test.go b/kms/pkcs11/softhsm2_test.go new file mode 100644 index 00000000..4df99b1b --- /dev/null +++ b/kms/pkcs11/softhsm2_test.go @@ -0,0 +1,60 @@ +// +build softhsm2,!yubihsm2 + +package pkcs11 + +import ( + "runtime" + "sync" + "testing" + + "github.com/ThalesIgnite/crypto11" +) + +var softHSM2Once sync.Once + +// mustPKCS11 configures a *PKCS11 KMS to be used with SoftHSM2. To initialize +// this tests, we should run: +// softhsm2-util --init-token --free \ +// --token pkcs11-test --label pkcs11-test \ +// --so-pin password --pin password +// +// To delete we should run: +// softhsm2-util --delete-token --token pkcs11-test +func mustPKCS11(t *testing.T) *PKCS11 { + t.Helper() + testModule = "SoftHSM2" + if runtime.GOARCH != "amd64" { + t.Fatalf("softHSM2 test skipped on %s:%s", runtime.GOOS, runtime.GOARCH) + } + + var path string + switch runtime.GOOS { + case "darwin": + path = "/usr/local/lib/softhsm/libsofthsm2.so" + case "linux": + path = "/usr/lib/softhsm/libsofthsm2.so" + default: + t.Skipf("softHSM2 test skipped on %s", runtime.GOOS) + return nil + } + p11, err := crypto11.Configure(&crypto11.Config{ + Path: path, + TokenLabel: "pkcs11-test", + Pin: "password", + }) + if err != nil { + t.Fatalf("failed to configure softHSM2 on %s: %v", runtime.GOOS, err) + } + + k := &PKCS11{ + p11: p11, + } + + // Setup + softHSM2Once.Do(func() { + teardown(t, k) + setup(t, k) + }) + + return k +} diff --git a/kms/pkcs11/yubihsm2_test.go b/kms/pkcs11/yubihsm2_test.go new file mode 100644 index 00000000..f0e7d965 --- /dev/null +++ b/kms/pkcs11/yubihsm2_test.go @@ -0,0 +1,55 @@ +// +build !softhsm2,yubihsm2 + +package pkcs11 + +import ( + "runtime" + "sync" + "testing" + + "github.com/ThalesIgnite/crypto11" +) + +var yubiHSM2Once sync.Once + +// mustPKCS11 configures a *PKCS11 KMS to be used with YubiHSM2. To initialize +// this tests, we should run: +// yubihsm-connector -d +func mustPKCS11(t *testing.T) *PKCS11 { + t.Helper() + testModule = "YubiHSM2" + if runtime.GOARCH != "amd64" { + t.Skipf("yubiHSM2 test skipped on %s:%s", runtime.GOOS, runtime.GOARCH) + } + + var path string + switch runtime.GOOS { + case "darwin": + path = "/usr/local/lib/pkcs11/yubihsm_pkcs11.dylib" + case "linux": + path = "/usr/lib/x86_64-linux-gnu/pkcs11/yubihsm_pkcs11.so" + default: + t.Skipf("yubiHSM2 test skipped on %s", runtime.GOOS) + return nil + } + p11, err := crypto11.Configure(&crypto11.Config{ + Path: path, + TokenLabel: "YubiHSM", + Pin: "0001password", + }) + if err != nil { + t.Fatalf("failed to configure yubiHSM2 on %s: %v", runtime.GOOS, err) + } + + k := &PKCS11{ + p11: p11, + } + + // Setup + yubiHSM2Once.Do(func() { + teardown(t, k) + setup(t, k) + }) + + return k +} From e78d45a060519e98aa9043a0757f2ab710cdf38c Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Jan 2021 19:46:48 -0800 Subject: [PATCH 08/33] Add benchmarks for signing operations. --- kms/pkcs11/benchmark_test.go | 82 ++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 kms/pkcs11/benchmark_test.go diff --git a/kms/pkcs11/benchmark_test.go b/kms/pkcs11/benchmark_test.go new file mode 100644 index 00000000..30e21117 --- /dev/null +++ b/kms/pkcs11/benchmark_test.go @@ -0,0 +1,82 @@ +// +build cgo + +package pkcs11 + +import ( + "crypto" + "crypto/rand" + "crypto/rsa" + "testing" + + "github.com/smallstep/certificates/kms/apiv1" +) + +func benchmarkSign(b *testing.B, signer crypto.Signer, opts crypto.SignerOpts) { + hash := opts.HashFunc() + h := hash.New() + h.Write([]byte("buggy-coheir-RUBRIC-rabbet-liberal-eaglet-khartoum-stagger")) + digest := h.Sum(nil) + b.ResetTimer() + for i := 0; i < b.N; i++ { + signer.Sign(rand.Reader, digest, opts) + } + b.StopTimer() +} + +func BenchmarkSignRSA(b *testing.B) { + k := setupPKCS11(b) + signer, err := k.CreateSigner(&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7371;object=rsa-key", + }) + if err != nil { + b.Fatalf("PKCS11.CreateSigner() error = %v", err) + } + benchmarkSign(b, signer, crypto.SHA256) +} + +func BenchmarkSignRSAPSS(b *testing.B) { + k := setupPKCS11(b) + signer, err := k.CreateSigner(&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7372;object=rsa-pss-key", + }) + if err != nil { + b.Fatalf("PKCS11.CreateSigner() error = %v", err) + } + benchmarkSign(b, signer, &rsa.PSSOptions{ + SaltLength: rsa.PSSSaltLengthEqualsHash, + Hash: crypto.SHA256, + }) +} + +func BenchmarkSignP256(b *testing.B) { + k := setupPKCS11(b) + signer, err := k.CreateSigner(&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7373;object=ecdsa-p256-key", + }) + if err != nil { + b.Fatalf("PKCS11.CreateSigner() error = %v", err) + } + benchmarkSign(b, signer, crypto.SHA256) +} + +func BenchmarkSignP384(b *testing.B) { + k := setupPKCS11(b) + signer, err := k.CreateSigner(&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7374;object=ecdsa-p384-key", + }) + if err != nil { + b.Fatalf("PKCS11.CreateSigner() error = %v", err) + } + benchmarkSign(b, signer, crypto.SHA384) +} + +func BenchmarkSignP521(b *testing.B) { + k := setupPKCS11(b) + signer, err := k.CreateSigner(&apiv1.CreateSignerRequest{ + SigningKey: "pkcs11:id=7375;object=ecdsa-p521-key", + }) + if err != nil { + b.Fatalf("PKCS11.CreateSigner() error = %v", err) + } + benchmarkSign(b, signer, crypto.SHA512) +} From 3a479cb0e8f1192512fd1631d72f834c6ec3cafc Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Jan 2021 19:47:44 -0800 Subject: [PATCH 09/33] Add support for nitrokey. --- kms/pkcs11/nitrokey_test.go | 60 +++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 kms/pkcs11/nitrokey_test.go diff --git a/kms/pkcs11/nitrokey_test.go b/kms/pkcs11/nitrokey_test.go new file mode 100644 index 00000000..6fb1cadf --- /dev/null +++ b/kms/pkcs11/nitrokey_test.go @@ -0,0 +1,60 @@ +// +build nitrokey + +package pkcs11 + +import ( + "runtime" + "sync" + + "github.com/ThalesIgnite/crypto11" +) + +var softHSM2Once sync.Once + +// mustPKCS11 configures a *PKCS11 KMS to be used with NitroKey through OpenSC. +// To initialize these tests we should run: +// sc-hsm-tool --initialize --so-pin 3537363231383830 --pin 123456 +// Or: +// pkcs11-tool --module /usr/local/lib/opensc-pkcs11.so \ +// --init-token --init-pin \ +// --so-pin=3537363231383830 --new-pin=123456 --pin=123456 \ +// --label="pkcs11-test" +func mustPKCS11(t TBTesting) *PKCS11 { + t.Helper() + testModule = "NitrokeyHSM" + if runtime.GOARCH != "amd64" { + t.Fatalf("softHSM2 test skipped on %s:%s", runtime.GOOS, runtime.GOARCH) + } + + var path string + switch runtime.GOOS { + case "darwin": + path = "/usr/local/lib/opensc-pkcs11.so" + case "linux": + path = "/usr/local/lib/opensc-pkcs11.so" + default: + t.Skipf("softHSM2 test skipped on %s", runtime.GOOS) + return nil + } + var zero int + p11, err := crypto11.Configure(&crypto11.Config{ + Path: path, + SlotNumber: &zero, + Pin: "123456", + }) + if err != nil { + t.Fatalf("failed to configure softHSM2 on %s: %v", runtime.GOOS, err) + } + + k := &PKCS11{ + p11: p11, + } + + // Setup + softHSM2Once.Do(func() { + teardown(t, k) + setup(t, k) + }) + + return k +} From b7afc92758ee9c576b0540ef4b1c73a80f50ee8d Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Jan 2021 19:48:08 -0800 Subject: [PATCH 10/33] Complete tests. --- kms/pkcs11/other_test.go | 11 +- kms/pkcs11/pkcs11.go | 40 ++++- kms/pkcs11/pkcs11_test.go | 306 +++++++++++++++++++++++++++++------- kms/pkcs11/setup_test.go | 42 +++-- kms/pkcs11/softhsm2_test.go | 7 +- kms/pkcs11/yubihsm2_test.go | 7 +- 6 files changed, 325 insertions(+), 88 deletions(-) diff --git a/kms/pkcs11/other_test.go b/kms/pkcs11/other_test.go index 602e101c..47a9ff83 100644 --- a/kms/pkcs11/other_test.go +++ b/kms/pkcs11/other_test.go @@ -1,4 +1,4 @@ -// +build !softhsm2,!yubihsm2 +// +build !softhsm2,!yubihsm2,!nitrokey package pkcs11 @@ -9,16 +9,14 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" - "fmt" "io" "math/big" - "testing" "github.com/ThalesIgnite/crypto11" "github.com/pkg/errors" ) -func mustPKCS11(t *testing.T) *PKCS11 { +func mustPKCS11(t TBTesting) *PKCS11 { t.Helper() testModule = "Golang crypto" k := &PKCS11{ @@ -63,10 +61,7 @@ func (s *stubPKCS11) FindKeyPair(id, label []byte) (crypto11.Signer, error) { if id == nil && label == nil { return nil, errors.New("id and label cannot both be nil") } - i, ok := s.signerIndex[newKey(id, label, nil)] - fmt.Println(i, ok) - if ok { - + if i, ok := s.signerIndex[newKey(id, label, nil)]; ok { return s.signers[i], nil } return nil, nil diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index d45f8045..9ac806d8 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -10,6 +10,7 @@ import ( "encoding/hex" "fmt" "math/big" + "strconv" "github.com/ThalesIgnite/crypto11" "github.com/pkg/errors" @@ -17,6 +18,9 @@ import ( "github.com/smallstep/certificates/kms/uri" ) +// Scheme is the scheme used in uris. +const Scheme = "pkcs11" + // DefaultRSASize is the number of bits of a new RSA key if not bitsize has been // specified. const DefaultRSASize = 3072 @@ -46,14 +50,22 @@ type PKCS11 struct { func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { var config crypto11.Config if opts.URI != "" { - u, err := uri.ParseWithScheme("pkcs11", opts.URI) + u, err := uri.ParseWithScheme(Scheme, opts.URI) if err != nil { return nil, err } + + config.Pin = u.Pin() config.Path = u.Get("module-path") config.TokenLabel = u.Get("token") config.TokenSerial = u.Get("serial") - config.Pin = u.Pin() + if v := u.Get("slot-id"); v != "" { + n, err := strconv.Atoi(v) + if err != nil { + return nil, errors.Wrap(err, "kms uri 'slot-id' is not valid") + } + config.SlotNumber = &n + } } if config.Pin == "" && opts.Pin != "" { config.Pin = opts.Pin @@ -62,12 +74,16 @@ func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { switch { case config.Path == "": return nil, errors.New("kms uri 'module-path' are required") - case config.TokenLabel == "" && config.TokenSerial == "": - return nil, errors.New("kms uri 'token' or 'serial' are required") + case config.TokenLabel == "" && config.TokenSerial == "" && config.SlotNumber == nil: + return nil, errors.New("kms uri 'token', 'serial' or 'slot-id' are required") case config.Pin == "": return nil, errors.New("kms 'pin' cannot be empty") case config.TokenLabel != "" && config.TokenSerial != "": - return nil, errors.New("kms uri 'token' or 'serial' are mutually exclusive") + return nil, errors.New("kms uri 'token' and 'serial' are mutually exclusive") + case config.TokenLabel != "" && config.SlotNumber != nil: + return nil, errors.New("kms uri 'token' and 'slot-id' are mutually exclusive") + case config.TokenSerial != "" && config.SlotNumber != nil: + return nil, errors.New("kms uri 'serial' and 'slot-id' are mutually exclusive") } p11, err := p11Configure(&config) @@ -167,6 +183,16 @@ func (k *PKCS11) StoreCertificate(req *apiv1.StoreCertificateRequest) error { return errors.Wrap(err, "storeCertificate failed") } + cert, err := k.p11.FindCertificate(id, object, nil) + if err != nil { + return errors.Wrap(err, "storeCertificate failed") + } + if cert != nil { + return errors.Wrap(apiv1.ErrAlreadyExists{ + Message: req.Name + " already exists", + }, "storeCertificate failed") + } + if err := k.p11.ImportCertificateWithLabel(id, object, req.Certificate); err != nil { return errors.Wrap(err, "storeCertificate failed") } @@ -218,7 +244,7 @@ func toByte(s string) []byte { } func parseObject(rawuri string) ([]byte, []byte, error) { - u, err := uri.ParseWithScheme("pkcs11", rawuri) + u, err := uri.ParseWithScheme(Scheme, rawuri) if err != nil { return nil, nil, err } @@ -290,7 +316,7 @@ func findSigner(ctx P11, rawuri string) (crypto11.Signer, error) { } func findCertificate(ctx P11, rawuri string) (*x509.Certificate, error) { - u, err := uri.ParseWithScheme("pkcs11", rawuri) + u, err := uri.ParseWithScheme(Scheme, rawuri) if err != nil { return nil, err } diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index 3c4d0bc3..e69b5598 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -12,14 +12,30 @@ import ( "crypto/x509" "math/big" "reflect" + "strings" "testing" + "github.com/ThalesIgnite/crypto11" + "github.com/pkg/errors" "github.com/smallstep/certificates/kms/apiv1" "golang.org/x/crypto/cryptobyte" "golang.org/x/crypto/cryptobyte/asn1" ) func TestNew(t *testing.T) { + tmp := p11Configure + t.Cleanup(func() { + p11Configure = tmp + }) + + k := mustPKCS11(t) + p11Configure = func(config *crypto11.Config) (P11, error) { + if strings.Index(config.Path, "fail") >= 0 { + return nil, errors.New("an error") + } + return k.p11, nil + } + type args struct { ctx context.Context opts apiv1.Options @@ -29,7 +45,71 @@ func TestNew(t *testing.T) { args args want *PKCS11 wantErr bool - }{} + }{ + {"ok", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;token=pkcs11-test?pin-value=password", + }}, k, false}, + {"ok with serial", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;serial=0123456789?pin-value=password", + }}, k, false}, + {"ok with slot-id", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;slot-id=0?pin-value=password", + }}, k, false}, + {"ok with pin", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;token=pkcs11-test", + Pin: "passowrd", + }}, k, false}, + {"fail missing module", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:token=pkcs11-test", + Pin: "passowrd", + }}, nil, true}, + {"fail missing pin", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;token=pkcs11-test", + }}, nil, true}, + {"fail missing token/serial/slot-id", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so", + Pin: "passowrd", + }}, nil, true}, + {"fail token+serial+slot-id", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;token=pkcs11-test;serial=0123456789;slot-id=0", + Pin: "passowrd", + }}, nil, true}, + {"fail token+serial", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;token=pkcs11-test;serial=0123456789", + Pin: "passowrd", + }}, nil, true}, + {"fail token+slot-id", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;token=pkcs11-test;slot-id=0", + Pin: "passowrd", + }}, nil, true}, + {"fail serial+slot-id", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;serial=0123456789;slot-id=0", + Pin: "passowrd", + }}, nil, true}, + {"fail slot-id", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/softhsm/libsofthsm2.so;slot-id=x?pin-value=password", + }}, nil, true}, + {"fail scheme", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "foo:module-path=/usr/local/lib/softhsm/libsofthsm2.so;token=pkcs11-test?pin-value=password", + }}, nil, true}, + {"fail configure", args{context.Background(), apiv1.Options{ + Type: "pkcs11", + URI: "pkcs11:module-path=/usr/local/lib/fail.so;token=pkcs11-test?pin-value=password", + }}, nil, true}, + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got, err := New(tt.args.ctx, tt.args.opts) @@ -104,8 +184,7 @@ func TestPKCS11_CreateKey(t *testing.T) { k := setupPKCS11(t) // Make sure to delete the created key - keyName := "pkcs11:id=7771;object=create-key" - k.DeleteKey(keyName) + k.DeleteKey(testObject) type args struct { req *apiv1.CreateKeyRequest @@ -118,140 +197,140 @@ func TestPKCS11_CreateKey(t *testing.T) { }{ // SoftHSM2 {"default", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &ecdsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"RSA SHA256WithRSA", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, SignatureAlgorithm: apiv1.SHA256WithRSA, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"RSA SHA384WithRSA", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, SignatureAlgorithm: apiv1.SHA384WithRSA, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"RSA SHA512WithRSA", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, SignatureAlgorithm: apiv1.SHA512WithRSA, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"RSA SHA256WithRSAPSS", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, SignatureAlgorithm: apiv1.SHA256WithRSAPSS, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"RSA SHA384WithRSAPSS", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, SignatureAlgorithm: apiv1.SHA384WithRSAPSS, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"RSA SHA512WithRSAPSS", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, SignatureAlgorithm: apiv1.SHA512WithRSAPSS, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"RSA 2048", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, SignatureAlgorithm: apiv1.SHA256WithRSA, Bits: 2048, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"RSA 4096", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, SignatureAlgorithm: apiv1.SHA256WithRSA, Bits: 4096, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &rsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"ECDSA P256", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, SignatureAlgorithm: apiv1.ECDSAWithSHA256, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &ecdsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"ECDSA P384", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, SignatureAlgorithm: apiv1.ECDSAWithSHA384, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &ecdsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"ECDSA P521", args{&apiv1.CreateKeyRequest{ - Name: keyName, + Name: testObject, SignatureAlgorithm: apiv1.ECDSAWithSHA512, }}, &apiv1.CreateKeyResponse{ - Name: keyName, + Name: testObject, PublicKey: &ecdsa.PublicKey{}, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: keyName, + SigningKey: testObject, }, }, false}, {"fail name", args{&apiv1.CreateKeyRequest{ Name: "", }}, nil, true}, {"fail bits", args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=9999;object=rsa-create-key", + Name: "pkcs11:id=9999;object=create-key", Bits: -1, SignatureAlgorithm: apiv1.SHA256WithRSAPSS, }}, nil, true}, {"fail ed25519", args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=9999;object=rsa-create-key", + Name: "pkcs11:id=9999;object=create-key", SignatureAlgorithm: apiv1.PureEd25519, }}, nil, true}, {"fail unknown", args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=9999;object=rsa-create-key", + Name: "pkcs11:id=9999;object=create-key", SignatureAlgorithm: apiv1.SignatureAlgorithm(100), }}, nil, true}, {"fail uri", args{&apiv1.CreateKeyRequest{ @@ -262,6 +341,10 @@ func TestPKCS11_CreateKey(t *testing.T) { Name: "pkcs11:foo=bar", SignatureAlgorithm: apiv1.SHA256WithRSAPSS, }}, nil, true}, + {"fail already exists", args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=7373;object=ecdsa-p256-key", + SignatureAlgorithm: apiv1.ECDSAWithSHA256, + }}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -322,8 +405,11 @@ func TestPKCS11_CreateSigner(t *testing.T) { SigningKey: "pkcs11:id=7371;object=rsa-key", }}, apiv1.SHA256WithRSA, crypto.SHA256, false}, {"RSA PSS", args{&apiv1.CreateSignerRequest{ - SigningKey: "pkcs11:id=7371;object=rsa-key", - }}, apiv1.SHA256WithRSA, crypto.SHA256, false}, + SigningKey: "pkcs11:id=7372;object=rsa-pss-key", + }}, apiv1.SHA256WithRSAPSS, &rsa.PSSOptions{ + SaltLength: rsa.PSSSaltLengthEqualsHash, + Hash: crypto.SHA256, + }, false}, {"ECDSA P256", args{&apiv1.CreateSignerRequest{ SigningKey: "pkcs11:id=7373;object=ecdsa-p256-key", }}, apiv1.ECDSAWithSHA256, crypto.SHA256, false}, @@ -406,22 +492,31 @@ func TestPKCS11_LoadCertificate(t *testing.T) { wantErr bool }{ {"load", args{&apiv1.LoadCertificateRequest{ - Name: "pkcs11:id=7370;object=root", + Name: "pkcs11:id=7376;object=test-root", }}, getCertFn(0, 0), false}, {"load by id", args{&apiv1.LoadCertificateRequest{ - Name: "pkcs11:id=7370", + Name: "pkcs11:id=7376", }}, getCertFn(0, 0), false}, {"load by label", args{&apiv1.LoadCertificateRequest{ - Name: "pkcs11:object=root", + Name: "pkcs11:object=test-root", + }}, getCertFn(0, 0), false}, + {"load by serial", args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:serial=64", }}, getCertFn(0, 0), false}, {"fail missing", args{&apiv1.LoadCertificateRequest{ - Name: "pkcs11:id=9999;object=root", + Name: "pkcs11:id=9999;object=test-root", }}, nil, true}, {"fail name", args{&apiv1.LoadCertificateRequest{ Name: "", }}, nil, true}, {"fail uri", args{&apiv1.LoadCertificateRequest{ - Name: "pkcs11:id=xxxx;object=root", + Name: "pkcs11:id=xxxx;object=test-root", + }}, nil, true}, + {"fail scheme", args{&apiv1.LoadCertificateRequest{ + Name: "foo:id=7376;object=test-root", + }}, nil, true}, + {"fail serial", args{&apiv1.LoadCertificateRequest{ + Name: "pkcs11:serial=foo", }}, nil, true}, {"fail FindCertificate", args{&apiv1.LoadCertificateRequest{ Name: "pkcs11:foo=bar", @@ -460,6 +555,11 @@ func TestPKCS11_StoreCertificate(t *testing.T) { t.Fatalf("x509.CreateCertificate() error = %v", err) } + // Make sure to delete the created certificate + t.Cleanup(func() { + k.DeleteCertificate(testObject) + }) + type args struct { req *apiv1.StoreCertificateRequest } @@ -469,19 +569,23 @@ func TestPKCS11_StoreCertificate(t *testing.T) { wantErr bool }{ {"ok", args{&apiv1.StoreCertificateRequest{ - Name: "pkcs11:id=7771;object=root", + Name: testObject, Certificate: cert, }}, false}, + {"fail already exists", args{&apiv1.StoreCertificateRequest{ + Name: testObject, + Certificate: cert, + }}, true}, {"fail name", args{&apiv1.StoreCertificateRequest{ Name: "", Certificate: cert, }}, true}, {"fail certificate", args{&apiv1.StoreCertificateRequest{ - Name: "pkcs11:id=7771;object=root", + Name: testObject, Certificate: nil, }}, true}, {"fail uri", args{&apiv1.StoreCertificateRequest{ - Name: "http:id=7771;object=root", + Name: "http:id=7770;object=create-cert", Certificate: cert, }}, true}, {"fail ImportCertificateWithLabel", args{&apiv1.StoreCertificateRequest{ @@ -504,9 +608,101 @@ func TestPKCS11_StoreCertificate(t *testing.T) { if !reflect.DeepEqual(got, cert) { t.Errorf("PKCS11.LoadCertificate() = %v, want %v", got, cert) } - if err := k.DeleteCertificate(tt.args.req.Name); err != nil { - t.Errorf("PKCS11.DeleteCertificate() error = %v", err) - } + } + }) + } +} + +func TestPKCS11_DeleteKey(t *testing.T) { + k := setupPKCS11(t) + + type args struct { + uri string + } + tests := []struct { + name string + args args + wantErr bool + }{ + {"delete", args{testObject}, false}, + {"delete by id", args{testObjectByID}, false}, + {"delete by label", args{testObjectByLabel}, false}, + {"delete missing", args{"pkcs11:id=9999;object=missing-key"}, false}, + {"fail name", args{""}, true}, + {"fail uri", args{"pkcs11:id=xxxx;object=missing-key"}, true}, + {"fail FindKeyPair", args{"pkcs11:foo=bar"}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, err := k.CreateKey(&apiv1.CreateKeyRequest{ + Name: testObject, + }); err != nil { + t.Fatalf("PKCS1.CreateKey() error = %v", err) + } + if err := k.DeleteKey(tt.args.uri); (err != nil) != tt.wantErr { + t.Errorf("PKCS11.DeleteKey() error = %v, wantErr %v", err, tt.wantErr) + } + if _, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ + Name: tt.args.uri, + }); err == nil { + t.Error("PKCS11.GetPublicKey() public key found and not expected") + } + // Make sure to delete the created one. + if err := k.DeleteKey(testObject); err != nil { + t.Errorf("PKCS11.DeleteKey() error = %v", err) + } + }) + } +} + +func TestPKCS11_DeleteCertificate(t *testing.T) { + k := setupPKCS11(t) + + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("ed25519.GenerateKey() error = %v", err) + } + + cert, err := generateCertificate(pub, priv) + if err != nil { + t.Fatalf("x509.CreateCertificate() error = %v", err) + } + + type args struct { + uri string + } + tests := []struct { + name string + args args + wantErr bool + }{ + {"delete", args{testObject}, false}, + {"delete by id", args{testObjectByID}, false}, + {"delete by label", args{testObjectByLabel}, false}, + {"delete missing", args{"pkcs11:id=9999;object=missing-key"}, false}, + {"fail name", args{""}, true}, + {"fail uri", args{"pkcs11:id=xxxx;object=missing-key"}, true}, + {"fail DeleteCertificate", args{"pkcs11:foo=bar"}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := k.StoreCertificate(&apiv1.StoreCertificateRequest{ + Name: testObject, + Certificate: cert, + }); err != nil { + t.Fatalf("PKCS11.StoreCertificate() error = %v", err) + } + if err := k.DeleteCertificate(tt.args.uri); (err != nil) != tt.wantErr { + t.Errorf("PKCS11.DeleteCertificate() error = %v, wantErr %v", err, tt.wantErr) + } + if _, err := k.LoadCertificate(&apiv1.LoadCertificateRequest{ + Name: tt.args.uri, + }); err == nil { + t.Error("PKCS11.LoadCertificate() certificate found and not expected") + } + // Make sure to delete the created one. + if err := k.DeleteCertificate(testObject); err != nil { + t.Errorf("PKCS11.DeleteCertificate() error = %v", err) } }) } diff --git a/kms/pkcs11/setup_test.go b/kms/pkcs11/setup_test.go index b912f79c..c9ff9311 100644 --- a/kms/pkcs11/setup_test.go +++ b/kms/pkcs11/setup_test.go @@ -8,17 +8,18 @@ import ( "crypto/x509" "crypto/x509/pkix" "math/big" - "testing" "time" "github.com/pkg/errors" - "github.com/smallstep/certificates/kms/apiv1" ) var ( - testModule = "" - testKeys = []struct { + testModule = "" + testObject = "pkcs11:id=7370;object=test-name" + testObjectByID = "pkcs11:id=7370" + testObjectByLabel = "pkcs11:object=test-name" + testKeys = []struct { Name string SignatureAlgorithm apiv1.SignatureAlgorithm Bits int @@ -35,10 +36,19 @@ var ( Key string Certificates []*x509.Certificate }{ - {"pkcs11:id=7370;object=root", "pkcs11:id=7373;object=ecdsa-p256-key", nil}, + {"pkcs11:id=7376;object=test-root", "pkcs11:id=7373;object=ecdsa-p256-key", nil}, } ) +type TBTesting interface { + Helper() + Cleanup(f func()) + Log(args ...interface{}) + Errorf(format string, args ...interface{}) + Fatalf(format string, args ...interface{}) + Skipf(format string, args ...interface{}) +} + func generateCertificate(pub crypto.PublicKey, signer crypto.Signer) (*x509.Certificate, error) { now := time.Now() template := &x509.Certificate{ @@ -60,7 +70,7 @@ func generateCertificate(pub crypto.PublicKey, signer crypto.Signer) (*x509.Cert return x509.ParseCertificate(b) } -func setup(t *testing.T, k *PKCS11) { +func setup(t TBTesting, k *PKCS11) { t.Log("Running using", testModule) for _, tk := range testKeys { _, err := k.CreateKey(&apiv1.CreateKeyRequest{ @@ -91,15 +101,26 @@ func setup(t *testing.T, k *PKCS11) { if err := k.StoreCertificate(&apiv1.StoreCertificateRequest{ Name: c.Name, Certificate: cert, - }); err != nil { - t.Errorf("PKCS1.StoreCertificate() error = %v", err) + }); err != nil && !errors.Is(errors.Cause(err), apiv1.ErrAlreadyExists{ + Message: c.Name + " already exists", + }) { + t.Errorf("PKCS1.StoreCertificate() error = %+v", err) continue } testCerts[i].Certificates = append(testCerts[i].Certificates, cert) } } -func teardown(t *testing.T, k *PKCS11) { +func teardown(t TBTesting, k *PKCS11) { + testObjects := []string{testObject, testObjectByID, testObjectByLabel} + for _, name := range testObjects { + if err := k.DeleteKey(name); err != nil { + t.Errorf("PKCS11.DeleteKey() error = %v", err) + } + if err := k.DeleteCertificate(name); err != nil { + t.Errorf("PKCS11.DeleteCertificate() error = %v", err) + } + } for _, tk := range testKeys { if err := k.DeleteKey(tk.Name); err != nil { t.Errorf("PKCS11.DeleteKey() error = %v", err) @@ -112,7 +133,8 @@ func teardown(t *testing.T, k *PKCS11) { } } -func setupPKCS11(t *testing.T) *PKCS11 { +func setupPKCS11(t TBTesting) *PKCS11 { + t.Helper() k := mustPKCS11(t) t.Cleanup(func() { k.Close() diff --git a/kms/pkcs11/softhsm2_test.go b/kms/pkcs11/softhsm2_test.go index 4df99b1b..379d7a11 100644 --- a/kms/pkcs11/softhsm2_test.go +++ b/kms/pkcs11/softhsm2_test.go @@ -1,11 +1,10 @@ -// +build softhsm2,!yubihsm2 +// +build softhsm2 package pkcs11 import ( "runtime" "sync" - "testing" "github.com/ThalesIgnite/crypto11" ) @@ -13,14 +12,14 @@ import ( var softHSM2Once sync.Once // mustPKCS11 configures a *PKCS11 KMS to be used with SoftHSM2. To initialize -// this tests, we should run: +// these tests, we should run: // softhsm2-util --init-token --free \ // --token pkcs11-test --label pkcs11-test \ // --so-pin password --pin password // // To delete we should run: // softhsm2-util --delete-token --token pkcs11-test -func mustPKCS11(t *testing.T) *PKCS11 { +func mustPKCS11(t TBTesting) *PKCS11 { t.Helper() testModule = "SoftHSM2" if runtime.GOARCH != "amd64" { diff --git a/kms/pkcs11/yubihsm2_test.go b/kms/pkcs11/yubihsm2_test.go index f0e7d965..7d508872 100644 --- a/kms/pkcs11/yubihsm2_test.go +++ b/kms/pkcs11/yubihsm2_test.go @@ -1,11 +1,10 @@ -// +build !softhsm2,yubihsm2 +// +build yubihsm2 package pkcs11 import ( "runtime" "sync" - "testing" "github.com/ThalesIgnite/crypto11" ) @@ -13,9 +12,9 @@ import ( var yubiHSM2Once sync.Once // mustPKCS11 configures a *PKCS11 KMS to be used with YubiHSM2. To initialize -// this tests, we should run: +// these tests, we should run: // yubihsm-connector -d -func mustPKCS11(t *testing.T) *PKCS11 { +func mustPKCS11(t TBTesting) *PKCS11 { t.Helper() testModule = "YubiHSM2" if runtime.GOARCH != "amd64" { From 84a3c8c9845ee51b6da2274ea6867bcac898b431 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Jan 2021 19:51:17 -0800 Subject: [PATCH 11/33] Rename nitrokey initialization to opensc. --- kms/pkcs11/{nitrokey_test.go => opensc_test.go} | 14 +++++++------- kms/pkcs11/other_test.go | 2 +- kms/pkcs11/pkcs11.go | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) rename kms/pkcs11/{nitrokey_test.go => opensc_test.go} (70%) diff --git a/kms/pkcs11/nitrokey_test.go b/kms/pkcs11/opensc_test.go similarity index 70% rename from kms/pkcs11/nitrokey_test.go rename to kms/pkcs11/opensc_test.go index 6fb1cadf..f3b61932 100644 --- a/kms/pkcs11/nitrokey_test.go +++ b/kms/pkcs11/opensc_test.go @@ -1,4 +1,4 @@ -// +build nitrokey +// +build opensc package pkcs11 @@ -11,8 +11,8 @@ import ( var softHSM2Once sync.Once -// mustPKCS11 configures a *PKCS11 KMS to be used with NitroKey through OpenSC. -// To initialize these tests we should run: +// mustPKCS11 configures a *PKCS11 KMS to be used with OpenSC, using for example +// a Nitrokey HSM. To initialize these tests we should run: // sc-hsm-tool --initialize --so-pin 3537363231383830 --pin 123456 // Or: // pkcs11-tool --module /usr/local/lib/opensc-pkcs11.so \ @@ -21,9 +21,9 @@ var softHSM2Once sync.Once // --label="pkcs11-test" func mustPKCS11(t TBTesting) *PKCS11 { t.Helper() - testModule = "NitrokeyHSM" + testModule = "OpenSC" if runtime.GOARCH != "amd64" { - t.Fatalf("softHSM2 test skipped on %s:%s", runtime.GOOS, runtime.GOARCH) + t.Fatalf("opensc test skipped on %s:%s", runtime.GOOS, runtime.GOARCH) } var path string @@ -33,7 +33,7 @@ func mustPKCS11(t TBTesting) *PKCS11 { case "linux": path = "/usr/local/lib/opensc-pkcs11.so" default: - t.Skipf("softHSM2 test skipped on %s", runtime.GOOS) + t.Skipf("opensc test skipped on %s", runtime.GOOS) return nil } var zero int @@ -43,7 +43,7 @@ func mustPKCS11(t TBTesting) *PKCS11 { Pin: "123456", }) if err != nil { - t.Fatalf("failed to configure softHSM2 on %s: %v", runtime.GOOS, err) + t.Fatalf("failed to configure opensc on %s: %v", runtime.GOOS, err) } k := &PKCS11{ diff --git a/kms/pkcs11/other_test.go b/kms/pkcs11/other_test.go index 47a9ff83..79d734e7 100644 --- a/kms/pkcs11/other_test.go +++ b/kms/pkcs11/other_test.go @@ -1,4 +1,4 @@ -// +build !softhsm2,!yubihsm2,!nitrokey +// +build !softhsm2,!yubihsm2,!opensc package pkcs11 diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 9ac806d8..5d4076a2 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -345,5 +345,5 @@ func findCertificate(ctx P11, rawuri string) (*x509.Certificate, error) { if cert == nil { return nil, errors.Errorf("certificate with uri %s not found", rawuri) } - return cert, nil + } From 50e9018a44d2bcd1615ed37faebf207bb29a3b74 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Jan 2021 19:53:25 -0800 Subject: [PATCH 12/33] Fix missing return. --- kms/pkcs11/pkcs11.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 5d4076a2..9ac806d8 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -345,5 +345,5 @@ func findCertificate(ctx P11, rawuri string) (*x509.Certificate, error) { if cert == nil { return nil, errors.Errorf("certificate with uri %s not found", rawuri) } - + return cert, nil } From 162c5357055e459cc6d8184fe3c216b1cf85291a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 28 Jan 2021 20:13:28 -0800 Subject: [PATCH 13/33] Add option to not store certificates in the pkcs11 module. --- cmd/step-pkcs11-init/main.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/step-pkcs11-init/main.go b/cmd/step-pkcs11-init/main.go index 174eed78..225557d4 100644 --- a/cmd/step-pkcs11-init/main.go +++ b/cmd/step-pkcs11-init/main.go @@ -42,6 +42,7 @@ type Config struct { RootFile string KeyFile string Pin string + NoCerts bool EnableSSH bool Force bool } @@ -105,6 +106,7 @@ func main() { flag.StringVar(&c.RootFile, "root", "", "Path to the root certificate to use.") flag.StringVar(&c.KeyFile, "key", "", "Path to the root key to use.") 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.Usage = usage flag.Parse() @@ -145,7 +147,7 @@ func main() { } if !c.Force { for _, u := range certUris { - if u != "" { + if u != "" && !c.NoCerts { checkObject(k, u) } } @@ -161,7 +163,7 @@ func main() { }) if ok { for _, u := range certUris { - if u != "" { + if u != "" && !c.NoCerts { if err := deleter.DeleteCertificate(u); err != nil { fatal(err) } @@ -285,7 +287,7 @@ func createPKI(k kms.KeyManager, c Config) error { return errors.Wrap(err, "error parsing root certificate") } - if cm, ok := k.(kms.CertificateManager); ok { + if cm, ok := k.(kms.CertificateManager); ok && !c.NoCerts { if err = cm.StoreCertificate(&apiv1.StoreCertificateRequest{ Name: c.RootObject, Certificate: root, @@ -362,7 +364,7 @@ func createPKI(k kms.KeyManager, c Config) error { return errors.Wrap(err, "error parsing intermediate certificate") } - if cm, ok := k.(kms.CertificateManager); ok { + if cm, ok := k.(kms.CertificateManager); ok && !c.NoCerts { if err = cm.StoreCertificate(&apiv1.StoreCertificateRequest{ Name: c.CrtObject, Certificate: intermediate, From 7f9d7eadc9e9c8647df0d3b0836296885348d3cf Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 29 Jan 2021 13:31:07 -0800 Subject: [PATCH 14/33] Attempt to delete key and certificate with the same name. Nitrokey will override the label of the key with the certificate one. If they are stored with the same id. --- cmd/step-pkcs11-init/main.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/cmd/step-pkcs11-init/main.go b/cmd/step-pkcs11-init/main.go index 225557d4..e7643265 100644 --- a/cmd/step-pkcs11-init/main.go +++ b/cmd/step-pkcs11-init/main.go @@ -149,6 +149,7 @@ func main() { for _, u := range certUris { if u != "" && !c.NoCerts { checkObject(k, u) + checkCertificate(k, u) } } for _, u := range keyUris { @@ -164,6 +165,11 @@ func main() { if ok { for _, u := range certUris { if u != "" && !c.NoCerts { + // Some HSMs like Nitrokey will overwrite the key with the + // certificate label. + if err := deleter.DeleteKey(u); err != nil { + fatal(err) + } if err := deleter.DeleteCertificate(u); err != nil { fatal(err) } @@ -215,6 +221,18 @@ COPYRIGHT os.Exit(1) } +func checkCertificate(k kms.KeyManager, rawuri string) { + if cm, ok := k.(kms.CertificateManager); ok { + if _, err := cm.LoadCertificate(&apiv1.LoadCertificateRequest{ + Name: rawuri, + }); err == nil { + fmt.Fprintf(os.Stderr, "⚠️ Your PKCS #11 module already has a certificate on %s.\n", rawuri) + fmt.Fprintln(os.Stderr, " If you want to delete it and start fresh, use `--force`.") + os.Exit(1) + } + } +} + func checkObject(k kms.KeyManager, rawuri string) { if _, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ Name: rawuri, From 51ac28656efa4c64698a33324e33e6b5c6d43f29 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 29 Jan 2021 16:11:25 -0800 Subject: [PATCH 15/33] Fix protection level for host keys in cloudkms script. Fixes #460 --- cmd/step-cloudkms-init/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/step-cloudkms-init/main.go b/cmd/step-cloudkms-init/main.go index d23a31e9..69573c5d 100644 --- a/cmd/step-cloudkms-init/main.go +++ b/cmd/step-cloudkms-init/main.go @@ -234,7 +234,7 @@ func createSSH(c *cloudkms.CloudKMS, project, location, keyRing string, protecti resp, err = c.CreateKey(&apiv1.CreateKeyRequest{ Name: parent + "/ssh-host-key", SignatureAlgorithm: apiv1.ECDSAWithSHA256, - ProtectionLevel: apiv1.Software, + ProtectionLevel: protectionLevel, }) if err != nil { return err From a74fc7a0b2a4a38eb694cfc7be436e87716076cf Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 1 Feb 2021 14:16:08 -0800 Subject: [PATCH 16/33] Remove unnecessary methods and add missing tests. --- kms/uri/uri.go | 108 ++++++-------------------------------------- kms/uri/uri_test.go | 63 ++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 94 deletions(-) diff --git a/kms/uri/uri.go b/kms/uri/uri.go index a5a4c55b..8e30a7c5 100644 --- a/kms/uri/uri.go +++ b/kms/uri/uri.go @@ -90,24 +90,30 @@ func (u *URI) Get(key string) string { if v == "" { v = u.URL.Query().Get(key) } - return StringDecode(v) + return v } -// GetHex returns the first value in the uri with the give n key, it will return -// empty nil if that field is not present. -func (u *URI) GetHex(key string) ([]byte, error) { - v := u.Values.Get(key) +// GetEncoded returns the first value in the uri with the give n key, it will +// return empty nil if that field is not present or is empty. If the return +// value is hex encoded it will decode it and return it. +func (u *URI) GetEncoded(key string) []byte { + v := u.Get(key) if v == "" { - v = u.URL.Query().Get(key) + return nil } - return HexDecode(v) + if len(v)%2 == 0 { + if b, err := hex.DecodeString(v); err == nil { + return b + } + } + return []byte(v) } // Pin returns the pin encoded in the url. It will read the pin from the // pin-value or the pin-source attributes. func (u *URI) Pin() string { if value := u.Get("pin-value"); value != "" { - return StringDecode(value) + return value } if path := u.Get("pin-source"); path != "" { if b, err := readFile(path); err == nil { @@ -128,89 +134,3 @@ func readFile(path string) ([]byte, error) { } return b, nil } - -// PercentEncode encodes the given bytes using the percent encoding described in -// RFC3986 (https://tools.ietf.org/html/rfc3986). -func PercentEncode(b []byte) string { - buf := new(strings.Builder) - for _, v := range b { - buf.WriteString("%" + hex.EncodeToString([]byte{v})) - } - return buf.String() -} - -// PercentDecode decodes the given string using the percent encoding described -// in RFC3986 (https://tools.ietf.org/html/rfc3986). -func PercentDecode(s string) ([]byte, error) { - if len(s)%3 != 0 { - return nil, errors.Errorf("error parsing %s: wrong length", s) - } - - var first string - buf := new(bytes.Buffer) - for i, r := range s { - mod := i % 3 - rr := string(r) - switch mod { - case 0: - if r != '%' { - return nil, errors.Errorf("error parsing %s: expected %% and found %s in position %d", s, rr, i) - } - case 1: - if !isHex(r) { - return nil, errors.Errorf("error parsing %s: %s in position %d is not an hexadecimal number", s, rr, i) - } - first = string(r) - case 2: - if !isHex(r) { - return nil, errors.Errorf("error parsing %s: %s in position %d is not an hexadecimal number", s, rr, i) - } - b, err := hex.DecodeString(first + rr) - if err != nil { - return nil, errors.Wrapf(err, "error parsing %s", s) - } - buf.Write(b) - } - } - return buf.Bytes(), nil -} - -// StringDecode returns the string given, but it will use Percent-Encoding if -// the string is percent encoded. -func StringDecode(s string) string { - if strings.HasPrefix(s, "%") { - if b, err := PercentDecode(s); err == nil { - return string(b) - } - } - return s -} - -// HexDecode deocdes the string s using Percent-Encoding or regular hex -// encoding. -func HexDecode(s string) ([]byte, error) { - if s == "" { - return nil, nil - } else if strings.HasPrefix(s, "%") { - return PercentDecode(s) - } - - b, err := hex.DecodeString(s) - if err != nil { - return nil, errors.Wrapf(err, "error parsing %s", s) - } - return b, nil -} - -func isHex(r rune) bool { - switch { - case r >= '0' && r <= '9': - return true - case r >= 'a' && r <= 'f': - return true - case r >= 'A' && r <= 'F': - return true - default: - return false - } -} diff --git a/kms/uri/uri_test.go b/kms/uri/uri_test.go index 3f001afc..3de07456 100644 --- a/kms/uri/uri_test.go +++ b/kms/uri/uri_test.go @@ -201,3 +201,66 @@ func TestURI_Get(t *testing.T) { }) } } + +func TestURI_GetEncoded(t *testing.T) { + mustParse := func(s string) *URI { + u, err := Parse(s) + if err != nil { + t.Fatal(err) + } + return u + } + type args struct { + key string + } + tests := []struct { + name string + uri *URI + args args + want []byte + }{ + {"ok", mustParse("yubikey:slot-id=9a"), args{"slot-id"}, []byte{0x9a}}, + {"ok first", mustParse("yubikey:slot-id=9a9b;slot-id=9b"), args{"slot-id"}, []byte{0x9a, 0x9b}}, + {"ok percent", mustParse("yubikey:slot-id=9a;foo=%9a%9b%9c"), args{"foo"}, []byte{0x9a, 0x9b, 0x9c}}, + {"ok in query", mustParse("yubikey:slot-id=9a?foo=9a"), args{"foo"}, []byte{0x9a}}, + {"ok in query percent", mustParse("yubikey:slot-id=9a?foo=%9a"), args{"foo"}, []byte{0x9a}}, + {"ok missing", mustParse("yubikey:slot-id=9a"), args{"foo"}, nil}, + {"ok missing query", mustParse("yubikey:slot-id=9a?bar=zar"), args{"foo"}, nil}, + {"ok no hex", mustParse("yubikey:slot-id=09a?bar=zar"), args{"slot-id"}, []byte{'0', '9', 'a'}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.uri.GetEncoded(tt.args.key) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("URI.GetEncoded() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestURI_Pin(t *testing.T) { + mustParse := func(s string) *URI { + u, err := Parse(s) + if err != nil { + t.Fatal(err) + } + return u + } + tests := []struct { + name string + uri *URI + want string + }{ + {"from value", mustParse("pkcs11:id=%72%73?pin-value=0123456789"), "0123456789"}, + {"from source", mustParse("pkcs11:id=%72%73?pin-source=testdata/pin.txt"), "trim-this-pin"}, + {"from missing", mustParse("pkcs11:id=%72%73"), ""}, + {"from source missing", mustParse("pkcs11:id=%72%73?pin-source=testdata/foo.txt"), ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.uri.Pin(); got != tt.want { + t.Errorf("URI.Pin() = %v, want %v", got, tt.want) + } + }) + } +} From b28db61d5de4049c96d664239fc69e312038e9a9 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 1 Feb 2021 14:16:55 -0800 Subject: [PATCH 17/33] Add missing close causing panic with softhsm2. --- kms/pkcs11/pkcs11_test.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index e69b5598..029beb66 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -29,6 +29,9 @@ func TestNew(t *testing.T) { }) k := mustPKCS11(t) + t.Cleanup(func() { + k.Close() + }) p11Configure = func(config *crypto11.Config) (P11, error) { if strings.Index(config.Path, "fail") >= 0 { return nil, errors.New("an error") @@ -333,10 +336,6 @@ func TestPKCS11_CreateKey(t *testing.T) { Name: "pkcs11:id=9999;object=create-key", SignatureAlgorithm: apiv1.SignatureAlgorithm(100), }}, nil, true}, - {"fail uri", args{&apiv1.CreateKeyRequest{ - Name: "pkcs11:id=xxxx;object=https", - SignatureAlgorithm: apiv1.SHA256WithRSAPSS, - }}, nil, true}, {"fail FindKeyPair", args{&apiv1.CreateKeyRequest{ Name: "pkcs11:foo=bar", SignatureAlgorithm: apiv1.SHA256WithRSAPSS, @@ -509,9 +508,6 @@ func TestPKCS11_LoadCertificate(t *testing.T) { {"fail name", args{&apiv1.LoadCertificateRequest{ Name: "", }}, nil, true}, - {"fail uri", args{&apiv1.LoadCertificateRequest{ - Name: "pkcs11:id=xxxx;object=test-root", - }}, nil, true}, {"fail scheme", args{&apiv1.LoadCertificateRequest{ Name: "foo:id=7376;object=test-root", }}, nil, true}, @@ -629,7 +625,6 @@ func TestPKCS11_DeleteKey(t *testing.T) { {"delete by label", args{testObjectByLabel}, false}, {"delete missing", args{"pkcs11:id=9999;object=missing-key"}, false}, {"fail name", args{""}, true}, - {"fail uri", args{"pkcs11:id=xxxx;object=missing-key"}, true}, {"fail FindKeyPair", args{"pkcs11:foo=bar"}, true}, } for _, tt := range tests { @@ -681,7 +676,6 @@ func TestPKCS11_DeleteCertificate(t *testing.T) { {"delete by label", args{testObjectByLabel}, false}, {"delete missing", args{"pkcs11:id=9999;object=missing-key"}, false}, {"fail name", args{""}, true}, - {"fail uri", args{"pkcs11:id=xxxx;object=missing-key"}, true}, {"fail DeleteCertificate", args{"pkcs11:foo=bar"}, true}, } for _, tt := range tests { From 128d07f148dcd6accc368673c3fe66ee0cef460d Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 1 Feb 2021 14:17:39 -0800 Subject: [PATCH 18/33] Use new GetEncoded method. --- kms/pkcs11/pkcs11.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 9ac806d8..df1e3d10 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -248,10 +248,7 @@ func parseObject(rawuri string) ([]byte, []byte, error) { if err != nil { return nil, nil, err } - id, err := u.GetHex("id") - if err != nil { - return nil, nil, err - } + id := u.GetEncoded("id") object := u.Get("object") if len(id) == 0 && object == "" { return nil, nil, errors.Errorf("key with uri %s is not valid, id or object are required", rawuri) @@ -320,11 +317,7 @@ func findCertificate(ctx P11, rawuri string) (*x509.Certificate, error) { if err != nil { return nil, err } - id, err := u.GetHex("id") - if err != nil { - return nil, err - } - object, serial := u.Get("object"), u.Get("serial") + id, object, serial := u.GetEncoded("id"), u.Get("object"), u.Get("serial") if len(id) == 0 && object == "" && serial == "" { return nil, errors.Errorf("key with uri %s is not valid, id, object or serial are required", rawuri) } From 41eff69fb33162c05e2372ed2a87aea025d89f66 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 1 Feb 2021 14:22:53 -0800 Subject: [PATCH 19/33] Fix linting errors. --- kms/pkcs11/other_test.go | 1 + kms/pkcs11/pkcs11_test.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/kms/pkcs11/other_test.go b/kms/pkcs11/other_test.go index 79d734e7..a0d810f4 100644 --- a/kms/pkcs11/other_test.go +++ b/kms/pkcs11/other_test.go @@ -28,6 +28,7 @@ func mustPKCS11(t TBTesting) *PKCS11 { for i := range testCerts { testCerts[i].Certificates = nil } + teardown(t, k) setup(t, k) return k } diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index 029beb66..ebecb7d0 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -33,7 +33,7 @@ func TestNew(t *testing.T) { k.Close() }) p11Configure = func(config *crypto11.Config) (P11, error) { - if strings.Index(config.Path, "fail") >= 0 { + if strings.Contains(config.Path, "fail") { return nil, errors.New("an error") } return k.p11, nil From a8260a3289c6c1682247b17ef52d5f3d3603e995 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 1 Feb 2021 14:25:49 -0800 Subject: [PATCH 20/33] Add missing test. --- kms/apiv1/options_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/kms/apiv1/options_test.go b/kms/apiv1/options_test.go index 72d9ba24..5405954f 100644 --- a/kms/apiv1/options_test.go +++ b/kms/apiv1/options_test.go @@ -50,3 +50,27 @@ func TestErrNotImplemented_Error(t *testing.T) { }) } } + +func TestErrAlreadyExists_Error(t *testing.T) { + type fields struct { + msg string + } + tests := []struct { + name string + fields fields + want string + }{ + {"default", fields{}, "key already exists"}, + {"custom", fields{"custom message: key already exists"}, "custom message: key already exists"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := ErrAlreadyExists{ + Message: tt.fields.msg, + } + if got := e.Error(); got != tt.want { + t.Errorf("ErrAlreadyExists.Error() = %v, want %v", got, tt.want) + } + }) + } +} From 3fdab93ab8c751dbf6f96aa499e7842ff8ae15b0 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 1 Feb 2021 15:27:53 -0800 Subject: [PATCH 21/33] Add missing file. --- kms/uri/testdata/pin.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 kms/uri/testdata/pin.txt diff --git a/kms/uri/testdata/pin.txt b/kms/uri/testdata/pin.txt new file mode 100644 index 00000000..2fca70ea --- /dev/null +++ b/kms/uri/testdata/pin.txt @@ -0,0 +1 @@ +trim-this-pin From 1d2146166b5be64f08db5ab01622a24f22c9ba9e Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 1 Feb 2021 15:28:09 -0800 Subject: [PATCH 22/33] Close key manager. --- cmd/step-pkcs11-init/main.go | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/cmd/step-pkcs11-init/main.go b/cmd/step-pkcs11-init/main.go index e7643265..4463153f 100644 --- a/cmd/step-pkcs11-init/main.go +++ b/cmd/step-pkcs11-init/main.go @@ -137,6 +137,10 @@ func main() { fatal(err) } + defer func() { + _ = k.Close() + }() + // Check if the slots are empty, fail if they are not certUris := []string{ c.RootObject, c.CrtObject, @@ -168,17 +172,17 @@ func main() { // Some HSMs like Nitrokey will overwrite the key with the // certificate label. if err := deleter.DeleteKey(u); err != nil { - fatal(err) + fatalClose(err, k) } if err := deleter.DeleteCertificate(u); err != nil { - fatal(err) + fatalClose(err, k) } } } for _, u := range keyUris { if u != "" { if err := deleter.DeleteKey(u); err != nil { - fatal(err) + fatalClose(err, k) } } } @@ -186,12 +190,8 @@ func main() { } if err := createPKI(k, c); err != nil { - fatal(err) + fatalClose(err, k) } - - defer func() { - _ = k.Close() - }() } func fatal(err error) { @@ -203,6 +203,11 @@ func fatal(err error) { os.Exit(1) } +func fatalClose(err error, k kms.KeyManager) { + _ = k.Close() + fatal(err) +} + func usage() { fmt.Fprintln(os.Stderr, "Usage: step-pkcs11-init") fmt.Fprintln(os.Stderr, ` @@ -228,6 +233,7 @@ func checkCertificate(k kms.KeyManager, rawuri string) { }); err == nil { fmt.Fprintf(os.Stderr, "⚠️ Your PKCS #11 module already has a certificate on %s.\n", rawuri) fmt.Fprintln(os.Stderr, " If you want to delete it and start fresh, use `--force`.") + _ = k.Close() os.Exit(1) } } @@ -239,6 +245,7 @@ func checkObject(k kms.KeyManager, rawuri string) { }); err == nil { fmt.Fprintf(os.Stderr, "⚠️ Your PKCS #11 module already has a key on %s.\n", rawuri) fmt.Fprintln(os.Stderr, " If you want to delete it and start fresh, use `--force`.") + _ = k.Close() os.Exit(1) } } From 97c8cd10cd573010aea83a3fa58226d1011d6945 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 1 Feb 2021 17:04:07 -0800 Subject: [PATCH 23/33] Test with CGO enabled --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6f37e121..282e26f2 100644 --- a/Makefile +++ b/Makefile @@ -119,7 +119,7 @@ generate: # Test ######################################### test: - $Q $(GOFLAGS) go test -short -coverprofile=coverage.out ./... + $Q go test -short -coverprofile=coverage.out ./... .PHONY: test From fbd2208044b9faa3125198b45fd8b62d1e7ddf0b Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 1 Feb 2021 17:14:44 -0800 Subject: [PATCH 24/33] Close key manager for safe reloads when a cgo module is used. --- authority/authority.go | 7 +++++++ authority/authority_test.go | 14 ++++++++++++++ ca/ca.go | 4 +++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/authority/authority.go b/authority/authority.go index 4518abdf..72fa081f 100644 --- a/authority/authority.go +++ b/authority/authority.go @@ -382,3 +382,10 @@ func (a *Authority) Shutdown() error { } return a.db.Shutdown() } + +// CloseForReload closes internal services, to allow a safe reload. +func (a *Authority) CloseForReload() { + if err := a.keyManager.Close(); err != nil { + log.Printf("error closing the key manager: %v", err) + } +} diff --git a/authority/authority_test.go b/authority/authority_test.go index 8b003572..e6625d6a 100644 --- a/authority/authority_test.go +++ b/authority/authority_test.go @@ -306,3 +306,17 @@ func TestNewEmbedded_GetTLSCertificate(t *testing.T) { assert.True(t, cert.Leaf.IPAddresses[0].Equal(net.ParseIP("127.0.0.1"))) assert.True(t, cert.Leaf.IPAddresses[1].Equal(net.ParseIP("::1"))) } + +func TestAuthority_CloseForReload(t *testing.T) { + tests := []struct { + name string + auth *Authority + }{ + {"ok", testAuthority(t)}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.auth.CloseForReload() + }) + } +} diff --git a/ca/ca.go b/ca/ca.go index 3c57b759..c43692f2 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -227,9 +227,11 @@ func (ca *CA) Reload() error { } // 1. Stop previous renewer - // 2. Replace ca properties + // 2. Close key manager + // 3. Replace ca properties // Do not replace ca.srv ca.renewer.Stop() + ca.auth.CloseForReload() ca.auth = newCA.auth ca.config = newCA.config ca.opts = newCA.opts From dd6a43ad139d726ead351b47df5cd749013e1846 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 4 Feb 2021 12:32:30 -0800 Subject: [PATCH 25/33] Add fake implementation of pkcs11 key manager without cgo. This allows other binaries to import pkcs11 directly even if they are compiled without cgo. --- kms/pkcs11/pkcs11_no_cgo.go | 42 +++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/kms/pkcs11/pkcs11_no_cgo.go b/kms/pkcs11/pkcs11_no_cgo.go index e60a7563..87c9a36b 100644 --- a/kms/pkcs11/pkcs11_no_cgo.go +++ b/kms/pkcs11/pkcs11_no_cgo.go @@ -4,6 +4,7 @@ package pkcs11 import ( "context" + "crypto" "os" "path/filepath" @@ -11,9 +12,46 @@ import ( "github.com/smallstep/certificates/kms/apiv1" ) +var errUnsupported error + func init() { + name := filepath.Base(os.Args[0]) + errUnsupported = errors.Errorf("unsupported kms type 'pkcs11': %s is compiled without cgo support", name) + apiv1.Register(apiv1.PKCS11, func(ctx context.Context, opts apiv1.Options) (apiv1.KeyManager, error) { - name := filepath.Base(os.Args[0]) - return nil, errors.Errorf("unsupported kms type 'pkcs11': %s is compiled without cgo support", name) + return nil, errUnsupported }) } + +// PKCS11 is the implementation of a KMS using the PKCS #11 standard. +type PKCS11 struct{} + +// New implements the kms.KeyManager interface and without CGO will always +// return an error. +func New(ctx context.Context, opts apiv1.Options) (*PKCS11, error) { + return nil, errUnsupported +} + +// GetPublicKey implements the kms.KeyManager interface and without CGO will always +// return an error. +func (*PKCS11) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKey, error) { + return nil, errUnsupported +} + +// CreateKey implements the kms.KeyManager interface and without CGO will always +// return an error. +func (*PKCS11) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyResponse, error) { + return nil, errUnsupported +} + +// CreateSigner implements the kms.KeyManager interface and without CGO will always +// return an error. +func (*PKCS11) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, error) { + return nil, errUnsupported +} + +// Close implements the kms.KeyManager interface and without CGO will always +// return an error. +func (*PKCS11) Close() error { + return errUnsupported +} From f425a81d368c87111cfc72996942d6376fe0d2ff Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 4 Feb 2021 12:53:08 -0800 Subject: [PATCH 26/33] Enforce the use of id and label when generating objects. --- kms/pkcs11/pkcs11.go | 7 +++++++ kms/pkcs11/pkcs11_test.go | 10 +++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index df1e3d10..64c1e72a 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -262,6 +262,7 @@ func generateKey(ctx P11, req *apiv1.CreateKeyRequest) (crypto11.Signer, error) if err != nil { return nil, err } + signer, err := ctx.FindKeyPair(id, object) if err != nil { return nil, err @@ -272,6 +273,12 @@ func generateKey(ctx P11, req *apiv1.CreateKeyRequest) (crypto11.Signer, error) } } + // 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 nil, errors.Errorf("key with uri %s is not valid, id and object are required", req.Name) + } + bits := req.Bits if bits == 0 { bits = DefaultRSASize diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index ebecb7d0..a74fb3fe 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -198,7 +198,6 @@ func TestPKCS11_CreateKey(t *testing.T) { want *apiv1.CreateKeyResponse wantErr bool }{ - // SoftHSM2 {"default", args{&apiv1.CreateKeyRequest{ Name: testObject, }}, &apiv1.CreateKeyResponse{ @@ -323,6 +322,15 @@ func TestPKCS11_CreateKey(t *testing.T) { {"fail name", args{&apiv1.CreateKeyRequest{ Name: "", }}, nil, true}, + {"fail no id", args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:object=create-key", + }}, nil, true}, + {"fail no object", args{&apiv1.CreateKeyRequest{ + Name: "pkcs11:id=9999", + }}, nil, true}, + {"fail schema", args{&apiv1.CreateKeyRequest{ + Name: "pkcs12:id=9999;object=create-key", + }}, nil, true}, {"fail bits", args{&apiv1.CreateKeyRequest{ Name: "pkcs11:id=9999;object=create-key", Bits: -1, From f289d1ee1f3efae2fdb53df96ae8a3e313b9a5a5 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 8 Feb 2021 12:01:21 -0800 Subject: [PATCH 27/33] Update to crypto11 v1.2.4 This version now includes my changes to delete a certificate. --- go.mod | 3 +-- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 3d9ce9a0..1cbf72fb 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.14 require ( cloud.google.com/go v0.70.0 github.com/Masterminds/sprig/v3 v3.1.0 - github.com/ThalesIgnite/crypto11 v1.2.3 + github.com/ThalesIgnite/crypto11 v1.2.4 github.com/aws/aws-sdk-go v1.30.29 github.com/go-chi/chi v4.0.2+incompatible github.com/go-piv/piv-go v1.7.0 @@ -34,4 +34,3 @@ require ( // replace github.com/smallstep/nosql => ../nosql // replace go.step.sm/crypto => ../crypto // replace github.com/smallstep/nosql => ../nosql -replace github.com/ThalesIgnite/crypto11 => github.com/maraino/crypto11 v1.2.4-0.20210127032225-7ed5319b45a1 diff --git a/go.sum b/go.sum index 9d256473..53ea92fa 100644 --- a/go.sum +++ b/go.sum @@ -51,6 +51,8 @@ github.com/Masterminds/sprig/v3 v3.1.0 h1:j7GpgZ7PdFqNsmncycTHsLmVPf5/3wJtlgW9TN github.com/Masterminds/sprig/v3 v3.1.0/go.mod h1:ONGMf7UfYGAbMXCZmQLy8x3lCDIPrEZE/rU8pmrbihA= github.com/OneOfOne/xxhash v1.2.2 h1:KMrpdQIwFcEqXDklaen+P1axHaj9BSKzvpUUfnHldSE= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= +github.com/ThalesIgnite/crypto11 v1.2.4 h1:3MebRK/U0mA2SmSthXAIZAdUA9w8+ZuKem2O6HuR1f8= +github.com/ThalesIgnite/crypto11 v1.2.4/go.mod h1:ILDKtnCKiQ7zRoNxcp36Y1ZR8LBPmR2E23+wTQe/MlE= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= github.com/aws/aws-sdk-go v1.30.29 h1:NXNqBS9hjOCpDL8SyCyl38gZX3LLLunKOJc5E7vJ8P0= github.com/aws/aws-sdk-go v1.30.29/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= @@ -213,8 +215,6 @@ github.com/lunixbochs/vtclean v1.0.0/go.mod h1:pHhQNgMf3btfWnGBVipUOjRYhoOsdGqdm github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/manifoldco/promptui v0.8.0 h1:R95mMF+McvXZQ7j1g8ucVZE1gLP3Sv6j9vlF9kyRqQo= github.com/manifoldco/promptui v0.8.0/go.mod h1:n4zTdgP0vr0S3w7/O/g98U+e0gwLScEXGwov2nIKuGQ= -github.com/maraino/crypto11 v1.2.4-0.20210127032225-7ed5319b45a1 h1:aj2ASiF6u9p576GdvGsRW5SiwiE/Hp5BeU2c/ldlYTI= -github.com/maraino/crypto11 v1.2.4-0.20210127032225-7ed5319b45a1/go.mod h1:ILDKtnCKiQ7zRoNxcp36Y1ZR8LBPmR2E23+wTQe/MlE= github.com/mattn/go-colorable v0.0.9 h1:UVL0vNpWh04HeJXV0KLcaT7r06gOH2l4OW6ddYRUIY4= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= github.com/mattn/go-colorable v0.1.4 h1:snbPLB8fVfU9iwbbo30TPtbLRzwWu6aJS6Xh4eaaviA= From ebaeae9008a3924d7df615cdb2c4cdc3800de296 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Mon, 8 Feb 2021 19:16:57 -0800 Subject: [PATCH 28/33] Avoid closing pkcs#11 context twice. --- kms/pkcs11/pkcs11.go | 11 ++++++++--- kms/pkcs11/pkcs11_test.go | 18 ++++++++++++++++++ kms/pkcs11/yubihsm2_test.go | 2 +- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 64c1e72a..25b3d447 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -11,6 +11,7 @@ import ( "fmt" "math/big" "strconv" + "sync" "github.com/ThalesIgnite/crypto11" "github.com/pkg/errors" @@ -43,7 +44,8 @@ var p11Configure = func(config *crypto11.Config) (P11, error) { // PKCS11 is the implementation of a KMS using the PKCS #11 standard. type PKCS11 struct { - p11 P11 + p11 P11 + closed sync.Once } // New returns a new PKCS11 KMS. @@ -232,8 +234,11 @@ func (k *PKCS11) DeleteCertificate(uri string) error { } // Close releases the connection to the PKCS#11 module. -func (k *PKCS11) Close() error { - return errors.Wrap(k.p11.Close(), "error closing pkcs#11 context") +func (k *PKCS11) Close() (err error) { + k.closed.Do(func() { + err = errors.Wrap(k.p11.Close(), "error closing pkcs#11 context") + }) + return } func toByte(s string) []byte { diff --git a/kms/pkcs11/pkcs11_test.go b/kms/pkcs11/pkcs11_test.go index a74fb3fe..77277366 100644 --- a/kms/pkcs11/pkcs11_test.go +++ b/kms/pkcs11/pkcs11_test.go @@ -709,3 +709,21 @@ func TestPKCS11_DeleteCertificate(t *testing.T) { }) } } + +func TestPKCS11_Close(t *testing.T) { + k := mustPKCS11(t) + tests := []struct { + name string + wantErr bool + }{ + {"ok", false}, + {"second", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := k.Close(); (err != nil) != tt.wantErr { + t.Errorf("PKCS11.Close() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/kms/pkcs11/yubihsm2_test.go b/kms/pkcs11/yubihsm2_test.go index 7d508872..54885070 100644 --- a/kms/pkcs11/yubihsm2_test.go +++ b/kms/pkcs11/yubihsm2_test.go @@ -37,7 +37,7 @@ func mustPKCS11(t TBTesting) *PKCS11 { Pin: "0001password", }) if err != nil { - t.Fatalf("failed to configure yubiHSM2 on %s: %v", runtime.GOOS, err) + t.Fatalf("failed to configure YubiHSM2 on %s: %v", runtime.GOOS, err) } k := &PKCS11{ From b487edbd135a6e13f7061fc123f23c160fb4080a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 11 Feb 2021 17:38:14 -0800 Subject: [PATCH 29/33] Clarify comment. --- ca/ca.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ca/ca.go b/ca/ca.go index c43692f2..5ba81e9e 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -227,7 +227,7 @@ func (ca *CA) Reload() error { } // 1. Stop previous renewer - // 2. Close key manager + // 2. Safely shutdown any internal resources (e.g. key manager) // 3. Replace ca properties // Do not replace ca.srv ca.renewer.Stop() From f6cbd9dc880f0a544f5406b35bf857b2f937e63d Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 11 Feb 2021 19:14:15 -0800 Subject: [PATCH 30/33] Fix typos. --- kms/pkcs11/pkcs11.go | 2 +- kms/uri/uri.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kms/pkcs11/pkcs11.go b/kms/pkcs11/pkcs11.go index 25b3d447..47c298a5 100644 --- a/kms/pkcs11/pkcs11.go +++ b/kms/pkcs11/pkcs11.go @@ -22,7 +22,7 @@ import ( // Scheme is the scheme used in uris. const Scheme = "pkcs11" -// DefaultRSASize is the number of bits of a new RSA key if not bitsize has been +// DefaultRSASize is the number of bits of a new RSA key if no size has been // specified. const DefaultRSASize = 3072 diff --git a/kms/uri/uri.go b/kms/uri/uri.go index 8e30a7c5..94009c47 100644 --- a/kms/uri/uri.go +++ b/kms/uri/uri.go @@ -83,7 +83,7 @@ func ParseWithScheme(scheme, rawuri string) (*URI, error) { return u, nil } -// Get returns the first value in the uri with the give n key, it will return +// Get returns the first value in the uri with the given key, it will return // empty string if that field is not present. func (u *URI) Get(key string) string { v := u.Values.Get(key) @@ -93,7 +93,7 @@ func (u *URI) Get(key string) string { return v } -// GetEncoded returns the first value in the uri with the give n key, it will +// GetEncoded returns the first value in the uri with the given key, it will // return empty nil if that field is not present or is empty. If the return // value is hex encoded it will decode it and return it. func (u *URI) GetEncoded(key string) []byte { From d03c088ab748c1c12649092ae547833bc831c6f6 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 11 Feb 2021 19:14:35 -0800 Subject: [PATCH 31/33] Add test cases for uris with only the schema. --- kms/uri/uri_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/kms/uri/uri_test.go b/kms/uri/uri_test.go index 3de07456..aa420db4 100644 --- a/kms/uri/uri_test.go +++ b/kms/uri/uri_test.go @@ -99,6 +99,10 @@ func TestParse(t *testing.T) { URL: &url.URL{Scheme: "yubikey", Opaque: "slot-id=9a"}, Values: url.Values{"slot-id": []string{"9a"}}, }, false}, + {"ok schema", args{"cloudkms:"}, &URI{ + URL: &url.URL{Scheme: "cloudkms"}, + Values: url.Values{}, + }, false}, {"ok query", args{"yubikey:slot-id=9a;foo=bar?pin=123456&foo=bar"}, &URI{ URL: &url.URL{Scheme: "yubikey", Opaque: "slot-id=9a;foo=bar", RawQuery: "pin=123456&foo=bar"}, Values: url.Values{"slot-id": []string{"9a"}, "foo": []string{"bar"}}, @@ -115,6 +119,7 @@ func TestParse(t *testing.T) { URL: &url.URL{Scheme: "file", Host: "tmp", Path: "/ca.cert"}, Values: url.Values{}, }, false}, + {"fail schema", args{"cloudkms"}, nil, true}, {"fail parse", args{"yubi%key:slot-id=9a"}, nil, true}, {"fail scheme", args{"yubikey"}, nil, true}, {"fail parse opaque", args{"yubikey:slot-id=%ZZ"}, nil, true}, @@ -148,12 +153,17 @@ func TestParseWithScheme(t *testing.T) { URL: &url.URL{Scheme: "yubikey", Opaque: "slot-id=9a"}, Values: url.Values{"slot-id": []string{"9a"}}, }, false}, + {"ok schema", args{"cloudkms", "cloudkms:"}, &URI{ + URL: &url.URL{Scheme: "cloudkms"}, + Values: url.Values{}, + }, false}, {"ok file", args{"file", "file:///tmp/ca.cert"}, &URI{ URL: &url.URL{Scheme: "file", Path: "/tmp/ca.cert"}, Values: url.Values{}, }, false}, {"fail parse", args{"yubikey", "yubikey"}, nil, true}, {"fail scheme", args{"yubikey", "awskms:slot-id=9a"}, nil, true}, + {"fail schema", args{"cloudkms", "cloudkms"}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 3648c3fab6ac447d25304f716476e8f8132169c4 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 11 Feb 2021 19:24:09 -0800 Subject: [PATCH 32/33] Fix error message when --kms is not passed. --- cmd/step-pkcs11-init/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/step-pkcs11-init/main.go b/cmd/step-pkcs11-init/main.go index 4463153f..7afe05f1 100644 --- a/cmd/step-pkcs11-init/main.go +++ b/cmd/step-pkcs11-init/main.go @@ -51,7 +51,7 @@ type Config struct { func (c *Config) Validate() error { switch { case c.KMS == "": - return errors.New("one of flag `--kms` is required") + return errors.New("flag `--kms` is required") case c.RootFile != "" && c.KeyFile == "": return errors.New("flag `--root` requires flag `--key`") case c.KeyFile != "" && c.RootFile == "": From e446e225201c03af54833bbd8895e14a5829eb02 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 11 Feb 2021 19:25:16 -0800 Subject: [PATCH 33/33] Remove extra default. --- cmd/step-pkcs11-init/main.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cmd/step-pkcs11-init/main.go b/cmd/step-pkcs11-init/main.go index 7afe05f1..0dd431ad 100644 --- a/cmd/step-pkcs11-init/main.go +++ b/cmd/step-pkcs11-init/main.go @@ -90,8 +90,6 @@ func main() { if home, err := os.UserHomeDir(); err == nil { kmsuri = "pkcs11:module-path=" + home + "\\yubihsm2-sdk\\bin\\yubihsm_pkcs11.dll" + ";token=YubiHSM" } - default: - } var c Config