HIGH #280: rate limiter bypassable via X-Forwarded-For #287

Closed
fen wants to merge 1 commits from fix-280 into dev
Collaborator

Fix attempt 1. Ref #280.

The deployed ingress does not append the peer address to X-Forwarded-For, so the rightmost XFF entry was fully client-controlled; rotating the header gave a fresh rate-limit bucket per request (pentest: 8 create requests -> 6x201).

Changes:

  • clientIP() keys buckets on the peer address (RemoteAddr) by default and never trusts client-supplied headers (XFF or X-Real-Ip).
  • Per-client granularity restored opt-in: PALETTE_CLIENT_IP_HEADER is honored only when the immediate peer is inside PALETTE_TRUSTED_PROXIES (default loopback + RFC1918) and the value parses as an IP. For prod this can be set to CF-Connecting-IP once the proxy chain is pinned (Traefik forwardedHeaders.trustedIPs = Cloudflare ranges, or origin firewall locked to CF) - owner decision, flagged on the issue.
  • Tests: rotating-XFF repro now shares the peer bucket (5 allowed then 429); trusted-header, untrusted-peer, bad-value, custom-CIDR and hostOnly cases.

Verified locally: go build/test green; live repro 201x5 then 429, no-header requests share the same bucket.

Fix attempt 1. Ref #280. The deployed ingress does not append the peer address to X-Forwarded-For, so the rightmost XFF entry was fully client-controlled; rotating the header gave a fresh rate-limit bucket per request (pentest: 8 create requests -> 6x201). Changes: - clientIP() keys buckets on the peer address (RemoteAddr) by default and never trusts client-supplied headers (XFF or X-Real-Ip). - Per-client granularity restored opt-in: PALETTE_CLIENT_IP_HEADER is honored only when the immediate peer is inside PALETTE_TRUSTED_PROXIES (default loopback + RFC1918) and the value parses as an IP. For prod this can be set to CF-Connecting-IP once the proxy chain is pinned (Traefik forwardedHeaders.trustedIPs = Cloudflare ranges, or origin firewall locked to CF) - owner decision, flagged on the issue. - Tests: rotating-XFF repro now shares the peer bucket (5 allowed then 429); trusted-header, untrusted-peer, bad-value, custom-CIDR and hostOnly cases. Verified locally: go build/test green; live repro 201x5 then 429, no-header requests share the same bucket.
fen added 1 commit 2026-09-18 01:35:20 +00:00
fix #280: key rate-limit buckets on peer address, not client XFF
CI / docker (pull_request) Skipped
CI / test (pull_request) Successful in 29s
a0b34378a4
The deployed ingress does not append the peer to X-Forwarded-For, so the
rightmost XFF entry was fully client-controlled and rotating the header
gave a fresh rate-limit bucket per request (pentest: 8 creates, 6x201).

clientIP() now defaults to the peer (RemoteAddr) and never trusts
client-supplied headers. Per-client granularity is restored opt-in via
PALETTE_CLIENT_IP_HEADER, honored only when the immediate peer is inside
PALETTE_TRUSTED_PROXIES (default loopback + RFC1918) and the value
parses as an IP.
fen closed this pull request 2026-09-18 01:37:12 +00:00
fen deleted branch fix-280 2026-09-18 01:37:12 +00:00
Author
Collaborator

Closing as superseded. PR #284 (same scope: rate-limit buckets keyed on peer address instead of client-controlled XFF) was merged to dev (eca8533) and verified live on palette-dev with the rotating-XFF repro now returning 429s; dev tip carries the fix in internal/api/clientip.go plus TestRotatingXFFDoesNotResetBucket. Issue #280 is open under Owner Review with one flagged wiring gap (PALETTE_TRUSTED_IP_HEADER not read in cmd/palette). Note for that decision: this PR's alternative design (PALETTE_CLIENT_IP_HEADER gated on PALETTE_TRUSTED_PROXIES, default loopback+RFC1918) remains fetchable via refs/pull/287/head if the owner prefers it for the proxy-chain pinning.

Closing as superseded. PR #284 (same scope: rate-limit buckets keyed on peer address instead of client-controlled XFF) was merged to dev (eca8533) and verified live on palette-dev with the rotating-XFF repro now returning 429s; dev tip carries the fix in internal/api/clientip.go plus TestRotatingXFFDoesNotResetBucket. Issue #280 is open under Owner Review with one flagged wiring gap (PALETTE_TRUSTED_IP_HEADER not read in cmd/palette). Note for that decision: this PR's alternative design (PALETTE_CLIENT_IP_HEADER gated on PALETTE_TRUSTED_PROXIES, default loopback+RFC1918) remains fetchable via refs/pull/287/head if the owner prefers it for the proxy-chain pinning.

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.