diff --git a/Cargo.lock b/Cargo.lock index 2ff0078..5bdf8b7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1708,9 +1708,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.52.4" +version = "1.53.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "317fafbbe3f02fc663dad00ea6186197de963cd4190e86a26d8d0fae095539af" +checksum = "d988bcd52dbe076d3d46903332f58c912b87a2c49b1428419a5845154762ffee" dependencies = [ "bytes", "libc", @@ -1724,9 +1724,9 @@ dependencies = [ [[package]] name = "tokio-macros" -version = "2.7.0" +version = "2.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "385a6cb71ab9ab790c5fe8d67f1645e6c450a7ce006a33de03daa956cf70a496" +checksum = "6328af13490e73a9b4694030fafd93f8c8c6a9dede33e821c3fc63eddf8042ba" dependencies = [ "proc-macro2", "quote", diff --git a/src/config.rs b/src/config.rs index df4fb7b..c1abe79 100644 --- a/src/config.rs +++ b/src/config.rs @@ -29,7 +29,11 @@ pub struct ServerConfig { /// HTTP API + UI, e.g. "0.0.0.0:8080" pub http_bind: String, - /// When the TCP peer is one of these addresses, use `X-Forwarded-For` for rate limiting. + /// When the TCP peer is one of these addresses, derive the client IP from + /// the rightmost `X-Forwarded-For` hop (the address appended by the + /// immediate trusted proxy). `X-Real-IP` is intentionally not trusted, as + /// proxies that don't set it themselves may pass a client-supplied value + /// through unmodified. pub http_trusted_proxies: Vec, /// HTTP rate-limit sliding window (seconds). diff --git a/src/rate_limit.rs b/src/rate_limit.rs index 2b5b306..4dbc890 100644 --- a/src/rate_limit.rs +++ b/src/rate_limit.rs @@ -144,15 +144,27 @@ fn client_ip(request: &Request, trusted_proxies: &[IpAddr]) -> IpAddr { .map(|info| info.0.ip()) .unwrap_or(IpAddr::from([127, 0, 0, 1])); - if trusted_proxies.contains(&peer) - && let Some(xff) = request + if trusted_proxies.contains(&peer) { + // Use the rightmost address: the one appended by the immediate trusted + // proxy ($proxy_add_x_forwarded_for), not client-controlled leftmost entries. + // + // X-Real-IP is deliberately NOT trusted here: unlike XFF, which the + // trusted proxy appends to, X-Real-IP is commonly just passed through + // unmodified by proxies that don't set it themselves, which would let + // a client pick an arbitrary rate-limit bucket by setting it directly. + if let Some(xff) = request .headers() .get("X-Forwarded-For") .and_then(|v| v.to_str().ok()) - && let Some(client) = xff.split(',').map(str::trim).find(|part| !part.is_empty()) - && let Ok(ip) = client.parse::() - { - return ip; + && let Some(rightmost) = xff.split(',').map(str::trim).rfind(|part| !part.is_empty()) + { + match rightmost.parse::() { + Ok(client) => return client, + Err(_) => crate::log_warn!( + "rate_limit: trusted proxy {peer} sent unparsable X-Forwarded-For hop '{rightmost}', falling back to peer IP" + ), + } + } } peer @@ -287,7 +299,7 @@ mod tests { } #[test] - fn trusted_proxy_uses_x_forwarded_for() { + fn trusted_proxy_uses_rightmost_x_forwarded_for() { use axum::body::Body; let proxy = IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1)); @@ -300,7 +312,47 @@ mod tests { .extensions_mut() .insert(ConnectInfo(SocketAddr::from(([10, 0, 0, 1], 12345)))); + let ip = client_ip(&request, &[proxy]); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1))); + } + + #[test] + fn trusted_proxy_ignores_client_supplied_leftmost_xff() { + use axum::body::Body; + + let proxy = IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1)); + let mut request = Request::builder() + .uri("/api/v1/health") + .header("X-Forwarded-For", "198.51.100.99, 203.0.113.5") + .body(Body::empty()) + .unwrap(); + request + .extensions_mut() + .insert(ConnectInfo(SocketAddr::from(([10, 0, 0, 1], 12345)))); + let ip = client_ip(&request, &[proxy]); assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(203, 0, 113, 5))); } + + #[test] + fn trusted_proxy_ignores_x_real_ip() { + // A proxy that only forwards X-Real-IP unmodified (rather than setting + // it itself) would let a client pick an arbitrary rate-limit bucket by + // sending this header directly, so it must never be trusted. + use axum::body::Body; + + let proxy = IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1)); + let mut request = Request::builder() + .uri("/api/v1/health") + .header("X-Real-IP", "203.0.113.5") + .header("X-Forwarded-For", "198.51.100.99, 10.0.0.1") + .body(Body::empty()) + .unwrap(); + request + .extensions_mut() + .insert(ConnectInfo(SocketAddr::from(([10, 0, 0, 1], 12345)))); + + let ip = client_ip(&request, &[proxy]); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1))); + } }