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