fix(crypto): bind the namespace into the key-wrapping AAD
What
Namespace KEKs were sealed with AAD [0x01] — the wire-format version byte and
nothing else. Every namespace's KEK in a deployment was therefore sealed under the
same root key with the same AAD, which made the stored wrapped_key ciphertexts
interchangeable between namespaces.
RootKeyProvider.WrapKey and UnwrapKey now take the namespace and bind
NamespaceKeyBinding(namespaceID) — the namespace id's 16 raw bytes — as caller
AAD, so a wrapped namespace key only unwraps in the namespace it was created for.
Nothing else could enforce this. No constraint anywhere references wrapped_key
(verified against pg_constraint; ADR 007 forbids constraints on ciphertext
internals by design), and the composite credential foreign key does not help — it
constrains which key row a credential row may name, not what is inside that
row. The AEAD is the only layer where the check can live.
Why now
The tier carries no data. No table has ns_key_id, ns_key_version,
wrapped_dek or any encrypted_* column, nothing has an inbound foreign key to
namespace_encryption_keys, and credentials still sit in tmp_plaintext_* on the
three remote-repository tables. Per-format columns are #417 and enablement is
#513.
So this is a pure code change today. After #417 the same change becomes a
migration over live encrypted rows. AAD is authenticated and never stored, so
wrapped_key stays 61 bytes and the schema is untouched.
Design decisions
namespace_id, not root_key_uri. namespace_id is immutable — primary key
and hash-partition key. root_key_uri is rewritten by rotation, so binding it
would need the rewrapRow round-trip check first.
The manager binds the requested namespace, not rec.NamespaceID.
NamespaceKeyStore promises every returned record matches the requesting call's
predicates and that consumers need not re-verify it. Passing the requested id
makes the AEAD enforce that promise rather than trust it, so a store that ever
crossed namespaces fails closed instead of serving a self-consistent record for
the wrong namespace. Free, and covered by a test.
Not bound: the key row id and version. Neither exists when the wrap happens —
the id is minted by the store's insert (newID() in insertKeyRow) and the
version is computed by nextVersion inside the store transaction, after the
FOR UPDATE barrier. Binding either would mean changing the store's insert
contract or moving key generation into internal/datastore. The residual is that
two key rows of the same namespace stay interchangeable, which costs
decryptability loudly rather than serving credentials to the wrong tenant. Stated
in the spec rather than left implicit.
Spec and plan amendments ride with the change
This is an evolution of S04-A's existing plan, not a new plan.
docs/plans/2026-07-07-s04a-column-level-encryption.md gains Step 11 (this
change, with its dependency on Steps 2, 4 and 6, the named code smell for the
three-package span), a Research Findings
entry recording that the key-wrapping AAD bound nothing identifying — a correction
to what Steps 2, 4 and 6 built — and a Status row pointing at this MR.
docs/specs/S04-a-column-level-encryption.md is amended in the same MR: the
wire-format AAD table, the Associated authenticated data section, the
RootKeyProvider interface and its "stable across all phases" label, the
cross-namespace Security Consideration that this change makes true, and a
Resolutions entry explaining why the binding is taken when the sibling row-swap
weakness in Open Questions is accepted.
Verification
- The regression tests are non-vacuous: with the binding removed, five assertions fail; restored, all pass. Checked by removing it rather than assumed.
TestManager_KeyRowSubstitutedFromAnotherNamespaceFailsClosedmoves one namespace'swrapped_keyonto another's key row and asserts bothGetKeyByIDandGetKeyfail closed on a cold cache.- A KEK wrapped for one namespace does not unwrap for another under any retained root key, so a retained key is not a bypass mid-rotation.
- Green locally:
go build,go vet(with and without-tags=integration), unit,-race,GODEBUG=fips140=only ./internal/crypto/..., and all 18 pre-commit hooks. golangci-lintreports 0 issues under default tags and 0 findings in every file this branch touches under both tag sets.- Integration suites for the affected packages run against PostgreSQL 16, 17 and 18 locally.
Process notes
Plan MR. The plan and spec amendments ship with the code rather than in a preceding plan MR — a deliberate deviation, since neither edit means anything without the change.
Reviewable size. 17 files, +534/−107, mostly the plan and spec amendments plus
test-only fake updates. The behavior change is confined to internal/crypto.
Found during the #608 data-consistency review of the merged S04 A/B/C tier.
Related to https://gitlab.com/gitlab-org/ops/artifact-registry/-/work_items/651