Skip to content

feat(aead): keep byte leaves inside Protected end-to-end; derive Encrypt for Key and tagged leaves - #295

Open
coderdan wants to merge 3 commits into
mainfrom
feat/aead-protected-bytes-custody
Open

feat(aead): keep byte leaves inside Protected end-to-end; derive Encrypt for Key and tagged leaves#295
coderdan wants to merge 3 commits into
mainfrom
feat/aead-protected-bytes-custody

Conversation

@coderdan

Copy link
Copy Markdown
Contributor

Follow-up to #287. A sweep for hand-written Encrypt/Decrypt impls that the new derive could replace found none that should be — but the reason was a gap in vitaminc-aead, not in the derive.

The gap

Protected<T>'s blanket impls did risky_unwrap() and let the leaf re-wrap, so a Protected<[u8; 32]> was briefly a bare array on the stack on its way to a cipher that already takes it wrapped (encrypt_bytes_array). Key, TaggedFixed and TaggedVariable all hand-wrote Encrypt purely to sidestep that. TaggedVariable couldn't derive at all: Vec<u8> had no Encrypt leaf (only Decrypt).

The fix

  • Encrypt::encrypt_protected(this: Protected<Self>, …) and Decrypt::decrypt_protected(…) -> D::Ok<Protected<Self>>, default methods the blanket Protected<T> impls now route through. Defaults unwrap as before; the byte leaves ([u8; N], Vec<u8>) override them to hand the wrapped value straight to the cipher / keep the decipher's Protected<Vec<u8>> intact. No specialization needed, no wire change.
  • Vec<u8>: Encrypt byte leaf (coherence permits it beside Vec<T: Encrypt> for the same reason the Decrypt side already compiles).
  • [u8; N] decrypt copies out of the wrapped buffer instead of Vec::try_into, which dropped the vector unwiped.
  • Key, TaggedFixed, TaggedVariable now #[derive(Encrypt)] — byte-identical wire, same custody, pinned by packages/encrypt/tests/protected_bytes.rs against real AES-256-GCM.
  • README: documents the seam; "Custom Types" now points cleartext fields at #[aead(passthrough)] and lists what still needs a hand-written impl.

Not in this PR

  • Equatable<T> still unwraps to the innermost value: it has no way to peel only its own layer and pass the Protected<T> beneath on intact. Needs a small vitaminc-protected API. Comment updated to say so.
  • #[aead(skip)] for fields left out of the ciphertext, and fieldless-enum support — derive-level gaps surfaced by the same sweep, tracked separately.

Verified: cargo test --workspace --all-features (kms needs localstack, fails on main too), cargo clippy --all-targets --all-features -D warnings, RUSTDOCFLAGS=-D warnings cargo doc --workspace --all-features --no-deps, cargo fmt --check.

https://claude.ai/code/session_01KGhQ75hjeeBam4HBjwPppL

…rypt

`Protected<T>`'s blanket `Encrypt`/`Decrypt` impls unwrapped to a bare `T`
and relied on the leaf impl re-wrapping it, so a `Protected<[u8; 32]>` was
copied onto the stack as a bare array on its way to a cipher that already
accepts it wrapped. That window is why `Key` and the FFI tagged leaves
carried hand-written impls instead of deriving.

Add `Encrypt::encrypt_protected` and `Decrypt::decrypt_protected` — default
methods the blanket impls route through — and override them on the byte
leaves (`[u8; N]`, `Vec<u8>`) to hand the still-wrapped value straight to
`encrypt_bytes_array` / `encrypt_bytes_vec`, and to keep the decipher's
`Protected<Vec<u8>>` intact on the way out. `[u8; N]` decryption now copies
out of the wrapped buffer rather than `try_into`, which dropped the vector
unwiped.

`Vec<u8>` gains the `Encrypt` leaf it was missing: it already decrypted as a
byte leaf, but could not be encrypted at all, since `u8` is not `Encrypt`.

Document the seam in the README and fold `#[aead(passthrough)]` into the
"custom types" guidance now that the derive covers cleartext fields.

Claude-Session: https://claude.ai/code/session_01KGhQ75hjeeBam4HBjwPppL
…leaves

With `Protected<[u8; N]>` and `Protected<Vec<u8>>` reaching the cipher
still wrapped via `encrypt_protected`, a transparent derived newtype seals
byte-for-byte as the hand-written impls did, with the same chain of
custody. Replace the three impls with `#[derive(Encrypt)]` and pin the
transparency, the wrapped/bare wire interchange, and `Key`'s raw-bytes
shape against the real AES-256-GCM cipher.

Claude-Session: https://claude.ai/code/session_01KGhQ75hjeeBam4HBjwPppL
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

🧬 Mutation testing (cargo-mutants, --in-diff)

caught missed unviable timeout
3 0 55 0

✅ Every mutant in the changed lines was caught by a test.

@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

✅ No CRAP threshold violations

541 function(s) analyzed · threshold 30

Comment thread packages/aead/README.md
}
```

But if `id` and `email` are not to be stored in this ciphertext at all, that is a hand-written impl:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That indicates that we should have a skip option as well.

cargo-mutants runs only the mutated crate's tests, so the three mutants
in `array_from_protected` (body replaced with a constant array, `!=`
flipped to `==`) survived: the tests that exercised it lived in
`vitaminc-encrypt` behind a real cipher. These unit tests drive the bare
and wrapped `[u8; N]` decrypt paths through `MockDecipher` so the copy
and the length rejection are both caught where the logic lives.

Claude-Session: https://claude.ai/code/session_01KGhQ75hjeeBam4HBjwPppL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant