From 2201d1d2ed2cca77196b1de98649219197edeabe Mon Sep 17 00:00:00 2001 From: Jeferson Siqueira Date: Fri, 4 Sep 2026 10:25:46 +0100 Subject: [PATCH] feat: rate limit by caller, and decide whose address that is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A public endpoint with no limit is an invitation, and the benchmarks made the invitation cheaper to accept: at 522ns a request, serving abuse costs the host almost nothing per call and everything in aggregate. A token bucket per caller, in memory. Not in Redis — protecting work that takes half a microsecond with a network round trip would cost five orders of magnitude more than the thing being protected. Idle buckets are swept, or the map grows for the life of the process. The interesting part is not the limiter, it is deciding whose address it counts. Behind a platform proxy the socket address belongs to the proxy, so with no configuration every caller shares one bucket and the limit is global. The obvious fix is to trust X-Forwarded-For, and it is worse: on a public host that header is written by whoever is calling, so a limit keyed on it is a limit anyone steps around by changing a string. Gin trusts all private ranges by default, which is exactly that mistake with a reassuring name. So it is explicit. TRUSTED_PLATFORM=cloudflare reads CF-Connecting-IP, which Cloudflare overwrites on every request and a caller cannot forge — and Render fronts services with Cloudflare, confirmed against production response headers, which makes it the right setting there. TRUSTED_PROXIES takes CIDRs for anything else. Unset, nothing is trusted and the limit applies to everyone together: safe and honest rather than silently forgeable, and the service logs a warning saying so. Per-address limiting carries a known cost, and it is the one a load test against a gateway of mine surfaced: everyone behind one NAT is throttled as a single abusive caller. The fix is an authenticated identity, which this service does not have, so the limit is generous instead — high enough that a shared office address is not punished, low enough that a script is. /health is exempt. A throttled monitor reports an outage that is not happening. golang.org/x/time is pinned to a release that keeps the Go directive where it was; go get had quietly moved it from 1.22 to 1.25, and a toolchain bump on a live deployment is not something to acquire as a side effect of adding a dependency. --- README.md | 14 ++++ go.mod | 3 +- go.sum | 2 + main.go | 41 ++++++++++++ middleware/ratelimit.go | 126 +++++++++++++++++++++++++++++++++++ middleware/ratelimit_test.go | 95 ++++++++++++++++++++++++++ 6 files changed, 280 insertions(+), 1 deletion(-) create mode 100644 middleware/ratelimit.go create mode 100644 middleware/ratelimit_test.go diff --git a/README.md b/README.md index bcaad29..e95e5c5 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,20 @@ Worth knowing before reaching for a cache: a CPF check costs about half a microsecond, against a Redis round trip measured at 150–220 ms in production. The cache is opt-in through `REDIS_ADDR` and off by default. +## Configuration + +Every value is optional and the service runs with none of them set. + +| Variable | Effect | +|---|---| +| `PORT` | Listen address. Defaults to 8080. | +| `TRUSTED_PLATFORM` | `cloudflare` reads the caller's address from `CF-Connecting-IP`. **Set this on Render**, which fronts services with Cloudflare — without it every caller shares one rate-limit bucket. | +| `TRUSTED_PROXIES` | CIDRs allowed to set `X-Forwarded-For`, for other hosts. | +| `RATE_LIMIT_RPS` | Requests per second per caller. Defaults to 20. | +| `RATE_LIMIT_BURST` | Burst allowance. Defaults to 60. | +| `REDIS_ADDR` | Enables the optional CPF cache. Off by default, and the benchmarks above explain why. | +| `LOG_LEVEL` | `debug` for cache diagnostics. | + ## Layout ``` diff --git a/go.mod b/go.mod index bda1375..550a7c6 100644 --- a/go.mod +++ b/go.mod @@ -44,7 +44,8 @@ require ( golang.org/x/crypto v0.28.0 // indirect golang.org/x/net v0.30.0 // indirect golang.org/x/sys v0.26.0 // indirect - golang.org/x/text v0.19.0 // indirect + golang.org/x/text v0.19.0 + golang.org/x/time v0.5.0 golang.org/x/tools v0.26.0 // indirect google.golang.org/protobuf v1.35.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index 1768bc2..2d05731 100644 --- a/go.sum +++ b/go.sum @@ -144,6 +144,8 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.19.0 h1:kTxAhCbGbxhK0IwgSKiMO5awPoDQ0RpfiVYBfK860YM= golang.org/x/text v0.19.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= +golang.org/x/time v0.5.0 h1:o7cqy6amK/52YcAKIPlM3a+Fpj35zvRj2TP+e1xFSfk= +golang.org/x/time v0.5.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= diff --git a/main.go b/main.go index ae5c823..c61cd93 100644 --- a/main.go +++ b/main.go @@ -7,6 +7,7 @@ import ( "net/http" "os" "os/signal" + "strings" "syscall" "time" @@ -34,6 +35,9 @@ func main() { router.Use(gin.Recovery()) router.Use(observability.RequestLogger(logger)) router.Use(middleware.CORS()) + router.Use(middleware.RateLimit()) + + configureClientAddressing(router, logger) router.GET("/health", handlers.HealthHandler) router.GET("/swagger/*any", ginSwagger.WrapHandler(swaggerFiles.Handler)) @@ -73,6 +77,43 @@ func main() { } } +// configureClientAddressing decides whose address the rate limiter counts. +// +// Behind a platform proxy the socket address belongs to the proxy, so without +// configuration every caller shares one bucket and the limit becomes global. +// The obvious fix — trusting X-Forwarded-For — is worse: on a public host that +// header is written by whoever is calling, so a limit keyed on it is a limit +// anyone can step around by changing a string. +// +// So it is explicit. TRUSTED_PLATFORM=cloudflare reads CF-Connecting-IP, which +// Cloudflare overwrites on every request and a caller therefore cannot forge; +// Render fronts services with Cloudflare, which is what makes this the right +// setting there. TRUSTED_PROXIES takes CIDRs for anything else. Unset, nothing +// is trusted: the limit is global, which is safe and honest rather than +// silently forgeable. +func configureClientAddressing(router *gin.Engine, logger *slog.Logger) { + if platform := strings.ToLower(os.Getenv("TRUSTED_PLATFORM")); platform == "cloudflare" { + router.TrustedPlatform = gin.PlatformCloudflare + logger.Info("client address taken from CF-Connecting-IP") + return + } + + if proxies := os.Getenv("TRUSTED_PROXIES"); proxies != "" { + if err := router.SetTrustedProxies(strings.Split(proxies, ",")); err != nil { + logger.Error("invalid TRUSTED_PROXIES", slog.String("error", err.Error())) + os.Exit(1) + } + logger.Info("client address taken from X-Forwarded-For", slog.String("trusted", proxies)) + return + } + + if err := router.SetTrustedProxies(nil); err != nil { + logger.Error("could not clear trusted proxies", slog.String("error", err.Error())) + os.Exit(1) + } + logger.Warn("no trusted proxy configured; the rate limit applies to all callers together") +} + // port honours the value the platform assigns; Render sets PORT and would // otherwise route to a port nothing is listening on. func port() string { diff --git a/middleware/ratelimit.go b/middleware/ratelimit.go new file mode 100644 index 0000000..3e01b7d --- /dev/null +++ b/middleware/ratelimit.go @@ -0,0 +1,126 @@ +package middleware + +import ( + "net/http" + "os" + "strconv" + "sync" + "time" + + "github.com/gin-gonic/gin" + "golang.org/x/time/rate" +) + +// Rate limiting here is per client address, which carries a known and +// deliberate cost: everyone behind one NAT — an office, a mobile carrier, a +// university — shares a bucket and is throttled as though they were a single +// abusive caller. +// +// That is not a theoretical objection. It is the finding a load test against a +// gateway of mine produced: unit and integration tests never generate +// concurrent traffic from distinct clients, so the behaviour is invisible until +// real load arrives. Keying on an authenticated identity is the fix, and this +// service has no authentication, so the limit is set generously instead — high +// enough that a shared address is not punished, low enough that a script is. +// +// The buckets live in memory rather than in Redis. Validation costs about half +// a microsecond; adding a network round trip to protect it would cost five +// orders of magnitude more than the work being protected. + +const ( + defaultRequestsPerSecond = 20 + defaultBurst = 60 + // idleBefore is how long a client is remembered after its last request. + // Without eviction the map grows for the lifetime of the process. + idleBefore = 10 * time.Minute + sweepEvery = time.Minute +) + +type client struct { + limiter *rate.Limiter + lastSeen time.Time +} + +type limiterSet struct { + mu sync.Mutex + clients map[string]*client + rps rate.Limit + burst int +} + +func newLimiterSet() *limiterSet { + set := &limiterSet{ + clients: make(map[string]*client), + rps: rate.Limit(envInt("RATE_LIMIT_RPS", defaultRequestsPerSecond)), + burst: envInt("RATE_LIMIT_BURST", defaultBurst), + } + + go set.sweep() + return set +} + +func (s *limiterSet) allow(address string) bool { + s.mu.Lock() + defer s.mu.Unlock() + + existing, seen := s.clients[address] + if !seen { + existing = &client{limiter: rate.NewLimiter(s.rps, s.burst)} + s.clients[address] = existing + } + existing.lastSeen = time.Now() + + return existing.limiter.Allow() +} + +func (s *limiterSet) sweep() { + for range time.Tick(sweepEvery) { + s.mu.Lock() + for address, c := range s.clients { + if time.Since(c.lastSeen) > idleBefore { + delete(s.clients, address) + } + } + s.mu.Unlock() + } +} + +// RateLimit throttles by client address. +// +// /health is exempt: a monitor that gets throttled reports an outage that is +// not happening, and health checks are the one caller whose rate is known and +// harmless. +func RateLimit() gin.HandlerFunc { + set := newLimiterSet() + + return func(c *gin.Context) { + if c.Request.URL.Path == "/health" { + c.Next() + return + } + + // ClientIP honours the trusted-proxy configuration set in main; behind + // a platform proxy the socket address is the proxy's, not the caller's. + if !set.allow(c.ClientIP()) { + c.Header("Retry-After", "1") + c.AbortWithStatusJSON(http.StatusTooManyRequests, gin.H{ + "status_code": http.StatusTooManyRequests, + "error_code": "RATE_LIMITED", + "message": "Too many requests from this address", + "is_valid": false, + }) + return + } + + c.Next() + } +} + +func envInt(name string, fallback int) int { + if raw := os.Getenv(name); raw != "" { + if value, err := strconv.Atoi(raw); err == nil && value > 0 { + return value + } + } + return fallback +} diff --git a/middleware/ratelimit_test.go b/middleware/ratelimit_test.go new file mode 100644 index 0000000..facb491 --- /dev/null +++ b/middleware/ratelimit_test.go @@ -0,0 +1,95 @@ +package middleware + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" +) + +func limitedRouter(t *testing.T) *gin.Engine { + t.Helper() + gin.SetMode(gin.TestMode) + + router := gin.New() + _ = router.SetTrustedProxies(nil) + router.Use(RateLimit()) + router.GET("/validate", func(c *gin.Context) { c.Status(http.StatusOK) }) + router.GET("/health", func(c *gin.Context) { c.Status(http.StatusOK) }) + return router +} + +func call(router *gin.Engine, path, address string) int { + recorder := httptest.NewRecorder() + request := httptest.NewRequest(http.MethodGet, path, nil) + request.RemoteAddr = address + ":54321" + router.ServeHTTP(recorder, request) + return recorder.Code +} + +func TestBurstIsAllowedThenThrottled(t *testing.T) { + router := limitedRouter(t) + + // The burst is spent first; what follows is refilled at the configured rate, + // which over a tight loop means refused. + allowed := 0 + for i := 0; i < defaultBurst+40; i++ { + if call(router, "/validate", "203.0.113.10") == http.StatusOK { + allowed++ + } + } + + if allowed == 0 { + t.Fatal("no request got through; the limiter is refusing everything") + } + if allowed > defaultBurst+5 { + t.Fatalf("the limiter let %d requests through, expected around the burst of %d", allowed, defaultBurst) + } +} + +func TestClientsAreLimitedIndependently(t *testing.T) { + router := limitedRouter(t) + + for i := 0; i < defaultBurst+40; i++ { + call(router, "/validate", "203.0.113.10") + } + + // A second address must be unaffected by the first exhausting its bucket. + if code := call(router, "/validate", "203.0.113.99"); code != http.StatusOK { + t.Fatalf("a different address was throttled by its neighbour: got %d", code) + } +} + +// A throttled monitor reports an outage that is not happening. +func TestHealthIsNeverThrottled(t *testing.T) { + router := limitedRouter(t) + + for i := 0; i < defaultBurst+200; i++ { + if code := call(router, "/health", "203.0.113.10"); code != http.StatusOK { + t.Fatalf("health was throttled after %d calls", i) + } + } +} + +func TestThrottledResponseTellsTheCallerWhen(t *testing.T) { + router := limitedRouter(t) + + var recorder *httptest.ResponseRecorder + for i := 0; i < defaultBurst+40; i++ { + recorder = httptest.NewRecorder() + request := httptest.NewRequest(http.MethodGet, "/validate", nil) + request.RemoteAddr = "203.0.113.55:1234" + router.ServeHTTP(recorder, request) + if recorder.Code == http.StatusTooManyRequests { + break + } + } + + if recorder.Code != http.StatusTooManyRequests { + t.Fatal("never reached the limit") + } + if recorder.Header().Get("Retry-After") == "" { + t.Fatal("a 429 without Retry-After leaves the caller guessing") + } +}