From 77a9a5b81831f0cc7da74aedc44593f8207d3d4d Mon Sep 17 00:00:00 2001 From: Amr Rezk Date: Mon, 24 Aug 2026 15:36:48 +0300 Subject: [PATCH 1/8] Add gcpkms keystore implementation --- keystore/gcpkms/client.go | 119 +++++++++ keystore/gcpkms/fake_client.go | 278 ++++++++++++++++++++ keystore/gcpkms/keystore.go | 418 +++++++++++++++++++++++++++++++ keystore/gcpkms/keystore_test.go | 340 +++++++++++++++++++++++++ keystore/go.mod | 36 ++- keystore/go.sum | 81 ++++-- keystore/reader.go | 2 +- 7 files changed, 1240 insertions(+), 34 deletions(-) create mode 100644 keystore/gcpkms/client.go create mode 100644 keystore/gcpkms/fake_client.go create mode 100644 keystore/gcpkms/keystore.go create mode 100644 keystore/gcpkms/keystore_test.go diff --git a/keystore/gcpkms/client.go b/keystore/gcpkms/client.go new file mode 100644 index 0000000000..e9aec2cb74 --- /dev/null +++ b/keystore/gcpkms/client.go @@ -0,0 +1,119 @@ +package gcpkms + +import ( + "context" + "errors" + "fmt" + + apiv1 "cloud.google.com/go/kms/apiv1" + "cloud.google.com/go/kms/apiv1/kmspb" + "github.com/googleapis/gax-go/v2" + "google.golang.org/api/iterator" + "google.golang.org/api/option" +) + +// Client is an interface that defines the operations needed by the keystore. It keeps the keystore +// independent of the generated Google Cloud KMS client. +// +// Every method here can be authorized per CryptoKey, so a deployment can bind exactly the keys it +// configures and nothing else. Listing a whole key ring is deliberately not part of this interface — +// see [KeyRingLister]. +// +// These methods are based on the Google Cloud KMS Go client interface. +// https://pkg.go.dev/cloud.google.com/go/kms/apiv1 +type Client interface { + GetCryptoKeyVersion(ctx context.Context, req *kmspb.GetCryptoKeyVersionRequest, opts ...gax.CallOption) (*kmspb.CryptoKeyVersion, error) + GetPublicKey(ctx context.Context, req *kmspb.GetPublicKeyRequest, opts ...gax.CallOption) (*kmspb.PublicKey, error) + AsymmetricSign(ctx context.Context, req *kmspb.AsymmetricSignRequest, opts ...gax.CallOption) (*kmspb.AsymmetricSignResponse, error) + ListCryptoKeyVersions(ctx context.Context, cryptoKeyName string) ([]*kmspb.CryptoKeyVersion, error) +} + +// KeyRingLister is an optional capability: a Client that can also enumerate a key ring. GetKeys only +// needs it when called with no key allowlist. +// +// It is kept out of [Client] on purpose. ListCryptoKeys takes the key ring as its parent, so +// cloudkms.cryptoKeys.list can only be bound at the key ring or above — strictly broader than every +// other permission the keystore needs, all of which bind to individual CryptoKeys. A least-privilege +// deployment that names its keys explicitly should never be forced to implement, or be granted, a +// ring-wide list. +type KeyRingLister interface { + ListCryptoKeys(ctx context.Context, keyRingName string) ([]*kmspb.CryptoKey, error) +} + +// ClientWithClose is the client returned by NewClient: the full Cloud KMS surface, including key +// ring listing, plus the underlying transport lifecycle. Whether listing actually succeeds is a +// matter of the credentials' IAM bindings, not of the Go type. +type ClientWithClose interface { + Client + KeyRingLister + Close() error +} + +// ClientOptions contains options for creating a Cloud KMS client. +type ClientOptions struct { + // CredentialsFile is the path to a GCP service account JSON key. Local development only — + // leave empty in production, where credentials come from the default credential chain + // (GKE Workload Identity, GCE instance/service accounts, or GOOGLE_APPLICATION_CREDENTIALS). + CredentialsFile string +} + +// NewClient constructs a new Google Cloud KMS client using the Go SDK. +// If CredentialsFile is specified, it uses service-account-key-based authentication (local dev). +// Otherwise, it uses Application Default Credentials (Workload Identity in production, etc.). +func NewClient(ctx context.Context, opts ClientOptions) (ClientWithClose, error) { + var clientOpts []option.ClientOption + if opts.CredentialsFile != "" { + clientOpts = append(clientOpts, option.WithCredentialsFile(opts.CredentialsFile)) + } + client, err := apiv1.NewKeyManagementClient(ctx, clientOpts...) + if err != nil { + return nil, fmt.Errorf("failed to create Google Cloud KMS client: %w", err) + } + return &clientAdapter{client: client}, nil +} + +type clientAdapter struct { + client *apiv1.KeyManagementClient +} + +func (c *clientAdapter) GetCryptoKeyVersion(ctx context.Context, req *kmspb.GetCryptoKeyVersionRequest, opts ...gax.CallOption) (*kmspb.CryptoKeyVersion, error) { + return c.client.GetCryptoKeyVersion(ctx, req, opts...) +} + +func (c *clientAdapter) GetPublicKey(ctx context.Context, req *kmspb.GetPublicKeyRequest, opts ...gax.CallOption) (*kmspb.PublicKey, error) { + return c.client.GetPublicKey(ctx, req, opts...) +} + +func (c *clientAdapter) AsymmetricSign(ctx context.Context, req *kmspb.AsymmetricSignRequest, opts ...gax.CallOption) (*kmspb.AsymmetricSignResponse, error) { + return c.client.AsymmetricSign(ctx, req, opts...) +} + +func (c *clientAdapter) ListCryptoKeys(ctx context.Context, keyRingName string) ([]*kmspb.CryptoKey, error) { + iter := c.client.ListCryptoKeys(ctx, &kmspb.ListCryptoKeysRequest{Parent: keyRingName}) + return drain(iter.Next, fmt.Sprintf("crypto keys in %s", keyRingName)) +} + +func (c *clientAdapter) ListCryptoKeyVersions(ctx context.Context, cryptoKeyName string) ([]*kmspb.CryptoKeyVersion, error) { + iter := c.client.ListCryptoKeyVersions(ctx, &kmspb.ListCryptoKeyVersionsRequest{Parent: cryptoKeyName}) + return drain(iter.Next, fmt.Sprintf("crypto key versions of %s", cryptoKeyName)) +} + +// drain reads a Cloud KMS iterator to completion. what describes the listed resources and is only +// used to build the error message. +func drain[T any](next func() (T, error), what string) ([]T, error) { + items := make([]T, 0) + for { + item, err := next() + if errors.Is(err, iterator.Done) { + return items, nil + } + if err != nil { + return nil, fmt.Errorf("failed to list %s: %w", what, err) + } + items = append(items, item) + } +} + +func (c *clientAdapter) Close() error { + return c.client.Close() +} diff --git a/keystore/gcpkms/fake_client.go b/keystore/gcpkms/fake_client.go new file mode 100644 index 0000000000..af8c55504b --- /dev/null +++ b/keystore/gcpkms/fake_client.go @@ -0,0 +1,278 @@ +package gcpkms + +import ( + "context" + "crypto/ed25519" + "crypto/x509" + "encoding/pem" + "errors" + "fmt" + "strconv" + "strings" + "time" + + "cloud.google.com/go/kms/apiv1/kmspb" + "github.com/ethereum/go-ethereum/crypto" + "github.com/googleapis/gax-go/v2" + "google.golang.org/protobuf/types/known/timestamppb" + "google.golang.org/protobuf/types/known/wrapperspb" + + "github.com/smartcontractkit/chainlink-common/keystore" + "github.com/smartcontractkit/chainlink-common/keystore/internal" + "github.com/smartcontractkit/chainlink-common/keystore/kms" +) + +// Key identifies one in-memory CryptoKeyVersion held by FakeGCPKMSClient. KeyID is a CryptoKey +// resource name (projects/

/locations//keyRings//cryptoKeys/); several Keys may share a +// KeyID to emulate a rotated key with multiple versions. +type Key struct { + KeyType keystore.KeyType + KeyID string + PrivateKey internal.Raw + + // VersionNumber is the CryptoKeyVersion number. Defaults to 1. + VersionNumber uint64 + // State is the version state. Defaults to ENABLED. + State kmspb.CryptoKeyVersion_CryptoKeyVersionState + // Purpose is the purpose reported for the parent CryptoKey. Defaults to ASYMMETRIC_SIGN; set it + // to emulate an unrelated key sharing the key ring. + Purpose kmspb.CryptoKey_CryptoKeyPurpose + // Algorithm is the algorithm reported for this version and for the parent CryptoKey's version + // template. Defaults to the algorithm matching KeyType; set it to emulate an unsupported one. + Algorithm kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm +} + +// FakeGCPKMSClient is an in-memory implementation of Client for tests. It emulates the parts of +// Google Cloud KMS that the keystore uses, producing the same wire formats (PEM SPKI public keys, +// DER ECDSA signatures, raw Ed25519 signatures) and the same resource-naming rules: asymmetric +// operations only accept CryptoKeyVersion names, and CryptoKeys never report a primary version. +type FakeGCPKMSClient struct { + keys []Key + createdAt time.Time +} + +func NewFakeGCPKMSClient(keys []Key) (*FakeGCPKMSClient, error) { + keys = append([]Key(nil), keys...) + for i := range keys { + if keys[i].KeyID == "" { + return nil, errors.New("key ID is required") + } + if keys[i].VersionNumber == 0 { + keys[i].VersionNumber = 1 + } + if keys[i].State == kmspb.CryptoKeyVersion_CRYPTO_KEY_VERSION_STATE_UNSPECIFIED { + keys[i].State = kmspb.CryptoKeyVersion_ENABLED + } + if keys[i].Purpose == kmspb.CryptoKey_CRYPTO_KEY_PURPOSE_UNSPECIFIED { + keys[i].Purpose = kmspb.CryptoKey_ASYMMETRIC_SIGN + } + if keys[i].Algorithm == kmspb.CryptoKeyVersion_CRYPTO_KEY_VERSION_ALGORITHM_UNSPECIFIED { + algorithm, err := keyTypeToAlgorithm(keys[i].KeyType) + if err != nil { + return nil, err + } + keys[i].Algorithm = algorithm + } + } + return &FakeGCPKMSClient{ + keys: keys, + createdAt: time.Now(), + }, nil +} + +func keyTypeToAlgorithm(keyType keystore.KeyType) (kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm, error) { + switch keyType { + case keystore.ECDSA_S256: + return kmspb.CryptoKeyVersion_EC_SIGN_SECP256K1_SHA256, nil + case keystore.Ed25519: + return kmspb.CryptoKeyVersion_EC_SIGN_ED25519, nil + default: + return 0, fmt.Errorf("unsupported key type: %s", keyType) + } +} + +// versionName returns the CryptoKeyVersion resource name of a key. +func (k Key) versionName() string { + return k.KeyID + cryptoKeyVersionsSegment + strconv.FormatUint(k.VersionNumber, 10) +} + +func (m *FakeGCPKMSClient) toCryptoKeyVersion(key *Key) *kmspb.CryptoKeyVersion { + return &kmspb.CryptoKeyVersion{ + Name: key.versionName(), + Algorithm: key.Algorithm, + State: key.State, + CreateTime: timestamppb.New(m.createdAt), + } +} + +// findVersion looks up a key by its CryptoKeyVersion resource name. Cloud KMS rejects a bare +// CryptoKey name on the asymmetric endpoints, so the fake does too. +func (m *FakeGCPKMSClient) findVersion(versionName string) (*Key, error) { + if !strings.Contains(versionName, cryptoKeyVersionsSegment) { + return nil, fmt.Errorf("%q is not a CryptoKeyVersion resource name", versionName) + } + for i := range m.keys { + if m.keys[i].versionName() == versionName { + return &m.keys[i], nil + } + } + return nil, errors.New("key not found") +} + +func (m *FakeGCPKMSClient) GetCryptoKeyVersion(ctx context.Context, req *kmspb.GetCryptoKeyVersionRequest, opts ...gax.CallOption) (*kmspb.CryptoKeyVersion, error) { + if req.Name == "" { + return nil, errors.New("key version name is required") + } + key, err := m.findVersion(req.Name) + if err != nil { + return nil, err + } + return m.toCryptoKeyVersion(key), nil +} + +func (m *FakeGCPKMSClient) ListCryptoKeyVersions(ctx context.Context, cryptoKeyName string) ([]*kmspb.CryptoKeyVersion, error) { + if cryptoKeyName == "" { + return nil, errors.New("crypto key name is required") + } + versions := make([]*kmspb.CryptoKeyVersion, 0, len(m.keys)) + for i := range m.keys { + if m.keys[i].KeyID != cryptoKeyName { + continue + } + versions = append(versions, m.toCryptoKeyVersion(&m.keys[i])) + } + if len(versions) == 0 { + return nil, errors.New("key not found") + } + return versions, nil +} + +func (m *FakeGCPKMSClient) ListCryptoKeys(ctx context.Context, keyRingName string) ([]*kmspb.CryptoKey, error) { + keys := make([]*kmspb.CryptoKey, 0, len(m.keys)) + seen := make(map[string]struct{}, len(m.keys)) + for i := range m.keys { + key := &m.keys[i] + if !strings.HasPrefix(key.KeyID, keyRingName+"/cryptoKeys/") { + continue + } + if _, ok := seen[key.KeyID]; ok { + continue + } + seen[key.KeyID] = struct{}{} + // Note: no Primary — Cloud KMS only sets it for ENCRYPT_DECRYPT keys. + keys = append(keys, &kmspb.CryptoKey{ + Name: key.KeyID, + Purpose: key.Purpose, + CreateTime: timestamppb.New(m.createdAt), + VersionTemplate: &kmspb.CryptoKeyVersionTemplate{Algorithm: key.Algorithm}, + }) + } + return keys, nil +} + +func (m *FakeGCPKMSClient) GetPublicKey(ctx context.Context, req *kmspb.GetPublicKeyRequest, opts ...gax.CallOption) (*kmspb.PublicKey, error) { + if req.Name == "" { + return nil, errors.New("key version name is required") + } + key, err := m.findVersion(req.Name) + if err != nil { + return nil, err + } + + var derPubKey []byte + switch key.KeyType { + case keystore.ECDSA_S256: + ecdsaKey, err := crypto.ToECDSA(internal.Bytes(key.PrivateKey)) + if err != nil { + return nil, err + } + derPubKey, err = kms.SEC1ToASN1PublicKey(crypto.FromECDSAPub(&ecdsaKey.PublicKey)) + if err != nil { + return nil, err + } + case keystore.Ed25519: + ed25519PrivKey, err := ed25519PrivateKey(key) + if err != nil { + return nil, err + } + pubKey := ed25519PrivKey.Public().(ed25519.PublicKey) + derPubKey, err = x509.MarshalPKIXPublicKey(pubKey) + if err != nil { + return nil, err + } + default: + return nil, fmt.Errorf("unsupported key type: %s", key.KeyType) + } + + pemBytes := pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: derPubKey}) + return &kmspb.PublicKey{ + Name: req.Name, + Algorithm: key.Algorithm, + Pem: string(pemBytes), + PemCrc32C: wrapperspb.Int64(crc32c(pemBytes)), + PublicKeyFormat: kmspb.PublicKey_PEM, + }, nil +} + +func (m *FakeGCPKMSClient) AsymmetricSign(ctx context.Context, req *kmspb.AsymmetricSignRequest, opts ...gax.CallOption) (*kmspb.AsymmetricSignResponse, error) { + if req.Name == "" { + return nil, errors.New("key version name is required") + } + key, err := m.findVersion(req.Name) + if err != nil { + return nil, err + } + + switch key.KeyType { + case keystore.ECDSA_S256: + if req.Digest == nil { + return nil, errors.New("digest is required for ECDSA signing") + } + ecdsaKey, err := crypto.ToECDSA(internal.Bytes(key.PrivateKey)) + if err != nil { + return nil, err + } + sec1Sig, err := crypto.Sign(req.Digest.GetSha256(), ecdsaKey) + if err != nil { + return nil, err + } + derSig, err := kms.SEC1ToASN1Sig(sec1Sig) + if err != nil { + return nil, err + } + return &kmspb.AsymmetricSignResponse{ + Name: req.Name, + Signature: derSig, + SignatureCrc32C: wrapperspb.Int64(crc32c(derSig)), + VerifiedDigestCrc32C: true, + }, nil + case keystore.Ed25519: + ed25519PrivKey, err := ed25519PrivateKey(key) + if err != nil { + return nil, err + } + signature := ed25519.Sign(ed25519PrivKey, req.Data) + return &kmspb.AsymmetricSignResponse{ + Name: req.Name, + Signature: signature, + SignatureCrc32C: wrapperspb.Int64(crc32c(signature)), + VerifiedDataCrc32C: true, + }, nil + default: + return nil, fmt.Errorf("unsupported key type: %s", key.KeyType) + } +} + +// ed25519PrivateKey returns the key's Ed25519 private key, erroring rather than letting the +// crypto/ed25519 helpers panic on a wrong-sized key. +func ed25519PrivateKey(key *Key) (ed25519.PrivateKey, error) { + privKey := ed25519.PrivateKey(internal.Bytes(key.PrivateKey)) + if len(privKey) != ed25519.PrivateKeySize { + return nil, fmt.Errorf("invalid Ed25519 private key length: expected %d bytes, got %d", ed25519.PrivateKeySize, len(privKey)) + } + return privKey, nil +} + +func (m *FakeGCPKMSClient) Close() error { + return nil +} diff --git a/keystore/gcpkms/keystore.go b/keystore/gcpkms/keystore.go new file mode 100644 index 0000000000..73bd930c0e --- /dev/null +++ b/keystore/gcpkms/keystore.go @@ -0,0 +1,418 @@ +package gcpkms + +import ( + "context" + "crypto/ed25519" + "crypto/x509" + "encoding/pem" + "errors" + "fmt" + "hash/crc32" + "sort" + "strconv" + "strings" + "sync" + + "cloud.google.com/go/kms/apiv1/kmspb" + "google.golang.org/protobuf/types/known/wrapperspb" + + "github.com/smartcontractkit/chainlink-common/keystore" + "github.com/smartcontractkit/chainlink-common/keystore/kms" +) + +// cryptoKeyVersionsSegment separates a CryptoKey resource name from its version number in a +// CryptoKeyVersion resource name. +const cryptoKeyVersionsSegment = "/cryptoKeyVersions/" + +// errNoEnabledVersion is returned when a CryptoKey has no enabled version to sign with. It is +// matchable so that key ring listings can skip such keys instead of failing outright. +var errNoEnabledVersion = errors.New("has no enabled version") + +// castagnoliTable is the CRC32C (Castagnoli) table used by Google Cloud KMS for integrity checks. +var castagnoliTable = crc32.MakeTable(crc32.Castagnoli) + +// crc32c returns the CRC32C checksum of data, matching the value Google Cloud KMS uses in its +// *_crc32c integrity fields. +func crc32c(data []byte) int64 { + return int64(crc32.Checksum(data, castagnoliTable)) +} + +// checkCrc32c verifies that a received CRC32C checksum matches the computed value of the data. +// This is Google's recommended integrity check for responses returned by Cloud KMS. A missing +// checksum is treated as a failure: the responses we check it on always carry one, so its absence +// means the response was truncated or tampered with in transit. +func checkCrc32c(data []byte, received *wrapperspb.Int64Value) error { + if received == nil { + return errors.New("CRC32C integrity check failed: response is missing its checksum") + } + if want := crc32c(data); want != received.Value { + return fmt.Errorf("CRC32C integrity check failed: computed %d, received %d", want, received.Value) + } + return nil +} + +type keystoreSignerReader struct { + client Client + keyRingName string + + // publicKeys caches public keys by CryptoKeyVersion resource name. A version's public key is + // immutable, so this is safe to cache indefinitely and saves a round trip per signature. + publicKeysMu sync.RWMutex + publicKeys map[string][]byte +} + +type KeystoreOptions struct { + // KeyRingName is required when GetKeys is called without an explicit key allowlist. + // It must be a resource name in the format projects/

