Skip to main content

max / makenotwork

Key Multithreaded rate limits on CF-Connecting-IP, not rightmost XFF Rightmost-X-Forwarded-For identifies the client only behind exactly one trusted proxy. Prod has two: forums.makenot.work is Cloudflare-fronted and proxies to localhost:3400, so Cloudflare appends the real client and Caddy then appends the Cloudflare edge. The rightmost entry was therefore always a Cloudflare edge address, collapsing every visitor routed through that edge into one rate-limit bucket. Per-IP limits stopped isolating anyone, and a single abuser could drain the bucket shared by everyone behind their edge. A trusted peer's CF-Connecting-IP now wins over X-Forwarded-For. Cloudflare sets it to the true client and it survives exactly one hop, and the vhost pins TLS client auth to the Cloudflare origin-pull CA, so nothing else can reach the origin to forge one. Stricter than the MNW server's extractor, which trusts the header from any peer: Multithreaded still requires the direct peer to be a configured proxy. Rightmost-XFF stays as the fallback for a single-proxy deployment with no Cloudflare in front. That new dependence needed a guard. The Caddyfile lint only covered the :3000 upstream, so dropping "import cloudflare_tls" from the forums block would have made CF-Connecting-IP forgeable again with nothing failing. It now covers :3400 too, and asserts a block count per upstream so a vhost cannot fall out of the lint silently instead of failing it. Prod env confirmed alongside this: HOST=127.0.0.1 and no TRUSTED_PROXIES override, so the listener is loopback-only and trusts loopback alone.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-28 17:18 UTC
Signed with PGP, not checked
Commit: c77a0181a8c23397240a4ccd40a035c85fb77fe6
Parent: 996dc00
4 files changed, +139 insertions, -31 deletions
M multithreaded/src/config.rs +4 -3
@@ -17,9 +17,10 @@
17 17 pub s3: Option<S3Config>,
18 18 /// Shared secret for HMAC-signed internal API requests from MNW.
19 19 pub internal_shared_secret: Option<String>,
20 - /// Proxy IPs whose `X-Forwarded-For` the rate limiter trusts. A request
21 - /// whose direct peer is in this set is keyed on its forwarded client IP;
22 - /// any other peer is keyed on its own address and its XFF is ignored.
20 + /// Proxy IPs whose forwarding headers the rate limiter trusts. A request
21 + /// whose direct peer is in this set is keyed on its forwarded client IP
22 + /// (`CF-Connecting-IP`, else rightmost `X-Forwarded-For`); any other peer is
23 + /// keyed on its own address and both headers are ignored.
23 24 /// Parsed from `TRUSTED_PROXIES` (comma-separated IPs); defaults to loopback
24 25 /// (`127.0.0.1`, `::1`) for the on-host Caddy deployment. A loopback peer is
25 26 /// only reachable on-box, so trusting its XFF cannot be spoofed remotely.
M multithreaded/src/main.rs +4 -3
@@ -175,9 +175,10 @@
175 175 ));
176 176
177 177 // Default to loopback. Rate limiting uses TrustedProxyKeyExtractor, which
178 - // honors X-Forwarded-For only from a configured trusted proxy (TRUSTED_PROXIES,
179 - // default loopback) and otherwise keys on the direct peer, so a spoofed XFF
180 - // from a non-proxy client is ignored regardless of bind address. Binding to
178 + // honors CF-Connecting-IP / X-Forwarded-For only from a configured trusted
179 + // proxy (TRUSTED_PROXIES, default loopback) and otherwise keys on the direct
180 + // peer, so spoofed forwarding headers from a non-proxy client are ignored
181 + // regardless of bind address. Binding to
181 182 // 127.0.0.1 is still the right default (only Caddy reaches the port); a
182 183 // tailnet-direct staging box sets HOST and lists its proxy in TRUSTED_PROXIES.
183 184 let host = std::env::var("HOST").unwrap_or_else(|_| "127.0.0.1".to_string());
M multithreaded/src/trusted_proxy.rs +99 -9
@@ -4,11 +4,29 @@
4 4 //! `tower_governor`'s `SmartIpKeyExtractor` reads `X-Forwarded-For` from *every*
5 5 //! request, so any direct client can mint a fresh rate-limit bucket per request
6 6 //! by rotating the header, defeating every limiter. [`TrustedProxyKeyExtractor`]
7 - //! closes that: it honors `X-Forwarded-For` only when the request's *direct
7 + //! closes that: it honors forwarding headers only when the request's *direct
8 8 //! peer* is a configured trusted proxy, and otherwise keys on the peer address
9 - //! and ignores the header entirely. There is deliberately **no `Default`**, the
9 + //! and ignores the headers entirely. There is deliberately **no `Default`**, the
10 10 //! set of trusted proxies must be supplied explicitly, so "trust this header"
11 11 //! can never be the silent default that the prior extractor made it.
12 + //!
13 + //! # Two hops, not one
14 + //!
15 + //! Rightmost-`X-Forwarded-For` identifies the real client only when exactly one
16 + //! trusted proxy sits in front of the app. Prod has two: Cloudflare, then the
17 + //! on-host Caddy (`forums.makenot.work` in `MNW/server/deploy/Caddyfile`).
18 + //! Cloudflare appends the real client to `X-Forwarded-For`, then Caddy appends
19 + //! the Cloudflare edge address, so the rightmost entry is a *Cloudflare* IP.
20 + //! Keying on it collapses every visitor routed through the same edge into one
21 + //! bucket: per-IP limits stop isolating abusers, and one abuser exhausts the
22 + //! bucket shared by everyone behind that edge.
23 + //!
24 + //! So a trusted peer's `CF-Connecting-IP` wins over `X-Forwarded-For`.
25 + //! Cloudflare sets that header to the true client and it survives exactly one
26 + //! hop, and the vhost pins TLS client auth to the Cloudflare origin-pull CA
27 + //! (`import cloudflare_tls`), so nothing but Cloudflare can reach the origin to
28 + //! forge one. Rightmost-XFF stays as the fallback for a single-proxy
29 + //! deployment (a tailnet-direct staging box) that has no Cloudflare in front.
12 30
13 31 use std::net::{IpAddr, SocketAddr};
14 32 use std::sync::Arc;
@@ -18,7 +36,8 @@
18 36 use tower_governor::key_extractor::KeyExtractor;
19 37
20 38 /// A [`KeyExtractor`] that keys on the real client IP, trusting
21 - /// `X-Forwarded-For` only when the direct peer is a known proxy.
39 + /// `CF-Connecting-IP` / `X-Forwarded-For` only when the direct peer is a known
40 + /// proxy.
22 41 ///
23 42 /// Construction requires the trusted-proxy set; there is no `Default`. An empty
24 43 /// set trusts no proxy, every request keys on its direct peer, which is safe
@@ -53,17 +72,35 @@
53 72
54 73 fn extract<T>(&self, req: &Request<T>) -> Result<Self::Key, GovernorError> {
55 74 let peer = Self::peer_ip(req).ok_or(GovernorError::UnableToExtractKey)?;
56 - // Only a trusted proxy's X-Forwarded-For is believed. A direct (untrusted)
57 - // peer keys on its own socket address, so it cannot spoof another key.
58 - if self.trusted_proxies.contains(&peer)
59 - && let Some(client) = rightmost_xff(req.headers())
60 - {
61 - return Ok(client);
75 + // Only a trusted proxy's forwarding headers are believed. A direct
76 + // (untrusted) peer keys on its own socket address, so it cannot spoof
77 + // another key.
78 + if self.trusted_proxies.contains(&peer) {
79 + // CF-Connecting-IP first: under Cloudflare + Caddy it names the true
80 + // client, while rightmost-XFF names the Cloudflare edge. See the
81 + // module docs.
82 + if let Some(client) = cf_connecting_ip(req.headers()) {
83 + return Ok(client);
84 + }
85 + if let Some(client) = rightmost_xff(req.headers()) {
86 + return Ok(client);
87 + }
62 88 }
63 89 Ok(peer)
64 90 }
65 91 }
66 92
93 + /// The client address Cloudflare observed, from `CF-Connecting-IP`.
94 + ///
95 + /// Cloudflare sets exactly one address here (not a chain), replacing any value
96 + /// the client sent, so there is no leftmost/rightmost question to get wrong.
97 + fn cf_connecting_ip(headers: &HeaderMap) -> Option<IpAddr> {
98 + headers
99 + .get("cf-connecting-ip")
100 + .and_then(|hv| hv.to_str().ok())
101 + .and_then(|s| s.trim().parse::<IpAddr>().ok())
102 + }
103 +
67 104 /// The rightmost parseable `X-Forwarded-For` entry, the address the trusted
68 105 /// proxy observed connecting to it. Taking the *rightmost* entry (not the
69 106 /// leftmost) means a client-injected hop to the left of the proxy's appended
@@ -81,10 +118,17 @@
81 118 use super::*;
82 119
83 120 fn req(peer: Option<&str>, xff: Option<&str>) -> Request<()> {
121 + req_cf(peer, xff, None)
122 + }
123 +
124 + fn req_cf(peer: Option<&str>, xff: Option<&str>, cf: Option<&str>) -> Request<()> {
84 125 let mut b = Request::builder();
85 126 if let Some(xff) = xff {
86 127 b = b.header("x-forwarded-for", xff);
87 128 }
129 + if let Some(cf) = cf {
130 + b = b.header("cf-connecting-ip", cf);
131 + }
88 132 let mut req = b.body(()).unwrap();
89 133 if let Some(peer) = peer {
90 134 let addr: SocketAddr = format!("{peer}:1234").parse().unwrap();
@@ -152,6 +196,52 @@
152 196 assert_eq!(key, "203.0.113.7".parse::<IpAddr>().unwrap());
153 197 }
154 198
199 + #[test]
200 + fn cf_connecting_ip_beats_xff_for_a_trusted_peer() {
201 + // The prod shape: Cloudflare appended the real client to XFF, then Caddy
202 + // appended the Cloudflare edge. Rightmost-XFF would key on the edge and
203 + // bucket every visitor behind it together; CF-Connecting-IP is the client.
204 + let key = extractor(&["127.0.0.1"])
205 + .extract(&req_cf(
206 + Some("127.0.0.1"),
207 + Some("203.0.113.7, 172.68.1.1"),
208 + Some("203.0.113.7"),
209 + ))
210 + .unwrap();
211 + assert_eq!(key, "203.0.113.7".parse::<IpAddr>().unwrap());
212 + }
213 +
214 + #[test]
215 + fn untrusted_peer_ignores_cf_connecting_ip() {
216 + // A direct client forging the Cloudflare header still keys on itself.
217 + let key = extractor(&["127.0.0.1"])
218 + .extract(&req_cf(Some("198.51.100.9"), None, Some("203.0.113.7")))
219 + .unwrap();
220 + assert_eq!(key, "198.51.100.9".parse::<IpAddr>().unwrap());
221 + }
222 +
223 + #[test]
224 + fn trusted_peer_garbage_cf_header_falls_back_to_xff() {
225 + // An unparseable CF header must not shadow a usable XFF chain.
226 + let key = extractor(&["127.0.0.1"])
227 + .extract(&req_cf(
228 + Some("127.0.0.1"),
229 + Some("203.0.113.7"),
230 + Some("not-an-ip"),
231 + ))
232 + .unwrap();
233 + assert_eq!(key, "203.0.113.7".parse::<IpAddr>().unwrap());
234 + }
235 +
236 + #[test]
237 + fn trusted_peer_cf_header_alone_is_enough() {
238 + // Caddy can be configured to strip XFF; CF-Connecting-IP still keys.
239 + let key = extractor(&["127.0.0.1"])
240 + .extract(&req_cf(Some("127.0.0.1"), None, Some("203.0.113.7")))
241 + .unwrap();
242 + assert_eq!(key, "203.0.113.7".parse::<IpAddr>().unwrap());
243 + }
244 +
155 245 #[test]
156 246 fn no_peer_errors() {
157 247 assert!(
M server/src/deploy_lint.rs +32 -16
@@ -8,8 +8,9 @@
8 8 //! that proxies the app must declare a safe IP-trust posture, and nothing
9 9 //! stopped a new block from forgetting.
10 10 //!
11 - //! This lint turns that forgetting into a build failure. Every top-level Caddy block
12 - //! that reverse-proxies the app (`localhost:3000`) must EITHER:
11 + //! This lint turns that forgetting into a build failure. Every top-level Caddy
12 + //! block that reverse-proxies an app which trusts `CF-Connecting-IP` (see
13 + //! [`APP_UPSTREAMS`]) must EITHER:
13 14 //! - `import cloudflare_tls`, the request can only arrive via Cloudflare
14 15 //! mTLS, which sets `CF-Connecting-IP` to the true client; or
15 16 //! - set `CF-Connecting-IP` itself via `header_up CF-Connecting-IP <value>`;
@@ -18,11 +19,16 @@
18 19 //! A block that does neither would let a client forge `CF-Connecting-IP`, so the
19 20 //! lint fails and names the block.
20 21
21 - /// The app's reverse-proxy upstream port. Matched host-agnostically (on a
22 - /// `reverse_proxy` line) so `localhost:3000`, `127.0.0.1:3000`, or a bare
23 - /// `:3000` upstream are all recognized, a host rewrite can't slip a new
24 - /// app-proxy block past the lint.
25 - const APP_UPSTREAM: &str = ":3000";
22 + /// The reverse-proxy upstream ports of every app that trusts `CF-Connecting-IP`
23 + /// for client identity: `:3000` is the MNW server, `:3400` is Multithreaded
24 + /// (`MNW/multithreaded/src/trusted_proxy.rs`, which prefers `CF-Connecting-IP`
25 + /// from a trusted peer because rightmost-`X-Forwarded-For` names the Cloudflare
26 + /// edge under this two-hop deployment, not the client).
27 + ///
28 + /// Matched host-agnostically (on a `reverse_proxy` line) so `localhost:3000`,
29 + /// `127.0.0.1:3000`, or a bare `:3000` upstream are all recognized, a host
30 + /// rewrite can't slip a new app-proxy block past the lint.
31 + const APP_UPSTREAMS: &[&str] = &[":3000", ":3400"];
26 32
27 33 /// The Caddyfile, embedded at compile time relative to the crate root so the
28 34 /// test does not depend on the working directory.
@@ -76,13 +82,14 @@
76 82 blocks
77 83 }
78 84
79 - /// Whether a block body actually reverse-proxies the app. Matches a
85 + /// Whether a block body actually reverse-proxies one of the apps. Matches a
80 86 /// `reverse_proxy ... localhost:3000` directive line, NOT an incidental mention
81 87 /// of the upstream (e.g. the `on_demand_tls ask http://localhost:3000/...` URL
82 88 /// in the global options block, which is not a proxy).
83 89 fn proxies_app(body: &str) -> bool {
84 - body.lines()
85 - .any(|line| line.contains("reverse_proxy") && line.contains(APP_UPSTREAM))
90 + body.lines().any(|line| {
91 + line.contains("reverse_proxy") && APP_UPSTREAMS.iter().any(|up| line.contains(up))
92 + })
86 93 }
87 94
88 95 /// Whether a block body declares a safe IP-trust posture (see module docs).
@@ -116,7 +123,7 @@
116 123 proxying += 1;
117 124 assert!(
118 125 has_safe_ip_posture(body),
119 - "Caddy block `{}` reverse-proxies the app ({APP_UPSTREAM}) but neither \
126 + "Caddy block `{}` reverse-proxies an app ({APP_UPSTREAMS:?}) but neither \
120 127 `import cloudflare_tls` (mTLS) nor sets `CF-Connecting-IP` via `header_up`. \
121 128 It would trust a client-forged source IP, defeating rate limits, lockouts, \
122 129 and audit-log IP attribution. Add one of the two postures (see \
@@ -130,11 +137,14 @@
130 137 }
131 138
132 139 // Guard against a silently-matching parser (file moved/renamed, upstream
133 - // port changed): there must be at least one app-proxy block to check.
140 + // port changed): one block per upstream at minimum, the MNW apex and the
141 + // Multithreaded forum vhost. A lower count means a block stopped
142 + // matching, which drops it from the lint rather than failing it.
134 143 assert!(
135 - proxying >= 1,
136 - "deploy lint found no Caddy block proxying {APP_UPSTREAM}; the parser or the \
137 - Caddyfile layout changed, fix the lint, do not delete it."
144 + proxying >= APP_UPSTREAMS.len(),
145 + "deploy lint found {proxying} Caddy blocks proxying {APP_UPSTREAMS:?}, expected at \
146 + least {}; the parser or the Caddyfile layout changed, fix the lint, do not delete it.",
147 + APP_UPSTREAMS.len()
138 148 );
139 149 }
140 150
@@ -164,7 +174,13 @@
164 174 ));
165 175 // Host-agnostic: a 127.0.0.1 (or bare-port) rewrite is still detected.
166 176 assert!(proxies_app("reverse_proxy 127.0.0.1:3000\n"));
167 - assert!(!proxies_app("reverse_proxy localhost:3400\n"));
177 + // Multithreaded's upstream counts too: it trusts CF-Connecting-IP the
178 + // same way, so its vhost owes the same posture.
179 + assert!(proxies_app("reverse_proxy localhost:3400\n"));
180 + // A non-app upstream (the object-storage CDN) is not linted.
181 + assert!(!proxies_app(
182 + "reverse_proxy https://fsn1.your-objectstorage.com\n"
183 + ));
168 184 }
169 185
170 186 #[test]