feat: read the forwarded client address from exactly one operator-named header (TRACKING_CLIENT_IP_HEADER, default x-forwarded-for with the proxy-appended last entry, cf-connecting-ip only when configured) so a client-supplied CF-Connecting-IP passed through a generic trusted proxy can no longer choose the rate-limit bucket or the stored page-hit location, with tests and the configuration, env example and compose entries

This commit is contained in:
Matthew Meszaros
2026-08-29 05:11:22 -07:00
parent 5821fc2a1c
commit 111a5a4034
6 changed files with 91 additions and 29 deletions
+3
View File
@@ -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
+1
View File
@@ -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.
@@ -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 `<prefix>.<topic>` | `nats://localhost:4222`, `warmbly` |
| `KAFKA_TRACKING_TOPIC` | Event topic, read by the Rust publisher **and** the Go subscriber | `tracking-events` |
@@ -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.
+17 -1
View File
@@ -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<ipnet::IpNet>,
/// 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(),
})
}
+67 -26
View File
@@ -43,8 +43,9 @@ pub struct AppState {
pub hits: Arc<HitForwarder>,
/// Tighter per-source budget for page views than for pixels
pub hit_rate_limiter: Arc<RateLimiter>,
/// Proxies whose forwarded-IP headers are believed
/// Proxies whose forwarded-IP header is believed, and which header
pub trusted_proxies: Arc<Vec<ipnet::IpNet>>,
pub client_ip_header: Arc<String>,
}
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::<IpAddr>().ok()) {
match candidate.and_then(|v| v.parse::<IpAddr>().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::<ipnet::IpNet>().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]