/locations//keyRings/. + KeyRingName string +} + +func NewKeystore(client Client, opts KeystoreOptions) (interface { + keystore.Reader + keystore.Signer +}, error) { + if client == nil { + return nil, fmt.Errorf("GCP KMS client is required") + } + return &keystoreSignerReader{ + client: client, + keyRingName: opts.KeyRingName, + publicKeys: make(map[string][]byte), + }, nil +} + +// cryptoKeyVersionAlgorithmToKeyType converts a Cloud KMS CryptoKeyVersionAlgorithm to a keystore +// KeyType. Google Cloud KMS supports: +// - EC_SIGN_SECP256K1_SHA256 (secp256k1) -> ECDSA_S256 +// - EC_SIGN_ED25519 (Ed25519) -> Ed25519 +func cryptoKeyVersionAlgorithmToKeyType(algo kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm) (keystore.KeyType, error) { + switch algo { + case kmspb.CryptoKeyVersion_EC_SIGN_SECP256K1_SHA256: + return keystore.ECDSA_S256, nil + case kmspb.CryptoKeyVersion_EC_SIGN_ED25519: + return keystore.Ed25519, nil + default: + return "", fmt.Errorf("unsupported Cloud KMS key algorithm: %s (supported: EC_SIGN_SECP256K1_SHA256, EC_SIGN_ED25519)", algo) + } +} + +type resolvedKey struct { + version *kmspb.CryptoKeyVersion + keyType keystore.KeyType +} + +// resolveKeyVersion resolves a key name to a specific, enabled CryptoKeyVersion and its keystore +// KeyType. +// +// Cloud KMS only populates CryptoKey.Primary for ENCRYPT_DECRYPT keys — asymmetric signing keys +// never have a primary version — so a concrete version has to be selected here. keyName may be +// either: +// - a CryptoKeyVersion resource name (.../cryptoKeys//cryptoKeyVersions/), which pins that +// exact version, or +// - a CryptoKey resource name, in which case the highest-numbered enabled version is used, so +// that rotations are picked up without a redeploy. +func (k *keystoreSignerReader) resolveKeyVersion(ctx context.Context, keyName string) (resolvedKey, error) { + var version *kmspb.CryptoKeyVersion + if strings.Contains(keyName, cryptoKeyVersionsSegment) { + got, err := k.client.GetCryptoKeyVersion(ctx, &kmspb.GetCryptoKeyVersionRequest{Name: keyName}) + if err != nil { + return resolvedKey{}, fmt.Errorf("failed to get crypto key version %s: %w", keyName, err) + } + if got == nil { + return resolvedKey{}, fmt.Errorf("Cloud KMS returned an empty crypto key version response for %s", keyName) + } + if got.Name != keyName { + return resolvedKey{}, fmt.Errorf("crypto key version response has name %q, expected %q", got.Name, keyName) + } + version = got + } else { + latest, err := k.latestEnabledVersion(ctx, keyName) + if err != nil { + return resolvedKey{}, err + } + version = latest + } + + if version.State != kmspb.CryptoKeyVersion_ENABLED { + return resolvedKey{}, fmt.Errorf("crypto key version %s is not enabled (state=%s)", version.Name, version.State) + } + if version.CreateTime == nil { + return resolvedKey{}, fmt.Errorf("crypto key version %s has no creation time", version.Name) + } + keyType, err := cryptoKeyVersionAlgorithmToKeyType(version.Algorithm) + if err != nil { + return resolvedKey{}, fmt.Errorf("crypto key %s: %w", keyName, err) + } + return resolvedKey{version: version, keyType: keyType}, nil +} + +// latestEnabledVersion returns the highest-numbered enabled version of a CryptoKey. +func (k *keystoreSignerReader) latestEnabledVersion(ctx context.Context, cryptoKeyName string) (*kmspb.CryptoKeyVersion, error) { + versions, err := k.client.ListCryptoKeyVersions(ctx, cryptoKeyName) + if err != nil { + return nil, err + } + var latest *kmspb.CryptoKeyVersion + var latestNumber uint64 + for _, version := range versions { + if version == nil || version.State != kmspb.CryptoKeyVersion_ENABLED { + continue + } + number, err := cryptoKeyVersionNumber(version.Name) + if err != nil { + return nil, err + } + if latest == nil || number > latestNumber { + latest, latestNumber = version, number + } + } + if latest == nil { + return nil, fmt.Errorf("crypto key %s %w", cryptoKeyName, errNoEnabledVersion) + } + return latest, nil +} + +// cryptoKeyVersionNumber extracts the trailing version number from a CryptoKeyVersion resource +// name. Cloud KMS assigns these sequentially, so a higher number means a newer version. +func cryptoKeyVersionNumber(versionName string) (uint64, error) { + index := strings.LastIndex(versionName, cryptoKeyVersionsSegment) + if index < 0 { + return 0, fmt.Errorf("unexpected crypto key version resource name %q", versionName) + } + number, err := strconv.ParseUint(versionName[index+len(cryptoKeyVersionsSegment):], 10, 64) + if err != nil { + return 0, fmt.Errorf("unexpected crypto key version resource name %q: %w", versionName, err) + } + return number, nil +} + +// getPublicKeyBytes fetches the public key for a crypto key version and converts it to the +// keystore's native format for the given key type. Results are cached per version name. +func (k *keystoreSignerReader) getPublicKeyBytes(ctx context.Context, versionName string, keyType keystore.KeyType) ([]byte, error) { + k.publicKeysMu.RLock() + cached, ok := k.publicKeys[versionName] + k.publicKeysMu.RUnlock() + if ok { + return cached, nil + } + + pk, err := k.client.GetPublicKey(ctx, &kmspb.GetPublicKeyRequest{Name: versionName}) + if err != nil { + return nil, fmt.Errorf("failed to get public key for %s: %w", versionName, err) + } + if pk == nil { + return nil, fmt.Errorf("Cloud KMS returned an empty public key response for %s", versionName) + } + if pk.Name != versionName { + return nil, fmt.Errorf("public key response has name %q, expected %q", pk.Name, versionName) + } + if err := checkCrc32c([]byte(pk.Pem), pk.PemCrc32C); err != nil { + return nil, fmt.Errorf("public key for %s: %w", versionName, err) + } + + block, _ := pem.Decode([]byte(pk.Pem)) + if block == nil { + return nil, fmt.Errorf("failed to decode PEM public key for %s", versionName) + } + + var publicKeyBytes []byte + switch keyType { + case keystore.ECDSA_S256: + // GCP returns the public key in ASN.1 DER-encoded SubjectPublicKeyInfo (SPKI) format, + // identical to AWS. Reuse the shared conversion. + publicKeyBytes, err = kms.ASN1ToSEC1PublicKey(block.Bytes) + if err != nil { + return nil, err + } + case keystore.Ed25519: + pubKey, err := x509.ParsePKIXPublicKey(block.Bytes) + if err != nil { + return nil, fmt.Errorf("failed to convert Ed25519 public key for %s: %w", versionName, err) + } + ed25519PubKey, ok := pubKey.(ed25519.PublicKey) + if !ok { + return nil, fmt.Errorf("failed to convert Ed25519 public key for %s to ed25519.PublicKey", versionName) + } + publicKeyBytes = ed25519PubKey + default: + return nil, fmt.Errorf("unsupported key type: %s", keyType) + } + + k.publicKeysMu.Lock() + k.publicKeys[versionName] = publicKeyBytes + k.publicKeysMu.Unlock() + return publicKeyBytes, nil +} + +// signingKeyNames filters a key ring listing down to the asymmetric signing keys this keystore can +// use. Key rings are commonly shared, so keys with another purpose or an unsupported algorithm are +// skipped rather than failing the whole listing. +func signingKeyNames(listedKeys []*kmspb.CryptoKey) ([]string, error) { + keyNames := make([]string, 0, len(listedKeys)) + for _, key := range listedKeys { + if key == nil || key.Name == "" { + return nil, fmt.Errorf("Cloud KMS returned a crypto key without a name") + } + if key.Purpose != kmspb.CryptoKey_ASYMMETRIC_SIGN { + continue + } + if template := key.VersionTemplate; template != nil { + if _, err := cryptoKeyVersionAlgorithmToKeyType(template.Algorithm); err != nil { + continue + } + } + keyNames = append(keyNames, key.Name) + } + return keyNames, nil +} + +// GetKeys lists keys in the Cloud KMS keystore. +// +// Key names are either CryptoKey resource names +// (projects/

/locations//keyRings//cryptoKeys/), for which the highest-numbered enabled +// version is resolved so that rotations are picked up without redeploying, or CryptoKeyVersion +// resource names, which pin one version. The returned key names match the requested ones, in the +// requested order. +// +// When no key names are given, the configured key ring is listed and keys that this keystore +// cannot use — another purpose, an unsupported algorithm, or no enabled version — are skipped, +// since a key ring may hold keys that have nothing to do with this keystore. Explicitly requested +// keys always surface their errors. +// +// Listing requires a client that implements [KeyRingLister] and credentials holding a ring-wide +// cloudkms.cryptoKeys.list. Deployments that follow least privilege grant neither and should pass an +// explicit key allowlist instead. +func (k *keystoreSignerReader) GetKeys(ctx context.Context, req keystore.GetKeysRequest) (keystore.GetKeysResponse, error) { + keyNames := append([]string(nil), req.KeyNames...) + listed := len(keyNames) == 0 + if listed { + if k.keyRingName == "" { + return keystore.GetKeysResponse{}, fmt.Errorf("key ring name is required to list Cloud KMS keys") + } + lister, ok := k.client.(KeyRingLister) + if !ok { + return keystore.GetKeysResponse{}, fmt.Errorf("cannot list key ring %s: this Cloud KMS client does not implement KeyRingLister; request keys explicitly by name instead", k.keyRingName) + } + listedKeys, err := lister.ListCryptoKeys(ctx, k.keyRingName) + if err != nil { + return keystore.GetKeysResponse{}, err + } + keyNames, err = signingKeyNames(listedKeys) + if err != nil { + return keystore.GetKeysResponse{}, err + } + // Cloud KMS does not guarantee a listing order; sort for a stable response. + sort.Strings(keyNames) + } + + keys := make([]keystore.GetKeyResponse, 0, len(keyNames)) + seen := make(map[string]struct{}, len(keyNames)) + for _, keyName := range keyNames { + if _, ok := seen[keyName]; ok { + return keystore.GetKeysResponse{}, fmt.Errorf("key %s provided multiple times", keyName) + } + seen[keyName] = struct{}{} + resolved, err := k.resolveKeyVersion(ctx, keyName) + if err != nil { + if listed && errors.Is(err, errNoEnabledVersion) { + continue + } + return keystore.GetKeysResponse{}, err + } + publicKeyBytes, err := k.getPublicKeyBytes(ctx, resolved.version.Name, resolved.keyType) + if err != nil { + return keystore.GetKeysResponse{}, err + } + createdAt := resolved.version.CreateTime.AsTime() + keys = append(keys, keystore.GetKeyResponse{ + KeyInfo: keystore.NewKeyInfo(keyName, resolved.keyType, createdAt, publicKeyBytes, []byte{}), + }) + } + return keystore.GetKeysResponse{Keys: keys}, nil +} + +// Sign signs data using the Cloud KMS crypto key specified by the key name. +func (k *keystoreSignerReader) Sign(ctx context.Context, req keystore.SignRequest) (keystore.SignResponse, error) { + resolved, err := k.resolveKeyVersion(ctx, req.KeyName) + if err != nil { + return keystore.SignResponse{}, err + } + versionName := resolved.version.Name + + switch resolved.keyType { + case keystore.ECDSA_S256: + if len(req.Data) != 32 { + return keystore.SignResponse{}, fmt.Errorf("data must be 32 bytes for ECDSA_S256, got %d: %w", len(req.Data), keystore.ErrInvalidSignRequest) + } + // Needed only to recover the SEC1 `v` byte below. Cached per version, so this is at most + // one extra round trip per key version for the lifetime of the keystore. + pubKeyBytes, err := k.getPublicKeyBytes(ctx, versionName, resolved.keyType) + if err != nil { + return keystore.SignResponse{}, fmt.Errorf("failed to get public key for key %s: %w", req.KeyName, err) + } + + // The data is a pre-hashed 32-byte digest. For EC_SIGN_SECP256K1_SHA256 the digest field is + // the exact bytes signed; Cloud KMS does not re-hash. + sig, err := k.client.AsymmetricSign(ctx, &kmspb.AsymmetricSignRequest{ + Name: versionName, + Digest: &kmspb.Digest{Digest: &kmspb.Digest_Sha256{Sha256: req.Data}}, + DigestCrc32C: wrapperspb.Int64(crc32c(req.Data)), + }) + if err != nil { + return keystore.SignResponse{}, fmt.Errorf("failed to sign data: %w", err) + } + if sig == nil { + return keystore.SignResponse{}, fmt.Errorf("Cloud KMS returned an empty signing response") + } + if sig.Name != versionName { + return keystore.SignResponse{}, fmt.Errorf("signing response has name %q, expected %q", sig.Name, versionName) + } + if !sig.VerifiedDigestCrc32C { + return keystore.SignResponse{}, fmt.Errorf("Cloud KMS did not verify the digest CRC32C checksum") + } + if err := checkCrc32c(sig.Signature, sig.SignatureCrc32C); err != nil { + return keystore.SignResponse{}, fmt.Errorf("signature for key %s: %w", req.KeyName, err) + } + // Cloud KMS returns the ECDSA signature in ASN.1 DER format, identical to AWS. Reuse the + // shared conversion to SEC1 (R || S || V). + signature, err := kms.ASN1ToSEC1Sig(sig.Signature, pubKeyBytes, req.Data) + if err != nil { + return keystore.SignResponse{}, fmt.Errorf("failed to convert Cloud KMS signature to SEC1 signature: %w", err) + } + return keystore.SignResponse{Signature: signature}, nil + case keystore.Ed25519: + // Ed25519 signs arbitrary length messages. For EC_SIGN_ED25519 the raw data field is the + // exact bytes signed; Cloud KMS does not hash. + sig, err := k.client.AsymmetricSign(ctx, &kmspb.AsymmetricSignRequest{ + Name: versionName, + Data: req.Data, + DataCrc32C: wrapperspb.Int64(crc32c(req.Data)), + }) + if err != nil { + return keystore.SignResponse{}, fmt.Errorf("failed to sign data: %w", err) + } + if sig == nil { + return keystore.SignResponse{}, fmt.Errorf("Cloud KMS returned an empty signing response") + } + if sig.Name != versionName { + return keystore.SignResponse{}, fmt.Errorf("signing response has name %q, expected %q", sig.Name, versionName) + } + if !sig.VerifiedDataCrc32C { + return keystore.SignResponse{}, fmt.Errorf("Cloud KMS did not verify the data CRC32C checksum") + } + if err := checkCrc32c(sig.Signature, sig.SignatureCrc32C); err != nil { + return keystore.SignResponse{}, fmt.Errorf("signature for key %s: %w", req.KeyName, err) + } + // Ed25519 signatures from Cloud KMS are already in the correct format (64 bytes). + if len(sig.Signature) != 64 { + return keystore.SignResponse{}, fmt.Errorf("invalid Ed25519 signature length: expected 64 bytes, got %d", len(sig.Signature)) + } + return keystore.SignResponse{Signature: sig.Signature}, nil + default: + return keystore.SignResponse{}, fmt.Errorf("key %s: %w", req.KeyName, keystore.ErrInvalidSignRequest) + } +} + +func (k *keystoreSignerReader) Verify(ctx context.Context, req keystore.VerifyRequest) (keystore.VerifyResponse, error) { + return keystore.Verify(ctx, req) +} diff --git a/keystore/gcpkms/keystore_test.go b/keystore/gcpkms/keystore_test.go new file mode 100644 index 0000000000..610dc426bd --- /dev/null +++ b/keystore/gcpkms/keystore_test.go @@ -0,0 +1,340 @@ +package gcpkms_test + +import ( + "crypto/ed25519" + "testing" + + "cloud.google.com/go/kms/apiv1/kmspb" + "github.com/ethereum/go-ethereum/crypto" + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink-common/keystore" + gcpkms "github.com/smartcontractkit/chainlink-common/keystore/gcpkms" + "github.com/smartcontractkit/chainlink-common/keystore/internal" +) + +const ( + keyRingName = "projects/test-project/locations/us-central1/keyRings/test-ring" + keyName = keyRingName + "/cryptoKeys/test-key" + keyName2 = keyRingName + "/cryptoKeys/test-key-2" +) + +func TestGCPKMSKeystore(t *testing.T) { + key, err := crypto.GenerateKey() + require.NoError(t, err) + key2, err := crypto.GenerateKey() + require.NoError(t, err) + fakeClient, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{ + { + KeyType: keystore.ECDSA_S256, + PrivateKey: internal.NewRaw(crypto.FromECDSA(key)), + KeyID: keyName, + }, + { + KeyType: keystore.ECDSA_S256, + PrivateKey: internal.NewRaw(crypto.FromECDSA(key2)), + KeyID: keyName2, + }, + }) + require.NoError(t, err) + ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + require.NoError(t, err) + ctx := t.Context() + + t.Run("GetKeys", func(t *testing.T) { + t.Run("listing all keys", func(t *testing.T) { + resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{}) + require.NoError(t, err) + require.Len(t, resp.Keys, 2) + require.Equal(t, keyName, resp.Keys[0].KeyInfo.Name) + require.Equal(t, keyName2, resp.Keys[1].KeyInfo.Name) + }) + t.Run("specific keys preserve the requested order", func(t *testing.T) { + resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{ + KeyNames: []string{keyName2, keyName}, + }) + require.NoError(t, err) + require.Len(t, resp.Keys, 2) + require.Equal(t, keyName2, resp.Keys[0].KeyInfo.Name) + require.Equal(t, keyName, resp.Keys[1].KeyInfo.Name) + require.Equal(t, keystore.ECDSA_S256, resp.Keys[1].KeyInfo.KeyType) + require.Equal(t, crypto.FromECDSAPub(&key.PublicKey), resp.Keys[1].KeyInfo.PublicKey) + }) + t.Run("explicit key version", func(t *testing.T) { + resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{ + KeyNames: []string{keyName + "/cryptoKeyVersions/1"}, + }) + require.NoError(t, err) + require.Len(t, resp.Keys, 1) + require.Equal(t, keyName+"/cryptoKeyVersions/1", resp.Keys[0].KeyInfo.Name) + require.Equal(t, crypto.FromECDSAPub(&key.PublicKey), resp.Keys[0].KeyInfo.PublicKey) + }) + t.Run("no such key", func(t *testing.T) { + _, err := ks.GetKeys(ctx, keystore.GetKeysRequest{ + KeyNames: []string{"projects/p/locations/l/keyRings/r/cryptoKeys/nope"}, + }) + require.Error(t, err) + }) + }) + + t.Run("SignVerify", func(t *testing.T) { + t.Run("invalid sign request", func(t *testing.T) { + _, err := ks.Sign(ctx, keystore.SignRequest{ + KeyName: keyName, + Data: make([]byte, 31), // 31 byte digest + }) + require.Error(t, err) + require.ErrorIs(t, err, keystore.ErrInvalidSignRequest) + }) + t.Run("no such key", func(t *testing.T) { + _, err := ks.Sign(ctx, keystore.SignRequest{ + KeyName: "projects/p/locations/l/keyRings/r/cryptoKeys/nope", + Data: make([]byte, 32), // 32 byte digest + }) + require.Error(t, err) + }) + t.Run("success", func(t *testing.T) { + signResp, err := ks.Sign(ctx, keystore.SignRequest{ + KeyName: keyName, + Data: make([]byte, 32), // 32 byte digest + }) + require.NoError(t, err) + require.NotNil(t, signResp.Signature) + verifyResp, err := ks.Verify(ctx, keystore.VerifyRequest{ + KeyType: keystore.ECDSA_S256, + PublicKey: crypto.FromECDSAPub(&key.PublicKey), + Data: make([]byte, 32), // 32 byte digest + Signature: signResp.Signature, + }) + require.NoError(t, err) + require.True(t, verifyResp.Valid) + }) + }) +} + +func TestGCPKMSKeystore_Ed25519(t *testing.T) { + _, ed25519PrivKey, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + ed25519PubKey := ed25519PrivKey.Public().(ed25519.PublicKey) + + fakeClient, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{ + { + KeyType: keystore.Ed25519, + KeyID: keyName, + PrivateKey: internal.NewRaw(ed25519PrivKey), + }, + }) + require.NoError(t, err) + ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + require.NoError(t, err) + ctx := t.Context() + + t.Run("GetKeys", func(t *testing.T) { + resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{ + KeyNames: []string{keyName}, + }) + require.NoError(t, err) + require.Len(t, resp.Keys, 1) + require.Equal(t, keyName, resp.Keys[0].KeyInfo.Name) + require.Equal(t, keystore.Ed25519, resp.Keys[0].KeyInfo.KeyType) + require.Equal(t, []byte(ed25519PubKey), resp.Keys[0].KeyInfo.PublicKey) + }) + + t.Run("SignVerify", func(t *testing.T) { + // Ed25519 can sign arbitrary length messages + testData := []byte("hello, world") + signResp, err := ks.Sign(ctx, keystore.SignRequest{ + KeyName: keyName, + Data: testData, + }) + require.NoError(t, err) + require.NotNil(t, signResp.Signature) + require.Len(t, signResp.Signature, 64) // Ed25519 signatures are 64 bytes + + verifyResp, err := ks.Verify(ctx, keystore.VerifyRequest{ + KeyType: keystore.Ed25519, + PublicKey: ed25519PubKey, + Data: testData, + Signature: signResp.Signature, + }) + require.NoError(t, err) + require.True(t, verifyResp.Valid) + }) +} + +// The keystore must never rely on CryptoKey.Primary: Cloud KMS only populates it for +// ENCRYPT_DECRYPT keys, so an asymmetric signing key's version has to be resolved by listing. +func TestGCPKMSKeystore_ResolvesLatestEnabledVersion(t *testing.T) { + oldKey, err := crypto.GenerateKey() + require.NoError(t, err) + newKey, err := crypto.GenerateKey() + require.NoError(t, err) + disabledKey, err := crypto.GenerateKey() + require.NoError(t, err) + + fakeClient, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{ + {KeyType: keystore.ECDSA_S256, KeyID: keyName, VersionNumber: 1, PrivateKey: internal.NewRaw(crypto.FromECDSA(oldKey))}, + {KeyType: keystore.ECDSA_S256, KeyID: keyName, VersionNumber: 2, PrivateKey: internal.NewRaw(crypto.FromECDSA(newKey))}, + { + KeyType: keystore.ECDSA_S256, + KeyID: keyName, + VersionNumber: 3, + State: kmspb.CryptoKeyVersion_DISABLED, + PrivateKey: internal.NewRaw(crypto.FromECDSA(disabledKey)), + }, + }) + require.NoError(t, err) + ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + require.NoError(t, err) + ctx := t.Context() + + t.Run("highest enabled version wins", func(t *testing.T) { + resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName}}) + require.NoError(t, err) + require.Len(t, resp.Keys, 1) + require.Equal(t, crypto.FromECDSAPub(&newKey.PublicKey), resp.Keys[0].KeyInfo.PublicKey) + }) + t.Run("signing uses the same version", func(t *testing.T) { + signResp, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyName, Data: make([]byte, 32)}) + require.NoError(t, err) + verifyResp, err := ks.Verify(ctx, keystore.VerifyRequest{ + KeyType: keystore.ECDSA_S256, + PublicKey: crypto.FromECDSAPub(&newKey.PublicKey), + Data: make([]byte, 32), + Signature: signResp.Signature, + }) + require.NoError(t, err) + require.True(t, verifyResp.Valid) + }) + t.Run("pinning an older version", func(t *testing.T) { + resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName + "/cryptoKeyVersions/1"}}) + require.NoError(t, err) + require.Len(t, resp.Keys, 1) + require.Equal(t, crypto.FromECDSAPub(&oldKey.PublicKey), resp.Keys[0].KeyInfo.PublicKey) + }) + t.Run("pinning a disabled version fails", func(t *testing.T) { + _, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName + "/cryptoKeyVersions/3"}}) + require.ErrorContains(t, err, "is not enabled") + }) +} + +// A key ring is commonly shared, so listing it must skip keys this keystore cannot use instead of +// failing the whole call. +func TestGCPKMSKeystore_ListSkipsUnusableKeys(t *testing.T) { + signingKey, err := crypto.GenerateKey() + require.NoError(t, err) + otherKey, err := crypto.GenerateKey() + require.NoError(t, err) + unsupportedKey, err := crypto.GenerateKey() + require.NoError(t, err) + disabledKey, err := crypto.GenerateKey() + require.NoError(t, err) + + fakeClient, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{ + {KeyType: keystore.ECDSA_S256, KeyID: keyName, PrivateKey: internal.NewRaw(crypto.FromECDSA(signingKey))}, + { + // An encryption key that happens to live in the same key ring. + KeyType: keystore.ECDSA_S256, + KeyID: keyRingName + "/cryptoKeys/encrypt-key", + Purpose: kmspb.CryptoKey_ENCRYPT_DECRYPT, + PrivateKey: internal.NewRaw(crypto.FromECDSA(otherKey)), + }, + { + // A signing key on a curve this keystore does not support. + KeyType: keystore.ECDSA_S256, + KeyID: keyRingName + "/cryptoKeys/p256-key", + Algorithm: kmspb.CryptoKeyVersion_EC_SIGN_P256_SHA256, + PrivateKey: internal.NewRaw(crypto.FromECDSA(unsupportedKey)), + }, + { + // A supported key whose only version has been disabled. + KeyType: keystore.ECDSA_S256, + KeyID: keyRingName + "/cryptoKeys/disabled-key", + State: kmspb.CryptoKeyVersion_DISABLED, + PrivateKey: internal.NewRaw(crypto.FromECDSA(disabledKey)), + }, + }) + require.NoError(t, err) + ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + require.NoError(t, err) + ctx := t.Context() + + resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{}) + require.NoError(t, err) + require.Len(t, resp.Keys, 1) + require.Equal(t, keyName, resp.Keys[0].KeyInfo.Name) + + // Explicitly requesting an unusable key still surfaces the error. + _, err = ks.GetKeys(ctx, keystore.GetKeysRequest{ + KeyNames: []string{keyRingName + "/cryptoKeys/p256-key"}, + }) + require.ErrorContains(t, err, "unsupported Cloud KMS key algorithm") + _, err = ks.GetKeys(ctx, keystore.GetKeysRequest{ + KeyNames: []string{keyRingName + "/cryptoKeys/disabled-key"}, + }) + require.ErrorContains(t, err, "has no enabled version") +} + +func TestGCPKMSKeystore_InvalidEd25519Key(t *testing.T) { + fakeClient, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{ + { + KeyType: keystore.Ed25519, + KeyID: keyName, + PrivateKey: internal.NewRaw(make([]byte, ed25519.SeedSize)), // seed, not a private key + }, + }) + require.NoError(t, err) + ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + require.NoError(t, err) + + // Must error rather than panic inside crypto/ed25519. + _, err = ks.Sign(t.Context(), keystore.SignRequest{KeyName: keyName, Data: []byte("hello")}) + require.ErrorContains(t, err, "invalid Ed25519 private key length") +} + +// A least-privilege deployment holds per-CryptoKey permissions only, with no ring-wide +// cloudkms.cryptoKeys.list. Such a client implements Client but not KeyRingLister, and must still be +// able to do everything except enumerate the ring. +type noListClient struct { + gcpkms.Client // embedded as an interface: promotes only Client's methods, not ListCryptoKeys +} + +func TestGCPKMSKeystore_ClientWithoutKeyRingLister(t *testing.T) { + key, err := crypto.GenerateKey() + require.NoError(t, err) + fakeClient, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{ + {KeyType: keystore.ECDSA_S256, KeyID: keyName, PrivateKey: internal.NewRaw(crypto.FromECDSA(key))}, + }) + require.NoError(t, err) + + var client gcpkms.Client = noListClient{Client: fakeClient} + _, isLister := client.(gcpkms.KeyRingLister) + require.False(t, isLister, "test double must not expose ListCryptoKeys") + + ks, err := gcpkms.NewKeystore(client, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + require.NoError(t, err) + ctx := t.Context() + + t.Run("explicit key names work", func(t *testing.T) { + resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName}}) + require.NoError(t, err) + require.Len(t, resp.Keys, 1) + require.Equal(t, crypto.FromECDSAPub(&key.PublicKey), resp.Keys[0].KeyInfo.PublicKey) + }) + t.Run("signing works", func(t *testing.T) { + signResp, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyName, Data: make([]byte, 32)}) + require.NoError(t, err) + verifyResp, err := ks.Verify(ctx, keystore.VerifyRequest{ + KeyType: keystore.ECDSA_S256, + PublicKey: crypto.FromECDSAPub(&key.PublicKey), + Data: make([]byte, 32), + Signature: signResp.Signature, + }) + require.NoError(t, err) + require.True(t, verifyResp.Valid) + }) + t.Run("listing reports a clear error", func(t *testing.T) { + _, err := ks.GetKeys(ctx, keystore.GetKeysRequest{}) + require.ErrorContains(t, err, "does not implement KeyRingLister") + }) +} diff --git a/keystore/go.mod b/keystore/go.mod index c63e014d7d..6d0622ec9e 100644 --- a/keystore/go.mod +++ b/keystore/go.mod @@ -3,6 +3,7 @@ module github.com/smartcontractkit/chainlink-common/keystore go 1.26.6 require ( + cloud.google.com/go/kms v1.33.0 github.com/NethermindEth/juno v0.15.11 github.com/NethermindEth/starknet.go v0.17.1 github.com/aws/aws-sdk-go-v2 v1.41.1 @@ -14,6 +15,7 @@ require ( github.com/ethereum/go-ethereum v1.17.4 github.com/gagliardetto/solana-go v1.13.0 github.com/google/uuid v1.6.0 + github.com/googleapis/gax-go/v2 v2.23.0 github.com/hdevalence/ed25519consensus v0.2.0 github.com/jmoiron/sqlx v1.4.0 github.com/lib/pq v1.10.9 @@ -29,10 +31,17 @@ require ( go.dedis.ch/fixbuf v1.0.3 go.dedis.ch/kyber/v3 v3.1.0 golang.org/x/crypto v0.53.0 + google.golang.org/api v0.287.1 google.golang.org/protobuf v1.36.11 ) require ( + cloud.google.com/go v0.123.0 // indirect + cloud.google.com/go/auth v0.20.0 // indirect + cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect + cloud.google.com/go/compute/metadata v0.9.0 // indirect + cloud.google.com/go/iam v1.11.0 // indirect + cloud.google.com/go/longrunning v1.2.0 // indirect cosmossdk.io/api v0.7.6 // indirect cosmossdk.io/collections v0.4.0 // indirect cosmossdk.io/core v0.11.0 // indirect @@ -99,6 +108,7 @@ require ( github.com/dvsekhvalnov/jose2go v1.7.0 // indirect github.com/ethereum/c-kzg-4844/v2 v2.1.6 // indirect github.com/fatih/color v1.18.0 // indirect + github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fjl/jsonw v0.1.0 // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/fxamacker/cbor/v2 v2.9.0 // indirect @@ -127,6 +137,8 @@ require ( github.com/google/btree v1.1.3 // indirect github.com/google/flatbuffers v25.2.10+incompatible // indirect github.com/google/go-cmp v0.7.0 // indirect + github.com/google/s2a-go v0.1.9 // indirect + github.com/googleapis/enterprise-certificate-proxy v0.3.17 // indirect github.com/gorilla/websocket v1.5.3 // indirect github.com/graph-gophers/graphql-go v1.5.0 // indirect github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect @@ -208,8 +220,9 @@ require ( go.mongodb.org/mongo-driver v1.17.7 // indirect go.opencensus.io v0.24.0 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect - go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.63.0 // indirect - go.opentelemetry.io/otel v1.43.0 // indirect + go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.67.0 // indirect + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.67.0 // indirect + go.opentelemetry.io/otel v1.44.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.12.2 // indirect go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp v0.19.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.36.0 // indirect @@ -221,29 +234,30 @@ require ( go.opentelemetry.io/otel/exporters/stdout/stdoutmetric v1.36.0 // indirect go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.36.0 // indirect go.opentelemetry.io/otel/log v0.19.0 // indirect - go.opentelemetry.io/otel/metric v1.43.0 // indirect - go.opentelemetry.io/otel/sdk v1.43.0 // indirect + go.opentelemetry.io/otel/metric v1.44.0 // indirect + go.opentelemetry.io/otel/sdk v1.44.0 // indirect go.opentelemetry.io/otel/sdk/log v0.19.0 // indirect - go.opentelemetry.io/otel/sdk/metric v1.43.0 // indirect - go.opentelemetry.io/otel/trace v1.43.0 // indirect + go.opentelemetry.io/otel/sdk/metric v1.44.0 // indirect + go.opentelemetry.io/otel/trace v1.44.0 // indirect go.opentelemetry.io/proto/otlp v1.10.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect golang.org/x/exp v0.0.0-20260112195511-716be5621a96 // indirect golang.org/x/mod v0.36.0 // indirect - golang.org/x/net v0.55.0 // indirect + golang.org/x/net v0.56.0 // indirect + golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.21.0 // indirect golang.org/x/sys v0.46.0 // indirect golang.org/x/telemetry v0.0.0-20260508192327-42602be52be6 // indirect golang.org/x/term v0.44.0 // indirect golang.org/x/text v0.38.0 // indirect - golang.org/x/time v0.14.0 // indirect + golang.org/x/time v0.15.0 // indirect golang.org/x/tools v0.45.0 // indirect golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da // indirect - google.golang.org/genproto v0.0.0-20240528184218-531527333157 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 // indirect + google.golang.org/genproto v0.0.0-20260319201613-d00831a3d3e7 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260630182238-925bb5da69e7 // indirect google.golang.org/grpc v1.82.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect gotest.tools/v3 v3.5.1 // indirect diff --git a/keystore/go.sum b/keystore/go.sum index 88afb5b5a1..853db99db5 100644 --- a/keystore/go.sum +++ b/keystore/go.sum @@ -1,5 +1,19 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= +cloud.google.com/go v0.123.0 h1:2NAUJwPR47q+E35uaJeYoNhuNEM9kM8SjgRgdeOJUSE= +cloud.google.com/go v0.123.0/go.mod h1:xBoMV08QcqUGuPW65Qfm1o9Y4zKZBpGS+7bImXLTAZU= +cloud.google.com/go/auth v0.20.0 h1:kXTssoVb4azsVDoUiF8KvxAqrsQcQtB53DcSgta74CA= +cloud.google.com/go/auth v0.20.0/go.mod h1:942/yi/itH1SsmpyrbnTMDgGfdy2BUqIKyd0cyYLc5Q= +cloud.google.com/go/auth/oauth2adapt v0.2.8 h1:keo8NaayQZ6wimpNSmW5OPc283g65QNIiLpZnkHRbnc= +cloud.google.com/go/auth/oauth2adapt v0.2.8/go.mod h1:XQ9y31RkqZCcwJWNSx2Xvric3RrU88hAYYbjDWYDL+c= +cloud.google.com/go/compute/metadata v0.9.0 h1:pDUj4QMoPejqq20dK0Pg2N4yG9zIkYGdBtwLoEkH9Zs= +cloud.google.com/go/compute/metadata v0.9.0/go.mod h1:E0bWwX5wTnLPedCKqk3pJmVgCBSM6qQI1yTBdEb3C10= +cloud.google.com/go/iam v1.11.0 h1:KieQ9Pb+LLPak1O3Rv3GgCxhnmkYf7Xyh0P5HfF1jFM= +cloud.google.com/go/iam v1.11.0/go.mod h1:KP+nKGugNJW4LcLx1uEZcq1ok5sQHFaQehQNl4QDgV4= +cloud.google.com/go/kms v1.33.0 h1:pG0X78m212b2pv9N4fdMoUO69LuZGQ9kSvn8sHBOFAo= +cloud.google.com/go/kms v1.33.0/go.mod h1:CSGvW6GnMQbY+1nOHcIzhMtHSbExXlOmCKjWtYVjcpA= +cloud.google.com/go/longrunning v1.2.0 h1:WjYH3YHBGCxGJP9M4dWGHBfXr/cFIjMkNgWcJj7/iMM= +cloud.google.com/go/longrunning v1.2.0/go.mod h1:5KMQALFGOCtFoi2xSOA1u3H7WKlhmckgiyFw7+LGQp0= cosmossdk.io/api v0.7.6 h1:PC20PcXy1xYKH2KU4RMurVoFjjKkCgYRbVAD4PdqUuY= cosmossdk.io/api v0.7.6/go.mod h1:IcxpYS5fMemZGqyYtErK7OqvdM0C8kdW3dq8Q/XIG38= cosmossdk.io/collections v0.4.0 h1:PFmwj2W8szgpD5nOd8GWH6AbYNi1f2J6akWXJ7P5t9s= @@ -128,6 +142,8 @@ github.com/cloudevents/sdk-go/binding/format/protobuf/v2 v2.16.1/go.mod h1:6Q+F2 github.com/cloudevents/sdk-go/v2 v2.16.1 h1:G91iUdqvl88BZ1GYYr9vScTj5zzXSyEuqbfE63gbu9Q= github.com/cloudevents/sdk-go/v2 v2.16.1/go.mod h1:v/kVOaWjNfbvc6tkhhlkhvLapj8Aa8kvXiH5GiOHCKI= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= +github.com/cncf/xds/go v0.0.0-20260202195803-dba9d589def2 h1:aBangftG7EVZoUb69Os8IaYg++6uMOdKK83QtkkvJik= +github.com/cncf/xds/go v0.0.0-20260202195803-dba9d589def2/go.mod h1:qwXFYgsP6T7XnJtbKlf1HP8AjxZZyzxMmc+Lq5GjlU4= github.com/cockroachdb/datadriven v1.0.3-0.20230413201302-be42291fc80f h1:otljaYPt5hWxV3MUfO5dFPFiOXg9CyG5/kCfayTqsJ4= github.com/cockroachdb/datadriven v1.0.3-0.20230413201302-be42291fc80f/go.mod h1:a9RdTaap04u637JoCzcUoIcDmvwSUtcUFtT/C3kJlTU= github.com/cockroachdb/errors v1.12.0 h1:d7oCs6vuIMUQRVbi6jWWWEJZahLCfJpnJSVobd1/sUo= @@ -209,7 +225,12 @@ github.com/emicklei/dot v1.6.2/go.mod h1:DeV7GvQtIw4h2u73RKBkkFdvVAz0D9fzeJrgPW6 github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= github.com/envoyproxy/go-control-plane v0.9.4/go.mod h1:6rpuAdCZL397s3pYoYcLgu1mIlRU8Am5FuJP05cCM98= +github.com/envoyproxy/go-control-plane v0.14.0 h1:hbG2kr4RuFj222B6+7T83thSPqLjwBIfQawTkC++2HA= +github.com/envoyproxy/go-control-plane/envoy v1.37.0 h1:u3riX6BoYRfF4Dr7dwSOroNfdSbEPe9Yyl09/B6wBrQ= +github.com/envoyproxy/go-control-plane/envoy v1.37.0/go.mod h1:DReE9MMrmecPy+YvQOAOHNYMALuowAnbjjEMkkWOi6A= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= +github.com/envoyproxy/protoc-gen-validate v1.3.3 h1:MVQghNeW+LZcmXe7SY1V36Z+WFMDjpqGAGacLe2T0ds= +github.com/envoyproxy/protoc-gen-validate v1.3.3/go.mod h1:TsndJ/ngyIdQRhMcVVGDDHINPLWB7C82oDArY51KfB0= github.com/ethereum/c-kzg-4844/v2 v2.1.6 h1:xQymkKCT5E2Jiaoqf3v4wsNgjZLY0lRSkZn27fRjSls= github.com/ethereum/c-kzg-4844/v2 v2.1.6/go.mod h1:8HMkUZ5JRv4hpw/XUrYWSQNAUzhHMg2UDb/U+5m+XNw= github.com/ethereum/go-bigmodexpfix v0.0.0-20250911101455-f9e208c548ab h1:rvv6MJhy07IMfEKuARQ9TKojGqLVNxQajaXEp/BoqSk= @@ -350,9 +371,15 @@ github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/ github.com/google/orderedcode v0.0.1 h1:UzfcAexk9Vhv8+9pNOgRu41f16lHq725vPwnSeiG/Us= github.com/google/orderedcode v0.0.1/go.mod h1:iVyU4/qPKHY5h/wSd6rZZCDcLJNxiWO6dvsYES2Sb20= github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= +github.com/google/s2a-go v0.1.9 h1:LGD7gtMgezd8a/Xak7mEWL0PjoTQFvpRudN895yqKW0= +github.com/google/s2a-go v0.1.9/go.mod h1:YA0Ei2ZQL3acow2O62kdp9UlnvMmU7kA6Eutn0dXayM= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/googleapis/enterprise-certificate-proxy v0.3.17 h1:73NfMHdiqo9JFU9+7a5ExpVa10/R29pXfZIaW559nrg= +github.com/googleapis/enterprise-certificate-proxy v0.3.17/go.mod h1:rSEsBUemEBZEexP2y6jPp16LUmUbjmSbcPMQizR0o4k= +github.com/googleapis/gax-go/v2 v2.23.0 h1:Tchl7qkvE7Ip3y+ztvNufYFvkfqTe7NfLTYGIdJRLuE= +github.com/googleapis/gax-go/v2 v2.23.0/go.mod h1:rBQKOVJCdb8IFEzg+FCwlt1LP/xMDGuqUXhUG+XMXEg= github.com/gorilla/handlers v1.5.2 h1:cLTUSsNkgcwhgRqvCNmdbRWG0A3N4F+M2nWKdScwyEE= github.com/gorilla/handlers v1.5.2/go.mod h1:dX+xVpaxdSw+q0Qek8SSsl3dfMk3jNddUkMzo0GtH0w= github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= @@ -572,6 +599,8 @@ github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= +github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= +github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1:t/avpk3KcrXxUnYOhZhMXJlSEyie6gQbtLq5NM3loB8= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -743,11 +772,13 @@ go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0= go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= -go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.63.0 h1:YH4g8lQroajqUwWbq/tr2QX1JFmEXaDLgG+ew9bLMWo= -go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.63.0/go.mod h1:fvPi2qXDqFs8M4B4fmJhE92TyQs9Ydjlg3RvfUp+NbQ= +go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.67.0 h1:yI1/OhfEPy7J9eoa6Sj051C7n5dvpj0QX8g4sRchg04= +go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.67.0/go.mod h1:NoUCKYWK+3ecatC4HjkRktREheMeEtrXoQxrqYFeHSc= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.67.0 h1:OyrsyzuttWTSur2qN/Lm0m2a8yqyIjUVBZcxFPuXq2o= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.67.0/go.mod h1:C2NGBr+kAB4bk3xtMXfZ94gqFDtg/GkI7e9zqGh5Beg= go.opentelemetry.io/otel v1.6.3/go.mod h1:7BgNga5fNlF/iZjG06hM3yofffp0ofKCDwSXx1GC4dI= -go.opentelemetry.io/otel v1.43.0 h1:mYIM03dnh5zfN7HautFE4ieIig9amkNANT+xcVxAj9I= -go.opentelemetry.io/otel v1.43.0/go.mod h1:JuG+u74mvjvcm8vj8pI5XiHy1zDeoCS2LB1spIq7Ay0= +go.opentelemetry.io/otel v1.44.0 h1:JjwHmHpA4iZ3wBxluu2fbbE7j4kqlE8jXyAyPXH7HqU= +go.opentelemetry.io/otel v1.44.0/go.mod h1:BMgjTHL9WPRlRjL2oZCBTL4whCGtXch2H4BhOPIAyYc= go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.12.2 h1:06ZeJRe5BnYXceSM9Vya83XXVaNGe3H1QqsvqRANQq8= go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc v0.12.2/go.mod h1:DvPtKE63knkDVP88qpatBj81JxN+w1bqfVbsbCbj1WY= go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp v0.19.0 h1:HIBTQ3VO5aupLKjC90JgMqpezVXwFuq6Ryjn0/izoag= @@ -770,19 +801,21 @@ go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.36.0 h1:G8Xec/SgZQricwW go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.36.0/go.mod h1:PD57idA/AiFD5aqoxGxCvT/ILJPeHy3MjqU/NS7KogY= go.opentelemetry.io/otel/log v0.19.0 h1:KUZs/GOsw79TBBMfDWsXS+KZ4g2Ckzksd1ymzsIEbo4= go.opentelemetry.io/otel/log v0.19.0/go.mod h1:5DQYeGmxVIr4n0/BcJvF4upsraHjg6vudJJpnkL6Ipk= -go.opentelemetry.io/otel/metric v1.43.0 h1:d7638QeInOnuwOONPp4JAOGfbCEpYb+K6DVWvdxGzgM= -go.opentelemetry.io/otel/metric v1.43.0/go.mod h1:RDnPtIxvqlgO8GRW18W6Z/4P462ldprJtfxHxyKd2PY= -go.opentelemetry.io/otel/sdk v1.43.0 h1:pi5mE86i5rTeLXqoF/hhiBtUNcrAGHLKQdhg4h4V9Dg= -go.opentelemetry.io/otel/sdk v1.43.0/go.mod h1:P+IkVU3iWukmiit/Yf9AWvpyRDlUeBaRg6Y+C58QHzg= +go.opentelemetry.io/otel/metric v1.44.0 h1:1w0gILTcHdr3YI+ixLyjemwrVnsMURbTZFrSYCdDdmc= +go.opentelemetry.io/otel/metric v1.44.0/go.mod h1:8O7hanEPBNgEMmybD3s2VBKcgWOCsA6tzHBPODAiquo= +go.opentelemetry.io/otel/metric/x v0.66.0 h1:YkCrx1zLOChi9ZcZ6euupOcsgzbVlec7D/xoEU1+cTA= +go.opentelemetry.io/otel/metric/x v0.66.0/go.mod h1:d1+BDj9t96do0/1LoU1ayfCv79ZgNE41qbhBvnMOBZk= +go.opentelemetry.io/otel/sdk v1.44.0 h1:nHYwb9lK+fJPU/dnT6s7W7Z8itMWyqrnVfbheVYrZ58= +go.opentelemetry.io/otel/sdk v1.44.0/go.mod h1:Osuydd3Se74nqjAKxid74N5eC+jfEqfTegHRnq58oK0= go.opentelemetry.io/otel/sdk/log v0.19.0 h1:scYVLqT22D2gqXItnWiocLUKGH9yvkkeql5dBDiXyko= go.opentelemetry.io/otel/sdk/log v0.19.0/go.mod h1:vFBowwXGLlW9AvpuF7bMgnNI95LiW10szrOdvzBHlAg= go.opentelemetry.io/otel/sdk/log/logtest v0.19.0 h1:BEbF7ZBB6qQloV/Ub1+3NQoOUnVtcGkU3XX4Ws3GQfk= go.opentelemetry.io/otel/sdk/log/logtest v0.19.0/go.mod h1:Lua81/3yM0wOmoHTokLj9y9ADeA02v1naRrVrkAZuKk= -go.opentelemetry.io/otel/sdk/metric v1.43.0 h1:S88dyqXjJkuBNLeMcVPRFXpRw2fuwdvfCGLEo89fDkw= -go.opentelemetry.io/otel/sdk/metric v1.43.0/go.mod h1:C/RJtwSEJ5hzTiUz5pXF1kILHStzb9zFlIEe85bhj6A= +go.opentelemetry.io/otel/sdk/metric v1.44.0 h1:3LlKgI+VjbVsjNRFZJZAJ30WjXC5VkNRks6si09iEfI= +go.opentelemetry.io/otel/sdk/metric v1.44.0/go.mod h1:5B5pMARnXxKhltooO4xUuCBorl65a4EpnTalObqOigA= go.opentelemetry.io/otel/trace v1.6.3/go.mod h1:GNJQusJlUgZl9/TQBPKU/Y/ty+0iVB5fjhKeJGZPGFs= -go.opentelemetry.io/otel/trace v1.43.0 h1:BkNrHpup+4k4w+ZZ86CZoHHEkohws8AY+WTX09nk+3A= -go.opentelemetry.io/otel/trace v1.43.0/go.mod h1:/QJhyVBUUswCphDVxq+8mld+AvhXZLhe+8WVFxiFff0= +go.opentelemetry.io/otel/trace v1.44.0 h1:jxF5CsGYCe74MCRx2X4g7WsY/VBKRqqpNvXlX/6gtIk= +go.opentelemetry.io/otel/trace v1.44.0/go.mod h1:oLl1jrMQAVo6v3GAggN+1VH9VIz9iUSvW53sW1Q8PIE= go.opentelemetry.io/proto/otlp v1.10.0 h1:IQRWgT5srOCYfiWnpqUYz9CVmbO8bFmKcwYxpuCSL2g= go.opentelemetry.io/proto/otlp v1.10.0/go.mod h1:/CV4QoCR/S9yaPj8utp3lvQPoqMtxXdzn7ozvvozVqk= go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= @@ -841,10 +874,12 @@ golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/net v0.0.0-20220607020251-c690dde0001d/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= -golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= -golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= +golang.org/x/net v0.56.0 h1:Rw8j/hFzGvJUZwNBXnAtf5sVDVt+65SK2C7IxCxZt5o= +golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= +golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= +golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -900,8 +935,8 @@ golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.38.0 h1:sXmwo9DwP3OK9EZ7PqAdaooSGozfl/3a6/xJcbzPRhE= golang.org/x/text v0.38.0/go.mod h1:YXZt3QhHUKYT53r2lLKFIVi6Ao1jdzrTR/KQ09qyxF4= -golang.org/x/time v0.14.0 h1:MRx4UaLrDotUKUdCIqzPC48t1Y9hANFKIRpNx+Te8PI= -golang.org/x/time v0.14.0/go.mod h1:eL/Oa2bBBK0TkX57Fyni+NgnyQQN4LitPmob2Hjnqw4= +golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= +golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY= @@ -923,6 +958,8 @@ golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da h1:noIWHXmPHxILtqtCOPIhS golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da/go.mod h1:NDW/Ps6MPRej6fsCIbMTohpP40sJ/P/vI1MoTEGwX90= gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= +google.golang.org/api v0.287.1 h1:LiyJx32VU3cwQfLchn/513qKhc25hq0pEANYJoWNnnI= +google.golang.org/api v0.287.1/go.mod h1:lM2kYRzYUCBY91P9h6VF1PYmvhxii3O5hji37qRvIcY= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc= @@ -930,12 +967,12 @@ google.golang.org/genproto v0.0.0-20180831171423-11092d34479b/go.mod h1:JiN7NxoA google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98Agz4BDEuKkezgsaosCRResVns1a3J2ZsMNc= google.golang.org/genproto v0.0.0-20200513103714-09dca8ec2884/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c= google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo= -google.golang.org/genproto v0.0.0-20240528184218-531527333157 h1:u7WMYrIrVvs0TF5yaKwKNbcJyySYf+HAIFXxWltJOXE= -google.golang.org/genproto v0.0.0-20240528184218-531527333157/go.mod h1:ubQlAQnzejB8uZzszhrTCU2Fyp6Vi7ZE5nn0c3W8+qQ= -google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 h1:yQugLulqltosq0B/f8l4w9VryjV+N/5gcW0jQ3N8Qec= -google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478/go.mod h1:C6ADNqOxbgdUUeRTU+LCHDPB9ttAMCTff6auwCVa4uc= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 h1:RmoJA1ujG+/lRGNfUnOMfhCy5EipVMyvUE+KNbPbTlw= -google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= +google.golang.org/genproto v0.0.0-20260319201613-d00831a3d3e7 h1:XzmzkmB14QhVhgnawEVsOn6OFsnpyxNPRY9QV01dNB0= +google.golang.org/genproto v0.0.0-20260319201613-d00831a3d3e7/go.mod h1:L43LFes82YgSonw6iTXTxXUX1OlULt4AQtkik4ULL/I= +google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7 h1:jQ9p21COKWjP3VwuFrNRiiOTMh3mPpN45R7SLrH/HUU= +google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7/go.mod h1:KqHwBx2upmfa1XSi1WuRvC+2VGCLtooKkfmyvRbUmqA= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260630182238-925bb5da69e7 h1:eM/YSd5bBFagF51o1E745Ta7RwzpW0h+z+QDNZOgmQ8= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260630182238-925bb5da69e7/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY= diff --git a/keystore/reader.go b/keystore/reader.go index 6f4d69a05e..12ee437cd9 100644 --- a/keystore/reader.go +++ b/keystore/reader.go @@ -24,7 +24,7 @@ type GetKeyResponse struct { // Reader is the interface for reading keys from the keystore. // GetKeys returns all keys in the keystore if no names are provided, or the keys with the given names. -// Keys are sorted by name in lexicographic order. +// The order of returned keys is implementation-specific; callers must not rely on a particular ordering. type Reader interface { GetKeys(ctx context.Context, req GetKeysRequest) (GetKeysResponse, error) } From a11f05c4a823444d2acb165b981b415b782034bb Mon Sep 17 00:00:00 2001 From: Amr Rezk Date: Mon, 24 Aug 2026 17:45:49 +0300 Subject: [PATCH 2/8] Fix lint issues --- keystore/gcpkms/client.go | 8 ++++++-- keystore/gcpkms/keystore.go | 24 ++++++++++++------------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/keystore/gcpkms/client.go b/keystore/gcpkms/client.go index e9aec2cb74..68c73435c8 100644 --- a/keystore/gcpkms/client.go +++ b/keystore/gcpkms/client.go @@ -63,6 +63,10 @@ type ClientOptions struct { func NewClient(ctx context.Context, opts ClientOptions) (ClientWithClose, error) { var clientOpts []option.ClientOption if opts.CredentialsFile != "" { + // WithCredentialsFile is deprecated upstream because long-lived key files are a standing + // credential-leak risk. It is kept for the local-development path only; production leaves + // CredentialsFile empty and authenticates through Application Default Credentials. + //nolint:staticcheck // deliberate: local-development-only service-account key support clientOpts = append(clientOpts, option.WithCredentialsFile(opts.CredentialsFile)) } client, err := apiv1.NewKeyManagementClient(ctx, clientOpts...) @@ -90,12 +94,12 @@ func (c *clientAdapter) AsymmetricSign(ctx context.Context, req *kmspb.Asymmetri func (c *clientAdapter) ListCryptoKeys(ctx context.Context, keyRingName string) ([]*kmspb.CryptoKey, error) { iter := c.client.ListCryptoKeys(ctx, &kmspb.ListCryptoKeysRequest{Parent: keyRingName}) - return drain(iter.Next, fmt.Sprintf("crypto keys in %s", keyRingName)) + return drain(iter.Next, "crypto keys in "+keyRingName) } func (c *clientAdapter) ListCryptoKeyVersions(ctx context.Context, cryptoKeyName string) ([]*kmspb.CryptoKeyVersion, error) { iter := c.client.ListCryptoKeyVersions(ctx, &kmspb.ListCryptoKeyVersionsRequest{Parent: cryptoKeyName}) - return drain(iter.Next, fmt.Sprintf("crypto key versions of %s", cryptoKeyName)) + return drain(iter.Next, "crypto key versions of "+cryptoKeyName) } // drain reads a Cloud KMS iterator to completion. what describes the listed resources and is only diff --git a/keystore/gcpkms/keystore.go b/keystore/gcpkms/keystore.go index 73bd930c0e..eb9d245aed 100644 --- a/keystore/gcpkms/keystore.go +++ b/keystore/gcpkms/keystore.go @@ -72,7 +72,7 @@ func NewKeystore(client Client, opts KeystoreOptions) (interface { keystore.Signer }, error) { if client == nil { - return nil, fmt.Errorf("GCP KMS client is required") + return nil, errors.New("GCP KMS client is required") } return &keystoreSignerReader{ client: client, @@ -119,7 +119,7 @@ func (k *keystoreSignerReader) resolveKeyVersion(ctx context.Context, keyName st return resolvedKey{}, fmt.Errorf("failed to get crypto key version %s: %w", keyName, err) } if got == nil { - return resolvedKey{}, fmt.Errorf("Cloud KMS returned an empty crypto key version response for %s", keyName) + return resolvedKey{}, fmt.Errorf("empty crypto key version response from Cloud KMS for %s", keyName) } if got.Name != keyName { return resolvedKey{}, fmt.Errorf("crypto key version response has name %q, expected %q", got.Name, keyName) @@ -201,12 +201,12 @@ func (k *keystoreSignerReader) getPublicKeyBytes(ctx context.Context, versionNam return nil, fmt.Errorf("failed to get public key for %s: %w", versionName, err) } if pk == nil { - return nil, fmt.Errorf("Cloud KMS returned an empty public key response for %s", versionName) + return nil, fmt.Errorf("empty public key response from Cloud KMS for %s", versionName) } if pk.Name != versionName { return nil, fmt.Errorf("public key response has name %q, expected %q", pk.Name, versionName) } - if err := checkCrc32c([]byte(pk.Pem), pk.PemCrc32C); err != nil { + if err = checkCrc32c([]byte(pk.Pem), pk.PemCrc32C); err != nil { return nil, fmt.Errorf("public key for %s: %w", versionName, err) } @@ -251,7 +251,7 @@ func signingKeyNames(listedKeys []*kmspb.CryptoKey) ([]string, error) { keyNames := make([]string, 0, len(listedKeys)) for _, key := range listedKeys { if key == nil || key.Name == "" { - return nil, fmt.Errorf("Cloud KMS returned a crypto key without a name") + return nil, errors.New("crypto key without a name returned by Cloud KMS") } if key.Purpose != kmspb.CryptoKey_ASYMMETRIC_SIGN { continue @@ -287,7 +287,7 @@ func (k *keystoreSignerReader) GetKeys(ctx context.Context, req keystore.GetKeys listed := len(keyNames) == 0 if listed { if k.keyRingName == "" { - return keystore.GetKeysResponse{}, fmt.Errorf("key ring name is required to list Cloud KMS keys") + return keystore.GetKeysResponse{}, errors.New("key ring name is required to list Cloud KMS keys") } lister, ok := k.client.(KeyRingLister) if !ok { @@ -362,15 +362,15 @@ func (k *keystoreSignerReader) Sign(ctx context.Context, req keystore.SignReques return keystore.SignResponse{}, fmt.Errorf("failed to sign data: %w", err) } if sig == nil { - return keystore.SignResponse{}, fmt.Errorf("Cloud KMS returned an empty signing response") + return keystore.SignResponse{}, errors.New("empty signing response from Cloud KMS") } if sig.Name != versionName { return keystore.SignResponse{}, fmt.Errorf("signing response has name %q, expected %q", sig.Name, versionName) } if !sig.VerifiedDigestCrc32C { - return keystore.SignResponse{}, fmt.Errorf("Cloud KMS did not verify the digest CRC32C checksum") + return keystore.SignResponse{}, errors.New("digest CRC32C checksum was not verified by Cloud KMS") } - if err := checkCrc32c(sig.Signature, sig.SignatureCrc32C); err != nil { + if err = checkCrc32c(sig.Signature, sig.SignatureCrc32C); err != nil { return keystore.SignResponse{}, fmt.Errorf("signature for key %s: %w", req.KeyName, err) } // Cloud KMS returns the ECDSA signature in ASN.1 DER format, identical to AWS. Reuse the @@ -392,15 +392,15 @@ func (k *keystoreSignerReader) Sign(ctx context.Context, req keystore.SignReques return keystore.SignResponse{}, fmt.Errorf("failed to sign data: %w", err) } if sig == nil { - return keystore.SignResponse{}, fmt.Errorf("Cloud KMS returned an empty signing response") + return keystore.SignResponse{}, errors.New("empty signing response from Cloud KMS") } if sig.Name != versionName { return keystore.SignResponse{}, fmt.Errorf("signing response has name %q, expected %q", sig.Name, versionName) } if !sig.VerifiedDataCrc32C { - return keystore.SignResponse{}, fmt.Errorf("Cloud KMS did not verify the data CRC32C checksum") + return keystore.SignResponse{}, errors.New("data CRC32C checksum was not verified by Cloud KMS") } - if err := checkCrc32c(sig.Signature, sig.SignatureCrc32C); err != nil { + if err = checkCrc32c(sig.Signature, sig.SignatureCrc32C); err != nil { return keystore.SignResponse{}, fmt.Errorf("signature for key %s: %w", req.KeyName, err) } // Ed25519 signatures from Cloud KMS are already in the correct format (64 bytes). From c8b9e8119dd400740ea1e5226ac9a1bee0dcc2a5 Mon Sep 17 00:00:00 2001 From: Amr Rezk Date: Tue, 25 Aug 2026 13:03:01 +0300 Subject: [PATCH 3/8] Address review comments --- keystore/gcpkms/client.go | 32 +++++-------- keystore/gcpkms/fake_client.go | 57 +++++++++++++++-------- keystore/gcpkms/keystore.go | 79 +++++++++++++++++++++++--------- keystore/gcpkms/keystore_test.go | 66 ++++++++++++++++++++++++-- keystore/reader.go | 2 +- 5 files changed, 171 insertions(+), 65 deletions(-) diff --git a/keystore/gcpkms/client.go b/keystore/gcpkms/client.go index 68c73435c8..d929be74bb 100644 --- a/keystore/gcpkms/client.go +++ b/keystore/gcpkms/client.go @@ -49,27 +49,19 @@ type ClientWithClose interface { Close() error } -// ClientOptions contains options for creating a Cloud KMS client. -type ClientOptions struct { - // CredentialsFile is the path to a GCP service account JSON key. Local development only — - // leave empty in production, where credentials come from the default credential chain - // (GKE Workload Identity, GCE instance/service accounts, or GOOGLE_APPLICATION_CREDENTIALS). - CredentialsFile string -} - // NewClient constructs a new Google Cloud KMS client using the Go SDK. -// If CredentialsFile is specified, it uses service-account-key-based authentication (local dev). -// Otherwise, it uses Application Default Credentials (Workload Identity in production, etc.). -func NewClient(ctx context.Context, opts ClientOptions) (ClientWithClose, error) { - var clientOpts []option.ClientOption - if opts.CredentialsFile != "" { - // WithCredentialsFile is deprecated upstream because long-lived key files are a standing - // credential-leak risk. It is kept for the local-development path only; production leaves - // CredentialsFile empty and authenticates through Application Default Credentials. - //nolint:staticcheck // deliberate: local-development-only service-account key support - clientOpts = append(clientOpts, option.WithCredentialsFile(opts.CredentialsFile)) - } - client, err := apiv1.NewKeyManagementClient(ctx, clientOpts...) +// +// Credentials always come from Application Default Credentials, which covers both production (GKE +// Workload Identity, GCE/Cloud Run service accounts) and local development (`gcloud auth +// application-default login`, or GOOGLE_APPLICATION_CREDENTIALS pointing at a service-account key +// file). There is deliberately no credentials-file option: ADC already reads that env var, so a +// dedicated field would only duplicate it while hard-coding a long-lived key file into config. +// +// opts is passed through to the SDK for the cases ADC does not cover — a custom endpoint or +// emulator, a quota project, a non-default token source. +// https://cloud.google.com/docs/authentication/application-default-credentials +func NewClient(ctx context.Context, opts ...option.ClientOption) (ClientWithClose, error) { + client, err := apiv1.NewKeyManagementClient(ctx, opts...) if err != nil { return nil, fmt.Errorf("failed to create Google Cloud KMS client: %w", err) } diff --git a/keystore/gcpkms/fake_client.go b/keystore/gcpkms/fake_client.go index af8c55504b..4da7378f8b 100644 --- a/keystore/gcpkms/fake_client.go +++ b/keystore/gcpkms/fake_client.go @@ -54,24 +54,8 @@ type FakeGCPKMSClient struct { func NewFakeGCPKMSClient(keys []Key) (*FakeGCPKMSClient, error) { keys = append([]Key(nil), keys...) for i := range keys { - if keys[i].KeyID == "" { - return nil, errors.New("key ID is required") - } - if keys[i].VersionNumber == 0 { - keys[i].VersionNumber = 1 - } - if keys[i].State == kmspb.CryptoKeyVersion_CRYPTO_KEY_VERSION_STATE_UNSPECIFIED { - keys[i].State = kmspb.CryptoKeyVersion_ENABLED - } - if keys[i].Purpose == kmspb.CryptoKey_CRYPTO_KEY_PURPOSE_UNSPECIFIED { - keys[i].Purpose = kmspb.CryptoKey_ASYMMETRIC_SIGN - } - if keys[i].Algorithm == kmspb.CryptoKeyVersion_CRYPTO_KEY_VERSION_ALGORITHM_UNSPECIFIED { - algorithm, err := keyTypeToAlgorithm(keys[i].KeyType) - if err != nil { - return nil, err - } - keys[i].Algorithm = algorithm + if err := normalizeKey(&keys[i]); err != nil { + return nil, err } } return &FakeGCPKMSClient{ @@ -80,6 +64,43 @@ func NewFakeGCPKMSClient(keys []Key) (*FakeGCPKMSClient, error) { }, nil } +// AddVersion appends a CryptoKeyVersion after construction, emulating a rotation that lands while +// the keystore is live. Not safe for concurrent use with the client's read methods. +func (m *FakeGCPKMSClient) AddVersion(key Key) error { + if err := normalizeKey(&key); err != nil { + return err + } + if _, err := m.findVersion(key.versionName()); err == nil { + return fmt.Errorf("version %s already exists", key.versionName()) + } + m.keys = append(m.keys, key) + return nil +} + +// normalizeKey validates a Key and fills in the fields a test left at their zero value. +func normalizeKey(key *Key) error { + if key.KeyID == "" { + return errors.New("key ID is required") + } + if key.VersionNumber == 0 { + key.VersionNumber = 1 + } + if key.State == kmspb.CryptoKeyVersion_CRYPTO_KEY_VERSION_STATE_UNSPECIFIED { + key.State = kmspb.CryptoKeyVersion_ENABLED + } + if key.Purpose == kmspb.CryptoKey_CRYPTO_KEY_PURPOSE_UNSPECIFIED { + key.Purpose = kmspb.CryptoKey_ASYMMETRIC_SIGN + } + if key.Algorithm == kmspb.CryptoKeyVersion_CRYPTO_KEY_VERSION_ALGORITHM_UNSPECIFIED { + algorithm, err := keyTypeToAlgorithm(key.KeyType) + if err != nil { + return err + } + key.Algorithm = algorithm + } + return nil +} + func keyTypeToAlgorithm(keyType keystore.KeyType) (kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm, error) { switch keyType { case keystore.ECDSA_S256: diff --git a/keystore/gcpkms/keystore.go b/keystore/gcpkms/keystore.go index eb9d245aed..9db6ced059 100644 --- a/keystore/gcpkms/keystore.go +++ b/keystore/gcpkms/keystore.go @@ -38,8 +38,13 @@ func crc32c(data []byte) int64 { } // checkCrc32c verifies that a received CRC32C checksum matches the computed value of the data. -// This is Google's recommended integrity check for responses returned by Cloud KMS. A missing -// checksum is treated as a failure: the responses we check it on always carry one, so its absence +// +// Cloud KMS documents this as the required client-side step for detecting corruption in transit: +// "you should verify the integrity of the response" by recomputing the CRC32C over the returned +// bytes and comparing it against the response's *_crc32c field. +// https://cloud.google.com/kms/docs/data-integrity-guidelines +// +// A missing checksum is treated as a failure: every response we check carries one, so its absence // means the response was truncated or tampered with in transit. func checkCrc32c(data []byte, received *wrapperspb.Int64Value) error { if received == nil { @@ -55,10 +60,13 @@ type keystoreSignerReader struct { client Client keyRingName string + mu sync.RWMutex + // versions pins each key name to the CryptoKeyVersion it first resolved to, so that the public + // key reported by GetKeys and the version used by Sign can never diverge. See resolveKeyVersion. + versions map[string]resolvedKey // publicKeys caches public keys by CryptoKeyVersion resource name. A version's public key is // immutable, so this is safe to cache indefinitely and saves a round trip per signature. - publicKeysMu sync.RWMutex - publicKeys map[string][]byte + publicKeys map[string][]byte } type KeystoreOptions struct { @@ -77,6 +85,7 @@ func NewKeystore(client Client, opts KeystoreOptions) (interface { return &keystoreSignerReader{ client: client, keyRingName: opts.KeyRingName, + versions: make(map[string]resolvedKey), publicKeys: make(map[string][]byte), }, nil } @@ -107,11 +116,29 @@ type resolvedKey struct { // Cloud KMS only populates CryptoKey.Primary for ENCRYPT_DECRYPT keys — asymmetric signing keys // never have a primary version — so a concrete version has to be selected here. keyName may be // either: -// - a CryptoKeyVersion resource name (.../cryptoKeys//cryptoKeyVersions/), which pins that -// exact version, or -// - a CryptoKey resource name, in which case the highest-numbered enabled version is used, so -// that rotations are picked up without a redeploy. +// - a CryptoKeyVersion resource name (.../cryptoKeys//cryptoKeyVersions/), which names one +// version outright, or +// - a CryptoKey resource name, in which case the highest-numbered enabled version is selected. +// +// The result is pinned for the lifetime of the keystore: the first resolution of a given key name +// wins, and every later GetKeys and Sign for that name reuses it. Re-resolving per call would let a +// rotation land between the two, so a caller could hold the public key of version N while its +// signatures came from version N+1 — a silent verification failure with nothing in SignResponse to +// identify which version signed. Pinning makes the public key reported by GetKeys authoritative for +// every signature this keystore will produce. +// +// The cost is that a rotation is picked up on restart rather than immediately. That is the safer +// default for signing keys, whose public keys are typically registered with peers or on-chain and +// cannot change under a running system. Deployments that want rotation on an explicit schedule +// should configure CryptoKeyVersion names directly. func (k *keystoreSignerReader) resolveKeyVersion(ctx context.Context, keyName string) (resolvedKey, error) { + k.mu.RLock() + pinned, ok := k.versions[keyName] + k.mu.RUnlock() + if ok { + return pinned, nil + } + var version *kmspb.CryptoKeyVersion if strings.Contains(keyName, cryptoKeyVersionsSegment) { got, err := k.client.GetCryptoKeyVersion(ctx, &kmspb.GetCryptoKeyVersionRequest{Name: keyName}) @@ -143,7 +170,16 @@ func (k *keystoreSignerReader) resolveKeyVersion(ctx context.Context, keyName st if err != nil { return resolvedKey{}, fmt.Errorf("crypto key %s: %w", keyName, err) } - return resolvedKey{version: version, keyType: keyType}, nil + + resolved := resolvedKey{version: version, keyType: keyType} + k.mu.Lock() + defer k.mu.Unlock() + // First writer wins, so concurrent resolutions racing a rotation still converge on one version. + if existing, ok := k.versions[keyName]; ok { + return existing, nil + } + k.versions[keyName] = resolved + return resolved, nil } // latestEnabledVersion returns the highest-numbered enabled version of a CryptoKey. @@ -189,9 +225,9 @@ func cryptoKeyVersionNumber(versionName string) (uint64, error) { // getPublicKeyBytes fetches the public key for a crypto key version and converts it to the // keystore's native format for the given key type. Results are cached per version name. func (k *keystoreSignerReader) getPublicKeyBytes(ctx context.Context, versionName string, keyType keystore.KeyType) ([]byte, error) { - k.publicKeysMu.RLock() + k.mu.RLock() cached, ok := k.publicKeys[versionName] - k.publicKeysMu.RUnlock() + k.mu.RUnlock() if ok { return cached, nil } @@ -238,9 +274,9 @@ func (k *keystoreSignerReader) getPublicKeyBytes(ctx context.Context, versionNam return nil, fmt.Errorf("unsupported key type: %s", keyType) } - k.publicKeysMu.Lock() + k.mu.Lock() k.publicKeys[versionName] = publicKeyBytes - k.publicKeysMu.Unlock() + k.mu.Unlock() return publicKeyBytes, nil } @@ -270,9 +306,9 @@ func signingKeyNames(listedKeys []*kmspb.CryptoKey) ([]string, error) { // // Key names are either CryptoKey resource names // (projects/

