Skip to content

XOF extends Hash: SHAKE128 and SHAKE256 are hashes, and squeezing gets its own type - #118

Merged
dghgit merged 93 commits into
feature/simple-ciphersfrom
feature/xof-cshake
Sep 28, 2026
Merged

dghgit merged 93 commits into
feature/simple-ciphersfrom
feature/xof-cshake

Conversation

@dghgit

@dghgit dghgit commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Executive Summary

This is a complete PR to add support for SP 800-185, it's attempting to address two things, the first being that XOF functions are treated in a lot of situations as message digests, and there's even OIDs for the same, the second is how to deal with variable length output across TupleHash, ParallelHash, and KMAC (the last one actually qualifying as a MAC and an XOF...).

After a bit of to-ing and fro-ing, partly due to the lack of overrides in Rust, the best solution to the problem seemed to be the creation of XofOutput, which is a small trait the getting of which disables the use of the parent structure - this allows do_update on the Hash trait to stay infallible, but thanks to the borrow checker, causes a compile time error if XofOutput is created and someone attempts to use the parent Hash. Most of the text here is devoted exclusively to that issue, it also includes the sample code for the trait, and the sample usage below. There's no need to look anywhere else (yet).

First question: given the constraints, does this look okay as a solution? Second question: If so, I guess it should be XOFOutput shouldn't it?

I note this will need to be re-based once #117 goes in.

Comments welcome.

Issue Link

No linked issue.

Summary

XOF now extends Hash, so SHAKE128 and SHAKE256 are hashes and can be used wherever one is
wanted; the squeezing phase becomes its own type, which turns the absorb-then-squeeze rule from a
runtime error into a compile error.

Description

Stacked on #115, so this PR is based on feature/symmetric-cipher and shows four commits:

Commit
e3c31e3 core, sha3: XOF extends Hash; squeezing becomes its own type
d02361e sha3: pin the SHAKE block_bitlen and output_len values, which three mutants survived
19f00a0 core: XofOutput gains do_final / do_final_out
a7dd1a4 gitignore: ignore editor swap and backup files

The goal. SHAKE128 and SHAKE256 should be usable as hashes. This is the relationship BC Java
draws with Xof extends ExtendedDigest extends Digest, and it is what lets a caller hold "some
hash" and pass a SHAKE.

What blocked it. Hash::do_update is infallible; XOF::absorb returned
HashError::InvalidState for absorb-after-squeeze. BC Java has the same rule but signals it with an
unchecked IllegalStateException from KeccakDigest.absorb, which is not an option here. So the
error had to go somewhere, and the three candidates were: make Hash::do_update fallible
everywhere, panic, or remove the reachable state.

Why the third. Every absorb site in ML-KEM and ML-DSA already read
.expect("absorb before squeeze is infallible") — 117 of them. The callers were unanimous that the
error could not happen in correct code, which is exactly the case QUALITY_AND_STYLE.md says to
redesign rather than wrap in a Result:

If you're returning Result for something the caller can't reasonably hit with valid usage,
redesign the signature instead.

So XOF::into_output consumes the XOF and returns an XofOutput. After output has begun there is
no value left to call do_update on, the error has nothing to report, and all 117 .expect(...)
calls are gone. Err() counts in core code dropped from 314 to 307.

The invariant that makes do_update infallible is that a SHAKEInternal a caller can name has
never squeezed. That holds because every KDF entry point also takes self by value, and it is
pinned by a debug_assert! in do_update plus a compile_fail doctest.

XofOutput

pub trait XofOutput {
    /// Produces the next `num_bytes` bytes of the output stream.
    ///
    /// BC Java's `Xof.doOutput(out, outOff, outLen)`.
    fn do_output(&mut self, num_bytes: usize) -> Vec<u8>;

    /// As [`do_output`](Self::do_output), filling the caller's buffer, which is zeroized first.
    /// Returns the number of bytes written.
    fn do_output_out(&mut self, output: &mut [u8]) -> usize;

    /// The last output: produces `num_bytes` bytes and ends the stream.
    ///
    /// It reads the same bytes [`do_output`](Self::do_output) would at the same point in the
    /// stream; the difference is only that nothing can follow it.
    fn do_final(mut self, num_bytes: usize) -> Vec<u8>
    where
        Self: Sized,
    {
        self.do_output(num_bytes)
    }

    /// As [`do_final`](Self::do_final), filling the caller's buffer, which is zeroized first.
    /// Returns the number of bytes written.
    fn do_final_out(mut self, output: &mut [u8]) -> usize
    where
        Self: Sized,
    {
        self.do_output_out(output)
    }
}

