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<SocketAddr> (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 <noreply@anthropic.com>
This commit is contained in:
@@ -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::<std::net::SocketAddr>(),
|
||||
)
|
||||
.with_graceful_shutdown(shutdown_signal())
|
||||
.await?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -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::<SocketAddr>()`.
|
||||
///
|
||||
/// 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::<ConnectInfo<SocketAddr>>() {
|
||||
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();
|
||||
|
||||
Reference in New Issue
Block a user