/locations//keyRings//cryptoKeys/), for which the highest-numbered enabled -// version is resolved so that rotations are picked up without redeploying, or CryptoKeyVersion -// resource names, which pin one version. The returned key names match the requested ones, in the -// requested order. +// version is resolved and pinned, or CryptoKeyVersion resource names, which name one version +// outright. Keys are returned sorted by name, per [keystore.Reader]; correlate them by +// KeyInfo.Name rather than by position. // // When no key names are given, the configured key ring is listed and keys that this keystore // cannot use — another purpose, an unsupported algorithm, or no enabled version — are skipped, @@ -301,9 +337,10 @@ func (k *keystoreSignerReader) GetKeys(ctx context.Context, req keystore.GetKeys if err != nil { return keystore.GetKeysResponse{}, err } - // Cloud KMS does not guarantee a listing order; sort for a stable response. - sort.Strings(keyNames) } + // keystore.Reader specifies keys sorted by name. Sorting the names up front also gives the + // listing path a stable order, which Cloud KMS does not otherwise guarantee. + sort.Strings(keyNames) keys := make([]keystore.GetKeyResponse, 0, len(keyNames)) seen := make(map[string]struct{}, len(keyNames)) @@ -403,9 +440,9 @@ func (k *keystoreSignerReader) Sign(ctx context.Context, req keystore.SignReques if err = checkCrc32c(sig.Signature, sig.SignatureCrc32C); err != nil { return keystore.SignResponse{}, fmt.Errorf("signature for key %s: %w", req.KeyName, err) } - // Ed25519 signatures from Cloud KMS are already in the correct format (64 bytes). - if len(sig.Signature) != 64 { - return keystore.SignResponse{}, fmt.Errorf("invalid Ed25519 signature length: expected 64 bytes, got %d", len(sig.Signature)) + // Ed25519 signatures from Cloud KMS are already in the correct format. + if len(sig.Signature) != ed25519.SignatureSize { + return keystore.SignResponse{}, fmt.Errorf("invalid Ed25519 signature length: expected %d bytes, got %d", ed25519.SignatureSize, len(sig.Signature)) } return keystore.SignResponse{Signature: sig.Signature}, nil default: diff --git a/keystore/gcpkms/keystore_test.go b/keystore/gcpkms/keystore_test.go index 610dc426bd..9c9bc7969b 100644 --- a/keystore/gcpkms/keystore_test.go +++ b/keystore/gcpkms/keystore_test.go @@ -49,16 +49,17 @@ func TestGCPKMSKeystore(t *testing.T) { require.Equal(t, keyName, resp.Keys[0].KeyInfo.Name) require.Equal(t, keyName2, resp.Keys[1].KeyInfo.Name) }) - t.Run("specific keys preserve the requested order", func(t *testing.T) { + t.Run("specific keys are sorted by name", func(t *testing.T) { + // keystore.Reader specifies lexicographic order regardless of request order. resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{ KeyNames: []string{keyName2, keyName}, }) require.NoError(t, err) require.Len(t, resp.Keys, 2) - require.Equal(t, keyName2, resp.Keys[0].KeyInfo.Name) - require.Equal(t, keyName, resp.Keys[1].KeyInfo.Name) - require.Equal(t, keystore.ECDSA_S256, resp.Keys[1].KeyInfo.KeyType) - require.Equal(t, crypto.FromECDSAPub(&key.PublicKey), resp.Keys[1].KeyInfo.PublicKey) + require.Equal(t, keyName, resp.Keys[0].KeyInfo.Name) + require.Equal(t, keyName2, resp.Keys[1].KeyInfo.Name) + require.Equal(t, keystore.ECDSA_S256, resp.Keys[0].KeyInfo.KeyType) + require.Equal(t, crypto.FromECDSAPub(&key.PublicKey), resp.Keys[0].KeyInfo.PublicKey) }) t.Run("explicit key version", func(t *testing.T) { resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{ @@ -338,3 +339,58 @@ func TestGCPKMSKeystore_ClientWithoutKeyRingLister(t *testing.T) { require.ErrorContains(t, err, "does not implement KeyRingLister") }) } + +// A rotation landing between GetKeys and Sign must not change which version signs: the public key a +// caller already holds has to stay the one that verifies its signatures. +func TestGCPKMSKeystore_PinsVersionAcrossRotation(t *testing.T) { + originalKey, err := crypto.GenerateKey() + require.NoError(t, err) + rotatedKey, err := crypto.GenerateKey() + require.NoError(t, err) + + fakeClient, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{ + {KeyType: keystore.ECDSA_S256, KeyID: keyName, VersionNumber: 1, PrivateKey: internal.NewRaw(crypto.FromECDSA(originalKey))}, + }) + require.NoError(t, err) + ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + require.NoError(t, err) + ctx := t.Context() + + resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName}}) + require.NoError(t, err) + require.Len(t, resp.Keys, 1) + publicKey := resp.Keys[0].KeyInfo.PublicKey + require.Equal(t, crypto.FromECDSAPub(&originalKey.PublicKey), publicKey) + + // Cloud KMS gains a newer enabled version after the caller has read the public key. + require.NoError(t, fakeClient.AddVersion(gcpkms.Key{ + KeyType: keystore.ECDSA_S256, + KeyID: keyName, + VersionNumber: 2, + PrivateKey: internal.NewRaw(crypto.FromECDSA(rotatedKey)), + })) + + signResp, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyName, Data: make([]byte, 32)}) + require.NoError(t, err) + + verifyResp, err := ks.Verify(ctx, keystore.VerifyRequest{ + KeyType: keystore.ECDSA_S256, + PublicKey: publicKey, + Data: make([]byte, 32), + Signature: signResp.Signature, + }) + require.NoError(t, err) + require.True(t, verifyResp.Valid, "signature must verify against the public key GetKeys reported") + + // GetKeys keeps reporting the pinned version too, so the two never diverge. + resp, err = ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName}}) + require.NoError(t, err) + require.Equal(t, publicKey, resp.Keys[0].KeyInfo.PublicKey) + + // A keystore started after the rotation picks up the newer version. + fresh, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + require.NoError(t, err) + resp, err = fresh.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName}}) + require.NoError(t, err) + require.Equal(t, crypto.FromECDSAPub(&rotatedKey.PublicKey), resp.Keys[0].KeyInfo.PublicKey) +} diff --git a/keystore/reader.go b/keystore/reader.go index 12ee437cd9..6f4d69a05e 100644 --- a/keystore/reader.go +++ b/keystore/reader.go @@ -24,7 +24,7 @@ type GetKeyResponse struct { // Reader is the interface for reading keys from the keystore. // GetKeys returns all keys in the keystore if no names are provided, or the keys with the given names. -// The order of returned keys is implementation-specific; callers must not rely on a particular ordering. +// Keys are sorted by name in lexicographic order. type Reader interface { GetKeys(ctx context.Context, req GetKeysRequest) (GetKeysResponse, error) } From ab7b0c6299fd75b386ab9bae87aa07d2dda41279 Mon Sep 17 00:00:00 2001 From: Amr Rezk Date: Wed, 26 Aug 2026 11:59:34 +0300 Subject: [PATCH 4/8] Address comments review --- keystore/gcpkms/client.go | 41 ++++++++++++++++---------------- keystore/gcpkms/fake_client.go | 18 +++++++++----- keystore/gcpkms/keystore.go | 34 +++++++++++++++----------- keystore/gcpkms/keystore_test.go | 15 +++++++++++- 4 files changed, 66 insertions(+), 42 deletions(-) diff --git a/keystore/gcpkms/client.go b/keystore/gcpkms/client.go index d929be74bb..9fafddf0c6 100644 --- a/keystore/gcpkms/client.go +++ b/keystore/gcpkms/client.go @@ -40,56 +40,55 @@ type KeyRingLister interface { ListCryptoKeys(ctx context.Context, keyRingName string) ([]*kmspb.CryptoKey, error) } -// ClientWithClose is the client returned by NewClient: the full Cloud KMS surface, including key -// ring listing, plus the underlying transport lifecycle. Whether listing actually succeeds is a -// matter of the credentials' IAM bindings, not of the Go type. -type ClientWithClose interface { - Client - KeyRingLister - Close() error -} - // NewClient constructs a new Google Cloud KMS client using the Go SDK. // // Credentials always come from Application Default Credentials, which covers both production (GKE // Workload Identity, GCE/Cloud Run service accounts) and local development (`gcloud auth -// application-default login`, or GOOGLE_APPLICATION_CREDENTIALS pointing at a service-account key -// file). There is deliberately no credentials-file option: ADC already reads that env var, so a -// dedicated field would only duplicate it while hard-coding a long-lived key file into config. +// application-default login`, or GOOGLE_APPLICATION_CREDENTIALS pointing at a service-account key file). // // opts is passed through to the SDK for the cases ADC does not cover — a custom endpoint or // emulator, a quota project, a non-default token source. // https://cloud.google.com/docs/authentication/application-default-credentials -func NewClient(ctx context.Context, opts ...option.ClientOption) (ClientWithClose, error) { +func NewClient(ctx context.Context, opts ...option.ClientOption) (*SDKClient, error) { client, err := apiv1.NewKeyManagementClient(ctx, opts...) if err != nil { return nil, fmt.Errorf("failed to create Google Cloud KMS client: %w", err) } - return &clientAdapter{client: client}, nil + return &SDKClient{client: client}, nil } -type clientAdapter struct { +// SDKClient adapts the generated Cloud KMS client to this package's interfaces. It satisfies both +// [Client] and [KeyRingLister], and owns the underlying transport, so callers must Close it. +// +// Satisfying KeyRingLister says only that the method exists; whether a listing call succeeds depends +// on the credentials' IAM bindings, not on the Go type. +type SDKClient struct { client *apiv1.KeyManagementClient } -func (c *clientAdapter) GetCryptoKeyVersion(ctx context.Context, req *kmspb.GetCryptoKeyVersionRequest, opts ...gax.CallOption) (*kmspb.CryptoKeyVersion, error) { +var ( + _ Client = (*SDKClient)(nil) + _ KeyRingLister = (*SDKClient)(nil) +) + +func (c *SDKClient) GetCryptoKeyVersion(ctx context.Context, req *kmspb.GetCryptoKeyVersionRequest, opts ...gax.CallOption) (*kmspb.CryptoKeyVersion, error) { return c.client.GetCryptoKeyVersion(ctx, req, opts...) } -func (c *clientAdapter) GetPublicKey(ctx context.Context, req *kmspb.GetPublicKeyRequest, opts ...gax.CallOption) (*kmspb.PublicKey, error) { +func (c *SDKClient) GetPublicKey(ctx context.Context, req *kmspb.GetPublicKeyRequest, opts ...gax.CallOption) (*kmspb.PublicKey, error) { return c.client.GetPublicKey(ctx, req, opts...) } -func (c *clientAdapter) AsymmetricSign(ctx context.Context, req *kmspb.AsymmetricSignRequest, opts ...gax.CallOption) (*kmspb.AsymmetricSignResponse, error) { +func (c *SDKClient) AsymmetricSign(ctx context.Context, req *kmspb.AsymmetricSignRequest, opts ...gax.CallOption) (*kmspb.AsymmetricSignResponse, error) { return c.client.AsymmetricSign(ctx, req, opts...) } -func (c *clientAdapter) ListCryptoKeys(ctx context.Context, keyRingName string) ([]*kmspb.CryptoKey, error) { +func (c *SDKClient) ListCryptoKeys(ctx context.Context, keyRingName string) ([]*kmspb.CryptoKey, error) { iter := c.client.ListCryptoKeys(ctx, &kmspb.ListCryptoKeysRequest{Parent: keyRingName}) return drain(iter.Next, "crypto keys in "+keyRingName) } -func (c *clientAdapter) ListCryptoKeyVersions(ctx context.Context, cryptoKeyName string) ([]*kmspb.CryptoKeyVersion, error) { +func (c *SDKClient) ListCryptoKeyVersions(ctx context.Context, cryptoKeyName string) ([]*kmspb.CryptoKeyVersion, error) { iter := c.client.ListCryptoKeyVersions(ctx, &kmspb.ListCryptoKeyVersionsRequest{Parent: cryptoKeyName}) return drain(iter.Next, "crypto key versions of "+cryptoKeyName) } @@ -110,6 +109,6 @@ func drain[T any](next func() (T, error), what string) ([]T, error) { } } -func (c *clientAdapter) Close() error { +func (c *SDKClient) Close() error { return c.client.Close() } diff --git a/keystore/gcpkms/fake_client.go b/keystore/gcpkms/fake_client.go index 4da7378f8b..e55fbf5d92 100644 --- a/keystore/gcpkms/fake_client.go +++ b/keystore/gcpkms/fake_client.go @@ -40,6 +40,9 @@ type Key struct { // Algorithm is the algorithm reported for this version and for the parent CryptoKey's version // template. Defaults to the algorithm matching KeyType; set it to emulate an unsupported one. Algorithm kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm + // OmitVersionTemplate drops VersionTemplate from the parent CryptoKey in ListCryptoKeys. The + // field is optional in the API, so a listing can arrive without it. + OmitVersionTemplate bool } // FakeGCPKMSClient is an in-memory implementation of Client for tests. It emulates the parts of @@ -181,12 +184,15 @@ func (m *FakeGCPKMSClient) ListCryptoKeys(ctx context.Context, keyRingName strin } seen[key.KeyID] = struct{}{} // Note: no Primary — Cloud KMS only sets it for ENCRYPT_DECRYPT keys. - keys = append(keys, &kmspb.CryptoKey{ - Name: key.KeyID, - Purpose: key.Purpose, - CreateTime: timestamppb.New(m.createdAt), - VersionTemplate: &kmspb.CryptoKeyVersionTemplate{Algorithm: key.Algorithm}, - }) + cryptoKey := &kmspb.CryptoKey{ + Name: key.KeyID, + Purpose: key.Purpose, + CreateTime: timestamppb.New(m.createdAt), + } + if !key.OmitVersionTemplate { + cryptoKey.VersionTemplate = &kmspb.CryptoKeyVersionTemplate{Algorithm: key.Algorithm} + } + keys = append(keys, cryptoKey) } return keys, nil } diff --git a/keystore/gcpkms/keystore.go b/keystore/gcpkms/keystore.go index 9db6ced059..060524f36b 100644 --- a/keystore/gcpkms/keystore.go +++ b/keystore/gcpkms/keystore.go @@ -24,9 +24,13 @@ import ( // CryptoKeyVersion resource name. const cryptoKeyVersionsSegment = "/cryptoKeyVersions/" -// errNoEnabledVersion is returned when a CryptoKey has no enabled version to sign with. It is -// matchable so that key ring listings can skip such keys instead of failing outright. -var errNoEnabledVersion = errors.New("has no enabled version") +// errNoEnabledVersion and errUnsupportedAlgorithm mark the two ways a CryptoKey can turn out to be +// unusable by this keystore. Both are matchable so that a key ring listing can skip such keys +// instead of failing outright, while an explicitly requested key still surfaces the error. +var ( + errNoEnabledVersion = errors.New("has no enabled version") + errUnsupportedAlgorithm = errors.New("unsupported Cloud KMS key algorithm") +) // castagnoliTable is the CRC32C (Castagnoli) table used by Google Cloud KMS for integrity checks. var castagnoliTable = crc32.MakeTable(crc32.Castagnoli) @@ -101,7 +105,7 @@ func cryptoKeyVersionAlgorithmToKeyType(algo kmspb.CryptoKeyVersion_CryptoKeyVer case kmspb.CryptoKeyVersion_EC_SIGN_ED25519: return keystore.Ed25519, nil default: - return "", fmt.Errorf("unsupported Cloud KMS key algorithm: %s (supported: EC_SIGN_SECP256K1_SHA256, EC_SIGN_ED25519)", algo) + return "", fmt.Errorf("%w: %s (supported: EC_SIGN_SECP256K1_SHA256, EC_SIGN_ED25519)", errUnsupportedAlgorithm, algo) } } @@ -281,13 +285,18 @@ func (k *keystoreSignerReader) getPublicKeyBytes(ctx context.Context, versionNam } // signingKeyNames filters a key ring listing down to the asymmetric signing keys this keystore can -// use. Key rings are commonly shared, so keys with another purpose or an unsupported algorithm are -// skipped rather than failing the whole listing. -func signingKeyNames(listedKeys []*kmspb.CryptoKey) ([]string, error) { +// use. Key rings are commonly shared, so anything unusable is skipped rather than failing the whole +// listing. +// +// The version-template check is an optimisation: it rejects keys whose configured algorithm is +// unsupported without spending an RPC on them. VersionTemplate is optional in the API, so a key that +// survives this filter is not necessarily usable; resolveKeyVersion checks the algorithm of the +// version it actually resolves, and GetKeys skips anything that fails there for the same reasons. +func signingKeyNames(listedKeys []*kmspb.CryptoKey) []string { keyNames := make([]string, 0, len(listedKeys)) for _, key := range listedKeys { if key == nil || key.Name == "" { - return nil, errors.New("crypto key without a name returned by Cloud KMS") + continue } if key.Purpose != kmspb.CryptoKey_ASYMMETRIC_SIGN { continue @@ -299,7 +308,7 @@ func signingKeyNames(listedKeys []*kmspb.CryptoKey) ([]string, error) { } keyNames = append(keyNames, key.Name) } - return keyNames, nil + return keyNames } // GetKeys lists keys in the Cloud KMS keystore. @@ -333,10 +342,7 @@ func (k *keystoreSignerReader) GetKeys(ctx context.Context, req keystore.GetKeys if err != nil { return keystore.GetKeysResponse{}, err } - keyNames, err = signingKeyNames(listedKeys) - if err != nil { - return keystore.GetKeysResponse{}, err - } + keyNames = signingKeyNames(listedKeys) } // keystore.Reader specifies keys sorted by name. Sorting the names up front also gives the // listing path a stable order, which Cloud KMS does not otherwise guarantee. @@ -351,7 +357,7 @@ func (k *keystoreSignerReader) GetKeys(ctx context.Context, req keystore.GetKeys seen[keyName] = struct{}{} resolved, err := k.resolveKeyVersion(ctx, keyName) if err != nil { - if listed && errors.Is(err, errNoEnabledVersion) { + if listed && (errors.Is(err, errNoEnabledVersion) || errors.Is(err, errUnsupportedAlgorithm)) { continue } return keystore.GetKeysResponse{}, err diff --git a/keystore/gcpkms/keystore_test.go b/keystore/gcpkms/keystore_test.go index 9c9bc7969b..79ca8dcaff 100644 --- a/keystore/gcpkms/keystore_test.go +++ b/keystore/gcpkms/keystore_test.go @@ -252,7 +252,16 @@ func TestGCPKMSKeystore_ListSkipsUnusableKeys(t *testing.T) { KeyType: keystore.ECDSA_S256, KeyID: keyRingName + "/cryptoKeys/disabled-key", State: kmspb.CryptoKeyVersion_DISABLED, - PrivateKey: internal.NewRaw(crypto.FromECDSA(disabledKey)), + PrivateKey: internal.NewRaw(crypto.FromECDSA(unsupportedKey)), + }, + { + // An unsupported key that reports no VersionTemplate, so the cheap pre-filter cannot + // see its algorithm and the skip has to happen after the version is resolved. + KeyType: keystore.ECDSA_S256, + KeyID: keyRingName + "/cryptoKeys/p256-key-no-template", + Algorithm: kmspb.CryptoKeyVersion_EC_SIGN_P256_SHA256, + OmitVersionTemplate: true, + PrivateKey: internal.NewRaw(crypto.FromECDSA(disabledKey)), }, }) require.NoError(t, err) @@ -274,6 +283,10 @@ func TestGCPKMSKeystore_ListSkipsUnusableKeys(t *testing.T) { KeyNames: []string{keyRingName + "/cryptoKeys/disabled-key"}, }) require.ErrorContains(t, err, "has no enabled version") + _, err = ks.GetKeys(ctx, keystore.GetKeysRequest{ + KeyNames: []string{keyRingName + "/cryptoKeys/p256-key-no-template"}, + }) + require.ErrorContains(t, err, "unsupported Cloud KMS key algorithm") } func TestGCPKMSKeystore_InvalidEd25519Key(t *testing.T) { From 180eeb7f171c47832ff40c19acc3257e00f5960e Mon Sep 17 00:00:00 2001 From: Amr Rezk Date: Thu, 3 Sep 2026 11:30:00 +0300 Subject: [PATCH 5/8] bump dependecies --- keystore/go.mod | 8 ++++---- keystore/go.sum | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/keystore/go.mod b/keystore/go.mod index 6d0622ec9e..eeea0202be 100644 --- a/keystore/go.mod +++ b/keystore/go.mod @@ -244,16 +244,16 @@ require ( go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect golang.org/x/exp v0.0.0-20260112195511-716be5621a96 // indirect - golang.org/x/mod v0.36.0 // indirect + golang.org/x/mod v0.37.0 // indirect golang.org/x/net v0.56.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.21.0 // indirect golang.org/x/sys v0.46.0 // indirect - golang.org/x/telemetry v0.0.0-20260508192327-42602be52be6 // indirect + golang.org/x/telemetry v0.0.0-20260625142307-59b4966ccb57 // indirect golang.org/x/term v0.44.0 // indirect - golang.org/x/text v0.38.0 // indirect + golang.org/x/text v0.39.0 // indirect golang.org/x/time v0.15.0 // indirect - golang.org/x/tools v0.45.0 // indirect + golang.org/x/tools v0.47.0 // indirect golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da // indirect google.golang.org/genproto v0.0.0-20260319201613-d00831a3d3e7 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20260630182238-925bb5da69e7 // indirect diff --git a/keystore/go.sum b/keystore/go.sum index 853db99db5..7e6adeeb9b 100644 --- a/keystore/go.sum +++ b/keystore/go.sum @@ -852,8 +852,8 @@ golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHl golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.36.0 h1:JJjpVx6myfUsUdAzZuOSTTmRE0PfZeNWzzvKrP7amb4= -golang.org/x/mod v0.36.0/go.mod h1:moc6ELqsWcOw5Ef3xVprK5ul/MvtVvkIXLziUOICjUQ= +golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ= +golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -923,8 +923,8 @@ golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/telemetry v0.0.0-20260508192327-42602be52be6 h1:HjU6IWBiAgRIdAJ9/y1rwCn+UELEmwV+VsTLzj/W4sE= -golang.org/x/telemetry v0.0.0-20260508192327-42602be52be6/go.mod h1:Eqhaxk/wZsWEH8CRxLwj6xzEJbz7k1EFGqx7nyCoabE= +golang.org/x/telemetry v0.0.0-20260625142307-59b4966ccb57 h1:nwGZBCt+FnXUrGsj5vjzAsEmkcaFvd82BbOjECiFYZc= +golang.org/x/telemetry v0.0.0-20260625142307-59b4966ccb57/go.mod h1:3AWMyWHS+caVoiEXpiq6+tzKA40J4vQT3MYr80ZtQpc= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.44.0 h1:0rLvDRCtNj0gZkyIXhCyOb2OAzEhLVqc4B+hrsBhrmc= @@ -933,8 +933,8 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/text v0.38.0 h1:sXmwo9DwP3OK9EZ7PqAdaooSGozfl/3a6/xJcbzPRhE= -golang.org/x/text v0.38.0/go.mod h1:YXZt3QhHUKYT53r2lLKFIVi6Ao1jdzrTR/KQ09qyxF4= +golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus= +golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -947,8 +947,8 @@ golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roY golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= -golang.org/x/tools v0.45.0 h1:18qN3FAooORvApf5XjCXgsuayZOEtXf6JK18I3+ONa8= -golang.org/x/tools v0.45.0/go.mod h1:LuUGqqaXcXMEFEruIVJVm5mgDD8vww/z/SR1gQ4uE/0= +golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q= +golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= From d6a2c96adc058fe1aca2b6636c86093c994c6774 Mon Sep 17 00:00:00 2001 From: Amr Rezk Date: Thu, 3 Sep 2026 13:02:01 +0300 Subject: [PATCH 6/8] Restrict to fully qualified key names --- keystore/gcpkms/client.go | 60 +----- keystore/gcpkms/fake_client.go | 60 +----- keystore/gcpkms/keystore.go | 311 ++++++++----------------------- keystore/gcpkms/keystore_test.go | 258 ++++++------------------- 4 files changed, 147 insertions(+), 542 deletions(-) diff --git a/keystore/gcpkms/client.go b/keystore/gcpkms/client.go index 9fafddf0c6..a94b4c8899 100644 --- a/keystore/gcpkms/client.go +++ b/keystore/gcpkms/client.go @@ -2,22 +2,19 @@ package gcpkms import ( "context" - "errors" "fmt" apiv1 "cloud.google.com/go/kms/apiv1" "cloud.google.com/go/kms/apiv1/kmspb" "github.com/googleapis/gax-go/v2" - "google.golang.org/api/iterator" "google.golang.org/api/option" ) // Client is an interface that defines the operations needed by the keystore. It keeps the keystore // independent of the generated Google Cloud KMS client. // -// Every method here can be authorized per CryptoKey, so a deployment can bind exactly the keys it -// configures and nothing else. Listing a whole key ring is deliberately not part of this interface — -// see [KeyRingLister]. +// Every method operates on a single CryptoKeyVersion, so each can be authorized with per-key IAM +// bindings; a deployment binds exactly the keys it configures and nothing else. // // These methods are based on the Google Cloud KMS Go client interface. // https://pkg.go.dev/cloud.google.com/go/kms/apiv1 @@ -25,19 +22,6 @@ type Client interface { GetCryptoKeyVersion(ctx context.Context, req *kmspb.GetCryptoKeyVersionRequest, opts ...gax.CallOption) (*kmspb.CryptoKeyVersion, error) GetPublicKey(ctx context.Context, req *kmspb.GetPublicKeyRequest, opts ...gax.CallOption) (*kmspb.PublicKey, error) AsymmetricSign(ctx context.Context, req *kmspb.AsymmetricSignRequest, opts ...gax.CallOption) (*kmspb.AsymmetricSignResponse, error) - ListCryptoKeyVersions(ctx context.Context, cryptoKeyName string) ([]*kmspb.CryptoKeyVersion, error) -} - -// KeyRingLister is an optional capability: a Client that can also enumerate a key ring. GetKeys only -// needs it when called with no key allowlist. -// -// It is kept out of [Client] on purpose. ListCryptoKeys takes the key ring as its parent, so -// cloudkms.cryptoKeys.list can only be bound at the key ring or above — strictly broader than every -// other permission the keystore needs, all of which bind to individual CryptoKeys. A least-privilege -// deployment that names its keys explicitly should never be forced to implement, or be granted, a -// ring-wide list. -type KeyRingLister interface { - ListCryptoKeys(ctx context.Context, keyRingName string) ([]*kmspb.CryptoKey, error) } // NewClient constructs a new Google Cloud KMS client using the Go SDK. @@ -46,7 +30,7 @@ type KeyRingLister interface { // Workload Identity, GCE/Cloud Run service accounts) and local development (`gcloud auth // application-default login`, or GOOGLE_APPLICATION_CREDENTIALS pointing at a service-account key file). // -// opts is passed through to the SDK for the cases ADC does not cover — a custom endpoint or +// opts is passed through to the SDK for the cases ADC does not cover a custom endpoint or // emulator, a quota project, a non-default token source. // https://cloud.google.com/docs/authentication/application-default-credentials func NewClient(ctx context.Context, opts ...option.ClientOption) (*SDKClient, error) { @@ -57,19 +41,13 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*SDKClient, er return &SDKClient{client: client}, nil } -// SDKClient adapts the generated Cloud KMS client to this package's interfaces. It satisfies both -// [Client] and [KeyRingLister], and owns the underlying transport, so callers must Close it. -// -// Satisfying KeyRingLister says only that the method exists; whether a listing call succeeds depends -// on the credentials' IAM bindings, not on the Go type. +// SDKClient adapts the generated Cloud KMS client to this package's [Client] interface and owns +// the underlying transport, so callers must Close it. type SDKClient struct { client *apiv1.KeyManagementClient } -var ( - _ Client = (*SDKClient)(nil) - _ KeyRingLister = (*SDKClient)(nil) -) +var _ Client = (*SDKClient)(nil) func (c *SDKClient) GetCryptoKeyVersion(ctx context.Context, req *kmspb.GetCryptoKeyVersionRequest, opts ...gax.CallOption) (*kmspb.CryptoKeyVersion, error) { return c.client.GetCryptoKeyVersion(ctx, req, opts...) @@ -83,32 +61,6 @@ func (c *SDKClient) AsymmetricSign(ctx context.Context, req *kmspb.AsymmetricSig return c.client.AsymmetricSign(ctx, req, opts...) } -func (c *SDKClient) ListCryptoKeys(ctx context.Context, keyRingName string) ([]*kmspb.CryptoKey, error) { - iter := c.client.ListCryptoKeys(ctx, &kmspb.ListCryptoKeysRequest{Parent: keyRingName}) - return drain(iter.Next, "crypto keys in "+keyRingName) -} - -func (c *SDKClient) ListCryptoKeyVersions(ctx context.Context, cryptoKeyName string) ([]*kmspb.CryptoKeyVersion, error) { - iter := c.client.ListCryptoKeyVersions(ctx, &kmspb.ListCryptoKeyVersionsRequest{Parent: cryptoKeyName}) - return drain(iter.Next, "crypto key versions of "+cryptoKeyName) -} - -// drain reads a Cloud KMS iterator to completion. what describes the listed resources and is only -// used to build the error message. -func drain[T any](next func() (T, error), what string) ([]T, error) { - items := make([]T, 0) - for { - item, err := next() - if errors.Is(err, iterator.Done) { - return items, nil - } - if err != nil { - return nil, fmt.Errorf("failed to list %s: %w", what, err) - } - items = append(items, item) - } -} - func (c *SDKClient) Close() error { return c.client.Close() } diff --git a/keystore/gcpkms/fake_client.go b/keystore/gcpkms/fake_client.go index e55fbf5d92..921a11b6bd 100644 --- a/keystore/gcpkms/fake_client.go +++ b/keystore/gcpkms/fake_client.go @@ -22,6 +22,10 @@ import ( "github.com/smartcontractkit/chainlink-common/keystore/kms" ) +// cryptoKeyVersionsSegment separates a CryptoKey resource name from its version number in a +// CryptoKeyVersion resource name. +const cryptoKeyVersionsSegment = "/cryptoKeyVersions/" + // Key identifies one in-memory CryptoKeyVersion held by FakeGCPKMSClient. KeyID is a CryptoKey // resource name (projects/

