diff --git a/playwright/compose/warden/build.sh b/playwright/compose/warden/build.sh index ee8b47fe..0a2d4694 100755 --- a/playwright/compose/warden/build.sh +++ b/playwright/compose/warden/build.sh @@ -24,7 +24,7 @@ if [[ ! -z "$REPO_URL" ]] && [[ ! -z "$COMMIT_HASH" ]] ; then fi # Lower the KDF iterations default for faster tests. -sed -i 's/(6e5,2e6,6e5)/(1e5,2e6,1e5)/' /web-vault/app/main.*.js +sed -i 's/(6e5,2e6,6e5)/(6e3,2e6,6e3)/' /web-vault/app/main.*.js # Generate a self signed cert mkdir -p /data/ssl; cd /data/ssl diff --git a/playwright/docker-compose.yml b/playwright/docker-compose.yml index 5bfc47a5..97f6faf5 100644 --- a/playwright/docker-compose.yml +++ b/playwright/docker-compose.yml @@ -23,12 +23,14 @@ services: env_file: ${DC_ENV_FILE:-.env} environment: - ADMIN_TOKEN + - CLIENT_KDF_ITER - DATABASE_URL - CLIENT_SUPPRESS_ONBOARDING - EMAIL_2FA_AUTO_FALLBACK - I_REALLY_WANT_VOLATILE_STORAGE - LOG_LEVEL - LOGIN_RATELIMIT_MAX_BURST + - PASSWORD_ITERATIONS - SMTP_HOST - SMTP_FROM - SMTP_DEBUG diff --git a/playwright/test.env b/playwright/test.env index 2260f860..c2fa354e 100644 --- a/playwright/test.env +++ b/playwright/test.env @@ -58,6 +58,8 @@ LOG_LEVEL=info,oidcwarden::sso=debug LOGIN_RATELIMIT_MAX_BURST=100 ADMIN_TOKEN=admin CLIENT_SUPPRESS_ONBOARDING=true +PASSWORD_ITERATIONS=6000 +CLIENT_KDF_ITER=6000 SMTP_SECURITY=off SMTP_PORT=${MAILDEV_SMTP_PORT} diff --git a/src/api/core/accounts.rs b/src/api/core/accounts.rs index 8cc5e55b..6f13ad01 100644 --- a/src/api/core/accounts.rs +++ b/src/api/core/accounts.rs @@ -11,9 +11,9 @@ use serde_json::Value; use crate::{ CONFIG, api::{ - AnonymousNotify, ApiResult, EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType, + AnonymousNotify, EmptyResult, JsonResult, Notify, PasswordOrOtpData, UpdateType, core::{accept_org_invite, log_user_event, two_factor::email}, - master_password_policy, register_push_device, unregister_push_device, + kdf_upgrade, master_password_policy, register_push_device, unregister_push_device, }, auth::{ClientHeaders, ClientIp, Headers, decode_delete, decode_invite, decode_verify_email}, crypto, @@ -665,8 +665,8 @@ async fn post_password(data: Json, headers: Headers, conn: DbCon } fn set_kdf_data(user: &mut User, data: &KDFData) -> EmptyResult { - if data.kdf == UserKdfType::Pbkdf2 as i32 && data.kdf_iterations < 100_000 { - err!("PBKDF2 KDF iterations must be at least 100000.") + if data.kdf == UserKdfType::Pbkdf2 as i32 && data.kdf_iterations < CONFIG.client_kdf_iter() { + err!(format!("PBKDF2 KDF iterations must be at least {}.", CONFIG.client_kdf_iter())) } if data.kdf == UserKdfType::Argon2id as i32 { @@ -1049,7 +1049,7 @@ async fn post_sstamp(data: Json, headers: Headers, conn: DbCo let data: PasswordOrOtpData = data.into_inner(); let mut user = headers.user; - data.validate(&user, true, &conn).await?; + data.validate(&mut user, true, &conn).await?; user.reset_security_stamp(&conn).await?; let save_result = user.save(&conn).await; @@ -1289,9 +1289,9 @@ async fn post_delete_account(data: Json, headers: Headers, co #[delete("/accounts", data = "")] async fn delete_account(data: Json, headers: Headers, conn: DbConn) -> EmptyResult { let data: PasswordOrOtpData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; - data.validate(&user, true, &conn).await?; + data.validate(&mut user, true, &conn).await?; user.delete(&conn).await } @@ -1370,7 +1370,7 @@ pub async fn prelogin(data: Json, ip: ClientIp, conn: DbConn) -> J let (kdf_type, kdf_iter, kdf_mem, kdf_para) = match User::find_by_mail(&data.email, &conn).await { Some(user) => (user.client_kdf_type, user.client_kdf_iter, user.client_kdf_memory, user.client_kdf_parallelism), - None => (User::CLIENT_KDF_TYPE_DEFAULT, User::CLIENT_KDF_ITER_DEFAULT, None, None), + None => (User::CLIENT_KDF_TYPE_DEFAULT, CONFIG.client_kdf_iter(), None, None), }; Ok(Json(json!({ @@ -1395,19 +1395,6 @@ struct SecretVerificationRequest { master_password_hash: String, } -// Change the KDF Iterations if necessary -pub async fn kdf_upgrade(user: &mut User, pwd_hash: &str, conn: &DbConn) -> ApiResult<()> { - if user.password_iterations < CONFIG.password_iterations() { - user.password_iterations = CONFIG.password_iterations(); - user.set_password(pwd_hash, None, false, None, conn).await?; - - if let Err(e) = user.save(conn).await { - error!("Error updating user: {e:#?}"); - } - } - Ok(()) -} - #[post("/accounts/verify-password", data = "")] async fn verify_password(data: Json, headers: Headers, conn: DbConn) -> JsonResult { let data: SecretVerificationRequest = data.into_inner(); @@ -1426,7 +1413,7 @@ async fn update_api_key(data: Json, rotate: bool, headers: He let data: PasswordOrOtpData = data.into_inner(); let mut user = headers.user; - data.validate(&user, true, &conn).await?; + data.validate(&mut user, true, &conn).await?; if rotate || user.api_key.is_none() { user.api_key = Some(crypto::generate_api_key()); diff --git a/src/api/core/ciphers.rs b/src/api/core/ciphers.rs index a5b7e58b..a962a719 100644 --- a/src/api/core/ciphers.rs +++ b/src/api/core/ciphers.rs @@ -1716,9 +1716,9 @@ async fn purge_org_vault( } let data: PasswordOrOtpData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; - data.validate(&user, true, &conn).await?; + data.validate(&mut user, true, &conn).await?; match Membership::find_confirmed_by_user_and_org(&user.uuid, &organization.org_id, &conn).await { Some(member) if member.atype == MembershipType::Owner => { @@ -1752,7 +1752,7 @@ async fn purge_personal_vault( let data: PasswordOrOtpData = data.into_inner(); let mut user = headers.user; - data.validate(&user, true, &conn).await?; + data.validate(&mut user, true, &conn).await?; for cipher in Cipher::find_owned_by_user(&user.uuid, &conn).await { cipher.delete(&conn).await?; diff --git a/src/api/core/organizations.rs b/src/api/core/organizations.rs index 36297d30..e520c0d9 100644 --- a/src/api/core/organizations.rs +++ b/src/api/core/organizations.rs @@ -232,12 +232,14 @@ async fn delete_organization( headers: OwnerHeaders, conn: DbConn, ) -> EmptyResult { + let mut user = headers.user; + if org_id != headers.org_id { err!("Organization not found", "Organization id's do not match"); } let data: PasswordOrOtpData = data.into_inner(); - data.validate(&headers.user, true, &conn).await?; + data.validate(&mut user, true, &conn).await?; match Organization::find_by_uuid(&org_id, &conn).await { None => err!("Organization not found"), @@ -3167,12 +3169,13 @@ async fn put_reset_password_enrollment( data: Json, conn: DbConn, ) -> EmptyResult { - if user_id != headers.user.uuid { + let mut membership = headers.membership; + let mut user = headers.user; + + if user_id != user.uuid { err!("User to enroll isn't member of required organization", "The user_id and acting user do not match"); } - let mut membership = headers.membership; - check_reset_password_applicable(&org_id, &conn).await?; let reset_request = data.into_inner(); @@ -3192,7 +3195,7 @@ async fn put_reset_password_enrollment( master_password_hash: reset_request.master_password_hash, otp: reset_request.otp, } - .validate(&headers.user, true, &conn) + .validate(&mut user, true, &conn) .await?; } @@ -3205,8 +3208,7 @@ async fn put_reset_password_enrollment( EventType::OrganizationUserResetPasswordWithdraw }; - log_event(event_type, &membership.uuid, &org_id, &headers.user.uuid, headers.device.atype, &headers.ip.ip, &conn) - .await; + log_event(event_type, &membership.uuid, &org_id, &user.uuid, headers.device.atype, &headers.ip.ip, &conn).await; Ok(()) } @@ -3240,10 +3242,10 @@ async fn api_key( err!("Organization not found", "Organization id's do not match"); } let data: PasswordOrOtpData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; // Validate the admin users password/otp - data.validate(&user, true, &conn).await?; + data.validate(&mut user, true, &conn).await?; let org_api_key = if let Some(mut org_api_key) = OrganizationApiKey::find_by_org_uuid(org_id, &conn).await { if rotate { diff --git a/src/api/core/two_factor/authenticator.rs b/src/api/core/two_factor/authenticator.rs index 692e8248..6394d90e 100644 --- a/src/api/core/two_factor/authenticator.rs +++ b/src/api/core/two_factor/authenticator.rs @@ -21,9 +21,9 @@ pub fn routes() -> Vec { #[post("/two-factor/get-authenticator", data = "")] async fn generate_authenticator(data: Json, headers: Headers, conn: DbConn) -> JsonResult { let data: PasswordOrOtpData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; - data.validate(&user, false, &conn).await?; + data.validate(&mut user, false, &conn).await?; let type_ = TwoFactorType::Authenticator as i32; let twofactor = TwoFactor::find_by_user_and_type(&user.uuid, type_, &conn).await; @@ -65,7 +65,7 @@ async fn activate_authenticator(data: Json, headers: He master_password_hash: data.master_password_hash, otp: data.otp, } - .validate(&user, true, &conn) + .validate(&mut user, true, &conn) .await?; // Validate key as base32 and 20 bytes length diff --git a/src/api/core/two_factor/duo.rs b/src/api/core/two_factor/duo.rs index ed112eb9..6973ce24 100644 --- a/src/api/core/two_factor/duo.rs +++ b/src/api/core/two_factor/duo.rs @@ -92,9 +92,9 @@ const DISABLED_MESSAGE_DEFAULT: &str = ", headers: Headers, conn: DbConn) -> JsonResult { let data: PasswordOrOtpData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; - data.validate(&user, false, &conn).await?; + data.validate(&mut user, false, &conn).await?; let data = get_user_duo_data(&user.uuid, &conn).await; @@ -164,7 +164,7 @@ async fn activate_duo(data: Json, headers: Headers, conn: DbConn) master_password_hash: data.master_password_hash.clone(), otp: data.otp.clone(), } - .validate(&user, true, &conn) + .validate(&mut user, true, &conn) .await?; let (data, data_str) = if check_duo_fields_custom(&data) { diff --git a/src/api/core/two_factor/email.rs b/src/api/core/two_factor/email.rs index 3667b871..6a0847fa 100644 --- a/src/api/core/two_factor/email.rs +++ b/src/api/core/two_factor/email.rs @@ -135,9 +135,9 @@ pub async fn send_token(user_id: &UserId, conn: &DbConn) -> EmptyResult { #[post("/two-factor/get-email", data = "")] async fn get_email(data: Json, headers: Headers, conn: DbConn) -> JsonResult { let data: PasswordOrOtpData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; - data.validate(&user, false, &conn).await?; + data.validate(&mut user, false, &conn).await?; let (enabled, mfa_email) = match TwoFactor::find_by_user_and_type(&user.uuid, TwoFactorType::Email as i32, &conn).await { @@ -168,13 +168,13 @@ struct SendEmailData { #[post("/two-factor/send-email", data = "")] async fn send_email(data: Json, headers: Headers, conn: DbConn) -> EmptyResult { let data: SendEmailData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; PasswordOrOtpData { master_password_hash: data.master_password_hash, otp: data.otp, } - .validate(&user, false, &conn) + .validate(&mut user, false, &conn) .await?; if !CONFIG._enable_email_2fa() { @@ -219,7 +219,7 @@ async fn email(data: Json, headers: Headers, conn: DbConn) -> JsonRes master_password_hash: data.master_password_hash, otp: data.otp, } - .validate(&user, true, &conn) + .validate(&mut user, true, &conn) .await?; let type_ = TwoFactorType::EmailVerificationChallenge as i32; diff --git a/src/api/core/two_factor/mod.rs b/src/api/core/two_factor/mod.rs index 0eb6563e..f6f82731 100644 --- a/src/api/core/two_factor/mod.rs +++ b/src/api/core/two_factor/mod.rs @@ -108,9 +108,9 @@ async fn get_twofactor(headers: Headers, conn: DbConn) -> Json { #[post("/two-factor/get-recover", data = "")] async fn get_recover(data: Json, headers: Headers, conn: DbConn) -> JsonResult { let data: PasswordOrOtpData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; - data.validate(&user, true, &conn).await?; + data.validate(&mut user, true, &conn).await?; Ok(Json(json!({ "code": user.totp_recover, @@ -137,14 +137,14 @@ struct DisableTwoFactorData { #[post("/two-factor/disable", data = "")] async fn disable_twofactor(data: Json, headers: Headers, conn: DbConn) -> JsonResult { let data: DisableTwoFactorData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; // Delete directly after a valid token has been provided PasswordOrOtpData { master_password_hash: data.master_password_hash, otp: data.otp, } - .validate(&user, true, &conn) + .validate(&mut user, true, &conn) .await?; let type_ = data.r#type.into_i32()?; diff --git a/src/api/core/two_factor/webauthn.rs b/src/api/core/two_factor/webauthn.rs index 07b964e5..8fb77fec 100644 --- a/src/api/core/two_factor/webauthn.rs +++ b/src/api/core/two_factor/webauthn.rs @@ -114,9 +114,9 @@ async fn get_webauthn(data: Json, headers: Headers, conn: DbC } let data: PasswordOrOtpData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; - data.validate(&user, false, &conn).await?; + data.validate(&mut user, false, &conn).await?; let (enabled, registrations) = get_webauthn_registrations(&user.uuid, &conn).await?; let registrations_json: Vec = registrations.iter().map(WebauthnRegistration::to_json).collect(); @@ -131,9 +131,9 @@ async fn get_webauthn(data: Json, headers: Headers, conn: DbC #[post("/two-factor/get-webauthn-challenge", data = "")] async fn generate_webauthn_challenge(data: Json, headers: Headers, conn: DbConn) -> JsonResult { let data: PasswordOrOtpData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; - data.validate(&user, false, &conn).await?; + data.validate(&mut user, false, &conn).await?; let registrations = get_webauthn_registrations(&user.uuid, &conn) .await? @@ -261,7 +261,7 @@ async fn activate_webauthn(data: Json, headers: Headers, con master_password_hash: data.master_password_hash, otp: data.otp, } - .validate(&user, true, &conn) + .validate(&mut user, true, &conn) .await?; // Retrieve and delete the saved challenge state diff --git a/src/api/core/two_factor/yubikey.rs b/src/api/core/two_factor/yubikey.rs index eb3d6dfd..3ff0622e 100644 --- a/src/api/core/two_factor/yubikey.rs +++ b/src/api/core/two_factor/yubikey.rs @@ -120,9 +120,9 @@ async fn generate_yubikey(data: Json, headers: Headers, conn: get_yubico_credentials()?; let data: PasswordOrOtpData = data.into_inner(); - let user = headers.user; + let mut user = headers.user; - data.validate(&user, false, &conn).await?; + data.validate(&mut user, false, &conn).await?; let user_id = &user.uuid; let yubikey_type = TwoFactorType::YubiKey as i32; @@ -156,7 +156,7 @@ async fn activate_yubikey(data: Json, headers: Headers, conn: master_password_hash: data.master_password_hash.clone(), otp: data.otp.clone(), } - .validate(&user, true, &conn) + .validate(&mut user, true, &conn) .await?; // Check if we already have some data diff --git a/src/api/identity.rs b/src/api/identity.rs index 6808ddde..b4426fe7 100644 --- a/src/api/identity.rs +++ b/src/api/identity.rs @@ -14,14 +14,14 @@ use crate::{ api::{ ApiResult, EmptyResult, JsonResult, core::{ - accounts::{PreloginData, RegisterData, kdf_upgrade, prelogin, register}, + accounts::{PreloginData, RegisterData, prelogin, register}, log_user_event, two_factor::{ authenticator, duo, duo_oidc, email, enforce_2fa_policy, is_twofactor_provider_usable, webauthn, yubikey, }, }, - master_password_policy, + kdf_upgrade, master_password_policy, push::register_push_device, }, auth, diff --git a/src/api/mod.rs b/src/api/mod.rs index 9a79ce95..5a30f7cc 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -54,11 +54,24 @@ struct PasswordOrOtpData { otp: Option, } +// Change the KDF Iterations if necessary +async fn kdf_upgrade(user: &mut User, pwd_hash: &str, conn: &DbConn) -> EmptyResult { + if user.password_iterations < CONFIG.password_iterations() { + user.password_iterations = CONFIG.password_iterations(); + user.set_password(pwd_hash, None, false, None, conn).await?; + + if let Err(e) = user.save(conn).await { + error!("Error updating user: {e:#?}"); + } + } + Ok(()) +} + impl PasswordOrOtpData { /// Tokens used via this struct can be used multiple times during the process /// First for the validation to continue, after that to enable or validate the following actions /// This is different per caller, so it can be adjusted to delete the token or not - pub async fn validate(&self, user: &User, delete_if_valid: bool, conn: &DbConn) -> EmptyResult { + pub async fn validate(&self, user: &mut User, delete_if_valid: bool, conn: &DbConn) -> EmptyResult { use crate::api::core::two_factor::protected_actions::validate_protected_action_otp; match (self.master_password_hash.as_deref(), self.otp.as_deref()) { @@ -66,6 +79,8 @@ impl PasswordOrOtpData { if !user.check_valid_password(pw_hash) { err!("Invalid password"); } + + kdf_upgrade(user, pw_hash, conn).await?; } (None, Some(otp)) => { validate_protected_action_otp(otp, &user.uuid, delete_if_valid, conn).await?; diff --git a/src/config.rs b/src/config.rs index 9f0ae2e1..3d90eacb 100644 --- a/src/config.rs +++ b/src/config.rs @@ -661,7 +661,9 @@ make_config! { client { /// Control whether clients onboarding interstitials are suppressed |> post-login welcome dialogs, extension install prompts, setup extension redirects, and premium upsell modals - client_suppress_onboarding: bool, true, def, false; + client_suppress_onboarding: bool, true, def, false; + /// In most cases this value is set by the client |> Will block clients from setting too low of a value in `accounts::post_kdf` + client_kdf_iter: i32, false, def, 600_000; }, /// Advanced settings @@ -938,6 +940,21 @@ make_config! { }, } +fn check_iterations(cfg: &ConfigItems, iterations: i32, key: &str) -> Result<(), Error> { + if iterations < 100_000 { + if let Ok(url) = Url::parse(&cfg.domain) + && url.domain().is_none() + { + // Warn only if it's probably not exposed externnaly + println!("[WARNING] {key} should be at least 100000 or higher. The default is 600000!"); + } else { + err!(format!("{key} should be at least 100000 or higher. The default is 600000!")); + } + } + + Ok(()) +} + fn validate_config(cfg: &ConfigItems, on_update: bool) -> Result<(), Error> { // Validate connection URL is valid and DB feature is enabled #[cfg(sqlite)] @@ -972,9 +989,8 @@ fn validate_config(cfg: &ConfigItems, on_update: bool) -> Result<(), Error> { } } - if cfg.password_iterations < 100_000 { - err!("PASSWORD_ITERATIONS should be at least 100000 or higher. The default is 600000!"); - } + check_iterations(cfg, cfg.password_iterations, "PASSWORD_ITERATIONS")?; + check_iterations(cfg, cfg.client_kdf_iter, "CLIENT_KDF_ITER")?; let limit = 256; if cfg.database_max_conns < 1 || cfg.database_max_conns > limit { diff --git a/src/db/models/user.rs b/src/db/models/user.rs index 3412b142..aa377834 100644 --- a/src/db/models/user.rs +++ b/src/db/models/user.rs @@ -109,7 +109,6 @@ pub struct UserStampException { /// Local methods impl User { pub const CLIENT_KDF_TYPE_DEFAULT: i32 = UserKdfType::Pbkdf2 as i32; - pub const CLIENT_KDF_ITER_DEFAULT: i32 = 600_000; pub fn new(email: &str, name: Option) -> Self { let now = Utc::now().naive_utc(); @@ -147,7 +146,7 @@ impl User { excluded_globals: "[]".to_owned(), client_kdf_type: Self::CLIENT_KDF_TYPE_DEFAULT, - client_kdf_iter: Self::CLIENT_KDF_ITER_DEFAULT, + client_kdf_iter: CONFIG.client_kdf_iter(), client_kdf_memory: None, client_kdf_parallelism: None,