[WIP] Use the phc crate

Companion PR to RustCrypto/traits#2116

The `phc` crate was recently extracted from the `password-hash` crate,
implementing the Password Hashing Competition (PHC) string format for
storing password hashes.

This also factored apart the error types so there are separate ones for
`phc::Error` and `password_hash::Error`, which is the primary source of
changes in this PR.

Author: tarcieri

Cargo.lock

@@ -15,3 +15,7 @@ exclude = ["benches", "fuzz"]
 
 [profile.dev]
 opt-level = 2
+
+[patch.crates-io]
+password-hash = { git = "https://github.com/RustCrypto/traits", branch = "password-hash/use-phc-crate" }
+phc = { git = "https://github.com/RustCrypto/formats" }

Cargo.toml

@@ -25,6 +25,7 @@ The following code example shows how to verify a password when stored using one
 of many possible password hashing algorithms implemented in this repository.
 
 ```rust
+# fn main() -> Result<(), Box<dyn core::error::Error>> {
 use password_hash::{phc, PasswordVerifier};
 
 use argon2::Argon2;
@@ -35,12 +36,20 @@ use scrypt::Scrypt;
 let hash_string = "$argon2i$v=19$m=65536,t=1,p=1$c29tZXNhbHQAAAAAAAAAAA$+r0d29hqEB0yasKr55ZgICsQGSkl0v0kgwhd+U3wyRo";
 let input_password = "password";
 
-let password_hash = phc::PasswordHash::new(&hash_string).expect("invalid password hash");
+let password_hash = phc::PasswordHash::new(&hash_string)?;
 
 // Trait objects for algorithms to support
 let algs: &[&dyn PasswordVerifier<phc::PasswordHash>] = &[&Argon2::default(), &Pbkdf2, &Scrypt];
 
-password_hash.verify_password(algs, input_password).expect("invalid password");
+for alg in algs {
+    if alg.verify_password(input_password.as_ref(), &password_hash).is_ok() {
+        return Ok(());
+    }
+}
+
+// If we get here, the password is invalid
+# Err(Box::new(password_hash::Error::PasswordInvalid))
+# }
 ```
 
 ## Minimum Supported Rust Version (MSRV) Policy

README.md

@@ -22,25 +22,24 @@ blake2 = { version = "0.11.0-rc.3", default-features = false }
 
 # optional dependencies
 rayon = { version = "1.7", optional = true }
-password-hash = { version = "0.6.0-rc.3", optional = true }
+password-hash = { version = "0.6.0-rc.3", optional = true, features = ["phc"] }
+phc = { version = "0.3.0-pre", optional = true, features = ["rand_core"] }
 zeroize = { version = "1", default-features = false, optional = true }
 
 [target.'cfg(any(target_arch = "x86", target_arch = "x86_64"))'.dependencies]
 cpufeatures = "0.2.17"
 
 [dev-dependencies]
 hex-literal = "1"
-password-hash = { version = "0.6.0-rc.3", features = ["rand_core"] }
 
 [features]
-default = ["alloc", "password-hash", "rand"]
+default = ["alloc", "getrandom", "simple"]
 alloc = ["password-hash?/alloc"]
 std = ["alloc", "base64ct/std"]
 
-getrandom = ["simple", "password-hash/getrandom"]
+getrandom = ["simple", "phc/getrandom"]
 parallel = ["dep:rayon"]
-rand = ["password-hash?/rand_core"]
-simple = ["password-hash"]
+simple = ["password-hash", "phc"]
 zeroize = ["dep:zeroize"]
 
 [lints.rust.unexpected_cfgs]

argon2/Cargo.toml

@@ -2,9 +2,6 @@
 
 use core::fmt;
 
-#[cfg(feature = "password-hash")]
-use {crate::Params, core::cmp::Ordering, password_hash::errors::InvalidValue};
-
 /// Result with argon2's [`Error`] type.
 pub type Result<T> = core::result::Result<T, Error>;
 