/locations//keyRings//cryptoKeys/); several Keys may share a // KeyID to emulate a rotated key with multiple versions. @@ -34,15 +38,9 @@ type Key struct { VersionNumber uint64 // State is the version state. Defaults to ENABLED. State kmspb.CryptoKeyVersion_CryptoKeyVersionState - // Purpose is the purpose reported for the parent CryptoKey. Defaults to ASYMMETRIC_SIGN; set it - // to emulate an unrelated key sharing the key ring. - Purpose kmspb.CryptoKey_CryptoKeyPurpose - // Algorithm is the algorithm reported for this version and for the parent CryptoKey's version - // template. Defaults to the algorithm matching KeyType; set it to emulate an unsupported one. + // Algorithm is the algorithm reported for this version. Defaults to the algorithm matching + // KeyType; set it to emulate an unsupported one. Algorithm kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm - // OmitVersionTemplate drops VersionTemplate from the parent CryptoKey in ListCryptoKeys. The - // field is optional in the API, so a listing can arrive without it. - OmitVersionTemplate bool } // FakeGCPKMSClient is an in-memory implementation of Client for tests. It emulates the parts of @@ -91,9 +89,6 @@ func normalizeKey(key *Key) error { if key.State == kmspb.CryptoKeyVersion_CRYPTO_KEY_VERSION_STATE_UNSPECIFIED { key.State = kmspb.CryptoKeyVersion_ENABLED } - if key.Purpose == kmspb.CryptoKey_CRYPTO_KEY_PURPOSE_UNSPECIFIED { - key.Purpose = kmspb.CryptoKey_ASYMMETRIC_SIGN - } if key.Algorithm == kmspb.CryptoKeyVersion_CRYPTO_KEY_VERSION_ALGORITHM_UNSPECIFIED { algorithm, err := keyTypeToAlgorithm(key.KeyType) if err != nil { @@ -154,49 +149,6 @@ func (m *FakeGCPKMSClient) GetCryptoKeyVersion(ctx context.Context, req *kmspb.G return m.toCryptoKeyVersion(key), nil } -func (m *FakeGCPKMSClient) ListCryptoKeyVersions(ctx context.Context, cryptoKeyName string) ([]*kmspb.CryptoKeyVersion, error) { - if cryptoKeyName == "" { - return nil, errors.New("crypto key name is required") - } - versions := make([]*kmspb.CryptoKeyVersion, 0, len(m.keys)) - for i := range m.keys { - if m.keys[i].KeyID != cryptoKeyName { - continue - } - versions = append(versions, m.toCryptoKeyVersion(&m.keys[i])) - } - if len(versions) == 0 { - return nil, errors.New("key not found") - } - return versions, nil -} - -func (m *FakeGCPKMSClient) ListCryptoKeys(ctx context.Context, keyRingName string) ([]*kmspb.CryptoKey, error) { - keys := make([]*kmspb.CryptoKey, 0, len(m.keys)) - seen := make(map[string]struct{}, len(m.keys)) - for i := range m.keys { - key := &m.keys[i] - if !strings.HasPrefix(key.KeyID, keyRingName+"/cryptoKeys/") { - continue - } - if _, ok := seen[key.KeyID]; ok { - continue - } - seen[key.KeyID] = struct{}{} - // Note: no Primary — Cloud KMS only sets it for ENCRYPT_DECRYPT keys. - cryptoKey := &kmspb.CryptoKey{ - Name: key.KeyID, - Purpose: key.Purpose, - CreateTime: timestamppb.New(m.createdAt), - } - if !key.OmitVersionTemplate { - cryptoKey.VersionTemplate = &kmspb.CryptoKeyVersionTemplate{Algorithm: key.Algorithm} - } - keys = append(keys, cryptoKey) - } - return keys, nil -} - func (m *FakeGCPKMSClient) GetPublicKey(ctx context.Context, req *kmspb.GetPublicKeyRequest, opts ...gax.CallOption) (*kmspb.PublicKey, error) { if req.Name == "" { return nil, errors.New("key version name is required") diff --git a/keystore/gcpkms/keystore.go b/keystore/gcpkms/keystore.go index 060524f36b..c7175fb380 100644 --- a/keystore/gcpkms/keystore.go +++ b/keystore/gcpkms/keystore.go @@ -9,9 +9,8 @@ import ( "fmt" "hash/crc32" "sort" - "strconv" - "strings" "sync" + "time" "cloud.google.com/go/kms/apiv1/kmspb" "google.golang.org/protobuf/types/known/wrapperspb" @@ -20,17 +19,8 @@ import ( "github.com/smartcontractkit/chainlink-common/keystore/kms" ) -// cryptoKeyVersionsSegment separates a CryptoKey resource name from its version number in a -// CryptoKeyVersion resource name. -const cryptoKeyVersionsSegment = "/cryptoKeyVersions/" - -// errNoEnabledVersion and errUnsupportedAlgorithm mark the two ways a CryptoKey can turn out to be -// unusable by this keystore. Both are matchable so that a key ring listing can skip such keys -// instead of failing outright, while an explicitly requested key still surfaces the error. -var ( - errNoEnabledVersion = errors.New("has no enabled version") - errUnsupportedAlgorithm = errors.New("unsupported Cloud KMS key algorithm") -) +// errUnsupportedAlgorithm marks a CryptoKeyVersion whose algorithm this keystore cannot use. +var errUnsupportedAlgorithm = errors.New("unsupported Cloud KMS key algorithm") // castagnoliTable is the CRC32C (Castagnoli) table used by Google Cloud KMS for integrity checks. var castagnoliTable = crc32.MakeTable(crc32.Castagnoli) @@ -61,25 +51,16 @@ func checkCrc32c(data []byte, received *wrapperspb.Int64Value) error { } type keystoreSignerReader struct { - client Client - keyRingName string + client Client mu sync.RWMutex - // versions pins each key name to the CryptoKeyVersion it first resolved to, so that the public - // key reported by GetKeys and the version used by Sign can never diverge. See resolveKeyVersion. - versions map[string]resolvedKey - // publicKeys caches public keys by CryptoKeyVersion resource name. A version's public key is - // immutable, so this is safe to cache indefinitely and saves a round trip per signature. - publicKeys map[string][]byte + // versions caches everything the keystore needs for one CryptoKeyVersion: its key type, + // creation time, and public key. It is keyed by CryptoKeyVersion resource name, whose + // algorithm and public key are immutable, so entries never need invalidating. + versions map[string]*keyVersion } -type KeystoreOptions struct { - // KeyRingName is required when GetKeys is called without an explicit key allowlist. - // It must be a resource name in the format projects/

