Harden HTTP defaults and proxy header trust.
Add explicit server timeout/header limits, require trusted proxy CIDRs when honoring forwarded IP headers, and document single-instance storage expectations. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -44,10 +44,10 @@ func (s *Server) Routes() http.Handler {
|
||||
|
||||
writeHandler := http.HandlerFunc(s.createScratch)
|
||||
middlewares := make([]func(http.Handler) http.Handler, 0, 2)
|
||||
middlewares = append(middlewares, AllowlistMiddleware(s.cfg.Security.AllowedPrefixes, s.cfg.Security.TrustProxyHeaders, s.logger))
|
||||
middlewares = append(middlewares, AllowlistMiddleware(s.cfg.Security.AllowedPrefixes, s.cfg.Security.TrustProxyHeaders, s.cfg.Security.TrustedProxyPrefixes, s.logger))
|
||||
if s.cfg.Security.RateLimit.Enabled {
|
||||
limiter := NewRateLimiter(s.cfg.Security.RateLimit.RequestsPerMinute, s.cfg.Security.RateLimit.Burst)
|
||||
middlewares = append(middlewares, RateLimitMiddleware(limiter, s.cfg.Security.TrustProxyHeaders))
|
||||
middlewares = append(middlewares, RateLimitMiddleware(limiter, s.cfg.Security.TrustProxyHeaders, s.cfg.Security.TrustedProxyPrefixes))
|
||||
}
|
||||
mux.Handle("POST /api/scratch", Chain(writeHandler, middlewares...))
|
||||
|
||||
@@ -298,7 +298,7 @@ func (s *Server) isUploadAllowedForRequest(r *http.Request) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
clientIP, ok := extractClientIP(r, s.cfg.Security.TrustProxyHeaders)
|
||||
clientIP, ok := extractClientIP(r, s.cfg.Security.TrustProxyHeaders, s.cfg.Security.TrustedProxyPrefixes)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -59,14 +59,14 @@ func (r *RateLimiter) Allow(ip string) bool {
|
||||
return v.limiter.Allow()
|
||||
}
|
||||
|
||||
func AllowlistMiddleware(allowed []netip.Prefix, trustProxyHeaders bool, logger *log.Logger) func(http.Handler) http.Handler {
|
||||
func AllowlistMiddleware(allowed []netip.Prefix, trustProxyHeaders bool, trustedProxies []netip.Prefix, logger *log.Logger) func(http.Handler) http.Handler {
|
||||
return func(next http.Handler) http.Handler {
|
||||
if len(allowed) == 0 {
|
||||
return next
|
||||
}
|
||||
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
clientIP, ok := extractClientIP(r, trustProxyHeaders)
|
||||
clientIP, ok := extractClientIP(r, trustProxyHeaders, trustedProxies)
|
||||
if !ok {
|
||||
http.Error(w, "unable to determine client ip", http.StatusForbidden)
|
||||
return
|
||||
@@ -85,10 +85,10 @@ func AllowlistMiddleware(allowed []netip.Prefix, trustProxyHeaders bool, logger
|
||||
}
|
||||
}
|
||||
|
||||
func RateLimitMiddleware(limiter *RateLimiter, trustProxyHeaders bool) func(http.Handler) http.Handler {
|
||||
func RateLimitMiddleware(limiter *RateLimiter, trustProxyHeaders bool, trustedProxies []netip.Prefix) func(http.Handler) http.Handler {
|
||||
return func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
clientIP, ok := extractClientIP(r, trustProxyHeaders)
|
||||
clientIP, ok := extractClientIP(r, trustProxyHeaders, trustedProxies)
|
||||
if !ok {
|
||||
http.Error(w, "unable to determine client ip", http.StatusTooManyRequests)
|
||||
return
|
||||
@@ -115,8 +115,13 @@ func Chain(handler http.Handler, middlewares ...func(http.Handler) http.Handler)
|
||||
return wrapped
|
||||
}
|
||||
|
||||
func extractClientIP(r *http.Request, trustProxyHeaders bool) (netip.Addr, bool) {
|
||||
if trustProxyHeaders {
|
||||
func extractClientIP(r *http.Request, trustProxyHeaders bool, trustedProxies []netip.Prefix) (netip.Addr, bool) {
|
||||
remoteIP, ok := remoteIPFromRequest(r)
|
||||
if !ok {
|
||||
return netip.Addr{}, false
|
||||
}
|
||||
|
||||
if trustProxyHeaders && isTrustedProxy(remoteIP, trustedProxies) {
|
||||
if xff := strings.TrimSpace(r.Header.Get("X-Forwarded-For")); xff != "" {
|
||||
parts := strings.Split(xff, ",")
|
||||
if len(parts) > 0 {
|
||||
@@ -131,7 +136,10 @@ func extractClientIP(r *http.Request, trustProxyHeaders bool) (netip.Addr, bool)
|
||||
}
|
||||
}
|
||||
}
|
||||
return remoteIP, true
|
||||
}
|
||||
|
||||
func remoteIPFromRequest(r *http.Request) (netip.Addr, bool) {
|
||||
host, _, err := net.SplitHostPort(strings.TrimSpace(r.RemoteAddr))
|
||||
if err != nil {
|
||||
if ip, parseErr := netip.ParseAddr(strings.TrimSpace(r.RemoteAddr)); parseErr == nil {
|
||||
@@ -146,6 +154,15 @@ func extractClientIP(r *http.Request, trustProxyHeaders bool) (netip.Addr, bool)
|
||||
return ip, true
|
||||
}
|
||||
|
||||
func isTrustedProxy(remoteIP netip.Addr, trusted []netip.Prefix) bool {
|
||||
for _, prefix := range trusted {
|
||||
if prefix.Contains(remoteIP) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func writeJSON(w http.ResponseWriter, status int, payload any) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(status)
|
||||
|
||||
@@ -17,7 +17,7 @@ func TestAllowlistMiddlewareDeniesAndAllowsByIP(t *testing.T) {
|
||||
|
||||
logger := log.New(io.Discard, "", 0)
|
||||
allowed := []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}
|
||||
mw := AllowlistMiddleware(allowed, false, logger)
|
||||
mw := AllowlistMiddleware(allowed, false, nil, logger)
|
||||
|
||||
next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
@@ -46,7 +46,8 @@ func TestAllowlistMiddlewareHonorsProxyHeaderWhenTrusted(t *testing.T) {
|
||||
|
||||
logger := log.New(io.Discard, "", 0)
|
||||
allowed := []netip.Prefix{netip.MustParsePrefix("203.0.113.5/32")}
|
||||
mw := AllowlistMiddleware(allowed, true, logger)
|
||||
trustedProxies := []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}
|
||||
mw := AllowlistMiddleware(allowed, true, trustedProxies, logger)
|
||||
handler := mw(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}))
|
||||
@@ -66,7 +67,7 @@ func TestRateLimitMiddlewareKeysByClientIP(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
limiter := NewRateLimiter(60, 1)
|
||||
handler := RateLimitMiddleware(limiter, false)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
handler := RateLimitMiddleware(limiter, false, nil)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}))
|
||||
|
||||
@@ -98,7 +99,7 @@ func TestRateLimitMiddlewareKeysByClientIP(t *testing.T) {
|
||||
func TestAllowlistMiddlewareNoRestrictionsReturnsNext(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
handler := AllowlistMiddleware(nil, false, log.New(io.Discard, "", 0))(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
handler := AllowlistMiddleware(nil, false, nil, log.New(io.Discard, "", 0))(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusAccepted)
|
||||
}))
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/scratch", nil)
|
||||
@@ -115,7 +116,7 @@ func TestAllowlistMiddlewareRejectsUnknownClientIP(t *testing.T) {
|
||||
|
||||
logger := log.New(io.Discard, "", 0)
|
||||
allowed := []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")}
|
||||
handler := AllowlistMiddleware(allowed, false, logger)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
handler := AllowlistMiddleware(allowed, false, nil, logger)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}))
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/scratch", nil)
|
||||
@@ -131,7 +132,7 @@ func TestRateLimitMiddlewareUnknownIP(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
limiter := NewRateLimiter(60, 1)
|
||||
handler := RateLimitMiddleware(limiter, false)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
handler := RateLimitMiddleware(limiter, false, nil)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}))
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/scratch", nil)
|
||||
@@ -147,7 +148,7 @@ func TestRateLimitMiddleware429PayloadAndRetryHeader(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
limiter := NewRateLimiter(1, 1)
|
||||
handler := RateLimitMiddleware(limiter, false)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
handler := RateLimitMiddleware(limiter, false, nil)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}))
|
||||
req1 := httptest.NewRequest(http.MethodPost, "/api/scratch", nil)
|
||||
@@ -195,7 +196,7 @@ func TestExtractClientIPFallbacks(t *testing.T) {
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.RemoteAddr = "203.0.113.9"
|
||||
ip, ok := extractClientIP(req, false)
|
||||
ip, ok := extractClientIP(req, false, nil)
|
||||
if !ok || ip.String() != "203.0.113.9" {
|
||||
t.Fatalf("extract direct ip = (%v,%v), want (203.0.113.9,true)", ip, ok)
|
||||
}
|
||||
@@ -204,14 +205,26 @@ func TestExtractClientIPFallbacks(t *testing.T) {
|
||||
req2.RemoteAddr = "192.0.2.50:8080"
|
||||
req2.Header.Set("X-Forwarded-For", "bad,198.51.100.1")
|
||||
req2.Header.Set("X-Real-IP", "198.51.100.5")
|
||||
ip2, ok2 := extractClientIP(req2, true)
|
||||
ip2, ok2 := extractClientIP(req2, true, []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")})
|
||||
if !ok2 || ip2.String() != "198.51.100.5" {
|
||||
t.Fatalf("extract x-real-ip = (%v,%v), want (198.51.100.5,true)", ip2, ok2)
|
||||
}
|
||||
|
||||
req3 := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req3.RemoteAddr = "still-not-an-ip"
|
||||
if _, ok3 := extractClientIP(req3, false); ok3 {
|
||||
if _, ok3 := extractClientIP(req3, false, nil); ok3 {
|
||||
t.Fatalf("extractClientIP() ok = true, want false")
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractClientIPIgnoresHeadersFromUntrustedProxy(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.RemoteAddr = "203.0.113.10:8080"
|
||||
req.Header.Set("X-Forwarded-For", "198.51.100.5")
|
||||
ip, ok := extractClientIP(req, true, []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")})
|
||||
if !ok || ip.String() != "203.0.113.10" {
|
||||
t.Fatalf("extractClientIP() = (%v,%v), want (203.0.113.10,true)", ip, ok)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user