diff --git a/deploy/config/env.example b/deploy/config/env.example index 81edb6a3..11644525 100644 --- a/deploy/config/env.example +++ b/deploy/config/env.example @@ -233,6 +233,9 @@ TRACKING_PAGEHIT_RATE_LIMIT_PER_MIN=60 # CIDRs whose forwarded-IP headers the tracking service believes. Empty uses the # socket peer. Set it behind a reverse proxy, like TRUSTED_PROXIES above. # TRACKING_TRUSTED_PROXIES=10.0.0.0/8 +# The header that proxy sets with the client address (x-forwarded-for, or +# cf-connecting-ip behind Cloudflare). Nothing else is read. +# TRACKING_CLIENT_IP_HEADER=x-forwarded-for # === Realtime service (Elixir/Phoenix) === PHX_HOST=localhost diff --git a/docker-compose.yml b/docker-compose.yml index 50186477..93649787 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -413,6 +413,7 @@ services: TRACKING_RATE_LIMIT_PER_MIN: ${TRACKING_RATE_LIMIT_PER_MIN:-} TRACKING_PAGEHIT_RATE_LIMIT_PER_MIN: ${TRACKING_PAGEHIT_RATE_LIMIT_PER_MIN:-} TRACKING_TRUSTED_PROXIES: ${TRACKING_TRUSTED_PROXIES:-} + TRACKING_CLIENT_IP_HEADER: ${TRACKING_CLIENT_IP_HEADER:-} SENTRY_DSN: ${SENTRY_DSN:-} # Overridable so `make dev` (native backend) can point at # host.docker.internal:8080 while `docker compose up` uses the container. diff --git a/docs/content/docs/development/configuration.mdx b/docs/content/docs/development/configuration.mdx index 459beee6..cb708730 100644 --- a/docs/content/docs/development/configuration.mdx +++ b/docs/content/docs/development/configuration.mdx @@ -384,7 +384,8 @@ The Rust open and click service. It reads its own environment, so these have to | `INTERNAL_API_TOKEN` | Bearer token for that lookup. **Required**: the service exits at boot on an empty value | none | | `TRACKING_RATE_LIMIT_PER_MIN` | Counted pixel and click requests per source per minute. Over budget, pixels are still served but not counted, and click redirects get `429` | `300` | | `TRACKING_PAGEHIT_RATE_LIMIT_PER_MIN` | Website page views accepted per source per minute, on top of the shared budget above. Over budget, the snippet gets `429` | `60` | -| `TRACKING_TRUSTED_PROXIES` | CIDRs whose `X-Forwarded-For` / `CF-Connecting-IP` headers the tracking service believes (the proxy-appended, last `X-Forwarded-For` entry). Empty trusts nothing and uses the socket peer, which is correct for a directly exposed service; set it behind a reverse proxy or the per-source rate limits and the location stored with page views are caller-controlled. Same convention as the backend's `TRUSTED_PROXIES` | empty | +| `TRACKING_TRUSTED_PROXIES` | CIDRs the tracking service accepts a forwarded client address from. Empty trusts nothing and uses the socket peer, which is correct for a directly exposed service; set it behind a reverse proxy or the per-source rate limits and the location stored with page views are caller-controlled. Same convention as the backend's `TRUSTED_PROXIES` | empty | +| `TRACKING_CLIENT_IP_HEADER` | The one header a trusted proxy sets with the client address. No other header is read, so a caller cannot smuggle an address past a generic proxy in `CF-Connecting-IP`. For `x-forwarded-for` the proxy-appended last entry is used; set `cf-connecting-ip` behind Cloudflare | `x-forwarded-for` | | `EVENTBUS_PROVIDER` | `nats` or `kafka`. Kafka needs an image built with `CARGO_FEATURES=kafka` | `nats` | | `NATS_URL`, `NATS_SUBJECT_PREFIX` | JetStream address and subject prefix. The publish subject is `.` | `nats://localhost:4222`, `warmbly` | | `KAFKA_TRACKING_TOPIC` | Event topic, read by the Rust publisher **and** the Go subscriber | `tracking-events` | diff --git a/docs/content/docs/guides/website-tracking.mdx b/docs/content/docs/guides/website-tracking.mdx index 345c64de..6ffe029a 100644 --- a/docs/content/docs/guides/website-tracking.mdx +++ b/docs/content/docs/guides/website-tracking.mdx @@ -77,4 +77,4 @@ Page views show in the **Activity** tab of a contact under the **Website** filte ## Abuse controls -The ingest endpoint on the tracking service is public, so it runs behind the same controls as the open pixel and click redirects: a per-source request budget (with a tighter one for page views on top), prefetch and crawler filtering, an 8 KB body cap with limits on every field, a short window that ignores reloads and double-fires, a per-source budget for unknown site keys, and a circuit breaker toward the backend. Forwarded-IP headers are believed only from proxies the operator lists in `TRACKING_TRUSTED_PROXIES`, so a caller cannot choose its own rate-limit bucket or the location stored with a view. Over budget requests get `429`; everything else that is declined is acknowledged quietly so a probe learns nothing about which keys exist. +The ingest endpoint on the tracking service is public, so it runs behind the same controls as the open pixel and click redirects: a per-source request budget (with a tighter one for page views on top), prefetch and crawler filtering, an 8 KB body cap with limits on every field, a short window that ignores reloads and double-fires, a per-source budget for unknown site keys, and a circuit breaker toward the backend. A forwarded client address is believed only from proxies the operator lists in `TRACKING_TRUSTED_PROXIES`, and only from the one header named in `TRACKING_CLIENT_IP_HEADER`, so a caller cannot choose its own rate-limit bucket or the location stored with a view. Over budget requests get `429`; everything else that is declined is acknowledged quietly so a probe learns nothing about which keys exist. diff --git a/tracking/src/config.rs b/tracking/src/config.rs index 076e8b73..d8f97949 100644 --- a/tracking/src/config.rs +++ b/tracking/src/config.rs @@ -61,6 +61,10 @@ pub struct Config { /// CIDRs whose forwarded-IP headers are believed. Empty trusts nothing, /// so the socket peer is the client, the same rule as the backend. pub trusted_proxies: Vec, + /// The one header a trusted proxy sets with the client address. Only this + /// header is read, so a client-supplied CF-Connecting-IP behind a generic + /// proxy is ignored. For x-forwarded-for the proxy-appended last entry wins. + pub client_ip_header: String, } impl Config { @@ -184,7 +188,15 @@ impl Config { let trusted_proxies = parse_trusted_proxies(&env::var("TRACKING_TRUSTED_PROXIES").unwrap_or_default()); - info!("Trusted proxies: {:?}", trusted_proxies); + let client_ip_header = env::var("TRACKING_CLIENT_IP_HEADER") + .ok() + .map(|v| v.trim().to_ascii_lowercase()) + .filter(|v| !v.is_empty()) + .unwrap_or_else(|| "x-forwarded-for".to_string()); + info!( + "Trusted proxies: {:?} (client ip header: {})", + trusted_proxies, client_ip_header + ); Ok(Self { env: env_name, @@ -205,6 +217,7 @@ impl Config { rate_limit_per_min, pagehit_rate_limit_per_min, trusted_proxies, + client_ip_header, }) } @@ -283,6 +296,9 @@ impl Config { trusted_proxies: parse_trusted_proxies( &env::var("TRACKING_TRUSTED_PROXIES").unwrap_or_default(), ), + client_ip_header: env::var("TRACKING_CLIENT_IP_HEADER") + .unwrap_or_else(|_| "x-forwarded-for".to_string()) + .to_ascii_lowercase(), }) } diff --git a/tracking/src/handlers.rs b/tracking/src/handlers.rs index 1d37b7f1..c2d2a583 100644 --- a/tracking/src/handlers.rs +++ b/tracking/src/handlers.rs @@ -43,8 +43,9 @@ pub struct AppState { pub hits: Arc, /// Tighter per-source budget for page views than for pixels pub hit_rate_limiter: Arc, - /// Proxies whose forwarded-IP headers are believed + /// Proxies whose forwarded-IP header is believed, and which header pub trusted_proxies: Arc>, + pub client_ip_header: Arc, } impl AppState { @@ -73,6 +74,7 @@ impl AppState { )), hit_rate_limiter: Arc::new(RateLimiter::new(config.pagehit_rate_limit_per_min)), trusted_proxies: Arc::new(config.trusted_proxies.clone()), + client_ip_header: Arc::new(config.client_ip_header.clone()), } } @@ -123,7 +125,12 @@ pub async fn track_open( } // Extract IP hash for deduplication + rate limiting - let ip_hash = Some(hash_ip(&client_ip(peer, &headers, &state.trusted_proxies))); + let ip_hash = Some(hash_ip(&client_ip( + peer, + &headers, + &state.trusted_proxies, + &state.client_ip_header, + ))); // Anti-flood: over-budget sources still get the pixel (real mail clients // must never see a broken image), but nothing is published. @@ -185,7 +192,12 @@ pub async fn track_click( } // Anti-flood: cap total request rate per source - let ip_hash = Some(hash_ip(&client_ip(peer, &headers, &state.trusted_proxies))); + let ip_hash = Some(hash_ip(&client_ip( + peer, + &headers, + &state.trusted_proxies, + &state.client_ip_header, + ))); let source = ip_hash.clone().unwrap_or_else(|| "unknown".to_string()); if !state.rate_limiter.allow(&source).await { return (StatusCode::TOO_MANY_REQUESTS, "Slow down").into_response(); @@ -299,7 +311,12 @@ pub async fn track_page_hit( headers: HeaderMap, body: Bytes, ) -> Response { - let ip = client_ip(peer, &headers, &state.trusted_proxies); + let ip = client_ip( + peer, + &headers, + &state.trusted_proxies, + &state.client_ip_header, + ); let source = hash_ip(&ip); // Anti-flood: page views have their own, tighter budget on top of the @@ -415,27 +432,33 @@ fn pixel_response() -> Response { .into_response() } -/// The client address. Forwarded headers are believed only when the socket -/// peer is a trusted proxy; otherwise anyone could pick their own rate-limit -/// bucket and the location stored with a page view. -fn client_ip(peer: SocketAddr, headers: &HeaderMap, trusted: &[ipnet::IpNet]) -> String { +/// The client address. The configured header is believed only when the +/// socket peer is a trusted proxy, and no other header is consulted, so a +/// caller cannot pick its rate-limit bucket or the location stored with a +/// page view by adding a header the proxy passes through. +fn client_ip( + peer: SocketAddr, + headers: &HeaderMap, + trusted: &[ipnet::IpNet], + header: &str, +) -> String { let peer_ip = peer.ip(); if !trusted.iter().any(|net| net.contains(&peer_ip)) { return peer_ip.to_string(); } - let header = |name: &str| { - headers - .get(name) - .and_then(|h| h.to_str().ok()) - .map(str::trim) - .filter(|v| !v.is_empty()) + let raw = headers + .get(header) + .and_then(|h| h.to_str().ok()) + .map(str::trim) + .filter(|v| !v.is_empty()); + // A proxy appends the address it saw as the LAST X-Forwarded-For entry; + // the earlier ones are whatever the client claimed. + let candidate = match raw { + Some(v) if header == "x-forwarded-for" => v.rsplit(',').next().map(str::trim), + Some(v) => Some(v), + None => None, }; - // The proxy appends the address it saw as the LAST X-Forwarded-For entry; - // the first is whatever the client claimed. - let forwarded = header("cf-connecting-ip") - .or_else(|| header("x-forwarded-for").and_then(|v| v.rsplit(',').next().map(str::trim))) - .or_else(|| header("x-real-ip")); - match forwarded.and_then(|v| v.parse::().ok()) { + match candidate.and_then(|v| v.parse::().ok()) { Some(ip) => ip.to_string(), None => peer_ip.to_string(), } @@ -477,19 +500,37 @@ mod tests { fn client_ip_ignores_forwarded_headers_from_untrusted_peers() { let peer: SocketAddr = "203.0.113.9:4000".parse().unwrap(); let h = hdr(&[("x-forwarded-for", "1.1.1.1, 2.2.2.2")]); - assert_eq!(client_ip(peer, &h, &[]), "203.0.113.9"); + assert_eq!(client_ip(peer, &h, &[], "x-forwarded-for"), "203.0.113.9"); } #[test] - fn client_ip_takes_the_proxy_appended_entry_from_trusted_peers() { + fn client_ip_reads_only_the_configured_header_from_trusted_peers() { let trusted = vec!["10.0.0.0/8".parse::().unwrap()]; let peer: SocketAddr = "10.1.2.3:4000".parse().unwrap(); + // Proxy-appended last entry wins over what the client claimed. let h = hdr(&[("x-forwarded-for", "1.1.1.1, 198.51.100.7")]); - assert_eq!(client_ip(peer, &h, &trusted), "198.51.100.7"); - let h = hdr(&[("cf-connecting-ip", "198.51.100.8")]); - assert_eq!(client_ip(peer, &h, &trusted), "198.51.100.8"); + assert_eq!( + client_ip(peer, &h, &trusted, "x-forwarded-for"), + "198.51.100.7" + ); + // A client-supplied CF-Connecting-IP passed through a generic proxy is + // ignored unless that header is the configured one. + let h = hdr(&[ + ("cf-connecting-ip", "8.8.8.8"), + ("x-forwarded-for", "198.51.100.7"), + ]); + assert_eq!( + client_ip(peer, &h, &trusted, "x-forwarded-for"), + "198.51.100.7" + ); + assert_eq!(client_ip(peer, &h, &trusted, "cf-connecting-ip"), "8.8.8.8"); + // Garbage or a missing header falls back to the proxy itself. let h = hdr(&[("x-forwarded-for", "not an ip")]); - assert_eq!(client_ip(peer, &h, &trusted), "10.1.2.3"); + assert_eq!(client_ip(peer, &h, &trusted, "x-forwarded-for"), "10.1.2.3"); + assert_eq!( + client_ip(peer, &hdr(&[]), &trusted, "x-forwarded-for"), + "10.1.2.3" + ); } #[test]