From dc1e60b24f91f7ef9916c49a4d7ff66f176b040d Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 15 Apr 2024 12:32:18 +0200 Subject: [PATCH 1/2] Fix GCP CloudKMS resource URIs sometimes starting with `cloudkms:` On staging an error similar to the one below occurred: `Resource name 'cloudkms:projects/staging-a23d/locations/global/keyRings/b84b7b21-0993-43fd-b0c4-3c8cd204e635/cryptoKeys/root-key/cryptoKeyVersions/1' does not match pattern 'projects/([^/]{1,100})/locations/([a-zA-Z0-9_-]{1,63})/keyRings/([a-zA-Z0-9_-]{1,63})/cryptoKeys/([a-zA-Z0-9_-]{1,63})/cryptoKeyVersions/([a-zA-Z0-9_-]{1,63})'."` Not all cases that interact with the actual GCP API used the right format for the key URI. The code was changed to use a GCP resource name while (pre)loadin resources from GCP. The case was not caught in unit tests yet, because those rely primarily on mocks and there were no assertions in the mocks. --- kms/cloudkms/cloudkms_test.go | 70 ++++++++++++++++++---------------- kms/cloudkms/decrypter.go | 6 +-- kms/cloudkms/decrypter_test.go | 25 ++++++++---- kms/cloudkms/signer.go | 6 +-- kms/cloudkms/signer_test.go | 45 +++++++++++----------- 5 files changed, 83 insertions(+), 69 deletions(-) diff --git a/kms/cloudkms/cloudkms_test.go b/kms/cloudkms/cloudkms_test.go index fd5b30cc..e32edb02 100644 --- a/kms/cloudkms/cloudkms_test.go +++ b/kms/cloudkms/cloudkms_test.go @@ -13,6 +13,7 @@ import ( "cloud.google.com/go/kms/apiv1/kmspb" gax "github.com/googleapis/gax-go/v2" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.step.sm/crypto/kms/apiv1" "go.step.sm/crypto/kms/uri" "go.step.sm/crypto/pemutil" @@ -174,13 +175,9 @@ func TestCloudKMS_CreateSigner(t *testing.T) { keyURI := uri.NewOpaque(Scheme, keyName).String() pemBytes, err := os.ReadFile("testdata/pub.pem") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) pk, err := pemutil.ParseKey(pemBytes) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) type fields struct { client KeyManagementClient @@ -196,17 +193,20 @@ func TestCloudKMS_CreateSigner(t *testing.T) { wantErr bool }{ {"ok", fields{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, args{&apiv1.CreateSignerRequest{SigningKey: keyName}}, &Signer{client: &MockClient{}, signingKey: keyName, publicKey: pk}, false}, {"ok with uri", fields{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, args{&apiv1.CreateSignerRequest{SigningKey: keyURI}}, &Signer{client: &MockClient{}, signingKey: keyName, publicKey: pk}, false}, {"fail", fields{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return nil, fmt.Errorf("test error") }, }}, args{&apiv1.CreateSignerRequest{SigningKey: ""}}, nil, true}, @@ -238,13 +238,9 @@ func TestCloudKMS_CreateKey(t *testing.T) { alreadyExists := status.Error(codes.AlreadyExists, "already exists") pemBytes, err := os.ReadFile("testdata/pub.pem") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) pk, err := pemutil.ParseKey(pemBytes) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) var retries int type fields struct { @@ -269,7 +265,8 @@ func TestCloudKMS_CreateKey(t *testing.T) { assert.Nil(t, req.CryptoKey.DestroyScheduledDuration) return &kmspb.CryptoKey{Name: keyName}, nil }, - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, @@ -284,7 +281,8 @@ func TestCloudKMS_CreateKey(t *testing.T) { assert.Equal(t, req.CryptoKey.DestroyScheduledDuration, durationpb.New(24*time.Hour)) return &kmspb.CryptoKey{Name: keyName}, nil }, - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, @@ -301,7 +299,8 @@ func TestCloudKMS_CreateKey(t *testing.T) { createCryptoKey: func(_ context.Context, _ *kmspb.CreateCryptoKeyRequest, _ ...gax.CallOption) (*kmspb.CryptoKey, error) { return &kmspb.CryptoKey{Name: keyName}, nil }, - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, @@ -318,7 +317,8 @@ func TestCloudKMS_CreateKey(t *testing.T) { createCryptoKeyVersion: func(_ context.Context, _ *kmspb.CreateCryptoKeyVersionRequest, _ ...gax.CallOption) (*kmspb.CryptoKeyVersion, error) { return &kmspb.CryptoKeyVersion{Name: keyName + "/cryptoKeyVersions/2"}, nil }, - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, @@ -332,7 +332,8 @@ func TestCloudKMS_CreateKey(t *testing.T) { createCryptoKey: func(_ context.Context, _ *kmspb.CreateCryptoKeyRequest, _ ...gax.CallOption) (*kmspb.CryptoKey, error) { return &kmspb.CryptoKey{Name: keyName}, nil }, - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") if retries != 2 { retries++ return nil, status.Error(codes.FailedPrecondition, "key is not enabled, current state is: PENDING_GENERATION") @@ -391,7 +392,8 @@ func TestCloudKMS_CreateKey(t *testing.T) { createCryptoKey: func(_ context.Context, _ *kmspb.CreateCryptoKeyRequest, _ ...gax.CallOption) (*kmspb.CryptoKey, error) { return &kmspb.CryptoKey{Name: keyName}, nil }, - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return nil, testError }, }}, @@ -424,13 +426,9 @@ func TestCloudKMS_GetPublicKey(t *testing.T) { testError := fmt.Errorf("an error") pemBytes, err := os.ReadFile("testdata/pub.pem") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) pk, err := pemutil.ParseKey(pemBytes) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) var retries int type fields struct { @@ -448,28 +446,32 @@ func TestCloudKMS_GetPublicKey(t *testing.T) { }{ {"ok", fields{ &MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, args{&apiv1.GetPublicKeyRequest{Name: keyName}}, pk, false}, {"ok with uri", fields{ &MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, args{&apiv1.GetPublicKeyRequest{Name: keyURI}}, pk, false}, {"ok with resource uri", fields{ &MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, args{&apiv1.GetPublicKeyRequest{Name: keyResource}}, pk, false}, {"ok with retries", fields{ &MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") if retries != 2 { retries++ return nil, status.Error(codes.FailedPrecondition, "key is not enabled, current state is: PENDING_GENERATION") @@ -481,14 +483,16 @@ func TestCloudKMS_GetPublicKey(t *testing.T) { {"fail name", fields{&MockClient{}}, args{&apiv1.GetPublicKeyRequest{}}, nil, true}, {"fail get public key", fields{ &MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return nil, testError }, }}, args{&apiv1.GetPublicKeyRequest{Name: keyName}}, nil, true}, {"fail parse pem", fields{ &MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string("bad pem")}, nil }, }}, diff --git a/kms/cloudkms/decrypter.go b/kms/cloudkms/decrypter.go index 5d925840..fdbb1afb 100644 --- a/kms/cloudkms/decrypter.go +++ b/kms/cloudkms/decrypter.go @@ -39,19 +39,19 @@ func NewDecrypter(client KeyManagementClient, decryptionKey string) (*Decrypter, client: client, decryptionKey: resourceName(decryptionKey), } - if err := decrypter.preloadKey(decryptionKey); err != nil { // TODO(hs): (option for) lazy load instead? + if err := decrypter.preloadKey(); err != nil { // TODO(hs): (option for) lazy load instead? return nil, err } return decrypter, nil } -func (d *Decrypter) preloadKey(signingKey string) error { +func (d *Decrypter) preloadKey() error { ctx, cancel := defaultContext() defer cancel() response, err := d.client.GetPublicKey(ctx, &kmspb.GetPublicKeyRequest{ - Name: signingKey, + Name: d.decryptionKey, }) if err != nil { return fmt.Errorf("cloudKMS GetPublicKey failed: %w", err) diff --git a/kms/cloudkms/decrypter_test.go b/kms/cloudkms/decrypter_test.go index 990076c4..a756cbae 100644 --- a/kms/cloudkms/decrypter_test.go +++ b/kms/cloudkms/decrypter_test.go @@ -39,22 +39,26 @@ func TestCloudKMS_CreateDecrypter(t *testing.T) { wantErr bool }{ {"ok", fields{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, args{&apiv1.CreateDecrypterRequest{DecryptionKey: keyName}}, &Decrypter{client: &MockClient{}, decryptionKey: keyName, publicKey: pk}, false}, {"ok with uri", fields{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, args{&apiv1.CreateDecrypterRequest{DecryptionKey: "cloudkms:resource=" + keyName}}, &Decrypter{client: &MockClient{}, decryptionKey: keyName, publicKey: pk}, false}, {"ok with opaque uri", fields{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }}, args{&apiv1.CreateDecrypterRequest{DecryptionKey: "cloudkms:" + keyName}}, &Decrypter{client: &MockClient{}, decryptionKey: keyName, publicKey: pk}, false}, {"fail", fields{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return nil, fmt.Errorf("test error") }, }}, args{&apiv1.CreateDecrypterRequest{DecryptionKey: ""}}, nil, true}, @@ -92,17 +96,20 @@ func TestNewDecrypter(t *testing.T) { wantErr bool }{ {"ok", args{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }, "decryptionKey"}, &Decrypter{client: &MockClient{}, decryptionKey: "decryptionKey", publicKey: pk}, false}, {"fail get public key", args{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return nil, fmt.Errorf("an error") }, }, "decryptionKey"}, nil, true}, {"fail parse pem", args{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string("bad pem")}, nil }, }, "decryptionKey"}, nil, true}, @@ -160,21 +167,25 @@ func TestDecrypter_Decrypt(t *testing.T) { keyName := "projects/p/locations/l/keyRings/k/cryptoKeys/c/cryptoKeyVersions/1" okClient := &MockClient{ asymmetricDecrypt: func(ctx context.Context, adr *kmspb.AsymmetricDecryptRequest, co ...gax.CallOption) (*kmspb.AsymmetricDecryptResponse, error) { + assert.NotContains(t, adr.Name, "cloudkms:") return &kmspb.AsymmetricDecryptResponse{Plaintext: []byte("decrypted"), PlaintextCrc32C: wrapperspb.Int64(crc32c([]byte("decrypted"))), VerifiedCiphertextCrc32C: true}, nil }, } failClient := &MockClient{ asymmetricDecrypt: func(ctx context.Context, adr *kmspb.AsymmetricDecryptRequest, co ...gax.CallOption) (*kmspb.AsymmetricDecryptResponse, error) { + assert.NotContains(t, adr.Name, "cloudkms:") return nil, fmt.Errorf("an error") }, } requestCRC32Client := &MockClient{ asymmetricDecrypt: func(ctx context.Context, adr *kmspb.AsymmetricDecryptRequest, co ...gax.CallOption) (*kmspb.AsymmetricDecryptResponse, error) { + assert.NotContains(t, adr.Name, "cloudkms:") return &kmspb.AsymmetricDecryptResponse{Plaintext: []byte("decrypted"), PlaintextCrc32C: wrapperspb.Int64(crc32c([]byte("decrypted"))), VerifiedCiphertextCrc32C: false}, nil }, } responseCRC32Client := &MockClient{ asymmetricDecrypt: func(ctx context.Context, adr *kmspb.AsymmetricDecryptRequest, co ...gax.CallOption) (*kmspb.AsymmetricDecryptResponse, error) { + assert.NotContains(t, adr.Name, "cloudkms:") return &kmspb.AsymmetricDecryptResponse{Plaintext: []byte("decrypted"), PlaintextCrc32C: wrapperspb.Int64(crc32c([]byte("wrong"))), VerifiedCiphertextCrc32C: true}, nil }, } diff --git a/kms/cloudkms/signer.go b/kms/cloudkms/signer.go index e6eda0a6..bac380a2 100644 --- a/kms/cloudkms/signer.go +++ b/kms/cloudkms/signer.go @@ -28,19 +28,19 @@ func NewSigner(c KeyManagementClient, signingKey string) (*Signer, error) { client: c, signingKey: resourceName(signingKey), } - if err := signer.preloadKey(signingKey); err != nil { + if err := signer.preloadKey(); err != nil { return nil, err } return signer, nil } -func (s *Signer) preloadKey(signingKey string) error { +func (s *Signer) preloadKey() error { ctx, cancel := defaultContext() defer cancel() response, err := s.client.GetPublicKey(ctx, &kmspb.GetPublicKeyRequest{ - Name: signingKey, + Name: s.signingKey, }) if err != nil { return errors.Wrap(err, "cloudKMS GetPublicKey failed") diff --git a/kms/cloudkms/signer_test.go b/kms/cloudkms/signer_test.go index e624b48e..0b391b04 100644 --- a/kms/cloudkms/signer_test.go +++ b/kms/cloudkms/signer_test.go @@ -13,18 +13,16 @@ import ( "cloud.google.com/go/kms/apiv1/kmspb" gax "github.com/googleapis/gax-go/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.step.sm/crypto/pemutil" ) -func Test_newSigner(t *testing.T) { +func Test_NewSigner(t *testing.T) { pemBytes, err := os.ReadFile("testdata/pub.pem") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) pk, err := pemutil.ParseKey(pemBytes) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) type args struct { c KeyManagementClient @@ -37,27 +35,32 @@ func Test_newSigner(t *testing.T) { wantErr bool }{ {"ok", args{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }, "signingKey"}, &Signer{client: &MockClient{}, signingKey: "signingKey", publicKey: pk}, false}, {"ok with uri", args{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }, "cloudkms:resource=signingKey"}, &Signer{client: &MockClient{}, signingKey: "signingKey", publicKey: pk}, false}, {"ok with opaque uri", args{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string(pemBytes)}, nil }, }, "cloudkms:signingKey"}, &Signer{client: &MockClient{}, signingKey: "signingKey", publicKey: pk}, false}, {"fail get public key", args{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return nil, fmt.Errorf("an error") }, }, "signingKey"}, nil, true}, {"fail parse pem", args{&MockClient{ - getPublicKey: func(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + getPublicKey: func(_ context.Context, r *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.PublicKey{Pem: string("bad pem")}, nil }, }, "signingKey"}, nil, true}, @@ -81,13 +84,9 @@ func Test_newSigner(t *testing.T) { func Test_signer_Public(t *testing.T) { pemBytes, err := os.ReadFile("testdata/pub.pem") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) pk, err := pemutil.ParseKey(pemBytes) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) type fields struct { client KeyManagementClient @@ -118,12 +117,14 @@ func Test_signer_Public(t *testing.T) { func Test_signer_Sign(t *testing.T) { keyName := "projects/p/locations/l/keyRings/k/cryptoKeys/c/cryptoKeyVersions/1" okClient := &MockClient{ - asymmetricSign: func(_ context.Context, _ *kmspb.AsymmetricSignRequest, _ ...gax.CallOption) (*kmspb.AsymmetricSignResponse, error) { + asymmetricSign: func(_ context.Context, r *kmspb.AsymmetricSignRequest, _ ...gax.CallOption) (*kmspb.AsymmetricSignResponse, error) { + assert.NotContains(t, r.Name, "cloudkms:") return &kmspb.AsymmetricSignResponse{Signature: []byte("ok signature")}, nil }, } failClient := &MockClient{ - asymmetricSign: func(_ context.Context, _ *kmspb.AsymmetricSignRequest, _ ...gax.CallOption) (*kmspb.AsymmetricSignResponse, error) { + asymmetricSign: func(_ context.Context, r *kmspb.AsymmetricSignRequest, _ ...gax.CallOption) (*kmspb.AsymmetricSignResponse, error) { + assert.NotContains(t, r.Name, "cloudkms:") return nil, fmt.Errorf("an error") }, } @@ -170,9 +171,7 @@ func Test_signer_Sign(t *testing.T) { func TestSigner_SignatureAlgorithm(t *testing.T) { pemBytes, err := os.ReadFile("testdata/pub.pem") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) client := &MockClient{ getPublicKey: func(_ context.Context, req *kmspb.GetPublicKeyRequest, _ ...gax.CallOption) (*kmspb.PublicKey, error) { From 927f0940620929d89151b3a31ef954dc1d1a07ea Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Mon, 15 Apr 2024 12:57:11 +0200 Subject: [PATCH 2/2] Fix typo in `cryptoKeyName` --- kms/cloudkms/cloudkms.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/kms/cloudkms/cloudkms.go b/kms/cloudkms/cloudkms.go index 807adbaa..fd319889 100644 --- a/kms/cloudkms/cloudkms.go +++ b/kms/cloudkms/cloudkms.go @@ -204,7 +204,7 @@ func (k *CloudKMS) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyRespo return nil, err } - var crytoKeyName string + var cryptoKeyName string ctx, cancel := defaultContext() defer cancel() @@ -240,13 +240,13 @@ func (k *CloudKMS) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyRespo if err != nil { return nil, errors.Wrap(err, "cloudKMS CreateCryptoKeyVersion failed") } - crytoKeyName = response.Name + cryptoKeyName = response.Name } else { - crytoKeyName = response.Name + "/cryptoKeyVersions/1" + cryptoKeyName = response.Name + "/cryptoKeyVersions/1" } // Use uri format for the keys - crytoKeyName = uri.NewOpaque(Scheme, crytoKeyName).String() + cryptoKeyName = uri.NewOpaque(Scheme, cryptoKeyName).String() // Sleep deterministically to avoid retries because of PENDING_GENERATING. // One second is often enough. @@ -256,17 +256,17 @@ func (k *CloudKMS) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyRespo // Retrieve public key to add it to the response. pk, err := k.GetPublicKey(&apiv1.GetPublicKeyRequest{ - Name: crytoKeyName, + Name: cryptoKeyName, }) if err != nil { return nil, errors.Wrap(err, "cloudKMS GetPublicKey failed") } return &apiv1.CreateKeyResponse{ - Name: crytoKeyName, + Name: cryptoKeyName, PublicKey: pk, CreateSignerRequest: apiv1.CreateSignerRequest{ - SigningKey: crytoKeyName, + SigningKey: cryptoKeyName, }, }, nil }