Repository navigation
Add public key version - #152
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes security-sensitive KMS signing/public-key-verification behavior and includes hand-committed generated protobuf descriptor bytes that cannot be fully verified in this environment, warranting human review.
Review effort: Balanced
Findings: 1
What changed in this PR
This PR adds version awareness to the KMS public-key flow so that signatures produced by a key can still be verified after the key has been rotated. Sign now reports the signing key version, and GetPublicKey can return either the current public key or a specific historical version. The change spans the proto contracts (services + plugins), the regenerated Go bindings, the Go KMS plugin providers, and the Rust gateway (validation, gRPC mapping, and JSON-RPC serialization), plus docs and tests.
Changes:
- Added
key_versiontoSignResponseand an optionalversiontoGetPublicKeyRequest/GetPublicKeyResponseacross both proto files and the generated bindings. - Introduced a
publicKeyProvidercapability and per-providerGetPublicKeyimplementations (Transit reads versioned Transit key data; Ethereum/PQC serve current-version metadata), withversion == 0rejected as invalid at both gateway and plugin. - Updated gateway validation/mapping, JSON-RPC PascalCase serialization tests, README docs, and Go/Rust unit tests.
| File | Description |
|---|---|
| proto/services/kms.proto | Adds key_version to SignResponse and optional version to GetPublicKey request/response. |
| proto/plugins/kms.proto | Mirrors the version fields on the plugin-facing contract. |
| plugins/proto/plugins/kms.pb.go | Regenerated bindings for the new proto fields. |
| plugins/pkg/plugin/kms/provider.go | Adds publicKeyRecord and the publicKeyProvider interface. |
| plugins/pkg/plugin/kms/provider_capabilities.go | Adds requirePublicKeyProvider capability check. |
| plugins/pkg/plugin/kms/public_key.go | New metadata-based public-key helper for Ethereum/PQC. |
| plugins/pkg/plugin/kms/transit_provider.go | Version-aware Transit public-key retrieval. |
| plugins/pkg/plugin/kms/ethereum_provider.go / pqc_provider.go | Wire providers to the metadata-based helper. |
| plugins/pkg/plugin/kms/openbao.go | Adds publicKeyForVersion lookup on Transit key data. |
| plugins/pkg/plugin/kms/plugin.go | Sets KeyVersion on Sign and routes GetPublicKey through the provider. |
| gateway/src/services/kms.rs | Validates version, maps new fields across gRPC. |
| gateway/src/services/jrpc.rs | Serialization/parse tests for the version fields. |
| plugins/pkg/plugin/kms/*_test.go | Coverage for version handling and zero-version rejection. |
| plugins/pkg/plugin/kms/README.md | Documents versioned GetPublicKey usage. |
Files not reviewed (1)
- plugins/proto/plugins/kms.pb.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| publicKeyProvider, err := requirePublicKeyProvider(provider, metadata.Scheme) | ||
| if err != nil { | ||
| logger.Warnf("GetPublicKey rejected: unsupported operation for key_id=%s scheme=%s", req.KeyId, metadata.Scheme) | ||
| return nil, err | ||
| } | ||
| publicKey, err := publicKeyProvider.GetPublicKey(ctx, metadata, version) |
| logger.Warnf("Sign failed for key_id=%s scheme=%s", req.KeyId, metadata.Scheme) | ||
| return nil, err | ||
| } | ||
| resp.KeyVersion = metadata.PrimaryVersion |
There was a problem hiding this comment.
KeyVersion comes from metadata, but the Transit sign call doesn't send key_version, so OpenBao always signs with the latest Transit version. The two can differ:
-
RotateKey rotates Transit first and saves primary_version afterwards. A Sign that runs in between signs with v2 but reports v1
-
if putMetadata fails after a successful Transit rotate, metadata stays on v1 until the next rotation succeeds. Every Sign in that time reports v1
in both cases the verifier later calls GetPublicKey(KeyId, 1), gets the wrong key, and verification fails
we could pass the version to Transit so the reported version is the one that signed
| if version == 0 { | ||
| version = transitInfo.LatestVersion | ||
| } | ||
| if version == 0 { | ||
| version = metadata.PrimaryVersion | ||
| } |
There was a problem hiding this comment.
with no version given, this returns Transit's LatestVersion, while Sign reports metadata.PrimaryVersion
both should come from the same source. If Sign pins key_version to metadata.PrimaryVersion (see my comment on plugin.go), this default should be metadata.PrimaryVersion too, so "current key" means the same thing in both calls
There was a problem hiding this comment.
good catch.
fixed.
| if publicKey == "" && version == metadata.PrimaryVersion { | ||
| publicKey = metadata.PublicKey | ||
| } |
There was a problem hiding this comment.
if Transit has no public key for the requested version, this falls back to metadata.PublicKey and labels it with that version. When metadata is stale, that returns a key that doesn't match the version in the response
I'd drop the fallback and return NotFound. Transit is the source of truth for per-version keys
Description
Adds version-aware public key retrieval for KMS keys so signatures can be verified later even after rotation has been executed.
This extends the KMS public-key flow by returning the signing key version from kms.Sign, allowing callers to fetch either the current public key or a specific previous historical public key version using kms.GetPublicKey.
Added KeyVersion to SignResponse.
Added optional Version to GetPublicKeyRequest.
Changed GetPublicKeyResponse to also return Version for the public key returned.
implemented kms.GetPublicKey support for:
Added provider-level public-key retrieval so Transit can read OpenBao's versioned key data.
PReserved private-key isolation: private material remains inside KMS/OpenBao and is never returned.
Updated gateway validation, JSON-RPC serializaiton, docs, proto and generated plugin bindings.
Validation flow:
Caller can now:
Validated both locally and on Dev Stack using tag v0.2.22-rc-4
CreateKey created a Transit ECDSA_P256 signing key at PrimaryVersion: 1
Sign returned KeyVersion: 1
GetPublickKey(KeyId, Version: 1) returned the version 1 pub key
RotateKey moved metadata to PrimaryVersion: 2
GetPublicKey(KeyId, Version: 1) still returned the original version 1 pub key.
GetPublicKey(KeyId) returned the current version 2 pub key.
Signing after rotation returned KeyVersion: 2
Changes
Checklist
Fixes #XX)