diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bee9b8..c1bc762 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Checking every feed no longer makes every icon in the feed list flash while it runs: the list keeps the icons it has already drawn. +### Security + +- A flood of sign-in attempts no longer stalls the site for everyone else. Passwords are checked a few at a time, away from the threads that answer every other request; forty wrong passwords at a time made everything else take four seconds. +- A wrong password is refused in the same time whether or not the name is an account here, so how long it takes no longer tells anyone which names are. + ## [0.10.0] - 2026-10-05 ### Added diff --git a/src/auth.rs b/src/auth.rs index 8922197..610e760 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -30,6 +30,35 @@ pub fn verify_password(password: &str, stored: &str) -> bool { .is_ok() } +/// How many password checks run at once: each is tens of milliseconds of CPU, and forty wrong +/// passwords at a time, run on the async workers, made every other request wait 4s (#137). +/// Half the cores, so a flood of sign-ins waits on itself and the rest of the server has the rest. +static CHECKS: std::sync::LazyLock = std::sync::LazyLock::new(|| { + tokio::sync::Semaphore::new(std::thread::available_parallelism().map_or(1, |n| (n.get() / 2).max(1))) +}); + +/// What a name that is not an account is checked against, so that refusing it takes as long as +/// refusing a wrong password does. Refused without a check, it came back 31ms sooner, and the +/// time told anyone which names are accounts here (#138). +static DECOY: std::sync::LazyLock = + std::sync::LazyLock::new(|| hash_password("no account here has this password").expect("hashing a fixed password")); + +/// A sign-in's password against the account's hash, or against `DECOY` when there is no account +/// or it has no password: false either way, in the same time. Off the async workers, and a few at +/// a time; see `CHECKS`. +pub async fn check_password(password: String, stored: Option) -> bool { + let Ok(_turn) = CHECKS.acquire().await else { return false }; + tokio::task::spawn_blocking(move || match stored { + Some(h) => verify_password(&password, &h), + None => { + verify_password(&password, &DECOY); + false + } + }) + .await + .unwrap_or(false) +} + /// A session id: 256 bits of urandom, hex. Long enough that guessing is not a strategy. pub fn new_session_token() -> String { let mut bytes = [0u8; 32]; @@ -76,6 +105,16 @@ pub fn name_from_header(raw: &str) -> Option { mod tests { use super::*; + #[tokio::test] + async fn a_sign_in_without_an_account_is_checked_all_the_same() { + let h = hash_password("correct horse battery").unwrap(); + assert!(check_password("correct horse battery".into(), Some(h.clone())).await); + assert!(!check_password("wrong".into(), Some(h)).await); + assert!(!check_password("correct horse battery".into(), None).await); + // A decoy that does not parse is refused before any hashing, and the time says so (#138). + assert!(PasswordHash::new(&DECOY).is_ok()); + } + #[test] fn a_password_verifies_only_against_itself() { let h = hash_password("correct horse battery").unwrap(); diff --git a/src/web.rs b/src/web.rs index 0e8d151..41c3dd1 100644 --- a/src/web.rs +++ b/src/web.rs @@ -315,11 +315,8 @@ async fn login( ) -> Result { let name = body.name.trim().to_ascii_lowercase(); let user = state.ctx.db.user_by_name(&name).await?; - // The same answer either way: whether a name exists is not something to leak. - let ok = user - .as_ref() - .and_then(|u| u.pass_hash.as_deref()) - .is_some_and(|h| crate::auth::verify_password(&body.password, h)); + // The same answer either way, in the same time: whether a name exists is not something to leak. + let ok = crate::auth::check_password(body.password, user.as_ref().and_then(|u| u.pass_hash.clone())).await; if !ok { tracing::warn!(user = %name, "failed sign-in"); return Ok((StatusCode::UNAUTHORIZED, "wrong name or password").into_response());