TRUSTED_PROXY_CIDRS: per-IP rate limits that work behind a proxy

CHANGELOG.md

@@ -24,6 +24,23 @@ exact tag (`ghcr.io/calnode/calnode:0.1.0`) if you need stability between upgrad
   copied price is how a paid meeting quietly starts selling for nothing. Bookings are not
   copied.
 
+- **`TRUSTED_PROXY_CIDRS`: per-IP rate limits that work behind a CDN.** Rate limits key
+  on the TCP peer, which is right for a directly-reachable instance and useless behind a
+  fronting CDN, where every visitor arrives from the same handful of addresses and shares
+  one bucket. List the networks you control, a fronting CDN's own ranges included, and
+  the client IP is taken from `X-Forwarded-For` walked right to left past those hops.
+
+  Nothing changes if you do not set it: a header from a peer you have not listed is still
+  not read at all, because it is a value the client chose. Within the header the *leftmost*
+  entry is likewise client-chosen, so the walk stops at the rightmost address one of your
+  proxies actually observed, and a malformed hop ends the walk on the peer rather than
+  being stepped over. Repeated `X-Forwarded-For` field lines are joined in order rather
+  than only the first being read, so a client's own line in front of a proxy that adds a
+  second one cannot hide the hop that matters. Single-value vendor headers (`CF-Connecting-IP`, `X-Real-IP`,
+  `True-Client-IP`) are never read, from any peer: the setting names networks rather than
+  CDNs, and a plain reverse proxy in the list forwards whatever the client sent.
+
+
 ## [0.8.0] - 2026-09-03
 
 ### Added

DEPLOY.md

@@ -35,6 +35,7 @@ This guide covers a generic Docker deploy and a step-by-step **Railway** deploy
 | `GOOGLE_CLIENT_ID` / `GOOGLE_CLIENT_SECRET` | no | — | Google sign-in + calendar. Can also be set in Settings → Google OAuth. |
 | `LITESTREAM_REPLICA_URL` | recommended | — | Enables continuous SQLite backup (see §6). |
 | `COOKIE_SECURE` | no | https→true | Override cookie Secure flag; defaults from `BASE_URL` scheme. |
+| `TRUSTED_PROXY_CIDRS` | no | — | Comma-separated CIDRs (a bare address = one host) whose `X-Forwarded-For` is believed when keying per-IP rate limits, e.g. `10.0.0.0/8`. Include a fronting CDN's own ranges so the walk steps over its edge and lands on the visitor. Unset ⇒ the header is ignored and the limit keys on the TCP peer, so behind a CDN every visitor shares one bucket. **Only list networks you control**: anything in the list can name any client IP it likes. Single-value vendor headers (`CF-Connecting-IP`, `X-Real-IP`) are never read, from any peer. |
 | `LOG_LEVEL` | no | `info` | `debug`/`info`/`warn`/`error`. |
 
 ¹ Email is optional to boot, but bookings won't send confirmations until SMTP is configured (env **or** the admin UI). Precedence is **env var > DB setting > default**.

audit/claims.yaml

@@ -276,25 +276,38 @@ claims:
 
   - id: rate-limit-keys-on-tcp-source-address
     claim: >
