Part Nirvana: Grand unification of stream and block encryptors under SymmetricCipherEncryptor/Decryptor, AES clean up - #115
Open
dghgit wants to merge 14 commits into
Open
Conversation
4 tasks
hubot
force-pushed
the
feature/stream-cipher
branch
from
September 8, 2026 03:56
93ee992 to
0404ab9
Compare
dghgit
force-pushed
the
feature/symmetric-cipher
branch
from
September 8, 2026 03:59
521efdc to
ca66411
Compare
dghgit
force-pushed
the
feature/stream-cipher
branch
from
September 9, 2026 01:25
0404ab9 to
4adbebb
Compare
…eme as well as a direction, via a PaddedMode projection, so the two block modes name their padding in the type
…ricCipherDecryptor with FINAL_LEN = 0, by blanket impls over the in-place methods, so any of the five modes can be held through one trait
…to AEADCipher, its only remaining user; the framework suite follows, and both AEAD security-strength loops gain the key-length guard the other suites already had
dghgit
force-pushed
the
feature/symmetric-cipher
branch
from
September 9, 2026 02:19
ca66411 to
8f932ca
Compare
…_2blocks (were *_blocks2), the reading Mike Ounsworth gave them in 736b0ac; modes, aes, the framework suite, benches and notes follow
… a debug self-check that sub_word's eight broadcast planes agree, in-place benches with decrypt paths for every key length and key-expansion throughput, summary.md removed, and acvp_tests.rs becomes bc-test-data.rs; the rest of Mike Ounsworth's 736b0ac review that still applied
…decrypt_8blocks (were *_blocks8), so they read like the pair methods; modes, the framework suite, benches and notes follow
…pt_4blocks (was eight): AES fills a pair and the u16/u32-plane engines fill four, so eight was two passes for every engine and left a four-lane engine half-empty on a 4-to-7-block tail; modes chunk fours, then pairs, then singles, the framework suite and the rotated-four toy pin the four path, benches and notes follow
…thods by their current name
…ipherEncryptor / SimpleCipherDecryptor; the framework suite becomes TestFrameworkSimpleCipher and the modes API test file is renamed to match; aes, padding, modes and the notes follow
…p Vim swap file that d98f703 swept in
Contributor
Author
|
Merge issue with of AES "Commit 736b0ac 2026-09-06 Mike Ounsworth MikeO adjustments to aes-lowmemory while The doc trimming across lib.rs, aes.rs, bitslice.rs, schedule.rs and the memory bench. It removes the spec-correspondence rationale that CLAUDE.md asks for." Checkout feature/symmetric-cipher, do the edit with claude and get it to update CLAUDE.md appropriately, I can then apply it across tdes, sm4, aria, and camellia as well. Suggest looking in lib.rs/aes.rs and identifying what shouldn't be there and just telling claude to remove it. Get it update CLAUDE.md, check one the other files, tell claude to clean it up, assuming all goes well it will have removed the excess documentation. |
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
No linked issue.
Summary
Brings every cipher in the workspace to one arbitrary-length API: the block modes reach
SymmetricCipherEncryptor/SymmetricCipherDecryptorthrough the padding adapters, the streammodes implement them directly, and the old one-shot-only
SymmetricCiphertrait is deleted.Description
Stacked on #113, so this PR is based on
feature/stream-cipherand shows six commits:7f465f4AES_CBC_*andAES_ECB_*take a padding scheme as well as a direction4f01a90SymmetricCipherEncryptor/SymmetricCipherDecryptor1f2a392SymmetricCipherdeleted, its one-shots moved ontoAEADCipher6b91279bouncycastle-aes-lowmemoryrenamed tobouncycastle-aes6c47625Block,PaddedModeand theAesParamstypes dropped from the public API521efdcElectronicCodeBookis the only public route to the permutationThe problem. Five modes had four different front doors.
CbcandEcbwere block-aligned andin-place;
Cfb,Cfb8andCtrwere stream ciphers taking any length; the padding adapterspresented a third shape; and
SymmetricCiphersat over the lot as a one-shot-only trait thatnothing implemented. Code that wanted to accept "a symmetric cipher" had no single trait to name.
The block modes name their padding.
AES_CBC_128<Dir>becomesAES_CBC_128<Dir, Pad>, and likewise for ECB, so a caller writesAES_CBC_128<Encrypting, PKCS7>orAES_ECB_256<Decrypting, NoPadding>. CBC and ECB are definedonly on whole blocks (SP 800-38A Sec 5.2), so on real data they are always mode plus scheme, and
the scheme changes the ciphertext and must be agreed by both ends. Putting it in the type makes a
mismatched pair a compile error instead of a decryption that returns plausible rubbish; there is a
test that a PKCS#7 ciphertext read back as
NoPadding"succeeds" with the wrong answer, which isthe failure this prevents.
PaddedEncryptorandPaddedDecryptorare distinct types, so a plain alias cannot select betweenthem on
Dir. A trait,PaddedMode, is implemented for the two direction markers andthe aliases project through it. It is crate-internal: a caller writes
AES_CBC_128<Encrypting, PKCS7>without ever naming it, and the projection resolves without the trait being reachable. One trait serves both modes: CBC passesINIT_DATA_LEN = BLOCK_LEN,ECB passes
0, and a future block mode needs an alias rather than another trait.The stream modes gain the same traits, by two blanket impls in core with
FINAL_LEN = 0, writtenover the in-place methods. An implementor still writes only
do_encrypt/do_decrypt, and a futureOFB gets the wider API for free. For a stream cipher
update_out_lenandencrypt_out_lenare theidentity,
decrypt_out_max_lenis exact rather than an upper bound, anddo_finalhas nothing toproduce.
SymmetricCipheris deleted. Its four one-shots move toAEADCipher, which was its onlyremaining user, so
AEADCipherdrops the supertrait and declares them itself againstNONCE_LEN.This closes the
todothat sat above the trait asking for exactly this. Their documentation isrewritten for the AEAD case: no additional authenticated data, and a ciphertext layout that is the
implementation's business, because the tag has to go somewhere.
Alternatives considered.
clarity. Folding the scheme into the existing name was chosen so the short name is the one that
handles real data; the block-aligned API is still reachable as
modes::Cbc/modes::Ecb.same six methods three times and on covering future modes.
Scope and Risk
Packages impacted:
bouncycastle-core(blanket impls,SymmetricCipherdeleted,AEADCiphergains four methods),
bouncycastle-core-test-framework(a suite moves, guards added),bouncycastle-aes(alias shapes, newbouncycastle-paddingdependency).Breaking changes, all compile-time:
AES_CBC_*andAES_ECB_*take two parameters, and are the arbitrary-length API rather than theblock one. Existing uses do not compile.
modes::Cbcandmodes::Ecbare unchanged for callerswho want block-aligned, in-place, compile-time-checked lengths.
SymmetricCipherno longer exists. Nothing in the workspace implemented it.do_encrypt_initthrough two traits with identical signatures, so code withboth in scope must qualify the call. Nothing in the workspace hit this; the new test file is
written that way on purpose to show it is workable.
Likelihood of regression: low. No cipher's ciphertext changes: the modes themselves are
untouched, and the padding layer and the blanket impls are wiring over existing, tested code. The
1853 ACVP CTR, 2138 CFB128, 2138 CFB8 and 2150 CBC vectors all still pass unchanged.
Worst case: the blanket impls sit on the path of every stream-cipher call made through the wider
trait, so a mistake there would be a wrong-ciphertext bug rather than a compile error. That is why
the new tests check the separate-output API against the in-place one byte for byte, and why all
three stream modes now run the same conformance suite the padded adapters run.
Validation
Seventeen new tests: ten on the aliases, seven on the stream modes through the wider traits.
Mutation testing needs
--test-workspacefor anything incore, because core has no implementorsof its own traits and a plain
-p bouncycastle-corerun reports long-standing, well-tested methodsas missed. Scoped to the blanket impls and run that way:
The four missed are one equivalent mutant repeated,
[]against[0; 0]and[1; 0]for azero-length array, which are the same value; both sites carry a comment saying so. The run also
found a genuinely uncovered mutant, the decryptor's output-buffer length comparison, which now has a
test. The
SymmetricCipherremoval has no executable code to mutate.Also fixed, because this PR moved the code: two security-strength loops in the test framework
unwrapped
set_security_strengthat all five strengths, which a key shorter than 32 bytes cannotcarry, so they would have panicked for the first
AEADCipherimplementor — ASCON-128 andAES-128-GCM among them. This was recorded as outstanding in
core-test-framework/summary.md.Relocating one of them into a method the AEAD suite calls would have made it worse, so both now
carry the same key-length guard the block and stream suites already had. Every strength loop in the
file is guarded, and the summary is updated.
The AES crate: renamed, and its public API narrowed
Three follow-up commits, added after review of the crate's surface.
Renamed
bouncycastle-aes-lowmemorytobouncycastle-aes: directory, package, umbrellare-export (
bouncycastle::aes) and criterion group prefix. There is one AES in the workspace, sothe qualifier described a property rather than distinguishing the crate from anything, and the
low-memory design it referred to is unchanged. Pure substitution, 108 lines each way. The crate has
not been published under the old name, so the release notes simply introduce it under the new one.
Six items left the public API —
Block,PaddedMode,AesParams,Aes128Params,Aes192Params,Aes256Params— after checking every import of the crate across the workspace.Nothing outside
crypto/aes/src/names any of them. They staypubinside private modules ratherthan becoming
pub(crate): the latter makes theprivate_interfaceslint fire onpub type Aes128 = Aes<Aes128Params>and on all fifteen mode aliases, and that lint is worthkeeping. Unreachable-
pubremoves them from rustdoc with a clean build.ElectronicCodeBookis now the only public route to the permutation.Aescarriedencrypt_block,decrypt_block,encrypt_blocks2,decrypt_blocks2and threenewmethods asinherent
pub fns that the trait impls duplicated exactly — the impls were one-line delegations, sothere were two names for every operation and the inherent one shadowed the trait at the call site.
All seven are now
pub(crate). Because the names match, method resolution falls through to thetrait: the ~40 call sites in the ACVP, FIPS 197 and SP 800-38A suites, the benches and the
stack-memory harness are unchanged, and five files gained a
useline. With nothing public left onit,
Aesitself is no longer exported;type.Aes128.htmlcarries the full trait documentation, sonothing became callable-but-undiscoverable.
The crate's public API is now twenty items: three engine aliases, fifteen mode aliases, and
BLOCK_LEN/CTR_NONCE_LEN.Verified at each commit: 889 tests pass, the workspace builds with
--all-targetsand no warnings,cargo fmt --all --checkis clean, rustdoc is clean under-D warnings, andaes128-ecbthroughthe CLI matches OpenSSL byte for byte.
unwrapandErr()counts are unchanged.AI Usage Statement
Did you use AI in creating this pull request:
If submitted code changes were generated by AI, fill in the following declaration:
Assisted-by: Claude Code:claude-fable-5-1
Assisted-by: Claude Code:claude-opus-5