feat(stack-kms): ZeroKMS as a vitaminc key provider (CIP-3965) by freshtonic · Pull Request #1084 · cipherstash/stack

freshtonic · 2026-10-05T23:51:10Z

Summary

This ports cipherstash/cipherstash-suite#2247 (https://github.com/cipherstash/cipherstash-suite/pull/2247) to this repository. The stack-* crates moved here in #1001 and #1003, so the suite PR cannot merge there.

It adds one thing to stack-kms and removes nothing. ZeroKmsKeyset resolves one keyset over a shared Arc<StackKms> and implements the vitaminc-kms traits KeyProvider<32> and IndexKeyProvider<32>. A client library can then be generic over those traits, and run on ZeroKMS or on another data key source without knowing which.

At the trait level, ZeroKMS is RECONSTRUCTION = ClientAndServer and CUSTODY = BackendHeld: the key id names a key that ZeroKMS rebuilds, not a wrapped copy of the key. Each value gets its own key. The binding is the descriptor, which ZeroKMS binds into the data key on the server and logs on every retrieval. The KeyId is the IV followed by the tag.

DataKeySource, IndexKeySource and FakeDataKeySource do not change. The next PR (the port of cipherstash-suite#2257) moves stack-encrypt onto the new shape.

Why the keyset is bound at construction

The vitaminc traits do not force this. ZeroKMS carries keyset_id once per request, not once per payload, and the server resolves it to one authority key for the whole batch. So a provider bound at construction models the wire protocol. See cipherstash/vitaminc ADR 0004.

What changed in the port

Replayed, not merged. The import rewrote history, so the suite SHAs do not exist here. The four suite commits were replayed with git format-patch and git am -3, with the original author. The source applied with no conflicts. Paths moved from bindings/go/... to languages/golang/....
Lockfiles regenerated. The suite's Cargo.lock changes were left out. Each lockfile that builds stack-kms by path was updated with cargo update -w: the root, the stackencrypt Go guest, the stack-kms and stack-encrypt fuzz crates, and packages/eql. Only the seven git-sourced vitaminc packages are added.
vitaminc repin, in its own commit. The suite pinned c6009a2, then 9cb1965. feat(kms): key-provider traits Stack Encrypt is generic over (CIP-3965) vitaminc#352 is now rebased onto vitaminc main, which holds the 0.5.1 release that this workspace takes from crates.io. Both pins (vitaminc-kms and its vitaminc-protected) were then b3bc6eb, the tip of claude/cip-3965-key-provider. The suite's own repin commit (9b9e4a889) was dropped in favour of this one.
vitaminc repin to the current PR stack. Both pins are now 5b8a9c7, the tip of claude/cip-5029-aws-request-shape-tests (vitaminc 0.8.0 in git). That stack removes KeyProvider::ISOLATION and ::BINDING, because every source now mints one key per value and binds it, and adds KeyProvider::CUSTODY. ZeroKmsKeyset declares BackendHeld. The branch is also rebased onto main, which gave every stack-kms error variant a diagnostic code and an ErrorPayload impl. BindingNotUtf8 and MalformedKeyId now have codes (stack_kms::binding_not_utf8, stack_kms::malformed_key_id) and empty payloads.
CI: the publish dry run skips a git-pinned crate. stack-kms is published from this repository, and tests-crates.yml dry-runs cargo publish for it. cargo will not package a crate that has a git dependency, and a version beside the git would make the packaged crate build against vitaminc-kms 0.5.1 from crates.io, which does not have the traits. The step now skips such a crate with a warning and still verifies the others, as test-eql.yml does for a stack-encrypt version that is not on crates.io yet. When vitaminc releases and the pin becomes a version, the skip stops by itself.
Changelog. stack-kms had no CHANGELOG.md. It now has one, in the stack-encrypt format, with this change under Unreleased.

Before this can ship

stack-kms cannot be published while it pins vitaminc-kms to a git rev. The pin becomes a version once cipherstash/vitaminc#352 is released.

Test plan

Run locally on Rust 1.94.1 (mise):

cargo fmt --all --check, cargo clippy --locked --workspace --all-targets --all-features -D warnings
cargo nextest run --locked --workspace --all-features. The trybuild ui binary fails only with the local kache rustc wrapper on; with the wrapper off it passes, unchanged.
mise run test:doc, mise run doc, cargo build -p stack-encrypt --examples
the publish dry run, as the new step runs it
mise run wasm:wasi-check, wasm:no-http-test, wasm:guest:test, wasm:guest:build, wasm:auth-guest:build
mise run go:test, scripts/go-binding-test.sh languages/golang
fuzz replays: client-key, sealed-value, term-decode, check-record, plan-build (-runs=0)
CI green

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Added a ZeroKMS-backed key provider for generating, retrieving, and resolving keysets and index keys.
- Added support for shared KMS client usage and server-issued key identifiers.
Bug Fixes
- Added validation for malformed key identifiers and non-UTF-8 bindings.
- These validation failures are now consistently reported as encoding errors.
Documentation
- Documented the new key provider, its key handling behavior, custody model, and current release status.

changeset-bot · 2026-10-05T23:51:15Z

⚠️ No Changeset found

Latest commit: e4df2d6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

coderabbitai · 2026-10-05T23:51:48Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: b9dd9c99-c05e-4cce-b8ae-bc90d151c3ed

📥 Commits

Reviewing files that changed from the base of the PR and between c5bcba9 and e4df2d6.

⛔ Files ignored due to path filters: 4

Cargo.lock is excluded by !**/*.lock
languages/golang/encrypt/guest/Cargo.lock is excluded by !**/*.lock
packages/stack-encrypt/fuzz/Cargo.lock is excluded by !**/*.lock
packages/stack-kms/fuzz/Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing: 8

.github/workflows/tests-crates.yml
languages/golang/encrypt/guest/src/status.rs
packages/stack-kms/CHANGELOG.md
packages/stack-kms/Cargo.toml
packages/stack-kms/src/client.rs
packages/stack-kms/src/errors.rs
packages/stack-kms/src/lib.rs
packages/stack-kms/src/provider.rs

🚧 Files skipped from review as they are similar to previous changes: 2

packages/stack-kms/CHANGELOG.md
packages/stack-kms/src/lib.rs

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The PR adds ZeroKmsKeyset, a ZeroKMS-backed implementation of key-provider traits. It adds binding and key-ID validation errors, maps them to STATUS_ENCODING in the Go guest, and updates crate dry-run verification to skip crates with non-dev Git dependencies.

Changes: ZeroKMS key-provider integration

Layer / File(s)	Summary
Provider API and error contract `packages/stack-kms/Cargo.toml`, `packages/stack-kms/src/errors.rs`, `packages/stack-kms/src/key_provider.rs`, `packages/stack-kms/src/lib.rs`, `packages/stack-kms/src/provider.rs`	Adds pinned provider dependencies, exposes `ZeroKmsKeyset`, and defines errors for non-UTF-8 bindings and key IDs shorter than the 16-byte IV.
Key operations and test fixtures `packages/stack-kms/src/provider.rs`, `packages/stack-kms/src/client.rs`, `packages/stack-kms/src/client/test_connection.rs`	Implements key generation, retrieval, and local index-key loading. Shared fixtures and tests cover request data, key IDs, validation, and round trips.
Guest encoding status mapping `languages/golang/encrypt/guest/src/status.rs`	Maps `BindingNotUtf8` and `MalformedKeyId` to `STATUS_ENCODING` and tests both mappings.
Crate verification and release notes `.github/workflows/tests-crates.yml`, `packages/stack-kms/CHANGELOG.md`	Skips crates with non-dev Git dependencies during dry-run verification. Adds changelog entries for the provider, errors, dependency, and release.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant ZeroKmsKeyset
  participant StackKms
  participant ZeroKMS
  Caller->>ZeroKmsKeyset: Generate or retrieve keys
  ZeroKmsKeyset->>StackKms: Send descriptors and keyset ID
  StackKms->>ZeroKMS: Submit key request
  ZeroKMS-->>StackKms: Return key material and tag
  StackKms-->>ZeroKmsKeyset: Return key response
  ZeroKmsKeyset-->>Caller: Return protected keys and key IDs

Suggested reviewers: coderdan

Merge Risk: ⚪ Minimal · up to e4df2

No actionable merge-blocking issue remains after normal checks.

Security architecture risk: 🔵 Low · up to c5bcb

The adapter preserves the existing authenticated client and key-reconstruction path, with no demonstrated new authorization bypass. Risk is limited by the absence of a new production consumer, but server-side binding enforcement and cached-key lifecycle guarantees remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — Potential exposure follows the supplied client's credentials and authorized keysets, not possession of a key ID alone. Multiple providers may share those credentials. The reviewed change does not establish access to additional tenants, services, environments, or infrastructure authority.

Security Findings and Attack Paths

inferred — Caller-controlled bindings and key IDs reach the existing authenticated retrieval endpoint after local format validation. The inspected adapter does not demonstrate an authorization bypass, but its Bound declaration cannot independently prove rejection of mismatched descriptors, tags, or unauthorized keysets: available tests use request capture and successful stubs rather than server enforcement.

Trust Boundaries and Controls

observed — Generation and retrieval obtain tokens through the existing credential strategy and include the client identity in requests. Returned server material passes through client-key re-encryption and hashing before becoming a data key. These client-side controls are preserved; they do not substitute for server-side authorization evidence.

Resilience and Maintainability Implications

observed — Cached index-key access performs no fresh authorization check, and the cached IndexKey zeroizes on drop. The base API already returned reusable index-key material to callers. No immediate revocation guarantee is established for either design, and the adapter provides no refresh mechanism.

Hardening Proposals

proposed — Before broader adoption, substantiate the binding contract with real-service negative cases for descriptor, tag, and keyset mismatches, and document cached index-key revocation expectations and the full-client authority exposed by kms(). These are assurance proposals, not observed vulnerabilities.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 44 functions across 8 files. (3 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the main change: adding ZeroKMS as a vitaminc key provider in stack-kms.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.

Full details: Docstring Coverage

Explanation

Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 44 functions across 8 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-10-05T23:52:45Z

Mutation testing (cargo-mutants, `--in-diff`, stack-auth + stack-encrypt)

No mutants were generated for the changed lines.

`ZeroKmsKeyset` resolves one keyset over a shared `Arc<StackKms>` and implements vitaminc-kms's `KeyProvider` and `IndexKeyProvider`, so a client library can be generic over those traits and run on ZeroKMS or on a vendor data key source without knowing which. The keyset is resolved once, at construction, and every call carries it. That is not a restriction the traits impose: ZeroKMS itself carries the keyset id once per *request*, and the server resolves it to a single authority key for the whole batch. Binding at construction is what the wire protocol already does. At the trait level ZeroKMS is `ClientAndServer` reconstruction, `PerValue` isolation, and `Bound` — the binding is the descriptor, which ZeroKMS binds into the key server-side and logs on every retrieval. The `KeyId` is the IV followed by the tag. Nothing is removed. `DataKeySource` and `IndexKeySource` stay exactly as they are, so this adds a second shape beside the first rather than switching anything over; consumers move in a later change. The test fixtures move out of `client.rs`'s test module into `test_connection.rs` so both it and `provider.rs` can drive the same stub. `vitaminc-kms` is pinned to a git rev because these traits are not published yet, with `default-features = false` to keep every vendor SDK out of this crate's graph.

`ZeroKmsKeyset` added two error variants — a binding that is not UTF-8 and a key id too short to hold an IV — and `stack_kms::Error` is not `#[non_exhaustive]`, so the guest's exhaustive match stopped compiling. Both are the provider refusing its own input before any request is sent, never a ZeroKMS verdict, so both are `STATUS_ENCODING`: something the host can fix, rather than something it can only log. The test pins the classification rather than the compile. A variant added upstream already fails the build; one reclassified here would not, and the difference between "fix your input" and "log this" is what the status word is for.

`split_key_id` refuses a key id shorter than the IV, and a test covers `IV_LEN - 1`. Nothing covered `IV_LEN` itself, so tightening the guard to `<=` — rejecting a key id the format permits — passed the whole suite. A key id of exactly the IV length splits into that IV and an empty tag, which is a shape the split accepts. The test asserts the request ZeroKMS receives carries the whole id as the IV and an empty tag. Found by running the mutation gate locally over `stack-kms`, which the CI gate does not cover — its crate list is stack-auth and stack-encrypt.

… onto vitaminc 0.5.1 cipherstash/vitaminc#352 is now on vitaminc main, which carries the 0.5.1 release that the workspace takes from crates.io. Pin b3bc6eb, the tip of claude/cip-3965-key-provider. The suite PR pinned c6009a2 and then 9cb1965; neither is on the branch now. Every lockfile that builds stack-kms by path moves with it: the root, the stackencrypt Go guest, the stack-kms and stack-encrypt fuzz crates and packages/eql. Only the seven git-sourced vitaminc packages are added.

…rning stack-kms now pins vitaminc-kms to a git rev, because vitaminc has not released the key-provider traits yet (cipherstash/vitaminc#352). cargo refuses to package a crate with a git dependency, so the dry run failed on stack-kms although nothing in this PR publishes it. A `version` beside the `git` would not help: the packaged crate would then build against vitaminc-kms 0.5.1 from crates.io, which does not have the traits. The step now finds the crates in its list that have a non-dev git dependency, skips each with a warning, and verifies the rest. This is the same shape test-eql.yml uses for a stack-encrypt not yet on crates.io. When vitaminc releases and the pin becomes a version, the skip stops applying by itself.

stack-kms had no CHANGELOG.md. It is published from 0.1.0 in its own version group, so it gets one in the format stack-encrypt's uses, with the entries for this change under Unreleased.

Move the vitaminc-kms and vitaminc-protected git pins from b3bc6eb to 5b8a9c7, the head of the vitaminc PR stack ending at claude/cip-5029-aws-request-shape-tests. That stack removes KeyProvider::ISOLATION and ::BINDING, because every source now mints one key per value and binds it. It adds KeyProvider::CUSTODY. ZeroKmsKeyset declares BackendHeld: its key id names a key that ZeroKMS rebuilds, not a wrapped copy of the key. Main added a diagnostic code to every stack-kms error variant and an ErrorPayload impl. Give BindingNotUtf8 and MalformedKeyId their codes, payload arms and rows in the every-variant test.

freshtonic requested a review from a team as a code owner October 5, 2026 23:51

This was referenced Oct 5, 2026

feat(stack-encrypt)!: keysets resolve through a KeysetRegistry, and sealed leaves move to format 2 (CIP-4140) #1085

Closed

feat(stack-encrypt): run vendor KMS backends through a static keyset registry (CIP-3965) #1083

Closed

auxesis modified the milestone: Rust crates live in stack repo Oct 7, 2026

coderdan mentioned this pull request Oct 16, 2026

chore: release candidate for EQL v4 and Stack Encrypt Rust and Go support #1229

Open

freshtonic added 7 commits October 21, 2026 21:46

docs(stack-kms): changelog ZeroKmsKeyset

3776cd4

stack-kms had no CHANGELOG.md. It is published from 0.1.0 in its own version group, so it gets one in the format stack-encrypt's uses, with the entries for this change under Unreleased.

freshtonic force-pushed the claude/cip-3965-key-provider branch from c5bcba9 to e4df2d6 Compare October 21, 2026 10:52

freshtonic marked this pull request as draft October 21, 2026 11:02

tobyhede approved these changes Oct 22, 2026

View reviewed changes

Navigation Menu

Repository navigation

feat(stack-kms): ZeroKMS as a vitaminc key provider (CIP-3965) - #1084

feat(stack-kms): ZeroKMS as a vitaminc key provider (CIP-3965)#1084
freshtonic wants to merge 7 commits into
mainfrom
claude/cip-3965-key-provider

freshtonic commented Oct 5, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

changeset-bot Bot commented Oct 5, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 5, 2026 •

edited

Loading

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Oct 5, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

freshtonic commented Oct 5, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why the keyset is bound at construction

What changed in the port

Before this can ship

Test plan

Summary by CodeRabbit

Uh oh!

changeset-bot Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

coderabbitai Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Oct 5, 2026

Mutation testing (cargo-mutants, --in-diff, stack-auth + stack-encrypt)

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

freshtonic commented Oct 5, 2026 •

edited by coderabbitai Bot

Loading

changeset-bot Bot commented Oct 5, 2026 •

edited

Loading

coderabbitai Bot commented Oct 5, 2026 •

edited

Loading

Mutation testing (cargo-mutants, `--in-diff`, stack-auth + stack-encrypt)