poslop/palette
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases 2 Wiki Activity

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

Closed
fen wants to merge 1 commits from fix-280 into dev
pull from: fix-280
merge into: :dev
Conversation 1 Commits 1 Files Changed 2
+166 -75
2 changed files with 166 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/api/ratelimit.go
+61 -20
View File
@@ -1,7 +1,9 @@
package api
import (
"net"
"net/http"
"os"
"strconv"
"strings"
"sync"
@@ -48,33 +50,72 @@ func (l *limiter) allow(key string, rate, burst float64) bool {
return true
}
// clientIP extracts the client IP for rate-limit keying (#85).
// clientIP extracts the client IP for rate-limit keying (#85, #280).
//
// Trust boundary: palette runs behind exactly ONE trusted reverse proxy
// (Traefik in the k3s pod network). Traefik APPENDS the real client IP to
// X-Forwarded-For, so the RIGHTMOST entry is the last value the trusted
// proxy observed and is unspoofable by the client (a client-supplied fake
// entry only lands on the LEFT and is ignored). This matches chi's
// middleware.RealIP semantics for a single trusted proxy hop.
// Trust boundary (revised in #280): the deployed ingress does NOT append the
// peer address to X-Forwarded-For, so every XFF entry (and X-Real-Ip) is
// client-controlled. Keying on any client-supplied header lets an attacker
// rotate the header per request and get a fresh rate-limit bucket every
// time. The bucket key therefore defaults to the PEER address (RemoteAddr),
// which the client cannot influence.
//
// Direct connections (no XFF header) fall back to RemoteAddr. Directly
// reachable deployments must NOT expose the app to untrusted networks
// without a proxy in front, or attackers could forge the rightmost entry.
// Deployments that pin their proxy chain can restore per-client granularity:
// set PALETTE_CLIENT_IP_HEADER (e.g. "CF-Connecting-IP" once the proxy chain
// is pinned, e.g. Traefik forwardedHeaders.trustedIPs limited to Cloudflare
// ranges or an origin firewall locked to CF) and the header value is honored
// ONLY when the immediate peer (RemoteAddr) is inside PALETTE_TRUSTED_PROXIES
// (default: loopback + RFC1918 private ranges, i.e. the in-cluster Traefik
// hop). A public peer never triggers header trust, and the header value must
// parse as an IP.
func clientIP(r *http.Request) string {
if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
if i := strings.LastIndex(xff, ","); i >= 0 {
return strings.TrimSpace(xff[i+1:])
peer := hostOnly(r.RemoteAddr)
if hdr := os.Getenv("PALETTE_CLIENT_IP_HEADER"); hdr != "" && trustedProxy(peer) {
if v := strings.TrimSpace(r.Header.Get(hdr)); net.ParseIP(v) != nil {
return v
}
return strings.TrimSpace(xff)
}
if xr := r.Header.Get("X-Real-Ip"); xr != "" {
return strings.TrimSpace(xr)
return peer
}
host := r.RemoteAddr
if i := strings.LastIndex(host, ":"); i > 0 {
host = host[:i]
// trustedProxy reports whether peer (an IP without port) falls inside any of
// the configured trusted proxy CIDRs (PALETTE_TRUSTED_PROXIES, default
// loopback + RFC1918 private ranges). Parsed once and cached.
func trustedProxy(peer string) bool {
trustedOnce.Do(func() {
spec := os.Getenv("PALETTE_TRUSTED_PROXIES")
if strings.TrimSpace(spec) == "" {
spec = "127.0.0.0/8,::1/128,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
}
return host
for _, c := range strings.Split(spec, ",") {
if _, cidr, err := net.ParseCIDR(strings.TrimSpace(c)); err == nil {
trustedCIDRs = append(trustedCIDRs, cidr)
}
}
})
ip := net.ParseIP(peer)
if ip == nil {
return false
}
for _, cidr := range trustedCIDRs {
if cidr.Contains(ip) {
return true
}
}
return false
}
var (
trustedOnce sync.Once
trustedCIDRs []*net.IPNet
)
// hostOnly strips the port from a host:port address.
func hostOnly(addr string) string {
host, _, err := net.SplitHostPort(addr)
if err != nil {
host = addr
}
return strings.TrimSpace(host)
}
var globalLimiter = newLimiter()
internal/api/ratelimit_xff_test.go
+105 -55
View File
@@ -1,85 +1,135 @@
package api
// Issue #85: the rate limit key must use the rightmost X-Forwarded-For entry
// (appended by the trusted Traefik proxy), never the raw/leftmost header
// value a client can forge. A spoofed FIRST XFF entry must not bypass the
// limit or rotate buckets.
// Issues #85 + #280: the rate limit bucket key must NEVER be derived from a
// client-controlled header. The deployed ingress does not append the peer
// address to X-Forwarded-For, so every XFF entry is attacker-controlled and
// an attacker can rotate the header per request to get a fresh bucket. The
// key defaults to the peer (RemoteAddr); an opt-in trusted header is honored
// only when the immediate peer is a trusted proxy.
import (
"bytes"
"net/http/httptest"
"sync"
"testing"
)
func TestClientIPTakesRightmostXFF(t *testing.T) {
r := httptest.NewRequest("POST", "/", nil)
r.RemoteAddr = "10.42.0.7:51000" // trusted Traefik pod
r.Header.Set("X-Forwarded-For", "1.2.3.4, 1.2.3.5, 203.0.113.9")
if got := clientIP(r); got != "203.0.113.9" {
t.Fatalf("clientIP = %q, want rightmost 203.0.113.9", got)
}
// resetTrustedProxyCache clears the sync.Once-cached CIDR list so each test
// re-parses PALETTE_TRUSTED_PROXIES under its own env.
func resetTrustedProxyCache(t *testing.T) {
t.Helper()
t.Cleanup(func() {
trustedOnce = sync.Once{}
trustedCIDRs = nil
})
trustedOnce = sync.Once{}
trustedCIDRs = nil
}
func TestClientIPXRealIPFallback(t *testing.T) {
r := httptest.NewRequest("POST", "/", nil)
r.RemoteAddr = "10.42.0.7:51000"
r.Header.Set("X-Real-Ip", "203.0.113.10")
if got := clientIP(r); got != "203.0.113.10" {
t.Fatalf("clientIP = %q, want 203.0.113.10", got)
}
}
func TestClientIPDirectFallback(t *testing.T) {
r := httptest.NewRequest("POST", "/", nil)
r.RemoteAddr = "198.51.100.5:51000"
if got := clientIP(r); got != "198.51.100.5" {
t.Fatalf("clientIP = %q, want 198.51.100.5", got)
}
}
// TestRateLimitSpoofedFirstXFFDoesNotBypass: an attacker rotating a fake
// leftmost XFF entry stays limited on their real (rightmost) IP.
func TestRateLimitSpoofedFirstXFFDoesNotBypass(t *testing.T) {
// Rotating XFF must not create fresh buckets: all requests share the peer's
// bucket. This is the #280 pentest repro (8 creates, 6x201 before the fix).
func TestRateLimitRotatingXFFSharesPeerBucket(t *testing.T) {
srv := newTestServer(t)
h := srv.routes()
for i := 0; i < 5; i++ {
codes := []int{}
for i := 0; i < 8; i++ {
req := httptest.NewRequest("POST", "/api/pastes", bytes.NewReader([]byte(`{"content":"hi"}`)))
req.RemoteAddr = "10.42.0.7:51000"
// each request spoofs a DIFFERENT leftmost entry
req.Header.Set("X-Forwarded-For", spoofN(i)+", 203.0.113.9")
req.RemoteAddr = "203.0.113.9:51000" // public peer
req.Header.Set("X-Forwarded-For", "9.9.9."+string(rune('0'+i)))
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
if rr.Code != 201 {
t.Fatalf("req %d: want 201, got %d", i, rr.Code)
codes = append(codes, rr.Code)
}
allowed := 0
for _, c := range codes {
if c == 201 {
allowed++
}
}
// 6th request, still the same real IP, new spoofed prefix: must 429
if allowed > 5 {
t.Fatalf("rotating XFF bypassed the limit: allowed=%d codes=%v", allowed, codes)
}
// and the no-header bucket is the SAME bucket: fully exhausted
req := httptest.NewRequest("POST", "/api/pastes", bytes.NewReader([]byte(`{"content":"hi"}`)))
req.RemoteAddr = "10.42.0.7:51000"
req.Header.Set("X-Forwarded-For", "9.9.9.9, 203.0.113.9")
req.RemoteAddr = "203.0.113.9:51000"
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
if rr.Code != 429 {
t.Fatalf("spoofed 6th req: want 429, got %d", rr.Code)
t.Fatalf("no-header request after rotating XFF: want 429, got %d", rr.Code)
}
}
func spoofN(i int) string {
return "1.2.3." + string(rune('0'+i))
// clientIP unit checks.
func TestClientIPIgnoresClientHeadersByDefault(t *testing.T) {
r := httptest.NewRequest("POST", "/", nil)
r.RemoteAddr = "198.51.100.5:51000"
r.Header.Set("X-Forwarded-For", "1.2.3.4, 5.6.7.8")
r.Header.Set("X-Real-Ip", "9.9.9.9")
if got := clientIP(r); got != "198.51.100.5" {
t.Fatalf("clientIP = %q, want peer 198.51.100.5", got)
}
}
// Distinct real IPs must still get distinct buckets (no over-limiting).
func TestRateLimitDistinctRightmostIPsIndependent(t *testing.T) {
srv := newTestServer(t)
h := srv.routes()
for _, ip := range []string{"203.0.113.20", "203.0.113.21"} {
req := httptest.NewRequest("POST", "/api/pastes", bytes.NewReader([]byte(`{"content":"hi"}`)))
req.RemoteAddr = "10.42.0.7:51000"
req.Header.Set("X-Forwarded-For", "6.6.6.6, "+ip)
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
if rr.Code != 201 {
t.Fatalf("ip %s: want 201, got %d", ip, rr.Code)
// Opt-in trusted header: honored only from a trusted private peer and only
// with a parseable IP value.
func TestClientIPTrustedHeaderFromPrivatePeer(t *testing.T) {
t.Setenv("PALETTE_CLIENT_IP_HEADER", "Cf-Connecting-Ip")
// reset the cached CIDR parse (sync.Once) for a clean run
resetTrustedProxyCache(t)
r := httptest.NewRequest("POST", "/", nil)
r.RemoteAddr = "10.42.0.7:51000" // in-cluster proxy hop
r.Header.Set("Cf-Connecting-Ip", "203.0.113.42")
if got := clientIP(r); got != "203.0.113.42" {
t.Fatalf("clientIP = %q, want 203.0.113.42", got)
}
// same header from a PUBLIC peer must be ignored (peer not trusted)
r2 := httptest.NewRequest("POST", "/", nil)
r2.RemoteAddr = "203.0.113.9:51000"
r2.Header.Set("Cf-Connecting-Ip", "6.6.6.6")
if got := clientIP(r2); got != "203.0.113.9" {
t.Fatalf("clientIP = %q, want peer 203.0.113.9 (untrusted peer)", got)
}
// garbage header value must be ignored
r3 := httptest.NewRequest("POST", "/", nil)
r3.RemoteAddr = "10.42.0.7:51000"
r3.Header.Set("Cf-Connecting-Ip", "not-an-ip; 6.6.6.6")
if got := clientIP(r3); got != "10.42.0.7" {
t.Fatalf("clientIP = %q, want peer 10.42.0.7 (bad header value)", got)
}
}
// Custom trusted proxy CIDR list is honored.
func TestClientIPCustomTrustedProxies(t *testing.T) {
t.Setenv("PALETTE_CLIENT_IP_HEADER", "Cf-Connecting-Ip")
t.Setenv("PALETTE_TRUSTED_PROXIES", "192.168.50.0/24")
resetTrustedProxyCache(t)
r := httptest.NewRequest("POST", "/", nil)
r.RemoteAddr = "192.168.50.10:51000"
r.Header.Set("Cf-Connecting-Ip", "203.0.113.77")
if got := clientIP(r); got != "203.0.113.77" {
t.Fatalf("clientIP = %q, want 203.0.113.77", got)
}
// outside the custom list
r2 := httptest.NewRequest("POST", "/", nil)
r2.RemoteAddr = "10.42.0.7:51000"
r2.Header.Set("Cf-Connecting-Ip", "203.0.113.77")
if got := clientIP(r2); got != "10.42.0.7" {
t.Fatalf("clientIP = %q, want peer 10.42.0.7 (outside custom trusted list)", got)
}
}
// IPv6 peer without port must not be mangled.
func TestHostOnly(t *testing.T) {
cases := map[string]string{
"198.51.100.5:51000": "198.51.100.5",
"[2001:db8::1]:8080": "2001:db8::1",
"198.51.100.5": "198.51.100.5",
}
for in, want := range cases {
if got := hostOnly(in); got != want {
t.Errorf("hostOnly(%q) = %q, want %q", in, got, want)
}
}
}