From 4dc4f80fa344d894c2ce62f75d239847cd13001a Mon Sep 17 00:00:00 2001 From: asepharyana Date: Thu, 16 Jul 2026 15:47:56 +0700 Subject: [PATCH] fix(middleware): jangan percaya header X-Forwarded-For/X-Real-IP secara default di rate limiter Co-Authored-By: Claude Sonnet 5 --- crates/zesdex-middleware/src/rate_limit.rs | 134 +++++++++++++++++---- 1 file changed, 113 insertions(+), 21 deletions(-) diff --git a/crates/zesdex-middleware/src/rate_limit.rs b/crates/zesdex-middleware/src/rate_limit.rs index 3228376..5a9d863 100644 --- a/crates/zesdex-middleware/src/rate_limit.rs +++ b/crates/zesdex-middleware/src/rate_limit.rs @@ -29,13 +29,26 @@ use tower::{Layer, Service}; #[derive(Debug)] pub struct RateLimiter { windows: Mutex>>, + trust_proxy_headers: bool, } impl RateLimiter { - /// Create an empty rate limiter. + /// Create a rate limiter that keys strictly on the real connection + /// socket address (default, safe when not behind a trusted proxy). pub fn new() -> Self { + Self::with_proxy_trust(false) + } + + /// Create a rate limiter with an explicit proxy-header trust policy. + /// + /// When `trust_proxy_headers` is `true`, the `X-Forwarded-For` and + /// `X-Real-IP` headers are used to derive the client bucket key. + /// This must only be enabled when the middleware sits behind a + /// reverse proxy known to overwrite (not merge) these headers. + pub fn with_proxy_trust(trust_proxy_headers: bool) -> Self { Self { windows: Mutex::new(HashMap::new()), + trust_proxy_headers, } } @@ -129,13 +142,31 @@ pub struct RateLimitLayer { } impl RateLimitLayer { - /// Create a new layer with the given limits. + /// Create a new layer with the given limits, keying strictly on the + /// real connection socket address (default, safe when not behind a + /// trusted proxy). /// /// * `max_requests` — max requests per window per client. /// * `window_secs` — sliding-window width in seconds. pub fn new(max_requests: u32, window_secs: u64) -> Self { + Self::with_proxy_trust(max_requests, window_secs, false) + } + + /// Create a new layer with an explicit proxy-header trust policy. + /// + /// When `trust_proxy_headers` is `true`, the `X-Forwarded-For` and + /// `X-Real-IP` headers are used to derive the client bucket key. + /// This must only be enabled when the middleware sits behind a + /// reverse proxy known to overwrite (not merge) these headers. + pub fn with_proxy_trust( + max_requests: u32, + window_secs: u64, + trust_proxy_headers: bool, + ) -> Self { Self { - limiter: std::sync::Arc::new(RateLimiter::new()), + limiter: std::sync::Arc::new(RateLimiter::with_proxy_trust( + trust_proxy_headers, + )), max_requests, window_secs, } @@ -157,20 +188,60 @@ impl Layer for RateLimitLayer { limiter: std::sync::Arc::clone(&self.limiter), max_requests: self.max_requests, window_secs: self.window_secs, + trust_proxy_headers: self.limiter.trust_proxy_headers, } } } +/// Derive the per-client rate-limit bucket key for a request. +/// +/// Flow: if `trust_proxy_headers` is true, use `X-Forwarded-For` (first +/// hop) then `X-Real-IP`; otherwise always use the real connection +/// socket address, ignoring any client-supplied headers. +/// +/// Why: without a trusted reverse proxy stripping/overwriting these +/// headers, they are attacker-controlled — trusting them by default lets +/// any direct caller reset their own rate-limit bucket on every request. +/// `trust_proxy_headers` must only be set to `true` when this middleware +/// sits behind a proxy that is known to overwrite (not merge) these headers. +fn client_id( + headers: &axum::http::HeaderMap, + socket_addr: std::net::SocketAddr, + trust_proxy_headers: bool, +) -> String { + if trust_proxy_headers { + if let Some(fwd) = headers + .get("x-forwarded-for") + .and_then(|v| v.to_str().ok()) + .and_then(|v| v.split(',').next()) + .map(str::trim) + { + if !fwd.is_empty() { + return fwd.to_string(); + } + } + if let Some(real_ip) = headers.get("x-real-ip").and_then(|v| v.to_str().ok()) { + if !real_ip.is_empty() { + return real_ip.to_string(); + } + } + } + socket_addr.ip().to_string() +} + /// Tower [`Service`] wrapping each request with a rate-limit check. /// -/// Client identity is extracted from the `X-Forwarded-For` header first, -/// falling back to the remote address, then to `"unknown"`. +/// Client identity is extracted from the real connection socket address +/// by default (safe). When `trust_proxy_headers` is `true`, +/// `X-Forwarded-For`/`X-Real-IP` headers are also considered — only +/// enable this behind a trusted reverse proxy. #[derive(Debug, Clone)] pub struct RateLimitMiddleware { inner: S, limiter: std::sync::Arc, max_requests: u32, window_secs: u64, + trust_proxy_headers: bool, } impl Service> for RateLimitMiddleware @@ -190,21 +261,9 @@ where fn call(&mut self, req: Request) -> Self::Future { let client_id = req - .headers() - .get("X-Forwarded-For") - .and_then(|v| v.to_str().ok()) - .map(|s| s.split(',').next().unwrap_or(s).trim().to_string()) - .or_else(|| { - req.headers() - .get("X-Real-IP") - .and_then(|v| v.to_str().ok()) - .map(|s| s.to_string()) - }) - .or_else(|| { - req.extensions() - .get::>() - .map(|ci| ci.0.ip().to_string()) - }) + .extensions() + .get::>() + .map(|ci| client_id(req.headers(), ci.0, self.trust_proxy_headers)) .unwrap_or_else(|| "unknown".to_string()); let limiter = std::sync::Arc::clone(&self.limiter); @@ -219,7 +278,7 @@ where } let fut = self.inner.call(req); - Box::pin(async move { fut.await }) + Box::pin(fut) } } @@ -265,4 +324,37 @@ mod tests { limiter.reset().unwrap(); assert!(limiter.check_rate_limit("client-3", 1, 60).unwrap()); } + + #[test] + fn client_id_ignores_spoofed_forwarded_headers_by_default() { + // A request carrying a spoofed X-Forwarded-For must NOT be treated + // as a distinct client from one with a different spoofed value — + // both should resolve to the same real socket address. + let socket_addr: std::net::SocketAddr = "127.0.0.1:9999".parse().unwrap(); + let mut headers_a = axum::http::HeaderMap::new(); + headers_a.insert("x-forwarded-for", "1.2.3.4".parse().unwrap()); + let mut headers_b = axum::http::HeaderMap::new(); + headers_b.insert("x-forwarded-for", "5.6.7.8".parse().unwrap()); + + let id_a = client_id(&headers_a, socket_addr, false); + let id_b = client_id(&headers_b, socket_addr, false); + + assert_eq!( + id_a, id_b, + "client_id must key on the real socket address when trust_proxy_headers is false, \ + not on attacker-controlled X-Forwarded-For" + ); + } + + #[test] + fn client_id_uses_forwarded_header_when_trust_enabled() { + // When explicitly told to trust a fronting proxy, the header value + // should be used (this is the opt-in, documented-risk path). + let socket_addr: std::net::SocketAddr = "127.0.0.1:9999".parse().unwrap(); + let mut headers = axum::http::HeaderMap::new(); + headers.insert("x-forwarded-for", "1.2.3.4".parse().unwrap()); + + let id = client_id(&headers, socket_addr, true); + assert_eq!(id, "1.2.3.4"); + } }