The load tests (#133) sent forty clients' wrong passwords to /api/login, 54 attempts a second, which takes no account. Everyone else's requests took 4s (median 3.96s for /api/feeds, about 20ms otherwise) and `ipx status`, the healthcheck, 1.3s (#137): each attempt was an Argon2id check, tens of milliseconds of CPU, run inside the handler on one of the runtime's workers, so a handful at once held every worker the rest of the server answers on. And a wrong password for an account's name was refused a median 31ms later than one for a made-up name (#138), since only a name with a hash was checked: the answer read the same, the time said which names are accounts. auth::check_password runs the check on the blocking pool, at most half the cores at once, so a flood waits on itself, and checks a name with no account, or no password, against a fixed decoy hash, false in the same time. Under the same flood the rest of the site answers at p95 57ms, `ipx status` at most 90ms, the gap is 0.2ms, and twice as many attempts are answered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
- 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
|
## [0.10.0] - 2026-10-05
|
||||||
|
|
||||||
### Added
|
### Added
|
||||||
|
|||||||
39
src/auth.rs
39
src/auth.rs
@@ -30,6 +30,35 @@ pub fn verify_password(password: &str, stored: &str) -> bool {
|
|||||||
.is_ok()
|
.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<tokio::sync::Semaphore> = 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<String> =
|
||||||
|
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<String>) -> 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.
|
/// A session id: 256 bits of urandom, hex. Long enough that guessing is not a strategy.
|
||||||
pub fn new_session_token() -> String {
|
pub fn new_session_token() -> String {
|
||||||
let mut bytes = [0u8; 32];
|
let mut bytes = [0u8; 32];
|
||||||
@@ -76,6 +105,16 @@ pub fn name_from_header(raw: &str) -> Option<String> {
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
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]
|
#[test]
|
||||||
fn a_password_verifies_only_against_itself() {
|
fn a_password_verifies_only_against_itself() {
|
||||||
let h = hash_password("correct horse battery").unwrap();
|
let h = hash_password("correct horse battery").unwrap();
|
||||||
|
|||||||
@@ -315,11 +315,8 @@ async fn login(
|
|||||||
) -> Result<Response, ApiError> {
|
) -> Result<Response, ApiError> {
|
||||||
let name = body.name.trim().to_ascii_lowercase();
|
let name = body.name.trim().to_ascii_lowercase();
|
||||||
let user = state.ctx.db.user_by_name(&name).await?;
|
let user = state.ctx.db.user_by_name(&name).await?;
|
||||||
// The same answer either way: whether a name exists is not something to leak.
|
// The same answer either way, in the same time: whether a name exists is not something to leak.
|
||||||
let ok = user
|
let ok = crate::auth::check_password(body.password, user.as_ref().and_then(|u| u.pass_hash.clone())).await;
|
||||||
.as_ref()
|
|
||||||
.and_then(|u| u.pass_hash.as_deref())
|
|
||||||
.is_some_and(|h| crate::auth::verify_password(&body.password, h));
|
|
||||||
if !ok {
|
if !ok {
|
||||||
tracing::warn!(user = %name, "failed sign-in");
|
tracing::warn!(user = %name, "failed sign-in");
|
||||||
return Ok((StatusCode::UNAUTHORIZED, "wrong name or password").into_response());
|
return Ok((StatusCode::UNAUTHORIZED, "wrong name or password").into_response());
|
||||||
|
|||||||
Reference in New Issue
Block a user