Hazmat namespace for the raw primitives: ElectronicCodeBook and KeyStream

[dghgit]

Name: Move ElectronicCodeBook, KeyStream and their implementors under hazmat modules

By way of background, hazmat is a convention that was landed on by the Rust Crypto project originally, for dealing with things that had to be public but that they wanted to label clearly as being risky or dangerous (for some reason it seems they felt the rest of us usually ignore what's in the documentation...). The following is a outline and plan of where the current code base is at and what should probably be marked with hazmat.

Comments welcome and encouraged.

Summary

ElectronicCodeBook and KeyStream are the two traits in bouncycastle-core that a caller can use
correctly and still end up with no security: a raw block permutation applied to data is ECB, and a
raw keystream with a caller-chosen nonce is a two-time pad waiting to happen. Today they sit in
bouncycastle_core::traits next to Hash, KEM and Signer, and their implementors sit at
ordinary paths (bouncycastle_aes::aes_internal::AES128Internal,
bouncycastle_modes::ctr::CtrKeyStream), distinguished only by a 🚨 Security 🚨 doc section.
This issue moves them under a hazmat module in each crate that has one, so that the path itself
says what the docs currently say, and a downstream can grep or lint for every use.

Details

What "hazmat" means in this library

This goes, once, in the bouncycastle_core::hazmat module docs; everything else links to it.

An item is hazmat when it is a correct, tested primitive whose composition is the caller's job,
and a wrong composition fails silently: the code compiles, runs and produces output, and the output
is insecure. Nothing under hazmat is a cipher for data. The supported uses are:

1. implementing a mode or construction that is generic over the trait (bouncycastle-modes,
   the CLI's generic run::<P, KEY_LEN> loops);
2. known-answer tests and vector harnesses;
3. a specification that mandates the raw operation: SP 800-38F key wrap, CMAC subkey generation,
   a protocol that fixes the nonce.

Everything outside hazmat keeps the INTRODUCTION.md contract, "if it compiles, then it's safe".
hazmat is the one place where that contract is suspended, and the path is the notice.

Design decisions

The boundary is the module path, not a cargo feature. bouncycastle-modes, bouncycastle-aes
and cli all need the traits, so a hazmat feature would be enabled by them and, through Cargo
feature unification, be on in every downstream build that uses any mode. It would be ceremony
without a boundary. The path is what gives a downstream something to act on: grep -rn hazmat
finds every raw-primitive use in an audit, and clippy's disallowed-types can name
bouncycastle_core::hazmat::ElectronicCodeBook in a project that wants to forbid it outright.
This is the pyca/cryptography and RustCrypto (aes::hazmat, ed25519_dalek::hazmat) model minus
the feature gate, for the reason above.

No #[doc(hidden)]. These are supported extension points. Hiding them hides the warnings.

Names stay. The trait names are right. AES128Internal stays AES128Internal; renaming the
permutation types is a separate decision that collides with the spec-capitalisation naming on the
per-cipher branches, and hazmat::AES128Internal reads fine.

One hazmat module per crate that has hazmat items, never a hazmat crate: the items belong
with the crate that owns them, and a separate crate would be a second dependency for the modes
crate to carry.

The safe adapters stay where they are. bouncycastle_core::stream_cipher::StreamCipher is the
thing that makes a KeyStream safe; it is not hazmat. Its from_keystream constructor bypasses
nonce generation, but it can only be called by someone already holding a KeyStream, so the hazmat
path is already on the way in.

Step 1: bouncycastle-core, move the two traits into core::hazmat

New file crypto/core/src/hazmat.rs, holding the module docs above and the two traits, moved
verbatim from traits.rs. Their existing 🚨 Security 🚨 sections shrink to what is specific to
each trait (nonce reuse and remaining_blocks for KeyStream; the codebook property for
ElectronicCodeBook) with the general contract left to the module docs, so each fact has one home.

// crypto/core/src/lib.rs
 pub mod errors;
+pub mod hazmat;
 pub mod key_material;

// crypto/core/src/hazmat.rs
//! Raw primitives whose safe use is the caller's responsibility.
//!
//! An item lives here when it is a correct, tested primitive whose *composition* is the caller's
//! job, and a wrong composition fails silently: the code compiles, runs and produces output, and
//! the output is insecure. Nothing here is a cipher for data. [...supported uses, as above...]
//!
//! Everything outside this module keeps the library's "if it compiles, then it's safe" contract.

use crate::errors::SymmetricCipherError;
use crate::key_material::KeyMaterial;
use crate::traits::Algorithm;

/// A keyed block permutation: the `CIPH_K` / `CIPH^-1_K` of NIST SP 800-38A Sec 5.1.
/// ...
pub trait ElectronicCodeBook<const KEY_LEN: usize, const BLOCK_LEN: usize>: Algorithm + Sized { ... }

/// A keyed keystream generator: the raw primitive under a stream cipher, as
/// [`ElectronicCodeBook`] is the raw primitive under a block cipher mode.
/// ...
pub trait KeyStream<const KEY_LEN: usize, const INIT_DATA_LEN: usize, const BLOCK_LEN: usize>: Algorithm + Sized { ... }

traits.rs loses about 120 lines. The docs that remain there and mention the traits (the
StreamCipherEncryptor docs' "should implement KeyStream and use StreamCipher" advice, and
the BlockCipherEncryptor docs) link to crate::hazmat::.... stream_cipher.rs changes its
import and its intra-doc links.

Every consumer changes one use line. There are 29 files today:

// crypto/modes/src/cbc.rs, and likewise cfb.rs, cfb8.rs, ctr.rs, ccm.rs, gcm.rs, ecb.rs
-use bouncycastle_core::traits::{Algorithm, BlockCipherDecryptor, BlockCipherEncryptor, ElectronicCodeBook, RNG};
+use bouncycastle_core::hazmat::ElectronicCodeBook;
+use bouncycastle_core::traits::{Algorithm, BlockCipherDecryptor, BlockCipherEncryptor, RNG};

// cli/src/aes_cbc_cmd.rs, and the other seven aes_*_cmd.rs and aead_mode_cmd.rs
-use bouncycastle::core::traits::ElectronicCodeBook;
+use bouncycastle::core::hazmat::ElectronicCodeBook;

// crypto/core-test-framework/src/electronic_code_book.rs and key_stream.rs
-use bouncycastle_core::traits::ElectronicCodeBook;
+use bouncycastle_core::hazmat::ElectronicCodeBook;

This is one commit across the workspace. A trait move cannot be split by crate without a temporary
re-export at the old path, and the alpha rules say no shims. The commit is reviewable by
git diff -M --stat: three files with content changes (lib.rs, hazmat.rs, traits.rs) and
the rest single-line use edits.

Step 2: bouncycastle-aes, aes_internal becomes hazmat

crypto/aes/src/aes_internal.rs is renamed to crypto/aes/src/hazmat.rs. Its module docs get a
first paragraph that says why it is here and links bouncycastle_core::hazmat; the existing
struct doc "This needs to be pub for the type aliases to work, but this is only a building-block
... not intended to be used directly" is replaced, because it is wrong: composing modes over it
generically is exactly what the CLI does, and that is a supported use.

// crypto/aes/src/lib.rs
-pub mod aes_internal;
+/// The AES block length in bytes: 16 (FIPS 197 Sec 3.4, `Nb` = 4 words).
+pub const BLOCK_LEN: usize = 16;
+
+pub mod hazmat;
 ...
-pub use aes_internal::BLOCK_LEN;

BLOCK_LEN moves to the crate root so that nothing non-hazardous is defined under hazmat. The
impl ElectronicCodeBook for AES128Internal blocks stay in ecb.rs where they are today; moving
them is not part of this change.

The crate docs' "Security Considerations" section, "A block permutation is not a cipher", keeps
its point and links the hazmat path instead of restating the general contract.

Path users: 61 files, 86 lines, all of the form

-use bouncycastle_aes::aes_internal::{AES128Internal, AES192Internal, AES256Internal};
+use bouncycastle_aes::hazmat::{AES128Internal, AES192Internal, AES256Internal};

in crypto/aes/{src,tests,benches}, crypto/modes/{src,tests,benches} (doctests included; the
Ecb and Cbc usage examples construct AES128Internal), cli/src, and mem_usage_benches.

Step 3: bouncycastle-modes, CtrKeyStream moves to modes::hazmat

CtrKeyStream is currently "deliberately not re-exported from the crate root", so it lives at
bouncycastle_modes::ctr::CtrKeyStream. That is a hiding place, not a notice. The struct, its
KeyStream impl, its start/start_at constructors and the mod tests that pin start_at move
to a new crypto/modes/src/hazmat.rs. ctr.rs keeps the Ctr alias, the module docs, and the
crate-private apply_counter_blocks that CCM shares.

// crypto/modes/src/lib.rs
 pub mod gcm;
+pub mod hazmat;

// crypto/modes/src/gcm.rs
-use crate::ctr::CtrKeyStream;
+use crate::hazmat::CtrKeyStream;

// crypto/modes/tests/ctr_tests.rs
-use bouncycastle_modes::ctr::CtrKeyStream;
+use bouncycastle_modes::hazmat::CtrKeyStream;

CcmKeyStream stays pub(crate); a crate-private type needs no notice. start_at stays
pub(crate).

Step 4: docs that define the convention

• QUALITY_AND_STYLE.md, under "APIs": a short paragraph. "A primitive whose safe use depends on
  the caller composing it correctly lives under a hazmat module in its crate, never at the crate
  root or next to the safe API; bouncycastle_core::hazmat defines the term. A crate's Security
  Considerations section names what it puts there."
• INTRODUCTION.md, "If it compiles, then it's safe": one paragraph naming hazmat as the single
  deliberate exception, with the sealed-params SHA3Params example as the contrast (sealed params
  keep the caller inside the safe set; hazmat hands them the raw operation).
• crypto/mlkem/src/lib.rs already says the crate "does not contain any 'hazmat'"; that sentence
  links the definition.
• The per-item 🚨 Security 🚨 sections on KeyStream, ElectronicCodeBook, CtrKeyStream and
  AESInternal are trimmed to what is specific to the item, per the one-home-per-fact rule.

Step 5: branches in flight

After this lands on the release branch, the per-cipher branches rebase and adopt the same layout.
feature/sm4 currently exports its permutation from the crate root
(pub use sm4::{BLOCK_LEN, KEY_LEN, LANES, SM4}); Camellia, ARIA and TDES follow the same pattern.
Each moves its permutation type to bouncycastle_<cipher>::hazmat. This is a one-file change per
branch plus the use lines, and it is cheaper to do at rebase time than after merge.

Ordering and commits

Steps 1 to 4 are one task list, four commits, in that order: each compiles and passes on its own, and
each is a git diff -M of one file move plus use lines. No logic changes anywhere, so no
cargo mutants run is owed; each commit message says so. Step 5 is one commit on each branch.

Scope

Touched: crypto/core/src/{lib,traits,hazmat,stream_cipher}.rs; crypto/aes/src/{lib,hazmat}.rs
and the rename; crypto/modes/src/{lib,ctr,hazmat,gcm}.rs; use lines in core-test-framework,
modes, aes, cli, mem_usage_benches and every test and bench that names the old paths; the
three documents above. About 90 files, of which six have content changes.

Not touched: any function body, any signature, any test assertion, any encoding, Cargo.toml of
any crate (no feature, no dependency change), the CLI's command set and flags, and
core-test-framework's module layout. ./dev_scripts/quality_stats.sh ./crypto gives identical
figures before and after.

Acceptance criteria

• bouncycastle_core::hazmat exists, holds ElectronicCodeBook and KeyStream, and its
  module docs define the term and the supported uses; neither trait is reachable from
  bouncycastle_core::traits.
• bouncycastle_aes::hazmat::{AES128Internal, AES192Internal, AES256Internal} exist and
  bouncycastle_aes::aes_internal does not; bouncycastle_aes::BLOCK_LEN still exists.
• bouncycastle_modes::hazmat::CtrKeyStream exists and bouncycastle_modes::ctr::CtrKeyStream
  does not; bouncycastle_modes::Ctr is unchanged.
• grep -rn "traits::ElectronicCodeBook\|traits::KeyStream\|aes_internal\|ctr::CtrKeyStream" --include=*.rs --include=*.md crypto cli mem_usage_benches src returns nothing.
• cargo test --workspace passes with the same test count as before the change.
• RUSTDOCFLAGS="-D rustdoc::broken_intra_doc_links" cargo doc --workspace --no-deps is clean
  (every moved item was an intra-doc link target).
• cargo bench --all --no-run and cargo build --workspace succeed.
• cargo +nightly fmt --check is clean.
• ./dev_scripts/quality_stats.sh ./crypto output is byte-identical before and after.
• QUALITY_AND_STYLE.md and INTRODUCTION.md name the convention; no crate doc restates the
  module docs' definition.
• Each commit message states that no logic changed and no mutation run is owed.

Other candidates flagged, not in scope here

Each of these came up while mapping the two traits. A verdict is given so that the discussion can
start from a position; none of them is part of the task list above.

1. bouncycastle_modes::Ecb and bouncycastle_aes::{AES_ECB_128, AES_ECB_192, AES_ECB_256}.
   The strongest candidate. ECB mode implements the same BlockCipherEncryptor /
   SymmetricCipherEncryptor traits as CBC, appears in the modes table, has padded aliases and a
   CLI subcommand, and its docs say "Do not use it to encrypt data". It looks like a cipher and is
   not one, which is the hazmat definition exactly. Verdict: move Ecb to modes::hazmat and the
   AES_ECB_* aliases to aes::hazmat in a follow-up, so that the only ECB in the library is
   spelled hazmat::Ecb. Kept out of this task list because it changes the modes table and the CLI's
   import, which deserve their own review.

2. do_hazardous_operations in bouncycastle_core::key_material. The library's one existing
   hazmat-style gate, by name. It guards key-material mutation rather than a raw primitive, so it
   is a different kind of hazard, but a downstream grepping for hazmat would expect to find it.
   Verdict: re-export it from core::hazmat (or move it there) in a follow-up; about 50 call
   sites, mechanical.

3. MLKEM::encaps_internal (mlkem, mlkem-lowmemory). Takes the encapsulation randomness m
   from the caller, returns the shared secret as raw bytes, and its docs already say "should not be
   used directly ... Please don't do it." It is an inherent method, so a path-based notice needs it
   to become a free function or a method on a hazmat extension trait. Verdict: worth doing when
   the KEM traits are next touched; a rename to encaps_with_randomness would also help.

4. keygen_from_seed (mldsa, mldsa-lowmemory, mlkem). Takes the seed from the caller, but as a
   KeyMaterial<32>, so the KeyType and security-strength checks apply, and the seed is a key
   format the library already supports, so this is key import, not composition. Verdict: not
   hazmat; the KeyMaterial wrapper is the boundary.

5. StreamCipher::from_keystream and StreamCipher::keystream. Bypass nonce generation, but
   take or return a KeyStream, so they are only reachable through the hazmat path. Verdict: leave;
   the existing 🚨 doc suffices.

6. HashDRBG80090A::new_unititialized. The doc says "WARNING: Dangerous!" and the name has a
   typo. It produces a DRBG that must be seeded by the caller. Verdict: candidate for
   rng::hazmat when the RNG crate is next touched, and the spelling should be fixed regardless.

7. The *Internal types in sha2 and sha3 (SHA3Internal, SHA256Internal, SHAKEInternal,
   KMACInternal, CSHAKEInternal, ...). Same suffix as AESInternal, different meaning:
   these are generic over sealed parameter traits, so only the standard instances can be built.
   Verdict: not hazmat. The Internal suffix now means two things in the workspace, "sealed
   generic behind aliases" and "raw primitive"; this issue resolves the second meaning by path and
   leaves the naming question open.

8. MuBuilder (mldsa). Lets a caller build the message representative mu externally. This
   is the "external mu" variant of ML-DSA; the risk is a caller feeding a wrong tr.
   Verdict: leave, note it in the crate's Security Considerations if it is not already.

9. core-test-framework::FixedSeedRNG. A deterministic RNG; hazardous if it ever left a
   dev-dependencies table. Verdict: not hazmat, it is test infrastructure, but worth a one-line
   guard in QUALITY_AND_STYLE.md that core-test-framework is never a runtime dependency.

10. Branches in flight beyond the block ciphers. feature/rsa-sig exposes pub mod modexp
    (raw RSA), and feature/ecdsa's bouncycastle-ec exposes per-curve *_point, *_scalar,
    *_comb and *_wnaf modules (raw group arithmetic). Both are the asymmetric analogue of a raw
    block permutation. Verdict: the owners of those branches should decide whether those modules
    are hazmat before merge, using the definition this issue lands.

11. The CLI's aes128-ecb subcommands. They exist for interoperability and test vectors, and
    their help text should say so. Verdict: a wording change alongside candidate 1.

---

Assisted-by: Claude Code:claude-fable-5-1

[ounsworth]

I love this initiative.

I think there are two separate issues that should be discussed separately:

1. Communicating to users "This thing needs to exist, and experts might need to use it, but only use it if you know what you're doing".
2. Some sort of compiler magic to block off-spec uses of our primitives.

1. Hazmat Label

You are proposing to create a pattern whereby crates that contain hazmat things create a sub-module ::hazmat, and you stick all the dangerous stuff in there. That way, the stuff is still visible, but someone would have to put ::hazmat in their import line in order to get at it.

I think that's fine, good even.
Though I would suggest one minor variation; Instead of crypto/core/src/hazmat.rs, I would suggest crypto/core/src/hazmat/mod.rs so that we can put crypto/core/src/hazmat/key_stream.rs, crypto/core/src/hazmat/ecb.rs, etc.

QUALITY_AND_STYLE.md should have a sentence that the pattern for crates containing hazmat objects is that a crate's lib.rs must expose pub mod hazmat; and must never pub use something from it since pub use hazmat::Thing; would then expose Thing at the crate root and completely bypass the hazmat tag.

2. Compiler sealing

I think there are two different sub-problems here that might require different solutions:

A. Optionally taking traits out of public crate API surface.
B. Optionally taking structs and enums out of the public crate API surface.

The idea that you consider of having a cargo feature that enables / disables hazmat might work for removing traits, but I'm pretty sure it won't work for removing structs / enums, since any public types that are built on top of them need to be able to see them.

Problem A is new -- the trait ElectronicCodeBook and friends are the first examples where we have a utility crate exposing a trait that our own other crates need to use, but we don't want 3rd party users using directly (except maybe for research purposes).

We do have lots of other examples of Problem B, which I've been solving using a design pattern called "sealed traits" where you have a public struct that you bound on a non-public trait.

// non-public on purpose
trait SealedThing {}

// A pub struct which is bounded on a private trait.
// That means it's usable only with OTHERs that are
// defined within the same crate, and users can't
// create their own OTHERs.
pub struct Thing<OTHER: SealedThing>;

pub struct OtherThing : SealedThing {}

pub type UsableThing = Thing<OtherThing>;

This does all the things you want:

• UsableThing is exposed publicly and usable from outside the crate (since both Thing and OtherThing are pub).
• Since trait SealedThing is non-pub, this trait is not visible outside the lib, so an external user cannot create their own OtherThing: SealedThing, and therefore they cannot create their own instantiations of Thing<>.

I do not believe there is any way in Rust for OtherThing to not be pub, because that would break UsableThing from compiling within a calling application.

In the existing bc-rust crates, every *Internal struct is already following this "sealed" pattern via a sealed Params object:

pub type SHA224 = SHA256Internal<SHA224Params>;

pub struct SHA256Internal<PARAMS: SHA256InitValue> {}

// non-pub sealed trait
trait SHA256InitValue: HashAlgParams + Clone {}

or

pub type MLDSA44 = MLDSA<
    MLDSA44Params,
    MLDSA44PublicKey,
    MLDSA44PrivateKey,
    MLDSA44_PK_LEN,
    MLDSA44_SK_LEN,
    MLDSA44_SIG_LEN,
>;

/// A crate-private (aka "sealed") trait that prevents a new ML-DSA parameter set from being defined
/// outside this crate.
trait MLDSAParamsInternalTrait {}

/// One ML-DSA parameter set: the values of FIPS 204, Table 1 and Table 2, and the types whose size
/// they determine.
///
/// Sealed via a private supertrait, so [`MLDSA44Params`], [`MLDSA65Params`] and [`MLDSA87Params`]
/// are the only implementations.
pub trait MLDSAParams: MLDSAParamsInternalTrait {..}

pub struct MLDSA44Params: MLDSAParams {..}

The problem you're hitting, I think, is that you have this pub type def:

pub type AES_CBC_128<Dir, Pad> = <Dir as PaddedMode<
    Cbc<AES128Internal, Encrypting, 16, BLOCK_LEN>,
    Cbc<AES128Internal, Decrypting, 16, BLOCK_LEN>,
    Pad,
    16,
    BLOCK_LEN,
>>::Mode;

which does not contain anything sealed, so somebody could define their own frankenstein version.

I suggest that you refactor this to match the sealed PARAMS pattern that we've established already in all the other bc-rust crates to solve this problem. Something like:

pub type AES_CBC_128<Dir, Pad> = AES_CBC_Internal<AES_CBC_128_PARAMS>;

// non-pub trait, aka a sealed trait
trait AES_CBC_PARAMS {
    // whataver fields you need
}

struct AES_CBC_Internal<PARAMS: AES_CBC_PARAMS> {
    // blah
}

impl BlockCipher for AES_CBC_Internal {
    
}

So I'm on board with creating a cargo feature hazmat and using it to remove trait ElectronicCodeBook and friends from the default build.

I agree with what I think is the conclusion you came do that a cargo feature hazmat doesn't do what we want here.

I also suggest that you seal all the AES public types with PARAMS objects.

[dghgit]

Done.

Dir is now sealed as well as of 160ac18: the aliases now project through core's Direction::Select. Only 'Encrypting' and 'Decrypting' can be projected. This linked to the earlier Ascon cipher work and the other block ciphers are now updated as well.