fix(security): stop trusting client-supplied forwarded headers in rate limiting

extract_client_ip took X-Real-IP/X-Forwarded-For from any request, so
a client talking to the backend directly (the FIPS peer listener, or
any non-proxy path) could rotate a fake IP per request and never trip
the login rate limiter. The accept loop now records the TCP peer
address in request extensions, and forwarded headers are honored only
when the connection itself is from loopback — where nginx overwrites
X-Real-IP with the real client address. Direct connections bucket
under their socket IP.

§C of the 1.8.0 hardening plan; 3 new unit tests cover the
loopback/direct/no-header matrix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
archipelago
2026-07-04 15:48:07 -04:00
co-authored by Claude Fable 5
parent bd7edb4376
commit 9020b8526c
4 changed files with 106 additions and 12 deletions
+81 -3
View File
@@ -179,13 +179,91 @@ pub(super) fn extract_cookie(headers: &hyper::HeaderMap, name: &str) -> Option<S
None
}
/// Extract the client IP from request headers (X-Real-IP or X-Forwarded-For).
pub(super) fn extract_client_ip(headers: &hyper::HeaderMap) -> IpAddr {
/// The TCP peer address of the connection a request arrived on, injected
/// into request extensions by the server accept loop.
#[derive(Debug, Clone, Copy)]
pub struct PeerAddr(pub std::net::SocketAddr);
/// Extract the client IP for rate limiting.
///
/// `X-Real-IP`/`X-Forwarded-For` are only honored when the connection
/// itself comes from loopback — i.e. from our local nginx, which sets
/// `X-Real-IP $remote_addr`. On a direct connection (the FIPS peer
/// listener, or anything that isn't the local proxy) the headers are
/// client-supplied, so trusting them let an attacker rotate per-request
/// "IPs" and defeat the login rate limiter; there we use the socket
/// address instead.
pub(super) fn extract_client_ip(parts: &hyper::http::request::Parts) -> IpAddr {
let socket_ip = parts.extensions.get::<PeerAddr>().map(|p| p.0.ip());
match socket_ip {
Some(ip) if ip.is_loopback() => forwarded_client_ip(&parts.headers).unwrap_or(ip),
Some(ip) => ip,
// No socket info recorded (shouldn't happen in the server path);
// fall back to the pre-extension behavior.
None => forwarded_client_ip(&parts.headers)
.unwrap_or(IpAddr::V4(std::net::Ipv4Addr::LOCALHOST)),
}
}
/// The proxy-reported client IP, if a forwarded header carries one.
fn forwarded_client_ip(headers: &hyper::HeaderMap) -> Option<IpAddr> {
headers
.get("x-real-ip")
.or_else(|| headers.get("x-forwarded-for"))
.and_then(|v| v.to_str().ok())
.and_then(|s| s.split(',').next())
.and_then(|s| s.trim().parse::<IpAddr>().ok())
.unwrap_or(IpAddr::V4(std::net::Ipv4Addr::LOCALHOST))
}
#[cfg(test)]
mod client_ip_tests {
use super::*;
use std::net::SocketAddr;
fn parts_with(
peer: Option<&str>,
real_ip: Option<&str>,
) -> hyper::http::request::Parts {
let mut builder = hyper::Request::builder().uri("/rpc/v1");
if let Some(ip) = real_ip {
builder = builder.header("x-real-ip", ip);
}
let (mut parts, _) = builder.body(()).unwrap().into_parts();
if let Some(addr) = peer {
parts
.extensions
.insert(PeerAddr(addr.parse::<SocketAddr>().unwrap()));
}
parts
}
#[test]
fn loopback_connection_trusts_forwarded_header() {
// nginx on loopback forwards the real client IP — use it.
let parts = parts_with(Some("127.0.0.1:44412"), Some("192.168.1.50"));
assert_eq!(
extract_client_ip(&parts),
"192.168.1.50".parse::<IpAddr>().unwrap()
);
}
#[test]
fn direct_connection_ignores_spoofed_header() {
// A direct (non-proxy) client rotating X-Real-IP per request must
// still bucket under its socket address.
let parts = parts_with(Some("203.0.113.9:9999"), Some("10.0.0.1"));
assert_eq!(
extract_client_ip(&parts),
"203.0.113.9".parse::<IpAddr>().unwrap()
);
}
#[test]
fn loopback_connection_without_header_uses_socket_ip() {
let parts = parts_with(Some("127.0.0.1:5000"), None);
assert_eq!(
extract_client_ip(&parts),
"127.0.0.1".parse::<IpAddr>().unwrap()
);
}
}
+4 -3
View File
@@ -58,6 +58,7 @@ use middleware::{
derive_csrf_token, extract_client_ip, extract_cookie, sanitize_error_message,
CACHEABLE_METHODS, UNAUTHENTICATED_METHODS,
};
pub use middleware::PeerAddr;
use response::{cookie_header, json_response, ResponseCache, RpcError, RpcRequest, RpcResponse};
/// Default dev password when no user is set up (matches mock-backend).
@@ -369,7 +370,7 @@ impl RpcHandler {
// Rate limit login attempts
if rpc_req.method == "auth.login" {
let client_ip = extract_client_ip(&parts.headers);
let client_ip = extract_client_ip(&parts);
if !self.login_rate_limiter.check(client_ip).await {
return Ok(self.rate_limit_response());
}
@@ -377,7 +378,7 @@ impl RpcHandler {
// Rate limit sensitive endpoints
{
let client_ip = extract_client_ip(&parts.headers);
let client_ip = extract_client_ip(&parts);
if !self
.endpoint_rate_limiter
.check(&rpc_req.method, client_ip)
@@ -451,7 +452,7 @@ impl RpcHandler {
let mut response = json_response(StatusCode::OK, &resp_body);
// Post-dispatch: set cookies for auth-related methods
let client_ip = extract_client_ip(&parts.headers);
let client_ip = extract_client_ip(&parts);
self.apply_auth_cookies(
&rpc_req.method,
&mut rpc_resp,
+5 -1
View File
@@ -1034,9 +1034,13 @@ async fn accept_loop(
let permit = active_connections.clone().acquire_owned().await;
tokio::spawn(async move {
let _permit = permit;
let service = service_fn(move |req: hyper::Request<hyper::Body>| {
let service = service_fn(move |mut req: hyper::Request<hyper::Body>| {
let handler = handler.clone();
async move {
// Record the TCP peer so rate limiting only trusts
// forwarded headers on loopback (nginx) connections.
req.extensions_mut()
.insert(crate::api::rpc::PeerAddr(peer_addr));
if peer_only && !is_peer_allowed_path(req.uri().path()) {
let resp = hyper::Response::builder()
.status(hyper::StatusCode::NOT_FOUND)