refactor(aead)!: thread AAD through the decrypt path to mirror encrypt#191
Open
coderdan wants to merge 4 commits into
Open
refactor(aead)!: thread AAD through the decrypt path to mirror encrypt#191coderdan wants to merge 4 commits into
coderdan wants to merge 4 commits into
Conversation
coderdan
force-pushed
the
feat/decipher-mirror-cipher
branch
from
a72d591 to
289cabf
Compare
Make decryption symmetric with encryption. AAD is now passed per-call through the Decipher methods and a new Decrypt::decrypt_with_aad entry point (the dual of Encrypt::encrypt_with_aad), instead of being baked into the concrete decipher at construction. Aes256Cipher::decrypt_with_aad now delegates to T::decrypt_with_aad. The visitor pattern (DecipherVisitor / SeqAccess / MapAccess) and the Ok<T> GAT are retained deliberately as the correct pull-side duals of the encrypt-side builder; map_ok stays infallible. Unblocks a follow-up ContextTag::decrypt_with_aad helper. Breaking change to the Decipher/Decrypt traits; pre-release, so no external consumers.
- Fix the map_ok doc example to override decrypt_with_aad and thread aad (the old example showed decrypt/T::decrypt, which would silently drop AAD in a wrapper-type impl). - Document on SeqAccess::next_element / MapAccess::next_entry that implementations must authenticate each element/value against the sequence/map AAD, surfacing the contract the encrypt side carries in SeqCipher::encrypt_next's signature. - Store the AAD in AesSeqAccess/AesMapAccess as Aad (copy-on-write) instead of an owned Vec<u8>, so a borrowed AAD stays borrowed and the per-element clone is cheap; also drops the duplicated .as_bytes().to_vec() lowering.
next_element/next_entry passed self.aad.clone(), which deep-copies the bytes once per element when the AAD is owned (String/Vec<u8>/u64) — N copies for an N-element sequence. Pass the borrowed bytes (&[u8]: IntoAad) instead; the borrow only needs to live for the synchronous element decrypt. Combined with the Aad field (owned AAD moves in zero-copy), the seq/map decrypt path is now allocation-free with respect to AAD.
coderdan
force-pushed
the
feat/decipher-mirror-cipher
branch
from
289cabf to
d6a15d0
Compare
Make AesDecipher public and add Aes256Cipher::decipher(ct) so callers can obtain a concrete Decipher and drive Decrypt::decrypt_with_aad directly — the decrypt-side counterpart to handing out &cipher (a Cipher). The inherent decrypt/decrypt_with_aad now delegate to it. Unblocks generic decrypt-side helpers (e.g. ContextTag::decrypt_with_aad) without a separate decryptor trait.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Makes the decrypt path symmetric with the encrypt path. Today AAD is baked into the concrete
AesDecipherat construction, while encryption threads it per-call throughCipher/Encrypt. That asymmetry meant there was no generic decrypt entry point and blocked a cleanContextTag::decrypt_with_aadhelper.AAD is now threaded per-call:
Decipher::decrypt_bytes/decrypt_seq/decrypt_map/decrypt_optiontake anaad: Aparameter.Decrypt::decrypt_with_aad— the dual ofEncrypt::encrypt_with_aad;Decrypt::decryptnow defaults to it withAad::empty().Aes256Cipher::decrypt_with_aaddelegates toT::decrypt_with_aad(theAesDecipherno longer carries anaadfield);AesSeqAccess/AesMapAccessre-supply their stored AAD per element.Deliberately kept
The visitor pattern (
DecipherVisitor/SeqAccess/MapAccess), theOk<T>GAT, and the infalliblemap_okare retained — they are the correct pull-side duals of the encrypt-side builder, not incidental indirection. Per-element AAD rides on theDeciphermethods soSeqAccess/MapAccesssignatures are untouched.Breaking change
Breaking to the
Decipher/Decrypttraits. These are unreleased (v0.2.0-pre), so there are no external consumers. The publiccipher.decrypt(ct)/cipher.decrypt_with_aad(ct, aad)entry points are unchanged.Verification
cargo test -p vitaminc-aead -p vitaminc-encrypt— 29 + 52 unit, all doctests, all ~40 roundtrip/negative cipher tests (incl. wrong-AAD, wrong-shape, option-tag-auth) pass.cargo build --workspace,cargo test -p vitaminc-aead --all-features(hlist),cargo fmt --checkall pass.Follow-up
Unblocks a
ContextTag::decrypt_with_aadhelper built onDecrypt::decrypt_with_aad(separate PR).