From 6abd6f02dabe818c44fd5fde97d07941ed265767 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 30 Oct 2026 18:42:04 +0200 Subject: [PATCH 1/6] fix(client-address): read the peer a tonic server records ClientAddressLayer read only axum's ConnectInfo, so a service mounted on a tonic server got an unresolved address even behind a trusted proxy. The layer now falls back to TcpConnectInfo. --- src/client_address.rs | 13 ++++++++++--- src/client_address/tests.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/client_address.rs b/src/client_address.rs index fafed73..a04adce 100644 --- a/src/client_address.rs +++ b/src/client_address.rs @@ -666,8 +666,9 @@ fn canonical_net(net: IpNet) -> IpNet { /// proxy parts such as the [`shield`](crate::shield) middleware directly, so /// they key by the client behind your trusted proxies rather than by the /// proxies' own address. The peer comes from the `ConnectInfo` an -/// axum server records (`into_make_service_with_connect_info`); without one -/// the address is [`Resolution::Unavailable`]. `required` is not enforced +/// axum server records (`into_make_service_with_connect_info`) or the +/// `TcpConnectInfo` a tonic server does; without one the address is +/// [`Resolution::Unavailable`]. `required` is not enforced /// here: read [`ClientAddress::resolution`] where it matters. /// /// # Examples @@ -741,7 +742,13 @@ where let peer = request .extensions() .get::>() - .map(|ConnectInfo(peer)| *peer); + .map(|ConnectInfo(peer)| *peer) + .or_else(|| { + request + .extensions() + .get::() + .and_then(tonic::transport::server::TcpConnectInfo::remote_addr) + }); self.resolver.apply(&mut request, peer); self.inner.call(request) } diff --git a/src/client_address/tests.rs b/src/client_address/tests.rs index a1bd0dc..ad77e50 100644 --- a/src/client_address/tests.rs +++ b/src/client_address/tests.rs @@ -796,3 +796,37 @@ fn a_trusted_proxy_is_a_range_or_one_address() { assert!(err.contains("trusted_proxies"), "{bad:?}: {err}"); } } + +/// A tonic server records its peer as `TcpConnectInfo`, not as axum's +/// `ConnectInfo`: the layer resolves from it, so a trusted proxy in front of a +/// tonic server is not taken for the client, nor the address left unresolved. +#[tokio::test] +async fn the_layer_reads_the_peer_a_tonic_server_records() { + use tonic::transport::server::TcpConnectInfo; + use tower::{service_fn, ServiceExt}; + + let layer = ClientAddressLayer::new(&ClientAddressConfig { + trusted_proxies: vec!["10.0.0.0/8".into()], + ..Default::default() + }) + .unwrap(); + let service = layer.layer(service_fn(|request: http::Request<()>| async move { + Ok::<_, std::convert::Infallible>(request.extensions().get::().cloned()) + })); + + let mut request = http::Request::new(()); + request + .headers_mut() + .insert(X_FORWARDED_FOR, HeaderValue::from_static("203.0.113.7")); + request.extensions_mut().insert(TcpConnectInfo { + local_addr: None, + remote_addr: peer("10.1.2.3:443"), + }); + + let client = service.oneshot(request).await.unwrap().unwrap(); + assert_eq!( + client.resolution(), + Resolution::Forwarded(ip("203.0.113.7")) + ); + assert_eq!(client.peer(), peer("10.1.2.3:443")); +} From 2d349e7987d330a8ebee0976a76043d62bcb1471 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 30 Oct 2026 19:06:19 +0200 Subject: [PATCH 2/6] fix(client-address): refuse a required address nobody reported A trusted proxy that forwarded nothing resolved to its own address, so a required address passed with the proxy standing in for the client. Where an address is required this now resolves nothing; an untrusted peer is still the client. --- src/client_address.rs | 6 ++++-- src/client_address/tests.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/client_address.rs b/src/client_address.rs index a04adce..60a9971 100644 --- a/src/client_address.rs +++ b/src/client_address.rs @@ -170,7 +170,7 @@ pub struct ClientAddress { #[non_exhaustive] pub enum Resolution { /// The connection's peer is the client: it is not a trusted proxy, or it - /// is one that forwarded no address. + /// is one that forwarded no address where none is required. Peer(IpAddr), /// A trusted proxy forwarded the address: the first one outside the /// trusted proxies, read right to left, or the leftmost when every hop is @@ -181,7 +181,8 @@ pub enum Resolution { /// not the client. Invalid(InvalidForwarding), /// The server recorded no connection information, so there is no peer to - /// start from. + /// start from, or a trusted proxy reported nothing where an address is + /// required. Unavailable, } @@ -381,6 +382,7 @@ impl Resolver { }; let resolution = match forwarded { Ok(Some(ip)) => Resolution::Forwarded(ip), + Ok(None) if self.required => Resolution::Unavailable, Ok(None) => Resolution::Peer(peer_ip), Err(invalid) => Resolution::Invalid(invalid), }; diff --git a/src/client_address/tests.rs b/src/client_address/tests.rs index ad77e50..9220084 100644 --- a/src/client_address/tests.rs +++ b/src/client_address/tests.rs @@ -797,6 +797,32 @@ fn a_trusted_proxy_is_a_range_or_one_address() { } } +/// Where an address is required, a trusted proxy that reports none has not +/// said who the client is: its own address never stands in, or everything +/// behind a sidecar would be the pod itself. +#[test] +fn a_trusted_peer_that_must_report_and_reports_nothing_resolves_nothing() { + let required = Resolver::build(&ClientAddressConfig { + trusted_proxies: vec!["10.0.0.0/8".into()], + required: true, + ..Default::default() + }) + .unwrap(); + assert_eq!( + required + .resolve(peer("10.1.2.3:443"), &HeaderMap::new()) + .resolution(), + Resolution::Unavailable + ); + // An untrusted peer still is the client: it is no proxy. + assert_eq!( + required + .resolve(peer("203.0.113.7:443"), &HeaderMap::new()) + .resolution(), + Resolution::Peer(ip("203.0.113.7")) + ); +} + /// A tonic server records its peer as `TcpConnectInfo`, not as axum's /// `ConnectInfo`: the layer resolves from it, so a trusted proxy in front of a /// tonic server is not taken for the client, nor the address left unresolved. From da87859962a84fa51125bf927b1bf5ca8ecae25c Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 30 Oct 2026 19:26:38 +0200 Subject: [PATCH 3/6] feat(client-address): read a cleaned report as one address The X-Real-IP profile reads that header alone, as it must behind nginx, so behind a proxy that already resolved the client a contradicting X-Forwarded-For passed and was then rewritten away. The cleaned_report profile takes one address and refuses a report whose X-Forwarded-For or Forwarded disagrees with it. --- src/client_address.rs | 23 +++++++++++++++++ src/client_address/tests.rs | 49 +++++++++++++++++++++++++++++++++++++ src/config.rs | 5 ++++ 3 files changed, 77 insertions(+) diff --git a/src/client_address.rs b/src/client_address.rs index 60a9971..6139c37 100644 --- a/src/client_address.rs +++ b/src/client_address.rs @@ -196,6 +196,8 @@ pub enum InvalidForwarding { TooManyHops, /// More than one `X-Real-IP` field. Repeated, + /// A cleaned report whose headers do not name one and the same address. + Conflicting, } impl std::fmt::Display for InvalidForwarding { @@ -204,6 +206,7 @@ impl std::fmt::Display for InvalidForwarding { Self::Malformed => "malformed", Self::TooManyHops => "beyond too many trusted hops", Self::Repeated => "repeated", + Self::Conflicting => "in conflict with itself", }) } } @@ -379,6 +382,7 @@ impl Resolver { self.walk(headers.get_all(X_FORWARDED_FOR).iter(), hops) } ForwardingHeader::XRealIp => real_ip(headers.get_all(X_REAL_IP).iter()), + ForwardingHeader::CleanedReport => cleaned_report(headers), }; let resolution = match forwarded { Ok(Some(ip)) => Resolution::Forwarded(ip), @@ -611,6 +615,25 @@ fn real_ip<'a>( .ok_or(InvalidForwarding::Malformed) } +/// The client's address in a cleaned report: its `X-Real-IP`, when every other +/// assertion of an address agrees with it. `None` when nothing is reported. +fn cleaned_report(headers: &HeaderMap) -> Result, InvalidForwarding> { + let reported = real_ip(headers.get_all(X_REAL_IP).iter())?; + let mut chain = headers.get_all(X_FORWARDED_FOR).iter(); + let (first, second) = (chain.next(), chain.next()); + let agrees = match (reported, first, second) { + (_, _, Some(_)) => false, + (None, None, None) => return Ok(None), + (None, Some(_), None) => false, + (Some(_), None, None) => true, + (Some(ip), Some(line), None) => real_ip(std::iter::once(line)).ok().flatten() == Some(ip), + }; + if !agrees || headers.contains_key(FORWARDED) { + return Err(InvalidForwarding::Conflicting); + } + Ok(reported) +} + /// An address as forwarding headers carry it: bare, IPv6 in brackets, or /// either with a port, which is dropped. Nothing else, so a name, `unknown` /// or an obfuscated identifier (RFC 7239 ยง6.3) never counts as an address. diff --git a/src/client_address/tests.rs b/src/client_address/tests.rs index 9220084..64c8de1 100644 --- a/src/client_address/tests.rs +++ b/src/client_address/tests.rs @@ -797,6 +797,55 @@ fn a_trusted_proxy_is_a_range_or_one_address() { } } +/// A cleaned report is one address, said once: a reporter whose other headers +/// disagree with it is refused before they are rewritten away. +#[test] +fn a_cleaned_report_must_agree_with_itself() { + let cleaned = resolver(&["10.0.0.0/8"], ForwardingHeader::CleanedReport); + let resolve = |real: &[&str], chain: &[&str], forwarded: &[&str]| { + let mut all = headers("x-real-ip", real); + all.extend(xff(chain)); + all.extend(headers("forwarded", forwarded)); + cleaned.resolve(peer("10.1.2.3:443"), &all).resolution() + }; + let client = Resolution::Forwarded(ip("203.0.113.7")); + let conflict = Resolution::Invalid(InvalidForwarding::Conflicting); + + assert_eq!(resolve(&["203.0.113.7"], &[], &[]), client); + assert_eq!(resolve(&["203.0.113.7"], &["203.0.113.7"], &[]), client); + assert_eq!(resolve(&["203.0.113.7"], &["198.51.100.9"], &[]), conflict); + assert_eq!( + resolve(&["203.0.113.7"], &["198.51.100.9, 203.0.113.7"], &[]), + conflict + ); + assert_eq!( + resolve(&["203.0.113.7"], &["203.0.113.7", "203.0.113.7"], &[]), + conflict + ); + assert_eq!( + resolve(&["203.0.113.7"], &[], &["for=198.51.100.9"]), + conflict + ); + assert_eq!(resolve(&[], &["203.0.113.7"], &[]), conflict); + assert_eq!( + resolve(&["203.0.113.7", "203.0.113.7"], &[], &[]), + Resolution::Invalid(InvalidForwarding::Repeated) + ); +} + +/// The generic `X-Real-IP` profile keeps a proxy's chain beside it: nginx sends +/// both, and they name different things. +#[test] +fn the_x_real_ip_profile_leaves_the_chain_alone() { + let plain = resolver(&["10.0.0.0/8"], ForwardingHeader::XRealIp); + let mut all = headers("x-real-ip", &["203.0.113.7"]); + all.extend(xff(&["198.51.100.9, 203.0.113.7"])); + assert_eq!( + plain.resolve(peer("10.1.2.3:443"), &all).resolution(), + Resolution::Forwarded(ip("203.0.113.7")) + ); +} + /// Where an address is required, a trusted proxy that reports none has not /// said who the client is: its own address never stands in, or everything /// behind a sidecar would be the pod itself. diff --git a/src/config.rs b/src/config.rs index eb40b6a..d63f9b1 100644 --- a/src/config.rs +++ b/src/config.rs @@ -286,6 +286,11 @@ pub enum ForwardingHeader { XForwardedFor, /// `X-Real-IP`: one address, the trusted proxy's own peer. XRealIp, + /// A cleaned report from a proxy that already resolved the client: + /// `X-Real-IP` with one address and, if present at all, an + /// `X-Forwarded-For` of that same one address, with no `Forwarded`. No + /// chain is walked; a report that disagrees with itself is invalid. + CleanedReport, } /// Which annotated RPCs are transcoded. From 251b2558af0e601486fa6fcbdfd81775560c1994 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 30 Oct 2026 19:27:12 +0200 Subject: [PATCH 4/6] fix(client-address): read the peer of a tonic TLS connection --- src/client_address.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/client_address.rs b/src/client_address.rs index 6139c37..eb6c669 100644 --- a/src/client_address.rs +++ b/src/client_address.rs @@ -769,10 +769,16 @@ where .get::>() .map(|ConnectInfo(peer)| *peer) .or_else(|| { - request - .extensions() - .get::() - .and_then(tonic::transport::server::TcpConnectInfo::remote_addr) + use tonic::transport::server::{TcpConnectInfo, TlsConnectInfo}; + let extensions = request.extensions(); + extensions + .get::() + .or_else(|| { + extensions + .get::>() + .map(TlsConnectInfo::get_ref) + }) + .and_then(TcpConnectInfo::remote_addr) }); self.resolver.apply(&mut request, peer); self.inner.call(request) From 7db3e452713de2bbb13ee899ad2bab794fed597d Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 30 Oct 2026 19:28:49 +0200 Subject: [PATCH 5/6] fix(client-address): name the TLS record and refuse a lone Forwarded --- src/client_address.rs | 26 ++++++++++++-------------- src/client_address/tests.rs | 1 + 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/client_address.rs b/src/client_address.rs index eb6c669..1e7dbad 100644 --- a/src/client_address.rs +++ b/src/client_address.rs @@ -621,14 +621,15 @@ fn cleaned_report(headers: &HeaderMap) -> Result, InvalidForwardi let reported = real_ip(headers.get_all(X_REAL_IP).iter())?; let mut chain = headers.get_all(X_FORWARDED_FOR).iter(); let (first, second) = (chain.next(), chain.next()); - let agrees = match (reported, first, second) { - (_, _, Some(_)) => false, - (None, None, None) => return Ok(None), - (None, Some(_), None) => false, - (Some(_), None, None) => true, - (Some(ip), Some(line), None) => real_ip(std::iter::once(line)).ok().flatten() == Some(ip), - }; - if !agrees || headers.contains_key(FORWARDED) { + let agrees = !headers.contains_key(FORWARDED) + && match (reported, first, second) { + (_, _, Some(_)) | (None, Some(_), None) => false, + (_, None, None) => true, + (Some(ip), Some(line), None) => { + real_ip(std::iter::once(line)).ok().flatten() == Some(ip) + } + }; + if !agrees { return Err(InvalidForwarding::Conflicting); } Ok(reported) @@ -769,15 +770,12 @@ where .get::>() .map(|ConnectInfo(peer)| *peer) .or_else(|| { - use tonic::transport::server::{TcpConnectInfo, TlsConnectInfo}; + use crate::service::TlsConnectInfo; + use tonic::transport::server::TcpConnectInfo; let extensions = request.extensions(); extensions .get::() - .or_else(|| { - extensions - .get::>() - .map(TlsConnectInfo::get_ref) - }) + .or_else(|| extensions.get::().map(|tls| tls.get_ref())) .and_then(TcpConnectInfo::remote_addr) }); self.resolver.apply(&mut request, peer); diff --git a/src/client_address/tests.rs b/src/client_address/tests.rs index 64c8de1..bbcac98 100644 --- a/src/client_address/tests.rs +++ b/src/client_address/tests.rs @@ -827,6 +827,7 @@ fn a_cleaned_report_must_agree_with_itself() { conflict ); assert_eq!(resolve(&[], &["203.0.113.7"], &[]), conflict); + assert_eq!(resolve(&[], &[], &["for=198.51.100.9"]), conflict); assert_eq!( resolve(&["203.0.113.7", "203.0.113.7"], &[], &[]), Resolution::Invalid(InvalidForwarding::Repeated) From a13bb86e0cf62455ae7ffa7415545d6b0f163c4a Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Fri, 30 Oct 2026 19:30:39 +0200 Subject: [PATCH 6/6] test(client-address): resolve through a completed TLS handshake --- tests/tls.rs | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/tls.rs b/tests/tls.rs index e5fe9b9..dcf6173 100644 --- a/tests/tls.rs +++ b/tests/tls.rs @@ -237,6 +237,54 @@ async fn listen() -> SocketAddr { addr } +/// `ClientAddressLayer` on a tonic server behind TLS: the record a completed +/// handshake gives names the peer, so a trusted proxy's report is read rather +/// than the address left unresolved. +#[tokio::test] +async fn the_client_address_layer_reads_the_peer_of_a_tls_connection() { + use structured_proxy::client_address::{ClientAddressLayer, Resolution}; + use structured_proxy::config::ClientAddressConfig; + use structured_proxy::ClientAddress; + use tower::{Layer, ServiceExt}; + + let config = rustls::ServerConfig::builder_with_provider(provider()) + .with_safe_default_protocol_versions() + .unwrap() + .with_no_client_auth() + .with_single_cert(chain(), key()) + .unwrap(); + let acceptor = tokio_rustls::TlsAcceptor::from(Arc::new(config)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let client = tokio::spawn(connect(addr, b"h2", Identity::Anonymous)); + let (tcp, _) = listener.accept().await.unwrap(); + let tls = acceptor.accept(tcp).await.unwrap(); + let client = client.await.unwrap(); + let client_addr = client.get_ref().0.local_addr().unwrap(); + + let info: TlsConnectInfo = tls.connect_info(); + let mut request = http::Request::new(()); + request.extensions_mut().insert(info); + request.headers_mut().insert( + "x-forwarded-for", + http::HeaderValue::from_static("203.0.113.7"), + ); + + let mut trust = ClientAddressConfig::default(); + trust.trusted_proxies = vec!["127.0.0.1".into()]; + let service = ClientAddressLayer::new(&trust) + .unwrap() + .layer(tower::service_fn(|request: http::Request<()>| async move { + Ok::<_, Infallible>(request.extensions().get::().cloned()) + })); + let resolved = service.oneshot(request).await.unwrap().unwrap(); + assert_eq!(resolved.peer(), Some(client_addr)); + assert_eq!( + resolved.resolution(), + Resolution::Forwarded("203.0.113.7".parse().unwrap()) + ); +} + /// The directory of the test PKI. const TESTDATA: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/src/tls/testdata");