Add NIST CAVP tests to the SHA3 crate - #92
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>
Adds crypto/sha3/tests/cavp_tests.rs, covering the CAVP SHA3VS sets for SHA3-224/256/384/512 and SHAKE128/256 in both bit- and byte-oriented form: ShortMsg, LongMsg, Monte Carlo (SHA3VS s. 6.2.2 / 6.2.3) and SHAKE VariableOut, ~13k cases in 40 test groups. Unlike the SHA-2 CAVP files, the SHA-3 files follow the FIPS 202 Appendix B.1 convention: the excess bits of a Len-bit message occupy the least significant bits of the final Msg byte, and likewise for an Outputlen-bit SHAKE output. That matches this library's convention, so no shifting is needed; the harness asserts the high bits are zero on every partial case in the files. This harness is what found the 4-mod-8 double-suffix bug fixed in the previous commit: with that fix reverted, the four bit-oriented SHAKE groups fail at Len = 4 and Len = 6724 (both 4 mod 8). The vendored FIPS 202 example-vector files in crypto/sha3/tests/data are replaced by the bc-test-data copies, using the same two-path lookup as the mldsa and mlkem crates: the vector tests print a warning and pass vacuously when ../bc-test-data is not checked out. Note that this means they contribute nothing to cargo mutants, which runs in a copied tree where that path does not resolve. Also replaces the println!-only keccak test_keccak with real assertions: chunked and single-shot absorb agree, successive squeezes continue the stream rather than repeating, and a different capacity gives different output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Review tasks:
|
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.
This stacks on top of #91