Merge 878b88919b into d2660324e6

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

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

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}

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<ChangePassData>, 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<PasswordOrOtpData>, 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<PasswordOrOtpData>, headers: Headers, co
 #[delete("/accounts", data = "<data>")]
 async fn delete_account(data: Json<PasswordOrOtpData>, 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<PreloginData>, 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 = "<data>")]
 async fn verify_password(data: Json<SecretVerificationRequest>, headers: Headers, conn: DbConn) -> JsonResult {
     let data: SecretVerificationRequest = data.into_inner();
@@ -1426,7 +1413,7 @@ async fn update_api_key(data: Json<PasswordOrOtpData>, 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());

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?;

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<OrganizationUserResetPasswordEnrollmentRequest>,
     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 {

src/api/core/two_factor/authenticator.rs

@@ -21,9 +21,9 @@ pub fn routes() -> Vec<Route> {
 #[post("/two-factor/get-authenticator", data = "<data>")]
 async fn generate_authenticator(data: Json<PasswordOrOtpData>, 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<EnableAuthenticatorData>, 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

src/api/core/two_factor/duo.rs

@@ -92,9 +92,9 @@ const DISABLED_MESSAGE_DEFAULT: &str = "<To use the global Duo keys, please leav
 #[post("/two-factor/get-duo", data = "<data>")]
 async fn get_duo(data: Json<PasswordOrOtpData>, 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<EnableDuoData>, 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) {

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 = "<data>")]
 async fn get_email(data: Json<PasswordOrOtpData>, 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 = "<data>")]
 async fn send_email(data: Json<SendEmailData>, 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<EmailData>, 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;

src/api/core/two_factor/mod.rs

@@ -108,9 +108,9 @@ async fn get_twofactor(headers: Headers, conn: DbConn) -> Json<Value> {
 #[post("/two-factor/get-recover", data = "<data>")]
 async fn get_recover(data: Json<PasswordOrOtpData>, 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 = "<data>")]
 async fn disable_twofactor(data: Json<DisableTwoFactorData>, 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()?;

src/api/core/two_factor/webauthn.rs

@@ -114,9 +114,9 @@ async fn get_webauthn(data: Json<PasswordOrOtpData>, 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<Value> = registrations.iter().map(WebauthnRegistration::to_json).collect();
@@ -131,9 +131,9 @@ async fn get_webauthn(data: Json<PasswordOrOtpData>, headers: Headers, conn: DbC
 #[post("/two-factor/get-webauthn-challenge", data = "<data>")]
 async fn generate_webauthn_challenge(data: Json<PasswordOrOtpData>, 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<EnableWebauthnData>, 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

src/api/core/two_factor/yubikey.rs

@@ -120,9 +120,9 @@ async fn generate_yubikey(data: Json<PasswordOrOtpData>, 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<EnableYubikeyData>, 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

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,

src/api/mod.rs

@@ -54,11 +54,24 @@ struct PasswordOrOtpData {
     otp: Option<String>,
 }
 
+// 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?;

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 {

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<String>) -> 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,