From c352d376ed4de2958c55dae960c6efd40b263c6d Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 30 Jan 2025 10:47:18 +1100 Subject: [PATCH 1/3] Do not implement Default for HmacEngine The `HmacEngine` should be created using a key. Currently we are providing a `Default` impl that uses `&[]` as the key. This is, I believe, a hangover from when we had a `Default` trait bound somewhere else. It is incorrect and an API footgun - remove it. --- hashes/src/hmac.rs | 13 +++---------- hashes/tests/regression.rs | 16 ---------------- 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/hashes/src/hmac.rs b/hashes/src/hmac.rs index 283bdc7d0..c3dca2d85 100644 --- a/hashes/src/hmac.rs +++ b/hashes/src/hmac.rs @@ -31,13 +31,6 @@ pub struct HmacEngine { oengine: T::Engine, } -impl Default for HmacEngine -where - ::Engine: Default, -{ - fn default() -> Self { HmacEngine::new(&[]) } -} - impl HmacEngine { /// Constructs a new keyed HMAC from `key`. /// @@ -328,7 +321,7 @@ mod benches { #[bench] pub fn hmac_sha256_10(bh: &mut Bencher) { - let mut engine = Hmac::::engine(); + let mut engine = HmacEngine::::new(&[]); let bytes = [1u8; 10]; bh.iter(|| { engine.input(&bytes); @@ -338,7 +331,7 @@ mod benches { #[bench] pub fn hmac_sha256_1k(bh: &mut Bencher) { - let mut engine = Hmac::::engine(); + let mut engine = HmacEngine::::new(&[]); let bytes = [1u8; 1024]; bh.iter(|| { engine.input(&bytes); @@ -348,7 +341,7 @@ mod benches { #[bench] pub fn hmac_sha256_64k(bh: &mut Bencher) { - let mut engine = Hmac::::engine(); + let mut engine = HmacEngine::::new(&[]); let bytes = [1u8; 65536]; bh.iter(|| { engine.input(&bytes); diff --git a/hashes/tests/regression.rs b/hashes/tests/regression.rs index d40998e36..0b6856794 100644 --- a/hashes/tests/regression.rs +++ b/hashes/tests/regression.rs @@ -56,22 +56,6 @@ fn regression_sha256t() { assert_eq!(got, want); } -#[test] -fn regression_hmac_sha256_with_default_key() { - let hash = Hmac::::hash(DATA.as_bytes()); - let got = format!("{}", hash); - let want = "58cc7ed8567bd86eba61f7ed2d5a4edab1774dc10488e57de2eb007a2d9ae82d"; - assert_eq!(got, want); -} - -#[test] -fn regression_hmac_sha512_with_default_key() { - let hash = Hmac::::hash(DATA.as_bytes()); - let got = format!("{}", hash); - let want = "5f5db2f3e1178bf19af5db38a0ed04dc5bc52d641648542886eea9b6bbec0db658ed7a5799ca18f5bc1949f39d24151a32990ee85974e40bb8a35e2288f494ce"; - assert_eq!(got, want); -} - #[test] fn regression_hmac_sha256_with_key() { let mut engine = HmacEngine::::new(HMAC_KEY); From 1eb8f1f9e044b4bb613f49d3e1b0073c8d11bb8e Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 31 Jan 2025 11:15:42 +1100 Subject: [PATCH 2/3] Add a Hmac::engine function The `HmacEngine` cannot be constructed by way of the `GeneralHash` trait method because it requires a key. However we can still add an inherent function to the type to construct an engine. Add the engine constructor and use it in bench code. --- hashes/src/hmac.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/hashes/src/hmac.rs b/hashes/src/hmac.rs index c3dca2d85..a661f64e3 100644 --- a/hashes/src/hmac.rs +++ b/hashes/src/hmac.rs @@ -19,6 +19,16 @@ use crate::{FromSliceError, GeneralHash, Hash, HashEngine}; #[repr(transparent)] pub struct Hmac(T); +impl Hmac { + /// Constructs a new keyed HMAC engine from `key`. + pub fn engine(key: &[u8]) -> HmacEngine + where + ::Engine: Default, + { + HmacEngine::new(key) + } +} + impl str::FromStr for Hmac { type Err = ::Err; fn from_str(s: &str) -> Result { Ok(Hmac(str::FromStr::from_str(s)?)) } @@ -32,7 +42,7 @@ pub struct HmacEngine { } impl HmacEngine { - /// Constructs a new keyed HMAC from `key`. + /// Constructs a new keyed HMAC engine from `key`. /// /// We only support underlying hashes whose block sizes are ≤ 128 bytes. /// @@ -321,7 +331,7 @@ mod benches { #[bench] pub fn hmac_sha256_10(bh: &mut Bencher) { - let mut engine = HmacEngine::::new(&[]); + let mut engine = Hmac::::engine(&[]); let bytes = [1u8; 10]; bh.iter(|| { engine.input(&bytes); @@ -331,7 +341,7 @@ mod benches { #[bench] pub fn hmac_sha256_1k(bh: &mut Bencher) { - let mut engine = HmacEngine::::new(&[]); + let mut engine = Hmac::::engine(&[]); let bytes = [1u8; 1024]; bh.iter(|| { engine.input(&bytes); @@ -341,7 +351,7 @@ mod benches { #[bench] pub fn hmac_sha256_64k(bh: &mut Bencher) { - let mut engine = HmacEngine::::new(&[]); + let mut engine = Hmac::::engine(&[]); let bytes = [1u8; 65536]; bh.iter(|| { engine.input(&bytes); From 18619a6d0b0bca7b7e3603e260b254b4aae6cebf Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 31 Jan 2025 11:17:01 +1100 Subject: [PATCH 3/3] api: Run just check-api --- api/hashes/all-features.txt | 4 ++-- api/hashes/alloc-only.txt | 4 ++-- api/hashes/no-features.txt | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/hashes/all-features.txt b/api/hashes/all-features.txt index 6a776f228..b860a654d 100644 --- a/api/hashes/all-features.txt +++ b/api/hashes/all-features.txt @@ -458,10 +458,10 @@ impl bitcoin_hashes::GeneralHash for bitcoin_has impl bitcoin_hashes::Hash for bitcoin_hashes::hmac::Hmac impl bitcoin_hashes::HashEngine for bitcoin_hashes::hmac::HmacEngine impl bitcoin_hashes::hkdf::Hkdf where ::Engine: core::default::Default +impl bitcoin_hashes::hmac::Hmac impl bitcoin_hashes::hmac::HmacEngine impl bitcoin_io::Write for bitcoin_hashes::hmac::HmacEngine impl core::convert::AsRef<[u8]> for bitcoin_hashes::hmac::Hmac -impl core::default::Default for bitcoin_hashes::hmac::HmacEngine where ::Engine: core::default::Default impl core::marker::StructuralPartialEq for bitcoin_hashes::hmac::Hmac impl std::io::Write for bitcoin_hashes::hmac::HmacEngine impl bitcoin_hashes::GeneralHash for bitcoin_hashes::sha256t::Hash @@ -669,6 +669,7 @@ pub fn bitcoin_hashes::hmac::Hmac::as_ref(&self) -> &[u8] pub fn bitcoin_hashes::hmac::Hmac::clone(&self) -> bitcoin_hashes::hmac::Hmac pub fn bitcoin_hashes::hmac::Hmac::cmp(&self, other: &bitcoin_hashes::hmac::Hmac) -> core::cmp::Ordering pub fn bitcoin_hashes::hmac::Hmac::deserialize>(d: D) -> core::result::Result, ::Error> +pub fn bitcoin_hashes::hmac::Hmac::engine(key: &[u8]) -> bitcoin_hashes::hmac::HmacEngine where ::Engine: core::default::Default pub fn bitcoin_hashes::hmac::Hmac::eq(&self, other: &bitcoin_hashes::hmac::Hmac) -> bool pub fn bitcoin_hashes::hmac::Hmac::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result pub fn bitcoin_hashes::hmac::Hmac::from_byte_array(bytes: ::Bytes) -> Self @@ -680,7 +681,6 @@ pub fn bitcoin_hashes::hmac::Hmac::partial_cmp(&self, other: &bitcoin_hashes: pub fn bitcoin_hashes::hmac::Hmac::serialize(&self, s: S) -> core::result::Result<::Ok, ::Error> pub fn bitcoin_hashes::hmac::Hmac::to_byte_array(self) -> Self::Bytes pub fn bitcoin_hashes::hmac::HmacEngine::clone(&self) -> bitcoin_hashes::hmac::HmacEngine -pub fn bitcoin_hashes::hmac::HmacEngine::default() -> Self pub fn bitcoin_hashes::hmac::HmacEngine::flush(&mut self) -> bitcoin_io::Result<()> pub fn bitcoin_hashes::hmac::HmacEngine::flush(&mut self) -> std::io::error::Result<()> pub fn bitcoin_hashes::hmac::HmacEngine::from_inner_engines(iengine: ::Engine, oengine: ::Engine) -> bitcoin_hashes::hmac::HmacEngine diff --git a/api/hashes/alloc-only.txt b/api/hashes/alloc-only.txt index 35bf80f4d..2000ca4f6 100644 --- a/api/hashes/alloc-only.txt +++ b/api/hashes/alloc-only.txt @@ -416,9 +416,9 @@ impl bitcoin_hashes::GeneralHash for bitcoin_has impl bitcoin_hashes::Hash for bitcoin_hashes::hmac::Hmac impl bitcoin_hashes::HashEngine for bitcoin_hashes::hmac::HmacEngine impl bitcoin_hashes::hkdf::Hkdf where ::Engine: core::default::Default +impl bitcoin_hashes::hmac::Hmac impl bitcoin_hashes::hmac::HmacEngine impl core::convert::AsRef<[u8]> for bitcoin_hashes::hmac::Hmac -impl core::default::Default for bitcoin_hashes::hmac::HmacEngine where ::Engine: core::default::Default impl core::marker::StructuralPartialEq for bitcoin_hashes::hmac::Hmac impl bitcoin_hashes::GeneralHash for bitcoin_hashes::sha256t::Hash impl bitcoin_hashes::Hash for bitcoin_hashes::sha256t::Hash @@ -598,6 +598,7 @@ pub fn bitcoin_hashes::hmac::Hmac::as_byte_array(&self) -> &Self::Bytes pub fn bitcoin_hashes::hmac::Hmac::as_ref(&self) -> &[u8] pub fn bitcoin_hashes::hmac::Hmac::clone(&self) -> bitcoin_hashes::hmac::Hmac pub fn bitcoin_hashes::hmac::Hmac::cmp(&self, other: &bitcoin_hashes::hmac::Hmac) -> core::cmp::Ordering +pub fn bitcoin_hashes::hmac::Hmac::engine(key: &[u8]) -> bitcoin_hashes::hmac::HmacEngine where ::Engine: core::default::Default pub fn bitcoin_hashes::hmac::Hmac::eq(&self, other: &bitcoin_hashes::hmac::Hmac) -> bool pub fn bitcoin_hashes::hmac::Hmac::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result pub fn bitcoin_hashes::hmac::Hmac::from_byte_array(bytes: ::Bytes) -> Self @@ -608,7 +609,6 @@ pub fn bitcoin_hashes::hmac::Hmac::hash<__H: core::hash::Hasher>(&self, state pub fn bitcoin_hashes::hmac::Hmac::partial_cmp(&self, other: &bitcoin_hashes::hmac::Hmac) -> core::option::Option pub fn bitcoin_hashes::hmac::Hmac::to_byte_array(self) -> Self::Bytes pub fn bitcoin_hashes::hmac::HmacEngine::clone(&self) -> bitcoin_hashes::hmac::HmacEngine -pub fn bitcoin_hashes::hmac::HmacEngine::default() -> Self pub fn bitcoin_hashes::hmac::HmacEngine::from_inner_engines(iengine: ::Engine, oengine: ::Engine) -> bitcoin_hashes::hmac::HmacEngine pub fn bitcoin_hashes::hmac::HmacEngine::input(&mut self, buf: &[u8]) pub fn bitcoin_hashes::hmac::HmacEngine::n_bytes_hashed(&self) -> u64 diff --git a/api/hashes/no-features.txt b/api/hashes/no-features.txt index cd19b4663..8ac99fc5d 100644 --- a/api/hashes/no-features.txt +++ b/api/hashes/no-features.txt @@ -380,9 +380,9 @@ impl bitcoin_hashes::GeneralHash for bitcoin_has impl bitcoin_hashes::Hash for bitcoin_hashes::hmac::Hmac impl bitcoin_hashes::HashEngine for bitcoin_hashes::hmac::HmacEngine impl bitcoin_hashes::hkdf::Hkdf where ::Engine: core::default::Default +impl bitcoin_hashes::hmac::Hmac impl bitcoin_hashes::hmac::HmacEngine impl core::convert::AsRef<[u8]> for bitcoin_hashes::hmac::Hmac -impl core::default::Default for bitcoin_hashes::hmac::HmacEngine where ::Engine: core::default::Default impl core::marker::StructuralPartialEq for bitcoin_hashes::hmac::Hmac impl bitcoin_hashes::GeneralHash for bitcoin_hashes::sha256t::Hash impl bitcoin_hashes::Hash for bitcoin_hashes::sha256t::Hash @@ -555,6 +555,7 @@ pub fn bitcoin_hashes::hmac::Hmac::as_byte_array(&self) -> &Self::Bytes pub fn bitcoin_hashes::hmac::Hmac::as_ref(&self) -> &[u8] pub fn bitcoin_hashes::hmac::Hmac::clone(&self) -> bitcoin_hashes::hmac::Hmac pub fn bitcoin_hashes::hmac::Hmac::cmp(&self, other: &bitcoin_hashes::hmac::Hmac) -> core::cmp::Ordering +pub fn bitcoin_hashes::hmac::Hmac::engine(key: &[u8]) -> bitcoin_hashes::hmac::HmacEngine where ::Engine: core::default::Default pub fn bitcoin_hashes::hmac::Hmac::eq(&self, other: &bitcoin_hashes::hmac::Hmac) -> bool pub fn bitcoin_hashes::hmac::Hmac::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result pub fn bitcoin_hashes::hmac::Hmac::from_byte_array(bytes: ::Bytes) -> Self @@ -565,7 +566,6 @@ pub fn bitcoin_hashes::hmac::Hmac::hash<__H: core::hash::Hasher>(&self, state pub fn bitcoin_hashes::hmac::Hmac::partial_cmp(&self, other: &bitcoin_hashes::hmac::Hmac) -> core::option::Option pub fn bitcoin_hashes::hmac::Hmac::to_byte_array(self) -> Self::Bytes pub fn bitcoin_hashes::hmac::HmacEngine::clone(&self) -> bitcoin_hashes::hmac::HmacEngine -pub fn bitcoin_hashes::hmac::HmacEngine::default() -> Self pub fn bitcoin_hashes::hmac::HmacEngine::from_inner_engines(iengine: ::Engine, oengine: ::Engine) -> bitcoin_hashes::hmac::HmacEngine pub fn bitcoin_hashes::hmac::HmacEngine::input(&mut self, buf: &[u8]) pub fn bitcoin_hashes::hmac::HmacEngine::n_bytes_hashed(&self) -> u64