Part 7: Stream cipher traits, CFB as a stream cipher, and new CFB8 and CTR modes

.gitignore

@@ -5,3 +5,13 @@ mutants.out*/
 
 .idea/
 .vscode/
+
+# Claude Code: ignore personal/local state, but share team tooling
+# (skills, slash commands, subagents, and project settings.json).
+.claude/*
+!.claude/settings.json
+!.claude/skills/
+!.claude/commands/
+!.claude/agents/
+.claude/settings.local.json
+.claude 2/

CLAUDE.md

@@ -33,12 +33,15 @@ fixed.
 
 ## Common commands
 
-Build / test / bench / docs run against the cargo workspace from the repo root:
+Build / test / bench / docs run against the cargo workspace from the repo root. `--workspace` is
+not optional: the root manifest is both the workspace and the umbrella `bouncycastle` package, so a
+bare `cargo build` builds only that package (no `cli`, no benches) and a bare `cargo test` runs
+**zero** tests and still exits 0, because the umbrella crate has none of its own.
 
 ```
-cargo build                     # whole workspace incl. `bc-rust` CLI binary
+cargo build --workspace         # whole workspace incl. `bc-rust` CLI binary
 cargo build -p bouncycastle-sha3   # one sub-crate
-cargo test                      # all tests
+cargo test --workspace          # all tests
 cargo test -p bouncycastle-mlkem   # tests for one crate
 cargo test -p bouncycastle-mlkem ml_kem_tests   # one integration test file
 cargo bench --all               # all criterion benches
@@ -54,19 +57,25 @@ Quality / mutation testing:
 cargo mutants                              # config in .cargo/mutants.toml (output: custom_mutants_output/)
 ```
 
-Stack-memory benches are separate binaries under `mem_usage_benches/`:
+Stack-memory benches are separate binaries under `mem_usage_benches/src/`, each declared as a
+`[[bin]]` in that crate's `Cargo.toml`:
 
 ```
 cargo run --release -p mem_usage_benches --bin bench_mlkem_mem_usage
 cargo run --release -p mem_usage_benches --bin bench_mldsa_mem_usage
 ```
 
+`mem_usage_benches/src/lib.rs` makes those sources modules of a lib target as well, so their `//!`
+headers are rustdoc'd and any indented or fenced block in them is compiled as a Rust doctest. The
+valgrind and `ms_print` recipes there are fenced as ```` ```text ```` for that reason — keep it that
+way when adding a harness, or `cargo test --workspace` fails to compile them.
+
 ## Workspace architecture
 
 The workspace has three top-level kinds of member:
 
-1. `crypto/*` — one sub-crate per primitive (`sha2`, `sha3`, `hmac`, `hkdf`, `mlkem`, `mlkem_lowmemory`, `mldsa`, `mldsa_lowmemory`, `rng`, `hex`, `base64`, `utils`) plus the spine crates `core`, `core-test-framework`, and `factory`. Each crate is published as `bouncycastle-<name>` and depended on internally via the `workspace.dependencies` table in the root `Cargo.toml`.
-2. `src/` — the umbrella `bouncycastle` crate, which is just `pub use` re-exports of every sub-crate (e.g. `bouncycastle::sha3`, `bouncycastle::mlkem`). It exists so downstream users can pull the whole library with one dependency; it has no code of its own.
+1. `crypto/*` — one sub-crate per primitive (`sha2`, `sha3`, `sm3`, `hmac`, `hkdf`, `mlkem`, `mlkem_lowmemory`, `mldsa`, `mldsa_lowmemory`, `rng`, `hex`, `base64`, `utils`) plus the spine crates `core`, `core-test-framework`, and `factory`. Each crate is published as `bouncycastle-<name>` and depended on internally via the `workspace.dependencies` table in the root `Cargo.toml`.
+2. `src/` — the umbrella `bouncycastle` crate, which is just `pub use` re-exports of every sub-crate (e.g. `bouncycastle::sha3`, `bouncycastle::sm3`, `bouncycastle::mlkem`). It exists so downstream users can pull the whole library with one dependency; it has no code of its own.
 3. `cli/` — the `bc-rust` binary built on top of `bouncycastle`, exposing every primitive as a streaming stdin→stdout subcommand using `clap`.
 4. `mem_usage_benches/` — stand-alone binary crates that measure peak stack usage of algorithms (cannot be done via criterion).
 
@@ -109,6 +118,19 @@ Repo mechanics behind those rules, which the documents don't spell out:
   pipelines; preserve that when adding subcommands.
 - Trait → factory → CLI is the wiring path for a new primitive; see [the workspace architecture](#the-core--core-test-framework--factory-spine) above for the crates involved.
 
+## Scope of changes
+
+Implement what was asked and stop. Unrequested refactors — extracting a trait, renaming for
+readability, restructuring impls — are not free even when they are correct: bundled into a feature
+commit they make the diff unreviewable, because a reviewer cannot separate the new behaviour from
+the restructuring, and the review time that costs is the reason not to do it.
+
+- If a refactor genuinely unblocks the task, give it **its own commit ahead of** the feature, so it
+  can be reviewed or dropped on its own.
+- If it unblocks nothing, propose it and wait rather than doing it.
+- The same goes for drive-by comment rewrites, reformatting and file moves in code you are only
+  passing through.
+
 ## Working from specifications
 
 QUALITY_AND_STYLE.md is where the requirement for spec-corresponding comments and justified deviations lives. This
@@ -144,10 +166,11 @@ What a crate must be tested against — including the mutation-testing expectati
 external vector suites — is specified in QUALITY_AND_STYLE.md and CONTRIBUTING.md. Repo-specific mechanics:
 
 - `cargo mutants` is expected to be run on each crate; surviving mutants must be investigated but not all need to die (e.g. XOR/OR equivalences in crypto code are acceptable). Config lives in `.cargo/mutants.toml` (output dir `custom_mutants_output/`).
-- Behaviour-critical private functions can use in-file `#[cfg(test)] mod tests` blocks when they can't be exercised from outside the crate.
+- Integration tests in `tests/` are preferred over in-file `#[cfg(test)] mod tests` blocks — see "Unit tests vs integration tests" in QUALITY_AND_STYLE.md for the reasoning and the exceptions. A unit test is justified for high-risk code that has known-answer values and cannot be reached through the public API; when you write one, all of its helpers go inside that `mod tests`.
+- A property that can be asserted at compile time (`const _: () = assert!(...)`) stays a compile-time assertion even when a test also covers it: `cargo mutants` cannot see a const assertion fail, so pair the two rather than trading the guarantee for the coverage.
 - For traits in `core`, the canonical tests live in `core-test-framework` and are invoked from each implementor's integration tests — don't duplicate them per-implementation.
 - The per-width `impl Condition<W>` blocks in `crypto/utils/src/ct.rs` (and their test modules) are deliberately duplicated rather than macro-generated: `cargo mutants` cannot see into `macro_rules!` bodies, so a macro would hide the mask identities from mutation testing. Do not fold them back into a macro. Any change to one width in a group (i64/i32, u64/u32) must be applied to every width in that group.
 
 ## CI
 
-The only workflow is `.github/workflows/publish_doc_benches_to_ghpages.yaml`: on every PR it builds rustdoc and runs `quality_stats.sh`; on `main` it additionally runs `cargo bench --all` and publishes docs, code stats, and benchmark results to GitHub Pages (`https://bcgit.github.io/bc-rust/`). There is no separate CI test/lint job — local `cargo test` is the gate.
+The only workflow is `.github/workflows/publish_doc_benches_to_ghpages.yaml`: on every PR it builds rustdoc and runs `quality_stats.sh`; on `main` it additionally runs `cargo bench --all` and publishes docs, code stats, and benchmark results to GitHub Pages (`https://bcgit.github.io/bc-rust/`). There is no separate CI test/lint job — local `cargo test --workspace` is the gate, and nothing but a developer running it stands between a broken test and `main`.

Cargo.toml

@@ -9,7 +9,9 @@ version = "0.1.3"
 
 # *** Internal Dependencies ***
 bouncycastle = { path = "./" }
+bouncycastle-aes-lowmemory = { path = "./crypto/aes-lowmemory" }
 bouncycastle-base64 = { path = "./crypto/base64" }
+bouncycastle-modes = { path = "./crypto/modes" }
 bouncycastle-core = { path = "crypto/core" }
 bouncycastle-core-test-framework = { path = "./crypto/core-test-framework" }
 bouncycastle-factory = { path = "./crypto/factory" }
@@ -20,9 +22,11 @@ bouncycastle-mlkem = { path = "./crypto/mlkem" }
 bouncycastle-mlkem-lowmemory = { path = "./crypto/mlkem-lowmemory" }
 bouncycastle-mldsa = { path = "./crypto/mldsa" }
 bouncycastle-mldsa-lowmemory = { path = "./crypto/mldsa-lowmemory" }
+bouncycastle-padding = { path = "./crypto/padding" }
 bouncycastle-rng = { path = "./crypto/rng" }
 bouncycastle-sha2 = { path = "./crypto/sha2" }
 bouncycastle-sha3 = { path = "./crypto/sha3" }
+bouncycastle-sm3 = { path = "./crypto/sm3" }
 bouncycastle-utils = { path = "./crypto/utils" }
 
 
@@ -41,6 +45,7 @@ version.workspace = true
 edition.workspace = true
 
 [dependencies]
+bouncycastle-aes-lowmemory.workspace = true
 bouncycastle-base64.workspace = true
 bouncycastle-core.workspace = true
 bouncycastle-factory.workspace = true
@@ -51,6 +56,9 @@ bouncycastle-mldsa.workspace = true
 bouncycastle-mldsa-lowmemory.workspace = true
 bouncycastle-mlkem.workspace = true
 bouncycastle-mlkem-lowmemory.workspace = true
+bouncycastle-modes.workspace = true
+bouncycastle-padding.workspace = true
 bouncycastle-rng.workspace = true
 bouncycastle-sha2.workspace = true
 bouncycastle-sha3.workspace = true
+bouncycastle-sm3.workspace = true

QUALITY_AND_STYLE.md

@@ -128,6 +128,36 @@ Note that rust macros tend not to play well with a lot of dev tooling for compil
 `cargo mutants`, which is a good reason to avoid macros in core algorithm or data processing code. Macros can be used
 more freely within test code.
 
+## Unit tests vs integration tests
+
+Unit tests are test code (and supporting helper functions) embedded in src/**.rs files. They have access to
+crate-private or module-private functions and constants.
+
+Integration tests are test code (and supporting helper functions) in tests/**.rs files. They test the crate's code from
+the outside -- ie through its public APIs -- since tests/ is a separate crate from src/.
+
+In general, integration tests are preferred over unit tests. This is for a number of reasons:
+
+* To reduce reviewer burden; reviewers will typically focus more effort on the src/ than the tests/, so we want to keep
+  src/ as short as is reasonable.
+* Usually it is easier to determine what is the correct behaviour at the public API level. For example, this is the
+  level at which we typically have KATs and test vectors.
+* Tools like cargo mutants are very helpful at detecting branches that are not exercisable via the public APIs, which
+  often is an indicator that the branch isn't doing what you think it's doing, or is simply not useful and can be
+  deleted. Unit tests that bypass the public APIs to pin these sorts of branches obscure the fact that this code is
+  unreachable.
+
+Unit tests are reasonable to include in the following cases:
+
+* There is high-risk code (usually meaning that it is complex code whose behaviour is not obvious from inspection) where
+  unit tests help to document the behaviour and protect against accidental breakage via a benign-looking change.
+* AND where known answer tests are available.
+* AND where this behaviour cannot be tested from integration tests.
+
+When writing unit tests, they should be contained with an `mod tests` at the bottom of the file, and ALL helper
+functions that support the unit tests must be contained within that module. The intention is to clearly signal to a code
+reviewer what is test code vs functional code.
+
 # Docs
 
 ## Usage Examples

alpha_0.1.3_release_notes.md

@@ -2,8 +2,20 @@
 
 ## Major features
 
+* New algorithms added to crypto/ :
+    * SM3 -- the SM3 hash (GB/T 32905-2016 / ISO/IEC 10118-3:2018), ported from bc-java.
+    * AES -- AES-128/192/256, along with its modes AES_ECB, AES_CBC, AES_GCM.
+
 ## Minor features / bug fixes
 
-* bug fixes to the way SHA3/SHAKE handled absorbing and squeezing a partial final byte.
 * Design discussions about whether core::traits::XOF (in the abstract) should allow interleaving absorb -> squeeze ->
   absorb (ie "absorb-after-squeeze). Outcome: absorb-after-squeeze forbidden. Could be changed in the future.
+* SHA2:
+    * Implemented SHA512/224 and SHA512/256.
+    * `Hash::do_final_partial_bits()` / `do_final_partial_bits_out()` are now implemented for SHA-2 (FIPS 180-4 s. 5.1).
+* SHA3:
+    * Fixed a bug in `XOF::squeeze_partial_byte_final()`: when it was the first squeeze it bypassed the SHAKE `1111`
+      domain suffix and returned raw Keccak output, and it returned the wrong `num_bits` bits of the output byte. The
+      existing test used
+      `0xFF`, which masked the second error.
+    * Changed the order of bits when absorbing a final partial byte to match ASN.1 DER BIT_STRING bit ordering.

cli/src/aes_cbc_cmd.rs

@@ -0,0 +1,68 @@
+//! AES-CBC encryption and decryption, streaming stdin to stdout.
+//!
+//! Only the mode wiring lives here: the IV convention, key loading, stdin framing and
+//! block-alignment enforcement are all in [`crate::block_mode_cmd`], shared with the `aes*-cfb` and
+//! `aes*-ecb` commands. See that module for the command-line contract.
+//!
+//! CBC (NIST SP 800-38A Sec 6.2) provides confidentiality only. It does not detect tampering, and
+//! neither the ciphertext nor the IV is authenticated -- a flipped ciphertext bit flips the same bit
+//! of the *next* block's plaintext (Appendix D). Do not decrypt data you have not authenticated
+//! separately.
+
+use crate::block_mode_cmd::{BLOCK_LEN, BlockModeAction, decrypt_stream, encrypt_stream, load_key};
+use bouncycastle::aes_lowmemory::{Aes128, Aes192, Aes256};
+use bouncycastle::core::key_material::KeyMaterial;
+use bouncycastle::core::traits::ElectronicCodeBook;
+use bouncycastle::modes::{Cbc, Decrypting, Encrypting};
+
+/// Names the mode in error messages.
+const MODE: &str = "CBC";
+
+pub(crate) fn aes128_cbc_cmd(
+    action: &BlockModeAction,
+    key: &Option<String>,
+    key_file: &Option<String>,
+    output_hex: bool,
+) {
+    run::<Aes128, 16>(action, &load_key::<16>(key, key_file, "AES-128"), output_hex);
+}
+
+pub(crate) fn aes192_cbc_cmd(
+    action: &BlockModeAction,
+    key: &Option<String>,
+    key_file: &Option<String>,
+    output_hex: bool,
+) {
+    run::<Aes192, 24>(action, &load_key::<24>(key, key_file, "AES-192"), output_hex);
+}
+
+pub(crate) fn aes256_cbc_cmd(
+    action: &BlockModeAction,
+    key: &Option<String>,
+    key_file: &Option<String>,
+    output_hex: bool,
+) {
+    run::<Aes256, 32>(action, &load_key::<32>(key, key_file, "AES-256"), output_hex);
+}
+
+/// Dispatches to the shared streaming loops with `Cbc` filled in as the mode.
+fn run<P, const KEY_LEN: usize>(
+    action: &BlockModeAction,
+    key: &KeyMaterial<KEY_LEN>,
+    output_hex: bool,
+) where
+    P: ElectronicCodeBook<KEY_LEN, BLOCK_LEN>,
+{
+    match action {
+        BlockModeAction::Encrypt => {
+            encrypt_stream::<Cbc<P, Encrypting, KEY_LEN, BLOCK_LEN>, KEY_LEN, BLOCK_LEN>(
+                key, output_hex, MODE,
+            )
+        }
+        BlockModeAction::Decrypt => {
+            decrypt_stream::<Cbc<P, Decrypting, KEY_LEN, BLOCK_LEN>, KEY_LEN, BLOCK_LEN>(
+                key, output_hex, MODE,
+            )
+        }
+    }
+}

cli/src/aes_cfb8_cmd.rs

@@ -0,0 +1,76 @@
+//! AES-CFB8 encryption and decryption, streaming stdin to stdout.
+//!
+//! Only the mode wiring lives here: the IV convention, key loading and stdin framing are in
+//! [`crate::stream_mode_cmd`] (and [`crate::block_mode_cmd`] for the key loader), shared with the
+//! `aes*-cfb` commands. See those modules for the command-line contract.
+//!
+//! # Which CFB
+//!
+//! These commands are **CFB8**: the segment size is one byte (`s = 8` in NIST SP 800-38A Sec 6.3).
+//! That is a different, non-interoperable mode from the CFB128 of `aes*-cfb`, not a variant of it:
+//! the two ciphertexts agree on their first byte and differ everywhere after it. It also costs a
+//! full AES call per byte of data, sixteen times the work of `aes*-cfb`, so prefer `aes*-cfb`
+//! unless a byte-granular self-synchronising stream is required or the format demands CFB8.
+//!
+//! # Any length
+//!
+//! CFB8's segment is a single byte, so these commands accept input of any length, pad nothing, and
+//! emit a ciphertext exactly as long as the plaintext.
+//!
+//! # Warning
+//!
+//! CFB8 provides confidentiality only. It does not detect tampering, and neither the ciphertext nor
+//! the IV is authenticated. Appendix D, Table D.2 gives "SBE in the decryption of Cj" plus random
+//! errors in the next `b/s` segments: flipping a ciphertext bit flips the *same* bit of the *same*
+//! plaintext byte, corrupts the following 16 bytes, and then decryption resynchronises. Do not
+//! decrypt data you have not authenticated separately.
+
+use crate::block_mode_cmd::{BLOCK_LEN, BlockModeAction, load_key};
+use crate::stream_mode_cmd::run_stream_mode;
+use bouncycastle::aes_lowmemory::{Aes128, Aes192, Aes256};
+use bouncycastle::core::key_material::KeyMaterial;
+use bouncycastle::core::traits::ElectronicCodeBook;
+use bouncycastle::modes::{Cfb8, Decrypting, Encrypting};
+
+pub(crate) fn aes128_cfb8_cmd(
+    action: &BlockModeAction,
+    key: &Option<String>,
+    key_file: &Option<String>,
+    output_hex: bool,
+) {
+    run::<Aes128, 16>(action, &load_key::<16>(key, key_file, "AES-128"), output_hex);
+}
+
+pub(crate) fn aes192_cfb8_cmd(
+    action: &BlockModeAction,
+    key: &Option<String>,
+    key_file: &Option<String>,
+    output_hex: bool,
+) {
+    run::<Aes192, 24>(action, &load_key::<24>(key, key_file, "AES-192"), output_hex);
+}
+
+pub(crate) fn aes256_cfb8_cmd(
+    action: &BlockModeAction,
+    key: &Option<String>,
+    key_file: &Option<String>,
+    output_hex: bool,
+) {
+    run::<Aes256, 32>(action, &load_key::<32>(key, key_file, "AES-256"), output_hex);
+}
+
+/// Dispatches to the shared streaming loops with `Cfb8` filled in as the mode.
+fn run<P, const KEY_LEN: usize>(
+    action: &BlockModeAction,
+    key: &KeyMaterial<KEY_LEN>,
+    output_hex: bool,
+) where
+    P: ElectronicCodeBook<KEY_LEN, BLOCK_LEN>,
+{
+    run_stream_mode::<
+        Cfb8<P, Encrypting, KEY_LEN, BLOCK_LEN>,
+        Cfb8<P, Decrypting, KEY_LEN, BLOCK_LEN>,
+        KEY_LEN,
+        BLOCK_LEN,
+    >(action, key, output_hex)
+}