-      Per-IP rate limiting (internal/server/middleware.go's RateLimit) keys
-      strictly on the TCP-level remote address of the connection, never on client-
-      supplied X-Forwarded-For/X-Real-IP headers — a client cannot spoof those
-      headers to evade or split its rate-limit bucket.
+      Per-IP rate limiting (internal/server/middleware.go's RateLimit) keys on the
+      TCP-level remote address of the connection, and never on a client-supplied
+      X-Forwarded-For unless the peer that sent it is inside an operator-configured
+      TRUSTED_PROXY_CIDRS range — so a client cannot spoof headers to evade or split
+      its rate-limit bucket. Single-value vendor headers (X-Real-IP,
+      CF-Connecting-IP, True-Client-IP) are never read, from any peer.
     verify:
       - "internal/server/middleware.go's remoteIP — net.SplitHostPort(r.RemoteAddr)
-         only; the proxy headers are never read."
+         via peerIP, unless TrustClientIP has resolved a client IP into the request
+         context, which it only does for a peer matching a trusted CIDR."
+      - "internal/server/middleware.go's resolveClientIP — returns the peer outright
+         for an untrusted peer, before any header is read."
       - "internal/server/ratelimit_test.go's TestRemoteIP_* — assert X-Forwarded-For
          and X-Real-IP are ignored even when RemoteAddr is loopback."
+      - "internal/server/trustedproxy_test.go — asserts an untrusted peer's spoofed
+         headers are ignored, that the X-Forwarded-For walk goes right-to-left past
+         trusted hops (never the client-seeded leftmost entry), that a malformed
+         header falls back to the peer, and that a vendor header is ignored even from
+         a TRUSTED peer."
     status: verified
     caveat: >
       Recorded here because a prior Layer 2 audit pass flagged this as trusting
       spoofable proxy headers — it doesn't; the flagged behavior traced back to a
       stale doc comment describing the opposite of what the code does (fixed
-      alongside this entry). Correct behavior does require the deployment's
-      reverse proxy to connect to Calnode directly (or over a trusted private
-      network) — see the deployment docs for reverse-proxy requirements (forward
-      the original Host header, connect over a trusted path, strip client-supplied
-      proxy headers at the edge).
+      alongside this entry). TRUSTED_PROXY_CIDRS is empty by default, so an
+      unconfigured instance behaves exactly as this claim originally described.
+      Anything an operator does list can name any client IP it likes — that is what
+      trusting a proxy means — so the list must hold only networks they control.
+      Correct behavior otherwise requires the deployment's reverse proxy to connect
+      to Calnode directly (or over a trusted private network) — see the deployment
+      docs for reverse-proxy requirements (forward the original Host header, connect
+      over a trusted path, strip client-supplied proxy headers at the edge).
 
   - id: caldav-connect-self-service-no-admin-gate
     claim: >

docs/ARCHITECTURE.md

@@ -694,10 +694,31 @@ as the desired state:
   **original `Host` header**. The CSRF same-origin check (§6) compares the request's
   `Origin`/`Referer` against `Host`, so a proxy that rewrites Host would *false-block
   admin writes* (403). Fly and Railway preserve Host by default; a hand-rolled nginx
-  needs `proxy_set_header Host $host;`. Related: per-IP rate limits (§8) key on the
-  **TCP remote address** (proxy headers like `X-Forwarded-For` are intentionally
-  ignored as forgeable), so behind a shared proxy the limit keys on the proxy's
-  connection — fine for per-instance Fly/Railway, worth knowing for a fronting proxy.
+  needs `proxy_set_header Host $host;`.
+- **Per-IP rate limits (§8) key on the TCP remote address by default**, and
+  `X-Forwarded-For` / `X-Real-IP` / `CF-Connecting-IP` are not read at all. That is not
+  an oversight: those headers are client-chosen values, so believing them unconditionally
+  would let anyone split their own rate-limit bucket by sending a different one each
+  request. Behind a shared proxy the limit therefore keys on the proxy's connection —
+  fine for a per-instance Fly/Railway deploy, worth knowing for a fronting CDN.
+  **`TRUSTED_PROXY_CIDRS`** (comma-separated CIDRs, a bare address meaning one host)
+  opts in per network: for a peer inside one of those ranges, `TrustClientIP`
+  (`internal/server/middleware.go`) resolves the client IP by walking `X-Forwarded-For`
+  **right to left past trusted hops** and taking the first untrusted address, else the
+  peer. Single-value vendor headers (`CF-Connecting-IP`, `X-Real-IP`, `True-Client-IP`)
+  are never consulted, even from a trusted peer: the list names *networks*, not CDNs, so
+  an ordinary reverse proxy in it forwards whatever the client sent, and the header
+  itself carries nothing that says which hop wrote it. A CDN sets `X-Forwarded-For` too
+  and its own ranges belong in the list, so the walk reaches the same visitor without
+  believing a header for a reason the code cannot check. ⛔ Not the leftmost entry: the left
+  of that header is whatever the original client sent, and every well-behaved proxy
+  preserves it. A hop that does not parse ends the walk and falls back to the peer rather
+  than being skipped, so one malformed entry cannot push the walk onto a value the client
+  chose. `X-Forwarded-For` is read as **every** field line joined in order, not just the
+  first, because a client's own line in front of a proxy that adds a second one would
+  otherwise be the only thing the walk saw. Headers from an **untrusted** peer are never read, which is what keeps the
+  default un-weakenable by a header. Resolution happens once, in the outermost
+  middleware, and is carried in the request context.
 
 ---
 

internal/config/config.go

@@ -59,6 +59,14 @@ type Config struct {
 	// the public endpoints are rate-limited regardless. Comma-separated.
 	EmbedAllowedOrigins []string
 
+	// TrustedProxyCIDRs lists the networks whose forwarded headers are believed when
+	// resolving the client IP for per-IP rate limiting. Empty (the default) ⇒ the limit
+	// keys on the TCP peer and X-Forwarded-For is ignored entirely, because a header
+	// from an unvetted peer is a client-chosen value. Comma-separated CIDRs; a bare
+	// address is taken as a single host. A fronting CDN's own ranges belong here: the
+	// walk steps over its edge address and lands on the visitor.
+	TrustedProxyCIDRs []string
+
 	// DemoMode turns this instance into a public, self-resetting demo: seeds sample
 	// data on every boot (there's no persistent volume, so every boot is a fresh DB),
 	// disables calendar/Zoom connect, serves a disallow-all robots.txt, and exposes
@@ -97,6 +105,7 @@ func Load() *Config {
 
 		EmbedAllowedOrigins: splitCSV(getEnv("EMBED_ALLOWED_ORIGINS", "")),
 		DataDir:             getEnv("DATA_DIR", "data"),
+		TrustedProxyCIDRs:   splitCSV(getEnv("TRUSTED_PROXY_CIDRS", "")),
 	}
 
 	cfg.EncryptionKey = os.Getenv("CALNODE_ENCRYPTION_KEY")

internal/server/middleware.go

@@ -18,6 +18,10 @@ type contextKey string
 
 const requestIDKey contextKey = "request_id"
 
+// clientIPKey carries the client IP resolved by TrustClientIP. Absent unless
+// TRUSTED_PROXY_CIDRS is configured, in which case remoteIP falls back to the peer.
+const clientIPKey contextKey = "client_ip"
+
 func RequestID(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		id := r.Header.Get("X-Request-Id")
@@ -236,18 +240,162 @@ func (rl *rateLimiter) cleanup() {
 	}
 }
 
-// remoteIP returns the TCP-level remote address, stripped of its port.
-// X-Real-IP and X-Forwarded-For are intentionally ignored: without a
-// configured trusted-proxy allowlist, those headers can be forged by any
-// client and would bypass the rate limit entirely. Operators behind a reverse
-// proxy should strip proxy headers at the proxy level and rely on the TCP
-// address the proxy connects with. See audit/claims.yaml's
-// rate-limit-keys-on-tcp-source-address claim, recorded specifically because a
-// prior Layer 2 audit pass mistook this deliberate behavior for a spoofable gap.
+// remoteIP returns the IP a per-IP limit keys on.
+//
+// By default that is the TCP-level remote address, stripped of its port, and the
+// forwarded headers are ignored: without a configured trusted-proxy allowlist those
+// headers can be forged by any client and would bypass the rate limit entirely.
+// Operators behind a reverse proxy should strip proxy headers at the proxy and rely on
+// the TCP address the proxy connects with. See audit/claims.yaml's
+// rate-limit-keys-on-tcp-source-address claim, recorded specifically because a prior
+// Layer 2 audit pass mistook this deliberate behavior for a spoofable gap.
+//
+// When TRUSTED_PROXY_CIDRS is set, TrustClientIP has already resolved the client IP for
+// this request and left it in the context — see resolveClientIP for what that means and
+// what it deliberately does not do. This function reads that value when it is there, so
+// the untrusted-peer path stays byte-for-byte the old behaviour.
 func remoteIP(r *http.Request) string {
+	if ip, ok := r.Context().Value(clientIPKey).(string); ok && ip != "" {
+		return ip
+	}
+	return peerIP(r)
+}
+
+// peerIP returns the TCP-level remote address, stripped of its port. This is the only
+// value in a request that a client cannot choose.
+func peerIP(r *http.Request) string {
 	host, _, err := net.SplitHostPort(r.RemoteAddr)
 	if err != nil {
 		return r.RemoteAddr
 	}
 	return host
 }
+
+// ParseTrustedProxies parses TRUSTED_PROXY_CIDRS entries. A bare address is accepted and
+// treated as a single-host range (/32 or /128), because "10.0.0.7" is what an operator
+// naming one proxy will write.
+//
+// Every parseable entry is returned even when others fail, and the error names the ones
+// that did not: one typo should cost that hop's trust, not the whole list's. The caller
+// is expected to log the error — an unparsed entry means that proxy's headers are NOT
+// believed, which degrades to per-proxy rate limiting rather than to trusting a forgery.
+func ParseTrustedProxies(entries []string) ([]*net.IPNet, error) {
+	var nets []*net.IPNet
+	var bad []string
+	for _, e := range entries {
+		e = strings.TrimSpace(e)
+		if e == "" {
+			continue
+		}
+		if _, n, err := net.ParseCIDR(e); err == nil {
+			nets = append(nets, n)
+			continue
+		}
+		if ip := net.ParseIP(e); ip != nil {
+			bits := 32
+			if ip.To4() == nil {
+				bits = 128
+			}
+			nets = append(nets, &net.IPNet{IP: ip, Mask: net.CIDRMask(bits, bits)})
+			continue
+		}
+		bad = append(bad, e)
+	}
+	if len(bad) > 0 {
+		return nets, fmt.Errorf("not a CIDR or IP address: %s", strings.Join(bad, ", "))
+	}
+	return nets, nil
+}
+
+// TrustClientIP returns middleware that resolves the client IP once per request and
+// stores it in the request context for remoteIP to read. With no trusted proxies it is a
+// pass-through, so an instance that has not configured any is unchanged.
+//
+// It is middleware rather than a package-level setting because the trust list belongs to
+// one server instance: a mutable global would leak between instances in a test binary
+// and would make "which requests are affected" unanswerable from the wiring.
+func TrustClientIP(trusted []*net.IPNet) func(http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		if len(trusted) == 0 {
+			return next
+		}
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			ctx := context.WithValue(r.Context(), clientIPKey, resolveClientIP(r, trusted))
+			next.ServeHTTP(w, r.WithContext(ctx))
+		})
+	}
+}
+
+// resolveClientIP applies the forwarded headers, but only for a peer inside a trusted
+// CIDR. A header from an untrusted peer is never read at all — that is the whole point,
+// and it is why the default (no trusted proxies) cannot be weakened by a header.
+//
+// For a trusted peer the answer comes from X-Forwarded-For, walked RIGHT TO LEFT past
+// trusted hops, returning the first untrusted address. ⛔ Not the leftmost entry: the
+// left of that header is whatever the original client sent, so a client that pre-seeds
+// "X-Forwarded-For: 1.2.3.4" gets it prepended and preserved by every well-behaved
+// proxy. The rightmost non-trusted hop is the last address a trusted proxy actually
+// observed, which is the only one in the header that anything vouched for. The peer is
+// the answer whenever the header is absent or unusable.
+//
+// ⛔ Single-value vendor headers (CF-Connecting-IP, X-Real-IP, True-Client-IP) are NOT
+// consulted, and that is deliberate rather than an omission. Trusting one means
+// trusting it from every peer in the list, and the list is a list of *networks* rather
+// than a list of CDNs: an ordinary reverse proxy inside it forwards whatever headers
+// the client sent, so a client could name its own rate-limit bucket by sending one. The
+// header carries nothing that says which hop set it, so there is no way to tell the
+// value a CDN wrote from the value a visitor typed.
+//
+// Nothing is lost by leaving them out. A CDN sets X-Forwarded-For as well, and its own
+// ranges belong in TRUSTED_PROXY_CIDRS anyway, so the walk steps over its edge address
+// and lands on the visitor. That is the same answer the vendor header would have given,
+// reached without having to believe a header for a reason the code cannot check.
+//
+// A hop that does not parse ends the walk and falls back to the peer rather than being
+// skipped. Skipping it would let a client inject one malformed entry to push the walk
+// past the real hop and onto a value it chose. Note this also means a proxy that appends
+// "ip:port" (rather than a bare address) reads as malformed and lands on the peer, which
+// is the safe direction to be wrong in.
+func resolveClientIP(r *http.Request, trusted []*net.IPNet) string {
+	peer := peerIP(r)
+	if !ipInAny(peer, trusted) {
+		return peer
+	}
+
+	// ⛔ Values, not Get. A header may arrive as several field lines, and Get returns
+	// only the FIRST. A client that sends its own "X-Forwarded-For: 1.2.3.4" followed by
+	// a proxy that ADDS a line rather than appending to the existing one leaves two
+	// lines, and Get would hand the walk the client's line alone — no trusted hop in it,
+	// so the walk returns the client's chosen address on its first step. RFC 9110 says
+	// repeated field lines are equivalent to one comma-joined value in the order
+	// received, so joining them is both correct and what the walk already assumes.
+	if xff := strings.Join(r.Header.Values("X-Forwarded-For"), ","); xff != "" {
+		hops := strings.Split(xff, ",")
+		for i := len(hops) - 1; i >= 0; i-- {
+			ip := net.ParseIP(strings.TrimSpace(hops[i]))
+			if ip == nil {
+				break
+			}
+			if ipInAny(ip.String(), trusted) {
+				continue // a trusted hop of our own; keep walking left
+			}
+			return ip.String()
+		}
+	}
+
+	return peer
+}
+
+// ipInAny reports whether ip (a textual address) falls inside any of nets.
+func ipInAny(ip string, nets []*net.IPNet) bool {
+	parsed := net.ParseIP(ip)
+	if parsed == nil {
+		return false
+	}
+	for _, n := range nets {
+		if n.Contains(parsed) {
+			return true
+		}
+	}
+	return false
+}

