Ten wrong passwords from one address hold it off sign-in for a quarter hour (#409)

POST /api/login answered every wrong password 401 for as long as someone
kept trying. Now ipx counts them by the client's address: the tenth in a
window holds that address off until the window, which starts at its first
wrong password, ends, answered 429 with how long is left. Counted by
address and not name, since by name anyone could lock anyone out; a right
password does not reset the count, or a guesser with an account of their
own could clear it between guesses.

Behind Caddy every peer is Caddy, so the address is the last one in
X-Forwarded-For, taken only from trusted_proxies. Caddy now sets that to
{client_ip}: Cf-Connecting-IP for the tunnel's requests, believed only
from TUNNEL_PEER (servers trusted_proxies), and the peer for everyone
else, replacing whatever the client sent. Checked with Caddy against an
echo upstream: a forged XFF and Cf-Connecting-IP from elsewhere reach ipx
as the peer, the tunnel's Cf-Connecting-IP as itself.

The sign-in load test sends each attempt from its own address, so it
still measures a flood from many, whose passwords are all checked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RqkrVDqdiQEMWAkNGQ2tYA
This commit is contained in:
2026-11-07 13:18:38 +00:00
parent 46392c55e1
commit 8acf57961e
6 changed files with 128 additions and 6 deletions

View File

@@ -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

View File

@@ -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");

View File

@@ -44,6 +44,8 @@ pub struct WebState {
/// Cloudflare Access's signing keys, fetched once and kept.
pub access: Arc<crate::access::Keys>,
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<WebState>, Json(body): Json<Credentials>) -> Result<Response, ApiError> {
/// 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<std::sync::Mutex<HashMap<String, (Instant, u32)>>>;
/// 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<String, (Instant, u32)>, from: &str, now: Instant) -> Option<Duration> {
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<String, (Instant, u32)>, 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<WebState>,
ConnectInfo(peer): ConnectInfo<SocketAddr>,
headers: HeaderMap,
Json(body): Json<Credentials>,
) -> Result<Response, ApiError> {
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;

View File

@@ -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);

View File

@@ -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

View File

@@ -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}
}
}