XOF extends Hash: SHAKE128 and SHAKE256 are hashes, and squeezing gets its own type - #118
Conversation
|
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:
in the QUALITY_AND_STYLE.md as a higher priority than turning runtime errors into compile-time errors. |
| pub trait XofOutput { | ||
| /// Produces the next `num_bytes` bytes of the output stream. | ||
| /// | ||
| /// BC Java's `Xof.doOutput(out, outOff, outLen)`. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| /// 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 { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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].
There was a problem hiding this comment.
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.
|
|
||
| /// 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; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
| //! 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. |
There was a problem hiding this comment.
Yeah, see, that comment directly and emphatically contradicts FIPS 202 appdx. A.2, and that makes me comfortable.
There was a problem hiding this comment.
See the last paragraph of A.2. The comment is correct.
|
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. |
b212aea to
a7dd1a4
Compare
a7dd1a4 to
9006d21
Compare
d2a9b35 to
d1dcf75
Compare
9006d21 to
e909d5e
Compare
e909d5e to
9006d21
Compare
2fec0ab to
5c45ae8
Compare
|
cSHAKE, TupleHash, ParallelHash, and KMAC now added. |
|
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...) |
|
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 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. |
|
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. |
|
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: 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. |
…ueezing becomes its own type
…oFinal after doOutput
… encodings and cshake CLI subcommands
…tions and the commit message style in CLAUDE.md
…tory registration and kmac CLI subcommands
…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>
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
XOFnow extendsHash, so SHAKE128 and SHAKE256 are hashes and can be used wherever one iswanted; 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-cipherand shows four commits:e3c31e3XOFextendsHash; squeezing becomes its own typed02361eblock_bitlenandoutput_lenvalues, which three mutants survived19f00a0XofOutputgainsdo_final/do_final_outa7dd1a4The 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 "somehash" and pass a SHAKE.
What blocked it.
Hash::do_updateis infallible;XOF::absorbreturnedHashError::InvalidStatefor absorb-after-squeeze. BC Java has the same rule but signals it with anunchecked
IllegalStateExceptionfromKeccakDigest.absorb, which is not an option here. So theerror had to go somewhere, and the three candidates were: make
Hash::do_updatefallibleeverywhere, 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 theerror could not happen in correct code, which is exactly the case QUALITY_AND_STYLE.md says to
redesign rather than wrap in a
Result:So
XOF::into_outputconsumes the XOF and returns anXofOutput. After output has begun there isno value left to call
do_updateon, 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_updateinfallible is that aSHAKEInternala caller can name hasnever squeezed. That holds because every
KDFentry point also takesselfby value, and it ispinned by a
debug_assert!indo_updateplus acompile_faildoctest.XofOutput
Sample code — all five of these compile and their assertions pass:
And the case that no longer compiles, which is the point of the change:
Method mapping.
Digest.getDigestSize()Hash::output_len— 32 / 64,fixedOutputLength / 4asSHAKEDigest.java:84ExtendedDigest.getByteLength()Hash::block_bitlen, in bits — 1344 / 1088, the rate1600 - 2cDigest.updateHash::do_updateDigest.doFinal(out, off)Hash::do_finalXof.doOutput(out, off, len)XOF::into_outputthenXofOutput::do_outputXof.doFinal(out, off, len)afterdoOutputXofOutput::do_finalDigest.reset()selfXofOutput::do_finalis a provided method delegating todo_output.SHAKEDigest.java:92-99shows Java's
doFinal(out, off, outLen)isdoOutputplusreset(); here takingselfby valueis the reset, and dropping the handle zeroizes the sponge through
Secret'sDrop. It is a namefor "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.
SHAKEOutputtherefore has its ownCloneandSuspendable, and eachfrom_suspendedrejects the other's phase withInvalidData. This is abehaviour change:
SHAKE128::from_suspendednow refuses a state that was suspended mid-output.Alternatives considered. Making
Hash::do_updatefallible would have put aResulton SHA-2,SHA-3 and SM3 for a condition only XOFs can reach. Panicking is against house rules. A shared
metadata-only supertrait for
HashandXOFwas considered and rejected: it avoids thedo_updateproblem but does not give substitutability, which was the whole point.Scope and Risk
core,core-test-framework,sha3,factory,mlkem,mlkem-lowmemory,mldsa,mldsa-lowmemory,cli,mem_usage_benches.openssl dgstbyte forbyte 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_suspendedrejecting a squeezing state.this is a breaking change to
XOFand every call site moved.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 mutantsonshake.rsagainst 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_bitlenoroutput_len, so1600 - 2csurviving as1600 + 2cwent unnoticed.d02361eadds that test and all three now die. The fourth is a genuine equivalent mutant,
|versus^ondisjoint bit ranges, verified exhaustively over all 2048
(num_bits, partial_byte)pairs andcommented at the site. The 33 unviable are the
.cargo/mutants.tomlerror values not typecheckingagainst
HashError/KDFError/SuspendableError.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-opus-5