internal/server/server.go

@@ -544,7 +544,19 @@ func New(ctx context.Context, cfg *config.Config, db *sql.DB, logger *slog.Logge
 	// permanently if a marketing landing page is ever added here.
 	mux.Handle("GET /{$}", http.RedirectHandler("/admin/", http.StatusFound))
 
-	return RequestID(Logging(logger, SameOriginCheck(mux))), drain
+	// Trusted-proxy resolution wraps everything, so the per-IP limiters and anything else
+	// asking for the client IP see one answer computed once. A bad CIDR is logged and
+	// dropped rather than fatal: the consequence is that that hop's headers are not
+	// believed, which costs shared rate-limit buckets, never a trusted forgery.
+	trustedProxies, err := ParseTrustedProxies(cfg.TrustedProxyCIDRs)
+	if err != nil {
+		logger.Error("TRUSTED_PROXY_CIDRS: ignoring unparseable entries", "error", err)
+	}
+	if len(trustedProxies) > 0 {
+		logger.Info("trusting forwarded headers from proxies", "cidrs", cfg.TrustedProxyCIDRs)
+	}
+
+	return TrustClientIP(trustedProxies)(RequestID(Logging(logger, SameOriginCheck(mux)))), drain
 }
 
 // seedSMTPToDB writes env-var SMTP settings into the DB on first boot so they