From 4902e45729498eceb4aa1db69ba12ce09d863b4a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 16 Feb 2021 12:52:14 -0800 Subject: [PATCH 1/6] Add URI support initializing an awskms. --- kms/awskms/awskms.go | 22 +++++++++++++++++++++- kms/awskms/awskms_test.go | 6 ++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/kms/awskms/awskms.go b/kms/awskms/awskms.go index 5e88eb80..0048a132 100644 --- a/kms/awskms/awskms.go +++ b/kms/awskms/awskms.go @@ -17,6 +17,9 @@ import ( "go.step.sm/crypto/pemutil" ) +// Scheme is the scheme used in uris. +const Scheme = "awskms" + // KMS implements a KMS using AWS Key Management Service. type KMS struct { session *session.Session @@ -69,7 +72,24 @@ var customerMasterKeySpecMapping = map[apiv1.SignatureAlgorithm]interface{}{ // AWS sessions can also be configured with environment variables, see docs at // https://docs.aws.amazon.com/sdk-for-go/api/aws/session/ for all the options. func New(ctx context.Context, opts apiv1.Options) (*KMS, error) { - o := session.Options{} + var o session.Options + + if opts.URI != "" { + u, err := uri.ParseWithScheme(Scheme, opts.URI) + if err != nil { + return nil, err + } + o.Profile = u.Get("profile") + if v := u.Get("region"); v != "" { + o.Config.Region = new(string) + *o.Config.Region = v + } + if f := u.Get("credentials-file"); f != "" { + o.SharedConfigFiles = []string{opts.CredentialsFile} + } + } + + // Deprecated way to setting configuration parameters. if opts.Region != "" { o.Config.Region = &opts.Region } diff --git a/kms/awskms/awskms_test.go b/kms/awskms/awskms_test.go index c86645e2..3c99fc4c 100644 --- a/kms/awskms/awskms_test.go +++ b/kms/awskms/awskms_test.go @@ -60,7 +60,13 @@ func TestNew(t *testing.T) { Profile: "smallstep", CredentialsFile: "~/aws/credentials", }}, expected, false}, + {"ok with uri", args{ctx, apiv1.Options{ + URI: "awskms:region=us-east-1;profile=smallstep;credentials-file=/var/run/aws/credentials", + }}, expected, false}, {"fail", args{ctx, apiv1.Options{}}, nil, true}, + {"fail uri", args{ctx, apiv1.Options{ + URI: "pkcs11:region=us-east-1;profile=smallstep;credentials-file=/var/run/aws/credentials", + }}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From a947779795c83ab0e2902994d96d259a7b3528a5 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 16 Feb 2021 13:11:47 -0800 Subject: [PATCH 2/6] Add uri support initializing cloudkms. --- kms/cloudkms/cloudkms.go | 22 +++++++++++++- kms/cloudkms/cloudkms_test.go | 57 +++++++++++++++++++++++++++++------ 2 files changed, 68 insertions(+), 11 deletions(-) diff --git a/kms/cloudkms/cloudkms.go b/kms/cloudkms/cloudkms.go index 547bfc62..98fe375c 100644 --- a/kms/cloudkms/cloudkms.go +++ b/kms/cloudkms/cloudkms.go @@ -14,11 +14,15 @@ import ( gax "github.com/googleapis/gax-go/v2" "github.com/pkg/errors" "github.com/smallstep/certificates/kms/apiv1" + "github.com/smallstep/certificates/kms/uri" "go.step.sm/crypto/pemutil" "google.golang.org/api/option" kmspb "google.golang.org/genproto/googleapis/cloud/kms/v1" ) +// Scheme is the scheme used in uris. +const Scheme = "cloudkms" + const pendingGenerationRetries = 10 // protectionLevelMapping maps step protection levels with cloud kms ones. @@ -71,6 +75,10 @@ type KeyManagementClient interface { CreateCryptoKeyVersion(ctx context.Context, req *kmspb.CreateCryptoKeyVersionRequest, opts ...gax.CallOption) (*kmspb.CryptoKeyVersion, error) } +var newKeyManagementClient = func(ctx context.Context, opts ...option.ClientOption) (KeyManagementClient, error) { + return cloudkms.NewKeyManagementClient(ctx, opts...) +} + // CloudKMS implements a KMS using Google's Cloud apiv1. type CloudKMS struct { client KeyManagementClient @@ -79,11 +87,23 @@ type CloudKMS struct { // New creates a new CloudKMS configured with a new client. func New(ctx context.Context, opts apiv1.Options) (*CloudKMS, error) { var cloudOpts []option.ClientOption + + if opts.URI != "" { + u, err := uri.ParseWithScheme(Scheme, opts.URI) + if err != nil { + return nil, err + } + if f := u.Get("credentials-file"); f != "" { + cloudOpts = append(cloudOpts, option.WithCredentialsFile(f)) + } + } + + // Deprecated way to setting configuration parameters. if opts.CredentialsFile != "" { cloudOpts = append(cloudOpts, option.WithCredentialsFile(opts.CredentialsFile)) } - client, err := cloudkms.NewKeyManagementClient(ctx, cloudOpts...) + client, err := newKeyManagementClient(ctx, cloudOpts...) if err != nil { return nil, err } diff --git a/kms/cloudkms/cloudkms_test.go b/kms/cloudkms/cloudkms_test.go index 1038432a..e04e0198 100644 --- a/kms/cloudkms/cloudkms_test.go +++ b/kms/cloudkms/cloudkms_test.go @@ -5,13 +5,13 @@ import ( "crypto" "fmt" "io/ioutil" - "os" "reflect" "testing" gax "github.com/googleapis/gax-go/v2" "github.com/smallstep/certificates/kms/apiv1" "go.step.sm/crypto/pemutil" + "google.golang.org/api/option" kmspb "google.golang.org/genproto/googleapis/cloud/kms/v1" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -50,26 +50,63 @@ func TestParent(t *testing.T) { } func TestNew(t *testing.T) { + tmp := newKeyManagementClient + t.Cleanup(func() { + newKeyManagementClient = tmp + }) + newKeyManagementClient = func(ctx context.Context, opts ...option.ClientOption) (KeyManagementClient, error) { + if len(opts) > 0 { + return nil, fmt.Errorf("test error") + } + return &MockClient{}, nil + } + type args struct { ctx context.Context opts apiv1.Options } tests := []struct { - name string - skipOnCI bool - args args - want *CloudKMS - wantErr bool + name string + args args + want *CloudKMS + wantErr bool }{ - {"fail authentication", true, args{context.Background(), apiv1.Options{}}, nil, true}, - {"fail credentials", false, args{context.Background(), apiv1.Options{CredentialsFile: "testdata/missing"}}, nil, true}, + {"ok", args{context.Background(), apiv1.Options{}}, &CloudKMS{client: &MockClient{}}, false}, + {"ok with uri", args{context.Background(), apiv1.Options{URI: "cloudkms:"}}, &CloudKMS{client: &MockClient{}}, false}, + {"fail credentials", args{context.Background(), apiv1.Options{CredentialsFile: "testdata/missing"}}, nil, true}, + {"fail with uri", args{context.Background(), apiv1.Options{URI: "cloudkms:credentials-file=testdata/missing"}}, nil, true}, + {"fail schema", args{context.Background(), apiv1.Options{URI: "pkcs11:"}}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.skipOnCI && os.Getenv("CI") == "true" { - t.SkipNow() + 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 TestNew_real(t *testing.T) { + type args struct { + ctx context.Context + opts apiv1.Options + } + tests := []struct { + name string + args args + want *CloudKMS + wantErr bool + }{ + {"fail credentials", args{context.Background(), apiv1.Options{CredentialsFile: "testdata/missing"}}, nil, true}, + {"fail with uri", args{context.Background(), apiv1.Options{URI: "cloudkms:credentials-file=testdata/missing"}}, nil, true}, + } + 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) From 4bec2b04ecd6a1519fe7414b73a30583be9d0770 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 16 Feb 2021 14:44:34 -0800 Subject: [PATCH 3/6] Add support for retired key management slots on yubikey. Fixes #461 --- kms/yubikey/yubikey.go | 46 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/kms/yubikey/yubikey.go b/kms/yubikey/yubikey.go index 3349ea63..943cd534 100644 --- a/kms/yubikey/yubikey.go +++ b/kms/yubikey/yubikey.go @@ -106,12 +106,12 @@ func (k *YubiKey) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKey return nil, err } - cert, err := k.yk.Certificate(slot) + pub, err := k.getPublicKey(slot) if err != nil { - return nil, errors.Wrap(err, "error retrieving certificate") + return nil, err } - return cert.PublicKey, nil + return pub, nil } // CreateKey generates a new key in the YubiKey and returns the public key. @@ -150,12 +150,12 @@ func (k *YubiKey) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, e return nil, err } - cert, err := k.yk.Certificate(slot) + pub, err := k.getPublicKey(slot) if err != nil { - return nil, errors.Wrap(err, "error retrieving certificate") + return nil, err } - priv, err := k.yk.PrivateKey(slot, cert.PublicKey, piv.KeyAuth{ + priv, err := k.yk.PrivateKey(slot, pub, piv.KeyAuth{ PIN: k.pin, PINPolicy: piv.PINPolicyAlways, }) @@ -175,6 +175,20 @@ func (k *YubiKey) Close() error { return errors.Wrap(k.yk.Close(), "error closing yubikey") } +// getPublicKey returns the public key on a slot. First it attempts to do +// attestation to get a certificate with the public key in it, if this succeeds +// means that the key was generated in the device. If not we'll try to get the +// key from a stored certificate in the same slot. +func (k *YubiKey) getPublicKey(slot piv.Slot) (crypto.PublicKey, error) { + cert, err := k.yk.Attest(slot) + if err != nil { + if cert, err = k.yk.Certificate(slot); err != nil { + return nil, errors.Wrap(err, "error retrieving public key") + } + } + return cert.PublicKey, nil +} + // signatureAlgorithmMapping is a mapping between the step signature algorithm, // and bits for RSA keys, with yubikey ones. var signatureAlgorithmMapping = map[apiv1.SignatureAlgorithm]interface{}{ @@ -228,6 +242,26 @@ var slotMapping = map[string]piv.Slot{ "9c": piv.SlotSignature, "9e": piv.SlotCardAuthentication, "9d": piv.SlotKeyManagement, + "82": {Key: 0x82, Object: 0x5FC10D}, + "83": {Key: 0x83, Object: 0x5FC10E}, + "84": {Key: 0x84, Object: 0x5FC10F}, + "85": {Key: 0x85, Object: 0x5FC110}, + "86": {Key: 0x86, Object: 0x5FC111}, + "87": {Key: 0x87, Object: 0x5FC112}, + "88": {Key: 0x88, Object: 0x5FC113}, + "89": {Key: 0x89, Object: 0x5FC114}, + "8a": {Key: 0x8a, Object: 0x5FC115}, + "8b": {Key: 0x8b, Object: 0x5FC116}, + "8c": {Key: 0x8c, Object: 0x5FC117}, + "8d": {Key: 0x8d, Object: 0x5FC118}, + "8e": {Key: 0x8e, Object: 0x5FC119}, + "8f": {Key: 0x8f, Object: 0x5FC11A}, + "90": {Key: 0x90, Object: 0x5FC11B}, + "91": {Key: 0x91, Object: 0x5FC11C}, + "92": {Key: 0x92, Object: 0x5FC11D}, + "93": {Key: 0x93, Object: 0x5FC11E}, + "94": {Key: 0x94, Object: 0x5FC11F}, + "95": {Key: 0x95, Object: 0x5FC120}, } func getSlot(name string) (piv.Slot, error) { From 4c562160fc933f4c9473832aadb171a2d1ad1823 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 16 Feb 2021 14:52:55 -0800 Subject: [PATCH 4/6] Fix typo. --- kms/awskms/awskms.go | 2 +- kms/cloudkms/cloudkms.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kms/awskms/awskms.go b/kms/awskms/awskms.go index 0048a132..da392989 100644 --- a/kms/awskms/awskms.go +++ b/kms/awskms/awskms.go @@ -89,7 +89,7 @@ func New(ctx context.Context, opts apiv1.Options) (*KMS, error) { } } - // Deprecated way to setting configuration parameters. + // Deprecated way to set configuration parameters. if opts.Region != "" { o.Config.Region = &opts.Region } diff --git a/kms/cloudkms/cloudkms.go b/kms/cloudkms/cloudkms.go index 98fe375c..cc533702 100644 --- a/kms/cloudkms/cloudkms.go +++ b/kms/cloudkms/cloudkms.go @@ -98,7 +98,7 @@ func New(ctx context.Context, opts apiv1.Options) (*CloudKMS, error) { } } - // Deprecated way to setting configuration parameters. + // Deprecated way to set configuration parameters. if opts.CredentialsFile != "" { cloudOpts = append(cloudOpts, option.WithCredentialsFile(opts.CredentialsFile)) } From 2ba4e3753095147239348e9222fbf62725f6f636 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 16 Feb 2021 15:02:20 -0800 Subject: [PATCH 5/6] Add URI support to configure yubikeys. --- kms/yubikey/yubikey.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/kms/yubikey/yubikey.go b/kms/yubikey/yubikey.go index 943cd534..19cef55e 100644 --- a/kms/yubikey/yubikey.go +++ b/kms/yubikey/yubikey.go @@ -13,8 +13,12 @@ import ( "github.com/go-piv/piv-go/piv" "github.com/pkg/errors" "github.com/smallstep/certificates/kms/apiv1" + "github.com/smallstep/certificates/kms/uri" ) +// Scheme is the scheme used in uris. +const Scheme = "yubikey" + // YubiKey implements the KMS interface on a YubiKey. type YubiKey struct { yk *piv.YubiKey @@ -26,6 +30,21 @@ type YubiKey struct { // TODO(mariano): only one card is currently supported. func New(ctx context.Context, opts apiv1.Options) (*YubiKey, error) { managementKey := piv.DefaultManagementKey + + if opts.URI != "" { + u, err := uri.ParseWithScheme(Scheme, opts.URI) + if err != nil { + return nil, err + } + if v := u.Pin(); v != "" { + opts.Pin = v + } + if v := u.Get("management-key"); v != "" { + opts.ManagementKey = v + } + } + + // Deprecated way to set configuration parameters. if opts.ManagementKey != "" { b, err := hex.DecodeString(opts.ManagementKey) if err != nil { From 3eb24d7d0192899eec24570ef68b277947504b43 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 16 Feb 2021 17:14:15 -0800 Subject: [PATCH 6/6] Remove duplicated replace. --- go.mod | 1 - 1 file changed, 1 deletion(-) diff --git a/go.mod b/go.mod index 1cbf72fb..68dce5ad 100644 --- a/go.mod +++ b/go.mod @@ -33,4 +33,3 @@ require ( // replace github.com/smallstep/nosql => ../nosql // replace go.step.sm/crypto => ../crypto -// replace github.com/smallstep/nosql => ../nosql