M6: Validate WORM/ledger key length from the vault, fail closed on mismatch #200

New Issue

2026-07-28T14:04:45Z

KofTwentyTwo commented

2026-07-28 14:04:45 +00:00

(Migrated from github.com)

Context

Story for milestone M6. Read docs/STORY-WORKFLOW.md first — it defines the process, gates, and patterns; this issue defines only the specific work. crates/nuncio-store/src/db.rs::resolve_engine_keys (db.rs:100-116) provisions three cryptographic keys from the vault (crate::vault::SecretManager::get_or_create_key_bytes). Verified in the tree: only the storage key is length-validated —

let storage_key_bytes = secrets
    .get_or_create_key_bytes(crate::vault::STORAGE_KEY_ACCOUNT, 32)
    .map_err(|e| DatabaseError::KeyProvisioning(e.to_string()))?;
let storage_key: [u8; 32] = storage_key_bytes.try_into().map_err(|_| {
    DatabaseError::KeyProvisioning("storage key material must be exactly 32 bytes".to_string())
})?;
let worm_key = secrets
    .get_or_create_key_bytes(crate::vault::WORM_KEY_ACCOUNT, 32)
    .map_err(|e| DatabaseError::KeyProvisioning(e.to_string()))?;
let ledger_key = secrets
    .get_or_create_key_bytes(crate::vault::LEDGER_KEY_ACCOUNT, 32)
    .map_err(|e| DatabaseError::KeyProvisioning(e.to_string()))?;
Ok((storage_key, worm_key, ledger_key))

