Fix #280: rate limiter keys buckets on peer address, not client-controlled X-Forwarded-For #284

Merged
fen merged 1 commits from fix-280 into dev 2026-09-18 01:33:04 +00:00
Collaborator

Ref #280

Problem

clientIP() (internal/api/ratelimit.go) keyed rate-limit buckets on the RIGHTMOST X-Forwarded-For entry, assuming traefik appends the real client IP. The deployed ingress does not rewrite XFF, so a client rotating its own XFF header got a fresh bucket per request — pentest evidence: 8 creates with rotating XFF -> 6x201, limits never tripped.

Fix

  • Bucket keys now use the actual peer address (RemoteAddr) by default. All client-supplied IP headers (X-Forwarded-For, X-Real-Ip) are ignored — no header a client controls can influence keying.
  • Per-client limits behind a trusted ingress are restored via an explicit opt-in: PALETTE_TRUSTED_IP_HEADER (e.g. CF-Connecting-IP when Cloudflare fronts traefik, as on prod p.archfox.org — Cloudflare strips any client-supplied value of that header, so it is attacker-proof when set). The header is honored ONLY when explicitly configured at startup; rightmost-XFF-style derivations are gone entirely.

Deployment note (prod)

p.archfox.org sits behind Cloudflare -> traefik. To keep per-client rate limits there, set PALETTE_TRUSTED_IP_HEADER=CF-Connecting-IP on the prod deployment (one env var in the k8s spec — deploy/QA's call, not done here). Unset, the behavior stays safe: all clients share one bucket per endpoint, so the limit is enforceable but coarse.

Tests

  • TestRotatingXFFDoesNotResetBucket: the pentest repro (8 creates, rotating XFF) now yields exactly burst-allowed + rest-429.
  • TestClientIPUsesRemoteAddrNotXFF: XFF/X-Real-Ip ignored, peer address used.
  • TestClientIPTrustedHeaderOnlyWhenConfigured: trusted header honored only when explicitly configured.

go build ./... and go test ./... pass. README env-var table documents PALETTE_TRUSTED_IP_HEADER.

Ref #280 ## Problem `clientIP()` (internal/api/ratelimit.go) keyed rate-limit buckets on the RIGHTMOST X-Forwarded-For entry, assuming traefik appends the real client IP. The deployed ingress does not rewrite XFF, so a client rotating its own XFF header got a fresh bucket per request — pentest evidence: 8 creates with rotating XFF -> 6x201, limits never tripped. ## Fix - Bucket keys now use the actual peer address (`RemoteAddr`) by default. All client-supplied IP headers (X-Forwarded-For, X-Real-Ip) are ignored — no header a client controls can influence keying. - Per-client limits behind a trusted ingress are restored via an explicit opt-in: `PALETTE_TRUSTED_IP_HEADER` (e.g. `CF-Connecting-IP` when Cloudflare fronts traefik, as on prod p.archfox.org — Cloudflare strips any client-supplied value of that header, so it is attacker-proof when set). The header is honored ONLY when explicitly configured at startup; rightmost-XFF-style derivations are gone entirely. ## Deployment note (prod) p.archfox.org sits behind Cloudflare -> traefik. To keep per-client rate limits there, set `PALETTE_TRUSTED_IP_HEADER=CF-Connecting-IP` on the prod deployment (one env var in the k8s spec — deploy/QA's call, not done here). Unset, the behavior stays safe: all clients share one bucket per endpoint, so the limit is enforceable but coarse. ## Tests - `TestRotatingXFFDoesNotResetBucket`: the pentest repro (8 creates, rotating XFF) now yields exactly burst-allowed + rest-429. - `TestClientIPUsesRemoteAddrNotXFF`: XFF/X-Real-Ip ignored, peer address used. - `TestClientIPTrustedHeaderOnlyWhenConfigured`: trusted header honored only when explicitly configured. `go build ./...` and `go test ./...` pass. README env-var table documents `PALETTE_TRUSTED_IP_HEADER`.
fen added 1 commit 2026-09-18 01:30:18 +00:00
Fix rate limiter bypass via client-controlled X-Forwarded-For (#280)
CI / docker (pull_request) Skipped
CI / test (pull_request) Successful in 25s
f63efc6d88
clientIP() keyed rate-limit buckets on the rightmost X-Forwarded-For
entry, assuming traefik appends the real client IP. The deployed ingress
does not rewrite XFF, so rotating the header gave a fresh bucket per
request (pentest H1: 8 creates with rotating XFF -> 6x201).

Now the bucket keys on the actual peer address (RemoteAddr) by default;
every client-supplied IP header is ignored. Deployments whose ingress
overwrites a client-IP header can opt in via PALETTE_TRUSTED_IP_HEADER
(e.g. CF-Connecting-IP behind Cloudflare) to restore per-client limits.

Adds tests: rotating XFF no longer resets the bucket; the trusted header
is honored only when explicitly configured.
fen merged commit eca8533d70 into dev 2026-09-18 01:33:04 +00:00
Sign in to join this conversation.