Various code hygiene issues (#858)

* Use direct sha2 updates and clarify sizes

* Fix blake3 hash elements

* Allow padded Digest192 hex and match zeroize paths

* Reject all-zero X25519 shared secret

* Handle zeroize generics and target dir

* Add subtree format header

* Use trailing zero checks in benches

* Fix partial MMR tracking

* Reject unknown subtree versions

* chore: comment adjustment

* Fix zeroize-audit target path

* Clarify Digest192 parsing and add check-features

* Use subtle for zero shared secret check

* addressing review comments

Remove legacy deserialization paths for PartialMmr and subtree data. Require the current marker or magic header and reject old formats.

Add small comments that explain key behavior: empty-input handling in x25519 all-zero checks, mutation dedup order in subtree updates, marker purpose in PartialMmr, and invalid leaf position handling in untrack().

Fix outdated docs wording in padded_elements_to_bytes and update tests to match the hard cutoff behavior.

Author: huitseeker

Makefile

@@ -57,7 +57,9 @@ cargo-deny: ## Run cargo-deny to check dependencies for security vulnerabilities
 .PHONY: zeroize-audit
 zeroize-audit: ## Run Zeroize audit using rustdoc JSON
 	cargo +nightly rustdoc -p miden-crypto --all-features -- -Zunstable-options --output-format json --document-private-items
-	cargo run --quiet --manifest-path tools/zeroize-audit/Cargo.toml -- target/doc/miden_crypto.json
+	@target_dir="$${CARGO_TARGET_DIR:-target}"; \
+	if [ "$$target_dir" = "/" ]; then target_dir=target; fi; \
+	cargo run --quiet --manifest-path tools/zeroize-audit/Cargo.toml -- "$$target_dir/doc/miden_crypto.json"
 
 .PHONY: lint
 lint: format fix clippy toml typos-check machete cargo-deny ## Run all linting tasks at once (Clippy, fixing, formatting, machete, cargo-deny)
@@ -100,6 +102,11 @@ test: test-default test-no-std test-docs test-large-smt ## Run all tests except
 check: ## Check all targets and features for errors without code generation
 	cargo check --all-targets --all-features
 
+.PHONY: check-features
+check-features: ## Check miden-crypto feature combinations
+	cargo check -p miden-crypto --all-targets --no-default-features
+	cargo check -p miden-crypto --all-targets --features ${ALL_FEATURES_EXCEPT_ROCKSDB}
+
 .PHONY: check-fuzz
 check-fuzz: ## Check miden-crypto-fuzz compilation
 	cd miden-crypto-fuzz && cargo check

miden-crypto/benches/common/macros.rs

@@ -779,17 +779,17 @@ macro_rules! benchmark_rand_draw_integers {
 macro_rules! benchmark_rand_check_leading_zeros {
     ($func_name:ident, $coin_type:ty, $seed:expr) => {
         fn $func_name(c: &mut Criterion) {
-            let mut group = c.benchmark_group("rand-check-leading-zeros");
+            let mut group = c.benchmark_group("rand-check-trailing-zeros");
             group.measurement_time($crate::common::config::DEFAULT_MEASUREMENT_TIME);
             group.sample_size($crate::common::config::DEFAULT_SAMPLE_SIZE);
 
             let coin = <$coin_type>::new($seed);
             let test_values: Vec<u64> = (0u64..100).collect();
 
-            group.bench_function("check_leading_zeros", |b| {
+            group.bench_function("check_trailing_zeros", |b| {
                 b.iter(|| {
                     for &value in &test_values {
-                        let _zeros = coin.check_leading_zeros(black_box(value));
+                        let _zeros = coin.check_trailing_zeros(black_box(value));
                     }
                 });
             });

miden-crypto/src/aead/aead_poseidon2/mod.rs

@@ -147,7 +147,8 @@ impl AuthTag {
 
 impl PartialEq for AuthTag {
     fn eq(&self, other: &Self) -> bool {
-        // Use constant-time comparison to prevent timing attacks
+        // Use constant-time comparison on non-wasm targets to reduce timing leaks. On wasm, the
+        // canonicalization helper is a host call and not guaranteed constant-time.
         let mut result = true;
         for (a, b) in self.0.iter().zip(other.0.iter()) {
             result &= bool::from(a.as_canonical_u64_ct().ct_eq(&b.as_canonical_u64_ct()));

miden-crypto/src/ecdh/mod.rs

@@ -61,4 +61,6 @@ pub(crate) trait KeyAgreementScheme {
 pub(crate) enum KeyAgreementError {
     #[error("hkdf expansion failed")]
     HkdfExpansionFailed,
+    #[error("shared secret is invalid")]
+    InvalidSharedSecret,
 }

miden-crypto/src/ecdh/x25519.rs

@@ -17,6 +17,7 @@ use alloc::vec::Vec;
 use hkdf::{Hkdf, hmac::SimpleHmac};
 use k256::sha2::Sha256;
 use rand::{CryptoRng, RngCore};
+use subtle::ConstantTimeEq;
 
 use crate::{
     dsa::eddsa_25519_sha512::{PublicKey, SecretKey},
@@ -200,7 +201,9 @@ impl KeyAgreementScheme for X25519 {
         static_pk: &Self::PublicKey,
     ) -> Result<Self::SharedSecret, super::KeyAgreementError> {
         let shared = ephemeral_sk.diffie_hellman(static_pk);
-        debug_assert!(!shared.as_ref().iter().all(|byte| *byte == 0), "all-zero shared secret");
+        if is_all_zero(shared.as_ref()) {
+            return Err(super::KeyAgreementError::InvalidSharedSecret);
+        }
         Ok(shared)
     }
 
@@ -209,7 +212,9 @@ impl KeyAgreementScheme for X25519 {
         ephemeral_pk: &Self::EphemeralPublicKey,
     ) -> Result<Self::SharedSecret, super::KeyAgreementError> {
         let shared = static_sk.get_shared_secret(ephemeral_pk.clone());
-        debug_assert!(!shared.as_ref().iter().all(|byte| *byte == 0), "all-zero shared secret");
+        if is_all_zero(shared.as_ref()) {
+            return Err(super::KeyAgreementError::InvalidSharedSecret);
+        }
         Ok(shared)
     }
 
@@ -226,6 +231,15 @@ impl KeyAgreementScheme for X25519 {
     }
 }
 
+fn is_all_zero(bytes: &[u8]) -> bool {
+    // Empty input is treated as invalid caller input rather than "all zero".
+    if bytes.is_empty() {
+        return false;
+    }
+    let acc = bytes.iter().fold(0u8, |acc, &byte| acc | byte);
+    acc.ct_eq(&0u8).into()
+}
+
 // TESTS
 // ================================================================================================
 
@@ -235,7 +249,8 @@ mod tests {
 
     use super::*;
     use crate::{
-        dsa::eddsa_25519_sha512::SecretKey, rand::test_utils::seeded_rng, utils::Deserializable,
+        dsa::eddsa_25519_sha512::SecretKey, ecdh::KeyAgreementError, rand::test_utils::seeded_rng,
+        utils::Deserializable,
     };
 
     #[test]
@@ -277,6 +292,27 @@ mod tests {
         assert!(result.is_err());
     }
 
+    #[test]
+    fn exchange_static_ephemeral_rejects_zero_shared_secret() {
+        let mut rng = seeded_rng([0u8; 32]);
+        let static_sk = SecretKey::with_rng(&mut rng);
+
+        let low_order_bytes = EIGHT_TORSION[0].to_montgomery().to_bytes();
+        let low_order_pk = EphemeralPublicKey {
+            inner: x25519_dalek::PublicKey::from(low_order_bytes),
+        };
+
+        let result = X25519::exchange_static_ephemeral(&static_sk, &low_order_pk);
+        assert!(matches!(result, Err(KeyAgreementError::InvalidSharedSecret)));
+    }
+
+    #[test]
+    fn is_all_zero_accepts_arbitrary_lengths() {
+        assert!(!is_all_zero(&[]));
+        assert!(is_all_zero(&[0u8; 16]));
+        assert!(!is_all_zero(&[0u8, 1u8, 0u8, 0u8]));
+    }
+
     fn find_twist_point_bytes() -> [u8; 32] {
         let mut bytes = [0u8; 32];
         for i in 0u16..=u16::MAX {

miden-crypto/src/hash/blake/mod.rs

@@ -125,7 +125,7 @@ impl Blake3_192 {
 
     /// Returns a hash of the provided field elements.
     #[inline(always)]
-    pub fn hash_elements<E: BasedVectorSpace<Felt>>(elements: &[E]) -> Digest256 {
+    pub fn hash_elements<E: BasedVectorSpace<Felt>>(elements: &[E]) -> Digest192 {
         Digest::new(hash_elements(elements))
     }
 

miden-crypto/src/hash/blake/tests.rs

@@ -21,6 +21,18 @@ fn blake3_hash_elements() {
     assert_eq!(&expected, &actual);
 }
 
+#[test]
+fn blake3_256_hash_elements_matches_hash() {
+    let elements = rand_vector::<Felt>(17);
+    let expected = Blake3_256::hash_elements(&elements);
+    let mut bytes = Vec::new();
+    for element in elements.iter() {
+        bytes.extend_from_slice(&((*element).as_canonical_u64()).to_le_bytes());
+    }
+    let actual = Blake3_256::hash(&bytes);
+    assert_eq!(expected, actual);
+}
+
 proptest! {
     #[test]
     fn blake192_wont_panic_with_arbitrary_input(ref vec in any::<Vec<u8>>()) {

miden-crypto/src/hash/digest.rs

@@ -28,6 +28,8 @@ pub const DIGEST512_BYTES: usize = 64;
 // ================================================================================================
 
 /// A 192-bit (24-byte) digest. Type alias for `Digest<24>`.
+///
+/// Hex parsing also accepts zero-padded 32-byte encodings for backward compatibility.
 pub type Digest192 = Digest<DIGEST192_BYTES>;
 
 /// A 256-bit (32-byte) digest. Type alias for `Digest<32>`.
@@ -116,6 +118,28 @@ impl<const N: usize> TryFrom<&str> for Digest<N> {
     type Error = HexParseError;
 
     fn try_from(value: &str) -> Result<Self, Self::Error> {
+        if N == DIGEST192_BYTES {
+            let short_len = (DIGEST192_BYTES * 2) + 2;
+            let long_len = (DIGEST256_BYTES * 2) + 2;
+
+            if value.len() == short_len {
+                let bytes = hex_to_bytes::<N>(value)?;
+                return Ok(Self(bytes));
+            }
+
+            if value.len() == long_len {
+                let bytes = hex_to_bytes::<DIGEST256_BYTES>(value)?;
+                let padding = &bytes[DIGEST192_BYTES..];
+                if padding.iter().all(|byte| *byte == 0) {
+                    let mut trimmed = [0u8; N];
+                    trimmed.copy_from_slice(&bytes[..N]);
+                    return Ok(Self(trimmed));
+                }
+            }
+
+            return Err(HexParseError::InvalidLength { expected: short_len, actual: value.len() });
+        }
+
         hex_to_bytes(value).map(Self)
     }
 }
@@ -198,6 +222,20 @@ mod tests {
         assert_eq!(digest.as_bytes(), &bytes);
     }
 
+    #[test]
+    fn test_digest192_accepts_zero_padded_hex() {
+        let mut bytes = [0u8; DIGEST192_BYTES];
+        bytes[0] = 1;
+        bytes[23] = 255;
+
+        let mut padded = [0u8; DIGEST256_BYTES];
+        padded[..DIGEST192_BYTES].copy_from_slice(&bytes);
+
+        let hex = bytes_to_hex_string(padded);
+        let parsed = Digest192::try_from(hex.as_str()).expect("digest192 should accept padding");
+        assert_eq!(parsed.as_bytes(), &bytes);
+    }
+
     #[test]
     fn test_digest_hex_roundtrip_32() {
         let bytes = [0xab; 32];

miden-crypto/src/hash/sha2/mod.rs

@@ -162,27 +162,14 @@ where
         const { assert!(FELT_BYTES == 8, "buffer arithmetic assumes 8-byte field elements") };
 
         let mut hasher = sha2::Sha256::new();
-        // SHA-256 block size: 64 bytes
-        let mut buf = [0_u8; 64];
-        let mut buf_offset = 0;
 
         for elem in elements.iter() {
             for &felt in E::as_basis_coefficients_slice(elem) {
-                buf[buf_offset..buf_offset + FELT_BYTES]
-                    .copy_from_slice(&felt.as_canonical_u64().to_le_bytes());
-                buf_offset += FELT_BYTES;
-
-                if buf_offset == 64 {
-                    hasher.update(buf);
-                    buf_offset = 0;
-                }
+                let felt_bytes = felt.as_canonical_u64().to_le_bytes();
+                hasher.update(felt_bytes);
             }
         }
 
-        if buf_offset > 0 {
-            hasher.update(&buf[..buf_offset]);
-        }
-
         hasher.finalize()
     };
     digest.into()
@@ -198,27 +185,14 @@ where
         const { assert!(FELT_BYTES == 8, "buffer arithmetic assumes 8-byte field elements") };
 
         let mut hasher = sha2::Sha512::new();
-        // SHA-512 block size: 128 bytes
-        let mut buf = [0_u8; 128];
-        let mut buf_offset = 0;
 
         for elem in elements.iter() {
             for &felt in E::as_basis_coefficients_slice(elem) {
-                buf[buf_offset..buf_offset + FELT_BYTES]
-                    .copy_from_slice(&felt.as_canonical_u64().to_le_bytes());
-                buf_offset += FELT_BYTES;
-
-                if buf_offset == 128 {
-                    hasher.update(buf);
-                    buf_offset = 0;
-                }
+                let felt_bytes = felt.as_canonical_u64().to_le_bytes();
+                hasher.update(felt_bytes);
             }
         }
 
-        if buf_offset > 0 {
-            hasher.update(&buf[..buf_offset]);
-        }
-
         hasher.finalize()
     };
     digest.into()