Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions crates/cli/src/commands/manage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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?;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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?);
}
Expand Down
24 changes: 14 additions & 10 deletions crates/cli/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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!({
Expand Down
24 changes: 22 additions & 2 deletions crates/config/src/sections/passwords.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ fn default_schemes() -> Vec<HashingScheme> {
cost: None,
secret: None,
secret_file: None,
unicode_normalization: false,
}]
}

Expand Down Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -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")]
Expand Down
1 change: 1 addition & 0 deletions crates/handlers/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 3 additions & 3 deletions crates/handlers/src/admin/v1/users/set_password.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions crates/handlers/src/compat/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion crates/handlers/src/graphql/mutations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions crates/handlers/src/graphql/mutations/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion crates/handlers/src/oauth2/introspection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Loading
Loading