diff --git a/crates/ipx/CHANGELOG.md b/crates/ipx/CHANGELOG.md index 13a0b06..c3541da 100644 --- a/crates/ipx/CHANGELOG.md +++ b/crates/ipx/CHANGELOG.md @@ -32,6 +32,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - A feed you already have, added again by the same address over http rather than https or the other way round, is the one you have, not a second subscription with every item twice. Changing a feed's address to another feed's over the other scheme is refused the same way (#362). - A podcast that was once in an OPML and is now a feed of its own is no longer hidden from All Subscriptions when that OPML is, so its unread items are listed there as the feed list counts them (#358). +### Security + +- Ten wrong passwords from one address in a quarter hour hold that address off sign-in for the rest of it, answered `429` with how long to wait; before, a guesser could try as fast as the server checked. The address is the one a trusted proxy names last in `X-Forwarded-For`, or the peer's (#409). + ## [1.5.1] - 2026-10-24 ### Changed diff --git a/crates/ipx/src/daemon.rs b/crates/ipx/src/daemon.rs index e924170..cbc795c 100644 --- a/crates/ipx/src/daemon.rs +++ b/crates/ipx/src/daemon.rs @@ -310,7 +310,14 @@ pub(crate) async fn start_web( let at = metrics::spawn(&cfg.web.metrics_bind, sources)?; tracing::info!(%at, "serving metrics for Prometheus at /metrics"); } - let state = web::WebState { ctx: ctx.clone(), cmds: cmds.clone(), events: events.clone(), access, playing }; + let state = web::WebState { + ctx: ctx.clone(), + cmds: cmds.clone(), + events: events.clone(), + access, + playing, + wrong: Default::default(), + }; Ok(Some(tokio::spawn(async move { if let Err(e) = web::serve(state, &bind).await { tracing::error!(error = %format!("{e:#}"), "web ui stopped"); diff --git a/crates/ipx/src/web.rs b/crates/ipx/src/web.rs index 3ae79bf..a331592 100644 --- a/crates/ipx/src/web.rs +++ b/crates/ipx/src/web.rs @@ -44,6 +44,8 @@ pub struct WebState { /// Cloudflare Access's signing keys, fetched once and kept. pub access: Arc, pub playing: crate::deadair::Playing, + /// Wrong passwords by address, for `login` to refuse a guesser (#409). + pub wrong: Wrong, } pub fn router(state: WebState) -> Router { @@ -301,13 +303,58 @@ struct Credentials { password: String, } -async fn login(State(state): State, Json(body): Json) -> Result { +/// Wrong passwords one address may send in `WRONG_WINDOW` (#409). Enough for someone who has +/// forgotten which password it was; a guesser gets ten a quarter hour instead of as many as the +/// server can check. Counted by address, not name: by name, anyone could lock anyone out. +const WRONG_PER_ADDRESS: u32 = 10; +const WRONG_WINDOW: Duration = Duration::from_secs(15 * 60); +pub type Wrong = Arc>>; + +/// Who is asking: the peer, or, when the peer is a trusted proxy, the address it put last in +/// `X-Forwarded-For`. Behind Caddy every peer is Caddy, which sets that header to the client's +/// address ({client_ip}, Cloudflare's `Cf-Connecting-IP` for the tunnel's requests). +fn client_addr(cfg: &crate::config::Config, peer: String, headers: &HeaderMap) -> String { + if !cfg.web.trusted_proxies.iter().any(|p| p == &peer) { + return peer; + } + let forwarded = headers.get("x-forwarded-for").and_then(|v| v.to_str().ok()); + forwarded.and_then(|f| f.rsplit(',').next()).map(str::trim).filter(|a| !a.is_empty()).map_or(peer, str::to_owned) +} + +/// How long `from` has still to wait, if it has used up its wrong passwords. +fn wrong_wait(wrong: &HashMap, from: &str, now: Instant) -> Option { + let (start, n) = wrong.get(from)?; + let gone = now.duration_since(*start); + (*n >= WRONG_PER_ADDRESS && gone < WRONG_WINDOW).then(|| WRONG_WINDOW - gone) +} + +/// Counts one against `from`'s window, which starts at its first wrong password in it. +fn count_wrong(wrong: &mut HashMap, from: &str, now: Instant) { + // Windows past their end are forgotten, or a flood from many addresses would be kept forever. + wrong.retain(|_, (start, _)| now.duration_since(*start) < WRONG_WINDOW); + wrong.entry(from.to_owned()).or_insert((now, 0)).1 += 1; +} + +async fn login( + State(state): State, + ConnectInfo(peer): ConnectInfo, + headers: HeaderMap, + Json(body): Json, +) -> Result { + let from = client_addr(&state.ctx.cfg(), peer.ip().to_string(), &headers); + let wait = wrong_wait(&state.wrong.lock().expect("a panic elsewhere poisoned this lock"), &from, Instant::now()); + if let Some(wait) = wait { + tracing::warn!(%from, "sign-in refused: too many wrong passwords"); + let minutes = wait.as_secs() / 60 + 1; + return Err(ApiError::too_many(format!("too many wrong passwords; try again in {minutes} minutes"))); + } let name = body.name.trim().to_ascii_lowercase(); let user = state.ctx.db.user_by_name(&name).await?; // 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"); + count_wrong(&mut state.wrong.lock().expect("a panic elsewhere poisoned this lock"), &from, Instant::now()); + tracing::warn!(user = %name, %from, "failed sign-in"); return Ok((StatusCode::UNAUTHORIZED, "wrong name or password").into_response()); } @@ -1305,6 +1352,7 @@ mod tests { events: broadcast::channel(8).0, access: Arc::new(access::keys()), playing: Default::default(), + wrong: Default::default(), }; (router(state), ctx) } @@ -1422,6 +1470,49 @@ mod tests { assert_eq!(ask(&app, "GET", "/api/me", "10.0.0.5", &[("cookie", "ipx_session=forged")], None).await.0, 401); } + #[tokio::test] + async fn ten_wrong_passwords_from_one_address_hold_it_off_and_no_other() { + let mut cfg = crate::config::Config::default(); + cfg.web.trusted_proxies = vec![CADDY.into()]; + let (app, ctx) = app(cfg).await; + let hash = crate::auth::hash_password("correct horse").unwrap(); + ctx.db.create_user("dee", Some(&hash), false).await.unwrap(); + let login = |pw: &str| Some(serde_json::json!({ "name": "dee", "password": pw })); + let via_caddy = |client| [("x-forwarded-for", client)]; + for _ in 0..10 { + let (s, _, _) = ask(&app, "POST", "/api/login", CADDY, &via_caddy("203.0.113.7"), login("wrong")).await; + assert_eq!(s, 401); + } + let (s, _, body) = + ask(&app, "POST", "/api/login", CADDY, &via_caddy("203.0.113.7"), login("correct horse")).await; + assert!(s == 429 && body.contains("try again in 15 minutes"), "{s} {body}"); + // Someone else behind the same Caddy is not held off with them, nor is Dee from elsewhere. + let (s, _, _) = + ask(&app, "POST", "/api/login", CADDY, &via_caddy("198.51.100.2"), login("correct horse")).await; + assert_eq!(s, 200); + // Only Caddy's word is taken for the address: anyone else sending the header is counted as + // themselves, and cannot step out of a window by naming a new address each time. + for n in 0..10 { + let hdr = [("x-forwarded-for", format!("192.0.2.{n}"))]; + let hdr: Vec<(&str, &str)> = hdr.iter().map(|(k, v)| (*k, v.as_str())).collect(); + assert_eq!(ask(&app, "POST", "/api/login", "10.0.0.9", &hdr, login("wrong")).await.0, 401); + } + assert_eq!(ask(&app, "POST", "/api/login", "10.0.0.9", &[], login("correct horse")).await.0, 429); + } + + #[test] + fn a_window_of_wrong_passwords_ends_and_is_forgotten() { + let mut wrong = HashMap::new(); + let t0 = Instant::now(); + for _ in 0..WRONG_PER_ADDRESS { + count_wrong(&mut wrong, "a", t0); + } + assert!(wrong_wait(&wrong, "a", t0).is_some()); + assert_eq!(wrong_wait(&wrong, "a", t0 + WRONG_WINDOW), None); + count_wrong(&mut wrong, "b", t0 + WRONG_WINDOW); + assert!(!wrong.contains_key("a"), "a past window is dropped"); + } + #[tokio::test] async fn a_signal_is_made_changed_listed_and_removed_by_its_owner_alone() { let (app, ctx) = app(proxied()).await; diff --git a/tests/load/signin.ts b/tests/load/signin.ts index 225e912..393099b 100644 --- a/tests/load/signin.ts +++ b/tests/load/signin.ts @@ -25,13 +25,19 @@ export const options = { signin_name_gap: ['med<10'], }, }; -const refused = { headers: { 'Content-Type': 'application/json' }, responseCallback: http.expectedStatuses(401) }; +// Each attempt from an address of its own, as Caddy would name it: one address is held off after +// ten (#409), and the flood to measure is one from many, which still has every password checked. +const refused = (n: number) => ({ + headers: { 'Content-Type': 'application/json', 'X-Forwarded-For': `10.${(n >> 16) & 255}.${(n >> 8) & 255}.${n & 255}` }, + responseCallback: http.expectedStatuses(401), +}); export function attempt() { + const n = __VU * 10_000 + __ITER; const known = http.post(`${BASE}/api/login`, JSON.stringify({ name: 'piper', password: 'not-the-password' }), - { ...refused, tags: { name: 'signin-known' } }); + { ...refused(2 * n), tags: { name: 'signin-known' } }); const unknown = http.post(`${BASE}/api/login`, JSON.stringify({ name: `nobody-${__VU}-${__ITER}`, password: 'not-the-password' }), - { ...refused, tags: { name: 'signin-unknown' } }); + { ...refused(2 * n + 1), tags: { name: 'signin-unknown' } }); check(known, { 'a wrong password refused': r => r.status === 401 }); check(unknown, { 'an unknown name refused': r => r.status === 401 }); gap.add(known.timings.duration - unknown.timings.duration); diff --git a/web/CHANGELOG.md b/web/CHANGELOG.md index 0664262..b86f5eb 100644 --- a/web/CHANGELOG.md +++ b/web/CHANGELOG.md @@ -35,6 +35,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - A tap or a key is its trace's root span: the page sends ipx when it began and when its last answer came, so Tempo shows how long it took beside the requests it made (#376). - Fonts of your choosing in Settings, beside the theme, kept by each browser as the theme is: one for the interface and one for reading an item's title and text, which is the interface's until chosen, with a little more room between the lines in a serif. Inter, your system's, the reading fonts Atkinson Hyperlegible, Lexend, Literata, Source Serif and OpenDyslexic, your device's serif, or monospace. The default keeps each theme's face as it was (#359). +### Security + +- Caddy passes ipx the address each request is from as `X-Forwarded-For`, Cloudflare's `Cf-Connecting-IP` for the tunnel's and taken from cloudflared alone, so ipx can hold off someone guessing passwords (#409). + ## [1.3.0] - 2026-10-23 ### Fixed diff --git a/web/Caddyfile b/web/Caddyfile index 533cd5c..31a90fe 100644 --- a/web/Caddyfile +++ b/web/Caddyfile @@ -13,6 +13,13 @@ { admin off auto_https off + # Who a request is from, {client_ip}: Cloudflare's Cf-Connecting-IP for the tunnel's requests, + # taken only from cloudflared, and the peer for everyone else's. ipx counts wrong passwords by it + # (#409), and without this every request through the tunnel is cloudflared's one address. + servers { + trusted_proxies static {$TUNNEL_PEER:255.255.255.255} + client_ip_headers Cf-Connecting-IP + } # Requests, their times and upstream errors as Prometheus metrics, served below (#371). metrics # Everything else Caddy logs, its errors among them, which carry the request too: the same @@ -95,6 +102,9 @@ http://{$WEB_BIND::8080} { # Live updates are a stream: flushed as they come, never buffered. reverse_proxy {$IPX_UPSTREAM:ipx:8099} { flush_interval -1 + # The client alone, not the chain Caddy would otherwise pass on, whose last address is + # cloudflared's for the tunnel: ipx takes the last as who is asking (#409). + header_up X-Forwarded-For {client_ip} } }