fix(client-address): read the peer a tonic server records

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,
 }
 
@@ -195,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 {
@@ -203,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",
         })
     }
 }
@@ -378,9 +382,11 @@ 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),
+            Ok(None) if self.required => Resolution::Unavailable,
             Ok(None) => Resolution::Peer(peer_ip),
             Err(invalid) => Resolution::Invalid(invalid),
         };
@@ -609,6 +615,26 @@ 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<Option<IpAddr>, 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 = !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)
+}
+
 /// 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.
@@ -666,8 +692,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<SocketAddr>` 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 +768,16 @@ where
         let peer = request
             .extensions()
             .get::<ConnectInfo<SocketAddr>>()
-            .map(|ConnectInfo(peer)| *peer);
+            .map(|ConnectInfo(peer)| *peer)
+            .or_else(|| {
+                use crate::service::TlsConnectInfo;
+                use tonic::transport::server::TcpConnectInfo;
+                let extensions = request.extensions();
+                extensions
+                    .get::<TcpConnectInfo>()
+                    .or_else(|| extensions.get::<TlsConnectInfo>().map(|tls| tls.get_ref()))
+                    .and_then(TcpConnectInfo::remote_addr)
+            });
         self.resolver.apply(&mut request, peer);
         self.inner.call(request)
     }

src/client_address/tests.rs

@@ -796,3 +796,113 @@ fn a_trusted_proxy_is_a_range_or_one_address() {
         assert!(err.contains("trusted_proxies"), "{bad:?}: {err}");
     }
 }
+
+/// 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(&[], &[], &["for=198.51.100.9"]), 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.
+#[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.
+#[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::<ClientAddress>().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"));
+}

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.

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::<ClientAddress>().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");