fix(middleware): jangan percaya header X-Forwarded-For/X-Real-IP secara default di rate limiter

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

crates/zesdex-middleware/src/rate_limit.rs

@@ -29,13 +29,26 @@ use tower::{Layer, Service};
 #[derive(Debug)]
 pub struct RateLimiter {
     windows: Mutex<HashMap<String, Vec<i64>>>,
+    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<S> Layer<S> 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<S> {
     inner: S,
     limiter: std::sync::Arc<RateLimiter>,
     max_requests: u32,
     window_secs: u64,
+    trust_proxy_headers: bool,
 }
 
 impl<S, ReqBody> Service<Request<ReqBody>> for RateLimitMiddleware<S>
@@ -190,21 +261,9 @@ where
 
     fn call(&mut self, req: Request<ReqBody>) -> 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::<axum::extract::ConnectInfo<std::net::SocketAddr>>()
-                    .map(|ci| ci.0.ip().to_string())
-            })
+            .extensions()
+            .get::<axum::extract::ConnectInfo<std::net::SocketAddr>>()
+            .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");
+    }
 }