diff --git a/backend/Cargo.toml b/backend/Cargo.toml index c091570..a126262 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.34.0" +version = "0.34.1" edition = "2021" default-run = "mangalord" diff --git a/backend/src/api/auth.rs b/backend/src/api/auth.rs index 3dd5cc2..085858c 100644 --- a/backend/src/api/auth.rs +++ b/backend/src/api/auth.rs @@ -4,6 +4,8 @@ //! expire naturally rather than being explicitly invalidated, so other //! devices keep their existing logins). +use std::sync::OnceLock; + use axum::extract::{Path, State}; use axum::http::StatusCode; use axum::response::IntoResponse; @@ -102,9 +104,15 @@ async fn login( )); } - let user = repo::user::find_by_username(&state.db, username) - .await? - .ok_or(AppError::Unauthenticated)?; + let user = repo::user::find_by_username(&state.db, username).await?; + let Some(user) = user else { + // No such user. Run argon2 against a stable dummy hash so the + // response time matches the wrong-password branch — otherwise + // an attacker can enumerate usernames by timing the no-user + // 401 against the wrong-password 401. + let _ = verify_password(&input.password, dummy_password_hash()); + return Err(AppError::Unauthenticated); + }; if !verify_password(&input.password, &user.password_hash) { return Err(AppError::Unauthenticated); } @@ -113,6 +121,21 @@ async fn login( Ok((StatusCode::OK, jar, Json(AuthResponse { user }))) } +/// Lazily-computed argon2 hash used to equalise login response time +/// across the "no such user" and "wrong password" branches. Computing +/// it once (on the first login of the process) is enough — the hash is +/// never compared against a real password, only used to force argon2 +/// to do the same amount of work it would for a real verify. +fn dummy_password_hash() -> &'static str { + static DUMMY: OnceLock = OnceLock::new(); + DUMMY + .get_or_init(|| { + crate::auth::password::hash_password("login-timing-equaliser") + .expect("hash_password on a fixed input cannot fail") + }) + .as_str() +} + async fn logout( State(state): State, jar: CookieJar, diff --git a/backend/tests/api_auth.rs b/backend/tests/api_auth.rs index 8ded505..c89814f 100644 --- a/backend/tests/api_auth.rs +++ b/backend/tests/api_auth.rs @@ -567,6 +567,91 @@ async fn user_a_cannot_delete_user_b_token(pool: PgPool) { assert_eq!(resp.status(), StatusCode::NO_CONTENT); } +/// Username enumeration via login response time: an attacker probes +/// for valid usernames by measuring how long /auth/login takes. Before +/// the equalisation fix, the no-user branch returned 401 in <1 ms +/// while the wrong-password branch took ~50-100 ms (the argon2 verify +/// cost). This test asserts the no-user branch now spends at least +/// some meaningful fraction of the wrong-password branch's time. +/// +/// Tolerance is intentionally loose so CI variance doesn't flap the +/// test. The unequalised gap is large enough (~50x) that even a noisy +/// CI run with a 5x slack still catches it. +#[sqlx::test(migrations = "./migrations")] +async fn login_no_user_branch_runs_argon2_for_timing_equalisation(pool: PgPool) { + use std::time::Instant; + + let h = common::harness(pool); + + // Register the victim user so the wrong-password branch has a real + // argon2 hash to verify against. + let _ = h + .app + .clone() + .oneshot(common::post_json( + "/api/v1/auth/register", + json!({ "username": "victim", "password": "hunter2hunter2" }), + )) + .await + .unwrap(); + + // Warm-up: first login of the process initialises the dummy hash + // lazily. Skip that cost when measuring. + let _ = h + .app + .clone() + .oneshot(common::post_json( + "/api/v1/auth/login", + json!({ "username": "victim", "password": "wrong" }), + )) + .await + .unwrap(); + let _ = h + .app + .clone() + .oneshot(common::post_json( + "/api/v1/auth/login", + json!({ "username": "ghost", "password": "wrong" }), + )) + .await + .unwrap(); + + // Median-of-N is more stable than a single sample. + async fn sample_min( + app: &axum::Router, + username: &str, + n: u32, + ) -> std::time::Duration { + let mut samples = Vec::with_capacity(n as usize); + for _ in 0..n { + let req = common::post_json( + "/api/v1/auth/login", + json!({ "username": username, "password": "wrong-guess" }), + ); + let t = Instant::now(); + let resp = app.clone().oneshot(req).await.unwrap(); + let d = t.elapsed(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + samples.push(d); + } + // Use the minimum: it's the floor that argon2 takes, robust + // against unrelated stalls (DB connection acquisition, etc.). + *samples.iter().min().unwrap() + } + + let wrong_pwd = sample_min(&h.app, "victim", 3).await; + let no_user = sample_min(&h.app, "ghost", 3).await; + + // 5x slack: argon2 dominates both branches, so they should be + // within an order of magnitude. Unequalised, no_user would be + // ~50-100x faster. Asserting "no_user >= wrong_pwd / 5" catches + // the bug without being flaky in CI. + assert!( + no_user * 5 >= wrong_pwd, + "login timing leaks user existence: no_user={no_user:?}, wrong_pwd={wrong_pwd:?}" + ); +} + #[sqlx::test(migrations = "./migrations")] async fn delete_unknown_token_is_404(pool: PgPool) { let h = common::harness(pool); diff --git a/frontend/package.json b/frontend/package.json index 159dad9..72a32cd 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.34.0", + "version": "0.34.1", "private": true, "type": "module", "scripts": {