From 79b496bdfb7500936bac49b656725dd0af2e56a7 Mon Sep 17 00:00:00 2001 From: rays Date: Fri, 23 Oct 2026 15:03:19 +0000 Subject: [PATCH] A password check holds its turn until it finishes (#324) check_password took a permit from CHECKS in the async function and ran Argon2 in spawn_blocking. A client that hung up dropped the future and with it the permit, while the blocking check ran on, so a flood of sign-ins that disconnected at once ran unbounded Argon2 work, the very thing CHECKS was added for (#137). The permit is now owned (CHECKS is an Arc) and moves into the blocking closure, so it is freed when the check ends, whoever stopped waiting. Co-Authored-By: Claude Opus 5.5 --- crates/ipx/CHANGELOG.md | 1 + crates/ipx/src/auth.rs | 24 ++++++++++++++++-------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/crates/ipx/CHANGELOG.md b/crates/ipx/CHANGELOG.md index 9a80dff..2bab34e 100644 --- a/crates/ipx/CHANGELOG.md +++ b/crates/ipx/CHANGELOG.md @@ -39,6 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - An open page stops getting live updates within half a minute of its account being removed. They went on until the page was closed (#335). - Only someone subscribed to a feed, or an admin, may mark it private. Anyone signed in could keep anyone's feed, or a whole OPML's, out of the directory, and only an admin could undo it (#333). - An OPML upload may add up to 2,000 feeds; one with more is refused and adds none. Importing tens of thousands held up every change to the feed list for minutes and left them all in it (#338). +- Password checks stay a few at a time when the person signing in gives up before the answer. Each sign-in that hung up let another start, so a flood of them used every core (#324). ## [1.4.0] - 2026-10-19 diff --git a/crates/ipx/src/auth.rs b/crates/ipx/src/auth.rs index 917c4e8..facb2bd 100644 --- a/crates/ipx/src/auth.rs +++ b/crates/ipx/src/auth.rs @@ -31,8 +31,10 @@ pub fn verify_password(password: &str, stored: &str) -> bool { /// 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))) +static CHECKS: std::sync::LazyLock> = std::sync::LazyLock::new(|| { + std::sync::Arc::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 @@ -45,12 +47,18 @@ static DECOY: std::sync::LazyLock = /// 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 + let Ok(turn) = CHECKS.clone().acquire_owned().await else { return false }; + // The turn goes with the check, not with this future: a client that hung up dropped the + // future, freeing the turn while the check ran on, so disconnecting sign-ins ran any number + // at once (#324). + tokio::task::spawn_blocking(move || { + let _turn = turn; + match stored { + Some(h) => verify_password(&password, &h), + None => { + verify_password(&password, &DECOY); + false + } } }) .await