(db.rs:103-115). worm_key and ledger_key are left as untyped Vec<u8> with no length check at all — if the vault ever returns a key of the wrong byte length (a hand-edited/corrupted OS keyring entry, a downgrade from a future format, etc.), it is silently accepted and used as the HMAC-SHA256 signing key for the WORM audit log (crates/nuncio-core/src/worm_audit.rs::WormAuditRecord::compute_hmac) and the filter execution ledger, weakening the tamper-evidence guarantee those logs exist to provide (reconciles #164).

Goal

Make WORM and ledger key provisioning fail closed on any length other than exactly 32 bytes, exactly like the storage key already does, so a corrupted or malformed vault entry can never silently become a weak/malformed signing key.

Files to touch / patterns to mirror

crates/nuncio-store/src/db.rs:79-85 (DatabaseEngine struct) — change worm_key: Vec<u8> and ledger_key: Vec<u8> to worm_key: [u8; 32] and ledger_key: [u8; 32], mirroring the existing storage_key: [u8; 32] field.
crates/nuncio-store/src/db.rs:99-116 (resolve_engine_keys) — apply the exact try_into().map_err(...) pattern already used for storage_key (db.rs:106-108) to worm_key and ledger_key as well. Update the function's return type from Result<([u8; 32], Vec<u8>, Vec<u8>), DatabaseError> to Result<([u8; 32], [u8; 32], [u8; 32]), DatabaseError> (the #[allow(clippy::type_complexity)] on db.rs:99 may become unnecessary once all three are the same simple array type — remove it only if cargo check-all confirms clippy no longer flags it).
crates/nuncio-store/src/db.rs:132-190-ish (DatabaseEngine::connect_file / connect_ephemeral or wherever resolve_engine_keys(..) is destructured, db.rs:142) — no signature change needed at the call site beyond the tuple type now being all-[u8; 32].
Every usage site of self.worm_key / self.ledger_key (db.rs:1262, 1354, 1525, 1585, 1603) passes the field as &self.worm_key / &self.ledger_key into functions expecting &[u8] (HmacSha256::new_from_slice, nuncio_core::verify_worm_chain) — these compile unchanged via deref coercion (&[u8; 32] coerces to &[u8]), so no call-site changes are required there.

Implementation steps

Change worm_key/ledger_key field types in DatabaseEngine to [u8; 32].
In resolve_engine_keys, add the same try_into().map_err(|_| DatabaseError::KeyProvisioning("<name> key material must be exactly 32 bytes".to_string()))? conversion for worm_key and ledger_key that storage_key already has, using a distinct error message per key (e.g. "WORM audit key material must be exactly 32 bytes", "ledger key material must be exactly 32 bytes") so a failure is diagnosable.
Update the function's return type and any type annotations that reference the old Vec<u8> shape.
Confirm cargo check-all compiles clean at every downstream usage site listed above (no code changes should be needed there beyond the type change itself, but verify).
Add tests per Acceptance Criteria, mirroring the existing get_or_create_key_bytes_fails_closed_on_corrupt_material test in crates/nuncio-store/src/vault.rs:342-353 (which tests SecretManager directly) but at the DatabaseEngine/resolve_engine_keys layer, since that vault-level test does not exercise the (currently missing) WORM/ledger length check at all.

Acceptance criteria

resolve_engine_keys fails closed (Err(DatabaseError::KeyProvisioning(_))) when the vault returns a WORM key of any length other than 32 bytes, and likewise for the ledger key — mirror the existing storage-key behavior exactly.
A new test (in crates/nuncio-store/src/db.rs's #[cfg(test)] mod tests) seeds a SecretManager::mock() with a wrong-length WORM key (e.g. via manager.set_secret(WORM_KEY_ACCOUNT, &hex::encode([0u8; 16])) — a 16-byte key hex-encoded, valid hex but wrong length) and asserts DatabaseEngine::connect_ephemeral-style construction (or whatever the real constructor path is) returns Err rather than silently proceeding with a 16-byte HMAC key. Repeat for the ledger key.
Existing tests (vault.rs:342-353's storage-key equivalent, plus all nuncio-store WORM-chain and ledger-HMAC tests) continue to pass unchanged.
No fabricated success: a malformed/corrupt vault key entry must never be silently accepted as valid key material.
cargo verify green (fmt + clippy -D warnings all-targets + test-all), 0 ignored.

Definition of done & process

Follow docs/STORY-WORKFLOW.md: no fabricated data; comment convention (intent-only, no story/GH/phase refs in .rs/.proto); mock-only tests. Branch story/m6-vault-key-length off dev; PR to dev with the gate output pasted as evidence; the PO reviews before the issue is closed.

Dependencies / notes

Not a [PROTO] story. Reconciles #164. Touches crates/nuncio-store/src/db.rs, which M6-S1 and M6-S2 also touch — coordinate landing order with the PO to avoid merge conflicts (this story's diff is small and localized to resolve_engine_keys + the DatabaseEngine field types, so it should be easy to rebase around). Size: S.

## Context Story for milestone M6. **Read [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md) first** — it defines the process, gates, and patterns; this issue defines only the specific work. `crates/nuncio-store/src/db.rs::resolve_engine_keys` (`db.rs:100-116`) provisions three cryptographic keys from the vault (`crate::vault::SecretManager::get_or_create_key_bytes`). Verified in the tree: only the storage key is length-validated — ```rust let storage_key_bytes = secrets .get_or_create_key_bytes(crate::vault::STORAGE_KEY_ACCOUNT, 32) .map_err(|e| DatabaseError::KeyProvisioning(e.to_string()))?; let storage_key: [u8; 32] = storage_key_bytes.try_into().map_err(|_| { DatabaseError::KeyProvisioning("storage key material must be exactly 32 bytes".to_string()) })?; let worm_key = secrets .get_or_create_key_bytes(crate::vault::WORM_KEY_ACCOUNT, 32) .map_err(|e| DatabaseError::KeyProvisioning(e.to_string()))?; let ledger_key = secrets .get_or_create_key_bytes(crate::vault::LEDGER_KEY_ACCOUNT, 32) .map_err(|e| DatabaseError::KeyProvisioning(e.to_string()))?; Ok((storage_key, worm_key, ledger_key)) ``` (`db.rs:103-115`). `worm_key` and `ledger_key` are left as untyped `Vec<u8>` with no length check at all — if the vault ever returns a key of the wrong byte length (a hand-edited/corrupted OS keyring entry, a downgrade from a future format, etc.), it is silently accepted and used as the HMAC-SHA256 signing key for the WORM audit log (`crates/nuncio-core/src/worm_audit.rs::WormAuditRecord::compute_hmac`) and the filter execution ledger, weakening the tamper-evidence guarantee those logs exist to provide (reconciles #164). ## Goal Make WORM and ledger key provisioning fail closed on any length other than exactly 32 bytes, exactly like the storage key already does, so a corrupted or malformed vault entry can never silently become a weak/malformed signing key. ## Files to touch / patterns to mirror - `crates/nuncio-store/src/db.rs:79-85` (`DatabaseEngine` struct) — change `worm_key: Vec<u8>` and `ledger_key: Vec<u8>` to `worm_key: [u8; 32]` and `ledger_key: [u8; 32]`, mirroring the existing `storage_key: [u8; 32]` field. - `crates/nuncio-store/src/db.rs:99-116` (`resolve_engine_keys`) — apply the exact `try_into().map_err(...)` pattern already used for `storage_key` (`db.rs:106-108`) to `worm_key` and `ledger_key` as well. Update the function's return type from `Result<([u8; 32], Vec<u8>, Vec<u8>), DatabaseError>` to `Result<([u8; 32], [u8; 32], [u8; 32]), DatabaseError>` (the `#[allow(clippy::type_complexity)]` on `db.rs:99` may become unnecessary once all three are the same simple array type — remove it only if `cargo check-all` confirms clippy no longer flags it). - `crates/nuncio-store/src/db.rs:132-190`-ish (`DatabaseEngine::connect_file` / `connect_ephemeral` or wherever `resolve_engine_keys(..)` is destructured, `db.rs:142`) — no signature change needed at the call site beyond the tuple type now being all-`[u8; 32]`. - Every usage site of `self.worm_key` / `self.ledger_key` (`db.rs:1262`, `1354`, `1525`, `1585`, `1603`) passes the field as `&self.worm_key` / `&self.ledger_key` into functions expecting `&[u8]` (`HmacSha256::new_from_slice`, `nuncio_core::verify_worm_chain`) — these compile unchanged via deref coercion (`&[u8; 32]` coerces to `&[u8]`), so no call-site changes are required there. ## Implementation steps 1. Change `worm_key`/`ledger_key` field types in `DatabaseEngine` to `[u8; 32]`. 2. In `resolve_engine_keys`, add the same `try_into().map_err(|_| DatabaseError::KeyProvisioning("<name> key material must be exactly 32 bytes".to_string()))?` conversion for `worm_key` and `ledger_key` that `storage_key` already has, using a distinct error message per key (e.g. `"WORM audit key material must be exactly 32 bytes"`, `"ledger key material must be exactly 32 bytes"`) so a failure is diagnosable. 3. Update the function's return type and any type annotations that reference the old `Vec<u8>` shape. 4. Confirm `cargo check-all` compiles clean at every downstream usage site listed above (no code changes should be needed there beyond the type change itself, but verify). 5. Add tests per Acceptance Criteria, mirroring the existing `get_or_create_key_bytes_fails_closed_on_corrupt_material` test in `crates/nuncio-store/src/vault.rs:342-353` (which tests `SecretManager` directly) but at the `DatabaseEngine`/`resolve_engine_keys` layer, since that vault-level test does not exercise the (currently missing) WORM/ledger length check at all. ## Acceptance criteria - [ ] `resolve_engine_keys` fails closed (`Err(DatabaseError::KeyProvisioning(_))`) when the vault returns a WORM key of any length other than 32 bytes, and likewise for the ledger key — mirror the existing storage-key behavior exactly. - [ ] A new test (in `crates/nuncio-store/src/db.rs`'s `#[cfg(test)] mod tests`) seeds a `SecretManager::mock()` with a wrong-length WORM key (e.g. via `manager.set_secret(WORM_KEY_ACCOUNT, &hex::encode([0u8; 16]))` — a 16-byte key hex-encoded, valid hex but wrong length) and asserts `DatabaseEngine::connect_ephemeral`-style construction (or whatever the real constructor path is) returns `Err` rather than silently proceeding with a 16-byte HMAC key. Repeat for the ledger key. - [ ] Existing tests (`vault.rs:342-353`'s storage-key equivalent, plus all `nuncio-store` WORM-chain and ledger-HMAC tests) continue to pass unchanged. - [ ] No fabricated success: a malformed/corrupt vault key entry must never be silently accepted as valid key material. - [ ] `cargo verify` green (fmt + clippy `-D warnings` all-targets + test-all), 0 ignored. ## Definition of done & process Follow [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md): no fabricated data; comment convention (intent-only, no story/GH/phase refs in `.rs`/`.proto`); mock-only tests. Branch `story/m6-vault-key-length` off `dev`; PR to `dev` with the gate output pasted as evidence; the PO reviews before the issue is closed. ## Dependencies / notes Not a [PROTO] story. Reconciles #164. Touches `crates/nuncio-store/src/db.rs`, which M6-S1 and M6-S2 also touch — coordinate landing order with the PO to avoid merge conflicts (this story's diff is small and localized to `resolve_engine_keys` + the `DatabaseEngine` field types, so it should be easy to rebase around). Size: S.

KofTwentyTwo commented

2026-07-30 03:19:16 +00:00

(Migrated from github.com)

Delivered via PR #239 (squash-merged to dev). resolve_engine_keys now length-validates the WORM audit key and the ledger key with the same try_into fail-closed pattern as the storage key (distinct KeyProvisioning messages) — a wrong-length vault entry can no longer become a weak/malformed HMAC signing key. All three key fields are Zeroizing<[u8;32]> (validated AND zeroized — no regression of #199). Two fail-closed tests (16-byte WORM/ledger keys → Err). Reconciles #164.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#200