/locations//keyRings/. - KeyRingName string -} - -func NewKeystore(client Client, opts KeystoreOptions) (interface { +func NewKeystore(client Client) (interface { keystore.Reader keystore.Signer }, error) { @@ -87,10 +68,8 @@ func NewKeystore(client Client, opts KeystoreOptions) (interface { return nil, errors.New("GCP KMS client is required") } return &keystoreSignerReader{ - client: client, - keyRingName: opts.KeyRingName, - versions: make(map[string]resolvedKey), - publicKeys: make(map[string][]byte), + client: client, + versions: make(map[string]*keyVersion), }, nil } @@ -109,133 +88,62 @@ func cryptoKeyVersionAlgorithmToKeyType(algo kmspb.CryptoKeyVersion_CryptoKeyVer } } -type resolvedKey struct { - version *kmspb.CryptoKeyVersion - keyType keystore.KeyType +// keyVersion is the cached per-version data: the keystore key type, the version's creation +// time, and its public key in the keystore's native format. +type keyVersion struct { + keyType keystore.KeyType + createdAt time.Time + publicKey []byte } -// resolveKeyVersion resolves a key name to a specific, enabled CryptoKeyVersion and its keystore -// KeyType. -// -// Cloud KMS only populates CryptoKey.Primary for ENCRYPT_DECRYPT keys — asymmetric signing keys -// never have a primary version — so a concrete version has to be selected here. keyName may be -// either: -// - a CryptoKeyVersion resource name (.../cryptoKeys//cryptoKeyVersions/), which names one -// version outright, or -// - a CryptoKey resource name, in which case the highest-numbered enabled version is selected. -// -// The result is pinned for the lifetime of the keystore: the first resolution of a given key name -// wins, and every later GetKeys and Sign for that name reuses it. Re-resolving per call would let a -// rotation land between the two, so a caller could hold the public key of version N while its -// signatures came from version N+1 — a silent verification failure with nothing in SignResponse to -// identify which version signed. Pinning makes the public key reported by GetKeys authoritative for -// every signature this keystore will produce. -// -// The cost is that a rotation is picked up on restart rather than immediately. That is the safer -// default for signing keys, whose public keys are typically registered with peers or on-chain and -// cannot change under a running system. Deployments that want rotation on an explicit schedule -// should configure CryptoKeyVersion names directly. -func (k *keystoreSignerReader) resolveKeyVersion(ctx context.Context, keyName string) (resolvedKey, error) { +// getKeyVersion returns the cached data for a CryptoKeyVersion resource name, fetching and +// validating the version and its public key from Cloud KMS on first use. A version can still be +// disabled after being cached; the next AsymmetricSign then fails server-side, surfacing the +// error at sign time. +func (k *keystoreSignerReader) getKeyVersion(ctx context.Context, versionName string) (*keyVersion, error) { k.mu.RLock() - pinned, ok := k.versions[keyName] + cached, ok := k.versions[versionName] k.mu.RUnlock() if ok { - return pinned, nil + return cached, nil } - var version *kmspb.CryptoKeyVersion - if strings.Contains(keyName, cryptoKeyVersionsSegment) { - got, err := k.client.GetCryptoKeyVersion(ctx, &kmspb.GetCryptoKeyVersionRequest{Name: keyName}) - if err != nil { - return resolvedKey{}, fmt.Errorf("failed to get crypto key version %s: %w", keyName, err) - } - if got == nil { - return resolvedKey{}, fmt.Errorf("empty crypto key version response from Cloud KMS for %s", keyName) - } - if got.Name != keyName { - return resolvedKey{}, fmt.Errorf("crypto key version response has name %q, expected %q", got.Name, keyName) - } - version = got - } else { - latest, err := k.latestEnabledVersion(ctx, keyName) - if err != nil { - return resolvedKey{}, err - } - version = latest + version, err := k.client.GetCryptoKeyVersion(ctx, &kmspb.GetCryptoKeyVersionRequest{Name: versionName}) + if err != nil { + return nil, fmt.Errorf("failed to get crypto key version %s: %w", versionName, err) + } + if version == nil { + return nil, fmt.Errorf("empty crypto key version response from Cloud KMS for %s", versionName) + } + if version.Name != versionName { + return nil, fmt.Errorf("crypto key version response has name %q, expected %q", version.Name, versionName) } - if version.State != kmspb.CryptoKeyVersion_ENABLED { - return resolvedKey{}, fmt.Errorf("crypto key version %s is not enabled (state=%s)", version.Name, version.State) + return nil, fmt.Errorf("crypto key version %s is not enabled (state=%s)", version.Name, version.State) } if version.CreateTime == nil { - return resolvedKey{}, fmt.Errorf("crypto key version %s has no creation time", version.Name) + return nil, fmt.Errorf("crypto key version %s has no creation time", version.Name) } keyType, err := cryptoKeyVersionAlgorithmToKeyType(version.Algorithm) if err != nil { - return resolvedKey{}, fmt.Errorf("crypto key %s: %w", keyName, err) + return nil, fmt.Errorf("crypto key version %s: %w", version.Name, err) } - - resolved := resolvedKey{version: version, keyType: keyType} - k.mu.Lock() - defer k.mu.Unlock() - // First writer wins, so concurrent resolutions racing a rotation still converge on one version. - if existing, ok := k.versions[keyName]; ok { - return existing, nil - } - k.versions[keyName] = resolved - return resolved, nil -} - -// latestEnabledVersion returns the highest-numbered enabled version of a CryptoKey. -func (k *keystoreSignerReader) latestEnabledVersion(ctx context.Context, cryptoKeyName string) (*kmspb.CryptoKeyVersion, error) { - versions, err := k.client.ListCryptoKeyVersions(ctx, cryptoKeyName) + publicKey, err := k.publicKeyBytes(ctx, versionName, keyType) if err != nil { return nil, err } - var latest *kmspb.CryptoKeyVersion - var latestNumber uint64 - for _, version := range versions { - if version == nil || version.State != kmspb.CryptoKeyVersion_ENABLED { - continue - } - number, err := cryptoKeyVersionNumber(version.Name) - if err != nil { - return nil, err - } - if latest == nil || number > latestNumber { - latest, latestNumber = version, number - } - } - if latest == nil { - return nil, fmt.Errorf("crypto key %s %w", cryptoKeyName, errNoEnabledVersion) - } - return latest, nil -} -// cryptoKeyVersionNumber extracts the trailing version number from a CryptoKeyVersion resource -// name. Cloud KMS assigns these sequentially, so a higher number means a newer version. -func cryptoKeyVersionNumber(versionName string) (uint64, error) { - index := strings.LastIndex(versionName, cryptoKeyVersionsSegment) - if index < 0 { - return 0, fmt.Errorf("unexpected crypto key version resource name %q", versionName) - } - number, err := strconv.ParseUint(versionName[index+len(cryptoKeyVersionsSegment):], 10, 64) - if err != nil { - return 0, fmt.Errorf("unexpected crypto key version resource name %q: %w", versionName, err) - } - return number, nil + info := &keyVersion{keyType: keyType, createdAt: version.CreateTime.AsTime(), publicKey: publicKey} + k.mu.Lock() + // A concurrent fill of the same version stores equivalent data, so a plain store is fine. + k.versions[versionName] = info + k.mu.Unlock() + return info, nil } -// getPublicKeyBytes fetches the public key for a crypto key version and converts it to the -// keystore's native format for the given key type. Results are cached per version name. -func (k *keystoreSignerReader) getPublicKeyBytes(ctx context.Context, versionName string, keyType keystore.KeyType) ([]byte, error) { - k.mu.RLock() - cached, ok := k.publicKeys[versionName] - k.mu.RUnlock() - if ok { - return cached, nil - } - +// publicKeyBytes fetches the public key for a crypto key version and converts it to the +// keystore's native format for the given key type. +func (k *keystoreSignerReader) publicKeyBytes(ctx context.Context, versionName string, keyType keystore.KeyType) ([]byte, error) { pk, err := k.client.GetPublicKey(ctx, &kmspb.GetPublicKeyRequest{Name: versionName}) if err != nil { return nil, fmt.Errorf("failed to get public key for %s: %w", versionName, err) @@ -255,15 +163,11 @@ func (k *keystoreSignerReader) getPublicKeyBytes(ctx context.Context, versionNam return nil, fmt.Errorf("failed to decode PEM public key for %s", versionName) } - var publicKeyBytes []byte switch keyType { case keystore.ECDSA_S256: // GCP returns the public key in ASN.1 DER-encoded SubjectPublicKeyInfo (SPKI) format, // identical to AWS. Reuse the shared conversion. - publicKeyBytes, err = kms.ASN1ToSEC1PublicKey(block.Bytes) - if err != nil { - return nil, err - } + return kms.ASN1ToSEC1PublicKey(block.Bytes) case keystore.Ed25519: pubKey, err := x509.ParsePKIXPublicKey(block.Bytes) if err != nil { @@ -273,126 +177,58 @@ func (k *keystoreSignerReader) getPublicKeyBytes(ctx context.Context, versionNam if !ok { return nil, fmt.Errorf("failed to convert Ed25519 public key for %s to ed25519.PublicKey", versionName) } - publicKeyBytes = ed25519PubKey + return ed25519PubKey, nil default: return nil, fmt.Errorf("unsupported key type: %s", keyType) } - - k.mu.Lock() - k.publicKeys[versionName] = publicKeyBytes - k.mu.Unlock() - return publicKeyBytes, nil } -// signingKeyNames filters a key ring listing down to the asymmetric signing keys this keystore can -// use. Key rings are commonly shared, so anything unusable is skipped rather than failing the whole -// listing. +// GetKeys returns the requested keys from the Cloud KMS keystore, sorted by name. // -// The version-template check is an optimisation: it rejects keys whose configured algorithm is -// unsupported without spending an RPC on them. VersionTemplate is optional in the API, so a key that -// survives this filter is not necessarily usable; resolveKeyVersion checks the algorithm of the -// version it actually resolves, and GetKeys skips anything that fails there for the same reasons. -func signingKeyNames(listedKeys []*kmspb.CryptoKey) []string { - keyNames := make([]string, 0, len(listedKeys)) - for _, key := range listedKeys { - if key == nil || key.Name == "" { - continue - } - if key.Purpose != kmspb.CryptoKey_ASYMMETRIC_SIGN { - continue - } - if template := key.VersionTemplate; template != nil { - if _, err := cryptoKeyVersionAlgorithmToKeyType(template.Algorithm); err != nil { - continue - } - } - keyNames = append(keyNames, key.Name) - } - return keyNames -} - -// GetKeys lists keys in the Cloud KMS keystore. -// -// Key names are either CryptoKey resource names -// (projects/

/locations//keyRings//cryptoKeys/), for which the highest-numbered enabled -// version is resolved and pinned, or CryptoKeyVersion resource names, which name one version -// outright. Keys are returned sorted by name, per [keystore.Reader]; correlate them by -// KeyInfo.Name rather than by position. -// -// When no key names are given, the configured key ring is listed and keys that this keystore -// cannot use — another purpose, an unsupported algorithm, or no enabled version — are skipped, -// since a key ring may hold keys that have nothing to do with this keystore. Explicitly requested -// keys always surface their errors. -// -// Listing requires a client that implements [KeyRingLister] and credentials holding a ring-wide -// cloudkms.cryptoKeys.list. Deployments that follow least privilege grant neither and should pass an -// explicit key allowlist instead. +// Key names are CryptoKeyVersion resource names +// (projects/

