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

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
Conversation 0 Commits 1 Files Changed 5
+101 -96
5 changed files with 101 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit f63efc6d88 - Show all commits
README.md
+1
View File
@@ -70,6 +70,7 @@ go build -o palette ./cmd/palette
| `PALETTE_ADMIN_KEY` | generated | Admin key; if unset a 32-char hex key is generated and persisted to `<db-dir>/admin-key` (0600) |
| `PALETTE_DEFAULT_DARK` | dark on | Default dark mode for new visitors. Set `false`, `0`, or `off` to default to light mode. Visitors who toggle dark mode keep their choice in their browser. |
| `PALETTE_UNLOCK_SECRET` | random per start | HMAC secret for password-unlock cookies. Set a fixed value to keep unlock sessions across restarts or across replicas. |
| `PALETTE_TRUSTED_IP_HEADER` | unset | Name of a proxy-controlled client-IP header to key API rate limits on (e.g. `CF-Connecting-IP` when Cloudflare is the ingress; Cloudflare strips any client-supplied value). Unset: rate limits key on the peer address only, and all client-supplied IP headers (X-Forwarded-For, X-Real-Ip) are ignored. (#280) |
An `/admin` page exists for runtime settings, protected by a key set at
install (`PALETTE_ADMIN_KEY` env var) and resettable locally. See
internal/api/clientip.go
+57
View File
@@ -0,0 +1,57 @@
// clientIP extracts the client IP for rate-limit keying.
//
// Trust boundary (issue #280): the bucket key MUST NOT come from any header a
// client can influence. The previous rightmost-X-Forwarded-For scheme (#85)
// assumed Traefik appends the real client IP, but the deployed ingress does
// not rewrite XFF, so a client rotating its own XFF value got a fresh bucket
// per request and the limit was unenforceable (pentest H1: 6x201 across 8
// rotating-XFF creates).
//
// Default: key on the actual peer address (RemoteAddr) only. Behind any
// reverse proxy this is the proxy's address, so all clients share one bucket
// per endpoint — coarse, but safe.
//
// Proxy-honoring mode: a deployment in front of a proxy that OVERWRITES (not
// appends to) a client-IP header can set PALETTE_TRUSTED_IP_HEADER (e.g.
// CF-Connecting-IP when Cloudflare is the ingress; Cloudflare strips any
// client-supplied value). The header is honored ONLY when explicitly
// configured at startup, and X-Forwarded-For / X-Real-Ip are never trusted.
package api
import (
"net"
"net/http"
"sync"
)
var (
trustedIPMu sync.RWMutex
trustedIPHeader string // empty = never trust any client-IP header
)
// SetTrustedIPHeader configures the single proxy-controlled header whose
// value may key rate-limit buckets. Called at startup; tests may reset it.
func SetTrustedIPHeader(name string) {
trustedIPMu.Lock()
defer trustedIPMu.Unlock()
trustedIPHeader = name
}
func getTrustedIPHeader() string {
trustedIPMu.RLock()
defer trustedIPMu.RUnlock()
return trustedIPHeader
}
func clientIP(r *http.Request) string {
if name := getTrustedIPHeader(); name != "" {
if v := r.Header.Get(name); v != "" {
return v
}
}
host := r.RemoteAddr
if h, _, err := net.SplitHostPort(r.RemoteAddr); err == nil {
host = h
}
return host
}
internal/api/ratelimit.go
-30
View File
@@ -3,7 +3,6 @@ package api
import (
"net/http"
"strconv"
"strings"
"sync"
"time"
)
@@ -48,35 +47,6 @@ func (l *limiter) allow(key string, rate, burst float64) bool {
return true
}
// clientIP extracts the client IP for rate-limit keying (#85).
//
// 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.
//
// 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.
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:])
}
return strings.TrimSpace(xff)
}
if xr := r.Header.Get("X-Real-Ip"); xr != "" {
return strings.TrimSpace(xr)
}
host := r.RemoteAddr
if i := strings.LastIndex(host, ":"); i > 0 {
host = host[:i]
}
return host
}
var globalLimiter = newLimiter()
// globalSettingsFn is set at startup; tests can point it at fixed settings.
internal/api/ratelimit_xff_test.go
+38 -66
View File
@@ -1,85 +1,57 @@
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.
import (
"bytes"
"fmt"
"net/http/httptest"
"testing"
)
func TestClientIPTakesRightmostXFF(t *testing.T) {
r := httptest.NewRequest("POST", "/", nil)
r.RemoteAddr = "10.42.0.7:51000" // trusted Traefik pod
func TestClientIPUsesRemoteAddrNotXFF(t *testing.T) {
SetTrustedIPHeader("")
defer SetTrustedIPHeader("")
r := httptest.NewRequest("POST", "/api/pastes", nil)
r.RemoteAddr = "203.0.113.7:4432"
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)
}
}
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)
if got := clientIP(r); got != "203.0.113.7" {
t.Fatalf("clientIP = %q, want peer 203.0.113.7", 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)
func TestClientIPTrustedHeaderOnlyWhenConfigured(t *testing.T) {
SetTrustedIPHeader("")
defer SetTrustedIPHeader("")
r := httptest.NewRequest("POST", "/api/pastes", nil)
r.RemoteAddr = "10.0.1.47:9999"
r.Header.Set("CF-Connecting-IP", "198.51.100.9")
if got := clientIP(r); got != "10.0.1.47" {
t.Fatalf("unconfigured: clientIP = %q, want peer 10.0.1.47", got)
}
SetTrustedIPHeader("CF-Connecting-IP")
if got := clientIP(r); got != "198.51.100.9" {
t.Fatalf("configured: clientIP = %q, want CF-Connecting-IP value", got)
}
}
// TestRateLimitSpoofedFirstXFFDoesNotBypass: an attacker rotating a fake
// leftmost XFF entry stays limited on their real (rightmost) IP.
func TestRateLimitSpoofedFirstXFFDoesNotBypass(t *testing.T) {
srv := newTestServer(t)
h := srv.routes()
for i := 0; i < 5; 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")
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
if rr.Code != 201 {
t.Fatalf("req %d: want 201, got %d", i, rr.Code)
// Issue #280: rotating X-Forwarded-For must NOT reset the bucket. Pentest
// repro was 8 creates with rotating XFF -> 6x201.
func TestRotatingXFFDoesNotResetBucket(t *testing.T) {
globalLimiter = newLimiter()
defer SetTrustedIPHeader("")
SetTrustedIPHeader("")
s := defaultSettings(Config{}) // burst/limit defaults; any header values are ignored anyway
var allowed, limited int
for i := 0; i < 8; i++ {
r := httptest.NewRequest("POST", "/api/pastes", nil)
r.RemoteAddr = "198.51.100.1:5000"
r.Header.Set("X-Forwarded-For", fmt.Sprintf("9.9.9.%d", i))
if rateLimitCreate(r, s) {
allowed++
} else {
limited++
}
}
// 6th request, still the same real IP, new spoofed prefix: must 429
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")
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
if rr.Code != 429 {
t.Fatalf("spoofed 6th req: want 429, got %d", rr.Code)
}
}
func spoofN(i int) string {
return "1.2.3." + string(rune('0'+i))
}
// 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)
}
if float64(allowed) != s.RateLimitBurst || limited != 8-int(s.RateLimitBurst) {
t.Fatalf("rotating XFF: allowed=%d limited=%d, want allowed=%v (burst), limited=%d", allowed, limited, s.RateLimitBurst, 8-int(s.RateLimitBurst))
}
}
internal/api/server.go
+5
View File
@@ -28,6 +28,10 @@ type Config struct {
DBPath string
MaxTextBytes int64
MaxItemBytes int64
// TrustedIPHeader optionally names a proxy-controlled client-IP header
// (e.g. CF-Connecting-IP behind Cloudflare) to key rate limits on. Empty
// (default) keys on the peer address only. See clientip.go (#280).
TrustedIPHeader string
}
type apiServer struct {
@@ -39,6 +43,7 @@ type apiServer struct {
}
func NewServer(st *store.Store, cfg Config, ui *web.UI, ss *settingsStore, adminKey string) *apiServer {
SetTrustedIPHeader(cfg.TrustedIPHeader) // #280
return &apiServer{store: st, cfg: cfg, ui: ui, settings: ss, adminKey: adminKey}
}