From 9495cd39ce3ccbb44c53aba0be6724191dc4d4a6 Mon Sep 17 00:00:00 2001 From: jgrusewski Date: Mon, 23 Feb 2026 23:25:05 +0100 Subject: [PATCH] fix(web-gateway): use TCP peer address for rate limiting instead of X-Forwarded-For The rate limiter trusted the client-controlled X-Forwarded-For header to identify clients. An attacker could rotate this header value on every request to bypass rate limits entirely. Now uses ConnectInfo (the actual TCP connection address) as the rate limiting key. Server is configured with into_make_service_with_connect_info to populate this. X-Forwarded-For and X-Real-Ip headers are no longer consulted. Falls back to "unknown" if ConnectInfo is unavailable (tests). Co-Authored-By: Claude Opus 4.6 --- web-gateway/src/main.rs | 9 ++++-- web-gateway/src/rate_limit.rs | 57 ++++++++++++++++++++--------------- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/web-gateway/src/main.rs b/web-gateway/src/main.rs index 3d7b1cb3a..53c206efa 100644 --- a/web-gateway/src/main.rs +++ b/web-gateway/src/main.rs @@ -67,9 +67,12 @@ async fn main() -> Result<()> { let listener = tokio::net::TcpListener::bind(&listen_addr).await?; info!("Web gateway listening on {}", listen_addr); - axum::serve(listener, app) - .with_graceful_shutdown(shutdown_signal()) - .await?; + axum::serve( + listener, + app.into_make_service_with_connect_info::(), + ) + .with_graceful_shutdown(shutdown_signal()) + .await?; Ok(()) } diff --git a/web-gateway/src/rate_limit.rs b/web-gateway/src/rate_limit.rs index be55d854c..eeb5ef099 100644 --- a/web-gateway/src/rate_limit.rs +++ b/web-gateway/src/rate_limit.rs @@ -1,8 +1,9 @@ use std::collections::HashMap; +use std::net::SocketAddr; use std::sync::Arc; use std::time::{Duration, Instant}; -use axum::extract::Request; +use axum::extract::{ConnectInfo, Request}; use axum::http::StatusCode; use axum::middleware::Next; use axum::response::{IntoResponse, Response}; @@ -63,23 +64,24 @@ impl RateLimiter { } } -/// Extract client IP from request headers (X-Forwarded-For, X-Real-Ip) -/// or fall back to "unknown". +/// Extract client IP from the TCP connection (ConnectInfo), NOT from +/// client-controlled headers like X-Forwarded-For. +/// +/// X-Forwarded-For is trivially spoofable by any client. We use the actual +/// TCP peer address from ConnectInfo instead, which requires the server to +/// be started with `into_make_service_with_connect_info::()`. +/// +/// Falls back to "unknown" if ConnectInfo is not available (e.g. in tests). fn client_ip(request: &Request) -> String { - request - .headers() - .get("x-forwarded-for") - .and_then(|v| v.to_str().ok()) - .and_then(|s| s.split(',').next()) - .map(|s| s.trim().to_owned()) - .or_else(|| { - request - .headers() - .get("x-real-ip") - .and_then(|v| v.to_str().ok()) - .map(|s| s.to_owned()) - }) - .unwrap_or_else(|| "unknown".to_owned()) + // Primary: use the actual TCP connection address (not spoofable) + if let Some(connect_info) = request.extensions().get::>() { + return connect_info.0.ip().to_string(); + } + + // Fallback: if ConnectInfo is not available (server not configured with + // into_make_service_with_connect_info), use "unknown" rather than + // trusting client-controlled headers. + "unknown".to_owned() } /// Axum middleware that enforces rate limiting using a shared [`RateLimiter`]. @@ -161,25 +163,30 @@ mod tests { } #[test] - fn test_client_ip_from_forwarded_for() { - let req = Request::builder() - .header("x-forwarded-for", "1.2.3.4, 5.6.7.8") + fn test_client_ip_from_connect_info() { + let addr: SocketAddr = "192.168.1.42:12345".parse().unwrap(); + let mut req = Request::builder() .body(axum::body::Body::empty()) .unwrap(); - assert_eq!(client_ip(&req), "1.2.3.4"); + req.extensions_mut().insert(ConnectInfo(addr)); + assert_eq!(client_ip(&req), "192.168.1.42"); } #[test] - fn test_client_ip_from_real_ip() { - let req = Request::builder() - .header("x-real-ip", "10.0.0.1") + fn test_client_ip_ignores_xff_header() { + // Even with X-Forwarded-For set, we should use ConnectInfo + let addr: SocketAddr = "10.0.0.1:9999".parse().unwrap(); + let mut req = Request::builder() + .header("x-forwarded-for", "1.2.3.4, 5.6.7.8") .body(axum::body::Body::empty()) .unwrap(); + req.extensions_mut().insert(ConnectInfo(addr)); + // Should return the TCP peer, NOT the spoofed XFF assert_eq!(client_ip(&req), "10.0.0.1"); } #[test] - fn test_client_ip_fallback() { + fn test_client_ip_fallback_without_connect_info() { let req = Request::builder() .body(axum::body::Body::empty()) .unwrap();