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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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<tokio::sync::Semaphore> = 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::Arc<tokio::sync::Semaphore>> = 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,13 +47,19 @@ static DECOY: std::sync::LazyLock<String> =
|
||||
/// 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<String>) -> bool {
|
||||
let Ok(_turn) = CHECKS.acquire().await else { return false };
|
||||
tokio::task::spawn_blocking(move || match stored {
|
||||
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
|
||||
.unwrap_or(false)
|
||||
|
||||
Reference in New Issue
Block a user