From 2b33af6e48ebd5f308e34a344e14bc3c9c63ee82 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Wed, 8 Jul 2026 17:25:10 +0800 Subject: [PATCH] fix(trusted-proxies): validate full Forwarded chain and bare IPv6 (#4461) --- crates/trusted-proxies/src/config/loader.rs | 42 +++++++ crates/trusted-proxies/src/proxy/validator.rs | 106 +++++++++++------- .../tests/unit/validator_tests.rs | 87 ++++++++++++++ 3 files changed, 192 insertions(+), 43 deletions(-) diff --git a/crates/trusted-proxies/src/config/loader.rs b/crates/trusted-proxies/src/config/loader.rs index 21cc97e02..d26d532bf 100644 --- a/crates/trusted-proxies/src/config/loader.rs +++ b/crates/trusted-proxies/src/config/loader.rs @@ -103,6 +103,12 @@ impl ConfigLoader { let private_networks = parse_ip_list_from_env(ENV_TRUSTED_PROXY_PRIVATE_NETWORKS, DEFAULT_TRUSTED_PROXY_PRIVATE_NETWORKS)?; + // Warn once at startup if the effective trusted set spans private/RFC1918 + // (or IPv6 unique-local) ranges. This trusts every host inside those + // ranges to set forwarding headers, which is broad; operators should + // narrow it to the specific proxy addresses in front of this server. + Self::warn_on_private_trusted_networks(&proxies); + Ok(TrustedProxyConfig::new( proxies, validation_mode, @@ -113,6 +119,42 @@ impl ConfigLoader { )) } + /// Emits a one-time startup warning when the trusted proxy set includes + /// private/RFC1918 (or IPv6 unique-local) ranges. + fn warn_on_private_trusted_networks(proxies: &[TrustedProxy]) { + let private_trusted: Vec = proxies + .iter() + .filter(|proxy| Self::is_private_trusted_entry(proxy)) + .map(|proxy| proxy.to_string()) + .collect(); + + if !private_trusted.is_empty() { + tracing::warn!( + event = "trusted_proxies.config", + component = "trusted_proxies", + subsystem = "config_loader", + result = "private_trust_warning", + private_networks = ?private_trusted, + "trusted proxy set includes private ranges; narrow RUSTFS_TRUSTED_PROXY_NETWORKS to the specific proxy addresses in front of this server to reduce client IP spoofing risk" + ); + } + } + + /// Returns true when a trusted proxy entry falls within a private/RFC1918 + /// IPv4 range or an IPv6 unique-local (fc00::/7) range. Loopback and public + /// addresses are not flagged. + fn is_private_trusted_entry(proxy: &TrustedProxy) -> bool { + let ip = match proxy { + TrustedProxy::Single(ip) => *ip, + TrustedProxy::Cidr(network) => network.network(), + }; + + match ip { + IpAddr::V4(v4) => v4.is_private(), + IpAddr::V6(v6) => (v6.segments()[0] & 0xfe00) == 0xfc00, + } + } + /// Loads cache configuration from environment variables. fn load_cache_config() -> CacheConfig { CacheConfig { diff --git a/crates/trusted-proxies/src/proxy/validator.rs b/crates/trusted-proxies/src/proxy/validator.rs index 5a37d1a9f..5269d56ae 100644 --- a/crates/trusted-proxies/src/proxy/validator.rs +++ b/crates/trusted-proxies/src/proxy/validator.rs @@ -333,45 +333,52 @@ impl ProxyValidator { } /// Parses the RFC 7239 "Forwarded" header value. + /// + /// Every comma-separated element is parsed in wire order (client-closest + /// first, each proxy appends its node to the right) so that the resulting + /// `proxy_chain` can be validated by the same right-to-left chain analysis + /// used for `X-Forwarded-For`. This prevents a client from spoofing the + /// real IP by supplying only the leftmost element. + /// + /// `host`/`proto` are taken solely from the last (most recent) element, + /// which is appended by the directly connected trusted proxy — mirroring + /// how the legacy path reads proxy-set `X-Forwarded-Host`/`Proto` headers. + /// Values injected by the client in earlier elements are ignored. fn parse_forwarded_header(header_value: &str, proxy_ip: IpAddr) -> Option { - // Simplified implementation: processes only the first entry in the header. - let first_part = header_value.split(',').next()?.trim(); + let elements: Vec<&str> = header_value + .split(',') + .map(|element| element.trim()) + .filter(|element| !element.is_empty()) + .collect(); let mut proxy_chain = Vec::new(); let mut forwarded_host = None; let mut forwarded_proto = None; - for part in first_part.split(';') { - let part = part.trim(); - if let Some((key, value)) = part.split_once('=') { - let key = key.trim().to_lowercase(); - let value = value.trim().trim_matches('"'); + let last_index = elements.len().saturating_sub(1); + for (index, element) in elements.iter().enumerate() { + let is_last = index == last_index; + for part in element.split(';') { + let part = part.trim(); + if let Some((key, value)) = part.split_once('=') { + let key = key.trim().to_lowercase(); + let value = value.trim().trim_matches('"'); - match key.as_str() { - "for" => { - // Extract IP address, handling IPv6 addresses in brackets as per RFC 7239. - let ip_str = if value.starts_with('[') { - if let Some(end) = value.find(']') { - &value[1..end] - } else { - continue; // Invalid format, skip + match key.as_str() { + "for" => { + if let Some(ip) = Self::parse_forwarded_node(value) { + proxy_chain.push(ip); } - } else { - // For IPv4 or IPv6 without brackets, take the part before the first colon. - value.split(':').next().unwrap_or(value) - }; - - if let Ok(ip) = ip_str.parse::() { - proxy_chain.push(ip); } + // Only honor host/proto from the trusted proxy-appended node. + "host" if is_last => { + forwarded_host = Some(value.to_string()); + } + "proto" if is_last => { + forwarded_proto = Some(value.to_string()); + } + _ => {} } - "host" => { - forwarded_host = Some(value.to_string()); - } - "proto" => { - forwarded_proto = Some(value.to_string()); - } - _ => {} } } } @@ -394,23 +401,36 @@ impl ProxyValidator { .split(',') .map(|s| s.trim()) .filter(|s| !s.is_empty()) - .filter_map(|s| { - // Handle IPv6 addresses in brackets, e.g., [::1]:8080 - let ip_str = if s.starts_with('[') { - if let Some(end) = s.find(']') { - &s[1..end] - } else { - s // Invalid format, try parsing as is - } - } else { - // For IPv4 or IPv6 without brackets, take the part before the first colon. - s.split(':').next().unwrap_or(s) - }; - ip_str.parse::().ok() - }) + .filter_map(Self::parse_forwarded_node) .collect() } + /// Parses a single forwarded node token into an `IpAddr`. + /// + /// Handles bracketed IPv6 (`[2001:db8::1]` and `[2001:db8::1]:443`), + /// IPv4 with an optional `:port`, and BARE IPv6 addresses. A trailing + /// `:port` is only stripped when the token is not itself a valid bare + /// address, so `2001:db8::1` is no longer truncated to `2001`. + fn parse_forwarded_node(token: &str) -> Option { + let token = token.trim(); + if token.is_empty() { + return None; + } + + let ip_str = if let Some(rest) = token.strip_prefix('[') { + // Bracketed IPv6, optionally followed by ":port". + rest.split(']').next()? + } else if token.parse::().is_ok() { + // Bare address (IPv4 or IPv6) with no port; parse the whole token. + token + } else { + // Not a bare address: strip a trailing ":port" (IPv4 host:port). + token.rsplit_once(':').map(|(host, _)| host).unwrap_or(token) + }; + + ip_str.parse::().ok() + } + /// Records the start of a validation attempt in metrics. fn record_metric_start(&self) { if let Some(metrics) = &self.metrics { diff --git a/crates/trusted-proxies/tests/unit/validator_tests.rs b/crates/trusted-proxies/tests/unit/validator_tests.rs index c9c4c8693..b95d2c421 100644 --- a/crates/trusted-proxies/tests/unit/validator_tests.rs +++ b/crates/trusted-proxies/tests/unit/validator_tests.rs @@ -41,6 +41,93 @@ fn test_parse_x_forwarded_for() { assert_eq!(result.len(), 2); } +// A client-supplied leftmost `Forwarded` element must not be treated as the +// real IP: the proxy-appended node further right must win via chain analysis. +#[test] +fn test_forwarded_resolves_proxy_appended_client_not_spoofed() { + let validator = ProxyValidator::new(create_test_config(), None); + let peer_addr = Some(SocketAddr::new(IpAddr::from_str("10.0.1.5").unwrap(), 1234)); + + let mut headers = HeaderMap::new(); + headers.insert("forwarded", "for=1.2.3.4, for=198.51.100.7".parse().unwrap()); + + let info = validator.validate_request(peer_addr, &headers).unwrap(); + assert!(info.is_from_trusted_proxy); + assert_eq!(info.real_ip, IpAddr::from_str("198.51.100.7").unwrap()); + assert_ne!(info.real_ip, IpAddr::from_str("1.2.3.4").unwrap()); +} + +// A multi-element `Forwarded` header whose rightmost node is trusted validates, +// resolving the client to the first untrusted (leftmost) element. +#[test] +fn test_forwarded_multi_element_trusted_rightmost_validates() { + let validator = ProxyValidator::new(create_test_config(), None); + let peer_addr = Some(SocketAddr::new(IpAddr::from_str("192.168.1.100").unwrap(), 1234)); + + let mut headers = HeaderMap::new(); + headers.insert("forwarded", "for=203.0.113.9, for=10.0.0.8".parse().unwrap()); + + let info = validator.validate_request(peer_addr, &headers).unwrap(); + assert!(info.is_from_trusted_proxy); + assert_eq!(info.real_ip, IpAddr::from_str("203.0.113.9").unwrap()); + assert_eq!(info.proxy_hops, 2); +} + +// A `proto=https` injected by the client in an earlier element must not set the +// forwarded protocol; only the trusted proxy-appended node is authoritative. +#[test] +fn test_forwarded_client_proto_injection_ignored() { + let validator = ProxyValidator::new(create_test_config(), None); + let peer_addr = Some(SocketAddr::new(IpAddr::from_str("10.0.1.5").unwrap(), 1234)); + + let mut headers = HeaderMap::new(); + headers.insert("forwarded", "for=1.2.3.4;proto=https, for=198.51.100.7".parse().unwrap()); + + let info = validator.validate_request(peer_addr, &headers).unwrap(); + assert_eq!(info.real_ip, IpAddr::from_str("198.51.100.7").unwrap()); + assert_eq!(info.forwarded_proto, None); +} + +// Bare IPv6, bracketed IPv6 with a port, and IPv4 with a port must all parse. +#[test] +fn test_parse_x_forwarded_for_ipv6_and_ports() { + assert_eq!( + ProxyValidator::parse_x_forwarded_for("2001:db8::1"), + vec![IpAddr::from_str("2001:db8::1").unwrap()] + ); + assert_eq!( + ProxyValidator::parse_x_forwarded_for("[2001:db8::1]:443"), + vec![IpAddr::from_str("2001:db8::1").unwrap()] + ); + assert_eq!( + ProxyValidator::parse_x_forwarded_for("203.0.113.1:8080"), + vec![IpAddr::from_str("203.0.113.1").unwrap()] + ); +} + +// An IPv6 client behind a trusted IPv6 proxy must resolve to the client, not +// collapse to the proxy because of bare-IPv6 truncation. +#[test] +fn test_ipv6_client_behind_trusted_proxy_resolves_client() { + let config = TrustedProxyConfig::new( + vec![TrustedProxy::Cidr("fd00::/8".parse().unwrap())], + ValidationMode::HopByHop, + true, + 5, + true, + vec![], + ); + let validator = ProxyValidator::new(config, None); + let peer_addr = Some(SocketAddr::new(IpAddr::from_str("fd00::5").unwrap(), 1234)); + + let mut headers = HeaderMap::new(); + headers.insert("x-forwarded-for", "2001:db8::1234".parse().unwrap()); + + let info = validator.validate_request(peer_addr, &headers).unwrap(); + assert!(info.is_from_trusted_proxy); + assert_eq!(info.real_ip, IpAddr::from_str("2001:db8::1234").unwrap()); +} + #[test] fn test_proxy_chain_analyzer_hop_by_hop() { let config = create_test_config();