From e049f9ff4fecdb51e0d2c3ee908a93b70e932b31 Mon Sep 17 00:00:00 2001 From: Maurus Decimus <11444311+mdecimus@users.noreply.github.com> Date: Thu, 14 May 2026 16:52:15 +0200 Subject: [PATCH] Accept password hashes with `$` or `{` prefixes as secure secrets --- CHANGELOG.md | 1 + crates/directory/src/core/secret.rs | 342 ++++++++++++++++++ crates/jmap/src/registry/mapping/principal.rs | 64 ++-- 3 files changed, 378 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e40b49f..b5db30f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ If you are upgrading from v0.16.x, replace the binary (or run `docker pull`). If - Dns updater: Log DNS record types and values. ## Changed +- Accept password hashes with `$` or `{` prefixes as secure secrets. ## Fixed - DAV: `acl-principal-prop-set` REPORT enforced the wrong privilege. diff --git a/crates/directory/src/core/secret.rs b/crates/directory/src/core/secret.rs index cd2d3bf7..842b9d70 100644 --- a/crates/directory/src/core/secret.rs +++ b/crates/directory/src/core/secret.rs @@ -289,3 +289,345 @@ pub async fn hash_secret(algorithm: PasswordHashAlgorithm, secret: Vec) -> t .reason(err)), } } + +pub fn is_password_hash(s: &str) -> bool { + if s.starts_with("$argon2") || s.starts_with("$pbkdf2") || s.starts_with("$scrypt") { + return is_complete_phc(s); + } + + if s.starts_with("$2") { + return is_bcrypt_format(s); + } + + if let Some(body) = s.strip_prefix("$1$") { + return is_md5_crypt(body); + } + + if let Some(body) = s.strip_prefix("$5$") { + return is_sha_crypt(body, 43); + } + + if let Some(body) = s.strip_prefix("$6$") { + return is_sha_crypt(body, 86); + } + + if let Some(body) = s.strip_prefix("$sha1$") { + return is_sha1_crypt(body); + } + + if let Some(rest) = s.strip_prefix('{') { + return rest + .split_once('}') + .map(|(scheme, body)| is_ldap_hash(scheme, body)) + .unwrap_or(false); + } + + false +} + +fn is_complete_phc(s: &str) -> bool { + PasswordHash::new(s) + .map(|h| h.hash.is_some() && h.salt.is_some()) + .unwrap_or(false) +} + +fn is_crypt_b64(b: u8) -> bool { + b.is_ascii_alphanumeric() || b == b'.' || b == b'/' +} + +fn all_crypt_b64(s: &str) -> bool { + !s.is_empty() && s.bytes().all(is_crypt_b64) +} + +fn is_bcrypt_format(s: &str) -> bool { + let bytes = s.as_bytes(); + if bytes.len() != 60 { + return false; + } + if bytes[1] != b'2' || !matches!(bytes[2], b'a' | b'b' | b'x' | b'y') { + return false; + } + if bytes[3] != b'$' + || !bytes[4].is_ascii_digit() + || !bytes[5].is_ascii_digit() + || bytes[6] != b'$' + { + return false; + } + bytes[7..].iter().copied().all(is_crypt_b64) +} + +fn is_md5_crypt(body: &str) -> bool { + let Some((salt, hash)) = body.split_once('$') else { + return false; + }; + !salt.is_empty() + && salt.len() <= 8 + && all_crypt_b64(salt) + && hash.len() == 22 + && all_crypt_b64(hash) +} + +fn is_sha_crypt(body: &str, hash_len: usize) -> bool { + let remainder = if let Some(after) = body.strip_prefix("rounds=") { + let Some((rounds, rest)) = after.split_once('$') else { + return false; + }; + if rounds.is_empty() || !rounds.bytes().all(|b| b.is_ascii_digit()) { + return false; + } + rest + } else { + body + }; + let Some((salt, hash)) = remainder.split_once('$') else { + return false; + }; + !salt.is_empty() + && salt.len() <= 16 + && all_crypt_b64(salt) + && hash.len() == hash_len + && all_crypt_b64(hash) +} + +fn is_sha1_crypt(body: &str) -> bool { + let mut parts = body.splitn(3, '$'); + let Some(rounds) = parts.next() else { + return false; + }; + let Some(salt) = parts.next() else { + return false; + }; + let Some(hash) = parts.next() else { + return false; + }; + if rounds.is_empty() || !rounds.bytes().all(|b| b.is_ascii_digit()) { + return false; + } + if salt.is_empty() || salt.len() > 64 || !all_crypt_b64(salt) { + return false; + } + hash.len() == 28 && all_crypt_b64(hash) +} + +fn is_ldap_hash(scheme: &str, body: &str) -> bool { + match scheme { + "SHA" => b64_decoded_len_eq(body, 20), + "SSHA" => b64_decoded_len_ge(body, 21), + "SHA256" => b64_decoded_len_eq(body, 32), + "SSHA256" => b64_decoded_len_ge(body, 33), + "SHA512" => b64_decoded_len_eq(body, 64), + "SSHA512" => b64_decoded_len_ge(body, 65), + "MD5" => b64_decoded_len_eq(body, 16), + "ARGON2" | "ARGON2I" | "ARGON2ID" | "PBKDF2" => is_complete_phc(body), + "CRYPT" | "crypt" => is_password_hash(body) || is_unix_des_crypt(body), + _ => false, + } +} + +fn is_unix_des_crypt(s: &str) -> bool { + let bytes = s.as_bytes(); + (bytes.len() == 13 && bytes.iter().copied().all(is_crypt_b64)) + || (bytes.len() == 20 && bytes[0] == b'_' && bytes[1..].iter().copied().all(is_crypt_b64)) +} + +fn b64_decoded_len_eq(body: &str, len: usize) -> bool { + b64_decode_loose(body) + .map(|d| d.len() == len) + .unwrap_or(false) +} + +fn b64_decoded_len_ge(body: &str, min: usize) -> bool { + b64_decode_loose(body) + .map(|d| d.len() >= min) + .unwrap_or(false) +} + +fn b64_decode_loose(s: &str) -> Option> { + use base64::Engine; + use base64::engine::general_purpose::STANDARD; + use base64::engine::general_purpose::STANDARD_NO_PAD; + STANDARD + .decode(s) + .ok() + .or_else(|| STANDARD_NO_PAD.decode(s).ok()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn b64(bytes: &[u8]) -> String { + String::from_utf8(base64_encode(bytes).unwrap()).unwrap() + } + + #[test] + fn is_password_hash_detects_phc_strings() { + let salt = SaltString::generate(&mut OsRng); + + let argon = Argon2::default() + .hash_password(b"hello", &salt) + .unwrap() + .to_string(); + assert!(is_password_hash(&argon), "argon2 not detected: {argon}"); + + let pbkdf = Pbkdf2.hash_password(b"hello", &salt).unwrap().to_string(); + assert!(is_password_hash(&pbkdf), "pbkdf2 not detected: {pbkdf}"); + + let scr = Scrypt.hash_password(b"hello", &salt).unwrap().to_string(); + assert!(is_password_hash(&scr), "scrypt not detected: {scr}"); + } + + #[test] + fn is_password_hash_detects_crypt_variants() { + let bc = bcrypt::hash("hello").unwrap(); + assert!(is_password_hash(&bc), "bcrypt not detected: {bc}"); + assert!(bcrypt::verify("hello", &bc)); + + let md5 = "$1$5pZSV9va$azfrPr6af3Fc7dLblQXVa0"; + assert!(is_password_hash(md5)); + assert!(md5_crypt::verify("password", md5)); + + let sha256 = "$5$WH1ABM5sKhxbkgCK$sOnTVjQn1Y3EWibd8gWqqJqjH.KaFrxJE5rijqxcPp7"; + assert!(is_password_hash(sha256)); + assert!(sha256_crypt::verify("test", sha256)); + + let sha256_rounds = "$5$rounds=11858$WH1ABM5sKhxbkgCK$aTQsjPkz0rBsH3lQlJxw9HDTDXPKBxC0LlVeV69P.t1"; + assert!(is_password_hash(sha256_rounds)); + assert!(sha256_crypt::verify("test", sha256_rounds)); + + let s512 = sha512_crypt::hash("hello").unwrap(); + assert!(is_password_hash(&s512), "sha512_crypt not detected: {s512}"); + assert!(sha512_crypt::verify("hello", &s512)); + + let s1 = sha1_crypt::hash("hello").unwrap(); + assert!(is_password_hash(&s1), "sha1_crypt not detected: {s1}"); + assert!(sha1_crypt::verify("hello", &s1)); + } + + #[test] + fn is_password_hash_detects_ldap_schemes() { + let mut h = Sha1::new(); + h.update(b"hello"); + let sha = b64(&h.finalize()[..]); + assert!(is_password_hash(&format!("{{SHA}}{sha}"))); + + let mut h = Sha1::new(); + h.update(b"hello"); + h.update(b"saltbytes"); + let mut buf = h.finalize().to_vec(); + buf.extend_from_slice(b"saltbytes"); + let ssha = b64(&buf); + assert!(is_password_hash(&format!("{{SSHA}}{ssha}"))); + + let mut h = Sha256::new(); + h.update(b"hello"); + let sha256 = b64(&h.finalize()[..]); + assert!(is_password_hash(&format!("{{SHA256}}{sha256}"))); + + let mut h = Sha256::new(); + h.update(b"hello"); + h.update(b"saltbytes"); + let mut buf = h.finalize().to_vec(); + buf.extend_from_slice(b"saltbytes"); + let ssha256 = b64(&buf); + assert!(is_password_hash(&format!("{{SSHA256}}{ssha256}"))); + + let mut h = Sha512::new(); + h.update(b"hello"); + let sha512 = b64(&h.finalize()[..]); + assert!(is_password_hash(&format!("{{SHA512}}{sha512}"))); + + let mut h = Sha512::new(); + h.update(b"hello"); + h.update(b"saltbytes"); + let mut buf = h.finalize().to_vec(); + buf.extend_from_slice(b"saltbytes"); + let ssha512 = b64(&buf); + assert!(is_password_hash(&format!("{{SSHA512}}{ssha512}"))); + + let digest = md5::compute(b"hello"); + let md5b = b64(&digest[..]); + assert!(is_password_hash(&format!("{{MD5}}{md5b}"))); + + let inner = sha512_crypt::hash("hello").unwrap(); + assert!(is_password_hash(&format!("{{CRYPT}}{inner}"))); + assert!(is_password_hash(&format!("{{crypt}}{inner}"))); + + assert!(is_password_hash("{CRYPT}$1$5pZSV9va$azfrPr6af3Fc7dLblQXVa0")); + assert!(is_password_hash("{CRYPT}abcdefghij012")); + assert!(is_password_hash("{CRYPT}_J9..K0AyUubDkQmPLeM")); + + let salt = SaltString::generate(&mut OsRng); + let a = Argon2::default() + .hash_password(b"hello", &salt) + .unwrap() + .to_string(); + assert!(is_password_hash(&format!("{{ARGON2ID}}{a}"))); + assert!(is_password_hash(&format!("{{ARGON2}}{a}"))); + assert!(is_password_hash(&format!("{{ARGON2I}}{a}"))); + + let p = Pbkdf2.hash_password(b"hello", &salt).unwrap().to_string(); + assert!(is_password_hash(&format!("{{PBKDF2}}{p}"))); + } + + #[test] + fn is_password_hash_rejects_passwords() { + let not_hashes = [ + "", + "hello", + "p@ssw0rd!", + "password123", + "correct horse battery staple", + "$myPassword", + "$1incomplete", + "$1$", + "$1$short", + "$1$abc$tooshorthash", + "$5$", + "$5$nohashpart$", + "$5$saltonly$alsotooshort", + "$6$", + "$$$", + "$$argon2$", + "$argon2id$broken", + "$argon2id$v=19$bad", + "$2", + "$2y$", + "$2y$10$short", + "$2z$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy", + "$sha1$", + "$sha1$notdigits$salt$hash", + "{", + "{}", + "{}foo", + "{SHA}", + "{SHA}not!valid!base!64", + "{SHA}aGVsbG8=", + "{MD5}", + "{MD5}aGVsbG8=", + "{SHA256}aGVsbG8=", + "{SHA512}aGVsbG8=", + "{SSHA}aGVsbG8=", + "{UNKNOWN}whatever", + "{PLAIN}stillplain", + "{plain}stillplain", + "{CLEAR}stillplain", + "{clear}stillplain", + "{CRYPT}plainpw", + "{CRYPT}", + "{CRYPT}toolongtobeunixcryptbutshortbsdi", + "{ARGON2ID}notaphcstring", + "_short", + "_J9..K0AyUubDkQmPLeM", + "_notvalidbsdi", + "regular_password", + "1234567890123", + "abcdefghij012", + "$5$rounds=$saltvalue$abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJK", + ]; + for p in not_hashes { + assert!(!is_password_hash(p), "false positive: {p:?}"); + } + } +} diff --git a/crates/jmap/src/registry/mapping/principal.rs b/crates/jmap/src/registry/mapping/principal.rs index 5bab6856..d8dc5765 100644 --- a/crates/jmap/src/registry/mapping/principal.rs +++ b/crates/jmap/src/registry/mapping/principal.rs @@ -9,7 +9,7 @@ use common::{ Server, auth::{Permissions, PermissionsGroup, permissions::BuildPermissions}, }; -use directory::core::secret::hash_secret; +use directory::core::secret::{hash_secret, is_password_hash}; use jmap_proto::error::set::SetError; use registry::{schema::structs::TaskStatus, types::datetime::UTCDateTime}; use registry::{ @@ -118,14 +118,6 @@ pub(crate) async fn validate_account( } if credential.secret != old_credential.secret { - if let Err(err) = - set.server.is_secure_password(&credential.secret, &[]) - { - return Ok(Err(SetError::invalid_properties() - .with_property(Property::Secret) - .with_description(err))); - } - if credential.expires_at == old_credential.expires_at && credential .expires_at @@ -142,12 +134,30 @@ pub(crate) async fn validate_account( )); } - credential.secret = hash_secret( - set.server.core.network.security.password_hash_algorithm, - std::mem::take(&mut credential.secret).into_bytes(), - ) - .await - .caused_by(trc::location!())?; + if !(matches!( + credential.secret.as_bytes().first(), + Some(&b'$' | &b'{') + ) && is_password_hash(&credential.secret)) + { + if let Err(err) = + set.server.is_secure_password(&credential.secret, &[]) + { + return Ok(Err(SetError::invalid_properties() + .with_property(Property::Secret) + .with_description(err))); + } + + credential.secret = hash_secret( + set.server + .core + .network + .security + .password_hash_algorithm, + std::mem::take(&mut credential.secret).into_bytes(), + ) + .await + .caused_by(trc::location!())?; + } } } ( @@ -181,7 +191,6 @@ pub(crate) async fn validate_account( credential, is_external_directory, has_password, - false, ) .await? { @@ -218,7 +227,6 @@ pub(crate) async fn validate_account( credential, is_external_directory, index > 0, - recover_account_id.is_some(), ) .await? { @@ -266,7 +274,6 @@ async fn validate_credential_creation( credential: &mut Credential, is_external_directory: bool, has_password: bool, - is_recovery_mode: bool, ) -> trc::Result>> { match credential { Credential::Password(credential) => { @@ -280,23 +287,22 @@ async fn validate_credential_creation( .with_description("Only one password credential is allowed."))); } - if is_recovery_mode && credential.secret.starts_with('$') { - return Ok(Ok(())); + if credential.expires_at.is_none() + && let Some(expires_at) = server.core.network.security.password_default_expiration + { + credential.expires_at = + Some(UTCDateTime::from_timestamp((now() + expires_at) as i64)); } - if let Err(err) = server.is_secure_password(&credential.secret, &[]) { + if matches!(credential.secret.as_bytes().first(), Some(&b'$' | &b'{')) + && is_password_hash(&credential.secret) + { + Ok(Ok(())) + } else if let Err(err) = server.is_secure_password(&credential.secret, &[]) { Ok(Err(SetError::invalid_properties() .with_property(Property::Secret) .with_description(err))) } else { - if credential.expires_at.is_none() - && let Some(expires_at) = - server.core.network.security.password_default_expiration - { - credential.expires_at = - Some(UTCDateTime::from_timestamp((now() + expires_at) as i64)); - } - credential.secret = hash_secret( server.core.network.security.password_hash_algorithm, std::mem::take(&mut credential.secret).into_bytes(),