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

Issue:
password-auth: provide an API to expose whether a password should be re-hashed
Ideally there would be a way to "upgrade" passwords from a deprecated method to a supported one, similar to passlib's [verify_and_update](https://passlib.readthedocs.io/en/stable/lib/passlib.context.html#passlib.context.CryptContext.verify_and_update) method.

This would allow users of this crate verify a password, check if it's using the most up-to-date hashing method, and return the updated hash so it can be stored elsewhere.


Discussion / hints:
I agree that some API to detect password hashes using obsolete algorithms would be good. However, one of the goals of `password-auth` is to provide a simple, unobtrusive API, so unlike passlib I don't want to add a ton of API surface such that it obscures the simple happy path of `generate_hash`/`verify_password`.

I'm also not sold on the `verify_and_update` API (or name). I think we can break password hash updates down to two different approaches:

- *lazy interactive updates*: the `verify_and_update` approach where updating a password hash requires the user's original password, and passwords are replaced as users login. This approach assumes old password hashes are weak enough they deserve updating, but not so weak that they need to be updated immediately and the old hashing algorithms disabled (a sort of uncanny valley in the threat model)
- *eager non-interactive updates*: updates which can be bulk-applied across the entire password database without requiring users to login. There are many approaches to this, but the most common is a "[password onion](https://bristolcrypto.blogspot.com/2015/01/password-hashing-according-to-facebook.html)" approach which looks something like `H2(H1(password, salt), ...)`. Some password hashing schemes also support offline work-factor updates or even more [advanced schemes](https://rwc.iacr.org/2016/Slides/Pass-RWC.pdf) (e.g based on bilinear pairings).

In my experience as a security professional the latter approach is far and away the one preferred by users at scale. The first may feel like good security hygiene but as I noted with the "uncanny valley" remark is hard to fit into a threat model, and has a major disadvantage that hash upgrades don't immediately help all users, only the ones who have recently logged in.

We can avoid painting ourselves into a corner and mandating one of these approaches while also keeping the API surface smaller by introducing a function which checks if the algorithm/params used by a particular password hash is considered suboptimal. Such a function avoids the question of "do I need the user's original password to upgrade the hash?" and allows for either lazy or eager updates to password hashes.

Passlib calls such a function [`needs_update`](https://passlib.readthedocs.io/en/stable/lib/passlib.context.html#passlib.context.CryptContext.needs_update), although I don't think that's a particularly good name and would prefer something a bit less ambiguous as a free function imported to a module dealing with a number of different concerns (e.g. related to user authentication).

How about something like this instead?

```rust
fn is_hash_obsolete(hash: &str) -> bool
```

I think that strikes a nice balance between simplicity and flexibility.
Thanks for the quick and thoughtful response!

I was under the impression that what you described as `lazy interactive updates` are more common, but I'm also not a security expert. I was familiar with it from passlib and the [django.contrib.auth](https://github.com/django/django/blob/main/django/contrib/auth/hashers.py#L67-L78) packages.

If I understand correctly, most of the things mentioned in `eager non-interactive updates` would require additional work in password-auth to support. The password onion and other bulk application approaches are new to me, but they seem very interesting. Maybe that would be worth a separate issue? Though it would take a bit of API design work to get it right and support it in the other hashing-related crates.

I think your proposal of `is_hash_obsolete` strikes a nice balance between convenience and flexibility. It would allow for building a system that does automatic hash upgrades, or requiring people to change their password if the hash was obsolete. The downside to that approach as-described is that it wouldn't support a password onion or other eager updates, though as mentioned above I assume you'd need to build additional APIs to support that anyway.

Fix patch:
diff --git a/password-auth/README.md b/password-auth/README.md
index 27361778..9b544444 100644
--- a/password-auth/README.md
+++ b/password-auth/README.md
@@ -15,8 +15,8 @@ with support for [Argon2], [PBKDF2], and [scrypt] password hashing algorithms.
 ## About
 
 `password-auth` is a high-level password authentication library with a simple
-API which eliminates as much complexity and user choice as possible. It only
-has two functions:
+interface which eliminates as much complexity and user choice as possible.
+The core API consists of two functions:
 
 - [`generate_hash`]: generates a password hash from the provided password. The
 - [`verify_password`]: verifies the provided password against a password hash,
diff --git a/password-auth/src/errors.rs b/password-auth/src/errors.rs
new file mode 100644
index 00000000..ce5b8042
--- /dev/null
+++ b/password-auth/src/errors.rs
@@ -0,0 +1,36 @@
+//! Error types.
+
+use core::fmt;
+
+/// Password hash parse errors.
+#[derive(Clone, Copy, Debug)]
+pub struct ParseError(password_hash::Error);
+
+impl ParseError {
+    /// Create a new parse error.
+    pub(crate) fn new(err: password_hash::Error) -> Self {
+        Self(err)
+    }
+}
+
+impl fmt::Display for ParseError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", &self.0)
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for ParseError {}
+
+/// Password hash verification errors.
+#[derive(Clone, Copy, Debug)]
+pub struct VerifyError;
+
+impl fmt::Display for VerifyError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("password verification error")
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for VerifyError {}
diff --git a/password-auth/src/lib.rs b/password-auth/src/lib.rs
index cafcb8ef..1990da7e 100644
--- a/password-auth/src/lib.rs
+++ b/password-auth/src/lib.rs
@@ -21,9 +21,12 @@ extern crate alloc;
 #[cfg(feature = "std")]
 extern crate std;
 
+mod errors;
+
+pub use crate::errors::{ParseError, VerifyError};
+
 use alloc::string::{String, ToString};
-use core::fmt;
-use password_hash::{PasswordHash, PasswordHasher, PasswordVerifier, SaltString};
+use password_hash::{ParamsString, PasswordHash, PasswordHasher, PasswordVerifier, SaltString};
 use rand_core::OsRng;
 
 #[cfg(not(any(feature = "argon2", feature = "pbkdf2", feature = "scrypt")))]
@@ -38,19 +41,6 @@ use pbkdf2::Pbkdf2;
 #[cfg(feature = "scrypt")]
 use scrypt::Scrypt;
 
-/// Opaque error type.
-#[derive(Clone, Copy, Debug)]
-pub struct VerifyError;
-
-impl fmt::Display for VerifyError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str("password verification error")
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for VerifyError {}
-
 /// Generate a password hash for the given password.
 ///
 /// Uses the best available password hashing algorithm given the enabled
@@ -104,9 +94,46 @@ pub fn verify_password(password: impl AsRef<[u8]>, hash: &str) -> Result<(), Ver
         .map_err(|_| VerifyError)
 }
 
+/// Determine if the given password hash is using the recommended algorithm and
+/// parameters.
+///
+/// This can be used by implementations which wish to lazily update their
+/// password hashes (i.e. by rehashing the password with [`generate_hash`])
+/// to determine if such an update should be applied.
+///
+/// # Returns
+/// - `true` if the hash can't be parsed or isn't using the latest
+///   recommended parameters
+/// - `false` if the hash is using the latest recommended parameters
+#[allow(unreachable_code)]
+pub fn is_hash_obsolete(hash: &str) -> Result<bool, ParseError> {
+    let hash = PasswordHash::new(hash).map_err(ParseError::new)?;
+
+    #[cfg(feature = "argon2")]
+    return Ok(hash.algorithm != argon2::Algorithm::default().ident()
+        || hash.params != default_params_string::<argon2::Params>());
+
+    #[cfg(feature = "scrypt")]
+    return Ok(hash.algorithm != scrypt::ALG_ID
+        || hash.params != default_params_string::<scrypt::Params>());
+
+    #[cfg(feature = "pbkdf2")]
+    return Ok(hash.algorithm != pbkdf2::Algorithm::default().ident()
+        || hash.params != default_params_string::<pbkdf2::Params>());
+
+    Ok(true)
+}
+
+fn default_params_string<T>() -> ParamsString
+where
+    T: Default + TryInto<ParamsString, Error = password_hash::Error>,
+{
+    T::default().try_into().expect("invalid default params")
+}
+
 #[cfg(test)]
 mod tests {
-    use super::{generate_hash, verify_password};
+    use super::{generate_hash, is_hash_obsolete, verify_password};
 
     const EXAMPLE_PASSWORD: &str = "password";
 
@@ -115,6 +142,7 @@ mod tests {
         let hash = generate_hash(EXAMPLE_PASSWORD);
         assert!(verify_password(EXAMPLE_PASSWORD, &hash).is_ok());
         assert!(verify_password("bogus", &hash).is_err());
+        assert!(!is_hash_obsolete(&hash).unwrap());
     }
 
     #[cfg(feature = "argon2")]


Test patch: