fix: restore wrapped CEK and region key version - #98
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Ingot’s registry storage layer and schema so the region-provider-wrapped CEK (ciphertext) and the region KEK version are persisted in Ingot’s DB, enabling GET-time unwrap via the provider (e.g., OpenBao transit) and supporting in-place rewraps during region key rotation.
Changes:
- Extend
BlobEncryptionParamsand the Postgres schema to storeregion_wrapped_cekandregion_key_version. - Add a targeted
RewrapEncryptionParamsstore API for region-key rotation that updates only wrapped key material. - Update in-memory store parity and add/extend live/unit tests to cover rewrap and constraint behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| registry/stores.go | Adds wrapped CEK + region key version fields and the RewrapEncryptionParams API contract; updates table semantics documentation. |
| registry/stores_postgres.go | Persists/loads the new columns and implements RewrapEncryptionParams as an in-place UPDATE with ErrNotFound on missing rows. |
| registry/postgres_live_test.go | Extends live store tests to cover round-trip of new fields, rewrap-in-place behavior, and constraint enforcement. |
| migrations/up_live_test.go | Updates migration assertions to include the new non-null columns. |
| migrations/sql/00014_blob_encryption.sql | Updates the table definition to add region_wrapped_cek + region_key_version with CHECK constraints. |
| inmem/stores.go | Implements RewrapEncryptionParams and ensures RegionWrappedCEK is cloned to avoid slice aliasing. |
| inmem/stores_test.go | Updates helper params and adds tests for rewrap-in-place + no-aliasing behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
bajtos
left a comment
bajtos
left a comment
There was a problem hiding this comment.
Reposting from Slack:
I re-read RFC 21, and you are right that the RFC says the CEK should be stored in Postgres.
Is that the best design, though?
Rotating Region-KEK requires rewrapping all CEKs. In the proposed design, that means iterating over all CEKs, calling transit decrypt using the old KEK and transit encrypt using the new KEK - is that even possible?
I am wondering: if we store CEKs inside OpenBao, can OpenBao transparently handle KEK rotation for us?
Now that I think about it, it would be great to do a bit more research about which option/approach is the best.
Having said that, I agree with you that under the current proposal, we need to store KEK & version in the DB.
OpenBao transit custodies the KEK and encrypts/decrypts on request but never stores ciphertexts, so the wrapped CEK must live in Ingot's DB and be sent to transit/decrypt on GET, per amendment RFC - "stores the ciphertext and key version in the FIL-480 columns"