From d2581489a38d18765611ef38bf0634bc949139e0 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 6 Oct 2021 18:39:12 -0700 Subject: [PATCH] Redefine uris and set proper type. URIs will now have the form: - azurekms:name=my-key;vault=my-vault - azurekms:name=my-key;vault=my-vault?version=my-version --- kms/azurekms/key_vault.go | 25 ++++++---- kms/azurekms/key_vault_test.go | 88 +++++++++++++++++----------------- kms/azurekms/signer_test.go | 12 ++--- kms/azurekms/utils.go | 42 ++++++++++++---- kms/azurekms/utils_test.go | 83 ++++++++++++++++++++++++++++++++ 5 files changed, 184 insertions(+), 66 deletions(-) create mode 100644 kms/azurekms/utils_test.go diff --git a/kms/azurekms/key_vault.go b/kms/azurekms/key_vault.go index dde368da..f7b00ec7 100644 --- a/kms/azurekms/key_vault.go +++ b/kms/azurekms/key_vault.go @@ -3,7 +3,7 @@ package azurekms import ( "context" "crypto" - "net/url" + "regexp" "time" "github.com/Azure/azure-sdk-for-go/services/keyvault/v7.1/keyvault" @@ -11,11 +11,10 @@ import ( "github.com/Azure/go-autorest/autorest/date" "github.com/pkg/errors" "github.com/smallstep/certificates/kms/apiv1" - "github.com/smallstep/certificates/kms/uri" ) func init() { - apiv1.Register(apiv1.CloudKMS, func(ctx context.Context, opts apiv1.Options) (apiv1.KeyManager, error) { + apiv1.Register(apiv1.AzureKMS, func(ctx context.Context, opts apiv1.Options) (apiv1.KeyManager, error) { return New(ctx, opts) }) } @@ -23,6 +22,10 @@ func init() { // Scheme is the scheme used for Azure Key Vault uris. const Scheme = "azurekms" +// keyIDRegexp is the regular experssion that Key Vault uses for on the kid. We +// can extract the vault, name and version of the key. +var keyIDRegexp = regexp.MustCompile("^https://([0-9a-zA-Z-]+).vault.azure.net/keys/([0-9a-zA-Z-]+)/([0-9a-zA-Z-]+)$") + var ( valueTrue = true value2048 int32 = 2048 @@ -107,6 +110,16 @@ type KeyVaultClient interface { // KeyVault implements a KMS using Azure Key Vault. // +// The URI format used in Azure Key Vault is the following: +// +// - azurekms:name=key-name;vault=vault-name +// - azurekms:name=key-name;vault=vault-name?version=key-version +// +// The scheme is "azurekms"; "name" is the key name; "vault" is the key vault +// name where the key is located; "version" is an optional parameter that +// defines the version of they key, if version is not given, the latest one will +// be used. +// // TODO(mariano): The implementation is using /services/keyvault/v7.1/keyvault // package, at some point Azure might create a keyvault client with all the // functionality in /sdk/keyvault, we should migrate to that once available. @@ -220,16 +233,12 @@ func (k *KeyVault) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyRespo return nil, errors.Wrap(err, "keyVault CreateKey failed") } - keyURI := uri.New("azurekms", url.Values{ - "vault": []string{vault}, - "id": []string{name}, - }).String() - publicKey, err := convertKey(resp.Key) if err != nil { return nil, err } + keyURI := getKeyName(vault, name, resp) return &apiv1.CreateKeyResponse{ Name: keyURI, PublicKey: publicKey, diff --git a/kms/azurekms/key_vault_test.go b/kms/azurekms/key_vault_test.go index 92048246..1f09b2d5 100644 --- a/kms/azurekms/key_vault_test.go +++ b/kms/azurekms/key_vault_test.go @@ -140,19 +140,19 @@ func TestKeyVault_GetPublicKey(t *testing.T) { wantErr bool }{ {"ok", fields{client}, args{&apiv1.GetPublicKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key", + Name: "azurekms:vault=my-vault;name=my-key", }}, pub, false}, {"ok with version", fields{client}, args{&apiv1.GetPublicKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key?version=my-version", + Name: "azurekms:vault=my-vault;name=my-key?version=my-version", }}, pub, false}, {"fail GetKey", fields{client}, args{&apiv1.GetPublicKeyRequest{ - Name: "azurekms:vault=my-vault;id=not-found?version=my-version", + Name: "azurekms:vault=my-vault;name=not-found?version=my-version", }}, nil, true}, {"fail vault", fields{client}, args{&apiv1.GetPublicKeyRequest{ - Name: "azurekms:vault=;id=not-found?version=my-version", + Name: "azurekms:vault=;name=not-found?version=my-version", }}, nil, true}, {"fail id", fields{client}, args{&apiv1.GetPublicKeyRequest{ - Name: "azurekms:vault=;id=?version=my-version", + Name: "azurekms:vault=;name=?version=my-version", }}, nil, true}, } for _, tt := range tests { @@ -244,136 +244,136 @@ func TestKeyVault_CreateKey(t *testing.T) { wantErr bool }{ {"ok P-256", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key", + Name: "azurekms:vault=my-vault;name=my-key", SignatureAlgorithm: apiv1.ECDSAWithSHA256, ProtectionLevel: apiv1.Software, }}, &apiv1.CreateKeyResponse{ - Name: "azurekms:id=my-key;vault=my-vault", + Name: "azurekms:name=my-key;vault=my-vault", PublicKey: ecPub, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "azurekms:id=my-key;vault=my-vault", + SigningKey: "azurekms:name=my-key;vault=my-vault", }, }, false}, {"ok P-256 HSM", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key", + Name: "azurekms:vault=my-vault;name=my-key", SignatureAlgorithm: apiv1.ECDSAWithSHA256, ProtectionLevel: apiv1.HSM, }}, &apiv1.CreateKeyResponse{ - Name: "azurekms:id=my-key;vault=my-vault", + Name: "azurekms:name=my-key;vault=my-vault", PublicKey: ecPub, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "azurekms:id=my-key;vault=my-vault", + SigningKey: "azurekms:name=my-key;vault=my-vault", }, }, false}, {"ok P-256 Default", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key", + Name: "azurekms:vault=my-vault;name=my-key", }}, &apiv1.CreateKeyResponse{ - Name: "azurekms:id=my-key;vault=my-vault", + Name: "azurekms:name=my-key;vault=my-vault", PublicKey: ecPub, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "azurekms:id=my-key;vault=my-vault", + SigningKey: "azurekms:name=my-key;vault=my-vault", }, }, false}, {"ok P-384", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key", + Name: "azurekms:vault=my-vault;name=my-key", SignatureAlgorithm: apiv1.ECDSAWithSHA384, }}, &apiv1.CreateKeyResponse{ - Name: "azurekms:id=my-key;vault=my-vault", + Name: "azurekms:name=my-key;vault=my-vault", PublicKey: ecPub, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "azurekms:id=my-key;vault=my-vault", + SigningKey: "azurekms:name=my-key;vault=my-vault", }, }, false}, {"ok P-521", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key", + Name: "azurekms:vault=my-vault;name=my-key", SignatureAlgorithm: apiv1.ECDSAWithSHA512, }}, &apiv1.CreateKeyResponse{ - Name: "azurekms:id=my-key;vault=my-vault", + Name: "azurekms:name=my-key;vault=my-vault", PublicKey: ecPub, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "azurekms:id=my-key;vault=my-vault", + SigningKey: "azurekms:name=my-key;vault=my-vault", }, }, false}, {"ok RSA 0", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key", + Name: "azurekms:vault=my-vault;name=my-key", Bits: 0, SignatureAlgorithm: apiv1.SHA256WithRSA, ProtectionLevel: apiv1.Software, }}, &apiv1.CreateKeyResponse{ - Name: "azurekms:id=my-key;vault=my-vault", + Name: "azurekms:name=my-key;vault=my-vault", PublicKey: rsaPub, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "azurekms:id=my-key;vault=my-vault", + SigningKey: "azurekms:name=my-key;vault=my-vault", }, }, false}, {"ok RSA 0 HSM", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key", + Name: "azurekms:vault=my-vault;name=my-key", Bits: 0, SignatureAlgorithm: apiv1.SHA256WithRSAPSS, ProtectionLevel: apiv1.HSM, }}, &apiv1.CreateKeyResponse{ - Name: "azurekms:id=my-key;vault=my-vault", + Name: "azurekms:name=my-key;vault=my-vault", PublicKey: rsaPub, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "azurekms:id=my-key;vault=my-vault", + SigningKey: "azurekms:name=my-key;vault=my-vault", }, }, false}, {"ok RSA 2048", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key", + Name: "azurekms:vault=my-vault;name=my-key", Bits: 2048, SignatureAlgorithm: apiv1.SHA384WithRSA, }}, &apiv1.CreateKeyResponse{ - Name: "azurekms:id=my-key;vault=my-vault", + Name: "azurekms:name=my-key;vault=my-vault", PublicKey: rsaPub, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "azurekms:id=my-key;vault=my-vault", + SigningKey: "azurekms:name=my-key;vault=my-vault", }, }, false}, {"ok RSA 3072", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key", + Name: "azurekms:vault=my-vault;name=my-key", Bits: 3072, SignatureAlgorithm: apiv1.SHA512WithRSA, }}, &apiv1.CreateKeyResponse{ - Name: "azurekms:id=my-key;vault=my-vault", + Name: "azurekms:name=my-key;vault=my-vault", PublicKey: rsaPub, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "azurekms:id=my-key;vault=my-vault", + SigningKey: "azurekms:name=my-key;vault=my-vault", }, }, false}, {"ok RSA 4096", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=my-key", + Name: "azurekms:vault=my-vault;name=my-key", Bits: 4096, SignatureAlgorithm: apiv1.SHA512WithRSAPSS, }}, &apiv1.CreateKeyResponse{ - Name: "azurekms:id=my-key;vault=my-vault", + Name: "azurekms:name=my-key;vault=my-vault", PublicKey: rsaPub, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: "azurekms:id=my-key;vault=my-vault", + SigningKey: "azurekms:name=my-key;vault=my-vault", }, }, false}, {"fail createKey", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=not-found", + Name: "azurekms:vault=my-vault;name=not-found", SignatureAlgorithm: apiv1.ECDSAWithSHA256, }}, nil, true}, {"fail convertKey", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=not-found", + Name: "azurekms:vault=my-vault;name=not-found", SignatureAlgorithm: apiv1.ECDSAWithSHA256, }}, nil, true}, {"fail name", fields{client}, args{&apiv1.CreateKeyRequest{ Name: "", }}, nil, true}, {"fail vault", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=;id=not-found?version=my-version", + Name: "azurekms:vault=;name=not-found?version=my-version", }}, nil, true}, {"fail id", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=?version=my-version", + Name: "azurekms:vault=my-vault;name=?version=my-version", }}, nil, true}, {"fail SignatureAlgorithm", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=not-found", + Name: "azurekms:vault=my-vault;name=not-found", SignatureAlgorithm: apiv1.PureEd25519, }}, nil, true}, {"fail bit size", fields{client}, args{&apiv1.CreateKeyRequest{ - Name: "azurekms:vault=my-vault;id=not-found", + Name: "azurekms:vault=my-vault;name=not-found", SignatureAlgorithm: apiv1.SHA384WithRSAPSS, Bits: 1024, }}, nil, true}, @@ -426,7 +426,7 @@ func TestKeyVault_CreateSigner(t *testing.T) { wantErr bool }{ {"ok", fields{client}, args{&apiv1.CreateSignerRequest{ - SigningKey: "azurekms:vault=my-vault;id=my-key", + SigningKey: "azurekms:vault=my-vault;name=my-key", }}, &Signer{ client: client, vaultBaseURL: "https://my-vault.vault.azure.net/", @@ -435,7 +435,7 @@ func TestKeyVault_CreateSigner(t *testing.T) { publicKey: pub, }, false}, {"ok with version", fields{client}, args{&apiv1.CreateSignerRequest{ - SigningKey: "azurekms:vault=my-vault;id=my-key;version=my-version", + SigningKey: "azurekms:vault=my-vault;name=my-key;version=my-version", }}, &Signer{ client: client, vaultBaseURL: "https://my-vault.vault.azure.net/", @@ -444,7 +444,7 @@ func TestKeyVault_CreateSigner(t *testing.T) { publicKey: pub, }, false}, {"fail GetKey", fields{client}, args{&apiv1.CreateSignerRequest{ - SigningKey: "azurekms:vault=my-vault;id=not-found;version=my-version", + SigningKey: "azurekms:vault=my-vault;name=not-found;version=my-version", }}, nil, true}, {"fail SigningKey", fields{client}, args{&apiv1.CreateSignerRequest{ SigningKey: "", diff --git a/kms/azurekms/signer_test.go b/kms/azurekms/signer_test.go index 798d69f9..389f65b3 100644 --- a/kms/azurekms/signer_test.go +++ b/kms/azurekms/signer_test.go @@ -44,24 +44,24 @@ func TestNewSigner(t *testing.T) { want crypto.Signer wantErr bool }{ - {"ok", args{client, "azurekms:vault=my-vault;id=my-key"}, &Signer{ + {"ok", args{client, "azurekms:vault=my-vault;name=my-key"}, &Signer{ client: client, vaultBaseURL: "https://my-vault.vault.azure.net/", name: "my-key", version: "", publicKey: pub, }, false}, - {"ok with version", args{client, "azurekms:id=my-key;vault=my-vault?version=my-version"}, &Signer{ + {"ok with version", args{client, "azurekms:name=my-key;vault=my-vault?version=my-version"}, &Signer{ client: client, vaultBaseURL: "https://my-vault.vault.azure.net/", name: "my-key", version: "my-version", publicKey: pub, }, false}, - {"fail GetKey", args{client, "azurekms:id=not-found;vault=my-vault?version=my-version"}, nil, true}, - {"fail vault", args{client, "azurekms:id=not-found;vault="}, nil, true}, - {"fail id", args{client, "azurekms:id=;vault=my-vault?version=my-version"}, nil, true}, - {"fail scheme", args{client, "kms:id=not-found;vault=my-vault?version=my-version"}, nil, true}, + {"fail GetKey", args{client, "azurekms:name=not-found;vault=my-vault?version=my-version"}, nil, true}, + {"fail vault", args{client, "azurekms:name=not-found;vault="}, nil, true}, + {"fail id", args{client, "azurekms:name=;vault=my-vault?version=my-version"}, nil, true}, + {"fail scheme", args{client, "kms:name=not-found;vault=my-vault?version=my-version"}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/kms/azurekms/utils.go b/kms/azurekms/utils.go index 46263726..6b6d1511 100644 --- a/kms/azurekms/utils.go +++ b/kms/azurekms/utils.go @@ -4,6 +4,7 @@ import ( "context" "crypto" "encoding/json" + "net/url" "time" "github.com/Azure/azure-sdk-for-go/services/keyvault/v7.1/keyvault" @@ -17,9 +18,34 @@ func defaultContext() (context.Context, context.CancelFunc) { return context.WithTimeout(context.Background(), 15*time.Second) } -// parseKeyName returns the key vault, name and version for urls like -// azurekms:vault=key-vault;id=key-name?version=key-version. If version is not -// passed the latest version will be used. +// getKeyName returns the uri of the key vault key. +func getKeyName(vault, name string, bundle keyvault.KeyBundle) string { + if bundle.Key != nil && bundle.Key.Kid != nil { + sm := keyIDRegexp.FindAllStringSubmatch(*bundle.Key.Kid, 1) + if len(sm) == 1 && len(sm[0]) == 4 { + m := sm[0] + u := uri.New(Scheme, url.Values{ + "vault": []string{m[1]}, + "name": []string{m[2]}, + }) + u.RawQuery = url.Values{"version": []string{m[3]}}.Encode() + return u.String() + } + } + // Fallback to URI without id. + return uri.New(Scheme, url.Values{ + "vault": []string{vault}, + "name": []string{name}, + }).String() +} + +// parseKeyName returns the key vault, name and version from URIs like: +// +// - azurekms:vault=key-vault;name=key-name +// - azurekms:vault=key-vault;name=key-name;id=key-id +// +// The key-id defines the version of the key, if it is not passed the latest +// version will be used. func parseKeyName(rawURI string) (vault, name, version string, err error) { var u *uri.URI @@ -27,13 +53,13 @@ func parseKeyName(rawURI string) (vault, name, version string, err error) { if err != nil { return } - - if vault = u.Get("vault"); vault == "" { - err = errors.Errorf("key uri %s is not valid: vault is missing", rawURI) + if name = u.Get("name"); name == "" { + err = errors.Errorf("key uri %s is not valid: name is missing", rawURI) return } - if name = u.Get("id"); name == "" { - err = errors.Errorf("key uri %s is not valid: id is missing", rawURI) + if vault = u.Get("vault"); vault == "" { + err = errors.Errorf("key uri %s is not valid: vault is missing", rawURI) + name = "" return } version = u.Get("version") diff --git a/kms/azurekms/utils_test.go b/kms/azurekms/utils_test.go new file mode 100644 index 00000000..53ec4057 --- /dev/null +++ b/kms/azurekms/utils_test.go @@ -0,0 +1,83 @@ +package azurekms + +import ( + "testing" + + "github.com/Azure/azure-sdk-for-go/services/keyvault/v7.1/keyvault" +) + +func Test_getKeyName(t *testing.T) { + getBundle := func(kid string) keyvault.KeyBundle { + return keyvault.KeyBundle{ + Key: &keyvault.JSONWebKey{ + Kid: &kid, + }, + } + } + + type args struct { + vault string + name string + bundle keyvault.KeyBundle + } + tests := []struct { + name string + args args + want string + }{ + {"ok", args{"my-vault", "my-key", getBundle("https://my-vault.vault.azure.net/keys/my-key/my-version")}, "azurekms:name=my-key;vault=my-vault?version=my-version"}, + {"ok default", args{"my-vault", "my-key", getBundle("https://my-vault.foo.net/keys/my-key/my-version")}, "azurekms:name=my-key;vault=my-vault"}, + {"ok too short", args{"my-vault", "my-key", getBundle("https://my-vault.vault.azure.net/keys/my-version")}, "azurekms:name=my-key;vault=my-vault"}, + {"ok too long", args{"my-vault", "my-key", getBundle("https://my-vault.vault.azure.net/keys/my-key/my-version/sign")}, "azurekms:name=my-key;vault=my-vault"}, + {"ok nil key", args{"my-vault", "my-key", keyvault.KeyBundle{}}, "azurekms:name=my-key;vault=my-vault"}, + {"ok nil kid", args{"my-vault", "my-key", keyvault.KeyBundle{Key: &keyvault.JSONWebKey{}}}, "azurekms:name=my-key;vault=my-vault"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := getKeyName(tt.args.vault, tt.args.name, tt.args.bundle); got != tt.want { + t.Errorf("getKeyName() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_parseKeyName(t *testing.T) { + type args struct { + rawURI string + } + tests := []struct { + name string + args args + wantVault string + wantName string + wantVersion string + wantErr bool + }{ + {"ok", args{"azurekms:name=my-key;vault=my-vault?version=my-version"}, "my-vault", "my-key", "my-version", false}, + {"ok no version", args{"azurekms:name=my-key;vault=my-vault"}, "my-vault", "my-key", "", false}, + {"fail scheme", args{"azure:name=my-key;vault=my-vault"}, "", "", "", true}, + {"fail parse uri", args{"azurekms:name=%ZZ;vault=my-vault"}, "", "", "", true}, + {"fail no name", args{"azurekms:vault=my-vault"}, "", "", "", true}, + {"fail empty name", args{"azurekms:name=;vault=my-vault"}, "", "", "", true}, + {"fail no vault", args{"azurekms:name=my-key"}, "", "", "", true}, + {"fail empty vault", args{"azurekms:name=my-key;vault="}, "", "", "", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotVault, gotName, gotVersion, err := parseKeyName(tt.args.rawURI) + if (err != nil) != tt.wantErr { + t.Errorf("parseKeyName() error = %v, wantErr %v", err, tt.wantErr) + return + } + if gotVault != tt.wantVault { + t.Errorf("parseKeyName() gotVault = %v, want %v", gotVault, tt.wantVault) + } + if gotName != tt.wantName { + t.Errorf("parseKeyName() gotName = %v, want %v", gotName, tt.wantName) + } + if gotVersion != tt.wantVersion { + t.Errorf("parseKeyName() gotVersion = %v, want %v", gotVersion, tt.wantVersion) + } + }) + } +}