From aadd7df2660a59319c47dd8da24861bd804e20a3 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sun, 9 Feb 2025 20:11:46 +0000 Subject: [PATCH 1/3] hashes: move `from_engine` fn impl out of macro body This macro is pretty weird -- it requires that a freestanding `from_engine` method exists, which it uses to implement a `from_engine` method within an impl block, by just calling through to the freestanding method. To reduce indirection, at a very small cost in increased repeated code (we now need to add a `impl Hash {` and `}` and doccomment around each freestanding function, we just remove this from the macro entirely. This will allow us to implement `from_engine` for `sha256t::Hash` in a different way than for the non-generic hash types. To minimize the diff, we do not re-indent the freestanding `from_engine`. We will do that in the next commit. However, the diff is still a bit noisy because I had to replace `fn from_engine` with `pub fn from_engine` in every case. I took the opportunity to also change the return type from `Hash` to `Self` to make it clearer that these are constructors. --- hashes/src/hash160/mod.rs | 6 +++- hashes/src/internal_macros.rs | 13 +++---- hashes/src/ripemd160/mod.rs | 9 +++-- hashes/src/sha1/mod.rs | 5 ++- hashes/src/sha256/mod.rs | 64 ++++++++++++++++++----------------- hashes/src/sha256d/mod.rs | 5 ++- hashes/src/sha256t/mod.rs | 2 +- hashes/src/sha384/mod.rs | 7 ++-- hashes/src/sha512/mod.rs | 8 +++-- hashes/src/sha512_256/mod.rs | 7 ++-- hashes/src/siphash24/mod.rs | 18 +++++----- 11 files changed, 82 insertions(+), 62 deletions(-) diff --git a/hashes/src/hash160/mod.rs b/hashes/src/hash160/mod.rs index 3c457c1aa..406bdc5ab 100644 --- a/hashes/src/hash160/mod.rs +++ b/hashes/src/hash160/mod.rs @@ -15,7 +15,10 @@ crate::internal_macros::general_hash_type! { "Output of the Bitcoin HASH160 hash function. (RIPEMD160(SHA256))" } -fn from_engine(e: HashEngine) -> Hash { + +impl Hash { +/// Finalize a hash engine to produce a hash. +pub fn from_engine(e: HashEngine) -> Self { let sha2 = sha256::Hash::from_engine(e.0); let rmd = ripemd160::Hash::hash(sha2.as_byte_array()); @@ -23,6 +26,7 @@ fn from_engine(e: HashEngine) -> Hash { ret.copy_from_slice(rmd.as_byte_array()); Hash(ret) } +} /// Engine to compute HASH160 hash function. #[derive(Debug, Clone)] diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index d325d13b7..f0f410167 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -15,10 +15,7 @@ /// /// Restrictions on usage: /// -/// * There must be a free-standing `fn from_engine(HashEngine) -> Hash` in the scope -/// * `fn internal_new([u8; $bits / 8]) -> Self` must exist on `Hash` -/// -/// `from_engine` obviously implements the finalization algorithm. +/// * The hash type must implement the `GeneralHash` trait. macro_rules! hash_trait_impls { ($bits:expr, $reverse:expr $(, $gen:ident: $gent:ident)*) => { $crate::impl_bytelike_traits!(Hash, { $bits / 8 } $(, $gen: $gent)*); @@ -62,8 +59,9 @@ pub(crate) use hash_trait_impls; /// * `$reverse` - `true` if the hash should be displayed backwards, `false` otherwise /// * `$doc` - doc string to put on the type /// -/// The `from_engine` free-standing function is still required with this macro. See the doc of -/// [`hash_trait_impls`]. +/// Restrictions on usage: +/// +/// * The hash type must implement the `GeneralHash` trait. macro_rules! general_hash_type { ($bits:expr, $reverse:expr, $doc:literal) => { /// Hashes some bytes. @@ -93,9 +91,6 @@ macro_rules! general_hash_type { $crate::internal_macros::hash_type_no_default!($bits, $reverse, $doc); impl Hash { - /// Produces a hash from the current state of a given engine. - pub fn from_engine(e: HashEngine) -> Hash { from_engine(e) } - /// Constructs a new engine. pub fn engine() -> HashEngine { Default::default() } diff --git a/hashes/src/ripemd160/mod.rs b/hashes/src/ripemd160/mod.rs index bb2941455..7b0e5377f 100644 --- a/hashes/src/ripemd160/mod.rs +++ b/hashes/src/ripemd160/mod.rs @@ -20,8 +20,11 @@ crate::internal_macros::general_hash_type! { "Output of the RIPEMD160 hash function." } + +impl Hash { +/// Finalize a hash engine to produce a hash. #[cfg(not(hashes_fuzz))] -fn from_engine(mut e: HashEngine) -> Hash { +pub fn from_engine(mut e: HashEngine) -> Self { // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining let n_bytes_hashed = e.bytes_hashed; @@ -40,12 +43,14 @@ fn from_engine(mut e: HashEngine) -> Hash { Hash(e.midstate()) } +/// Finalize a hash engine to produce a hash. #[cfg(hashes_fuzz)] -fn from_engine(e: HashEngine) -> Hash { +pub fn from_engine(e: HashEngine) -> Self { let mut res = e.midstate(); res[0] ^= (e.bytes_hashed & 0xff) as u8; Hash(res) } +} const BLOCK_SIZE: usize = 64; diff --git a/hashes/src/sha1/mod.rs b/hashes/src/sha1/mod.rs index 69ca30897..ae49c364f 100644 --- a/hashes/src/sha1/mod.rs +++ b/hashes/src/sha1/mod.rs @@ -20,7 +20,9 @@ crate::internal_macros::general_hash_type! { "Output of the SHA1 hash function." } -fn from_engine(mut e: HashEngine) -> Hash { +impl Hash { +/// Finalize a hash engine to produce a hash. +pub fn from_engine(mut e: HashEngine) -> Self { // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining let n_bytes_hashed = e.bytes_hashed; @@ -38,6 +40,7 @@ fn from_engine(mut e: HashEngine) -> Hash { Hash(e.midstate()) } +} const BLOCK_SIZE: usize = 64; diff --git a/hashes/src/sha256/mod.rs b/hashes/src/sha256/mod.rs index a45eb0bfe..1def47bf5 100644 --- a/hashes/src/sha256/mod.rs +++ b/hashes/src/sha256/mod.rs @@ -22,37 +22,6 @@ crate::internal_macros::general_hash_type! { "Output of the SHA256 hash function." } -#[cfg(not(hashes_fuzz))] -fn from_engine(mut e: HashEngine) -> Hash { - // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining - let n_bytes_hashed = e.bytes_hashed; - - let zeroes = [0; BLOCK_SIZE - 8]; - e.input(&[0x80]); - if incomplete_block_len(&e) > zeroes.len() { - e.input(&zeroes); - } - let pad_length = zeroes.len() - incomplete_block_len(&e); - e.input(&zeroes[..pad_length]); - debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); - - e.input(&(8 * n_bytes_hashed).to_be_bytes()); - debug_assert_eq!(incomplete_block_len(&e), 0); - - Hash(e.midstate_unchecked().bytes) -} - -#[cfg(hashes_fuzz)] -fn from_engine(e: HashEngine) -> Hash { - let mut hash = e.midstate_unchecked().bytes; - if hash == [0; 32] { - // Assume sha256 is secure and never generate 0-hashes (which represent invalid - // secp256k1 secret keys, causing downstream application breakage). - hash[0] = 1; - } - Hash(hash) -} - const BLOCK_SIZE: usize = 64; /// Engine to compute SHA256 hash function. @@ -141,6 +110,39 @@ impl crate::HashEngine for HashEngine { } impl Hash { +/// Finalize a hash engine to obtain a hash. +#[cfg(not(hashes_fuzz))] +pub fn from_engine(mut e: HashEngine) -> Self { + // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining + let n_bytes_hashed = e.bytes_hashed; + + let zeroes = [0; BLOCK_SIZE - 8]; + e.input(&[0x80]); + if incomplete_block_len(&e) > zeroes.len() { + e.input(&zeroes); + } + let pad_length = zeroes.len() - incomplete_block_len(&e); + e.input(&zeroes[..pad_length]); + debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); + + e.input(&(8 * n_bytes_hashed).to_be_bytes()); + debug_assert_eq!(incomplete_block_len(&e), 0); + + Hash(e.midstate_unchecked().bytes) +} + +/// Finalize a hash engine to obtain a hash. +#[cfg(hashes_fuzz)] +pub fn from_engine(e: HashEngine) -> Self { + let mut hash = e.midstate_unchecked().bytes; + if hash == [0; 32] { + // Assume sha256 is secure and never generate 0-hashes (which represent invalid + // secp256k1 secret keys, causing downstream application breakage). + hash[0] = 1; + } + Hash(hash) +} + /// Iterate the sha256 algorithm to turn a sha256 hash into a sha256d hash #[must_use] pub fn hash_again(&self) -> sha256d::Hash { sha256d::Hash::from_byte_array(hash(&self.0).0) } diff --git a/hashes/src/sha256d/mod.rs b/hashes/src/sha256d/mod.rs index 69bb8a72a..8ec236e2c 100644 --- a/hashes/src/sha256d/mod.rs +++ b/hashes/src/sha256d/mod.rs @@ -10,7 +10,9 @@ crate::internal_macros::general_hash_type! { "Output of the SHA256d hash function." } -fn from_engine(e: HashEngine) -> Hash { +impl Hash { +/// Finalize a hash engine to produce a hash. +pub fn from_engine(e: HashEngine) -> Self { let sha2 = sha256::Hash::from_engine(e.0); let sha2d = sha256::Hash::hash(sha2.as_byte_array()); @@ -18,6 +20,7 @@ fn from_engine(e: HashEngine) -> Hash { ret.copy_from_slice(sha2d.as_byte_array()); Hash(ret) } +} /// Engine to compute SHA256d hash function. #[derive(Debug, Clone)] diff --git a/hashes/src/sha256t/mod.rs b/hashes/src/sha256t/mod.rs index d8f0a1ca0..6b38aaee4 100644 --- a/hashes/src/sha256t/mod.rs +++ b/hashes/src/sha256t/mod.rs @@ -83,7 +83,7 @@ where } /// Produces a hash from the current state of a given engine. - pub fn from_engine(e: HashEngine) -> Hash { + pub fn from_engine(e: HashEngine) -> Self { Hash::from_byte_array(sha256::Hash::from_engine(e.0).to_byte_array()) } diff --git a/hashes/src/sha384/mod.rs b/hashes/src/sha384/mod.rs index 3d6538af0..e127e1b93 100644 --- a/hashes/src/sha384/mod.rs +++ b/hashes/src/sha384/mod.rs @@ -10,11 +10,14 @@ crate::internal_macros::general_hash_type! { "Output of the SHA384 hash function." } -fn from_engine(e: HashEngine) -> Hash { +impl Hash { +/// Finalize a hash engine to produce a hash. +pub fn from_engine(e: HashEngine) -> Self { let mut ret = [0; 48]; - ret.copy_from_slice(&sha512::from_engine(e.0).as_byte_array()[..48]); + ret.copy_from_slice(&sha512::Hash::from_engine(e.0).as_byte_array()[..48]); Hash(ret) } +} /// Engine to compute SHA384 hash function. #[derive(Debug, Clone)] diff --git a/hashes/src/sha512/mod.rs b/hashes/src/sha512/mod.rs index 31203aba4..91814488e 100644 --- a/hashes/src/sha512/mod.rs +++ b/hashes/src/sha512/mod.rs @@ -20,8 +20,10 @@ crate::internal_macros::general_hash_type! { "Output of the SHA512 hash function." } +impl Hash { +/// Finalize a hash engine to produce a hash. #[cfg(not(hashes_fuzz))] -pub(crate) fn from_engine(mut e: HashEngine) -> Hash { +pub fn from_engine(mut e: HashEngine) -> Self { // pad buffer with a single 1-bit then all 0s, until there are exactly 16 bytes remaining let n_bytes_hashed = e.bytes_hashed; @@ -41,12 +43,14 @@ pub(crate) fn from_engine(mut e: HashEngine) -> Hash { Hash(e.midstate()) } +/// Finalize a hash engine to produce a hash. #[cfg(hashes_fuzz)] -pub(crate) fn from_engine(e: HashEngine) -> Hash { +pub fn from_engine(e: HashEngine) -> Self { let mut hash = e.midstate(); hash[0] ^= 0xff; // Make this distinct from SHA-256 Hash(hash) } +} pub(crate) const BLOCK_SIZE: usize = 128; diff --git a/hashes/src/sha512_256/mod.rs b/hashes/src/sha512_256/mod.rs index cf12d4604..7cd817820 100644 --- a/hashes/src/sha512_256/mod.rs +++ b/hashes/src/sha512_256/mod.rs @@ -15,11 +15,14 @@ crate::internal_macros::general_hash_type! { "Output of the SHA512/256 hash function.\n\nSHA512/256 is a hash function that uses the sha512 algorithm but it truncates the output to 256 bits. It has different initial constants than sha512 so it produces an entirely different hash compared to sha512. More information at ." } -fn from_engine(e: HashEngine) -> Hash { +impl Hash { +/// Finalize a hash engine to produce a hash. +pub fn from_engine(e: HashEngine) -> Self { let mut ret = [0; 32]; - ret.copy_from_slice(&sha512::from_engine(e.0).as_byte_array()[..32]); + ret.copy_from_slice(&sha512::Hash::from_engine(e.0).as_byte_array()[..32]); Hash(ret) } +} /// Engine to compute SHA512/256 hash function. /// diff --git a/hashes/src/siphash24/mod.rs b/hashes/src/siphash24/mod.rs index 50abe8575..61f3dcf05 100644 --- a/hashes/src/siphash24/mod.rs +++ b/hashes/src/siphash24/mod.rs @@ -12,15 +12,6 @@ crate::internal_macros::hash_type_no_default! { "Output of the SipHash24 hash function." } -#[cfg(not(hashes_fuzz))] -fn from_engine(e: HashEngine) -> Hash { Hash::from_u64(Hash::from_engine_to_u64(e)) } - -#[cfg(hashes_fuzz)] -fn from_engine(e: HashEngine) -> Hash { - let state = e.state.clone(); - Hash::from_u64(state.v0 ^ state.v1 ^ state.v2 ^ state.v3) -} - macro_rules! compress { ($state:expr) => {{ compress!($state.v0, $state.v1, $state.v2, $state.v3) @@ -176,7 +167,14 @@ impl Hash { pub fn engine(k0: u64, k1: u64) -> HashEngine { HashEngine::with_keys(k0, k1) } /// Produces a hash from the current state of a given engine. - pub fn from_engine(e: HashEngine) -> Hash { from_engine(e) } + #[cfg(not(hashes_fuzz))] + pub fn from_engine(e: HashEngine) -> Self { Hash::from_u64(Hash::from_engine_to_u64(e)) } + + #[cfg(hashes_fuzz)] + pub fn from_engine(e: HashEngine) -> Self { + let state = e.state.clone(); + Hash::from_u64(state.v0 ^ state.v1 ^ state.v2 ^ state.v3) + } /// Hashes the given data with an engine with the provided keys. pub fn hash_with_keys(k0: u64, k1: u64, data: &[u8]) -> Hash { From 0aeff359f515651eafcad3122057bc75303b47c2 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 5 May 2025 16:55:15 +0000 Subject: [PATCH 2/3] hashes: reformat Essentially this just adds indentation after the previous commit. --- hashes/src/hash160/mod.rs | 17 ++++++----- hashes/src/ripemd160/mod.rs | 51 ++++++++++++++++---------------- hashes/src/sha1/mod.rs | 34 +++++++++++----------- hashes/src/sha256/mod.rs | 56 ++++++++++++++++++------------------ hashes/src/sha256d/mod.rs | 16 +++++------ hashes/src/sha384/mod.rs | 12 ++++---- hashes/src/sha512/mod.rs | 52 ++++++++++++++++----------------- hashes/src/sha512_256/mod.rs | 12 ++++---- 8 files changed, 124 insertions(+), 126 deletions(-) diff --git a/hashes/src/hash160/mod.rs b/hashes/src/hash160/mod.rs index 406bdc5ab..fbb11e1d0 100644 --- a/hashes/src/hash160/mod.rs +++ b/hashes/src/hash160/mod.rs @@ -15,17 +15,16 @@ crate::internal_macros::general_hash_type! { "Output of the Bitcoin HASH160 hash function. (RIPEMD160(SHA256))" } - impl Hash { -/// Finalize a hash engine to produce a hash. -pub fn from_engine(e: HashEngine) -> Self { - let sha2 = sha256::Hash::from_engine(e.0); - let rmd = ripemd160::Hash::hash(sha2.as_byte_array()); + /// Finalize a hash engine to produce a hash. + pub fn from_engine(e: HashEngine) -> Self { + let sha2 = sha256::Hash::from_engine(e.0); + let rmd = ripemd160::Hash::hash(sha2.as_byte_array()); - let mut ret = [0; 20]; - ret.copy_from_slice(rmd.as_byte_array()); - Hash(ret) -} + let mut ret = [0; 20]; + ret.copy_from_slice(rmd.as_byte_array()); + Hash(ret) + } } /// Engine to compute HASH160 hash function. diff --git a/hashes/src/ripemd160/mod.rs b/hashes/src/ripemd160/mod.rs index 7b0e5377f..36f2b2416 100644 --- a/hashes/src/ripemd160/mod.rs +++ b/hashes/src/ripemd160/mod.rs @@ -20,36 +20,35 @@ crate::internal_macros::general_hash_type! { "Output of the RIPEMD160 hash function." } - impl Hash { -/// Finalize a hash engine to produce a hash. -#[cfg(not(hashes_fuzz))] -pub fn from_engine(mut e: HashEngine) -> Self { - // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining - let n_bytes_hashed = e.bytes_hashed; + /// Finalize a hash engine to produce a hash. + #[cfg(not(hashes_fuzz))] + pub fn from_engine(mut e: HashEngine) -> Self { + // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining + let n_bytes_hashed = e.bytes_hashed; - let zeroes = [0; BLOCK_SIZE - 8]; - e.input(&[0x80]); - if crate::incomplete_block_len(&e) > zeroes.len() { - e.input(&zeroes); + let zeroes = [0; BLOCK_SIZE - 8]; + e.input(&[0x80]); + if crate::incomplete_block_len(&e) > zeroes.len() { + e.input(&zeroes); + } + let pad_length = zeroes.len() - incomplete_block_len(&e); + e.input(&zeroes[..pad_length]); + debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); + + e.input(&(8 * n_bytes_hashed).to_le_bytes()); + debug_assert_eq!(incomplete_block_len(&e), 0); + + Hash(e.midstate()) } - let pad_length = zeroes.len() - incomplete_block_len(&e); - e.input(&zeroes[..pad_length]); - debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); - e.input(&(8 * n_bytes_hashed).to_le_bytes()); - debug_assert_eq!(incomplete_block_len(&e), 0); - - Hash(e.midstate()) -} - -/// Finalize a hash engine to produce a hash. -#[cfg(hashes_fuzz)] -pub fn from_engine(e: HashEngine) -> Self { - let mut res = e.midstate(); - res[0] ^= (e.bytes_hashed & 0xff) as u8; - Hash(res) -} + /// Finalize a hash engine to produce a hash. + #[cfg(hashes_fuzz)] + pub fn from_engine(e: HashEngine) -> Self { + let mut res = e.midstate(); + res[0] ^= (e.bytes_hashed & 0xff) as u8; + Hash(res) + } } const BLOCK_SIZE: usize = 64; diff --git a/hashes/src/sha1/mod.rs b/hashes/src/sha1/mod.rs index ae49c364f..ca5e92acb 100644 --- a/hashes/src/sha1/mod.rs +++ b/hashes/src/sha1/mod.rs @@ -21,25 +21,25 @@ crate::internal_macros::general_hash_type! { } impl Hash { -/// Finalize a hash engine to produce a hash. -pub fn from_engine(mut e: HashEngine) -> Self { - // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining - let n_bytes_hashed = e.bytes_hashed; + /// Finalize a hash engine to produce a hash. + pub fn from_engine(mut e: HashEngine) -> Self { + // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining + let n_bytes_hashed = e.bytes_hashed; - let zeroes = [0; BLOCK_SIZE - 8]; - e.input(&[0x80]); - if incomplete_block_len(&e) > zeroes.len() { - e.input(&zeroes); + let zeroes = [0; BLOCK_SIZE - 8]; + e.input(&[0x80]); + if incomplete_block_len(&e) > zeroes.len() { + e.input(&zeroes); + } + let pad_length = zeroes.len() - incomplete_block_len(&e); + e.input(&zeroes[..pad_length]); + debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); + + e.input(&(8 * n_bytes_hashed).to_be_bytes()); + debug_assert_eq!(incomplete_block_len(&e), 0); + + Hash(e.midstate()) } - let pad_length = zeroes.len() - incomplete_block_len(&e); - e.input(&zeroes[..pad_length]); - debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); - - e.input(&(8 * n_bytes_hashed).to_be_bytes()); - debug_assert_eq!(incomplete_block_len(&e), 0); - - Hash(e.midstate()) -} } const BLOCK_SIZE: usize = 64; diff --git a/hashes/src/sha256/mod.rs b/hashes/src/sha256/mod.rs index 1def47bf5..39c3257be 100644 --- a/hashes/src/sha256/mod.rs +++ b/hashes/src/sha256/mod.rs @@ -110,38 +110,38 @@ impl crate::HashEngine for HashEngine { } impl Hash { -/// Finalize a hash engine to obtain a hash. -#[cfg(not(hashes_fuzz))] -pub fn from_engine(mut e: HashEngine) -> Self { - // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining - let n_bytes_hashed = e.bytes_hashed; + /// Finalize a hash engine to obtain a hash. + #[cfg(not(hashes_fuzz))] + pub fn from_engine(mut e: HashEngine) -> Self { + // pad buffer with a single 1-bit then all 0s, until there are exactly 8 bytes remaining + let n_bytes_hashed = e.bytes_hashed; - let zeroes = [0; BLOCK_SIZE - 8]; - e.input(&[0x80]); - if incomplete_block_len(&e) > zeroes.len() { - e.input(&zeroes); + let zeroes = [0; BLOCK_SIZE - 8]; + e.input(&[0x80]); + if incomplete_block_len(&e) > zeroes.len() { + e.input(&zeroes); + } + let pad_length = zeroes.len() - incomplete_block_len(&e); + e.input(&zeroes[..pad_length]); + debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); + + e.input(&(8 * n_bytes_hashed).to_be_bytes()); + debug_assert_eq!(incomplete_block_len(&e), 0); + + Hash(e.midstate_unchecked().bytes) } - let pad_length = zeroes.len() - incomplete_block_len(&e); - e.input(&zeroes[..pad_length]); - debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); - e.input(&(8 * n_bytes_hashed).to_be_bytes()); - debug_assert_eq!(incomplete_block_len(&e), 0); - - Hash(e.midstate_unchecked().bytes) -} - -/// Finalize a hash engine to obtain a hash. -#[cfg(hashes_fuzz)] -pub fn from_engine(e: HashEngine) -> Self { - let mut hash = e.midstate_unchecked().bytes; - if hash == [0; 32] { - // Assume sha256 is secure and never generate 0-hashes (which represent invalid - // secp256k1 secret keys, causing downstream application breakage). - hash[0] = 1; + /// Finalize a hash engine to obtain a hash. + #[cfg(hashes_fuzz)] + pub fn from_engine(e: HashEngine) -> Self { + let mut hash = e.midstate_unchecked().bytes; + if hash == [0; 32] { + // Assume sha256 is secure and never generate 0-hashes (which represent invalid + // secp256k1 secret keys, causing downstream application breakage). + hash[0] = 1; + } + Hash(hash) } - Hash(hash) -} /// Iterate the sha256 algorithm to turn a sha256 hash into a sha256d hash #[must_use] diff --git a/hashes/src/sha256d/mod.rs b/hashes/src/sha256d/mod.rs index 8ec236e2c..add1935e1 100644 --- a/hashes/src/sha256d/mod.rs +++ b/hashes/src/sha256d/mod.rs @@ -11,15 +11,15 @@ crate::internal_macros::general_hash_type! { } impl Hash { -/// Finalize a hash engine to produce a hash. -pub fn from_engine(e: HashEngine) -> Self { - let sha2 = sha256::Hash::from_engine(e.0); - let sha2d = sha256::Hash::hash(sha2.as_byte_array()); + /// Finalize a hash engine to produce a hash. + pub fn from_engine(e: HashEngine) -> Self { + let sha2 = sha256::Hash::from_engine(e.0); + let sha2d = sha256::Hash::hash(sha2.as_byte_array()); - let mut ret = [0; 32]; - ret.copy_from_slice(sha2d.as_byte_array()); - Hash(ret) -} + let mut ret = [0; 32]; + ret.copy_from_slice(sha2d.as_byte_array()); + Hash(ret) + } } /// Engine to compute SHA256d hash function. diff --git a/hashes/src/sha384/mod.rs b/hashes/src/sha384/mod.rs index e127e1b93..06d55e62e 100644 --- a/hashes/src/sha384/mod.rs +++ b/hashes/src/sha384/mod.rs @@ -11,12 +11,12 @@ crate::internal_macros::general_hash_type! { } impl Hash { -/// Finalize a hash engine to produce a hash. -pub fn from_engine(e: HashEngine) -> Self { - let mut ret = [0; 48]; - ret.copy_from_slice(&sha512::Hash::from_engine(e.0).as_byte_array()[..48]); - Hash(ret) -} + /// Finalize a hash engine to produce a hash. + pub fn from_engine(e: HashEngine) -> Self { + let mut ret = [0; 48]; + ret.copy_from_slice(&sha512::Hash::from_engine(e.0).as_byte_array()[..48]); + Hash(ret) + } } /// Engine to compute SHA384 hash function. diff --git a/hashes/src/sha512/mod.rs b/hashes/src/sha512/mod.rs index 91814488e..0f73f328e 100644 --- a/hashes/src/sha512/mod.rs +++ b/hashes/src/sha512/mod.rs @@ -21,35 +21,35 @@ crate::internal_macros::general_hash_type! { } impl Hash { -/// Finalize a hash engine to produce a hash. -#[cfg(not(hashes_fuzz))] -pub fn from_engine(mut e: HashEngine) -> Self { - // pad buffer with a single 1-bit then all 0s, until there are exactly 16 bytes remaining - let n_bytes_hashed = e.bytes_hashed; + /// Finalize a hash engine to produce a hash. + #[cfg(not(hashes_fuzz))] + pub fn from_engine(mut e: HashEngine) -> Self { + // pad buffer with a single 1-bit then all 0s, until there are exactly 16 bytes remaining + let n_bytes_hashed = e.bytes_hashed; - let zeroes = [0; BLOCK_SIZE - 16]; - e.input(&[0x80]); - if incomplete_block_len(&e) > zeroes.len() { - e.input(&zeroes); + let zeroes = [0; BLOCK_SIZE - 16]; + e.input(&[0x80]); + if incomplete_block_len(&e) > zeroes.len() { + e.input(&zeroes); + } + let pad_length = zeroes.len() - incomplete_block_len(&e); + e.input(&zeroes[..pad_length]); + debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); + + e.input(&[0; 8]); + e.input(&(8 * n_bytes_hashed).to_be_bytes()); + debug_assert_eq!(incomplete_block_len(&e), 0); + + Hash(e.midstate()) } - let pad_length = zeroes.len() - incomplete_block_len(&e); - e.input(&zeroes[..pad_length]); - debug_assert_eq!(incomplete_block_len(&e), zeroes.len()); - e.input(&[0; 8]); - e.input(&(8 * n_bytes_hashed).to_be_bytes()); - debug_assert_eq!(incomplete_block_len(&e), 0); - - Hash(e.midstate()) -} - -/// Finalize a hash engine to produce a hash. -#[cfg(hashes_fuzz)] -pub fn from_engine(e: HashEngine) -> Self { - let mut hash = e.midstate(); - hash[0] ^= 0xff; // Make this distinct from SHA-256 - Hash(hash) -} + /// Finalize a hash engine to produce a hash. + #[cfg(hashes_fuzz)] + pub fn from_engine(e: HashEngine) -> Self { + let mut hash = e.midstate(); + hash[0] ^= 0xff; // Make this distinct from SHA-256 + Hash(hash) + } } pub(crate) const BLOCK_SIZE: usize = 128; diff --git a/hashes/src/sha512_256/mod.rs b/hashes/src/sha512_256/mod.rs index 7cd817820..291eac9f7 100644 --- a/hashes/src/sha512_256/mod.rs +++ b/hashes/src/sha512_256/mod.rs @@ -16,12 +16,12 @@ crate::internal_macros::general_hash_type! { } impl Hash { -/// Finalize a hash engine to produce a hash. -pub fn from_engine(e: HashEngine) -> Self { - let mut ret = [0; 32]; - ret.copy_from_slice(&sha512::Hash::from_engine(e.0).as_byte_array()[..32]); - Hash(ret) -} + /// Finalize a hash engine to produce a hash. + pub fn from_engine(e: HashEngine) -> Self { + let mut ret = [0; 32]; + ret.copy_from_slice(&sha512::Hash::from_engine(e.0).as_byte_array()[..32]); + Hash(ret) + } } /// Engine to compute SHA512/256 hash function. From a5d96ceb3965f684d7a58fcb8b25ca926b3b555b Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 5 May 2025 16:22:58 +0000 Subject: [PATCH 3/3] hashes: remove private `internal_new` method This method is used to implement `from_byte_array`. But there is no need for the method to exist. We can just inline it in `from_byte_array` and reduce the amount of indirection in our macros. Also make the same change in sha256t. --- hashes/src/internal_macros.rs | 8 ++------ hashes/src/sha256t/mod.rs | 4 +--- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index f0f410167..f3d2cecfb 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -130,12 +130,8 @@ macro_rules! hash_type_no_default { } impl Hash { - const fn internal_new(arr: [u8; $bits / 8]) -> Self { Hash(arr) } - /// Constructs a new hash from the underlying byte array. - pub const fn from_byte_array(bytes: [u8; $bits / 8]) -> Self { - Self::internal_new(bytes) - } + pub const fn from_byte_array(bytes: [u8; $bits / 8]) -> Self { Hash(bytes) } /// Copies a byte slice into a hash object. #[deprecated(since = "0.15.0", note = "use `from_byte_array` instead")] @@ -151,7 +147,7 @@ macro_rules! hash_type_no_default { } else { let mut ret = [0; $bits / 8]; ret.copy_from_slice(sl); - Ok(Self::internal_new(ret)) + Ok(Self::from_byte_array(ret)) } } diff --git a/hashes/src/sha256t/mod.rs b/hashes/src/sha256t/mod.rs index 6b38aaee4..5b6d91db6 100644 --- a/hashes/src/sha256t/mod.rs +++ b/hashes/src/sha256t/mod.rs @@ -62,10 +62,8 @@ impl Hash where T: Tag, { - const fn internal_new(arr: [u8; 32]) -> Self { Hash(PhantomData, arr) } - /// Constructs a new hash from the underlying byte array. - pub const fn from_byte_array(bytes: [u8; 32]) -> Self { Self::internal_new(bytes) } + pub const fn from_byte_array(bytes: [u8; 32]) -> Self { Self(PhantomData, bytes) } /// Copies a byte slice into a hash object. #[deprecated(since = "0.15.0", note = "use `from_byte_array` instead")]