/locations//keyRings//cryptoKeys//cryptoKeyVersions/): a key name +// always names exactly one version, so rotating a key means configuring the new version's name. func (k *keystoreSignerReader) GetKeys(ctx context.Context, req keystore.GetKeysRequest) (keystore.GetKeysResponse, error) { - keyNames := append([]string(nil), req.KeyNames...) - listed := len(keyNames) == 0 - if listed { - if k.keyRingName == "" { - return keystore.GetKeysResponse{}, errors.New("key ring name is required to list Cloud KMS keys") - } - lister, ok := k.client.(KeyRingLister) - if !ok { - return keystore.GetKeysResponse{}, fmt.Errorf("cannot list key ring %s: this Cloud KMS client does not implement KeyRingLister; request keys explicitly by name instead", k.keyRingName) - } - listedKeys, err := lister.ListCryptoKeys(ctx, k.keyRingName) - if err != nil { - return keystore.GetKeysResponse{}, err - } - keyNames = signingKeyNames(listedKeys) + if len(req.KeyNames) == 0 { + return keystore.GetKeysResponse{}, errors.New("key names are required: this keystore does not list key rings") } - // keystore.Reader specifies keys sorted by name. Sorting the names up front also gives the - // listing path a stable order, which Cloud KMS does not otherwise guarantee. - sort.Strings(keyNames) + versionNames := append([]string(nil), req.KeyNames...) + sort.Strings(versionNames) - keys := make([]keystore.GetKeyResponse, 0, len(keyNames)) - seen := make(map[string]struct{}, len(keyNames)) - for _, keyName := range keyNames { - if _, ok := seen[keyName]; ok { - return keystore.GetKeysResponse{}, fmt.Errorf("key %s provided multiple times", keyName) - } - seen[keyName] = struct{}{} - resolved, err := k.resolveKeyVersion(ctx, keyName) - if err != nil { - if listed && (errors.Is(err, errNoEnabledVersion) || errors.Is(err, errUnsupportedAlgorithm)) { - continue - } - return keystore.GetKeysResponse{}, err + keys := make([]keystore.GetKeyResponse, 0, len(versionNames)) + seen := make(map[string]struct{}, len(versionNames)) + for _, versionName := range versionNames { + if _, ok := seen[versionName]; ok { + return keystore.GetKeysResponse{}, fmt.Errorf("key %s provided multiple times", versionName) } - publicKeyBytes, err := k.getPublicKeyBytes(ctx, resolved.version.Name, resolved.keyType) + seen[versionName] = struct{}{} + info, err := k.getKeyVersion(ctx, versionName) if err != nil { return keystore.GetKeysResponse{}, err } - createdAt := resolved.version.CreateTime.AsTime() keys = append(keys, keystore.GetKeyResponse{ - KeyInfo: keystore.NewKeyInfo(keyName, resolved.keyType, createdAt, publicKeyBytes, []byte{}), + KeyInfo: keystore.NewKeyInfo(versionName, info.keyType, info.createdAt, info.publicKey, []byte{}), }) } return keystore.GetKeysResponse{Keys: keys}, nil } -// Sign signs data using the Cloud KMS crypto key specified by the key name. +// Sign signs data using the Cloud KMS crypto key version specified by the key name. +// +// The key name must be a CryptoKeyVersion resource name: Cloud KMS rejects bare CryptoKey names +// on AsymmetricSign, so a rotation can never change which version a configured name signs with. func (k *keystoreSignerReader) Sign(ctx context.Context, req keystore.SignRequest) (keystore.SignResponse, error) { - resolved, err := k.resolveKeyVersion(ctx, req.KeyName) + info, err := k.getKeyVersion(ctx, req.KeyName) if err != nil { return keystore.SignResponse{}, err } - versionName := resolved.version.Name + versionName := req.KeyName - switch resolved.keyType { + switch info.keyType { case keystore.ECDSA_S256: if len(req.Data) != 32 { return keystore.SignResponse{}, fmt.Errorf("data must be 32 bytes for ECDSA_S256, got %d: %w", len(req.Data), keystore.ErrInvalidSignRequest) } - // Needed only to recover the SEC1 `v` byte below. Cached per version, so this is at most - // one extra round trip per key version for the lifetime of the keystore. - pubKeyBytes, err := k.getPublicKeyBytes(ctx, versionName, resolved.keyType) - if err != nil { - return keystore.SignResponse{}, fmt.Errorf("failed to get public key for key %s: %w", req.KeyName, err) - } // The data is a pre-hashed 32-byte digest. For EC_SIGN_SECP256K1_SHA256 the digest field is // the exact bytes signed; Cloud KMS does not re-hash. @@ -417,8 +253,9 @@ func (k *keystoreSignerReader) Sign(ctx context.Context, req keystore.SignReques return keystore.SignResponse{}, fmt.Errorf("signature for key %s: %w", req.KeyName, err) } // Cloud KMS returns the ECDSA signature in ASN.1 DER format, identical to AWS. Reuse the - // shared conversion to SEC1 (R || S || V). - signature, err := kms.ASN1ToSEC1Sig(sig.Signature, pubKeyBytes, req.Data) + // shared conversion to SEC1 (R || S || V); the cached public key is needed to recover the + // `v` byte. + signature, err := kms.ASN1ToSEC1Sig(sig.Signature, info.publicKey, req.Data) if err != nil { return keystore.SignResponse{}, fmt.Errorf("failed to convert Cloud KMS signature to SEC1 signature: %w", err) } diff --git a/keystore/gcpkms/keystore_test.go b/keystore/gcpkms/keystore_test.go index 79ca8dcaff..1920abbebd 100644 --- a/keystore/gcpkms/keystore_test.go +++ b/keystore/gcpkms/keystore_test.go @@ -17,6 +17,10 @@ const ( keyRingName = "projects/test-project/locations/us-central1/keyRings/test-ring" keyName = keyRingName + "/cryptoKeys/test-key" keyName2 = keyRingName + "/cryptoKeys/test-key-2" + + // Key names are CryptoKeyVersion resource names: a name always names exactly one version. + keyVersion1 = keyName + "/cryptoKeyVersions/1" + key2Version1 = keyName2 + "/cryptoKeyVersions/1" ) func TestGCPKMSKeystore(t *testing.T) { @@ -37,51 +41,44 @@ func TestGCPKMSKeystore(t *testing.T) { }, }) require.NoError(t, err) - ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + ks, err := gcpkms.NewKeystore(fakeClient) require.NoError(t, err) ctx := t.Context() t.Run("GetKeys", func(t *testing.T) { - t.Run("listing all keys", func(t *testing.T) { - resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{}) - require.NoError(t, err) - require.Len(t, resp.Keys, 2) - require.Equal(t, keyName, resp.Keys[0].KeyInfo.Name) - require.Equal(t, keyName2, resp.Keys[1].KeyInfo.Name) + t.Run("no key names is rejected", func(t *testing.T) { + _, err := ks.GetKeys(ctx, keystore.GetKeysRequest{}) + require.ErrorContains(t, err, "does not list key rings") }) t.Run("specific keys are sorted by name", func(t *testing.T) { - // keystore.Reader specifies lexicographic order regardless of request order. resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{ - KeyNames: []string{keyName2, keyName}, + KeyNames: []string{keyVersion1, key2Version1}, }) require.NoError(t, err) require.Len(t, resp.Keys, 2) - require.Equal(t, keyName, resp.Keys[0].KeyInfo.Name) - require.Equal(t, keyName2, resp.Keys[1].KeyInfo.Name) - require.Equal(t, keystore.ECDSA_S256, resp.Keys[0].KeyInfo.KeyType) - require.Equal(t, crypto.FromECDSAPub(&key.PublicKey), resp.Keys[0].KeyInfo.PublicKey) - }) - t.Run("explicit key version", func(t *testing.T) { - resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{ - KeyNames: []string{keyName + "/cryptoKeyVersions/1"}, - }) - require.NoError(t, err) - require.Len(t, resp.Keys, 1) - require.Equal(t, keyName+"/cryptoKeyVersions/1", resp.Keys[0].KeyInfo.Name) - require.Equal(t, crypto.FromECDSAPub(&key.PublicKey), resp.Keys[0].KeyInfo.PublicKey) + require.Equal(t, key2Version1, resp.Keys[0].KeyInfo.Name) + require.Equal(t, keyVersion1, resp.Keys[1].KeyInfo.Name) + require.Equal(t, keystore.ECDSA_S256, resp.Keys[1].KeyInfo.KeyType) + require.Equal(t, crypto.FromECDSAPub(&key.PublicKey), resp.Keys[1].KeyInfo.PublicKey) }) t.Run("no such key", func(t *testing.T) { _, err := ks.GetKeys(ctx, keystore.GetKeysRequest{ - KeyNames: []string{"projects/p/locations/l/keyRings/r/cryptoKeys/nope"}, + KeyNames: []string{"projects/p/locations/l/keyRings/r/cryptoKeys/nope/cryptoKeyVersions/1"}, }) require.Error(t, err) }) + t.Run("bare crypto key name is rejected", func(t *testing.T) { + _, err := ks.GetKeys(ctx, keystore.GetKeysRequest{ + KeyNames: []string{keyName}, + }) + require.ErrorContains(t, err, "not a CryptoKeyVersion resource name") + }) }) t.Run("SignVerify", func(t *testing.T) { t.Run("invalid sign request", func(t *testing.T) { _, err := ks.Sign(ctx, keystore.SignRequest{ - KeyName: keyName, + KeyName: keyVersion1, Data: make([]byte, 31), // 31 byte digest }) require.Error(t, err) @@ -89,14 +86,21 @@ func TestGCPKMSKeystore(t *testing.T) { }) t.Run("no such key", func(t *testing.T) { _, err := ks.Sign(ctx, keystore.SignRequest{ - KeyName: "projects/p/locations/l/keyRings/r/cryptoKeys/nope", + KeyName: "projects/p/locations/l/keyRings/r/cryptoKeys/nope/cryptoKeyVersions/1", Data: make([]byte, 32), // 32 byte digest }) require.Error(t, err) }) + t.Run("bare crypto key name is rejected", func(t *testing.T) { + _, err := ks.Sign(ctx, keystore.SignRequest{ + KeyName: keyName, + Data: make([]byte, 32), + }) + require.ErrorContains(t, err, "not a CryptoKeyVersion resource name") + }) t.Run("success", func(t *testing.T) { signResp, err := ks.Sign(ctx, keystore.SignRequest{ - KeyName: keyName, + KeyName: keyVersion1, Data: make([]byte, 32), // 32 byte digest }) require.NoError(t, err) @@ -126,17 +130,17 @@ func TestGCPKMSKeystore_Ed25519(t *testing.T) { }, }) require.NoError(t, err) - ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + ks, err := gcpkms.NewKeystore(fakeClient) require.NoError(t, err) ctx := t.Context() t.Run("GetKeys", func(t *testing.T) { resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{ - KeyNames: []string{keyName}, + KeyNames: []string{keyVersion1}, }) require.NoError(t, err) require.Len(t, resp.Keys, 1) - require.Equal(t, keyName, resp.Keys[0].KeyInfo.Name) + require.Equal(t, keyVersion1, resp.Keys[0].KeyInfo.Name) require.Equal(t, keystore.Ed25519, resp.Keys[0].KeyInfo.KeyType) require.Equal(t, []byte(ed25519PubKey), resp.Keys[0].KeyInfo.PublicKey) }) @@ -145,7 +149,7 @@ func TestGCPKMSKeystore_Ed25519(t *testing.T) { // Ed25519 can sign arbitrary length messages testData := []byte("hello, world") signResp, err := ks.Sign(ctx, keystore.SignRequest{ - KeyName: keyName, + KeyName: keyVersion1, Data: testData, }) require.NoError(t, err) @@ -163,130 +167,43 @@ func TestGCPKMSKeystore_Ed25519(t *testing.T) { }) } -// The keystore must never rely on CryptoKey.Primary: Cloud KMS only populates it for -// ENCRYPT_DECRYPT keys, so an asymmetric signing key's version has to be resolved by listing. -func TestGCPKMSKeystore_ResolvesLatestEnabledVersion(t *testing.T) { - oldKey, err := crypto.GenerateKey() - require.NoError(t, err) - newKey, err := crypto.GenerateKey() +// Requesting a version this keystore cannot use e.g. an unsupported algorithm or a disabled +// version surfaces the error from both GetKeys and Sign. +func TestGCPKMSKeystore_UnusableVersions(t *testing.T) { + p256Key, err := crypto.GenerateKey() require.NoError(t, err) disabledKey, err := crypto.GenerateKey() require.NoError(t, err) fakeClient, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{ - {KeyType: keystore.ECDSA_S256, KeyID: keyName, VersionNumber: 1, PrivateKey: internal.NewRaw(crypto.FromECDSA(oldKey))}, - {KeyType: keystore.ECDSA_S256, KeyID: keyName, VersionNumber: 2, PrivateKey: internal.NewRaw(crypto.FromECDSA(newKey))}, - { - KeyType: keystore.ECDSA_S256, - KeyID: keyName, - VersionNumber: 3, - State: kmspb.CryptoKeyVersion_DISABLED, - PrivateKey: internal.NewRaw(crypto.FromECDSA(disabledKey)), - }, - }) - require.NoError(t, err) - ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) - require.NoError(t, err) - ctx := t.Context() - - t.Run("highest enabled version wins", func(t *testing.T) { - resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName}}) - require.NoError(t, err) - require.Len(t, resp.Keys, 1) - require.Equal(t, crypto.FromECDSAPub(&newKey.PublicKey), resp.Keys[0].KeyInfo.PublicKey) - }) - t.Run("signing uses the same version", func(t *testing.T) { - signResp, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyName, Data: make([]byte, 32)}) - require.NoError(t, err) - verifyResp, err := ks.Verify(ctx, keystore.VerifyRequest{ - KeyType: keystore.ECDSA_S256, - PublicKey: crypto.FromECDSAPub(&newKey.PublicKey), - Data: make([]byte, 32), - Signature: signResp.Signature, - }) - require.NoError(t, err) - require.True(t, verifyResp.Valid) - }) - t.Run("pinning an older version", func(t *testing.T) { - resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName + "/cryptoKeyVersions/1"}}) - require.NoError(t, err) - require.Len(t, resp.Keys, 1) - require.Equal(t, crypto.FromECDSAPub(&oldKey.PublicKey), resp.Keys[0].KeyInfo.PublicKey) - }) - t.Run("pinning a disabled version fails", func(t *testing.T) { - _, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName + "/cryptoKeyVersions/3"}}) - require.ErrorContains(t, err, "is not enabled") - }) -} - -// A key ring is commonly shared, so listing it must skip keys this keystore cannot use instead of -// failing the whole call. -func TestGCPKMSKeystore_ListSkipsUnusableKeys(t *testing.T) { - signingKey, err := crypto.GenerateKey() - require.NoError(t, err) - otherKey, err := crypto.GenerateKey() - require.NoError(t, err) - unsupportedKey, err := crypto.GenerateKey() - require.NoError(t, err) - disabledKey, err := crypto.GenerateKey() - require.NoError(t, err) - - fakeClient, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{ - {KeyType: keystore.ECDSA_S256, KeyID: keyName, PrivateKey: internal.NewRaw(crypto.FromECDSA(signingKey))}, - { - // An encryption key that happens to live in the same key ring. - KeyType: keystore.ECDSA_S256, - KeyID: keyRingName + "/cryptoKeys/encrypt-key", - Purpose: kmspb.CryptoKey_ENCRYPT_DECRYPT, - PrivateKey: internal.NewRaw(crypto.FromECDSA(otherKey)), - }, { // A signing key on a curve this keystore does not support. KeyType: keystore.ECDSA_S256, - KeyID: keyRingName + "/cryptoKeys/p256-key", + KeyID: keyName, Algorithm: kmspb.CryptoKeyVersion_EC_SIGN_P256_SHA256, - PrivateKey: internal.NewRaw(crypto.FromECDSA(unsupportedKey)), + PrivateKey: internal.NewRaw(crypto.FromECDSA(p256Key)), }, { - // A supported key whose only version has been disabled. KeyType: keystore.ECDSA_S256, - KeyID: keyRingName + "/cryptoKeys/disabled-key", + KeyID: keyName2, State: kmspb.CryptoKeyVersion_DISABLED, - PrivateKey: internal.NewRaw(crypto.FromECDSA(unsupportedKey)), - }, - { - // An unsupported key that reports no VersionTemplate, so the cheap pre-filter cannot - // see its algorithm and the skip has to happen after the version is resolved. - KeyType: keystore.ECDSA_S256, - KeyID: keyRingName + "/cryptoKeys/p256-key-no-template", - Algorithm: kmspb.CryptoKeyVersion_EC_SIGN_P256_SHA256, - OmitVersionTemplate: true, - PrivateKey: internal.NewRaw(crypto.FromECDSA(disabledKey)), + PrivateKey: internal.NewRaw(crypto.FromECDSA(disabledKey)), }, }) require.NoError(t, err) - ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + ks, err := gcpkms.NewKeystore(fakeClient) require.NoError(t, err) ctx := t.Context() - resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{}) - require.NoError(t, err) - require.Len(t, resp.Keys, 1) - require.Equal(t, keyName, resp.Keys[0].KeyInfo.Name) - - // Explicitly requesting an unusable key still surfaces the error. - _, err = ks.GetKeys(ctx, keystore.GetKeysRequest{ - KeyNames: []string{keyRingName + "/cryptoKeys/p256-key"}, - }) + _, err = ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyVersion1}}) require.ErrorContains(t, err, "unsupported Cloud KMS key algorithm") - _, err = ks.GetKeys(ctx, keystore.GetKeysRequest{ - KeyNames: []string{keyRingName + "/cryptoKeys/disabled-key"}, - }) - require.ErrorContains(t, err, "has no enabled version") - _, err = ks.GetKeys(ctx, keystore.GetKeysRequest{ - KeyNames: []string{keyRingName + "/cryptoKeys/p256-key-no-template"}, - }) + _, err = ks.Sign(ctx, keystore.SignRequest{KeyName: keyVersion1, Data: make([]byte, 32)}) require.ErrorContains(t, err, "unsupported Cloud KMS key algorithm") + + _, err = ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{key2Version1}}) + require.ErrorContains(t, err, "is not enabled") + _, err = ks.Sign(ctx, keystore.SignRequest{KeyName: key2Version1, Data: make([]byte, 32)}) + require.ErrorContains(t, err, "is not enabled") } func TestGCPKMSKeystore_InvalidEd25519Key(t *testing.T) { @@ -298,64 +215,18 @@ func TestGCPKMSKeystore_InvalidEd25519Key(t *testing.T) { }, }) require.NoError(t, err) - ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + ks, err := gcpkms.NewKeystore(fakeClient) require.NoError(t, err) // Must error rather than panic inside crypto/ed25519. - _, err = ks.Sign(t.Context(), keystore.SignRequest{KeyName: keyName, Data: []byte("hello")}) + _, err = ks.Sign(t.Context(), keystore.SignRequest{KeyName: keyVersion1, Data: []byte("hello")}) require.ErrorContains(t, err, "invalid Ed25519 private key length") } -// A least-privilege deployment holds per-CryptoKey permissions only, with no ring-wide -// cloudkms.cryptoKeys.list. Such a client implements Client but not KeyRingLister, and must still be -// able to do everything except enumerate the ring. -type noListClient struct { - gcpkms.Client // embedded as an interface: promotes only Client's methods, not ListCryptoKeys -} - -func TestGCPKMSKeystore_ClientWithoutKeyRingLister(t *testing.T) { - key, err := crypto.GenerateKey() - require.NoError(t, err) - fakeClient, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{ - {KeyType: keystore.ECDSA_S256, KeyID: keyName, PrivateKey: internal.NewRaw(crypto.FromECDSA(key))}, - }) - require.NoError(t, err) - - var client gcpkms.Client = noListClient{Client: fakeClient} - _, isLister := client.(gcpkms.KeyRingLister) - require.False(t, isLister, "test double must not expose ListCryptoKeys") - - ks, err := gcpkms.NewKeystore(client, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) - require.NoError(t, err) - ctx := t.Context() - - t.Run("explicit key names work", func(t *testing.T) { - resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName}}) - require.NoError(t, err) - require.Len(t, resp.Keys, 1) - require.Equal(t, crypto.FromECDSAPub(&key.PublicKey), resp.Keys[0].KeyInfo.PublicKey) - }) - t.Run("signing works", func(t *testing.T) { - signResp, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyName, Data: make([]byte, 32)}) - require.NoError(t, err) - verifyResp, err := ks.Verify(ctx, keystore.VerifyRequest{ - KeyType: keystore.ECDSA_S256, - PublicKey: crypto.FromECDSAPub(&key.PublicKey), - Data: make([]byte, 32), - Signature: signResp.Signature, - }) - require.NoError(t, err) - require.True(t, verifyResp.Valid) - }) - t.Run("listing reports a clear error", func(t *testing.T) { - _, err := ks.GetKeys(ctx, keystore.GetKeysRequest{}) - require.ErrorContains(t, err, "does not implement KeyRingLister") - }) -} - -// A rotation landing between GetKeys and Sign must not change which version signs: the public key a -// caller already holds has to stay the one that verifies its signatures. -func TestGCPKMSKeystore_PinsVersionAcrossRotation(t *testing.T) { +// A CryptoKeyVersion name names one version forever, so a rotation can never change which key a +// configured name signs with: the public key a caller already holds stays the one that verifies +// its signatures. Adopting a rotation means configuring the new version's name. +func TestGCPKMSKeystore_Rotation(t *testing.T) { originalKey, err := crypto.GenerateKey() require.NoError(t, err) rotatedKey, err := crypto.GenerateKey() @@ -365,11 +236,11 @@ func TestGCPKMSKeystore_PinsVersionAcrossRotation(t *testing.T) { {KeyType: keystore.ECDSA_S256, KeyID: keyName, VersionNumber: 1, PrivateKey: internal.NewRaw(crypto.FromECDSA(originalKey))}, }) require.NoError(t, err) - ks, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) + ks, err := gcpkms.NewKeystore(fakeClient) require.NoError(t, err) ctx := t.Context() - resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName}}) + resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyVersion1}}) require.NoError(t, err) require.Len(t, resp.Keys, 1) publicKey := resp.Keys[0].KeyInfo.PublicKey @@ -383,7 +254,7 @@ func TestGCPKMSKeystore_PinsVersionAcrossRotation(t *testing.T) { PrivateKey: internal.NewRaw(crypto.FromECDSA(rotatedKey)), })) - signResp, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyName, Data: make([]byte, 32)}) + signResp, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyVersion1, Data: make([]byte, 32)}) require.NoError(t, err) verifyResp, err := ks.Verify(ctx, keystore.VerifyRequest{ @@ -395,15 +266,8 @@ func TestGCPKMSKeystore_PinsVersionAcrossRotation(t *testing.T) { require.NoError(t, err) require.True(t, verifyResp.Valid, "signature must verify against the public key GetKeys reported") - // GetKeys keeps reporting the pinned version too, so the two never diverge. - resp, err = ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName}}) + // GetKeys keeps reporting the configured version too, so the two never diverge. + resp, err = ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyVersion1}}) require.NoError(t, err) require.Equal(t, publicKey, resp.Keys[0].KeyInfo.PublicKey) - - // A keystore started after the rotation picks up the newer version. - fresh, err := gcpkms.NewKeystore(fakeClient, gcpkms.KeystoreOptions{KeyRingName: keyRingName}) - require.NoError(t, err) - resp, err = fresh.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName}}) - require.NoError(t, err) - require.Equal(t, crypto.FromECDSAPub(&rotatedKey.PublicKey), resp.Keys[0].KeyInfo.PublicKey) } From 1f99d8f6df707b62c8dedf7d75a85712301c6d26 Mon Sep 17 00:00:00 2001 From: Amr Rezk Date: Thu, 3 Sep 2026 13:55:20 +0300 Subject: [PATCH 7/8] Remove cache entirely --- keystore/gcpkms/keystore.go | 82 ++++++++++++------------------------- 1 file changed, 26 insertions(+), 56 deletions(-) diff --git a/keystore/gcpkms/keystore.go b/keystore/gcpkms/keystore.go index c7175fb380..2f28d74a31 100644 --- a/keystore/gcpkms/keystore.go +++ b/keystore/gcpkms/keystore.go @@ -9,8 +9,6 @@ import ( "fmt" "hash/crc32" "sort" - "sync" - "time" "cloud.google.com/go/kms/apiv1/kmspb" "google.golang.org/protobuf/types/known/wrapperspb" @@ -52,12 +50,6 @@ func checkCrc32c(data []byte, received *wrapperspb.Int64Value) error { type keystoreSignerReader struct { client Client - - mu sync.RWMutex - // versions caches everything the keystore needs for one CryptoKeyVersion: its key type, - // creation time, and public key. It is keyed by CryptoKeyVersion resource name, whose - // algorithm and public key are immutable, so entries never need invalidating. - versions map[string]*keyVersion } func NewKeystore(client Client) (interface { @@ -67,10 +59,7 @@ func NewKeystore(client Client) (interface { if client == nil { return nil, errors.New("GCP KMS client is required") } - return &keystoreSignerReader{ - client: client, - versions: make(map[string]*keyVersion), - }, nil + return &keystoreSignerReader{client: client}, nil } // cryptoKeyVersionAlgorithmToKeyType converts a Cloud KMS CryptoKeyVersionAlgorithm to a keystore @@ -88,57 +77,30 @@ func cryptoKeyVersionAlgorithmToKeyType(algo kmspb.CryptoKeyVersion_CryptoKeyVer } } -// keyVersion is the cached per-version data: the keystore key type, the version's creation -// time, and its public key in the keystore's native format. -type keyVersion struct { - keyType keystore.KeyType - createdAt time.Time - publicKey []byte -} - -// getKeyVersion returns the cached data for a CryptoKeyVersion resource name, fetching and -// validating the version and its public key from Cloud KMS on first use. A version can still be -// disabled after being cached; the next AsymmetricSign then fails server-side, surfacing the -// error at sign time. -func (k *keystoreSignerReader) getKeyVersion(ctx context.Context, versionName string) (*keyVersion, error) { - k.mu.RLock() - cached, ok := k.versions[versionName] - k.mu.RUnlock() - if ok { - return cached, nil - } - +// getKeyVersion fetches a CryptoKeyVersion from Cloud KMS and validates that it is enabled and +// uses a supported algorithm. +func (k *keystoreSignerReader) getKeyVersion(ctx context.Context, versionName string) (*kmspb.CryptoKeyVersion, keystore.KeyType, error) { version, err := k.client.GetCryptoKeyVersion(ctx, &kmspb.GetCryptoKeyVersionRequest{Name: versionName}) if err != nil { - return nil, fmt.Errorf("failed to get crypto key version %s: %w", versionName, err) + return nil, "", fmt.Errorf("failed to get crypto key version %s: %w", versionName, err) } if version == nil { - return nil, fmt.Errorf("empty crypto key version response from Cloud KMS for %s", versionName) + return nil, "", fmt.Errorf("empty crypto key version response from Cloud KMS for %s", versionName) } if version.Name != versionName { - return nil, fmt.Errorf("crypto key version response has name %q, expected %q", version.Name, versionName) + return nil, "", fmt.Errorf("crypto key version response has name %q, expected %q", version.Name, versionName) } if version.State != kmspb.CryptoKeyVersion_ENABLED { - return nil, fmt.Errorf("crypto key version %s is not enabled (state=%s)", version.Name, version.State) + return nil, "", fmt.Errorf("crypto key version %s is not enabled (state=%s)", version.Name, version.State) } if version.CreateTime == nil { - return nil, fmt.Errorf("crypto key version %s has no creation time", version.Name) + return nil, "", fmt.Errorf("crypto key version %s has no creation time", version.Name) } keyType, err := cryptoKeyVersionAlgorithmToKeyType(version.Algorithm) if err != nil { - return nil, fmt.Errorf("crypto key version %s: %w", version.Name, err) - } - publicKey, err := k.publicKeyBytes(ctx, versionName, keyType) - if err != nil { - return nil, err + return nil, "", fmt.Errorf("crypto key version %s: %w", version.Name, err) } - - info := &keyVersion{keyType: keyType, createdAt: version.CreateTime.AsTime(), publicKey: publicKey} - k.mu.Lock() - // A concurrent fill of the same version stores equivalent data, so a plain store is fine. - k.versions[versionName] = info - k.mu.Unlock() - return info, nil + return version, keyType, nil } // publicKeyBytes fetches the public key for a crypto key version and converts it to the @@ -202,12 +164,16 @@ func (k *keystoreSignerReader) GetKeys(ctx context.Context, req keystore.GetKeys return keystore.GetKeysResponse{}, fmt.Errorf("key %s provided multiple times", versionName) } seen[versionName] = struct{}{} - info, err := k.getKeyVersion(ctx, versionName) + version, keyType, err := k.getKeyVersion(ctx, versionName) + if err != nil { + return keystore.GetKeysResponse{}, err + } + publicKey, err := k.publicKeyBytes(ctx, versionName, keyType) if err != nil { return keystore.GetKeysResponse{}, err } keys = append(keys, keystore.GetKeyResponse{ - KeyInfo: keystore.NewKeyInfo(versionName, info.keyType, info.createdAt, info.publicKey, []byte{}), + KeyInfo: keystore.NewKeyInfo(versionName, keyType, version.CreateTime.AsTime(), publicKey, []byte{}), }) } return keystore.GetKeysResponse{Keys: keys}, nil @@ -218,17 +184,22 @@ func (k *keystoreSignerReader) GetKeys(ctx context.Context, req keystore.GetKeys // The key name must be a CryptoKeyVersion resource name: Cloud KMS rejects bare CryptoKey names // on AsymmetricSign, so a rotation can never change which version a configured name signs with. func (k *keystoreSignerReader) Sign(ctx context.Context, req keystore.SignRequest) (keystore.SignResponse, error) { - info, err := k.getKeyVersion(ctx, req.KeyName) + _, keyType, err := k.getKeyVersion(ctx, req.KeyName) if err != nil { return keystore.SignResponse{}, err } versionName := req.KeyName - switch info.keyType { + switch keyType { case keystore.ECDSA_S256: if len(req.Data) != 32 { return keystore.SignResponse{}, fmt.Errorf("data must be 32 bytes for ECDSA_S256, got %d: %w", len(req.Data), keystore.ErrInvalidSignRequest) } + // Needed to recover the SEC1 `v` byte from the ASN.1 signature below. + pubKeyBytes, err := k.publicKeyBytes(ctx, versionName, keyType) + if err != nil { + return keystore.SignResponse{}, fmt.Errorf("failed to get public key for key %s: %w", req.KeyName, err) + } // The data is a pre-hashed 32-byte digest. For EC_SIGN_SECP256K1_SHA256 the digest field is // the exact bytes signed; Cloud KMS does not re-hash. @@ -253,9 +224,8 @@ func (k *keystoreSignerReader) Sign(ctx context.Context, req keystore.SignReques return keystore.SignResponse{}, fmt.Errorf("signature for key %s: %w", req.KeyName, err) } // Cloud KMS returns the ECDSA signature in ASN.1 DER format, identical to AWS. Reuse the - // shared conversion to SEC1 (R || S || V); the cached public key is needed to recover the - // `v` byte. - signature, err := kms.ASN1ToSEC1Sig(sig.Signature, info.publicKey, req.Data) + // shared conversion to SEC1 (R || S || V). + signature, err := kms.ASN1ToSEC1Sig(sig.Signature, pubKeyBytes, req.Data) if err != nil { return keystore.SignResponse{}, fmt.Errorf("failed to convert Cloud KMS signature to SEC1 signature: %w", err) } From 7f637fca6bff6350794d4005c33fc65f0f2063e0 Mon Sep 17 00:00:00 2001 From: Amr Rezk Date: Thu, 3 Sep 2026 16:35:19 +0300 Subject: [PATCH 8/8] Add more tests and documentation --- keystore/README.md | 56 ++++++++++++++++++++ keystore/cli/cli.go | 5 +- keystore/gcpkms/fake_client.go | 41 ++++++++++++-- keystore/gcpkms/keystore.go | 11 ++++ keystore/gcpkms/keystore_test.go | 91 ++++++++++++++++++++++++++++++++ keystore/kms/asn1_test.go | 46 ++++++++++++++++ 6 files changed, 244 insertions(+), 6 deletions(-) diff --git a/keystore/README.md b/keystore/README.md index 468c829909..dd5d77a031 100644 --- a/keystore/README.md +++ b/keystore/README.md @@ -103,6 +103,59 @@ func main() { } ``` +##### Google Cloud KMS +```go +package main + +import ( + "context" + "crypto/sha256" + + "github.com/smartcontractkit/chainlink-common/keystore" + gcpkms "github.com/smartcontractkit/chainlink-common/keystore/gcpkms" +) + +func main() { + ctx := context.Background() + + // Create a Cloud KMS backed keystore. Credentials come from Application Default + // Credentials (Workload Identity on GKE, GOOGLE_APPLICATION_CREDENTIALS locally). + client, _ := gcpkms.NewClient(ctx) + defer client.Close() + ks, _ := gcpkms.NewKeystore(client) + + // Cloud KMS key names are CryptoKeyVersion resource names: + // projects/

