diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index d7b3c49af..ae389e40a 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -77,6 +77,7 @@ mod interface; mod k8s_auth; mod listener; mod mapping; +mod oslo_middleware; mod policy; mod resource; mod revoke; @@ -108,6 +109,7 @@ pub use interface::*; pub use k8s_auth::*; pub use listener::*; pub use mapping::*; +pub use oslo_middleware::*; pub use policy::*; pub use resource::*; pub use revoke::*; @@ -202,6 +204,10 @@ pub struct Config { #[serde(default)] pub mapping: MappingProvider, + /// `[oslo_middleware]` configuration (proxy header parsing). + #[serde(default)] + pub oslo_middleware: OsloMiddleware, + /// Server listener configuration for the internal interface. #[serde(rename = "interface_internal", default)] pub interface_internal: Option, diff --git a/crates/config/src/oslo_middleware.rs b/crates/config/src/oslo_middleware.rs new file mode 100644 index 000000000..7db8c88bf --- /dev/null +++ b/crates/config/src/oslo_middleware.rs @@ -0,0 +1,94 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # `[oslo_middleware]` configuration +//! +//! Mirrors the subset of upstream Python Keystone's `[oslo_middleware]` +//! section that is relevant to client-address capture. + +use serde::Deserialize; + +use crate::common::csv; + +/// `[oslo_middleware]` section. +#[derive(Debug, Deserialize, Clone, Default)] +pub struct OsloMiddleware { + /// Whether to parse proxy forwarding headers (`Forwarded` per RFC 7239, + /// falling back to `X-Forwarded-For`) on the public interface to recover + /// the originating client address, overwriting the raw TCP peer captured + /// in `ConnectInfo`. + /// + /// The name and default (**off**) match upstream oslo.middleware's + /// `HTTPProxyToWSGI`. Even when enabled, a header is only honoured when the + /// immediate TCP peer matches [`trusted_proxies`](Self::trusted_proxies), so + /// a client reaching the listener directly cannot spoof its address. + #[serde(default)] + pub enable_proxy_headers_parsing: bool, + + /// CIDR blocks of reverse proxies trusted to set the forwarding headers + /// (e.g. `10.0.0.0/8, 192.168.0.0/16`). The client address is recovered + /// only when the immediate TCP peer falls within one of these ranges; the + /// effective client is then the rightmost address in the forwarding chain + /// that is not itself a trusted proxy. Empty (the default) means no proxy + /// is trusted, so the raw peer address is always used even when parsing is + /// enabled. + #[serde(default, deserialize_with = "csv")] + pub trusted_proxies: Vec, +} + +#[cfg(test)] +mod tests { + use config::{Config, File, FileFormat}; + + use super::*; + + #[test] + fn defaults_to_disabled() { + let sot = OsloMiddleware::default(); + assert!(!sot.enable_proxy_headers_parsing); + } + + #[test] + fn parses_enabled_flag_from_ini() { + let c = Config::builder() + .add_source(File::from_str( + "enable_proxy_headers_parsing = true", + FileFormat::Ini, + )) + .build() + .unwrap(); + let sot: OsloMiddleware = c.try_deserialize().unwrap(); + assert!(sot.enable_proxy_headers_parsing); + } + + #[test] + fn defaults_to_no_trusted_proxies() { + assert!(OsloMiddleware::default().trusted_proxies.is_empty()); + } + + #[test] + fn parses_trusted_proxies_csv_from_ini() { + let c = Config::builder() + .add_source(File::from_str( + "trusted_proxies = 10.0.0.0/8,192.168.0.0/16", + FileFormat::Ini, + )) + .build() + .unwrap(); + let sot: OsloMiddleware = c.try_deserialize().unwrap(); + assert_eq!( + sot.trusted_proxies, + vec!["10.0.0.0/8".to_string(), "192.168.0.0/16".to_string()] + ); + } +} diff --git a/crates/core/src/api.rs b/crates/core/src/api.rs index b10d92a46..bb8a5b1cc 100644 --- a/crates/core/src/api.rs +++ b/crates/core/src/api.rs @@ -15,6 +15,7 @@ pub mod api_key_auth; pub mod auth; pub mod common; +pub mod forwarded; pub mod v3; pub mod v4; diff --git a/crates/core/src/api/api_key_auth.rs b/crates/core/src/api/api_key_auth.rs index 663c0b508..09b951b2c 100644 --- a/crates/core/src/api/api_key_auth.rs +++ b/crates/core/src/api/api_key_auth.rs @@ -34,6 +34,7 @@ use openstack_keystone_core_types::mapping::resolution::IdentitySource; use openstack_keystone_core_types::mapping::virtual_user::MatchResult; use crate::api::KeystoneApiError; +use crate::api::forwarded::resolve_client_ip; use crate::api_key::{crypto, token}; use crate::auth::{ExecutionContext, ValidatedSecurityContext}; use crate::keystone::ServiceState; @@ -214,51 +215,6 @@ fn spawn_last_used_update(state: &ServiceState, resource: &ApiClientResource, no }); } -/// Compute the effective client IP using the rightmost-non-trusted-proxy -/// algorithm (ADR 0021 §3 Step 2, §6.E, Invariant 4): append the raw TCP -/// peer to the right of the `X-Forwarded-For` chain, then walk right to -/// left, returning the first address not in `trusted_proxies`. If the raw -/// TCP peer itself is not trusted, it is used directly without consulting -/// XFF at all. -fn resolve_client_ip( - headers: &axum::http::HeaderMap, - peer_ip: Option, - trusted_proxies: &[String], -) -> Option { - let trusted: Vec = trusted_proxies - .iter() - .filter_map(|c| c.parse::().ok()) - .collect(); - - let is_trusted = |ip: &IpAddr| trusted.iter().any(|net| net.contains(ip)); - - let peer = peer_ip?; - if !is_trusted(&peer) { - return Some(peer); - } - - let xff_chain: Vec = headers - .get(axum::http::header::HeaderName::from_static( - "x-forwarded-for", - )) - .and_then(|h| h.to_str().ok()) - .map(|h| { - h.split(',') - .filter_map(|s| s.trim().parse::().ok()) - .collect() - }) - .unwrap_or_default(); - - let mut chain = xff_chain; - chain.push(peer); - - chain - .into_iter() - .rev() - .find(|ip| !is_trusted(ip)) - .or(Some(peer)) -} - /// Whether `client_ip` satisfies the key's `allowed_ips` CIDR allowlist. /// `None` (missing field) means no restriction applies (ADR 0021 Invariant /// 5); a missing field and `Some(vec![])` are treated identically. @@ -394,83 +350,11 @@ async fn hydrate_ephemeral_context( #[cfg(test)] mod tests { use super::*; - use axum::http::HeaderMap; - - fn headers_with_xff(xff: &str) -> HeaderMap { - let mut headers = HeaderMap::new(); - headers.insert( - axum::http::header::HeaderName::from_static("x-forwarded-for"), - xff.parse().unwrap(), - ); - headers - } fn ip(s: &str) -> IpAddr { s.parse().unwrap() } - // --------------------------------------------------------------------- - // resolve_client_ip (ADR 0021 §3 Step 2, §6.E, Invariant 4) - // --------------------------------------------------------------------- - - #[test] - fn untrusted_peer_ignores_xff_entirely() { - // Peer itself is not a trusted proxy: XFF must not be consulted at - // all, even if present (prevents spoofing via an untrusted hop). - let headers = headers_with_xff("1.2.3.4"); - let peer = Some(ip("203.0.113.5")); - let trusted = vec!["10.0.0.0/8".to_string()]; - assert_eq!( - resolve_client_ip(&headers, peer, &trusted), - Some(ip("203.0.113.5")) - ); - } - - #[test] - fn trusted_peer_walks_xff_rightmost_non_trusted() { - // Chain (left to right): 1.2.3.4 (attacker-controlled), 10.0.0.5 - // (trusted intermediate hop), peer 10.0.0.1 (trusted, terminal - // proxy). Effective IP must be 10.0.0.5's predecessor scanning - // right-to-left: append peer, walk right-to-left, first non-trusted. - let headers = headers_with_xff("1.2.3.4, 10.0.0.5"); - let peer = Some(ip("10.0.0.1")); - let trusted = vec!["10.0.0.0/8".to_string()]; - assert_eq!( - resolve_client_ip(&headers, peer, &trusted), - Some(ip("1.2.3.4")) - ); - } - - #[test] - fn trusted_peer_all_hops_trusted_falls_back_to_peer() { - let headers = headers_with_xff("10.0.0.9"); - let peer = Some(ip("10.0.0.1")); - let trusted = vec!["10.0.0.0/8".to_string()]; - assert_eq!( - resolve_client_ip(&headers, peer, &trusted), - Some(ip("10.0.0.1")) - ); - } - - #[test] - fn leftmost_xff_entry_is_never_trusted_blindly() { - // Regression guard for the leftmost-take vulnerability (ADR 0021 F2): - // an attacker prepending a spoofed IP as the leftmost XFF entry must - // not be accepted just because it's present. - let headers = headers_with_xff("203.0.113.99, 1.2.3.4, 10.0.0.5"); - let peer = Some(ip("10.0.0.1")); - let trusted = vec!["10.0.0.0/8".to_string()]; - let effective = resolve_client_ip(&headers, peer, &trusted); - assert_ne!(effective, Some(ip("203.0.113.99"))); - assert_eq!(effective, Some(ip("1.2.3.4"))); - } - - #[test] - fn no_peer_ip_resolves_to_none() { - let headers = HeaderMap::new(); - assert_eq!(resolve_client_ip(&headers, None, &[]), None); - } - // --------------------------------------------------------------------- // ip_allowed (ADR 0021 Invariant 5) // --------------------------------------------------------------------- diff --git a/crates/core/src/api/forwarded.rs b/crates/core/src/api/forwarded.rs new file mode 100644 index 000000000..ea9de812d --- /dev/null +++ b/crates/core/src/api/forwarded.rs @@ -0,0 +1,261 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Client-IP resolution from proxy forwarding headers +//! +//! Shared by the API-key ingress (ADR 0021 §3) and the public-listener proxy +//! header middleware. It implements the trusted-proxy model: forwarding headers +//! are consulted **only** when the immediate TCP peer is a configured trusted +//! proxy, and the effective client is the rightmost address in the chain that +//! is not itself a trusted proxy. A client able to reach the listener directly +//! therefore cannot spoof its apparent address by prepending an +//! `X-Forwarded-For`/`Forwarded` entry. + +use std::net::{IpAddr, Ipv6Addr, SocketAddr}; + +use axum::http::HeaderMap; +use ipnet::IpNet; + +/// Upper bound on forwarding-chain entries parsed, guarding against an +/// unbounded comma list (parsing DoS). Only the rightmost hops are relevant to +/// the rightmost-non-trusted walk, so any excess left-hand entries are ignored. +const MAX_FORWARDED_HOPS: usize = 10; + +/// Resolve the effective client IP behind trusted reverse proxies. +/// +/// The raw TCP `peer_ip` is appended to the right of the forwarding chain +/// (RFC 7239 `Forwarded` preferred, else `X-Forwarded-For`); the chain is then +/// walked right to left, returning the first address that is not a member of +/// `trusted_proxies`. If the immediate peer is itself not trusted, the headers +/// are ignored entirely and the peer is returned unchanged. +/// +/// `trusted_proxies` is a list of CIDR blocks (e.g. `10.0.0.0/8`); unparsable +/// entries are skipped. Returns `None` only when no peer address is available. +pub fn resolve_client_ip( + headers: &HeaderMap, + peer_ip: Option, + trusted_proxies: &[String], +) -> Option { + let trusted: Vec = trusted_proxies + .iter() + .filter_map(|c| c.trim().parse::().ok()) + .collect(); + let is_trusted = |ip: &IpAddr| trusted.iter().any(|net| net.contains(ip)); + + let peer = peer_ip?; + // The immediate peer must be a trusted proxy before any header is honoured. + if !is_trusted(&peer) { + return Some(peer); + } + + let mut chain = forwarded_chain(headers); + chain.push(peer); + chain + .into_iter() + .rev() + .find(|ip| !is_trusted(ip)) + .or(Some(peer)) +} + +/// Parse the forwarding chain in left-to-right order, capped to the rightmost +/// [`MAX_FORWARDED_HOPS`] entries. Prefers the RFC 7239 `Forwarded` header, +/// falling back to `X-Forwarded-For`. +fn forwarded_chain(headers: &HeaderMap) -> Vec { + if let Some(value) = headers.get("forwarded").and_then(|v| v.to_str().ok()) { + return capped_chain(value, forwarded_element_ip); + } + if let Some(value) = headers.get("x-forwarded-for").and_then(|v| v.to_str().ok()) { + return capped_chain(value, |s| parse_ip_maybe_port(s.trim())); + } + Vec::new() +} + +/// Take the rightmost [`MAX_FORWARDED_HOPS`] comma-separated elements of a +/// header value, parse each with `parse`, and return them in left-to-right +/// order. Bounding the split before parsing caps the work an attacker can force. +fn capped_chain(value: &str, parse: impl Fn(&str) -> Option) -> Vec { + let mut ips: Vec = value + .rsplit(',') + .take(MAX_FORWARDED_HOPS) + .filter_map(parse) + .collect(); + ips.reverse(); + ips +} + +/// Extract the `for=` IP from a single RFC 7239 `Forwarded` list element such as +/// `for=192.0.2.60;proto=http;by=203.0.113.43`. Obfuscated identifiers +/// (`for=_hidden`) yield `None`. +fn forwarded_element_ip(element: &str) -> Option { + for param in element.split(';') { + let mut kv = param.trim().splitn(2, '='); + let key = kv.next()?.trim(); + if key.eq_ignore_ascii_case("for") { + let raw = kv.next()?.trim().trim_matches('"'); + return parse_ip_maybe_port(raw); + } + } + None +} + +/// Parse an address token that may be a bare IP, an `ip:port`, or a bracketed +/// IPv6 (`[::1]` / `[::1]:443`), returning just the IP. Obfuscated or malformed +/// tokens yield `None`. +fn parse_ip_maybe_port(s: &str) -> Option { + let s = s.trim(); + if s.is_empty() { + return None; + } + // Bracketed IPv6 (RFC 7239 form), with an optional trailing port. + if let Some(rest) = s.strip_prefix('[') { + let end = rest.find(']')?; + return rest[..end].parse::().ok().map(IpAddr::V6); + } + // `ip:port` (only unambiguous for IPv4; a bare IPv6 has many colons and is + // handled by the bare-IP branch below). + if let Ok(sa) = s.parse::() { + return Some(sa.ip()); + } + // Bare IPv4 or IPv6, no port. + s.parse::().ok() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn headers(pairs: &[(&str, &str)]) -> HeaderMap { + let mut h = HeaderMap::new(); + for (k, v) in pairs { + h.insert( + axum::http::HeaderName::from_bytes(k.as_bytes()).unwrap(), + v.parse().unwrap(), + ); + } + h + } + + fn ip(s: &str) -> IpAddr { + s.parse().unwrap() + } + + const TRUSTED: &[&str] = &["10.0.0.0/8"]; + + fn trusted() -> Vec { + TRUSTED.iter().map(|s| s.to_string()).collect() + } + + #[test] + fn no_peer_ip_resolves_to_none() { + assert_eq!(resolve_client_ip(&HeaderMap::new(), None, &trusted()), None); + } + + #[test] + fn untrusted_peer_ignores_headers_entirely() { + // Peer is not a trusted proxy: the header must not be consulted at all, + // even when present (prevents spoofing via a direct connection). + let h = headers(&[("x-forwarded-for", "1.2.3.4")]); + let peer = Some(ip("203.0.113.5")); + assert_eq!( + resolve_client_ip(&h, peer, &trusted()), + Some(ip("203.0.113.5")) + ); + } + + #[test] + fn trusted_peer_walks_xff_rightmost_non_trusted() { + let h = headers(&[("x-forwarded-for", "1.2.3.4, 10.0.0.5")]); + let peer = Some(ip("10.0.0.1")); + assert_eq!(resolve_client_ip(&h, peer, &trusted()), Some(ip("1.2.3.4"))); + } + + #[test] + fn leftmost_xff_entry_is_never_trusted_blindly() { + // Attacker prepends a spoofed leftmost entry; it must not be returned. + let h = headers(&[("x-forwarded-for", "203.0.113.99, 1.2.3.4, 10.0.0.5")]); + let peer = Some(ip("10.0.0.1")); + let effective = resolve_client_ip(&h, peer, &trusted()); + assert_ne!(effective, Some(ip("203.0.113.99"))); + assert_eq!(effective, Some(ip("1.2.3.4"))); + } + + #[test] + fn all_hops_trusted_falls_back_to_peer() { + let h = headers(&[("x-forwarded-for", "10.0.0.9")]); + let peer = Some(ip("10.0.0.1")); + assert_eq!( + resolve_client_ip(&h, peer, &trusted()), + Some(ip("10.0.0.1")) + ); + } + + #[test] + fn forwarded_header_is_preferred_and_walked_right_to_left() { + let h = headers(&[ + ( + "forwarded", + "for=203.0.113.7;proto=https, for=10.0.0.5;proto=https", + ), + ("x-forwarded-for", "198.51.100.1"), + ]); + let peer = Some(ip("10.0.0.1")); + assert_eq!( + resolve_client_ip(&h, peer, &trusted()), + Some(ip("203.0.113.7")) + ); + } + + #[test] + fn forwarded_quoted_ipv6_with_port_is_parsed() { + let h = headers(&[("forwarded", r#"for="[2001:db8:cafe::17]:4711""#)]); + let peer = Some(ip("10.0.0.1")); + assert_eq!( + resolve_client_ip(&h, peer, &trusted()), + Some(ip("2001:db8:cafe::17")) + ); + } + + #[test] + fn forwarded_obfuscated_identifier_falls_back_to_peer() { + let h = headers(&[("forwarded", "for=_hidden")]); + let peer = Some(ip("10.0.0.1")); + assert_eq!( + resolve_client_ip(&h, peer, &trusted()), + Some(ip("10.0.0.1")) + ); + } + + #[test] + fn xff_ipv4_with_port_strips_port() { + let h = headers(&[("x-forwarded-for", "203.0.113.7:5555")]); + let peer = Some(ip("10.0.0.1")); + assert_eq!( + resolve_client_ip(&h, peer, &trusted()), + Some(ip("203.0.113.7")) + ); + } + + #[test] + fn hop_count_is_capped_against_long_chains() { + // 12 spoofed public entries followed by two trusted hops and the peer. + // The cap keeps only the rightmost 10 parsed hops, but the real client + // (first non-trusted from the right) is still resolved correctly. + let spoof = std::iter::repeat_n("203.0.113.1", 12) + .collect::>() + .join(", "); + let value = format!("{spoof}, 8.8.8.8, 10.0.0.5"); + let h = headers(&[("x-forwarded-for", value.as_str())]); + let peer = Some(ip("10.0.0.1")); + assert_eq!(resolve_client_ip(&h, peer, &trusted()), Some(ip("8.8.8.8"))); + } +} diff --git a/crates/keystone/src/api/mod.rs b/crates/keystone/src/api/mod.rs index a3bcde006..c47e0ccd6 100644 --- a/crates/keystone/src/api/mod.rs +++ b/crates/keystone/src/api/mod.rs @@ -214,4 +214,59 @@ pub(crate) mod tests { .unwrap(); assert_eq!(&body[..], b"192.0.2.4:5555"); } + + /// Issue #358 follow-up: when proxy-header parsing is enabled (config-gated, + /// off by default), the public listener wraps the router with the + /// `rewrite_client_addr` layer, exactly as `spawn_public_listener` does: + /// the layer sits on the outer `Router`, *outside* the #734 + /// `NormalizePathLayer` fallback. This drives that full production + /// composition in-process and asserts that a `/echo/` request carrying + /// `X-Forwarded-For` (a) still normalizes the trailing slash, (b) reaches + /// the handler, and (c) delivers the *proxy-resolved* client address — not + /// the raw TCP peer — proving the layer runs before routing/normalization + /// and rewrites `ConnectInfo` end to end. + #[tokio::test] + async fn proxy_headers_rewrite_client_addr_and_normalize() { + async fn echo_addr(ConnectInfo(addr): ConnectInfo) -> String { + addr.ip().to_string() + } + + let router = Router::new().route("/echo", get(echo_addr)); + // Mirror `build_router`: the API service is `NormalizePath`-wrapped and + // mounted as the outer router's fallback. + let normalized = NormalizePathLayer::trim_trailing_slash().layer(router); + // The raw peer (10.0.0.9) must be a trusted proxy for the header to be + // honoured, mirroring the `[oslo_middleware] trusted_proxies` allowlist. + let trusted = std::sync::Arc::new(vec!["10.0.0.0/8".to_string()]); + let app = + Router::new() + .fallback_service(normalized) + .layer(axum::middleware::from_fn_with_state( + trusted, + crate::server::proxy_headers::rewrite_client_addr, + )); + + let make = + AxumServiceExt::>::into_make_service_with_connect_info::(app); + // Raw TCP peer is the reverse proxy; the header carries the real client. + let peer: SocketAddr = "10.0.0.9:5555".parse().unwrap(); + let svc = make.oneshot(peer).await.unwrap(); + + let response = svc + .oneshot( + Request::builder() + .uri("/echo/") + .header("x-forwarded-for", "203.0.113.7, 10.0.0.9") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let body = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .unwrap(); + assert_eq!(&body[..], b"203.0.113.7"); + } } diff --git a/crates/keystone/src/bin/keystone.rs b/crates/keystone/src/bin/keystone.rs index 590711191..9135e3513 100644 --- a/crates/keystone/src/bin/keystone.rs +++ b/crates/keystone/src/bin/keystone.rs @@ -74,6 +74,7 @@ use openstack_keystone::revoke::RevokeHook; use openstack_keystone::role::RoleHook; use openstack_keystone::scim; use openstack_keystone::server::listener::{raft_grpc, spiffe_tls, spiffe_tls_uds}; +use openstack_keystone::server::proxy_headers; use openstack_keystone::token::TokenHook; use openstack_keystone::trust::TrustHook; use openstack_keystone::webauthn; @@ -613,11 +614,16 @@ async fn build_router( .layer( TraceLayer::new(common::KeystoneResponseClassifier) .make_span_with(|request: &Request<_>| { - // Raw TCP peer address captured by - // `into_make_service_with_connect_info` on the public - // listener (the keystone-ng analogue of Python Keystone's - // WSGI REMOTE_ADDR / flask.request.remote_addr). `None` for - // the SPIFFE interfaces, which do not populate ConnectInfo. + // Client address captured into `ConnectInfo` + // (the keystone-ng analogue of Python Keystone's WSGI + // REMOTE_ADDR / flask.request.remote_addr): the raw TCP peer + // on the public listener via + // `into_make_service_with_connect_info`, or the mTLS peer on + // the internal SPIFFE-TLS listener (injected by hand). When + // `enable_proxy_headers_parsing` is on, the public value has + // been overwritten with the proxy-resolved client address. + // `None` on the admin UDS interface, which has no meaningful + // `SocketAddr`. let client_addr = request .extensions() .get::>() @@ -783,7 +789,27 @@ async fn spawn_public_listener( info!("Starting Rest API at {}", cfg.interface_public.tcp_address); let listener = TcpListener::bind(&cfg.interface_public.tcp_address).await?; let rest_cancel_token = token.clone(); - let rest_app = app; + // When operating behind a trusted reverse proxy (config-gated, + // off by default), parse `Forwarded`/`X-Forwarded-For` and rewrite + // the raw-peer `ConnectInfo` with the originating client address + // *before* the tracing span and handlers read it. The header is + // honoured only when the immediate peer matches `trusted_proxies`, + // so an empty allowlist keeps the raw peer. The layer is added only + // on this public interface — never on the internal SPIFFE/admin + // listeners, whose peers are the mTLS mesh. + let rest_app = if cfg.oslo_middleware.enable_proxy_headers_parsing { + info!( + trusted_proxies = cfg.oslo_middleware.trusted_proxies.len(), + "Proxy header parsing enabled on the public interface" + ); + let trusted = Arc::new(cfg.oslo_middleware.trusted_proxies.clone()); + app.layer(axum::middleware::from_fn_with_state( + trusted, + proxy_headers::rewrite_client_addr, + )) + } else { + app + }; handles.spawn(async move { // `rest_app` is a `Router` whose fallback is the // `NormalizePath`-wrapped API service (issue #734, #1467); use @@ -793,11 +819,11 @@ async fn spawn_public_listener( // `into_make_service_with_connect_info::` stores the // raw TCP peer address in a `ConnectInfo` request // extension (the analogue of Python Keystone's WSGI REMOTE_ADDR). - // This is the *raw* peer, not proxy-resolved: behind a reverse - // proxy/LB it is the proxy's address. A trusted forwarded-header - // layer (mirroring oslo_middleware's `enable_proxy_headers_parsing`, - // off by default) is a deliberate follow-up before this is used - // for any IP-based login control. See issue #358. + // Behind a reverse proxy/LB this is the proxy's address; the + // `rewrite_client_addr` layer wired above (when + // `enable_proxy_headers_parsing` is on) overwrites it with the + // trusted-proxy-resolved client address before any consumer + // reads it. let cancel_token = rest_cancel_token.clone(); if let Err(e) = axum::serve( listener, diff --git a/crates/keystone/src/server.rs b/crates/keystone/src/server.rs index 12f3d3b72..9d4c7d1fa 100644 --- a/crates/keystone/src/server.rs +++ b/crates/keystone/src/server.rs @@ -13,3 +13,4 @@ // SPDX-License-Identifier: Apache-2.0 //! # Keystone server listeners pub mod listener; +pub mod proxy_headers; diff --git a/crates/keystone/src/server/listener/spiffe_tls.rs b/crates/keystone/src/server/listener/spiffe_tls.rs index 4ee270062..79036acbf 100644 --- a/crates/keystone/src/server/listener/spiffe_tls.rs +++ b/crates/keystone/src/server/listener/spiffe_tls.rs @@ -14,6 +14,7 @@ //! # TLS server listener with the SPIFFE integration use axum::Router; +use axum::extract::ConnectInfo; use color_eyre::eyre::{Report, Result}; use hyper_util::rt::{TokioExecutor, TokioIo}; use hyper_util::server::conn::auto::Builder; @@ -75,13 +76,12 @@ pub async fn start_axum_app( let interface_clone = interface_clone.clone(); async move { let mut req = req; - if let Some(spiffe_id) = spiffe_id { - // Move the client TLS certificate into the request extensions - tracing::debug!("The client supplied certificate for spiffe_id: {:?}", spiffe_id); - - req.extensions_mut().insert(spiffe_id); - } - req.extensions_mut().insert(interface_clone); + attach_request_context( + req.extensions_mut(), + spiffe_id, + peer_addr, + interface_clone, + ); // Call Axum and explicitly wrap the result app.call(req).await } @@ -120,3 +120,72 @@ pub async fn start_axum_app( Ok(()) } + +/// Attach the per-request context derived from an mTLS connection onto the +/// request extensions before it is handed to the axum app. +/// +/// Because `hyper::service::service_fn` bypasses axum's make-service, the +/// context the public HTTP path gets for free must be injected by hand here: +/// * the peer's validated [`CoreSpiffeId`], when present; +/// * the raw TCP peer address in the same [`ConnectInfo`] +/// extension the public listener populates via +/// `into_make_service_with_connect_info` (issue #358), so `client.addr` +/// is captured on the internal interface too; +/// * the [`Interface`] the request arrived on. +fn attach_request_context( + extensions: &mut axum::http::Extensions, + spiffe_id: Option, + peer_addr: std::net::SocketAddr, + interface: Interface, +) { + if let Some(spiffe_id) = spiffe_id { + // Move the client TLS certificate into the request extensions + tracing::debug!( + "The client supplied certificate for spiffe_id: {:?}", + spiffe_id + ); + extensions.insert(spiffe_id); + } + extensions.insert(ConnectInfo(peer_addr)); + extensions.insert(interface); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn attaches_connect_info_and_interface_without_spiffe_id() { + let mut ext = axum::http::Extensions::new(); + let peer: std::net::SocketAddr = "203.0.113.7:4711".parse().unwrap(); + + attach_request_context(&mut ext, None, peer, Interface::Internal); + + // Issue #358: the mTLS peer address is captured in the same extension + // type the public listener uses, so `client.addr` is populated here. + assert_eq!( + ext.get::>() + .map(|ci| ci.0), + Some(peer) + ); + assert_eq!(ext.get::(), Some(&Interface::Internal)); + // No SVID presented → no SpiffeId extension. + assert!(ext.get::().is_none()); + } + + #[test] + fn attaches_spiffe_id_when_present() { + let mut ext = axum::http::Extensions::new(); + let peer: std::net::SocketAddr = "[2001:db8::1]:8443".parse().unwrap(); + let spiffe_id = CoreSpiffeId::new("spiffe://example.org/workload").unwrap(); + + attach_request_context(&mut ext, Some(spiffe_id.clone()), peer, Interface::Internal); + + assert_eq!(ext.get::(), Some(&spiffe_id)); + assert_eq!( + ext.get::>() + .map(|ci| ci.0), + Some(peer) + ); + } +} diff --git a/crates/keystone/src/server/proxy_headers.rs b/crates/keystone/src/server/proxy_headers.rs new file mode 100644 index 000000000..36c445026 --- /dev/null +++ b/crates/keystone/src/server/proxy_headers.rs @@ -0,0 +1,159 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 +//! # Proxy forwarded-header parsing +//! +//! When Keystone runs behind a trusted reverse proxy / load balancer, the raw +//! TCP peer captured in [`ConnectInfo`] is the proxy's address, not +//! the real client's. This middleware recovers the originating client address +//! from the standard forwarding headers and overwrites `ConnectInfo` with it, +//! so every downstream consumer (the `request` tracing span, the API-Key IP +//! allowlist, any future IP-based login control) transparently observes the +//! real client. +//! +//! It mirrors upstream Python Keystone's `[oslo_middleware] +//! enable_proxy_headers_parsing`: the [`rewrite_client_addr`] layer is only +//! wired onto the **public** listener when that flag is enabled, and it is +//! **off by default**. +//! +//! Extraction is not blind. The header is honoured only when the immediate TCP +//! peer matches the operator-configured `trusted_proxies` allowlist, and the +//! effective client is the rightmost address in the chain that is not itself a +//! trusted proxy — the same rightmost-non-trusted-proxy algorithm the API-Key +//! ingress uses ([`openstack_keystone_core::api::forwarded`]). A client able to +//! reach the listener directly therefore cannot spoof its apparent address, and +//! an empty allowlist trusts no one (the raw peer is always kept). + +use std::net::SocketAddr; +use std::sync::Arc; + +use axum::extract::ConnectInfo; +use axum::extract::Request; +use axum::extract::State; +use axum::middleware::Next; +use axum::response::Response; + +use openstack_keystone_core::api::forwarded::resolve_client_ip; + +/// Axum middleware that overwrites the request's [`ConnectInfo`] +/// with the proxy-resolved client address when the immediate peer is a trusted +/// proxy and a forwarding header carries a different upstream client. +/// +/// `trusted_proxies` is the operator-configured CIDR allowlist (`[oslo_middleware] +/// trusted_proxies`). When the resolved client equals the raw peer (direct +/// client, untrusted peer, or an all-trusted chain), `ConnectInfo` is left +/// untouched so the real source port is preserved. The recovered address has no +/// meaningful source port, so port `0` is used. +pub async fn rewrite_client_addr( + State(trusted_proxies): State>>, + mut req: Request, + next: Next, +) -> Response { + let peer_ip = req + .extensions() + .get::>() + .map(|ConnectInfo(addr)| addr.ip()); + + if let Some(client_ip) = resolve_client_ip(req.headers(), peer_ip, &trusted_proxies) + && Some(client_ip) != peer_ip + { + req.extensions_mut() + .insert(ConnectInfo(SocketAddr::new(client_ip, 0))); + } + next.run(req).await +} + +#[cfg(test)] +mod tests { + use super::*; + + use axum::body::Body; + use axum::routing::get; + use axum::{Router, ServiceExt as AxumServiceExt}; + use tower::ServiceExt as _; + + async fn echo(ConnectInfo(addr): ConnectInfo) -> String { + addr.to_string() + } + + /// Build the app with the middleware carrying a fixed trusted-proxy list, + /// drive it with `peer` as the raw TCP peer, and return the address the + /// handler observed. + async fn observed_addr(trusted: &[&str], peer: &str, headers: &[(&str, &str)]) -> String { + let trusted = Arc::new(trusted.iter().map(|s| s.to_string()).collect::>()); + let app = + Router::new() + .route("/echo", get(echo)) + .layer(axum::middleware::from_fn_with_state( + trusted, + rewrite_client_addr, + )); + let make = + AxumServiceExt::::into_make_service_with_connect_info::(app); + let peer: SocketAddr = peer.parse().unwrap(); + let svc = make.oneshot(peer).await.unwrap(); + + let mut builder = Request::builder().uri("/echo"); + for (k, v) in headers { + builder = builder.header(*k, *v); + } + let resp = svc + .oneshot(builder.body(Body::empty()).unwrap()) + .await + .unwrap(); + let body = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + String::from_utf8(body.to_vec()).unwrap() + } + + #[tokio::test] + async fn rewrites_to_client_when_peer_is_trusted() { + // Raw peer 10.0.0.9 is a trusted proxy; the XFF chain carries the client. + let addr = observed_addr( + &["10.0.0.0/8"], + "10.0.0.9:1111", + &[("x-forwarded-for", "203.0.113.7, 10.0.0.9")], + ) + .await; + // Rewritten to the client IP (with the conventional port 0). + assert_eq!(addr, "203.0.113.7:0"); + } + + #[tokio::test] + async fn ignores_header_when_peer_is_untrusted() { + // A direct (untrusted) client cannot spoof its address via the header; + // the raw peer address — including its real port — is preserved. + let addr = observed_addr( + &["10.0.0.0/8"], + "203.0.113.50:2222", + &[("x-forwarded-for", "1.2.3.4")], + ) + .await; + assert_eq!(addr, "203.0.113.50:2222"); + } + + #[tokio::test] + async fn preserves_peer_when_no_forwarded_header() { + let addr = observed_addr(&["10.0.0.0/8"], "10.0.0.9:3333", &[]).await; + // Peer is trusted but there is no header, so it stays the peer. + assert_eq!(addr, "10.0.0.9:3333"); + } + + #[tokio::test] + async fn empty_allowlist_trusts_no_one() { + // With no trusted proxies configured, the header is never honoured. + let addr = observed_addr(&[], "10.0.0.9:4444", &[("x-forwarded-for", "203.0.113.7")]).await; + assert_eq!(addr, "10.0.0.9:4444"); + } +}