Repository navigation
Feat: Add ChaCha20, Poly1305, and Chacha20-Poly1305-AEAD - #157
Draft
officialfrancismendoza wants to merge 394 commits into
Conversation
… AES-128 crate doctest, plus two doc facts dropped in the 7535274 merge and dcec6b8 -- the doctest's key had become an ascending byte sequence carrying 0x08 where 0x06 belongs (0x08 twice, no 0x06) while the "FIPS 197 Appendix B" comment above the plaintext stayed put, and the ciphertext assertion had been replaced by a "`block` now contains the ciphertext" comment, leaving an example that only round-trips: encrypt-then-decrypt agreeing shows the two are inverses, not that either is AES, so the doctest passed with nothing checking the cipher's output at all. Key and assertion are restored from FIPS 197 (upd1) Appendix B, whose Input and Key are 3243f6a8885a308d313198a2e0370734 and 2b7e151628aed2a6abf7158809cf4f3c, and whose output state array read column-wise is 3925841d02dc09fbdc118597196a0b32. Also restores the bitslice.rs note that test_layout_matches_the_documented_table pins the bit-plane table exhaustively and that every mask in the crate is only correct relative to it, and the BlockCipherEncryptor note that the init data's length is specified by the implementing struct via the INIT_DATA_LEN constant -- the only prose documenting that const parameter, on the trait that carries it. Fixes "not fixable while with a lookp table based implementation" to "not fixable with a lookup-table-based implementation". Doc comments only; no code changes, and the other editorial rewording from those two commits is left alone. Assisted-by: Claude:claude-opus-5 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… for a cipher with no init data, and document that contract on the three encryptor traits and the one-shots provided over them, per ounsworth's review on bcgit#133 -- the thread asked how the RNG-taking constructor should behave for a cipher that needs no randomness (Rot13 was the example) and for the answer to be written into the function contract either way. Ecb was silently doing the opposite: `INIT_DATA_LEN == 0`, `_rng` ignored, delegating to do_encrypt_init, so the codebase had already answered "alias" by accident. It now panics. Reaching for the RNG-taking constructor means the caller expects a randomized mode, and ECB is not one (SP 800-38A Table D.2 lists its IV column as "Not applicable"), so returning a deterministic encryptor would leave that mistaken expectation undisturbed; QUALITY_AND_STYLE.md's fallibility rule puts "programmer didn't read the docs" on the panic side of the line rather than the Result side, so it is unimplemented!() and not a SymmetricCipherError. The contract goes on BlockCipherEncryptor, StreamCipherEncryptor and SymmetricCipherEncryptor: an implementation with INIT_DATA_LEN == 0 must panic rather than ignore the RNG, one with INIT_DATA_LEN > 0 must draw its init data from it and must not panic. The RNG-taking one-shots (encrypt_rng on the two block/stream traits, encrypt_out_rng on the symmetric one) are provided over the constructor, so each documents that it panics in exactly the cases the constructor does. The shared conformance suites drove all three of those paths unconditionally, which would have failed ECB for obeying the new rule, so each RNG-driven block now sits behind `if INIT_DATA_LEN > 0`, the same guard already used a few lines below for the "init data differs on two runs" check -- most of that file's diff is re-indentation. ecb_tests replaces the_rng_constructor_draws_nothing, which pinned the old aliasing behaviour, with should_panic tests for the constructor and for the encrypt_rng one-shot, and the determinism test's repeatability check now calls encrypt twice instead of encrypt then encrypt_rng. The modes and aes ECB module docs note the panic where they already explain INIT_DATA_LEN == 0; the aes one matters because the padded aliases inherit it through PaddedEncryptor's delegation. One commit across the four crates rather than one per crate: the ECB change and the framework guard are the two halves of a single contract, and landing either alone leaves cargo test --workspace failing. Assisted-by: Claude:claude-opus-5 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ipherDecryptor extend SymmetricCipherEncryptor/SymmetricCipherDecryptor, with the detached-tag methods named *_detached and the inline one-shots with AAD named *_with_aad
The inherited SymmetricCipher{En,De}cryptor methods are the AEAD with no
associated data and the tag inline (ciphertext || tag), so an AEAD can be held
and used as a plain symmetric cipher. FINAL_LEN keeps one meaning across both
traits: the tag plus anything the cipher holds back. Every AEAD decryptor now
holds back the last TAG_LEN bytes it has seen, since do_update_out is shared
and cannot know which final will be called: SymmetricCipherDecryptor::do_final
checks those bytes as the tag, AEADCipherDecryptor::do_final_out_detached
decrypts them as ciphertext.
The AEAD traits keep do_update_aad and add, named for the base method they
mirror: do_final_detached / do_final_out_detached (the _out form is the
required one), encrypt_out_detached, encrypt_out_rng_detached,
encrypt_detached, decrypt_out_detached, decrypt_detached and the
*_len_detached sizing helpers; and encrypt_out_with_aad,
encrypt_out_rng_with_aad, encrypt_with_aad, decrypt_out_with_aad and
decrypt_with_aad for the inline layout with AAD. do_encrypt_init,
do_decrypt_init, update_out_len and do_update_out are now inherited, and
tagged_do_aead_{en,de}crypt_final and the tagged_*_len helpers are gone,
their jobs taken by the inherited do_final and length helpers.
SymmetricCipherDecryptor::decrypt_out now zeroizes what it wrote when
do_final fails, since an AEAD reaches it through this trait and the AEAD
one-shots always have.
AsconAead128Encryptor / AsconAead128Decryptor implement both layers with
FINAL_LEN = TAG_LEN; the decryptor carries the 16-byte hold-back. The CLI's
Ascon-AEAD128 decrypt stream drops its hand-rolled tag tail, which the
decryptor now does itself. TestFrameworkAEADCipher::test_encryptor_decryptor
runs the whole TestFrameworkSymmetricCipher suite first, then the AEAD
checks; the buffering toy implements both layers and drives every one-shot.
cargo mutants -p bouncycastle-core -f crypto/core/src/traits.rs --re
'AEADCipher|SymmetricCipherDecryptor::decrypt_out' --test-package
bouncycastle-core --test-package bouncycastle-ascon: 134 mutants, 107 caught,
27 unviable, 0 missed. cargo mutants -p bouncycastle-ascon -f
crypto/ascon/src/ascon_aead128.rs --re 'AsconAead128(En|De)cryptor'
--test-package bouncycastle-ascon --test-package cli: 76 mutants, 49 caught,
27 unviable, 0 missed.
Assisted-by: Claude:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ascon_AEAD128<Encrypting> is AsconAead128Encryptor and Ascon_AEAD128<Decrypting> is AsconAead128Decryptor, spelled as SP 800-232 spells the algorithm. A plain type alias cannot choose between two distinct types, so it is written as a projection through AsconAead128Mode, which is implemented for the two bouncycastle-modes direction markers and nothing else; any other Dir is a compile error. bouncycastle-ascon now depends on bouncycastle-modes for those markers only. The alias is re-exported at the crate root, carries a doctest of the no-AAD inline-tag round trip, and aead128_tests.rs runs the AEAD conformance suite through it. Assisted-by: Claude:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…cm-on-aead-new # Conflicts: # crypto/core/src/traits.rs
(cherry picked from commit fb594fa)
…aped for CCM CCM was implemented in part to test whether the AEAD streaming traits could support a packet cipher; it confirmed they cannot without buffering, since SP 800-38C needs the total AAD and payload length before it can authenticate anything, and the trait's do_encrypt_init/do_update_aad/do_update_out are open-ended by design for the common case (Ascon-AEAD128, and GCM once it exists) that never needs a total up front. Record the finding and the chosen resolution -- buffer internally or ship a dedicated non-buffering API, not a length parameter on the shared trait -- at the trait definition itself, cross referenced from CcmEncryptor, so a future implementor doesn't have to re-derive it. (cherry picked from commit a1b2245)
read_from_file's hex-or-raw heuristic is fine for a key, where a wrong guess only produces a mismatch, but for a CCM nonce it can turn two distinct binary nonce files into the same nonce value if both happen to be valid hex text for it -- and a repeated nonce under one key breaks CCM's authentication (SP 800-38C Appendix B). Add read_from_file_raw and use it for --nonce-file specifically; --nonce (hex on the command line) is unaffected. PR bcgit#126 review, finding F1. (cherry picked from commit 6a194ae)
…payload limit a compile error Ccm::y held Yr (the raw tag before the S0 mask) and every intermediate CBC-MAC chaining value in a plain array, unlike the keystream beside it, which is a Secret for the same reason; wrap it and finish_mac's local S0 the same way. Separately, CcmEncryptor/CcmDecryptor's BUFFER_LEN could exceed the payload limit NONCE_LEN implies (A.1's 2^8q - 1) and only fail at do_*_final, after buffering the whole message for nothing; assert the relationship at construction instead, which also makes MAX_PAYLOAD_LEN pub and lets do_*_final's # Errors sections state the guarantee precisely. Document the same capacity error as a general possibility on the trait's do_update_aad/do_update_out. PR bcgit#126 review, findings F3 and F4. (cherry picked from commit cd84a2c)
apply_keystream generated one counter block per encrypt_block call, even though A.3's Ctrj depends only on j and the counter blocks are exactly as independent as CTR's -- only the CBC-MAC half is genuinely serial (Sec 6.1 step 3). Restructure it like Ctr::apply: finish any open keystream block byte-wise, batch aligned whole blocks through encrypt_4blocks/encrypt_2blocks, then finish the tail byte-wise. Measured ~35-38% throughput gain (26->36 MiB/s for AES-128, no AAD; matches the buffering pair too), all 480 ACVP cases and 4 Appendix C vectors still pass. That made three doc passages actively wrong, since they said this was inherent: modes/src/lib.rs's mode comparison, modes_benches.rs's CCM doc comment (both rewritten with the new ratios against CTR), and lib.rs's "CCM takes no direction"/"there is no direction parameter" claims, which were already false against the code (Dir is very much a parameter) and predate this session. Also: fixed lib.rs's "264 B" vs the documented and now-tested 256 B, added size_of assertions pinning Ccm/CcmEncryptor's sizes against the memory table (previously undocumented by a test), and added the CCM aliases to the AES crate's "Modes of operation" section, which listed every other mode but this one. PR bcgit#126 review, findings F5 and F7. (cherry picked from commit 99effdc)
… doesn't have The Encrypt/Decrypt value help (rendered by clap under --help for every mode subcommand, including the three CCM ones) said a fresh IV or nonce is generated and written to the output. CCM's nonce is supplied via --nonce and never written, so bc-rust aes128-ccm --help printed instructions that produce "authentication failed" if followed. Trim the shared enum's help to direction only and point at each subcommand's own --help, which already documents its mode's exact framing (CBC/CFB/CFB8/CTR already do; CCM's own help already explains the nonce is supplied, not generated). PR bcgit#126 review, finding F6. (cherry picked from commit 8b74a5c)
go() called the *_detached one-shots, each of which needs a fresh ciphertext/plaintext buffer the size of the input on top of the input buffer already read from stdin. Use Ccm::new plus do_*_update/do_*_final directly on the buffer already in hand: input.len() is exactly the declared payload length and is supplied in one call, so the two do_*_update/do_*_final calls this replaces cannot fail, which the .expect()s explain. Also: decrypt's tag split now goes through split_last_chunk_mut, matching Ccm::decrypt's own reasoning for admitting Clen == Tlen instead of restating the spec's stricter Clen <= Tlen and then testing < anyway; and the payload-limit error message reads Ccm::MAX_PAYLOAD_LEN (now pub) instead of re-deriving it. Documented the packet-AEAD exception to CLAUDE.md's CLI-streams rule this relies on. PR bcgit#126 review, finding F8 (buffer only; the pre-existing duplicated nonce-range check is deliberate and stays, per its own comment). (cherry picked from commit 81a0cea)
… the redundant key check CcmEncryptor and CcmDecryptor carried seven identical fields and byte-for-byte identical do_update_aad, differing only in one error string in do_update_out and in which Ccm direction do_*_final builds; the "set data_started before the length check" comment was on the encryptor's copy only. Factor the buffering itself into a private CcmBuffer that both now wrap as newtypes (the same pattern bouncycastle-ascon uses for AsconAead128Encryptor/Decryptor), so the shared behavior has one body. Also: Ccm::checked_perm re-checked KeyType::SymmetricCipherKey, which P::new (AES_128::new and friends) already checks per ElectronicCodeBook::new's own documented contract -- confirmed no other mode in this crate duplicates it, so it bought nothing but a second, differently-worded error message for the same bad key. Removed, and Ccm::new/CcmEncryptor/CcmDecryptor now call P::new(key) directly like every other mode. CcmEncryptor's nonce draw now calls crate::iv::random_iv, the same OS-backed draw Cbc/Cfb/Ctr already share, instead of a CCM-specific copy of the same three lines. No behavior or memory-layout change: CcmEncryptor/CcmDecryptor are still 8400 B at BUFFER_LEN=4096, all 480 ACVP cases and 4 Appendix C vectors still pass. PR bcgit#126 review, finding F10. (cherry picked from commit 395e955)
…used no private API crypto/modes/tests/wycheproof_ccm_tests.rs drives bc-test-data's vendored aes_ccm_test.json (552 tests) through Ccm::encrypt_detached/decrypt_detached, following the file/skip-with-warning convention acvp_ccm_tests.rs already uses. Unlike the ACVP set (one nonce length, no malformed inputs), this one is deliberately adversarial: every nonce length from 8 to 2144 bits, tag sizes A.1 forbids, truncated and bit-flipped tags. Ccm's NONCE_LEN/TAG_LEN are const generics restricted to A.1's sets, so a case whose sizes fall outside them has no instantiation to dispatch to at all -- not a runtime failure, a compile-time non-option -- and those are counted as skipped rather than silently dropped. Locally: 486 of 552 cases run (405 valid, 81 invalid), 66 skipped across 63 out-of-range groups, all passing. bc-test-data/crypto/wycheproof/ already vendors sm4_ccm_test.json for this exact purpose; aes_ccm_test.json needs adding there too (copied from https://github.com/C2SP/wycheproof, testvectors_v1) for this suite to run anywhere but here -- that's a separate repository this PR cannot touch. Also, per QUALITY_AND_STYLE.md's unit-vs-integration-test rule (a unit test only where the behaviour cannot be reached from outside): moved payload_longer_than_the_q_limit_is_refused and a_short_or_long_payload_is_refused out of ccm.rs's #[cfg(test)] block into sp800_38c_tests.rs (converted from the toy Identity permutation to AES_128, matching that file's convention), since both exercise only Ccm::new/do_encrypt_update/do_encrypt_final. Deleted both_directions_mac_the_plaintext outright: it was byte-for-byte the same check as sp800_38c_tests.rs's each_direction_has_its_own_methods, just against Identity instead of AES_128. What remains in ccm.rs's own test module is exactly what its module doc says it should be: the private formatting helpers (format_b0, encode_aad_len, put_q_field) that no public API exposes directly. PR bcgit#126 review, finding F9. (cherry picked from commit 35dfb03)
…ry changes left Scoped cargo-mutants (apply_keystream, counter_block, CcmBuffer) found 7 survivors after the F5/F7/F10 commits: 3 on apply_keystream's head_len comparison/subtraction, 2 more on the same expression, and 2 on CcmBuffer::do_update_aad/do_update_out's `end > BUFFER_LEN` checks. The two BUFFER_LEN checks were genuinely untested at the exact boundary (end == BUFFER_LEN, which must be accepted, not refused) -- added the_buffering_pair_accepts_a_message_that_exactly_fills_its_buffer. apply_keystream's gap needed an actual bug, caught it, then a second attempt to test it: no existing test ever calls it with `ks_pos` genuinely strictly between 0 and BLOCK_LEN followed by a chunk large enough to reach the batched fours/pairs path -- every chunking sp800_38c_tests.rs sweeps is uniform, and Appendix C.4's 32-byte payload (Plen = 256 *bits*, not bytes) is too short regardless. Added resuming_a_part_way_open_block_agrees_with_a_one_shot, a dedicated 123-byte case; verified by hand-applying each surviving mutation and confirming it now fails before restoring the correct code. One mutant remains and is provably equivalent (`<` vs `<=` on `ks_pos < BLOCK_LEN`, since `ks_pos` never exceeds `BLOCK_LEN` and both arms agree at that boundary) -- same class as format_b0's documented `|`/`^` equivalence, now commented the same way. Re-run: 34 caught, 116 unviable, 1 equivalent, 0 missed. (cherry picked from commit 199bd56)
(cherry picked from commit 26dda13)
(cherry picked from commit d362f46)
…Internal, AES128Internal, AES192Internal, AES256Internal to indicate that this is not intended to be used directly -- the same convention that is used in SHA2 and SHA3 crates. Also exposed the modes sub-modules publicly so that each can carry its own docs.
…e AEAD traits that extend SymmetricCipherEncryptor/SymmetricCipherDecryptor, with FINAL_LEN replacing BUFFER_LEN Rebased onto feature/aead-new, the CCM adapters no longer compiled: the AEAD traits now inherit do_encrypt_init, update_out_len, do_update_out and the inline-tag do_final from the SymmetricCipher traits, and the detached finals and one-shots are the *_detached methods. The size parameter changes meaning. The inline do_final returns the whole buffered ciphertext followed by the tag, so the trait's FINAL_LEN has to be the buffer size plus TAG_LEN, which Rust cannot write as a const-generic sum. The adapters' last parameter is therefore FINAL_LEN itself: both buffers are FINAL_LEN long, the streaming capacity for the payload and the AAD is FINAL_LEN - TAG_LEN, and CcmBuffer::new asserts FINAL_LEN >= TAG_LEN and FINAL_LEN - TAG_LEN <= Ccm::MAX_PAYLOAD_LEN at compile time. A CcmEncryptor<.., BUFFER_LEN> written for the old API now has TAG_LEN less capacity; the AES_CCM_*_Encryptor/_Decryptor aliases take FINAL_LEN too. The decryptor buffers up to the full FINAL_LEN, since it cannot know before the final call whether the tag is inline. SymmetricCipherDecryptor::do_final takes the last TAG_LEN bytes as the tag (DecryptionFailed if there are fewer); do_final_out_detached treats every buffered byte as ciphertext and refuses more than FINAL_LEN - TAG_LEN, the same limit the encryptor applies. Every one-shot -- encrypt_out, encrypt_out_rng, the *_detached and *_with_aad variants, decrypt_out and decrypt_out_with_aad -- still bypasses the buffer and runs the non-buffering Ccm under the full q limit. TestFrameworkSymmetricCipher and TestFrameworkAEADCipher gain a max_message_len option (default usize::MAX), which caps the lengths they stream: the symmetric suite's 3 * FINAL_LEN + 5 assumes an unbounded stream, and a buffering cipher's capacity is below FINAL_LEN by construction. sp800_38c_tests sets it to each pair's capacity, checks the inline do_final against Appendix C.3's C, and pins the decryptor's FINAL_LEN / capacity limits. cargo mutants -p bouncycastle-modes -f crypto/modes/src/ccm.rs --re 'CcmEncryptor|CcmDecryptor|CcmBuffer' --test-package bouncycastle-modes: 170 mutants, 115 caught, 55 unviable, 0 missed. Assisted-by: Claude:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 9883bd0)
…it#124) Rebased onto the CCM branch (bcgit#126), GCM no longer compiled against the traits it was written for. Nothing about its shape changes -- it already implemented the symmetric pair with FINAL_LEN = TAG_LEN and a decryptor that holds back the last TAG_LEN bytes -- only names and one signature: - SimpleCipherEncryptor / SimpleCipherDecryptor are SymmetricCipherEncryptor / SymmetricCipherDecryptor, and TestFrameworkSimpleCipher is TestFrameworkSymmetricCipher. - SymmetricCipherError::IncorrectOutputBufferLength(&str, usize) is OutputBufferTooSmall(usize). - StreamCipherDecryptor::do_decrypt now returns the byte count, so Gcm's two in-place decrypt paths discard it with `?` and return Ok(()). Replaying the GCM commit onto CCM conflicted wherever the two add the same kind of thing: cli/src/main.rs keeps both sets of three subcommands and match arms (CCM's, then GCM's) and gains GCM's aead_mode_cmd and aes_gcm_cmd modules; the aes and modes crate docs keep both CCM's and GCM's paragraphs and table rows. CCM's docs called it the only authenticated mode, in the table, the overview, the usage section and "Choosing between the modes"; each now says it is one of two, and the recommendation in that last section is left as written. cargo mutants -p bouncycastle-modes -f crypto/modes/src/gcm.rs --re 'Gcm.*::(do_decrypt|verify_then_decrypt|do_update_out)' --test-package bouncycastle-modes (the functions this touches), with the four survivors re-run against bouncycastle-aes and cli as well: 43 mutants, 29 caught, 11 unviable, 3 missed -- the three `> 0` guards in the decryptor's do_update_out, unchanged here. Two guard zero-length copies and are equivalent; the third skips do_decrypt when nothing is released, which may leave the AAD phase open after a first do_update_out shorter than TAG_LEN. Assisted-by: Claude:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r GCM and make its inherent API private (bcgit#124) Gcm already had the shape the AEAD traits build on -- the symmetric pair with FINAL_LEN = TAG_LEN and a decryptor that holds back the last TAG_LEN bytes -- so the two impls are small: do_update_aad, and do_final_out_detached, which flushes nothing on encryption and on decryption decrypts the held-back bytes as ciphertext before checking the detached tag (zeroizing on failure). The decrypting one-shots -- decrypt_out, decrypt_out_detached and decrypt_out_with_aad -- keep Gcm's verify-then-decrypt path, so none writes plaintext before the tag is checked. With the traits covering it, the inherent API was a second, overlapping one: do_update_aad / do_encrypt / do_decrypt / finish and the in-place one-shots encrypt_detached / encrypt_detached_rng / decrypt_detached. Two of those shared names with the traits' allocating encrypt_detached / decrypt_detached but not their signatures, and mixing the inherent finish(tag) with the trait do_update_out silently ignored the held-back bytes. The streaming ones are now private helpers (absorb_aad, encrypt_in_place, decrypt_in_place, finish); the one-shots, which nothing inside the type needs, are removed. The CLI's aes*-gcm commands, the AES_GCM_* alias docs, the module docs and the GCM test suites move to the trait methods. Two behaviour fixes the AEAD conformance suite found: - The decryptor's do_update_out now closes the AAD phase on every call, not only once it releases a byte. A first call shorter than TAG_LEN releases nothing, and a do_update_aad after it was accepted and absorbed as if it preceded the ciphertext. - The one-shots staged the ciphertext in the caller's buffer before checking the tag and left it there on failure; they now zeroize it, as the trait contract requires. gcm_tests runs TestFrameworkAEADCipher over AES-128 and AES-256 (16- and 12-byte tags) and pins the two fixes; gcm_alias_tests moves the three AES_GCM_* aliases to the AEAD suite. cargo mutants -p bouncycastle-modes -f crypto/modes/src/gcm.rs --test-package bouncycastle-modes --test-package bouncycastle-aes --test-package cli: 139 mutants, 89 caught, 46 unviable, 4 missed. check_shape -> () removes only a const assertion, which no runtime test can see; the other three are the `> 0` guards in the decryptor's do_update_out, now all equivalent -- two guard zero-length copies, and the third guards decrypt_in_place, which is a no-op on an empty slice now that the AAD phase closes before it. Assisted-by: Claude:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…S_256 rename from b8a217f That commit renamed the permutation types to AES128Internal, AES192Internal and AES256Internal but only updated the aes crate's own lib.rs and aes.rs. Every other user of the old names -- the five CLI aes_*_cmd.rs files and the CFB CLI test, the modes crate's tests and benches, the bc-test-data harness, the memory-usage bench, and the doc references in the aes crate's cbc.rs and padded_mode.rs -- still spelled them the old way, so `cargo build --workspace` and `cargo test --workspace` failed to compile. Mechanical word-boundary replacement of the three identifiers, including in doc comments that name the type. The criterion group and function name strings in the modes benches (`modes::cbc::AES_128` and so on) are deliberately left as they were: they name the benchmark, not the type, and changing them would break the benchmark history published to gh-pages. Assisted-by: Claude:claude-fable-5-1 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ypt_2blocks / decrypt_2blocks / encrypt_4blocks / decrypt_4blocks required, per ounsworth's review on bcgit#133 -- the defaults (two single-block calls; two pair calls) were right only for an engine with no unit wider than a block, and silently cost twice the work for a wider one that forgot to override them: SM4, Camellia and ARIA's single-block path already runs all four lanes, so the default pair would be two full passes for two blocks. Each implementor now states its batch methods: AES spells out its fours as two bit-sliced pair calls (what the default did for it), the modes test toys and the UnpairedAes128 bench cipher get single-block loops, and SwappedPairToy's fours stay two swapped pair calls, which cfb8_tests relies on. TestFrameworkElectronicCodeBook's equivalence checks are unchanged and now pin every hand-written body. The three "trait default" bench IDs (CBC, CFB, ECB) are renamed "single-block loops", which breaks their gh-pages history. One commit across the four crates because removing the defaults does not build without the implementor bodies. cargo mutants on aes/src/ecb.rs: 21 mutants, 19 caught, 2 unviable. Assisted-by: Claude:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/125-AES-lightengine-CCM-mode Brings in b8a217f / 9da20d3 (AES_128 / AES_192 / AES_256 renamed AES128Internal / AES192Internal / AES256Internal, and the aes mode modules made public with the crate-level usage section cut to a list of links) and 334cd2b (ElectronicCodeBook's batch methods now required). - aes/src/lib.rs: upstream's link list plus AES_CCM; `ccm` is a `pub mod` like the other modes. - modes/src/lib.rs, modes/benches/modes_benches.rs: this branch's CCM additions, renamed. - The CCM code, tests, CLI and mem bench use the new AES*Internal names (bench group IDs left as upstream leaves its own). - The test-only `Identity` permutation in modes/src/ccm.rs gains the four batch methods. Assisted-by: Claude:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
officialfrancismendoza
added a commit
that referenced
this pull request
Oct 22, 2026
Keeping a key to one algorithm is moving to compile-time key types (#167), so the opt-in runtime binding that 0de70ae added goes. Nothing outside the tests ever bound a key, so no cipher call outside them changes outcome; KeyMaterial's Display/Debug lose the `algorithm` field. Removed: - KeyMaterial's `algorithm` field and KeyMaterialTrait::algorithm, set_algorithm and check_algorithm. - The 26 check_algorithm calls: 24 in crypto/cipher, one in Ascon's checked_key, one in the ToyCbc test double. The cipher imports go back to KeyMaterial alone. - The framework's key_binding module and its four suite blocks, two tests in key_material_tests.rs, and aes/tests/key_binding_tests.rs, including the cross-size test 6994217 added there. This is not `git revert 0de70ae`: that commit also folded the two ToyBlockCipher copies into the cipher crate's, which stays, and a plain revert conflicts with 6994217. Files no later commit touched are back to 0de70ae~1 byte for byte. #172's from_key_material and #157's ChaCha20 keys still call check_algorithm, and need the same removal when they next merge this. cargo test --workspace --no-fail-fast: 1106 passed, 1 failed. The failure is ascon_wycheproof, whose vector file is missing from the local wycheproof checkout; it fails the same way without this change. Assisted-by: Claude:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
officialfrancismendoza
marked this pull request as ready for review
October 22, 2026 09:53
dghgit
force-pushed
the
feature/key-rework-memory
branch
from
October 24, 2026 10:17
af15bb9 to
6c5e223
Compare
Contributor
Author
|
@dghgit recent merge from the latest bcgit:feature/key-rework-memory into here after the upstream force-push (72b3faa). Summary of changes:
Verification: All 217 CLI tests pass, along with workspace compilation, formatting, and documentation checks. The previous workspace test run had only the known environmental Ascon Wycheproof failure. |
officialfrancismendoza
marked this pull request as draft
October 26, 2026 13:07
…past the counter Driven through the hazmat API, apply_blocks could take next_counter past 2^32, after which remaining_blocks underflowed. In a release build it wrapped to about u64::MAX, so a ChaCha20 made over that keystream accepted unlimited data and re-emitted the counter-0 keystream; in a debug build it panicked on the subtraction instead. The per-block guard in apply_blocks is now an assert, which KeyStream allows on an overrun, and remaining_blocks saturates. read_state already refused a counter past 2^32. cargo mutants on hazmat.rs: 29 mutants, 24 caught, 5 unviable. It generates none inside the assert or for saturating_sub; deleting the assert, or making it <=, by hand is caught by the new should_panic test. With the assert, no reachable state tells saturating_sub from -. Assisted-by: Claude:claude-opus-5-5
into_generator and into_verifier consumed the key, then cloned its three Secrets into the worker through from_key(&self) and dropped the originals, and the key fields were declared on both types. The worker now holds the Poly1305Key itself, moved in by from_key(key), and process_block and finalize read r, r5 and pad through it. Both into_* names stay; the key still implements neither MAC key role, since it authenticates one message. size_of::<Poly1305>() is unchanged at 192, as the memory table says. Also #![no_std], with Vec from alloc for the MAC trait's mac and do_final, as QUALITY_AND_STYLE.md requires and the other two crates already have. Assisted-by: Claude:claude-opus-5-5
…buffered do_update copied every input byte through the 16-byte buffer and read it back, even for block-aligned input. process_block now takes the block and its high bit as arguments. do_update tops up a partial block left by an earlier call, feeds the whole blocks of the rest straight from the input through as_chunks, and buffers only the tail; finalize pads the partial block as before and passes it in. The arithmetic is unchanged. cargo bench, 16 KiB mac_out: 13.05 us before, 10.55 us after (criterion -13.7%, p = 0.00). The RFC 8439 vectors, the streaming tests and the AEAD suites pass, and a new test feeds one message in pieces of 1, 15, 16, 17 and 33 bytes and compares the tag with the one-shot tag. Assisted-by: Claude:claude-opus-5-5
…er has made do_encrypt_out checks the length itself, so that a refused call leaves the zero-filled output untouched, and is the only caller of encrypt_bytes, whose own check was therefore dead. decrypt_bytes keeps its check: do_decrypt_final_detached_out calls it without checking first. Assisted-by: Claude:claude-opus-5-5
The worker's suspended state is h as five u64 limbs, the buffered partial block and its length: 64 bytes. The one-time key is not in it. The component's Key is the raw 32-byte key, re-supplied on resume, which is how ChaCha20-Poly1305 uses it: it re-derives the key from block 0 of the nonce. Resume refuses a limb wider than process_block leaves it (26 bits, 27 for h[1]), which could also overflow the next block's products, and a whole buffered block. This is the component form only. A standalone SuspendableKeyed on Poly1305 has no sensible key to resume from: a Poly1305Key is consumed by the one worker it makes, so it cannot be re-supplied. Assisted-by: Claude:claude-opus-5-5
One impl per direction, as Ascon_AEAD128 has, each with its published length in the memory table: SUSPENDED_CHACHA20_POLY1305_ENCRYPTOR_STATE_LEN (176) and _DECRYPTOR_STATE_LEN (193). The state is the version header, the inner ChaCha20's component state, Poly1305's component state, the AAD and data counts, and a call-state byte whose non-zero marker carries the direction; the decrypting side adds the held-back bytes and their count. No key is in it: the key object is re-supplied, and the one-time Poly1305 key is re-derived from block 0 of the nonce, as new does. Resume refuses the other direction's state, a zeroed or unknown call state, data before the AAD phase ended or past the message limit, more held back than TAG_LEN, and a block counter out of step with the data, which a crafted state could otherwise use to repeat keystream, block 0 (the one-time key) included. ChaCha20's component resumes from a ChaCha20Key, so ChaCha20Poly1305Key now holds its key as one rather than as a bare ChaCha20KeyState, and starts each message's keystream through that key's create_decryptor; new takes the keystream. Nothing in the chacha20 crate changes. Tests: the framework suite per direction, through a Clone stand-in that holds a suspended state, since the worker is deliberately not Clone; suspend and resume mid-AAD and mid-data, on and inside block boundaries, against an uninterrupted run in both directions; each refusal; and, as a unit test, a worker at the message limit. Assisted-by: Claude:claude-opus-5-5
The poly1305 subcommand had no shell test. This one, in test_hmac.sh's shape, covers the round trip (a 32-hex-character tag that verifies with -v, and fails on a flipped tag byte, a flipped message byte and another key); RFC 8439 Appendix A.3 test vectors bcgit#1, the all-zero key, through load_mac_key's zeroized-key path with both --key and --key-file, and bcgit#4, copied from a fresh download of the RFC; and key files of 31 and 33 bytes, which the subcommand refuses, and of 65, which load_mac_key does. Also test_chacha20.sh and test_chacha20_poly1305.sh are now 100755, as every sibling is: run directly, they failed with "Permission denied". Assisted-by: Claude:claude-opus-5-5
…eature/officialfrancismendoza/145-chacha20-poly1305-AEAD-updated
…45-chacha20-poly1305-AEAD-updated
dghgit
added a commit
that referenced
this pull request
Oct 27, 2026
The call-state byte held only the phase, and the SuspendableKeyed impl is one impl generic over `Dir`, so a state suspended while encrypting resumed as a decryptor and the reverse. `Direction` gains `STATE_BITS` (0x10 encrypting, 0x20 decrypting), GCM writes them above the phase nibble and refuses a state whose bits are the other direction's. The layout and length are unchanged. Found by the PR #157 review follow-up; Ascon already refuses the other direction. Assisted-by: Claude:claude-mythos-5-1
…mendoza/145-chacha20-poly1305-AEAD-updated
Contributor
|
Francis, |
dghgit
reviewed
Oct 27, 2026
dghgit
left a comment
dghgit
left a comment
Contributor
There was a problem hiding this comment.
Three public items in bouncycastle-poly1305 that the API does not need; details inline. Everything else on the surface is what the traits and the key-object pattern require.
| /// Authentication tag length in bytes. | ||
| pub const TAG_LEN: usize = 16; | ||
| /// Message block length in bytes. | ||
| pub const BLOCK_LEN: usize = 16; |
Contributor
There was a problem hiding this comment.
BLOCK_LEN is public but used nowhere outside this crate. Poly1305's 16-byte block is a detail of the limb arithmetic rather than something a caller supplies, so it can be a private const.
| } | ||
|
|
||
| /// Consumes the key into the worker that checks the tag of its one message. | ||
| pub fn into_verifier(self) -> Poly1305 { |
Contributor
There was a problem hiding this comment.
into_verifier is a byte-for-byte copy of into_generator, and only this crate's tests call it. The brief's reason for keeping both (the MAC key roles) did not hold, as the fix report noted: a one-time key implements neither, by design. One method is enough, named for what it does, since the same worker both generates and verifies.
| } | ||
|
|
||
| /// Consumes this message and returns its full authentication tag without allocation. | ||
| pub fn finalize(mut self) -> [u8; TAG_LEN] { |
Contributor
There was a problem hiding this comment.
finalize is an inherent one-shot beside the MAC impl. The Ascon lessons in CLAUDE.md ask that the trait pair be the API, with inherent methods only for what the traits lack; MAC::do_final_out into a 16-byte array gives the same tag without allocation, and the AEAD's tag check can go through do_verify_final rather than comparing a returned array. Its only non-test caller is the AEAD crate, which is why it is public. Drop it, or if the array return is wanted for finish_tag, move it under hazmat.
Poly1305's 16-byte block is a detail of the limb arithmetic, not something a caller supplies, and nothing outside the crate used the constant. TAG_LEN and KEY_LEN stay public. Assisted-by: Claude:claude-opus-5-5
…or and into_verifier into_verifier was a byte-for-byte copy of into_generator, and the reason for keeping both, the MAC key roles, never applied: a one-time key implements neither. The one method is named for the trait its worker implements, since the same Poly1305 worker both computes a tag and checks one. It still consumes the key, so a key authenticates one message. The test comparing the two methods' workers goes with them. Assisted-by: Claude:claude-opus-5-5
…y do_verify_final The AEAD took its tag from Poly1305's inherent finalize, which the next commit makes private, since the MAC trait is the API. finish_mac absorbs the data's padding and the two lengths and returns the worker. Encrypting writes the tag with do_final_out into a TAG_LEN array, which cannot fail; decrypting checks it with do_verify_final, a constant-time comparison as the ct_eq_bytes here was. Assisted-by: Claude:claude-opus-5-5
finalize was an inherent one-shot beside the MAC impl, public only because the AEAD took its tag from it, which it now does through do_final_out and do_verify_final. It stays as the private body of the MAC impl's three finals, unchanged. The usage example writes the tag with do_final_out into a TAG_LEN array, and the tests use do_final. Assisted-by: Claude:claude-opus-5-5
Contributor
|
The suspend removal and the base conflict are done on The PR does not allow maintainer edits, so it still points at your fork's a76132e. Resetting your branch to |
Contributor
|
Finding 4 is done on the same bcgit branch, |
Contributor
|
Five of the smaller review items are done on |
Contributor
|
Constant-time equality on |
dghgit
approved these changes
Oct 31, 2026
dghgit
left a comment
dghgit
left a comment
Contributor
There was a problem hiding this comment.
There were a couple of minor items due to the left over SuspendableKey - I've cleaned it up here and left the rest of it to the planned sweep.
I couldn't update your branch so the updates are on:
feature/145-chacha20-poly1305
Further instructions are in the GitHub comments. Once the the branch is merged, or a new PR is created pointing at it. This one can be merged. For now we'll just stack it up with the other ciphers.
This branch has not been deployed
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
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.
Issue Link
Implements #145.
Summary
Based on RFC 8439, implement three separate chacha20, poly1305, and chacha20-poly1305-AEAD crates compatible with the new API shape (implements #145).
Description
bouncycastle-chacha20:ChaCha20Key(built once throughFromKeyMaterial, 256-bit strength) handing out theChaCha20<Dir>stream cipher through the symmetric role pair, with the raw keystream (ChaCha20KeyState,ChaCha20KeyStream::start_at) underhazmatfor protocols and vectors.bouncycastle-poly1305: a one-timePoly1305Key, made only byhazmat::key_from_bytes, consumed into the onePoly1305worker it makes, which implementsMACwith full 16-byte tags; neither key nor worker isClone, and the key carries neither MAC role, soMACFactoryrefuses the name.bouncycastle-chacha20-poly1305-aead:ChaCha20Poly1305Keyhanding outChaCha20_Poly1305<Encrypting>/<Decrypting>through the AEAD role pair, tag inline or detached, nonce drawn from the caller's RNG or supplied through the_nonceforms. The CLI gainschacha20,chacha20-poly1305andpoly1305on the existing stream, AEAD and MAC contracts, with the AEAD stream helpers made generic over the AEAD traits rather than GCM's nonce length. The umbrella crate re-exports all three.test_one_time_macfor a MAC with no reusable key), construction policy, the message and AAD limits through private state, RFC 8439 vectors from bc-test-data (crypto/rfc8439, committed there as 1cbfcdf), Wycheproofchacha20_poly1305_test.jsonwith the unsupported nonce/tag sizes counted and asserted, and shell tests for the three commands.cargo mutants: chacha20 40 mutants, 31 caught, 0 missed, 9 unviable; poly1305 202 mutants; chacha20-poly1305-aead 113 mutants, 69 caught, 2 missed, 42 unviable (figures in the commit bodies).mem_usage_benchesharness, under the exemption QUALITY_AND_STYLE's "should" allows: the crate docs givesize_oftables for every type, and there are no message-sized buffers or allocations to measure.Suspend and resume
The head implements
SuspendableKeyedonChaCha20_Poly1305andSuspendableComponentonChaCha20KeyStreamandPoly1305. Per the decision on #173 (2026-10-30), cipher workers do not suspend: holding the worker is the supported way to pause a message, and only hashes, XOFs, HMAC and HKDF keep suspend support. Those impls, their state-length constants and their tests are removed on bcgit'sfeature/145-chacha20-poly1305(dbe7607, 2026-10-31), which also merges the current base and closes the review's three resume findings; QUALITY_AND_STYLE's suspend sentence was amended for this at 4e8576b. The PR head will match once the fork's branch is reset to it.Scope and Risk
chacha20,poly1305,chacha20-poly1305-aead(new),core-test-framework,factory(test only),cli, the root manifest and umbrella crate, release notes. 32 files, +3824 / -27.aes*-gcmcommands now run through the generic AEAD helpers with the same framing._nonceforms document; a one-time Poly1305 key used twice, which the type system prevents for one key value and the hazmat constructor's docs put on the caller.Validation
With
../bc-test-dataand../wycheproofchecked out beside the repository, the RFC 8439 suites run 12 + 12 + 2 vectors and the Wycheproof suite 256 valid and 60 invalid cases.Review
Reviewed 2026-10-31 against QUALITY_AND_STYLE.md and CONTRIBUTING.md with RFC 8439 open. Four items before merge, all done on bcgit's
feature/145-chacha20-poly1305(54d8925): the three resume findings, resolved by dropping suspend support, andkeystream_atbuilding a throwaway decryptor to clone a keystream out ofChaCha20Key, replaced bychacha20::hazmat::keystream_atsoChaCha20KeyStreamis no longerClone. Of the nine smaller items in the review note, seven are done on the same branch (clamp-mask derivation, indexed loads andas_chunksfor the unwraps, the dead zero-fill, one limit check per call, the GCM forwarders, constant-time equality on the two keys, and this description's harness exemption) and one is dropped (an unboundedDirmatches every sibling worker); thestdfeature plumbing remains. The branch conflicts with the base only incli/src/mac_cmd.rsand carries pre-rebase history that a rebase ontofeature/key-rework-memoryshould drop.AI Usage Statement
Did you use AI in creating this pull request:
Assisted-by: Claude:claude-opus-5-5
Assisted-by: Claude:claude-mythos-5-1