/locations//keyRings//cryptoKeys//cryptoKeyVersions/ + // A name always names exactly one version; rotate by configuring the new version's name. + versionName := "projects/my-project/locations/us-central1/keyRings/my-ring/cryptoKeys/my-key/cryptoKeyVersions/1" + + // GetKeys requires explicit key names. + keysResp, _ := ks.GetKeys(ctx, keystore.GetKeysRequest{ + KeyNames: []string{versionName}, + }) + + data := []byte("hello world") + hash := sha256.Sum256(data) + signResp, _ := ks.Sign(ctx, keystore.SignRequest{ + KeyName: versionName, + Data: hash[:], + }) + + verifyResp, _ := ks.Verify(ctx, keystore.VerifyRequest{ + KeyType: keysResp.Keys[0].KeyInfo.KeyType, + PublicKey: keysResp.Keys[0].KeyInfo.PublicKey, + Data: hash[:], + Signature: signResp.Signature, + }) + // verifyResp.Valid == true +} +``` +Note: unlike the file/DB and AWS backends, the GCP backend does **not** support listing key +rings. `GetKeys` requires each key name to be provided explicitly (a fully-qualified +`CryptoKeyVersion` resource name) and errors on an empty request. Callers relying +on "list all keys when no names are provided" (e.g. `keystore.CoreKeystore.Accounts`) must be +configured with explicit key names before wiring them to a GCP-backed keystore. + #### Encryption ```go @@ -177,6 +230,9 @@ export KEYSTORE_KMS_PROFILE="my-aws-profile" keys list # Lists KMS keys keys sign -d '{"KeyName": "arn:aws:kms:us-west-2:123456789012:key/abc123", "Data": ""}' ``` +Note: the CLI's KMS mode is currently **AWS-only** (`KEYSTORE_KMS_PROFILE` selects an AWS profile). +The Google Cloud KMS backend is available programmatically via `gcpkms` (see above) but has no CLI +selector yet. ### Design Principles - **Embeddable CLI** The cli package is designed to support diff --git a/keystore/cli/cli.go b/keystore/cli/cli.go index 24cc89817f..a21536b2f8 100644 --- a/keystore/cli/cli.go +++ b/keystore/cli/cli.go @@ -32,6 +32,8 @@ CLI for managing keystore keys. If KEYSTORE_KMS_PROFILE is set, will load the keystore from KMS. KEYSTORE_KMS_PROFILE: is the AWS profile to use for KMS (region will be taken from the profile). +Note: the CLI KMS mode is currently AWS-only; the Google Cloud KMS backend (keystore/gcpkms) has +no CLI selector yet and must be used programmatically. Otherwise, will load the keystore from a file or database. KEYSTORE_PASSWORD: password used to encrypt the key material before storage, must be provided. @@ -407,7 +409,8 @@ func loadKeystoreSignerReader(ctx context.Context, cmd *cobra.Command) (interfac ks.Reader ks.Signer }, error) { - // Check if KMS mode is enabled + // Check if KMS mode is enabled. AWS only: KEYSTORE_KMS_PROFILE selects an AWS profile. + // There is no GCP (keystore/gcpkms) selector yet; use the gcpkms package programmatically. kmsProfile := os.Getenv("KEYSTORE_KMS_PROFILE") if kmsProfile != "" { client, err := kms.NewClient(ctx, kms.ClientOptions{ diff --git a/keystore/gcpkms/fake_client.go b/keystore/gcpkms/fake_client.go index 921a11b6bd..b5a4c644e6 100644 --- a/keystore/gcpkms/fake_client.go +++ b/keystore/gcpkms/fake_client.go @@ -50,6 +50,37 @@ type Key struct { type FakeGCPKMSClient struct { keys []Key createdAt time.Time + + // CorruptPublicKeyCrc reports a wrong PemCrc32C from GetPublicKey so the keystore's + // CRC32C integrity check fails. + CorruptPublicKeyCrc bool + // OmitPublicKeyCrc omits PemCrc32C from GetPublicKey so the keystore treats the response + // as having no checksum. + OmitPublicKeyCrc bool + // CorruptSignatureCrc reports a wrong SignatureCrc32C from AsymmetricSign so the keystore's + // CRC32C integrity check fails. + CorruptSignatureCrc bool + // OmitSignatureCrc omits SignatureCrc32C from AsymmetricSign so the keystore treats the + // response as having no checksum. + OmitSignatureCrc bool + // SkipVerifiedDigestCrc32C reports VerifiedDigestCrc32C=false on ECDSA AsymmetricSign + // responses, exercising the keystore's rejection of an unverified digest. + SkipVerifiedDigestCrc32C bool + // SkipVerifiedDataCrc32C reports VerifiedDataCrc32C=false on Ed25519 AsymmetricSign + // responses, exercising the keystore's rejection of an unverified data payload. + SkipVerifiedDataCrc32C bool +} + +// crc32cField builds the *_crc32c integrity field, honoring the fake's corruption/omission knobs. +func (m *FakeGCPKMSClient) crc32cField(correct int64, corrupt, omit bool) *wrapperspb.Int64Value { + if omit { + return nil + } + value := correct + if corrupt { + value++ + } + return wrapperspb.Int64(value) } func NewFakeGCPKMSClient(keys []Key) (*FakeGCPKMSClient, error) { @@ -188,7 +219,7 @@ func (m *FakeGCPKMSClient) GetPublicKey(ctx context.Context, req *kmspb.GetPubli Name: req.Name, Algorithm: key.Algorithm, Pem: string(pemBytes), - PemCrc32C: wrapperspb.Int64(crc32c(pemBytes)), + PemCrc32C: m.crc32cField(crc32c(pemBytes), m.CorruptPublicKeyCrc, m.OmitPublicKeyCrc), PublicKeyFormat: kmspb.PublicKey_PEM, }, nil } @@ -222,8 +253,8 @@ func (m *FakeGCPKMSClient) AsymmetricSign(ctx context.Context, req *kmspb.Asymme return &kmspb.AsymmetricSignResponse{ Name: req.Name, Signature: derSig, - SignatureCrc32C: wrapperspb.Int64(crc32c(derSig)), - VerifiedDigestCrc32C: true, + SignatureCrc32C: m.crc32cField(crc32c(derSig), m.CorruptSignatureCrc, m.OmitSignatureCrc), + VerifiedDigestCrc32C: !m.SkipVerifiedDigestCrc32C, }, nil case keystore.Ed25519: ed25519PrivKey, err := ed25519PrivateKey(key) @@ -234,8 +265,8 @@ func (m *FakeGCPKMSClient) AsymmetricSign(ctx context.Context, req *kmspb.Asymme return &kmspb.AsymmetricSignResponse{ Name: req.Name, Signature: signature, - SignatureCrc32C: wrapperspb.Int64(crc32c(signature)), - VerifiedDataCrc32C: true, + SignatureCrc32C: m.crc32cField(crc32c(signature), m.CorruptSignatureCrc, m.OmitSignatureCrc), + VerifiedDataCrc32C: !m.SkipVerifiedDataCrc32C, }, nil default: return nil, fmt.Errorf("unsupported key type: %s", key.KeyType) diff --git a/keystore/gcpkms/keystore.go b/keystore/gcpkms/keystore.go index 2f28d74a31..20512f23ac 100644 --- a/keystore/gcpkms/keystore.go +++ b/keystore/gcpkms/keystore.go @@ -52,6 +52,13 @@ type keystoreSignerReader struct { client Client } +// NewKeystore wraps a Cloud KMS client as a keystore.Reader and keystore.Signer. +// +// Unlike the file/DB and AWS KMS backends, this keystore does not support listing key rings: +// GetKeys requires every key name to be provided explicitly (each a CryptoKeyVersion resource +// name) and returns an error if none are. Callers that expect the documented "GetKeys returns all keys when +// no names are provided" behavior (e.g. keystore.CoreKeystore.Accounts) must be configured with +// explicit key names before wiring them to a GCP-backed keystore. func NewKeystore(client Client) (interface { keystore.Reader keystore.Signer @@ -150,6 +157,10 @@ func (k *keystoreSignerReader) publicKeyBytes(ctx context.Context, versionName s // Key names are CryptoKeyVersion resource names // (projects/

/locations//keyRings//cryptoKeys//cryptoKeyVersions/): a key name // always names exactly one version, so rotating a key means configuring the new version's name. +// +// This deviates from keystore.Reader's documented contract ("GetKeys returns all keys in the +// keystore if no names are provided"): this backend does not list key rings, so an empty +// KeyNames request errors. See [NewKeystore]. func (k *keystoreSignerReader) GetKeys(ctx context.Context, req keystore.GetKeysRequest) (keystore.GetKeysResponse, error) { if len(req.KeyNames) == 0 { return keystore.GetKeysResponse{}, errors.New("key names are required: this keystore does not list key rings") diff --git a/keystore/gcpkms/keystore_test.go b/keystore/gcpkms/keystore_test.go index 1920abbebd..f3e90ec1ab 100644 --- a/keystore/gcpkms/keystore_test.go +++ b/keystore/gcpkms/keystore_test.go @@ -271,3 +271,94 @@ func TestGCPKMSKeystore_Rotation(t *testing.T) { require.NoError(t, err) require.Equal(t, publicKey, resp.Keys[0].KeyInfo.PublicKey) } + +// TestGCPKMSKeystore_CRC32CIntegrityChecks exercises the keystore's defense-in-depth against +// transport corruption: it must reject a public key or signature whose CRC32C checksum is missing +// or wrong, and reject signatures Cloud KMS reports it did not verify. +func TestGCPKMSKeystore_CRC32CIntegrityChecks(t *testing.T) { + ctx := t.Context() + + key, err := crypto.GenerateKey() + require.NoError(t, err) + + newECDSAKeystore := func(mutate func(*gcpkms.FakeGCPKMSClient)) interface { + keystore.Reader + keystore.Signer + } { + c, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{{ + KeyType: keystore.ECDSA_S256, + KeyID: keyName, + PrivateKey: internal.NewRaw(crypto.FromECDSA(key)), + }}) + require.NoError(t, err) + mutate(c) + ks, err := gcpkms.NewKeystore(c) + require.NoError(t, err) + return ks + } + + t.Run("public key wrong checksum", func(t *testing.T) { + ks := newECDSAKeystore(func(c *gcpkms.FakeGCPKMSClient) { c.CorruptPublicKeyCrc = true }) + _, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyVersion1}}) + require.ErrorContains(t, err, "CRC32C integrity check failed") + }) + t.Run("public key missing checksum", func(t *testing.T) { + ks := newECDSAKeystore(func(c *gcpkms.FakeGCPKMSClient) { c.OmitPublicKeyCrc = true }) + _, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyVersion1}}) + require.ErrorContains(t, err, "CRC32C integrity check failed") + }) + t.Run("signature wrong checksum", func(t *testing.T) { + ks := newECDSAKeystore(func(c *gcpkms.FakeGCPKMSClient) { c.CorruptSignatureCrc = true }) + _, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyVersion1, Data: make([]byte, 32)}) + require.ErrorContains(t, err, "CRC32C integrity check failed") + }) + t.Run("signature missing checksum", func(t *testing.T) { + ks := newECDSAKeystore(func(c *gcpkms.FakeGCPKMSClient) { c.OmitSignatureCrc = true }) + _, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyVersion1, Data: make([]byte, 32)}) + require.ErrorContains(t, err, "CRC32C integrity check failed") + }) + t.Run("digest not verified by KMS", func(t *testing.T) { + ks := newECDSAKeystore(func(c *gcpkms.FakeGCPKMSClient) { c.SkipVerifiedDigestCrc32C = true }) + _, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyVersion1, Data: make([]byte, 32)}) + require.ErrorContains(t, err, "digest CRC32C checksum was not verified") + }) +} + +func TestGCPKMSKeystore_Ed25519CRC32CIntegrityChecks(t *testing.T) { + ctx := t.Context() + + _, privKey, err := ed25519.GenerateKey(nil) + require.NoError(t, err) + + newEd25519Keystore := func(mutate func(*gcpkms.FakeGCPKMSClient)) interface { + keystore.Reader + keystore.Signer + } { + c, err := gcpkms.NewFakeGCPKMSClient([]gcpkms.Key{{ + KeyType: keystore.Ed25519, + KeyID: keyName, + PrivateKey: internal.NewRaw(privKey), + }}) + require.NoError(t, err) + mutate(c) + ks, err := gcpkms.NewKeystore(c) + require.NoError(t, err) + return ks + } + + t.Run("signature wrong checksum", func(t *testing.T) { + ks := newEd25519Keystore(func(c *gcpkms.FakeGCPKMSClient) { c.CorruptSignatureCrc = true }) + _, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyVersion1, Data: []byte("hello")}) + require.ErrorContains(t, err, "CRC32C integrity check failed") + }) + t.Run("signature missing checksum", func(t *testing.T) { + ks := newEd25519Keystore(func(c *gcpkms.FakeGCPKMSClient) { c.OmitSignatureCrc = true }) + _, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyVersion1, Data: []byte("hello")}) + require.ErrorContains(t, err, "CRC32C integrity check failed") + }) + t.Run("data not verified by KMS", func(t *testing.T) { + ks := newEd25519Keystore(func(c *gcpkms.FakeGCPKMSClient) { c.SkipVerifiedDataCrc32C = true }) + _, err := ks.Sign(ctx, keystore.SignRequest{KeyName: keyVersion1, Data: []byte("hello")}) + require.ErrorContains(t, err, "data CRC32C checksum was not verified") + }) +} diff --git a/keystore/kms/asn1_test.go b/keystore/kms/asn1_test.go index f0407a6e70..9cb4d97a9f 100644 --- a/keystore/kms/asn1_test.go +++ b/keystore/kms/asn1_test.go @@ -1,6 +1,8 @@ package kms_test import ( + "math/big" + "slices" "testing" "github.com/ethereum/go-ethereum/crypto" @@ -51,3 +53,47 @@ func TestASN1SignatureToSEC1Signature(t *testing.T) { require.Len(t, sec1Sig, 65) require.Equal(t, sig, sec1Sig) } + +// TestASN1SignatureToSEC1SignatureHighS exercises the EIP-2 high-S normalization branch in +// ASN1ToSEC1Sig. go-ethereum's crypto.Sign always returns a low-S signature (S <= N/2), so the +// happy path never covers it. A non-normalizing signer such as Cloud KMS emits S > N/2 roughly +// half the time, which is exactly the input this test builds: same R, S negated (S' = N - S). +func TestASN1SignatureToSEC1SignatureHighS(t *testing.T) { + privateKey, err := crypto.GenerateKey() + require.NoError(t, err) + sec1PubKey := crypto.FromECDSAPub(&privateKey.PublicKey) + + hash := crypto.Keccak256Hash([]byte("high-S test")) + + // crypto.Sign normalizes S to <= N/2 (decred SignCompact). Confirm the assumption. + sig, err := crypto.Sign(hash[:], privateKey) + require.NoError(t, err) + require.Len(t, sig, 65) + + n := crypto.S256().Params().N + halfN := new(big.Int).Div(n, big.NewInt(2)) + s := new(big.Int).SetBytes(sig[32:64]) + require.LessOrEqual(t, s.Cmp(halfN), 0, "crypto.Sign must produce a low-S signature") + + // Negate S to obtain the high-S (S > N/2) value a non-normalizing signer would emit. + highS := new(big.Int).Sub(n, s) + require.Positive(t, highS.Cmp(halfN), "negating S must yield a high-S signature") + + // Rebuild a SEC1 signature carrying the high S, then DER-encode it as Cloud KMS would. + highSSec1 := slices.Concat(sig[:32], highS.FillBytes(make([]byte, 32)), []byte{0}) + highSDER, err := kms.SEC1ToASN1Sig(highSSec1) + require.NoError(t, err) + + // ASN1ToSEC1Sig must flip S back below N/2 per EIP-2 and recover the correct V. + sec1Sig, err := kms.ASN1ToSEC1Sig(highSDER, sec1PubKey, hash[:]) + require.NoError(t, err) + require.Len(t, sec1Sig, 65) + + // The normalized signature must be byte-for-byte the original low-S signature (same R, S, V). + require.Equal(t, sig, sec1Sig) + require.Equal(t, sig[64], sec1Sig[64], "recovery id must match the low-S signature") + + recovered, err := crypto.Ecrecover(hash[:], sec1Sig) + require.NoError(t, err) + require.Equal(t, sec1PubKey, recovered, "normalized high-S signature must recover the signer") +}