feat(aead): Passthrough<T> wrapper for unencrypted fields - #289
Conversation
🧬 Mutation testing (cargo-mutants,
|
| caught | missed | unviable | timeout |
|---|---|---|---|
| 0 | 0 | 8 | 0 |
✅ Every mutant in the changed lines was caught by a test.
✅ No CRAP threshold violations536 function(s) analyzed · threshold 30 |
…mic path A custom type's Encrypt/Decrypt impl is generic over every cipher, so it cannot name a specific cipher's Passthrough payload type and has no way to call MapCipher::passthrough_entry / SeqCipher::passthrough_next with a value it owns. The type-erased hooks (Cipher::passthrough_boxed, DecipherVisitor::visit_passthrough) close that gap at the trait level; this wrapper packages them so a field can be declared "stored in the clear" with one type annotation and driven through the ordinary encrypt_entry / next_entry calls like any other field. Encrypt hands the value to passthrough_boxed; Decrypt expects a passthrough node and downcasts, returning Unspecified (never panicking) for any other shape or a foreign payload type. Dynamic-path counterpart of hlist::Passthrough. Documents the non-sensitive-only / unauthenticated contract, pinned by a tamper test. Claude-Session: https://claude.ai/code/session_01T5iYiJc6xwzMcHDtMwCadG
07d4455 to
6343b84
Compare
freshtonic
left a comment
There was a problem hiding this comment.
Standards
Changes requested for two security-relevant documentation problems. The new docs recommend passthrough for tenant/routing identifiers without carrying over the existing rule that unauthenticated fields must never drive authorization or selection of trusted encrypted data. The README also teaches positional heterogeneous map decoding and states that entries return in encryption order, conflicting with the crate’s documented contract that map order is unauthenticated.
There is also a minor commit-standard issue: 6343b84 doc: Updated Passthrough doc to explain motivation should use the conventional docs(<scope>): <why> form required by CLAUDE.md. The independently repeated passthrough safety narratives have already begun to diverge; consider linking or single-sourcing a canonical contract.
Spec
The implementation otherwise satisfies #298. Passthrough<T> is publicly re-exported, routes Encrypt through passthrough_boxed while ignoring AAD, routes Decrypt through decrypt_any and a fallible Any downcast, rejects wrong node shapes/types with Unspecified, and works through top-level, map, and sequence paths. Its bounds match the type-erased channel, the tamper behavior is tested, and CI is green.
Please correct the safety guidance and order-dependent example before merging.
| //! # Why carry a field in the clear | ||
| //! | ||
| //! An encrypted record still has to work as a record. The identifier a row is | ||
| //! fetched by, the tenant it is scoped to, the schema version that says how to |
There was a problem hiding this comment.
Security documentation: tenant scope and the identifier used to fetch a row are often authorization- or trusted-selection inputs. Because passthrough values and map keys are attacker-modifiable, recommending them here without qualification can turn the wrapper into an access-control/confused-record vulnerability. Please carry over the existing aead-derive rule: passthrough is for non-security-deciding data only and must never influence authorization, tenancy, access control, or selection of trusted encrypted data. Prefer an innocuous motivating example such as display metadata or a schema hint.
There was a problem hiding this comment.
Agreed — fixed in ad8048d. The motivation now leads with the actual use case (a table where only some columns need encrypting: an id column, a display name, a schema version) and drops tenant as the motivating example. The warning section is renamed "No security guarantees whatsoever" and carries the #[aead(passthrough)] rule over verbatim in spirit: non-sensitive, non-security-deciding data only; never authorization, tenancy, access control, or selection of trusted encrypted data.
On single-sourcing: both the module docs and the README now link the #[aead(passthrough)] reference as the canonical contract rather than restating it a third time.
| type Value = User; | ||
|
|
||
| fn visit_map<A: MapAccess<'c>>(self, mut map: A) -> Result<Self::Value, Unspecified> { | ||
| // Entries come back in encryption order; pull each with its type. |
There was a problem hiding this comment.
Incorrect map contract: stored map order is not authenticated, so a reader cannot rely on encryption order. This visitor also ignores the returned keys and fixes the expected value type positionally; reordering the entries makes a valid record fail, and the example teaches the opposite of MapAccess’s key-first heterogeneous decoding contract. Please loop over next_key, match the key, then call next_value::<Passthrough<u32>>() or next_value::<String>(), with strict missing/unknown/duplicate handling.
There was a problem hiding this comment.
Good catch — the example was wrong, not just the comment. Fixed in ad8048d: the visitor now loops next_key, matches on the key, and calls next_value::<Passthrough<u32>>() / next_value::<String>() from the arm, with duplicate, unknown, and missing keys all returning Unspecified. Same shape the derive emits. The "encryption order" claim is gone; the comment now states that stored entry order is unauthenticated and why key-first is required.
The doc: commit is also reworded to docs(aead): ....
Address review on the Passthrough docs. The motivation now leads with the real use case — a table where only some columns need encrypting — and says plainly that a passthrough field carries no security guarantee at all, with the non-security-deciding rule from #[aead(passthrough)] carried over and that attribute's reference linked as the single canonical contract rather than restated a third time. The README visitor decoded entries positionally and claimed they return in encryption order. Entry order in a stored ciphertext is not authenticated, so the example now reads key-first, lets the key choose the value type, and refuses duplicate, unknown, and missing keys — the same shape the derive emits. Claude-Session: https://claude.ai/code/session_01PWcS13jzUo9vjU7toeFhVY
6343b84 to
ad8048d
Compare
What
Adds
vitaminc_aead::Passthrough<T>: a wrapper carrying a value through the ciphertext container without encrypting it, as the typed front door to the existingCipher::passthrough_boxed/DecipherVisitor::visit_passthroughchannel.cipher .encrypt_map(aad) .encrypt_entry("id", Passthrough(self.id))? // in the clear .encrypt_entry("email", self.email)? // encrypted .end()Why
A custom type's
Encrypt/Decryptimpl is generic over every cipher, so it cannot name a specific cipher'sCipher::Passthroughpayload type and has no way to callMapCipher::passthrough_entry/SeqCipher::passthrough_nextwith a value it owns (an impl cannot add bounds the trait lacks). The type-erased hooks close the gap at the trait level, but every struct that wants a clear field today has to hand-roll the sameEncrypt+ visitor-downcast pair. This wrapper is that pair, once — and the thing a future#[derive(Encrypt)]would emit for a#[encrypt(passthrough)]field.Dynamic-path counterpart of
hlist::Passthrough(which serves the static HList encoding only).Behaviour
Encrypt: hands the value topassthrough_boxed; AAD ignored (nothing to authenticate).Decrypt: expects a passthrough node and downcasts toT. Any other node shape, or a payload of a different type, isUnspecified— never a panic.Tests
aeadunit tests (spy cipher / decipher): encrypt routes through the boxed channel; decrypt downcasts; foreign payload type errors.encryptintegration tests against real AES-256-GCM: mixed struct round-trip,Vecof mixed structs, top-level passthrough, passthrough readable in stored ciphertext, wrong type fails, tamper-undetected caveat.Closes #298
Found while building the
stack-encrypttarget-directed layer in cipherstash-suite (cipherstash/cipherstash-suite#2147), whosemixed_userexample currently carries a local copy of this wrapper.https://claude.ai/code/session_01T5iYiJc6xwzMcHDtMwCadG