mirror of
https://github.com/element-hq/matrix-authentication-service.git
synced 2026-03-31 10:35:48 +00:00
Allow applying unicode normalisation to passwords before hashing
This commit is contained in:
1
Cargo.lock
generated
1
Cargo.lock
generated
@@ -3312,6 +3312,7 @@ dependencies = [
|
||||
"headers",
|
||||
"hex",
|
||||
"hyper",
|
||||
"icu_normalizer",
|
||||
"indexmap 2.9.0",
|
||||
"insta",
|
||||
"lettre",
|
||||
|
||||
@@ -31,6 +31,7 @@ use mas_storage_pg::{DatabaseError, PgRepository};
|
||||
use rand::{RngCore, SeedableRng};
|
||||
use sqlx::{Acquire, types::Uuid};
|
||||
use tracing::{error, info, info_span, warn};
|
||||
use zeroize::Zeroizing;
|
||||
|
||||
use crate::util::{
|
||||
database_connection_from_config, homeserver_connection_from_config,
|
||||
@@ -210,7 +211,7 @@ impl Options {
|
||||
return Ok(ExitCode::from(1));
|
||||
}
|
||||
|
||||
let password = password.into_bytes().into();
|
||||
let password = Zeroizing::new(password);
|
||||
|
||||
let (version, hashed_password) = password_manager.hash(&mut rng, password).await?;
|
||||
|
||||
@@ -566,7 +567,7 @@ impl Options {
|
||||
|
||||
// Hash the password if it's provided
|
||||
let hashed_password = if let Some(password) = password {
|
||||
let password = password.into_bytes().into();
|
||||
let password = Zeroizing::new(password);
|
||||
Some(password_manager.hash(&mut rng, password).await?)
|
||||
} else {
|
||||
None
|
||||
@@ -641,7 +642,7 @@ impl Options {
|
||||
.interact()
|
||||
})
|
||||
.await??;
|
||||
let password = password.into_bytes().into();
|
||||
let password = Zeroizing::new(password);
|
||||
req.hashed_password =
|
||||
Some(password_manager.hash(&mut rng, password).await?);
|
||||
}
|
||||
|
||||
@@ -36,20 +36,24 @@ pub async fn password_manager_from_config(
|
||||
return Ok(PasswordManager::disabled());
|
||||
}
|
||||
|
||||
let schemes = config
|
||||
.load()
|
||||
.await?
|
||||
.into_iter()
|
||||
.map(|(version, algorithm, cost, secret)| {
|
||||
let schemes = config.load().await?.into_iter().map(
|
||||
|(version, algorithm, cost, secret, unicode_normalization)| {
|
||||
use mas_handlers::passwords::Hasher;
|
||||
let hasher = match algorithm {
|
||||
mas_config::PasswordAlgorithm::Pbkdf2 => Hasher::pbkdf2(secret),
|
||||
mas_config::PasswordAlgorithm::Bcrypt => Hasher::bcrypt(cost, secret),
|
||||
mas_config::PasswordAlgorithm::Argon2id => Hasher::argon2id(secret),
|
||||
mas_config::PasswordAlgorithm::Pbkdf2 => {
|
||||
Hasher::pbkdf2(secret, unicode_normalization)
|
||||
}
|
||||
mas_config::PasswordAlgorithm::Bcrypt => {
|
||||
Hasher::bcrypt(cost, secret, unicode_normalization)
|
||||
}
|
||||
mas_config::PasswordAlgorithm::Argon2id => {
|
||||
Hasher::argon2id(secret, unicode_normalization)
|
||||
}
|
||||
};
|
||||
|
||||
(version, hasher)
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
PasswordManager::new(config.minimum_complexity(), schemes)
|
||||
}
|
||||
@@ -492,7 +496,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn test_password_manager_from_config() {
|
||||
let mut rng = rand_chacha::ChaChaRng::seed_from_u64(42);
|
||||
let password = Zeroizing::new(b"hunter2".to_vec());
|
||||
let password = Zeroizing::new("hunter2".to_owned());
|
||||
|
||||
// Test a valid, enabled config
|
||||
let config = serde_json::from_value(serde_json::json!({
|
||||
|
||||
@@ -20,6 +20,7 @@ fn default_schemes() -> Vec<HashingScheme> {
|
||||
cost: None,
|
||||
secret: None,
|
||||
secret_file: None,
|
||||
unicode_normalization: false,
|
||||
}]
|
||||
}
|
||||
|
||||
@@ -124,7 +125,7 @@ impl PasswordsConfig {
|
||||
/// not be read.
|
||||
pub async fn load(
|
||||
&self,
|
||||
) -> Result<Vec<(u16, Algorithm, Option<u32>, Option<Vec<u8>>)>, anyhow::Error> {
|
||||
) -> Result<Vec<(u16, Algorithm, Option<u32>, Option<Vec<u8>>, bool)>, anyhow::Error> {
|
||||
let mut schemes: Vec<&HashingScheme> = self.schemes.iter().collect();
|
||||
schemes.sort_unstable_by_key(|a| Reverse(a.version));
|
||||
schemes.dedup_by_key(|a| a.version);
|
||||
@@ -151,13 +152,24 @@ impl PasswordsConfig {
|
||||
(None, None) => None,
|
||||
};
|
||||
|
||||
mapped_result.push((scheme.version, scheme.algorithm, scheme.cost, secret));
|
||||
mapped_result.push((
|
||||
scheme.version,
|
||||
scheme.algorithm,
|
||||
scheme.cost,
|
||||
secret,
|
||||
scheme.unicode_normalization,
|
||||
));
|
||||
}
|
||||
|
||||
Ok(mapped_result)
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::trivially_copy_pass_by_ref)]
|
||||
const fn is_default_false(value: &bool) -> bool {
|
||||
!*value
|
||||
}
|
||||
|
||||
/// Parameters for a password hashing scheme
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)]
|
||||
pub struct HashingScheme {
|
||||
@@ -168,6 +180,14 @@ pub struct HashingScheme {
|
||||
/// The hashing algorithm to use
|
||||
pub algorithm: Algorithm,
|
||||
|
||||
/// Whether to apply Unicode normalization to the password before hashing
|
||||
///
|
||||
/// Defaults to `false`, and generally recommended to stay false. This is
|
||||
/// although recommended when importing password hashs from Synapse, as it
|
||||
/// applies an NFKC normalization to the password before hashing it.
|
||||
#[serde(default, skip_serializing_if = "is_default_false")]
|
||||
pub unicode_normalization: bool,
|
||||
|
||||
/// Cost for the bcrypt algorithm
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
#[schemars(default = "default_bcrypt_cost")]
|
||||
|
||||
@@ -74,6 +74,7 @@ chrono.workspace = true
|
||||
elliptic-curve.workspace = true
|
||||
hex.workspace = true
|
||||
governor.workspace = true
|
||||
icu_normalizer = "1.5.0"
|
||||
indexmap.workspace = true
|
||||
pkcs8.workspace = true
|
||||
psl = "2.1.111"
|
||||
|
||||
@@ -122,7 +122,7 @@ pub async fn handler(
|
||||
return Err(RouteError::PasswordTooWeak);
|
||||
}
|
||||
|
||||
let password = Zeroizing::new(params.password.into_bytes());
|
||||
let password = Zeroizing::new(params.password);
|
||||
let (version, hashed_password) = password_manager
|
||||
.hash(&mut rng, password)
|
||||
.await
|
||||
@@ -184,7 +184,7 @@ mod tests {
|
||||
// Check that the user now has a password
|
||||
let mut repo = state.repository().await.unwrap();
|
||||
let user_password = repo.user_password().active(&user).await.unwrap().unwrap();
|
||||
let password = Zeroizing::new(b"this is a good enough password".to_vec());
|
||||
let password = Zeroizing::new(String::from("this is a good enough password"));
|
||||
state
|
||||
.password_manager
|
||||
.verify(
|
||||
@@ -243,7 +243,7 @@ mod tests {
|
||||
// Check that the user now has a password
|
||||
let mut repo = state.repository().await.unwrap();
|
||||
let user_password = repo.user_password().active(&user).await.unwrap().unwrap();
|
||||
let password = Zeroizing::new(b"password".to_vec());
|
||||
let password = Zeroizing::new("password".to_owned());
|
||||
state
|
||||
.password_manager
|
||||
.verify(
|
||||
|
||||
@@ -574,7 +574,7 @@ async fn user_password_login(
|
||||
.ok_or(RouteError::NoPassword)?;
|
||||
|
||||
// Verify the password
|
||||
let password = Zeroizing::new(password.into_bytes());
|
||||
let password = Zeroizing::new(password);
|
||||
|
||||
let new_password_hash = password_manager
|
||||
.verify_and_upgrade(
|
||||
@@ -787,7 +787,7 @@ mod tests {
|
||||
.unwrap();
|
||||
let (version, hash) = state
|
||||
.password_manager
|
||||
.hash(&mut rng, Zeroizing::new(password.as_bytes().to_vec()))
|
||||
.hash(&mut rng, Zeroizing::new(password.to_owned()))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
|
||||
@@ -76,7 +76,7 @@ async fn verify_password_if_needed(
|
||||
return Ok(false);
|
||||
};
|
||||
|
||||
let password = Zeroizing::new(password.into_bytes());
|
||||
let password = Zeroizing::new(password);
|
||||
|
||||
let res = password_manager
|
||||
.verify(
|
||||
|
||||
@@ -742,7 +742,7 @@ impl UserMutations {
|
||||
if let Err(_err) = password_manager
|
||||
.verify(
|
||||
active_password.version,
|
||||
Zeroizing::new(current_password_attempt.into_bytes()),
|
||||
Zeroizing::new(current_password_attempt),
|
||||
active_password.hashed_password,
|
||||
)
|
||||
.await
|
||||
@@ -754,7 +754,7 @@ impl UserMutations {
|
||||
}
|
||||
|
||||
let (new_password_version, new_password_hash) = password_manager
|
||||
.hash(state.rng(), Zeroizing::new(input.new_password.into_bytes()))
|
||||
.hash(state.rng(), Zeroizing::new(input.new_password))
|
||||
.await?;
|
||||
|
||||
repo.user_password()
|
||||
@@ -849,7 +849,7 @@ impl UserMutations {
|
||||
}
|
||||
|
||||
let (new_password_version, new_password_hash) = password_manager
|
||||
.hash(state.rng(), Zeroizing::new(input.new_password.into_bytes()))
|
||||
.hash(state.rng(), Zeroizing::new(input.new_password))
|
||||
.await?;
|
||||
|
||||
repo.user_password()
|
||||
|
||||
@@ -844,7 +844,7 @@ mod tests {
|
||||
|
||||
let (version, hashed_password) = state
|
||||
.password_manager
|
||||
.hash(&mut state.rng(), Zeroizing::new(b"password".to_vec()))
|
||||
.hash(&mut state.rng(), Zeroizing::new("password".to_owned()))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
// Copyright 2024 New Vector Ltd.
|
||||
// Copyright 2024, 2025 New Vector Ltd.
|
||||
// Copyright 2022-2024 The Matrix.org Foundation C.I.C.
|
||||
//
|
||||
// SPDX-License-Identifier: AGPL-3.0-only
|
||||
@@ -116,7 +116,7 @@ impl PasswordManager {
|
||||
pub async fn hash<R: CryptoRng + RngCore + Send>(
|
||||
&self,
|
||||
rng: R,
|
||||
password: Zeroizing<Vec<u8>>,
|
||||
password: Zeroizing<String>,
|
||||
) -> Result<(SchemeVersion, String), anyhow::Error> {
|
||||
let inner = self.get_inner()?;
|
||||
|
||||
@@ -130,7 +130,7 @@ impl PasswordManager {
|
||||
let version = inner.current_version;
|
||||
|
||||
let hashed = tokio::task::spawn_blocking(move || {
|
||||
span.in_scope(move || inner.current_hasher.hash_blocking(rng, &password))
|
||||
span.in_scope(move || inner.current_hasher.hash_blocking(rng, password))
|
||||
})
|
||||
.await??;
|
||||
|
||||
@@ -147,7 +147,7 @@ impl PasswordManager {
|
||||
pub async fn verify(
|
||||
&self,
|
||||
scheme: SchemeVersion,
|
||||
password: Zeroizing<Vec<u8>>,
|
||||
password: Zeroizing<String>,
|
||||
hashed_password: String,
|
||||
) -> Result<(), anyhow::Error> {
|
||||
let inner = self.get_inner()?;
|
||||
@@ -164,7 +164,7 @@ impl PasswordManager {
|
||||
.context("Hashing scheme not found")?
|
||||
};
|
||||
|
||||
hasher.verify_blocking(&hashed_password, &password)
|
||||
hasher.verify_blocking(&hashed_password, password)
|
||||
})
|
||||
})
|
||||
.await??;
|
||||
@@ -184,7 +184,7 @@ impl PasswordManager {
|
||||
&self,
|
||||
rng: R,
|
||||
scheme: SchemeVersion,
|
||||
password: Zeroizing<Vec<u8>>,
|
||||
password: Zeroizing<String>,
|
||||
hashed_password: String,
|
||||
) -> Result<Option<(SchemeVersion, String)>, anyhow::Error> {
|
||||
let inner = self.get_inner()?;
|
||||
@@ -209,43 +209,78 @@ impl PasswordManager {
|
||||
/// A hashing scheme, with an optional pepper
|
||||
pub struct Hasher {
|
||||
algorithm: Algorithm,
|
||||
unicode_normalization: bool,
|
||||
pepper: Option<Vec<u8>>,
|
||||
}
|
||||
|
||||
impl Hasher {
|
||||
/// Creates a new hashing scheme based on the bcrypt algorithm
|
||||
#[must_use]
|
||||
pub const fn bcrypt(cost: Option<u32>, pepper: Option<Vec<u8>>) -> Self {
|
||||
pub const fn bcrypt(
|
||||
cost: Option<u32>,
|
||||
pepper: Option<Vec<u8>>,
|
||||
unicode_normalization: bool,
|
||||
) -> Self {
|
||||
let algorithm = Algorithm::Bcrypt { cost };
|
||||
Self { algorithm, pepper }
|
||||
Self {
|
||||
algorithm,
|
||||
unicode_normalization,
|
||||
pepper,
|
||||
}
|
||||
}
|
||||
|
||||
/// Creates a new hashing scheme based on the argon2id algorithm
|
||||
#[must_use]
|
||||
pub const fn argon2id(pepper: Option<Vec<u8>>) -> Self {
|
||||
pub const fn argon2id(pepper: Option<Vec<u8>>, unicode_normalization: bool) -> Self {
|
||||
let algorithm = Algorithm::Argon2id;
|
||||
Self { algorithm, pepper }
|
||||
Self {
|
||||
algorithm,
|
||||
unicode_normalization,
|
||||
pepper,
|
||||
}
|
||||
}
|
||||
|
||||
/// Creates a new hashing scheme based on the pbkdf2 algorithm
|
||||
#[must_use]
|
||||
pub const fn pbkdf2(pepper: Option<Vec<u8>>) -> Self {
|
||||
pub const fn pbkdf2(pepper: Option<Vec<u8>>, unicode_normalization: bool) -> Self {
|
||||
let algorithm = Algorithm::Pbkdf2;
|
||||
Self { algorithm, pepper }
|
||||
Self {
|
||||
algorithm,
|
||||
unicode_normalization,
|
||||
pepper,
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_password(&self, password: Zeroizing<String>) -> Zeroizing<String> {
|
||||
if self.unicode_normalization {
|
||||
// This is the normalization method used by Synapse
|
||||
let normalizer = icu_normalizer::ComposingNormalizer::new_nfkc();
|
||||
Zeroizing::new(normalizer.normalize(&password))
|
||||
} else {
|
||||
password
|
||||
}
|
||||
}
|
||||
|
||||
fn hash_blocking<R: CryptoRng + RngCore>(
|
||||
&self,
|
||||
rng: R,
|
||||
password: &[u8],
|
||||
password: Zeroizing<String>,
|
||||
) -> Result<String, anyhow::Error> {
|
||||
let password = self.normalize_password(password);
|
||||
|
||||
self.algorithm
|
||||
.hash_blocking(rng, password, self.pepper.as_deref())
|
||||
.hash_blocking(rng, password.as_bytes(), self.pepper.as_deref())
|
||||
}
|
||||
|
||||
fn verify_blocking(&self, hashed_password: &str, password: &[u8]) -> Result<(), anyhow::Error> {
|
||||
fn verify_blocking(
|
||||
&self,
|
||||
hashed_password: &str,
|
||||
password: Zeroizing<String>,
|
||||
) -> Result<(), anyhow::Error> {
|
||||
let password = self.normalize_password(password);
|
||||
|
||||
self.algorithm
|
||||
.verify_blocking(hashed_password, password, self.pepper.as_deref())
|
||||
.verify_blocking(hashed_password, password.as_bytes(), self.pepper.as_deref())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -461,8 +496,8 @@ mod tests {
|
||||
// after changing the hashing schemes. The salt generation is done with a seeded
|
||||
// RNG, so that we can do stable snapshots of hashed passwords
|
||||
let mut rng = rand_chacha::ChaChaRng::seed_from_u64(42);
|
||||
let password = Zeroizing::new(b"hunter2".to_vec());
|
||||
let wrong_password = Zeroizing::new(b"wrong-password".to_vec());
|
||||
let password = Zeroizing::new("hunter2".to_owned());
|
||||
let wrong_password = Zeroizing::new("wrong-password".to_owned());
|
||||
|
||||
let manager = PasswordManager::new(
|
||||
0,
|
||||
@@ -470,7 +505,7 @@ mod tests {
|
||||
// Start with one hashing scheme: the one used by synapse, bcrypt + pepper
|
||||
(
|
||||
1,
|
||||
Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec())),
|
||||
Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec()), false),
|
||||
),
|
||||
],
|
||||
)
|
||||
@@ -519,10 +554,10 @@ mod tests {
|
||||
let manager = PasswordManager::new(
|
||||
0,
|
||||
[
|
||||
(2, Hasher::argon2id(None)),
|
||||
(2, Hasher::argon2id(None, false)),
|
||||
(
|
||||
1,
|
||||
Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec())),
|
||||
Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec()), false),
|
||||
),
|
||||
],
|
||||
)
|
||||
@@ -575,11 +610,14 @@ mod tests {
|
||||
let manager = PasswordManager::new(
|
||||
0,
|
||||
[
|
||||
(3, Hasher::argon2id(Some(b"a-secret-pepper".to_vec()))),
|
||||
(2, Hasher::argon2id(None)),
|
||||
(
|
||||
3,
|
||||
Hasher::argon2id(Some(b"a-secret-pepper".to_vec()), false),
|
||||
),
|
||||
(2, Hasher::argon2id(None, false)),
|
||||
(
|
||||
1,
|
||||
Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec())),
|
||||
Hasher::bcrypt(Some(10), Some(b"a-secret-pepper".to_vec()), false),
|
||||
),
|
||||
],
|
||||
)
|
||||
|
||||
@@ -194,7 +194,7 @@ impl TestState {
|
||||
let password_manager = if site_config.password_login_enabled {
|
||||
PasswordManager::new(
|
||||
site_config.minimum_password_complexity,
|
||||
[(1, Hasher::argon2id(None))],
|
||||
[(1, Hasher::argon2id(None, false))],
|
||||
)?
|
||||
} else {
|
||||
PasswordManager::disabled()
|
||||
|
||||
@@ -244,7 +244,7 @@ pub(crate) async fn post(
|
||||
.await;
|
||||
};
|
||||
|
||||
let password = Zeroizing::new(form.password.as_bytes().to_vec());
|
||||
let password = Zeroizing::new(form.password);
|
||||
|
||||
// Verify the password, and upgrade it on-the-fly if needed
|
||||
let user_password = match password_manager
|
||||
@@ -581,7 +581,7 @@ mod test {
|
||||
.unwrap();
|
||||
let (version, hash) = state
|
||||
.password_manager
|
||||
.hash(&mut rng, Zeroizing::new(password.as_bytes().to_vec()))
|
||||
.hash(&mut rng, Zeroizing::new(password.to_owned()))
|
||||
.await
|
||||
.unwrap();
|
||||
repo.user_password()
|
||||
|
||||
@@ -364,7 +364,7 @@ pub(crate) async fn post(
|
||||
.await?;
|
||||
|
||||
// Hash the password
|
||||
let password = Zeroizing::new(form.password.into_bytes());
|
||||
let password = Zeroizing::new(form.password);
|
||||
let (version, hashed_password) = password_manager
|
||||
.hash(&mut rng, password)
|
||||
.await
|
||||
|
||||
@@ -199,9 +199,9 @@ pub async fn synapse_config_check_against_mas_config(
|
||||
// Look for the MAS password hashing scheme that will be used for imported
|
||||
// Synapse passwords, then check the configuration matches so that Synapse
|
||||
// passwords will be compatible with MAS.
|
||||
if let Some((_, algorithm, _, secret)) = mas_password_schemes
|
||||
if let Some((_, algorithm, _, secret, _)) = mas_password_schemes
|
||||
.iter()
|
||||
.find(|(version, _, _, _)| *version == MIGRATED_PASSWORD_VERSION)
|
||||
.find(|(version, _, _, _, _)| *version == MIGRATED_PASSWORD_VERSION)
|
||||
{
|
||||
if algorithm != &PasswordAlgorithm::Bcrypt {
|
||||
errors.push(CheckError::PasswordSchemeNotBcrypt);
|
||||
|
||||
@@ -179,6 +179,7 @@ impl Config {
|
||||
cost: self.bcrypt_rounds,
|
||||
secret: self.password_config.pepper,
|
||||
secret_file: None,
|
||||
unicode_normalization: true,
|
||||
},
|
||||
// Use the default algorithm MAS uses as a second hashing scheme, so that users
|
||||
// will get their password hash upgraded to a more modern algorithm over time
|
||||
@@ -188,6 +189,7 @@ impl Config {
|
||||
cost: None,
|
||||
secret: None,
|
||||
secret_file: None,
|
||||
unicode_normalization: false,
|
||||
},
|
||||
];
|
||||
|
||||
|
||||
@@ -1613,6 +1613,10 @@
|
||||
}
|
||||
]
|
||||
},
|
||||
"unicode_normalization": {
|
||||
"description": "Whether to apply Unicode normalization to the password before hashing\n\nDefaults to `false`, and generally recommended to stay false. This is although recommended when importing password hashs from Synapse, as it applies an NFKC normalization to the password before hashing it.",
|
||||
"type": "boolean"
|
||||
},
|
||||
"cost": {
|
||||
"description": "Cost for the bcrypt algorithm",
|
||||
"default": 12,
|
||||
|
||||
@@ -47,7 +47,7 @@ When using this tool, be careful to examine the log output for any warnings abou
|
||||
#### Local passwords
|
||||
|
||||
Synapse uses bcrypt as its password hashing scheme, while MAS defaults to using the newer argon2id.
|
||||
You will have to configure the version 1 scheme as bcrypt for migrated passwords to work.
|
||||
You will have to configure the version 1 scheme as bcrypt with `unicode_normalization: true` for migrated passwords to work.
|
||||
It is also recommended that you keep argon2id as version 2 so that once users log in, their hashes will be updated to the newer, recommended scheme.
|
||||
|
||||
Example passwords configuration:
|
||||
@@ -57,6 +57,7 @@ passwords:
|
||||
schemes:
|
||||
- version: 1
|
||||
algorithm: bcrypt
|
||||
unicode_normalization: true
|
||||
# Optional, must match the `password_config.pepper` in the Synapse config
|
||||
#secret: secretPepperValue
|
||||
- version: 2
|
||||
|
||||
Reference in New Issue
Block a user