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:
@@ -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
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user