Refactor/sha3 code structure - #93
Open
ounsworth wants to merge 2 commits into
Open
Conversation
…artial-bit validation * `XOF::squeeze_partial_byte_final()` squeezed raw Keccak, skipping the SHAKE `1111` suffix (FIPS 202 s. 6.2) when it was the first squeeze, and returned the high rather than the low `num_bits` bits. Now goes via `squeeze_out()` and masks the low bits. The old test's output byte happened to be `0xFF`, which hid the high/low error. * `KeccakInternal::absorb_bits()` returned early for `bits == 0` without switching to squeezing, so a suffix already folded into a whole byte was applied a second time. Broke every SHAKE message of bit length 4 mod 8 (6 mod 8 for SHA-3). Now accepts 0..=7 and always switches phase. * `num_partial_bits` was unvalidated before use as a shift amount: SHA-3 absorbed garbage for 8..15 and panicked at >= 16; SHAKE rejected 0. Both now accept 0..=7 (0 meaning the message ends on a byte boundary) and return `HashError::InvalidLength` otherwise. * `Hash` / `XOF` docs: state the FIPS 202 Appendix B.1 bit ordering, note the opposite MSB-first packing in the CAVP SHAVS (SHA-2) vector files, and explain why absorb-after-squeeze is rejected (duplex, not SHAKE). * Regression tests for each fix, including the 4-bit SHAKE128 vector from the CAVP SHA3VS bit-oriented set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rams structs Two structural cleanups with no change in externally-visible behaviour. do_final_out() and do_final_partial_bits_out() each had their own copy of the FIPS 202 s. 6.1 finalization: append the "01" suffix above any trailing message bits, absorb a whole byte first if the two together fill one, then squeeze with truncation to OUTPUT_LEN. Both now delegate to a private do_final_bits_out(), with the whole-byte case expressed as num_partial_bits == 0. The helper is infallible -- it consumes self, the queue is byte-aligned, and the range check stays at the public entry point -- so the absorb_bits() Result is discharged with a documented expect() rather than propagated. HashAlgParams was implemented four times on the public alias types (SHA3_224 .. SHA3_512) and again on each *Params struct, with the two copies of OUTPUT_LEN / BLOCK_LEN kept in sync by hand. A single blanket impl over SHA3Internal<PARAMS> now forwards from the params struct, which is also where FIPS 202 Table 3 is cited. Note SHA3_224 etc. are aliases for SHA3Internal<...>, so the resolved constants are unchanged. Also in this file: the stale commented-out SHA-2 block sizes next to the FIPS 202 Table 3 values, an "ait" -> "aid" typo in the crate docs, and six empty /// on the algorithm-name constants. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Break up of #87, so this is actually dgh's submission. So I will be reviewing / approving.
Stacks on top of #91