diff --git a/platform/internal/middleware/ratelimit_test.go b/platform/internal/middleware/ratelimit_test.go index 4f17ce56..43655915 100644 --- a/platform/internal/middleware/ratelimit_test.go +++ b/platform/internal/middleware/ratelimit_test.go @@ -45,6 +45,107 @@ func TestRateLimit_HeadersPresentOnAllowedRequest(t *testing.T) { } } +// TestRateLimit_XFF_BypassDocumented shows that WITHOUT SetTrustedProxies(nil) +// a spoofed X-Forwarded-For header can rotate an attacker's effective IP and +// bypass per-IP rate limiting (documents the issue #179 vulnerability). +func TestRateLimit_XFF_BypassDocumented(t *testing.T) { + gin.SetMode(gin.TestMode) + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + rl := NewRateLimiter(2, 5*time.Second, ctx) + + r := gin.New() + // Intentionally NOT calling r.SetTrustedProxies(nil) — replicates the + // pre-fix behaviour where Gin trusts all proxies by default. + r.Use(rl.Middleware()) + r.GET("/x", func(c *gin.Context) { c.String(http.StatusOK, "ok") }) + + // Exhaust both tokens for the real IP 10.0.0.1. + for i := 0; i < 2; i++ { + req := httptest.NewRequest(http.MethodGet, "/x", nil) + req.RemoteAddr = "10.0.0.1:1234" + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("setup request %d: want 200, got %d", i+1, w.Code) + } + } + // Third request without XFF must be rate-limited. + { + req := httptest.NewRequest(http.MethodGet, "/x", nil) + req.RemoteAddr = "10.0.0.1:1234" + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusTooManyRequests { + t.Fatalf("3rd request (no XFF): want 429, got %d", w.Code) + } + } + // With default proxy trust, spoofing X-Forwarded-For rotates the effective + // IP → new bucket → bypass succeeds (returns 200). + { + req := httptest.NewRequest(http.MethodGet, "/x", nil) + req.RemoteAddr = "10.0.0.1:1234" + req.Header.Set("X-Forwarded-For", "20.0.0.1") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Skipf("bypass no longer works without trusted-proxy config (Gin version changed?): got %d", w.Code) + } + } +} + +// TestRateLimit_XFF_NoBypassWithTrustedProxiesNil is the regression test for +// issue #179: after r.SetTrustedProxies(nil) is added to router.Setup(), a +// spoofed X-Forwarded-For header is ignored and the real RemoteAddr is used, +// so the bypass no longer works. +func TestRateLimit_XFF_NoBypassWithTrustedProxiesNil(t *testing.T) { + gin.SetMode(gin.TestMode) + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + rl := NewRateLimiter(2, 5*time.Second, ctx) + + r := gin.New() + // Fix for issue #179 — mirror what router.Setup() now does. + if err := r.SetTrustedProxies(nil); err != nil { + t.Fatalf("SetTrustedProxies: %v", err) + } + r.Use(rl.Middleware()) + r.GET("/x", func(c *gin.Context) { c.String(http.StatusOK, "ok") }) + + // Exhaust both tokens for RemoteAddr 10.0.0.2. + for i := 0; i < 2; i++ { + req := httptest.NewRequest(http.MethodGet, "/x", nil) + req.RemoteAddr = "10.0.0.2:9999" + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("setup request %d: want 200, got %d", i+1, w.Code) + } + } + // Third plain request must be rate-limited. + { + req := httptest.NewRequest(http.MethodGet, "/x", nil) + req.RemoteAddr = "10.0.0.2:9999" + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusTooManyRequests { + t.Fatalf("3rd plain request: want 429, got %d", w.Code) + } + } + // Spoofed XFF must NOT rotate the bucket — still 429 because + // SetTrustedProxies(nil) forces c.ClientIP() to return RemoteAddr. + { + req := httptest.NewRequest(http.MethodGet, "/x", nil) + req.RemoteAddr = "10.0.0.2:9999" + req.Header.Set("X-Forwarded-For", "99.99.99.99") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + if w.Code != http.StatusTooManyRequests { + t.Errorf("XFF bypass still works after fix: want 429, got %d — SetTrustedProxies(nil) not effective", w.Code) + } + } +} + // TestRateLimit_RetryAfterOn429 — throttled responses must carry Retry-After // per RFC 6585, so curl/fetch clients back off the exact required window. func TestRateLimit_RetryAfterOn429(t *testing.T) { diff --git a/platform/internal/router/router.go b/platform/internal/router/router.go index 58ce2fd2..b1e482fc 100644 --- a/platform/internal/router/router.go +++ b/platform/internal/router/router.go @@ -25,6 +25,14 @@ import ( func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provisioner, platformURL, configsDir string, wh *handlers.WorkspaceHandler, channelMgr *channels.Manager) *gin.Engine { r := gin.Default() + // Issue #179 — trust no reverse-proxy headers. Without this call Gin's + // default is to trust ALL X-Forwarded-For values, which lets any caller + // spoof their IP and bypass per-IP rate limiting. With nil, c.ClientIP() + // always returns the real TCP RemoteAddr. + if err := r.SetTrustedProxies(nil); err != nil { + panic("router: SetTrustedProxies: " + err.Error()) + } + // CORS origins — configurable via CORS_ORIGINS env var (comma-separated) corsOrigins := []string{"http://localhost:3000", "http://localhost:3001"} if v := os.Getenv("CORS_ORIGINS"); v != "" {