@@ -97,29 +94,20 @@ impl From<base64ct::Error> for Error {
 impl From<Error> for password_hash::Error {
     fn from(err: Error) -> password_hash::Error {
         match err {
-            Error::AdTooLong => InvalidValue::TooLong.param_error(),
+            Error::AdTooLong
+            | Error::KeyIdTooLong
+            | Error::MemoryTooLittle
+            | Error::SecretTooLong
+            | Error::ThreadsTooFew
+            | Error::ThreadsTooMany
+            | Error::TimeTooSmall => password_hash::Error::ParamsInvalid,
             Error::AlgorithmInvalid => password_hash::Error::Algorithm,
-            Error::B64Encoding(inner) => password_hash::Error::B64Encoding(inner),
-            Error::KeyIdTooLong => InvalidValue::TooLong.param_error(),
-            Error::MemoryTooLittle => InvalidValue::TooShort.param_error(),
-            Error::MemoryTooMuch => InvalidValue::TooLong.param_error(),
-            Error::PwdTooLong => password_hash::Error::Password,
-            Error::OutputTooShort => password_hash::Error::OutputSize {
-                provided: Ordering::Less,
-                expected: Params::MIN_OUTPUT_LEN,
-            },
-            Error::OutputTooLong => password_hash::Error::OutputSize {
-                provided: Ordering::Greater,
-                expected: Params::MAX_OUTPUT_LEN,
-            },
-            Error::SaltTooShort => InvalidValue::TooShort.salt_error(),
-            Error::SaltTooLong => InvalidValue::TooLong.salt_error(),
-            Error::SecretTooLong => InvalidValue::TooLong.param_error(),
-            Error::ThreadsTooFew => InvalidValue::TooShort.param_error(),
-            Error::ThreadsTooMany => InvalidValue::TooLong.param_error(),
-            Error::TimeTooSmall => InvalidValue::TooShort.param_error(),
+            Error::B64Encoding(_) => password_hash::Error::EncodingInvalid,
+            Error::MemoryTooMuch | Error::OutOfMemory => password_hash::Error::OutOfMemory,
+            Error::PwdTooLong => password_hash::Error::PasswordInvalid,
+            Error::OutputTooShort | Error::OutputTooLong => password_hash::Error::OutputSize,
+            Error::SaltTooShort | Error::SaltTooLong => password_hash::Error::SaltInvalid,
             Error::VersionInvalid => password_hash::Error::Version,
-            Error::OutOfMemory => password_hash::Error::OutOfMemory,
         }
     }
 }

argon2/src/error.rs

@@ -659,16 +659,20 @@ impl CustomizedPasswordHasher<PasswordHash> for Argon2<'_> {
 #[cfg(all(feature = "alloc", feature = "password-hash"))]
 impl PasswordHasher<PasswordHash> for Argon2<'_> {
     fn hash_password(&self, password: &[u8], salt: &[u8]) -> password_hash::Result<PasswordHash> {
-        let salt = Salt::new(salt)?;
+        let salt = Salt::new(salt).map_err(|_| password_hash::Error::SaltInvalid)?;
 
         let output_len = self
             .params
             .output_len()
             .unwrap_or(Params::DEFAULT_OUTPUT_LEN);
 
-        let output = Output::init_with(output_len, |out| {
-            Ok(self.hash_password_into(password, &salt, out)?)
-        })?;
+        let mut buffer = [0u8; Output::MAX_LENGTH];
+        let out = buffer
+            .get_mut(..output_len)
+            .ok_or(password_hash::Error::OutputSize)?;
+
+        self.hash_password_into(password, &salt, out)?;
+        let output = Output::new(out).map_err(|_| password_hash::Error::OutputSize)?;
 
         Ok(PasswordHash {
             algorithm: self.algorithm.ident(),
@@ -713,12 +717,7 @@ mod tests {
         let res =
             argon2.hash_password_customized(EXAMPLE_PASSWORD, salt, None, None, Params::default());
 
-        assert_eq!(
-            res,
-            Err(password_hash::Error::SaltInvalid(
-                password_hash::errors::InvalidValue::TooShort
-            ))
-        );
+        assert_eq!(res, Err(password_hash::Error::SaltInvalid));
     }
 
     #[test]

argon2/src/lib.rs

@@ -360,7 +360,9 @@ impl FromStr for Params {
     type Err = password_hash::Error;
 
     fn from_str(s: &str) -> password_hash::Result<Self> {
-        Self::try_from(&ParamsString::from_str(s)?)
+        let params_string =
+            ParamsString::from_str(s).map_err(|_| password_hash::Error::ParamsInvalid)?;
+        Self::try_from(&params_string)
     }
 }
 
@@ -374,21 +376,43 @@ impl TryFrom<&ParamsString> for Params {
         for (ident, value) in params.iter() {
             match ident.as_str() {
                 "m" => {
-                    builder.m_cost(value.decimal()?);
+                    builder.m_cost(
+                        value
+                            .decimal()
+                            .map_err(|_| password_hash::Error::ParamInvalid { name: "m" })?,
+                    );
                 }
                 "t" => {
-                    builder.t_cost(value.decimal()?);
+                    builder.t_cost(
+                        value
+                            .decimal()
+                            .map_err(|_| password_hash::Error::ParamInvalid { name: "t" })?,
+                    );
                 }
                 "p" => {
-                    builder.p_cost(value.decimal()?);
+                    builder.p_cost(
+                        value
+                            .decimal()
+                            .map_err(|_| password_hash::Error::ParamInvalid { name: "p" })?,
+                    );
                 }
                 "keyid" => {
-                    builder.keyid(value.as_str().parse()?);
+                    builder.keyid(
+                        value
+                            .as_str()
+                            .parse()
+                            .map_err(|_| password_hash::Error::ParamsInvalid)?,
+                    );
                 }
                 "data" => {
-                    builder.data(value.as_str().parse()?);
+                    builder.data(
+                        value
+                            .as_str()
+                            .parse()
+                            .map_err(|_| password_hash::Error::ParamsInvalid)?,
+                    );
                 }
-                _ => return Err(password_hash::Error::ParamNameInvalid),
+                _ => return Err(password_hash::Error::ParamsInvalid),
             }
         }
 
@@ -426,16 +450,27 @@ impl TryFrom<&Params> for ParamsString {
 
     fn try_from(params: &Params) -> password_hash::Result<ParamsString> {
         let mut output = ParamsString::new();
-        output.add_decimal("m", params.m_cost)?;
-        output.add_decimal("t", params.t_cost)?;
-        output.add_decimal("p", params.p_cost)?;
+
+        for (name, value) in [
+            ("m", params.m_cost),
+            ("t", params.t_cost),
+            ("p", params.p_cost),
+        ] {
+            output
+                .add_decimal(name, value)
+                .map_err(|_| password_hash::Error::ParamInvalid { name })?;
+        }
 
         if !params.keyid.is_empty() {
-            output.add_b64_bytes("keyid", params.keyid.as_bytes())?;
+            output
+                .add_b64_bytes("keyid", params.keyid.as_bytes())
+                .map_err(|_| password_hash::Error::ParamInvalid { name: "keyid" })?;
         }
 
         if !params.data.is_empty() {
-            output.add_b64_bytes("data", params.data.as_bytes())?;
+            output
+                .add_b64_bytes("data", params.data.as_bytes())
+                .map_err(|_| password_hash::Error::ParamInvalid { name: "keyid" })?;
         }
 
         Ok(output)

argon2/src/params.rs

@@ -583,7 +583,7 @@ fn reference_argon2i_v0x10_mismatching_hash() {
     .unwrap();
     assert_eq!(
         Argon2::default().verify_password(b"password", &hash),
-        Err(password_hash::errors::Error::Password)
+        Err(password_hash::Error::PasswordInvalid)
     );
 }
 
@@ -716,7 +716,7 @@ fn reference_argon2i_v0x13_mismatching_hash() {
     .unwrap();
     assert_eq!(
         Argon2::default().verify_password(b"password", &hash),
-        Err(password_hash::errors::Error::Password)
+        Err(password_hash::Error::PasswordInvalid)
     );
 }