Sample code — all five of these compile and their assertions pass:

use bouncycastle_core::traits::{Hash, XOF, XofOutput};
use bouncycastle_sha3::SHAKE128;

// 1. As a hash. XOF: Hash, so SHAKE is usable wherever a hash is wanted.
let digest: Vec<u8> = SHAKE128::new().hash(b"Hello, world!");
assert_eq!(digest.len(), 32);              // SHAKE128's nominal digest size

// 2. Streaming input, then a fixed-size digest.
let mut shake = SHAKE128::new();
shake.do_update(b"Hello, ");
shake.do_update(b"world!");
assert_eq!(shake.do_final(), digest);

// 3. Extendable output: into_output() ends the input phase and hands back the stream.
let mut shake = SHAKE128::new();
shake.do_update(b"Hello, world!");
let mut out = shake.into_output();
let first  = out.do_output(16);
let second = out.do_final(16);             // last read; the handle is consumed
assert_eq!([first, second].concat(), digest);   // one continuous stream

// 4. One-shot, any length.
let long: Vec<u8> = SHAKE128::new().hash_xof(b"Hello, world!", 1024);
assert_eq!(&long[..32], &digest[..]);

// 5. Into a caller-owned buffer, no allocation.
let mut buf = [0u8; 64];
let mut shake = SHAKE128::new();
shake.do_update(b"Hello, world!");
let n = shake.into_output().do_final_out(&mut buf);
assert_eq!(n, 64);

And the case that no longer compiles, which is the point of the change:

let mut shake = SHAKE128::new();
shake.do_update(b"abc");
let mut out = shake.into_output();
shake.do_update(b"more");   // error[E0382]: use of moved value: `shake`

Method mapping.

BC Java here
Digest.getDigestSize() Hash::output_len — 32 / 64, fixedOutputLength / 4 as SHAKEDigest.java:84
ExtendedDigest.getByteLength() Hash::block_bitlen, in bits — 1344 / 1088, the rate 1600 - 2c
Digest.update Hash::do_update
Digest.doFinal(out, off) Hash::do_final
Xof.doOutput(out, off, len) XOF::into_output then XofOutput::do_output
Xof.doFinal(out, off, len) after doOutput XofOutput::do_final
Digest.reset() none — the final methods take self

XofOutput::do_final is a provided method delegating to do_output. SHAKEDigest.java:92-99
shows Java's doFinal(out, off, outLen) is doOutput plus reset(); here taking self by value
is the reset, and dropping the handle zeroizes the sponge through Secret's Drop. It is a name
for "this read is my last", not new behaviour.

Suspendable. The suite suspends a squeezing SHAKE and resumes it, which under the new design
must not hand back an absorbing value. SHAKEOutput therefore has its own Clone and
Suspendable, and each from_suspended rejects the other's phase with InvalidData. This is a
behaviour change: SHAKE128::from_suspended now refuses a state that was suspended mid-output.

Alternatives considered. Making Hash::do_update fallible would have put a Result on SHA-2,
SHA-3 and SM3 for a condition only XOFs can reach. Panicking is against house rules. A shared
metadata-only supertrait for Hash and XOF was considered and rejected: it avoids the
do_update problem but does not give substitutability, which was the whole point.

Scope and Risk

  • Impacted: core, core-test-framework, sha3, factory, mlkem, mlkem-lowmemory,
    mldsa, mldsa-lowmemory, cli, mem_usage_benches.
  • Runtime behaviour: SHAKE output is unchanged — cross-checked against openssl dgst byte for
    byte for both variants, and the NIST CAVP bit-oriented vectors and the ML-KEM / ML-DSA
    known-answer tests all pass unmodified. The one deliberate behaviour change is
    SHAKE128::from_suspended rejecting a squeezing state.
  • Regression risk: low for output correctness (the KATs are the gate), moderate for API churn —
    this is a breaking change to XOF and every call site moved.
  • Worst case: a phase transition inserted in the wrong place in a rejection-sampling loop would
    restart the output stream instead of continuing it. That would break the ML-KEM / ML-DSA KATs
    loudly rather than silently, and it did during development.

Validation

cargo test --workspace                                    # 889 pass
cargo build --workspace --all-targets                     # no warnings
cargo fmt --all --check
RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --workspace

# SHAKE output is unchanged
printf 'Hello, world!' | openssl dgst -shake128 -xoflen 32
printf 'Hello, world!' | openssl dgst -shake256 -xoflen 64

# mutation testing (--file is silently ignored by this cargo-mutants; -F works)
cargo mutants --test-workspace=true -p bouncycastle-sha3 -F 'shake\.rs' --jobs 3

cargo mutants on shake.rs against the full workspace suite: 126 mutants, 89 caught, 4 missed,
33 unviable
. Three of the four survivors were real gaps — nothing pinned the actual values of
block_bitlen or output_len, so 1600 - 2c surviving as 1600 + 2c went unnoticed. d02361e
adds that test and all three now die. The fourth is a genuine equivalent mutant, | versus ^ on
disjoint bit ranges, verified exhaustively over all 2048 (num_bits, partial_byte) pairs and
commented at the site. The 33 unviable are the .cargo/mutants.toml error values not typechecking
against HashError / KDFError / SuspendableError.

AI Usage Statement

Did you use AI in creating this pull request:

  • No
  • Yes, indirectly - no submitted code was generated by AI (e.g., answering questions, performing a review, suggestions, etc.)
  • Yes, trivial code changes were generated by AI (e.g., autocompletion of a single line, reformatting, or spell-checking)
  • Yes, non-trivial code changes were generated by AI

If submitted code changes were generated by AI, fill in the following declaration:
Assisted-by: Claude Code:claude-opus-5

@ounsworth

Copy link
Copy Markdown
Contributor

At a quick first read of the PR description (which is very long, so I didn't read the whole thing), I am not really sure what problem this is solving, so I will need to take an actual read of the code diff to see what's going on.

My immediate thoughts are:

  1. XOFs are not hashes. FIPS 202 makes that point very clearly with a whole appendix about why it's dangerous to think of them as hashes -- this boils down to the fact that SHAKE(m, 31) and SHAKE(m, 32) will be the same for the first 31 bytes and that can lead to significant weakening of the collision resistance or pre-image resistance of your protocol, if you're not careful. So the framing of this PR makes me uncomfortable. But I'll need to look at the code to see what you've actually done.

  2. Returning output of type XofOutput to hide some types of state errors. I'm aware of that trick but have not used it, on purpose, in bc-rust. I have never really formalized my thinking on this point until now, but I think it's that the raw inputs and outputs to bc-rust functions (plaintexts, ciphertexts, signature values, hash outputs, etc) are things the application will need to hand to other bits of rust ecosystem -- for example, file or network IO, HTTP libs, etc -- so we should treat all inputs / outputs as raw byte types ([u8] or Vec<u8>). My feeling is that wrapping outputs in a bc-rust custom type (even if that custom type coerces to a [u8]) will create at least some confusion and user friction. I think my intuition here is that we should put

  • Inputs and outputs to bc-rust functions (plaintexts, ciphertexts, signature values, hash outputs, etc) should be basic rust types (typically [u8] or Vec<u8>) and not custom bc-rust types. Even if a custom type coerces to a basic type, this will add user friction.

in the QUALITY_AND_STYLE.md as a higher priority than turning runtime errors into compile-time errors.
As always, this is my opinion, but I'd love discussion.

Comment thread crypto/core/src/traits.rs Outdated
pub trait XofOutput {
/// Produces the next `num_bytes` bytes of the output stream.
///
/// BC Java's `Xof.doOutput(out, outOff, outLen)`.

@ounsworth ounsworth Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's extremely weird to have a comment in bc-rust that references bc-java.
I guess the design / architecture discussion that needs to be had here is whether it's a goal to align bc-rust APIs to bc-java APIs, or whether we consider this a greenfield implementation and should be free to do what makes sense, regardless of what other parts of the BC family do.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a guide, I wouldn't commit this, it's still under development though and I'm using the BC implementations of SP 800-185 to guide the process and provide compatibility tests. I'll clean these up before final commit.

Comment thread crypto/core/src/traits.rs
/// agree on their first 32 bytes. An attacker who only needs to know that two values came from the
/// same input -- enough to break an anonymity property -- learns it from the overlap. Where that
/// matters, salt the input.
pub trait XOF: Hash {

@ounsworth ounsworth Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial reaction is that I don't like this.
NIST has explicitly stated that XOFs are not hashes, and must not be used as hashes. FIPS 202 Appendix A.2 is essentially an essay on that topic, so I think this is going to get us into trouble.

I'm open to having my mind changed though.

@dghgit dghgit Sep 7, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understand how could happen. You'll see the issue when you get to the last paragraph of FIPS 202 Appendix A.2, it starts out like that, but by the end we can see it's all just one big happy family. The issue they talk about at the start is the motivation behind the design of cSHAKE, so I agree it would make things clearer if it was actually here.

So I've had a look further in BC, I think the only example of an XOF being used as both a hash and an XOF together is RSA-PSS. It would be possible to split the two services, although an RSA-PSS implementation would require both an XOF and a digest passed to it, as opposed to just digest that happens to be an XOF. I think everywhere else it's either digest or XOF. PSS might be a sign of things to come though, as in where the XOF is used to replace a KDF/MGF so it really does feel that having XOF extend Hash is better. The real problem we need to address is that Rust allows us to have a method which says "I'm always good" and for an XOF, update as "I'm always good" is not correct...

In that respect the current solution seems like the most "Rusty" one, as activating the XOF side of the service disables it's digest side, but I'm more than happy to entertain alternatives - XofOutput was just the only thing that the Internet, LLM, and myself could come up with that worked.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Backing up a step: why would someone want SHAKE to behave as a hash? I guess because you want a fixed-size output at a size larger than 512 bits (cause otherwise you'd just use SHA3_512 and truncate it). But since you don't get any additional security out of that -- it's still only 256-bit collision-resistant -- I'm not entirely sure why someone would want to do that. Probably in 98% of cases where someone wants to do that, they would be better served by using the [KDF] interface that SHAKE already impls.

As you say, the proper way to turn SHAKE into a hash function is to include the output length in the input. Like

SHAKE_X( m ) = SHAKE( u32::to_bytes(X) + m, X) = <X bit output>

Oddly, this is not what cSHAKE is because it doesn't swallow the L param, so cSHAKE is still not a [Hash].

@dghgit dghgit Sep 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, believe it or not, people actually use SHAKE as a digest instead of SHA3-512 or SHA3-256. RSA-PSS is actually an example of this (using SHAKE as both a digest and a mask generator), CMS as well.

Some other interesting examples of SHAKE as a digest, SP 800-208 (LMS and XMSS) and FIPS PUB 205.

Comment thread crypto/core/src/traits.rs

/// As [`do_output`](Self::do_output), filling the caller's buffer, which is zeroized first.
/// Returns the number of bytes written.
fn do_output_out(&mut self, output: &mut [u8]) -> usize;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I defined the XOF trait in the language of sponge constructions (absorb / squeeze) because that's what FIPS 202 does, and sponge functions have a well-defined API.

Do you have a reference (ideally a FIPS or RFC) that defined XOFs in an abstract way using the update / output language?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.rfc-editor.org/rfc/rfc8702.html is probably the most general one. The gap in it is it misses the -len OIDs, which mean the AlgorithmIdentifier carrying the OID is also carrying an INTEGER saying how long the output should be. There's also https://www.rfc-editor.org/rfc/rfc8692.html. https://www.rfc-editor.org/rfc/rfc8419.html is an example of a standard using -len OIDs. They're starting to appear everywhere now, KMAC as well. Haven't seen much with TupleHash/ParallelHash yet, but I'm guessing it's just a matter of time. You're probably getting the picture though.

@ounsworth ounsworth Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any of those three RFCs defining SHAKE with a do_update / do_final API instead of an absorb / squeeze API. In fact, I don't see them describe an API for SHAKE at all?

I do see them using the word "digest".
To me, the word "digest" means to take an arbitrary-length input and turn it into something smaller and there are two types of cryptographic digest functions:

  • Hash functions always produce the same length output.
  • XOFs produce a variable-length output.

This is why I haven't used the word "digest" in bc-rust, but instead define [Hash] and [XOF] directly to encapsulate their differences in behaviour.

(btw, this kind of discussion is fun 😊 This is setting the theoretical foundations of the library)

@dghgit dghgit Sep 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RFC 8702, section 1, second paragraph:

"In the SHA-3 family, two extendable-output functions (SHAKEs), SHAKE128 and SHAKE256, are defined. Four other hash function instances (SHA3-224, SHA3-256, SHA3-384, and SHA3-512) are also defined but are out of scope for this document. A SHAKE is a variable-length hash function defined as SHAKE(M, d) where the output is a d-bit-long digest of message M. The corresponding collision and second-preimage-resistance strengths for SHAKE128 are min(d/2,128) and min(d,128) bits, respectively (see Appendix A.1 of [SHA3]). And the corresponding collision and second-preimage-resistance strengths for SHAKE256 are min(d/2,256) and min(d,256) bits, respectively. In this specification, we use d=256 (for SHAKE128) and d=512 (for SHAKE256)."

The length is based on the calculations in Appendix A.1 of FIPS PUB 202. 256 and 512 give are the minimum lengths required to meet 128 bit and 256 bit security respectively, and as a result encountering an OID for either SHAKE128 or SHAKE256 means a digest of length 256 and 512 bits respectively. If there's an arbitrary length involved the -len OIDs are used. The OIDs are defined here:

https://csrc.nist.gov/projects/computer-security-objects-register/algorithm-registration#Hash

They get used as digests, we've been using them as digests in BC Java/C# since 2015, I know it might seem weird, but they're digests. As I mention somewhere else, in PSS they are even used as both digest and XOF (if SHAKE is the signature digest, the mask-generator function is not MGF1 but SHAKE). Anyway, they're digests.

As an interesting aside, the section of the extendible hashes finishes with:

"If d > r + c/2, then SHAKE128 and SHAKE256 provide more than 128 and 256 bits of preimage
resistance, respectively; moreover, if d > 1600, a preimage probably does not exist."

A very brave statement, especially for NIST!

I guess the other, cautionary note, I should add - these functions all come with padding. Defining the API in terms of squeeze() and absorb() makes it look like an internal API which is being made public, it's likely the CMVP will complain about it, especially as they'll be expecting them to look like digests with a bit extra.

Comment thread crypto/sha3/src/lib.rs
//! SHA3 offers Extendable-Output Functions in the form of SHAKE, which is accessed through the [`XOF`] trait,
//! which is implemented by [`SHAKE128`] and [`SHAKE256`].
//! The difference from [`Hash`] is that SHAKE can produce output of any length.
//! [`XOF`] extends [`Hash`] -- SHAKE *is* a hash -- and adds the ability to choose the output length.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, see, that comment directly and emphatically contradicts FIPS 202 appdx. A.2, and that makes me comfortable.

@dghgit dghgit Sep 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the last paragraph of A.2. The comment is correct.

@ounsworth

Copy link
Copy Markdown
Contributor

The branch name and PR description says that this PR implements cSHAKE from SP 800-185, but I don't see any cshake in the actual diff. I'm confused.

@hubot
hubot force-pushed the feature/xof-cshake branch from b212aea to a7dd1a4 Compare September 7, 2026 21:53
@dghgit
dghgit changed the base branch from feature/symmetric-cipher to release/0.1.3alpha September 7, 2026 22:52
@hubot
hubot force-pushed the feature/xof-cshake branch from a7dd1a4 to 9006d21 Compare September 7, 2026 23:05
@hubot
hubot force-pushed the release/0.1.3alpha branch from d2a9b35 to d1dcf75 Compare September 9, 2026 01:44
@dghgit
dghgit force-pushed the feature/xof-cshake branch from 9006d21 to e909d5e Compare September 9, 2026 04:17
@dghgit
dghgit changed the base branch from release/0.1.3alpha to feature/symmetric-cipher September 9, 2026 04:17
@hubot
hubot force-pushed the feature/xof-cshake branch from e909d5e to 9006d21 Compare September 9, 2026 04:35
@dghgit
dghgit force-pushed the feature/xof-cshake branch 2 times, most recently from 2fec0ab to 5c45ae8 Compare September 9, 2026 05:36
@dghgit

dghgit commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

cSHAKE, TupleHash, ParallelHash, and KMAC now added.

@dghgit

dghgit commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

I'm really beginning to feel an active dislike for the way this thing manages conversations...

I was just re-reading, on 2 mentioned way, way, above, XofOutput is not a wrapper, it's a generator - the idea is that the digest passes that off containing a reference to itself and then becomes a pure XoF with the original construction becoming unusable courtesy of the borrow checker. This is why I asked about Clone earlier - otherwise an XoF would become single shot, a disaster if you were in a situation where you could take advantage of pre-computation, as you wouldn't be able to re-use the previous runs of the compression function as soon as you created an XoF (which is still at the end of the day, just a fancy digest...)

@dghgit
dghgit requested a review from ounsworth September 10, 2026 12:25
@dghgit

dghgit commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor Author

So, I've been re-reading SP 800-185... the situation is actually a bit more nuanced than either of us actually appreciated, especially me.

It actually looks more like this:

(Hash) SHA3-224, SHA3-256, SHA3-384, SHA3-512
(Hash/XOF) SHAKE, cSHAKE
(Hash) TupleHash, ParallelHash, KMAC
(XOF) TupleHashXOF, ParallelHashXOF, KMACXOF

We got away with combining hash and XOF in bc-java as SP 800-185 arrived later and we'd already implemented SHAKE (we actually got CAVP cert #2 for SHA-3). It worked for TupleHash, ParallelHash, and KMAC as BC Java switches between the hash variant and the XOF variant of the last three by the doFinal() call used, so calling doFinal() without an output length tells it to behave like a hash, calling do final and passing an output length makes it an XOF unless the length is the default one. Any calls to doOutput prior to doFinal() force XOF behavior throughout.

So while KMAC128 - doFinal(0, buf)/doFinal(0, buf, 32) produce the same result - either a call to doOutput() first or a non-default-length to doFinal() run as KMACXOF128. Rust doesn't quite work like that because of lack of overrides, which is partly what lead to the use of the XofOutput generator.

Anyway I've done some further refactoring which removed some duplicated code that crept in. XofOutput is now XOFOutput and simplified.

Possible Decision Thing One thing that did show up though is the non-XOFOutput for SHAKE.do_final_out() fills the array it's given, rather than just writing 32 or 64 bytes. Given SHAKE really is Hash/XOF this is possibly okay, it's just an oddity of the function, the same would apply for cSHAKE. The solution would be to add an equivalent do_final_out() to XOFOutput, and then force the super trait one to only write the default number of bytes... the cSHAKE variants are okay because the actual types are present, unlike in bc-java, so the hash variant is already locked down, and the XOF will either be a hash for the default length, or some other result, with do_output on the XOF encoding "infinite output" correctly.

@dghgit

dghgit commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

One other approach possible approach just occurred to me - we could keep Hash and XOF separate - but we'd need to duplicate SHAKE and cSHAKE (at least the externals). It's a bit of a vibe thing - with Clone plus XOF, extending Hash works fine, the only difference is the XOFs will do a fill on do_final_out() and produce the default output length on hash(), which does actually make sense, if you think about it. We could avoid that by introducing SHAKEXOF and cSHAKEXOF, which in some ways makes less sense, even though in some ways it does.

@dghgit

dghgit commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor Author

Okay, I have an answer. It looks similar to the earlier ones, but needs to incorporate one extra change.

We do the Clone/swap trick as before. The XOF trait extends Hash but contains something like:

pub trait XOF: Hash {
    /// The squeezing state this XOF turns into.
    type Squeezer: XOFSqueezer;

    fn into_squeezer(self) -> Self::Squeezer;

    fn into_squeezer_partial_bits(
        self,
        partial_byte: u8,
        num_bits: usize,
    ) -> Result<Self::Squeezer, HashError>;

/// other stuff goes here...
}

pub trait XOFSqueezer {
    fn do_output(&mut self, num_bytes: usize) -> Vec<u8>;

    fn do_output_out(&mut self, output: &mut [u8]) -> usize;

    fn do_final(&mut self, num_bytes: usize) -> Vec<u8>;

    fn do_final_out(&mut self, output: &mut [u8]) -> usize;
}

Hash.do_final for the algorithms supporting Digest/XOF so that the Hash do_final* produces a digest of the correct length. This puts the oneshot hash and do_final into line, for XOF behaviour (where the generation of n bytes is required) people can use the do_final* on the XOFSqueezer to fill a byte array as expected - this will keep the Hash-side of the function consistent across all uses in Hash, while at the same time allowing for use of XOF-side behaviour when requested (so do_final() with no do_output uses right_encode(L), whereas if do_output is called first it's simply a do_output saying I'm finished). As an added advantage it will mean RSA-PSS will only need one instance of it's digest, rather than two.

dghgit and others added 29 commits September 24, 2026 12:59
…r GCM and make its inherent API private (#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>
…/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>
…AES-lightengine-GCM-mode

Brings in 52e63bc, which merged feature/xof-cshake into the CCM branch this one stacks on: the
AES128Internal / AES192Internal / AES256Internal rename, public aes mode modules with a link list
in the crate docs, and ElectronicCodeBook's now-required batch methods.

- aes/src/lib.rs: the link list gains AES_GCM, and `gcm` is a `pub mod` like the other modes.
- The GCM code, tests and CLI, and the CTR unit test this branch touches, use the new
  AES*Internal names.

Assisted-by: Claude:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in PR #137 (ML-DSA missing reduce32) and 8823c7b, which moved
SecurityStrength out of core::traits into core::security_strength and the
stream-cipher blanket impls into core::impls. Conflicts were all fallout
from that move: traits.rs takes the move (neither block had changed on this
branch), and the use-lists keep this branch's Hash/XOFSqueezer imports with
SecurityStrength taken from its new path, including in the SP 800-185 files
that exist only here (cshake, kmac, parallelhash, tuplehash, kmac_tests).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…officialfrancismendoza/125-AES-lightengine-CCM-mode
…25-AES-lightengine-CCM-mode' into feature/officialfrancismendoza/125-AES-lightengine-CCM-mode

Merged origin CCM history into updated xof-cshake CCM branch
…officialfrancismendoza/124-AES-lightengine-GCM-mode
…4-AES-lightengine-GCM-mode

Realigns this branch with the CCM branch it stacks on, after both merged feature/xof-cshake
separately. The one conflict is the core::traits import list in modes/benches/modes_benches.rs,
resolved to the CCM side: it keeps AEADCipherEncryptor, which 16a4ba5's resolution dropped,
breaking the bench build (CcmEncryptor::encrypt_out_rng_detached is a trait method).

Assisted-by: Claude:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…id for CCM

Ctr::apply_batch built its 2- or 4-block keystream in a plain local, and
refill/apply_one enciphered into a plain local before copying into the
Secret, so live keystream was left on the stack unzeroized -- the exact
leak 2161a04 fixed in the CCM copy of this code. The batch scratch is now
one Secret per width held for the whole apply() call rather than one per
batch, and refill/apply_one encipher in place inside the Secret.

Assisted-by: Claude:claude-fable-5-1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…or a short inline C, empty update keeps the AAD phase open, per-side capacity messages, hoisted keystream scratch

* CcmDecryptor now carries the same NONCE_LEN >= 12 assertion as
  CcmEncryptor (moved into CcmBuffer and run from every entry point of
  both, one-shots included), so a parameter set compiles for both sides
  or neither; compile_fail doctest added for the decryptor.
* An inline ciphertext shorter than TAG_LEN is DecryptionFailed from all
  three inline entry points (Ccm::decrypt previously said GenericError),
  the variant SymmetricCipherDecryptor::do_final specifies for a
  malformed ciphertext.
* CcmBuffer::do_update_out with an empty slice is a no-op and no longer
  closes the AAD phase, matching the trait's empty-AAD-at-any-point rule.
* The over-capacity message names the caller's real bound: the encryptor
  is limited to FINAL_LEN - TAG_LEN, the decryptor to FINAL_LEN.
* The batched keystream Secret is held per apply_keystream call instead
  of per 2/4-block batch, the same shape Ctr now uses.
* Docs: short tags are justified by Appendix C.1/C.2 (Tlen=32, 48), not
  the ACVP set, which has only 96- and 128-bit tags; the AAD sharing
  FINAL_LEN's bound is stated on CcmEncryptor with the sizing rule.

Tests: sp800_38c_tests gains an_empty_update_does_not_close_the_aad_phase,
extends the short-ciphertext test to all three entry points, and pins the
encryptor's message to its bound. SP 800-38C Appendix C parameters
verified against the downloaded PDF.

Assisted-by: Claude:claude-fable-5-1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ptor too, and the AAD shares FINAL_LEN's bound

Assisted-by: Claude:claude-fable-5-1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…n the q limit on decrypt as well as encrypt

A --nonce-file written with echo rather than echo -n is a valid 13-byte
nonce, so it was silently accepted as a different nonce from the 12 bytes
intended; the bytes are still used as they are (stripping would collapse
two distinct nonces into one), but stderr now says so and gives the
remedy. The decrypt arm printed the raw Debug form of Ccm::new's payload
limit error; both arms now go through one helper that reports the payload
length, q and the limit.

Assisted-by: Claude:claude-fable-5-1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… claimed to, and record what it measures

bench_buffering_* called the one-shots, which CcmEncryptor/CcmDecryptor
override to bypass the buffer, so the "3 * BUFFER_LEN" the header claimed
was never exercised, and BUFFER_LEN was the pre-rename name. The benches
now drive do_*_init -> do_update_out -> do_final (and the detached final),
the one-shot is kept as its own bench for the "bypasses the buffer" claim,
FINAL_LEN is 16 KiB so every path clears massif's ~7.7 KB start-up floor,
and the message is pinned through a black_box reference so the compiler
places it identically in every bench. Measured figures are in the header:
the one-shot is within 1.3 KB (the DRBG) of the direct path, and the
streaming path costs about 7 * FINAL_LEN, not 3, because each consuming
final takes the 2 * FINAL_LEN value by value.

Assisted-by: Claude:claude-fable-5-1
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ccm and CcmEncryptor are imported into crypto/aes/src/ccm.rs, so
[`Ccm`](bouncycastle_modes::Ccm) and its CcmEncryptor twin resolve without
the target, and `cargo doc` with -D warnings failed on them with
rustdoc::redundant_explicit_links.

Assisted-by: Claude:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ne the table supports

bench_streaming_encrypt's doc said ~7 * FINAL_LEN above
bench_direct_encrypt_detached, but the measured 134 968 B is ~6.1x above
that bench (which also holds a ciphertext array) and ~7.2x above the
message array alone, which is how the module header states it. The
decrypt bench holds both the message and the sealed array, and its ~7x
(149 976 B, ~7.15x) is against the two of them, not the sealed array
alone. No figures change.

Assisted-by: Claude:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…24-AES-lightengine-GCM-mode' into feature/officialfrancismendoza/124-AES-lightengine-GCM-mode
A local Claude Code permissions file, committed with the initial GCM add
(7a6e2a4); checked in, it pre-approves git rebase/add for every
contributor's session in the repo.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The crate docs predated Gcm, or described its pre-788fd06 shape:
- an "inherent detached-tag API", made private by 788fd06; the AEAD
  traits are Gcm's whole API
- "For a new design, use Ccm", and a Security Considerations section
  that exempted only CCM from being unauthenticated
- GCM listed under Not yet implemented; replaced with the two GCM
  options Gcm deliberately omits (non-96-bit IVs, 32/64-bit tags)
- the CLI section counted six modes and had no -gcm framing

Also adds a GCM alias and a doctested detached-tag round trip alongside
the CCM ones. Spec references checked against SP 800-38D (Sec 5.2.1.1,
5.2.1.2, 8.3, Algorithm 4 step 2, Appendices A and C).

Assisted-by: Claude:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…k copies (#124)

setup built H = CIPH_K(0^128) and CIPH_K(J0) in plain stack arrays before
moving them into their Secrets, tag_block copied the mask back out with
`*self.ek_j0`, and Ghash::absorb copied Y out with `*self.y` -- none of
which were zeroized. SP 800-38D Sec 5.3 requires GCM intermediates to stay
secret, and Appendix A: H recovered is authentication lost. The module
docs already claimed all of these lived in Secret.

H and CIPH_K(J0) are now encrypted in place inside their Secrets; Ghash
updates Y in place, and absorb is an associated function over the
fields so the pending block is passed by reference rather than copied.
Ghash::finish and Gcm::tag_block write into a caller's Secret instead of
returning an array: on the decrypting side that value is the expected
tag T', which is a forgery for the rejected ciphertext if it survives
a failed comparison.

cargo mutants over gcm.rs and ghash.rs: 277 mutants, 222 caught,
46 unviable, 1 timeout, 8 missed -- all equivalent (check_shape's const
assert, the documented | vs ^ in impl_mul64, and > vs >= guards on
zero-length copies).

Assisted-by: Claude:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
load_aad used read_from_file, whose hex-or-raw guess changes what the tag
covers without any error: an AAD file of "cafe" was authenticated as 2
bytes, sixteen zero bytes (which the hex decoder skips) as empty AAD, and
a file ending in a backslash panicked out of bounds in hex::decode_out.
The tag then fails against any other GCM implementation given the same
file. Same fix as 3dd3266 made for CCM's --nonce-file: read_from_file_raw.

New test aad_file_is_raw_bytes_not_hex_decoded covers all three cases;
it fails before this change and passes after.

Assisted-by: Claude:claude-opus-5-5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in f8f7a2e, which keeps Ctr's keystream out of unzeroized stack
arrays (refill, apply_batch, apply_one). Merged cleanly; no conflicts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/officialfrancismendoza/124-AES-lightengine-GCM-mode

Brings in f8f7a2e (via 8575ea7), which keeps Ctr's keystream out of
unzeroized stack arrays -- GCM's GCTR runs through Ctr, so this closes the
last of the stack copies behind gcm.rs's claim that the CTR keystream lives
in Secret. Merged cleanly; this branch's only ctr.rs change is start_at.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…/officialfrancismendoza/125-AES-lightengine-CCM-mode

Brings in f8f7a2e (via 8575ea7), feature/simple-ciphers' fix for Ctr's
keystream being left in unzeroized stack arrays -- the same leak 985eb94
already fixed on this branch. The one conflict, ctr.rs's refill,
apply_batch and apply_one, takes this branch's 985eb94 versions (one
Secret per batch width held for the whole apply() call, rather than one
per batch), keeping f8f7a2e's module-doc sentence; the crate docs' Memory
Usage note is reworded to match. Resolving it here means the same
conflict does not recur when this branch lands on feature/xof-cshake.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…4-AES-lightengine-GCM-mode

Brings in the CCM branch's review fixes since 174563c, including 985eb94's
Ctr keystream zeroization, and its merge of feature/xof-cshake, so this
branch now contains the CCM branch's tip and PR #132 can be based on it:
the diff against it is the GCM work alone. Merged cleanly; ctr.rs is the
CCM branch's version plus this branch's start_at.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-mode: AES LightEngine CCM Mode) into feature/xof-cshake

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-mode: AES LightEngine GCM Mode) into feature/xof-cshake

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dghgit
dghgit merged commit 6705a3f into feature/simple-ciphers Sep 28, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants