Feature/sha3 final patches

alpha_0.1.3_release_notes.md

@@ -3,3 +3,36 @@
 ## Major features
 
 ## Minor features / bug fixes
+
+Bit-oriented messages:
+
+* `Hash::do_final_partial_bits()` / `do_final_partial_bits_out()` accept `num_partial_bits` in 0..=7 (0 meaning the
+  message ends on a byte boundary); larger values return `HashError::InvalidLength` instead of panicking. The convention
+  is the same for every hash family: the trailing bits are in the least significant bits of `partial_byte` (FIPS 202
+  Appendix B.1) -- see the `Hash` trait docs, including the note on the MSB-first packing used by the NIST CAVP SHA-2
+  vector files.
+
+SHA-3 / SHAKE (PR #87):
+
+* Fixed `XOF::squeeze_partial_byte_final()`: when it was the first squeeze it bypassed the SHAKE `1111` domain suffix
+  and returned raw Keccak output, and it returned the *high* rather than the low `num_bits` bits of the output byte.
+  The existing test used `0xFF`, which masked the second error.
+* Fixed `XOF::absorb_last_partial_byte()` for `num_partial_bits == 4`: the 4 message bits plus the `1111` suffix
+  exactly filled a byte and the sponge did not switch to squeezing, so the first squeeze appended the suffix a second
+  time. Every SHAKE message with a bit length of 4 mod 8 was affected. Found by the new CAVP harness.
+* `absorb_last_partial_byte()` and `do_final_partial_bits*()` now validate `num_partial_bits` before use; previously
+  SHA-3 accepted 8..15 and absorbed garbage, panicked for >= 16, and SHAKE rejected 0 with an error message claiming
+  `[0,7]`.
+* Interleaving absorb -> squeeze -> absorb remains rejected with `HashError::InvalidState`; the `XOF` trait docs now
+  explain why (it is the duplex construction, not SHAKE).
+* `HashAlgParams` for the SHA-3 types is now forwarded from the `*Params` structs, so `OUTPUT_LEN` / `BLOCK_LEN` are
+  defined once. Removed misleading leftover SHA-2 block-size comments.
+* Crate docs gained "Memory Usage" and "Security Considerations" sections.
+
+Testing:
+
+* SHA-3 / SHAKE now run the NIST CAVP SHA3VS vector sets from bc-test-data (`crypto/sha3`: ShortMsg, LongMsg, Monte
+  Carlo and SHAKE VariableOut; bit- and byte-oriented, ~13k cases) using the same `../bc-test-data` lookup convention as
+  the mldsa/mlkem crates; the tests skip with a warning if the repo is not checked out. The vendored FIPS 202 example
+  vectors in `crypto/sha3/tests/data` were removed in favour of the bc-test-data copies. Note that `cargo mutants` runs
+  in a copied tree where `../bc-test-data` does not resolve, so these tests do not contribute to mutation coverage.

crypto/core/src/traits.rs

@@ -310,15 +310,31 @@ pub trait Hash: Algorithm + Default {
     fn do_final_out(self, output: &mut [u8]) -> usize;
 
     /// The same as [`Hash::do_final`], but allows for supplying a partial byte as the last input.
-    /// Assumes that the input is in the least significant bits (big endian).
+    /// The `num_partial_bits` message bits are taken from the least significant bits of
+    /// `partial_byte`, in order (bit 0 of `partial_byte` is the first message bit). This is the
+    /// FIPS 202 Appendix B.1 convention and is used uniformly for every hash family in this library,
+    /// including SHA-2, for which FIPS 180-4 defines no bit-to-byte packing. Note that the NIST CAVP
+    /// SHAVS (SHA-2) test vector files pack trailing bits MSB-first (left-justified) and must be
+    /// shifted right by `8 - num_partial_bits` before being passed here; the SHA3VS files already use
+    /// the LSB convention.
+    /// `num_partial_bits` must be in `0..=7`; 0 is valid and means the message ends on a byte
+    /// boundary (equivalent to [`Hash::do_final`]). Larger values return [`HashError::InvalidLength`].
     fn do_final_partial_bits(
         self,
         partial_byte: u8,
         num_partial_bits: usize,
     ) -> Result<Vec<u8>, HashError>;
 
     /// The same as [`Hash::do_final_out`], but allows for supplying a partial byte as the last input.
-    /// Assumes that the input is in the least significant bits (big endian).
+    /// The `num_partial_bits` message bits are taken from the least significant bits of
+    /// `partial_byte`, in order (bit 0 of `partial_byte` is the first message bit). This is the
+    /// FIPS 202 Appendix B.1 convention and is used uniformly for every hash family in this library,
+    /// including SHA-2, for which FIPS 180-4 defines no bit-to-byte packing. Note that the NIST CAVP
+    /// SHAVS (SHA-2) test vector files pack trailing bits MSB-first (left-justified) and must be
+    /// shifted right by `8 - num_partial_bits` before being passed here; the SHA3VS files already use
+    /// the LSB convention.
+    /// `num_partial_bits` must be in `0..=7`; 0 is valid and means the message ends on a byte
+    /// boundary (equivalent to [`Hash::do_final_out`]). Larger values return [`HashError::InvalidLength`].
     /// will be placed in the first [`Hash::output_len`] bytes.
     /// The entire output buffer is zeroized before the hash output is written.
     /// The return value is the number of bytes written.
@@ -1057,6 +1073,14 @@ pub trait SignatureVerifier<
 /// to break anonymity-preserving technology.
 /// Applications that require the arbitrary-length output of an XOF, but also care about these
 /// distinguishing attacks should consider adding a cryptographic salt to diversify the inputs.
+///
+/// # Absorbing after squeezing
+/// Once squeezing has begun, further calls to [`XOF::absorb`] / [`XOF::absorb_last_partial_byte`]
+/// return [`HashError::InvalidState`] and leave the object usable for further squeezing. FIPS 202
+/// defines SHAKE as a function of a single, complete message; the sponge's absorb/squeeze phases are
+/// internal to computing it. Interleaving absorb → squeeze → absorb → squeeze is the *duplex*
+/// construction, which is a different (unapproved) primitive whose output is not the SHAKE of any
+/// message and is not reproducible by other SHAKE implementations, so it is deliberately rejected.
 pub trait XOF: Default {
     /// A static one-shot API that digests the input data and produces `result_len` bytes of output.
     fn hash_xof(self, data: &[u8], result_len: usize) -> Vec<u8>;
@@ -1069,7 +1093,9 @@ pub trait XOF: Default {
     /// Absorb some amount of input.
     fn absorb(&mut self, data: &[u8]) -> Result<(), HashError>;
 
-    /// Switches to squeezing.
+    /// Absorbs the final `num_partial_bits` (`0..=7`, least significant bits of `partial_byte`) of the
+    /// message and switches to squeezing. 0 is valid and means the message ends on a byte boundary.
+    /// Values above 7 return [`HashError::InvalidLength`].
     fn absorb_last_partial_byte(
         &mut self,
         partial_byte: u8,
@@ -1084,8 +1110,11 @@ pub trait XOF: Default {
     /// The entire output buffer is zeroized before the output is written.
     fn squeeze_out(&mut self, output: &mut [u8]) -> usize;
 
-    /// Squeezes a partial byte from the XOF.
-    /// Output will be in the top `num_bits` bits of the returned u8 (ie Big Endian).
+    /// Squeezes a partial byte (`num_bits` in `1..=7`) from the XOF.
+    /// The bits are returned in the least significant `num_bits` bits of the returned u8, with the
+    /// remaining high bits zero. This follows the FIPS 202 Appendix B.1 bit-string convention
+    /// (the first bit of a byte is its least significant bit) and matches the input convention of
+    /// [`XOF::absorb_last_partial_byte`].
     /// This is a final call and consumes self.
     fn squeeze_partial_byte_final(self, num_bits: usize) -> Result<u8, HashError>;
 

crypto/sha3/src/keccak.rs

@@ -250,12 +250,14 @@ impl KeccakInternal {
         }
     }
 
+    /// Absorbs the final `bits` (0..=7, in the least significant bits of `data`) of the message and
+    /// switches the sponge to the squeezing phase. `bits == 0` means "no further bits": the sponge is
+    /// padded and switched to squeezing without absorbing anything. Callers that have already applied a
+    /// domain-separation suffix rely on this — if the switch did not happen here, a later squeeze would
+    /// see `squeezing == false` and apply the suffix a second time.
     pub(super) fn absorb_bits(&mut self, data: u8, bits: usize) -> Result<(), HashError> {
-        if bits == 0 {
-            return Ok(());
-        }
-        if !(1..=7).contains(&bits) {
-            return Err(HashError::InvalidLength("bits must be in the range 1 to 7"));
+        if bits > 7 {
+            return Err(HashError::InvalidLength("bits must be in the range 0 to 7"));
         }
         if (self.bits_in_queue & 7) != 0 {
             return Err(HashError::InvalidState("attempt to absorb with odd length queue"));
@@ -264,11 +266,13 @@ impl KeccakInternal {
             return Err(HashError::InvalidState("attempt to absorb while squeezing"));
         }
 
-        let mask = (1 << bits) - 1;
-        self.data_queue[self.bits_in_queue >> 3] = data & mask;
+        if bits != 0 {
+            let mask = (1 << bits) - 1;
+            self.data_queue[self.bits_in_queue >> 3] = data & mask;
 
-        // NOTE: After this, bits_in_queue is no longer a multiple of 8, so no more absorbs will work
-        self.bits_in_queue += bits;
+            // NOTE: After this, bits_in_queue is no longer a multiple of 8, so no more absorbs will work
+            self.bits_in_queue += bits;
+        }
         self.pad_and_switch_to_squeezing_phase();
         Ok(())
     }
@@ -500,18 +504,57 @@ mod keccak_tests {
     use super::*;
     use bouncycastle_hex as hex;
 
+    /// Basic sponge sanity: absorbing in one chunk or many gives the same output, successive
+    /// squeezes continue the stream (do not repeat), and different capacities give different output.
     #[test]
     fn test_keccak() {
-        let mut d = KeccakInternal::new(KeccakSize::_256);
         let m_vec = hex::decode("6d657373616765").unwrap();
+
+        let mut d = KeccakInternal::new(KeccakSize::_256);
         d.absorb(&m_vec);
+        let mut out1 = [0u8; 32];
+        d.squeeze(&mut out1);
+        let mut out2 = [0u8; 32];
+        d.squeeze(&mut out2);
+        assert_ne!(out1, [0u8; 32]);
+        assert_ne!(out1, out2, "successive squeezes must continue the output stream");
+
+        // chunked absorb + single 64-byte squeeze must reproduce out1 || out2
+        let mut d = KeccakInternal::new(KeccakSize::_256);
+        for b in &m_vec {
+            d.absorb(core::slice::from_ref(b));
+        }
+        let mut out64 = [0u8; 64];
+        d.squeeze(&mut out64);
+        assert_eq!(&out64[..32], &out1);
+        assert_eq!(&out64[32..], &out2);
 
-        let mut out = [0u8; 32];
-        d.squeeze(&mut out);
-        println!("n1: {:x?}", &out);
+        let mut d = KeccakInternal::new(KeccakSize::_512);
+        d.absorb(&m_vec);
+        let mut out_c512 = [0u8; 32];
+        d.squeeze(&mut out_c512);
+        assert_ne!(out_c512, out1);
+    }
 
-        d.squeeze(&mut out);
-        println!("n2: {:x?}", &out);
+    /// absorb_bits(): 0..=7 bits are accepted and always switch the sponge to squeezing (0 bits
+    /// included — see the doc comment); 8+ bits are rejected; a second call is rejected as squeezing.
+    #[test]
+    fn absorb_bits_range_and_phase() {
+        for bits in 0..=7usize {
+            let mut d = KeccakInternal::new(KeccakSize::_256);
+            d.absorb(b"abc");
+            d.absorb_bits(0xFF, bits).unwrap();
+            assert!(d.squeezing, "bits={bits}: must switch to squeezing");
+            assert!(matches!(d.absorb_bits(0, 1), Err(HashError::InvalidState(_))));
+        }
+        for bits in [8usize, 9, 16, usize::MAX] {
+            let mut d = KeccakInternal::new(KeccakSize::_256);
+            assert!(
+                matches!(d.absorb_bits(0, bits), Err(HashError::InvalidLength(_))),
+                "bits={bits}"
+            );
+            assert!(!d.squeezing, "rejected call must not change phase");
+        }
     }
 
     /// Regression test for from_serialized_state's validation of a not-yet-squeezing queue: a corrupt

crypto/sha3/src/lib.rs

@@ -89,7 +89,7 @@
 //! [`KDF`] acts on [`KeyMaterial`] objects as both the input and output values.
 //! In the case of SHA3, the [`KDF`] interfaces are simple wrapper functions around the underlying SHA3 or SHAKE
 //! primitive that correctly maintains the length and entropy metadata of the key material that it is acting on.
-//! This is intended to act as a developer ait to prevent  some classes of developer mistakes, such as
+//! This is intended to act as a developer aid to prevent some classes of developer mistakes, such as
 //! deriving a cryptographic key from uninitialized (aka zeroized) input key material, or using low-entropy
 //! input key material to derive a MAC, symmetric, or asymmetric key.
 //!
@@ -108,6 +108,31 @@
 //! [`KeyType::CryptographicRandom`] since the input [`KeyMaterial`] is 16 bytes but [`SHA3_256`] needs at least 32 bytes of
 //! full-entropy input key material in order to be able to produce full entropy output key material.
 //!
+//! # Memory Usage
+//!
+//! All SHA3 and SHAKE variants share the same Keccak-f\[1600\] sponge and so have identical memory
+//! footprints. No heap memory is used by the algorithms themselves; the `Vec<u8>`-returning
+//! convenience methods allocate only the output buffer, and the `*_out` variants allocate nothing.
+//!
+//! | Object                                  | Size (bytes) |
+//! |-----------------------------------------|--------------|
+//! | `SHA3_224` .. `SHA3_512`, `SHAKE128/256` | 440          |
+//! | Suspended state ([`Suspendable`])       | 415          |
+//!
+//! Sizes are `core::mem::size_of` values reported by `mem_usage_benches/bench_sha3_mem_usage.rs`
+//! (`cargo run --release -p mem_usage_benches --bin bench_sha3_mem_usage`), which also has valgrind
+//! massif entry points for measuring peak stack usage of the hash, XOF and suspend/resume paths.
+//!
+//! # Security Considerations
+//!
+//! * SHA3-224/256/384/512 offer 112/128/192/256 bits of collision resistance respectively; SHAKE128
+//!   and SHAKE256 offer 128 and 256 bits of security for output lengths at least twice that size
+//!   (FIPS 202 Appendix A.1).
+//! * SHAKE is an XOF, not a hash: `SHAKE128(m, 32)` is a prefix of `SHAKE128(m, 64)`. If the output
+//!   length must be bound to the digest, include it in the message (FIPS 202 Appendix A.2).
+//! * The sponge state and queue are held in [`bouncycastle_utils::secret::Secret`] and zeroized on
+//!   drop.
+//!
 //! # Suspending and resuming execution
 //!
 //! When hashing a large message, it can be advantageous to be able to suspend the operation
@@ -160,17 +185,17 @@ mod sha3;
 mod shake;
 
 /*** String constants ***/
-///
+/// Algorithm name string for SHA3-224, as used by the factories and CLI.
 pub const SHA3_224_NAME: &str = "SHA3-224";
-///
+/// Algorithm name string for SHA3-256, as used by the factories and CLI.
 pub const SHA3_256_NAME: &str = "SHA3-256";
-///
+/// Algorithm name string for SHA3-384, as used by the factories and CLI.
 pub const SHA3_384_NAME: &str = "SHA3-384";
-///
+/// Algorithm name string for SHA3-512, as used by the factories and CLI.
 pub const SHA3_512_NAME: &str = "SHA3-512";
-///
+/// Algorithm name string for SHAKE128, as used by the factories and CLI.
 pub const SHAKE128_NAME: &str = "SHAKE128";
-///
+/// Algorithm name string for SHAKE256, as used by the factories and CLI.
 pub const SHAKE256_NAME: &str = "SHAKE256";
 
 /*** pub types ***/
@@ -205,11 +230,13 @@ trait SHA3Params: HashAlgParams {
 
 // TODO: it would probably be more elegant to macro these.
 
-impl HashAlgParams for SHA3_224 {
-    const OUTPUT_LEN: usize = 28;
-    // const BLOCK_LEN: usize = 64;
-    const BLOCK_LEN: usize = 144; // FIPS 202 Table 3
+/// The public hash types expose the same parameters as their `*Params` marker, so the constants
+/// are defined exactly once (on the params struct) and forwarded here.
+impl<PARAMS: SHA3Params> HashAlgParams for SHA3Internal<PARAMS> {
+    const OUTPUT_LEN: usize = PARAMS::OUTPUT_LEN;
+    const BLOCK_LEN: usize = PARAMS::BLOCK_LEN;
 }
+
 /// The parameters for SHA3_224.
 #[derive(Clone)]
 pub struct SHA3_224Params;
@@ -219,7 +246,6 @@ impl Algorithm for SHA3_224Params {
 }
 impl HashAlgParams for SHA3_224Params {
     const OUTPUT_LEN: usize = 28;
-    // const BLOCK_LEN: usize = 64;
     const BLOCK_LEN: usize = 144; // FIPS 202 Table 3
 }
 impl SHA3Params for SHA3_224Params {
@@ -233,11 +259,6 @@ impl AlgorithmOID for SHA3_224 {
         &[0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x07];
 }
 
-impl HashAlgParams for SHA3_256 {
-    const OUTPUT_LEN: usize = 32;
-    // const BLOCK_LEN: usize = 64;
-    const BLOCK_LEN: usize = 136; // FIPS 202 Table 3
-}
 /// The parameters for SHA3_256.
 #[derive(Clone)]
 pub struct SHA3_256Params;
@@ -247,7 +268,6 @@ impl Algorithm for SHA3_256Params {
 }
 impl HashAlgParams for SHA3_256Params {
     const OUTPUT_LEN: usize = 32;
-    // const BLOCK_LEN: usize = 64;
     const BLOCK_LEN: usize = 136; // FIPS 202 Table 3
 }
 impl SHA3Params for SHA3_256Params {
@@ -263,18 +283,12 @@ impl AlgorithmOID for SHA3_256 {
 /// The parameters for SHA3_384.
 #[derive(Clone)]
 pub struct SHA3_384Params;
-impl HashAlgParams for SHA3_384 {
-    const OUTPUT_LEN: usize = 48;
-    // const BLOCK_LEN: usize = 128;
-    const BLOCK_LEN: usize = 104; // FIPS 202 Table 3
-}
 impl Algorithm for SHA3_384Params {
     const ALG_NAME: &'static str = SHA3_384_NAME;
     const MAX_SECURITY_STRENGTH: SecurityStrength = SecurityStrength::_192bit;
 }
 impl HashAlgParams for SHA3_384Params {
     const OUTPUT_LEN: usize = 48;
-    // const BLOCK_LEN: usize = 128;
     const BLOCK_LEN: usize = 104; // FIPS 202 Table 3
 }
 impl SHA3Params for SHA3_384Params {
@@ -290,18 +304,12 @@ impl AlgorithmOID for SHA3_384 {
 /// The parameters for SHA3_512.
 #[derive(Clone)]
 pub struct SHA3_512Params;
-impl HashAlgParams for SHA3_512 {
-    const OUTPUT_LEN: usize = 64;
-    // const BLOCK_LEN: usize = 128;
-    const BLOCK_LEN: usize = 72; // FIPS 202 Table 3
-}
 impl Algorithm for SHA3_512Params {
     const ALG_NAME: &'static str = SHA3_512_NAME;
     const MAX_SECURITY_STRENGTH: SecurityStrength = SecurityStrength::_256bit;
 }
 impl HashAlgParams for SHA3_512Params {
     const OUTPUT_LEN: usize = 64;
-    // const BLOCK_LEN: usize = 128;
     const BLOCK_LEN: usize = 72; // FIPS 202 Table 3
 }
 impl SHA3Params for SHA3_512Params {