diff --git a/README.md b/README.md index a074dbe..ba79f23 100644 --- a/README.md +++ b/README.md @@ -267,7 +267,7 @@ internal/ logger/logger.go slog structured logging (TTY detection) healthcheck/healthcheck.go Health check service middleware/middleware.go HTTP middleware (logging, CORS, security - headers, metrics auth) + headers, metrics auth and rate limit) handlers/handlers.go HTTP request handlers server/ server.go HTTP server lifecycle @@ -335,6 +335,15 @@ is a misconfiguration, so dnswatcher fails fast with a clear error message rather than running silently. Set `DNSWATCHER_TARGETS` to a comma-separated list of DNS names before starting. +**`/metrics` is rate limited.** Each client address may send it 30 requests a +minute, failed logins included; beyond that it answers `429 Too Many Requests` +without checking the password. A Prometheus server scraping every 15 seconds +sends 4 a minute. IPv6 addresses in one /64 count as one client. When the +request comes from a private or loopback address, such as a reverse proxy's, +the client address is taken from the `X-Real-IP` or `X-Forwarded-For` header +the proxy sets; a proxy that sets neither makes all its clients share one +allowance. + ### Example `.env` ```sh diff --git a/TODO.md b/TODO.md index 1e32630..0b048af 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,8 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104 # Completed Steps +- 2026-10-01: `/metrics` allows each client address 30 requests a minute, + counted before Basic Auth, and answers 429 beyond that (closes #101). - 2026-10-01: the image built by `make docker` reports the `git describe` version, not `dev`, and the startup log now shows it (closes #109). - 2026-10-01: two notify shutdown tests always release the delivery they hold, @@ -97,8 +99,6 @@ https://git.eeqj.de/sneak/dnswatcher/issues/104 https://git.eeqj.de/sneak/dnswatcher/issues/107 - invalid DNS or TLS interval silently replaced by the default: https://git.eeqj.de/sneak/dnswatcher/issues/177 -- rate limit on `/metrics` Basic Auth: - https://git.eeqj.de/sneak/dnswatcher/issues/101 - trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 - 1.0 readiness: run it with a real config and read the logs: diff --git a/go.mod b/go.mod index 0078dcf..a9012dd 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/99designs/basicauth-go v0.0.0-20230316000542-bf6f9cbbf0f8 github.com/go-chi/chi/v5 v5.2.5 github.com/go-chi/cors v1.2.2 + github.com/go-chi/httprate v0.16.0 github.com/joho/godotenv v1.5.1 github.com/miekg/dns v1.1.72 github.com/prometheus/client_golang v1.23.2 @@ -22,6 +23,7 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/go-viper/mapstructure/v2 v2.4.0 // indirect + github.com/klauspost/cpuid/v2 v2.2.10 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pelletier/go-toml/v2 v2.2.4 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect @@ -34,6 +36,7 @@ require ( github.com/spf13/cast v1.10.0 // indirect github.com/spf13/pflag v1.0.10 // indirect github.com/subosito/gotenv v1.6.0 // indirect + github.com/zeebo/xxh3 v1.0.2 // indirect go.uber.org/dig v1.19.0 // indirect go.uber.org/multierr v1.10.0 // indirect go.uber.org/zap v1.26.0 // indirect diff --git a/go.sum b/go.sum index 720b18f..1570cf1 100644 --- a/go.sum +++ b/go.sum @@ -14,6 +14,8 @@ github.com/go-chi/chi/v5 v5.2.5 h1:Eg4myHZBjyvJmAFjFvWgrqDTXFyOzjj7YIm3L3mu6Ug= github.com/go-chi/chi/v5 v5.2.5/go.mod h1:X7Gx4mteadT3eDOMTsXzmI4/rwUpOwBHLpAfupzFJP0= github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE= github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58= +github.com/go-chi/httprate v0.16.0 h1:8V5DH9j6pSK6UQoBsTpvMyFxycqaKEIToyPKzHJjUa8= +github.com/go-chi/httprate v0.16.0/go.mod h1:A8lo+qRhk+s9LiuP5saS7XCGDXRXMcrueq0NfIuCa/I= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= @@ -22,6 +24,8 @@ github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0= github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4= github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo= github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ= +github.com/klauspost/cpuid/v2 v2.2.10 h1:tBs3QSyvjDyFTq3uoc/9xFpCuOsJQFNPiAhYdw2skhE= +github.com/klauspost/cpuid/v2 v2.2.10/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= @@ -62,6 +66,10 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= +github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ= +github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0= +github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0= +github.com/zeebo/xxh3 v1.0.2/go.mod h1:5NWz9Sef7zIDm2JHfFlcQvNekmcEl9ekUZQQKCYaDcA= go.uber.org/dig v1.19.0 h1:BACLhebsYdpQ7IROQ1AGPjrXcP5dF80U3gKoFzbaq/4= go.uber.org/dig v1.19.0/go.mod h1:Us0rSJiThwCv2GteUN0Q7OKvU7n5J4dxZ9JKUXozFdE= go.uber.org/fx v1.24.0 h1:wE8mruvpg2kiiL1Vqd0CC+tr0/24XIB10Iwp2lLWzkg= diff --git a/internal/middleware/export_test.go b/internal/middleware/export_test.go new file mode 100644 index 0000000..1abde0f --- /dev/null +++ b/internal/middleware/export_test.go @@ -0,0 +1,10 @@ +package middleware + +import "time" + +// The /metrics rate limit, exported so the tests can count requests +// against it. +const ( + MetricsRequestLimit = metricsRequestLimit + MetricsRequestWindow time.Duration = metricsRequestWindow +) diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 4ba05a8..f65f63f 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -5,12 +5,14 @@ import ( "log/slog" "net" "net/http" + "net/netip" "strings" "time" "github.com/99designs/basicauth-go" "github.com/go-chi/chi/v5/middleware" "github.com/go-chi/cors" + "github.com/go-chi/httprate" "go.uber.org/fx" "sneak.berlin/go/dnswatcher/internal/config" @@ -21,6 +23,17 @@ import ( // corsMaxAge is the maximum age for CORS preflight responses. const corsMaxAge = 300 +// Rate limit for /metrics: each client address may send +// metricsRequestLimit requests per metricsRequestWindow. Every request +// counts, so password guessing gets at most 30 tries a minute per +// address. One Prometheus server scraping every 15 seconds sends 4 +// requests a minute, and two scraping every 5 seconds from one address +// send 24, so normal scraping stays under the limit. +const ( + metricsRequestLimit = 30 + metricsRequestWindow = time.Minute +) + // Security response header values applied to every response. // // The CSP is as strict as the dashboard allows: the template ships no @@ -268,6 +281,32 @@ func (m *Middleware) SecurityHeaders() func(http.Handler) http.Handler { } } +// MetricsRateLimit returns middleware for /metrics that answers 429 +// Too Many Requests to a client address over the rate limit. The +// address is the one realIP works out, so a client that is not a +// trusted proxy cannot get a fresh allowance by sending its own +// X-Real-IP or X-Forwarded-For. CanonicalizeIP counts all IPv6 +// addresses in one /64 as one client, since a client usually holds a +// whole /64. An IPv4 address a proxy reports in IPv6-mapped form +// (::ffff:203.0.113.1) is turned back into plain IPv4 first, as every +// such address is in the same /64. +func (m *Middleware) MetricsRateLimit() func(http.Handler) http.Handler { + return httprate.LimitBy( + metricsRequestLimit, + metricsRequestWindow, + func(request *http.Request) (string, error) { + ip := realIP(request) + + addr, err := netip.ParseAddr(ip) + if err == nil { + ip = addr.Unmap().String() + } + + return httprate.CanonicalizeIP(ip), nil + }, + ) +} + // MetricsAuth returns basic auth middleware for /metrics. func (m *Middleware) MetricsAuth() func(http.Handler) http.Handler { if m.params.Config.MetricsUsername == "" { diff --git a/internal/middleware/middleware_test.go b/internal/middleware/middleware_test.go index da51423..c7c1a8c 100644 --- a/internal/middleware/middleware_test.go +++ b/internal/middleware/middleware_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/go-chi/chi/v5" "go.uber.org/fx/fxtest" @@ -340,3 +341,144 @@ func TestDashboardRendersWithSecurityHeaders(t *testing.T) { t.Errorf("CSP would block %q: %q", stylesheetPath, csp) } } + +// Addresses for the rate limit tests: a client connecting directly, a +// trusted proxy, and a client behind that proxy as its X-Real-IP +// header names it. +const ( + directClient = "198.51.100.1:4000" + trustedProxy = "10.0.0.1:4000" + proxiedClient = "203.0.113.1" +) + +// statusFrom sends a GET through handler as if from remoteAddr, with +// an X-Real-IP header when xRealIP is not empty, and returns the +// response status. +func statusFrom( + t *testing.T, + handler http.Handler, + remoteAddr string, + xRealIP string, +) int { + t.Helper() + + req := httptest.NewRequestWithContext( + t.Context(), http.MethodGet, "/metrics", nil, + ) + req.RemoteAddr = remoteAddr + + if xRealIP != "" { + req.Header.Set("X-Real-IP", xRealIP) + } + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + return rec.Code +} + +// TestMetricsRateLimitAllowsScraping checks that one address can send, +// within one window, what two Prometheus servers scraping every 5 +// seconds send in that time, without being turned away. +func TestMetricsRateLimitAllowsScraping(t *testing.T) { + t.Parallel() + + const scrapeInterval = 5 * time.Second + + scrapes := 2 * int(middleware.MetricsRequestWindow/scrapeInterval) + limited := newTestMiddleware(t).MetricsRateLimit()(okHandler()) + + for i := range scrapes { + got := statusFrom(t, limited, directClient, "") + if got != http.StatusOK { + t.Fatalf( + "scrape %d of %d: status = %d, want 200", + i+1, scrapes, got, + ) + } + } +} + +// TestMetricsRateLimitKeysOnClientAddress checks which requests share +// an allowance. Each case uses up the allowance of one client, then +// sends one more request. +func TestMetricsRateLimitKeysOnClientAddress(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + usedRemoteAddr string + usedXRealIP string + nextRemoteAddr string + nextXRealIP string + want int + }{ + { + "same address", + directClient, "", + directClient, "", + http.StatusTooManyRequests, + }, + { + "another address", + directClient, "", + "198.51.100.2:4000", "", + http.StatusOK, + }, + { + "own X-Real-IP from an untrusted address", + directClient, "", + directClient, "203.0.113.9", + http.StatusTooManyRequests, + }, + { + "same client behind the proxy", + trustedProxy, proxiedClient, + trustedProxy, proxiedClient, + http.StatusTooManyRequests, + }, + { + "another client behind the proxy", + trustedProxy, proxiedClient, + trustedProxy, "203.0.113.2", + http.StatusOK, + }, + { + "another client behind the proxy, IPv6-mapped", + trustedProxy, "::ffff:203.0.113.1", + trustedProxy, "::ffff:203.0.113.2", + http.StatusOK, + }, + { + "same IPv6 /64", + "[2001:db8::1]:4000", "", + "[2001:db8::2]:4000", "", + http.StatusTooManyRequests, + }, + { + "another IPv6 /64", + "[2001:db8::1]:4000", "", + "[2001:db8:0:1::1]:4000", "", + http.StatusOK, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + limited := newTestMiddleware(t).MetricsRateLimit()(okHandler()) + + for range middleware.MetricsRequestLimit { + statusFrom(t, limited, tt.usedRemoteAddr, tt.usedXRealIP) + } + + got := statusFrom( + t, limited, tt.nextRemoteAddr, tt.nextXRealIP, + ) + if got != tt.want { + t.Errorf("status = %d, want %d", got, tt.want) + } + }) + } +} diff --git a/internal/server/routes.go b/internal/server/routes.go index 6305e1d..0a8e8ed 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -64,9 +64,12 @@ func (s *Server) SetupRoutes() { // Prometheus scraper is not a browser. It is mounted rather than // added with Get so that every method on /metrics, OPTIONS // included, ends here instead of falling through to the public - // router and its CORS. + // router and its CORS. The rate limit comes before Basic Auth, so + // failed logins count against it and a request over the limit + // never reaches the password check. if s.params.Config.MetricsUsername != "" { metrics := chi.NewRouter() + metrics.Use(s.mw.MetricsRateLimit()) metrics.Use(s.mw.MetricsAuth()) metrics.Get("/", promhttp.Handler().ServeHTTP) s.router.Mount("/metrics", metrics) diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index a16c762..f8f9c47 100644 --- a/internal/server/routes_test.go +++ b/internal/server/routes_test.go @@ -219,3 +219,72 @@ func TestPreflightAllowsOnlyWhatPublicRoutesServe(t *testing.T) { } } } + +// metricsRequest builds a GET for /metrics from remoteAddr that logs +// in with the given password. +func metricsRequest( + t *testing.T, + remoteAddr string, + password string, +) *http.Request { + t.Helper() + + req := httptest.NewRequestWithContext( + t.Context(), http.MethodGet, "/metrics", nil, + ) + req.RemoteAddr = remoteAddr + req.SetBasicAuth(metricsUsername, password) + + return req +} + +// TestMetricsRateLimitComesBeforeAuth checks that failed logins to +// /metrics count against the rate limit; that once an address is over +// it, even the right password gets 429, with the same body as a wrong +// one; and that another address still gets in. +func TestMetricsRateLimitComesBeforeAuth(t *testing.T) { + viper.Reset() + t.Setenv("DNSWATCHER_TARGETS", "example.com") + t.Setenv("DNSWATCHER_METRICS_USERNAME", metricsUsername) + t.Setenv("DNSWATCHER_METRICS_PASSWORD", metricsPassword) + + const ( + guesser = "198.51.100.1:4000" + other = "198.51.100.2:4000" + + // Far more guesses than the rate limit allows. + maxGuesses = 1000 + ) + + srv := routedServer(t) + + var guess *httptest.ResponseRecorder + + for range maxGuesses { + guess = serve(srv, metricsRequest(t, guesser, "wrong")) + if guess.Code != http.StatusUnauthorized { + break + } + } + + if guess.Code != http.StatusTooManyRequests { + t.Fatalf("wrong password: status = %d, want 429", guess.Code) + } + + right := serve(srv, metricsRequest(t, guesser, metricsPassword)) + if right.Code != http.StatusTooManyRequests { + t.Errorf("right password: status = %d, want 429", right.Code) + } + + if right.Body.String() != guess.Body.String() { + t.Errorf( + "429 body with right password = %q, with wrong one = %q", + right.Body.String(), guess.Body.String(), + ) + } + + rec := serve(srv, metricsRequest(t, other, metricsPassword)) + if rec.Code != http.StatusOK { + t.Errorf("another address: status = %d, want 200", rec.Code) + } +}