SHA3 bug fixes: around partial-byte handling - #91
Open
ounsworth wants to merge 1 commit 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>
Contributor
Author
|
Review Tasks (for me):
I plan to just make changes directly to the branch rather than use github's review tool. |
This was referenced Aug 27, 2026
dghgit
approved these changes
Aug 28, 2026
Contributor
Author
|
dhg says:
|
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
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.
XOF::squeeze_partial_byte_final()squeezed raw Keccak, skipping the SHAKE1111suffix (FIPS 202 s. 6.2) when it was the first squeeze, and returned the high rather than the lownum_bitsbits. Now goes viasqueeze_out()and masks the low bits. The old test's output byte happened to be0xFF, which hid the high/low error.KeccakInternal::absorb_bits()returned early forbits == 0without 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_bitswas unvalidated before use as a shift amount: SHA-3 absorbed garbage for 8..15 and panicked atHash/XOFdocs: 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).