Repository: RustCrypto/password-hashes (language: rust)

Issue:
balloon-hash: provide `hash_password_into` functionality
I'm in the process of migrating from `argon2id` to balloon hashing, but I'm unable to do so in a relatively-safe manner.

I relied on `argon2`'s `hash_password_into` in order to keep track of where the key is, and prevent potential cloning and such. `balloon-hash` doesn't seem to provide this functionality, and instead returns a GenericArray - which complicates this task further.

What should've been a few lines now requires lots of manual zeroizing, and I just don't think it's a great alternative.

Would it be possible to provide `hash_password_into` functionality? I had a quick peek into the `argon2` crate and it doesn't seem out of scope, but I'm not familiar with the RustCrypto traits, so I don't think it's best for me to attempt.

Thank you.


Discussion / hints:

Fix patch:
diff --git a/balloon-hash/src/balloon.rs b/balloon-hash/src/balloon.rs
index e86bc570..7c59935f 100644
--- a/balloon-hash/src/balloon.rs
+++ b/balloon-hash/src/balloon.rs
@@ -28,12 +28,13 @@ pub fn balloon_m<D: Digest + FixedOutputReset>(
     secret: Option<&[u8]>,
     params: Params,
     memory_blocks: &mut [GenericArray<u8, D::OutputSize>],
-) -> Result<GenericArray<u8, D::OutputSize>>
+    output: &mut GenericArray<u8, D::OutputSize>,
+) -> Result<()>
 where
     GenericArray<u8, D::OutputSize>: ArrayDecoding,
 {
     #[cfg(not(feature = "parallel"))]
-    let output = {
+    let output_xor = {
         let mut output = GenericArray::<_, D::OutputSize>::default();
 
         for thread in 1..=u64::from(params.p_cost.get()) {
@@ -45,7 +46,7 @@ where
     };
 
     #[cfg(feature = "parallel")]
-    let output = {
+    let output_xor = {
         use rayon::iter::{ParallelBridge, ParallelIterator};
 
         if memory_blocks.len() < (params.s_cost.get() * params.p_cost.get()) as usize {
@@ -76,8 +77,10 @@ where
         Digest::update(&mut digest, secret);
     }
 
-    Digest::update(&mut digest, output);
-    Ok(digest.finalize_reset())
+    Digest::update(&mut digest, output_xor);
+    Digest::finalize_into(digest, output);
+
+    Ok(())
 }
 
 fn hash_internal<D: Digest + FixedOutputReset>(
diff --git a/balloon-hash/src/error.rs b/balloon-hash/src/error.rs
index 771996e6..7c9d5675 100644
--- a/balloon-hash/src/error.rs
+++ b/balloon-hash/src/error.rs
@@ -3,13 +3,14 @@
 use core::fmt;
 
 #[cfg(feature = "password-hash")]
-use password_hash::errors::InvalidValue;
+use {core::cmp::Ordering, password_hash::errors::InvalidValue};
 
 /// Result with balloon's [`Error`] type.
 pub type Result<T> = core::result::Result<T, Error>;
 
 /// Error type.
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
+#[non_exhaustive]
 pub enum Error {
     /// Algorithm identifier invalid.
     AlgorithmInvalid,
@@ -21,17 +22,27 @@ pub enum Error {
     ThreadsTooMany,
     /// Time cost is too small.
     TimeTooSmall,
+    /// Output size not correct.
+    OutputSize {
+        /// Output size provided.
+        actual: usize,
+        /// Output size expected.
+        expected: usize,
+    },
 }
 
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str(match self {
-            Error::AlgorithmInvalid => "algorithm identifier invalid",
-            Error::MemoryTooLittle => "memory cost is too small",
-            Error::ThreadsTooFew => "not enough threads",
-            Error::ThreadsTooMany => "too many threads",
-            Error::TimeTooSmall => "time cost is too small",
-        })
+        match self {
+            Error::AlgorithmInvalid => f.write_str("algorithm identifier invalid"),
+            Error::MemoryTooLittle => f.write_str("memory cost is too small"),
+            Error::ThreadsTooFew => f.write_str("not enough threads"),
+            Error::ThreadsTooMany => f.write_str("too many threads"),
+            Error::TimeTooSmall => f.write_str("time cost is too small"),
+            Error::OutputSize { expected, .. } => {
+                write!(f, "unexpected output size, expected {} bytes", expected)
+            }
+        }
     }
 }
 
@@ -45,6 +56,12 @@ impl From<Error> for password_hash::Error {
             Error::ThreadsTooFew => InvalidValue::TooShort.param_error(),
             Error::ThreadsTooMany => InvalidValue::TooLong.param_error(),
             Error::TimeTooSmall => InvalidValue::TooShort.param_error(),
+            // TODO: Update after RustCrypto/traits#1026.
+            Error::OutputSize { actual, expected } => match actual.cmp(&expected) {
+                Ordering::Less => password_hash::Error::OutputTooShort,
+                Ordering::Greater => password_hash::Error::OutputTooLong,
+                Ordering::Equal => unreachable!("unexpected correct output size"),
+            },
         }
     }
 }
diff --git a/balloon-hash/src/lib.rs b/balloon-hash/src/lib.rs
index f63e746b..d607c0b8 100644
--- a/balloon-hash/src/lib.rs
+++ b/balloon-hash/src/lib.rs
@@ -84,6 +84,7 @@ pub use password_hash::{self, PasswordHash, PasswordHasher, PasswordVerifier};
 use core::marker::PhantomData;
 use crypto_bigint::ArrayDecoding;
 use digest::generic_array::GenericArray;
+use digest::typenum::Unsigned;
 use digest::{Digest, FixedOutputReset};
 
 #[cfg(all(feature = "alloc", feature = "password-hash"))]
@@ -131,16 +132,27 @@ where
     #[cfg(feature = "alloc")]
     #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
     pub fn hash(&self, pwd: &[u8], salt: &[u8]) -> Result<GenericArray<u8, D::OutputSize>> {
+        let mut output = GenericArray::default();
+        self.hash_into(pwd, salt, &mut output)?;
+
+        Ok(output)
+    }
+
+    /// Hash a password and associated parameters.
+    ///
+    /// The `output` has to have the same size as the hash output size: `D::OutputSize`.
+    #[cfg(feature = "alloc")]
+    #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
+    pub fn hash_into(&self, pwd: &[u8], salt: &[u8], output: &mut [u8]) -> Result<()> {
         #[cfg(not(feature = "parallel"))]
         let mut memory = alloc::vec![GenericArray::default(); self.params.s_cost.get() as usize];
         #[cfg(feature = "parallel")]
         let mut memory = alloc::vec![GenericArray::default(); (self.params.s_cost.get() * self.params.p_cost.get()) as usize];
 
-        #[cfg_attr(not(feature = "zeroize"), allow(clippy::let_and_return))]
-        let output = self.hash_with_memory(pwd, salt, &mut memory);
+        self.hash_into_with_memory(pwd, salt, &mut memory, output)?;
         #[cfg(feature = "zeroize")]
         memory.iter_mut().for_each(|block| block.zeroize());
-        output
+        Ok(())
     }
 
     /// Hash a password and associated parameters.
@@ -152,19 +164,50 @@ where
     ///   to have it allocated for them.
     /// - `no_std` users on "heapless" targets can use an array of the [`GenericArray`] type
     ///   to stack allocate this buffer. It needs a minimum size of `s_cost` or `s_cost * p_cost`
-    ///   with the `parallel` feature enabled.
+    ///   with the `parallel` crate feature enabled.
     pub fn hash_with_memory(
         &self,
         pwd: &[u8],
         salt: &[u8],
         memory_blocks: &mut [GenericArray<u8, D::OutputSize>],
     ) -> Result<GenericArray<u8, D::OutputSize>> {
+        let mut output = GenericArray::default();
+        self.hash_into_with_memory(pwd, salt, memory_blocks, &mut output)?;
+
+        Ok(output)
+    }
+
+    /// Hash a password and associated parameters into the provided `output` buffer.
+    ///
+    /// The `output` has to have the same size as the hash output size: `D::OutputSize`.
+    ///
+    /// See [`Balloon::hash_with_memory`] for more details.
+    pub fn hash_into_with_memory(
+        &self,
+        pwd: &[u8],
+        salt: &[u8],
+        memory_blocks: &mut [GenericArray<u8, D::OutputSize>],
+        output: &mut [u8],
+    ) -> Result<()> {
+        let output = if output.len() == D::OutputSize::USIZE {
+            GenericArray::from_mut_slice(output)
+        } else {
+            return Err(Error::OutputSize {
+                actual: output.len(),
+                expected: D::OutputSize::USIZE,
+            });
+        };
+
         match self.algorithm {
             Algorithm::Balloon => {
-                balloon::balloon::<D>(pwd, salt, self.secret, self.params, memory_blocks)
+                balloon::balloon::<D>(pwd, salt, self.secret, self.params, memory_blocks).map(
+                    |hash| {
+                        output.copy_from_slice(&hash);
+                    },
+                )
             }
             Algorithm::BalloonM => {
-                balloon::balloon_m::<D>(pwd, salt, self.secret, self.params, memory_blocks)
+                balloon::balloon_m::<D>(pwd, salt, self.secret, self.params, memory_blocks, output)
             }
         }
     }


Test patch: