From f3cb33b7cad810d625cfaae227a966cc5b7118c6 Mon Sep 17 00:00:00 2001 From: fylorn <249551762+fylorn@users.noreply.github.com> Date: Tue, 20 Oct 2026 22:55:43 +0800 Subject: [PATCH] Run Argon2 on the blocking pool in request handlers Login, password changes, admin-created users, password resets and first-run setup hashed or verified passwords with Argon2 directly on the async runtime, holding a worker for tens of milliseconds per call. The password module gains off-runtime versions that use spawn_blocking; the handlers await them. Hash parameters and the constant-time login against a dummy hash are unchanged. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 6 ++++ crates/auth/src/password.rs | 44 +++++++++++++++++++++++ crates/server/src/handlers/admin/users.rs | 4 +-- crates/server/src/handlers/auth.rs | 19 +++++----- crates/server/src/handlers/setup.rs | 2 +- 5 files changed, 64 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ada2f09f..02a54a30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,12 @@ target. handler does: `503 Service Unavailable` with the usual JSON error when the database is unavailable, `500` with it for any other database error, and an error log naming the lookup that failed. +- **Password hashing no longer holds up other requests.** Logins, + password changes, new users and first-run setup ran Argon2 (tens of + milliseconds of CPU by design) on the server's async workers, so every + other request scheduled on that worker waited for it. They now run it on + the blocking thread pool; the hashes and the constant-time login are + unchanged. ## [3.8.3] — 2026-10-20 diff --git a/crates/auth/src/password.rs b/crates/auth/src/password.rs index 6e62b3b4..3e8773b7 100644 --- a/crates/auth/src/password.rs +++ b/crates/auth/src/password.rs @@ -52,10 +52,54 @@ pub fn verify_password_or_dummy(password: &str, hash: Option<&str>) -> bool { } } +// Argon2 costs tens of milliseconds of CPU on purpose. Called from a +// request handler it would hold an async worker for that long, stalling +// every other request scheduled on that thread, so the handlers use +// these versions, which run it on the blocking pool. + +/// [`hash_password`] on the blocking pool. +pub async fn hash_password_off_runtime(password: String) -> anyhow::Result { + tokio::task::spawn_blocking(move || hash_password(&password)) + .await + .map_err(|e| anyhow::anyhow!("Password hashing task failed: {e}"))? +} + +/// [`verify_password`] on the blocking pool. +pub async fn verify_password_off_runtime(password: String, hash: String) -> anyhow::Result { + tokio::task::spawn_blocking(move || verify_password(&password, &hash)) + .await + .map_err(|e| anyhow::anyhow!("Password verification task failed: {e}"))? +} + +/// [`verify_password_or_dummy`] on the blocking pool. A task that cannot +/// run counts as a mismatch, like an unreadable hash. +pub async fn verify_password_or_dummy_off_runtime(password: String, hash: Option) -> bool { + tokio::task::spawn_blocking(move || verify_password_or_dummy(&password, hash.as_deref())) + .await + .unwrap_or(false) +} + #[cfg(test)] mod tests { use super::*; + #[tokio::test] + async fn the_off_runtime_versions_agree_with_the_blocking_ones() { + let hash = hash_password_off_runtime("pw".into()).await.unwrap(); + assert!( + verify_password_off_runtime("pw".into(), hash.clone()) + .await + .unwrap() + ); + assert!( + !verify_password_off_runtime("nope".into(), hash.clone()) + .await + .unwrap() + ); + assert!(verify_password_or_dummy_off_runtime("pw".into(), Some(hash)).await); + assert!(!verify_password_or_dummy_off_runtime("pw".into(), None).await); + } + /// A hash written by argon2 0.5.3 (`Argon2::default()`, fixed salt). /// Stored hashes outlive crate upgrades: they must keep verifying. const KAT_HASH: &str = "$argon2id$v=19$m=19456,t=2,p=1$dGhpbmt3YXRjaC1rYXQtc2FsdA$6xffcgfh1b9KJMWg4mBS4qOWsK5MNUM1asLNHvgV0v8"; diff --git a/crates/server/src/handlers/admin/users.rs b/crates/server/src/handlers/admin/users.rs index 5a31d264..e304670f 100644 --- a/crates/server/src/handlers/admin/users.rs +++ b/crates/server/src/handlers/admin/users.rs @@ -458,7 +458,7 @@ pub async fn create_user( return Err(AppError::Conflict("Email already registered".into())); } - let password_hash = password::hash_password(&raw_password)?; + let password_hash = password::hash_password_off_runtime(raw_password.clone()).await?; let mut tx = state.db.begin().await?; @@ -920,7 +920,7 @@ pub async fn reset_user_password( } let new_password = password::generate_random_password(); - let hash = password::hash_password(&new_password)?; + let hash = password::hash_password_off_runtime(new_password.clone()).await?; user_repository::update_password_hash(&state.db, user_id, &hash, true).await?; diff --git a/crates/server/src/handlers/auth.rs b/crates/server/src/handlers/auth.rs index 71d868e7..4be44119 100644 --- a/crates/server/src/handlers/auth.rs +++ b/crates/server/src/handlers/auth.rs @@ -497,10 +497,11 @@ pub async fn login( // Constant-time login: always run Argon2, against a dummy hash when // there is no account or it has no password, so the response time does // not tell which addresses have accounts. - let password_valid = password::verify_password_or_dummy( - &req.password, - user.as_ref().and_then(|u| u.password_hash.as_deref()), - ); + let password_valid = password::verify_password_or_dummy_off_runtime( + req.password.clone(), + user.as_ref().and_then(|u| u.password_hash.clone()), + ) + .await; if !password_valid || user.is_none() { // Per-email cross-IP failure counter. The `count` above is keyed @@ -1053,7 +1054,7 @@ pub async fn register( let email = normalize_email(&req.email); validate_email(&email)?; - let password_hash = password::hash_password(&req.password)?; + let password_hash = password::hash_password_off_runtime(req.password.clone()).await?; // Use a transaction to ensure user creation + role assignment are atomic let mut tx = state.db.begin().await?; @@ -1467,7 +1468,9 @@ pub async fn change_password( return Err(AppError::Unauthorized); } - if !password::verify_password(&req.old_password, current_hash)? { + if !password::verify_password_off_runtime(req.old_password.clone(), current_hash.to_owned()) + .await? + { let fails = crate::services::auth_lockout::record_failure(&state.redis, &pwchange_fail_key).await?; if let Some(secs) = crate::services::auth_lockout::ladder_secs(fails) { @@ -1481,7 +1484,7 @@ pub async fn change_password( crate::services::auth_lockout::clear(&state.redis, &[&pwchange_fail_key, &pwchange_locked_key]) .await; - let new_hash = password::hash_password(&req.new_password)?; + let new_hash = password::hash_password_off_runtime(req.new_password.clone()).await?; repo::set_own_password(&state.db, user.id, &new_hash).await?; // Revoke all signing public keys for this user (invalidates sessions) @@ -1832,7 +1835,7 @@ pub async fn totp_disable( let hash = user.password_hash.as_ref().ok_or(AppError::BadRequest( "SSO accounts cannot manage TOTP here".into(), ))?; - if !password::verify_password(&req.old_password, hash)? { + if !password::verify_password_off_runtime(req.old_password.clone(), hash.to_owned()).await? { return Err(AppError::Unauthorized); } diff --git a/crates/server/src/handlers/setup.rs b/crates/server/src/handlers/setup.rs index 0084f427..6254e643 100644 --- a/crates/server/src/handlers/setup.rs +++ b/crates/server/src/handlers/setup.rs @@ -145,7 +145,7 @@ pub async fn setup_initialize( // Create the super_admin user with the first API key, and mark // setup done. - let password_hash = password::hash_password(&req.admin.password)?; + let password_hash = password::hash_password_off_runtime(req.admin.password.clone()).await?; let generated = api_key::generate_api_key(); let site_name = req.site_name.as_deref().unwrap_or("ThinkWatch"); let admin_user = repo::create_first_admin(