bugfix: equalise login response time across user-existence branches (0.34.1)
A login attempt against a non-existent username returned 401 in <1ms, while the wrong-password branch ran argon2 verify (~50-100ms). Timing the difference let an attacker enumerate valid usernames without ever seeing a successful response. Run verify_password against a fixed dummy argon2id hash on the no-user branch so both paths spend the same compute. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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<String> = 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<AppState>,
|
||||
jar: CookieJar,
|
||||
|
||||
Reference in New Issue
Block a user