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_KeyRowSubstitutedFromAnotherNamespaceFailsClosed moves one namespace's wrapped_key onto another's key row and asserts both GetKeyByID and GetKey fail 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-lint reports 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

Edited by Suleimi Ahmed

Merge request reports

Loading
Loading