From db5c9b6d9a9ce0b65f64e0d33a5531e661fbb8eb Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Sun, 9 Aug 2026 19:49:04 +0200 Subject: [PATCH] feat(deny-list): per-service deny rules with --deny-ip and --deny-user-agent ## Summary Static deny rules for operators fronting public tenant fleets, where an allow list is definitionally impossible and a rate limit cannot express "this network gets nothing" (#88). Rules are evaluated inline in serviceRequestWithTarget as the very first gate -- deny, then allow, then TLS redirect, rate limit, basic auth -- with the client resolved through the shared forwardedResolver. Denials answer 403 through the error-page machinery, are counted in kamal_proxy_denials_total by rule kind, and persist in ServiceOptions like every other knob. User-Agent rules are RE2, compiled once at deploy, anchored to the full header value, and checked only after the IP rules pass. A missing User-Agent only matches an explicit '^$' pattern. ## Test Coverage - deny_list_test.go: parsing/validation, IP matching (exact IPv6, no /64 collapse, IPv4-mapped normalization), anchored UA matching, empty-UA semantics, trusted-proxy justification, health-check-path guard - deny_list_service_test.go: 403 ordering vs allow/redirect/rate-limit/ basic-auth, health-check + internal exemptions, forwarded-chain resolution, denial metrics by kind, old-state-file safety, JSON round-trip, fail-closed on unreadable stored rules, redeploy removal ## Verification - [x] gofmt -l internal/ cmd/ clean - [x] make test passes - [x] go vet ./... and make lint (golangci-lint) clean - [x] go test -race on the request-gate paths clean --- README.md | 41 +++ internal/cmd/deploy.go | 7 +- internal/metrics/metrics.go | 20 ++ internal/server/cert_metrics_test.go | 22 ++ internal/server/deny_list.go | 242 ++++++++++++++ internal/server/deny_list_service_test.go | 382 ++++++++++++++++++++++ internal/server/deny_list_test.go | 221 +++++++++++++ internal/server/ip_allow_list.go | 8 +- internal/server/router.go | 4 + internal/server/service.go | 18 + 10 files changed, 960 insertions(+), 5 deletions(-) create mode 100644 internal/server/deny_list.go create mode 100644 internal/server/deny_list_service_test.go create mode 100644 internal/server/deny_list_test.go diff --git a/README.md b/README.md index 7fbcebf3..f2c91a81 100644 --- a/README.md +++ b/README.md @@ -324,6 +324,47 @@ Things worth knowing: If you use `--error-pages`, add a `403.html` to that directory. +### Denying abusive clients + +An allow list is the wrong shape for a service the whole internet is meant to +reach. When a scraper or a misbehaving bot is hammering a public service right +now, deploy it with deny rules instead: + + kamal-proxy deploy service1 --target web-1:3000 --deny-ip 203.0.113.0/24 + kamal-proxy deploy service1 --target web-1:3000 --deny-user-agent 'BadBot/.*' + +Matching requests get a `403`. `--deny-ip` takes addresses or CIDR ranges and +may be repeated or comma-separated. `--deny-user-agent` takes an RE2 pattern +matched against the full `User-Agent` value, compiled once at deploy, and may +be repeated (but not comma-separated — patterns can contain commas). + +Things worth knowing: + +* **The client is resolved exactly as it is for `--allow-ip`** — the connecting + address unless `--trusted-proxy` says the peer is yours, in which case the + forwarded chain is walked with the same rules. Denying the connecting peer + behind a trusted load balancer would deny everyone. +* **Deny runs first.** Before the allow list (an address on both lists is + denied), before the TLS redirect (a `403` solicits nothing), before the rate + limit (a denied client never spends budget), and before basic auth (a denied + network never learns credentials are wanted). +* **IPv6 addresses are matched exactly as written** — an explicit address + denies that address alone, with none of the rate limiter's `/64` grouping. A + deny names exactly what you wrote; write the CIDR if you mean the network. +* **A missing `User-Agent` is not a crime.** Only an explicit `^$` pattern + denies requests that send no `User-Agent` at all — even `.*` does not. + Patterns match the whole header value: `BadBot/.*` means that agent, not any + agent mentioning it somewhere. +* **The health check path stays open**, and deploying deny rules with a health + check path of `/` is rejected, same as `--allow-ip`. +* **Denials are counted** in the `kamal_proxy_denials_total` metric, labeled by + service and rule kind (`ip`, `user_agent`), and logged rate-limited per + service. +* **Redeploying without the flags removes the block.** Rules live in the + service's deploy options like every other knob. + +If you use `--error-pages`, add a `403.html` to that directory. + ### Rate limiting a service per client To cap how fast a single client may hit a service, deploy it with diff --git a/internal/cmd/deploy.go b/internal/cmd/deploy.go index d3aa7b85..cb1022b4 100644 --- a/internal/cmd/deploy.go +++ b/internal/cmd/deploy.go @@ -102,7 +102,12 @@ func newDeployCommand() *deployCommand { deployCommand.cmd.Flags().IntSliceVar(&deployCommand.args.ServiceOptions.InterceptErrorStatuses, "intercept-errors", nil, "Replace these response statuses from the target with the proxy's error pages, as 4xx or 5xx codes (e.g. 502,503,504; default none)") deployCommand.cmd.Flags().StringVar(&deployCommand.basicAuth, "basic-auth", "", "Require HTTP Basic credentials on every request to this service, as :. The health check path stays open. Use with --tls, or terminate TLS in front of the proxy -- Basic credentials are replayable and are sent on every request") deployCommand.cmd.Flags().StringSliceVar(&deployCommand.args.ServiceOptions.AllowIPs, "allow-ip", nil, "Serve this service only to these addresses or CIDR ranges (e.g. 10.0.0.0/8,203.0.113.7; default empty, serve everyone). Matches the connecting address, so list IPv6 ranges too if clients reach the proxy over IPv6. The health check path stays open") - deployCommand.cmd.Flags().StringSliceVar(&deployCommand.args.ServiceOptions.TrustedProxies, "trusted-proxy", nil, "Addresses or CIDR ranges of proxies in front of this one. Only when the connecting address is one of these are --allow-ip and --rate-limit matched against the forwarded chain instead. List every hop, not just the one that connects to kamal-proxy") + deployCommand.cmd.Flags().StringSliceVar(&deployCommand.args.ServiceOptions.DenyIPs, "deny-ip", nil, "Refuse this service to these addresses or CIDR ranges (e.g. 203.0.113.0/24,198.51.100.7; default empty, deny nobody). Checked before --allow-ip: an address on both lists is denied. Denied clients get a 403 and never spend rate-limit budget. The health check path stays open") + // StringArray rather than StringSlice: an RE2 pattern may contain commas, + // as `Bad(Bot|Crawler){1,3}` does, and StringSlice would split it into two + // broken rules. + deployCommand.cmd.Flags().StringArrayVar(&deployCommand.args.ServiceOptions.DenyUserAgents, "deny-user-agent", nil, "Refuse requests whose full User-Agent matches this RE2 pattern (e.g. 'BadBot/.*'; may be specified multiple times). Checked after the IP rules. A missing User-Agent only matches an explicit '^$' pattern") + deployCommand.cmd.Flags().StringSliceVar(&deployCommand.args.ServiceOptions.TrustedProxies, "trusted-proxy", nil, "Addresses or CIDR ranges of proxies in front of this one. Only when the connecting address is one of these are --allow-ip, --deny-ip and --rate-limit matched against the forwarded chain instead. List every hop, not just the one that connects to kamal-proxy") deployCommand.cmd.Flags().Float64Var(&deployCommand.args.ServiceOptions.RateLimit, "rate-limit", 0, "Max requests per second from a single client (default 0, no limit). Requests over the limit get a 429. Counts the connecting address unless --trusted-proxy is set; IPv6 clients are counted per /64, since a client can pick any address in its own. The health check path stays open") deployCommand.cmd.Flags().IntVar(&deployCommand.args.ServiceOptions.RateLimitBurst, "rate-limit-burst", 0, "How many requests a client may make back to back before --rate-limit applies (default 0, meaning the rate rounded up)") deployCommand.cmd.Flags().StringSliceVar(&deployCommand.args.ServiceOptions.RateLimitExempt, "rate-limit-exempt", nil, "Addresses or CIDR ranges that --rate-limit does not apply to (e.g. 10.0.0.0/8 for monitoring; default empty)") diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 430e6b2f..3c27ff99 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -24,6 +24,7 @@ type tracker interface { SetDynamicRedirects(service string, hosts, rules int) TrackDynamicRedirectPoll(service, outcome string) TrackDynamicRedirect(service string, status int) + TrackDenial(service, rule string) } var Tracker tracker = &nullTracker{} @@ -49,6 +50,7 @@ func (nullTracker) TrackCacheEviction(service, state string) func (nullTracker) SetDynamicRedirects(service string, hosts, rules int) {} func (nullTracker) TrackDynamicRedirectPoll(service, outcome string) {} func (nullTracker) TrackDynamicRedirect(service string, status int) {} +func (nullTracker) TrackDenial(service, rule string) {} type prometheusTracker struct { httpRequests *prometheus.CounterVec @@ -71,6 +73,9 @@ type prometheusTracker struct { dynamicRedirectMapSize *prometheus.GaugeVec dynamicRedirectPolls *prometheus.CounterVec dynamicRedirects *prometheus.CounterVec + + // Deny rule metrics + denials *prometheus.CounterVec } func NewPrometheusTracker() *prometheusTracker { @@ -206,6 +211,16 @@ func NewPrometheusTracker() *prometheusTracker { []string{"service", "status"}, ), + denials: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "denials_total", + Namespace: "kamal", + Subsystem: "proxy", + Help: "Requests refused by --deny-ip or --deny-user-agent, labeled by service and rule kind (ip, user_agent).", + }, + []string{"service", "rule"}, + ), + certCount: prometheus.NewGaugeVec( prometheus.GaugeOpts{ Name: "certificates_total", @@ -232,6 +247,7 @@ func NewPrometheusTracker() *prometheusTracker { tracker.dynamicRedirectMapSize, tracker.dynamicRedirectPolls, tracker.dynamicRedirects, + tracker.denials, ) return tracker @@ -308,6 +324,10 @@ func (p *prometheusTracker) TrackDynamicRedirect(service string, status int) { p.dynamicRedirects.WithLabelValues(service, strconv.Itoa(status)).Inc() } +func (p *prometheusTracker) TrackDenial(service, rule string) { + p.denials.WithLabelValues(service, rule).Inc() +} + // Private func normalizeMethod(method string) string { diff --git a/internal/server/cert_metrics_test.go b/internal/server/cert_metrics_test.go index de4c2395..db350c30 100644 --- a/internal/server/cert_metrics_test.go +++ b/internal/server/cert_metrics_test.go @@ -33,6 +33,8 @@ type fakeTracker struct { redirectMapSizes map[string][2]int // service -> {hosts, rules} redirectPolls map[string]int // "service:outcome" -> count redirectHits map[string]int // "service:status" -> count + + denials map[string]int // "service:rule" -> count } type certCountSample struct { @@ -53,6 +55,8 @@ func newFakeTracker() *fakeTracker { redirectMapSizes: make(map[string][2]int), redirectPolls: make(map[string]int), redirectHits: make(map[string]int), + + denials: make(map[string]int), } } @@ -108,6 +112,18 @@ func (f *fakeTracker) TrackDynamicRedirect(service string, status int) { f.redirectHits[service+":"+strconv.Itoa(status)]++ } +func (f *fakeTracker) TrackDenial(service, rule string) { + f.mu.Lock() + defer f.mu.Unlock() + f.denials[service+":"+rule]++ +} + +func (f *fakeTracker) denialCount(service, rule string) int { + f.mu.Lock() + defer f.mu.Unlock() + return f.denials[service+":"+rule] +} + func (f *fakeTracker) redirectPollCount(service, outcome string) int { f.mu.Lock() defer f.mu.Unlock() @@ -306,6 +322,12 @@ func (s *switchableTracker) TrackDynamicRedirect(service string, status int) { } } +func (s *switchableTracker) TrackDenial(service, rule string) { + if fake := s.current(); fake != nil { + fake.TrackDenial(service, rule) + } +} + // installFakeTracker points the tracker at a fresh capturing tracker for the // duration of one test. func installFakeTracker(t *testing.T) *fakeTracker { diff --git a/internal/server/deny_list.go b/internal/server/deny_list.go new file mode 100644 index 00000000..fa6f62ae --- /dev/null +++ b/internal/server/deny_list.go @@ -0,0 +1,242 @@ +package server + +import ( + "fmt" + "log/slog" + "net/http" + "net/netip" + "regexp" + "strings" + + "github.com/basecamp/kamal-proxy/internal/metrics" +) + +// emptyUserAgentPattern is the one pattern an absent User-Agent matches. Every +// other rule -- including one like `.*` that technically matches the empty +// string -- requires an agent to actually be present: absence is not a crime, +// and half the non-browser HTTP clients in the world send no User-Agent at all. +const emptyUserAgentPattern = "^$" + +// denyList refuses requests whose client address or User-Agent matches a rule +// the operator wrote. It is the abuse-blocking counterpart to ipAllowList: an +// allow list is definitionally impossible for a fleet serving the whole +// internet, and a rate limit bounds sustained abuse but cannot express "this +// network gets nothing". +// +// The client address is resolved through the same forwardedResolver as the +// allow list and the rate limiter -- behind a trusted load balancer, denying +// the connecting peer would deny everyone. +type denyList struct { + forwardedResolver + + prefixes []netip.Prefix + userAgents []denyUserAgentRule + + // denyAll refuses every request. It is only set for stored rules that can + // no longer be read back: a block that silently lapsed would serve the very + // traffic the operator asked to refuse. + denyAll bool + + denied *tokenBucket +} + +// denyUserAgentRule is one --deny-user-agent pattern, compiled once at deploy. +// The pattern is kept alongside the matcher so an absent User-Agent can be +// tested against the pattern the operator wrote rather than what it matches. +type denyUserAgentRule struct { + pattern string + matcher *regexp.Regexp +} + +func newDenyList(denyIPs, denyUserAgents, trustedProxies []string, clientIPHeader string) (*denyList, error) { + prefixes, err := parseIPPrefixes(denyIPs, "deny-ip") + if err != nil { + return nil, err + } + + userAgents, err := parseDenyUserAgents(denyUserAgents) + if err != nil { + return nil, err + } + + resolver, err := newForwardedResolver(trustedProxies, clientIPHeader) + if err != nil { + return nil, err + } + + return &denyList{ + forwardedResolver: resolver, + prefixes: prefixes, + userAgents: userAgents, + denied: newTokenBucket(deniedLogBurst, deniedLogInterval), + }, nil +} + +// deniesAddr reports whether addr matches a deny rule. The zero Addr matches +// nothing: a deny names exactly what the operator wrote, and an unresolvable +// client is not any of those things. (The allow list still refuses the zero +// Addr when one is configured; the two fail in the direction each is for.) +func (l *denyList) deniesAddr(addr netip.Addr) bool { + return addr.IsValid() && containsAddr(l.prefixes, addr) +} + +// deniesUserAgent reports whether the request's User-Agent matches a deny +// rule. Patterns match the full header value, not a substring of it. +func (l *denyList) deniesUserAgent(userAgent string) bool { + if userAgent == "" { + for _, rule := range l.userAgents { + if rule.pattern == emptyUserAgentPattern { + return true + } + } + + return false + } + + for _, rule := range l.userAgents { + if rule.matcher.MatchString(userAgent) { + return true + } + } + + return false +} + +// Private + +func parseDenyUserAgents(patterns []string) ([]denyUserAgentRule, error) { + if len(patterns) == 0 { + return nil, nil + } + + rules := make([]denyUserAgentRule, 0, len(patterns)) + for _, pattern := range patterns { + trimmed := strings.TrimSpace(pattern) + if trimmed == "" { + return nil, fmt.Errorf("%w: deny-user-agent: pattern cannot be empty (use %q to deny requests without a User-Agent)", ErrServiceOptionsInvalid, emptyUserAgentPattern) + } + + // Anchored to the whole header value: `BadBot/.*` means that agent, not + // any agent that happens to mention it somewhere. + matcher, err := regexp.Compile(`\A(?:` + trimmed + `)\z`) + if err != nil { + return nil, fmt.Errorf("%w: deny-user-agent: %q is not a valid RE2 pattern", ErrServiceOptionsInvalid, trimmed) + } + + rules = append(rules, denyUserAgentRule{pattern: trimmed, matcher: matcher}) + } + + return rules, nil +} + +func (so ServiceOptions) validateDeny() error { + if _, err := parseIPPrefixes(so.DenyIPs, "deny-ip"); err != nil { + return err + } + + if _, err := parseDenyUserAgents(so.DenyUserAgents); err != nil { + return err + } + + // Same trap as allow-ip: without a declared proxy the header is just + // something the client wrote, so honouring it would appear to consult it + // while matching the connecting peer instead. + if len(so.DenyIPs) > 0 && so.ClientIPHeader != "" && len(so.TrustedProxies) == 0 { + return fmt.Errorf("%w: deny-ip with client-ip-header requires trusted-proxy, or the header would be ignored while appearing to be honored", ErrServiceOptionsInvalid) + } + + return nil +} + +// hasDenyRules reports whether any deny rule is configured, of either kind. +func (so ServiceOptions) hasDenyRules() bool { + return len(so.DenyIPs) > 0 || len(so.DenyUserAgents) > 0 +} + +// validateDenyHealthCheck rejects the health check path that would quietly +// unblock the service, matching the equivalent rule for basic auth, allow-ip +// and rate-limit. +func validateDenyHealthCheck(options ServiceOptions, targetOptions TargetOptions) error { + if !options.hasDenyRules() { + return nil + } + + path := targetOptions.HealthCheckConfig.Path + if path == "" || path == rootPath { + return fmt.Errorf("%w: health-check-path cannot be %q when deny rules are set, as that path is served without them", ErrServiceOptionsInvalid, rootPath) + } + + return nil +} + +// resolveDenyList prepares the stored rules for serving. It never returns an +// error: this runs from initialize, which runs while decoding saved state, and +// failing there would abort the decode of every other service too. +func (s *Service) resolveDenyList(options ServiceOptions) *denyList { + if !options.hasDenyRules() { + return nil + } + + list, err := newDenyList(options.DenyIPs, options.DenyUserAgents, options.TrustedProxies, options.ClientIPHeader) + if err != nil { + slog.Error("Unable to read the stored deny rules; denying every request to this service", "service", s.name, "error", err) + + return &denyList{denyAll: true, denied: newTokenBucket(deniedLogBurst, deniedLogInterval)} + } + + slog.Info("Deny rules enabled", "service", s.name, "deny_ips", options.DenyIPs, + "deny_user_agents", options.DenyUserAgents, "trusted_proxies", options.TrustedProxies) + + return list +} + +// rejectDenied refuses a request matching this service's deny rules, reporting +// whether it handled the response. +// +// It runs as the very first check in serviceRequestWithTarget -- before the +// allow list, so an address on both lists is denied; before the TLS redirect, +// because a 403 solicits nothing; before the rate limit, so a denied client +// never spends budget; and before basic auth, so a denied network never learns +// credentials are wanted. It deliberately does NOT live in createMiddleware -- +// that chain includes the certificate manager's handler, so filtering up there +// would block ACME HTTP-01 validation and break renewal weeks later. +func (s *Service) rejectDenied(w http.ResponseWriter, r *http.Request) bool { + if s.denyRules == nil { + return false + } + + // Probes the proxy makes about itself, and health checks a downstream load + // balancer needs in order to see this service drain during a deploy. + if isInternalRequest(r) || s.targetOptions.IsHealthCheckRequest(r) { + return false + } + + kind := "" + switch { + case s.denyRules.denyAll: + kind = "unreadable" + case s.denyRules.deniesAddr(s.denyRules.clientAddr(r)): + // The address checks run before the User-Agent ones: cheapest first. + kind = "ip" + case s.denyRules.deniesUserAgent(r.Header.Get("User-Agent")): + kind = "user_agent" + default: + return false + } + + s.logDenied(r, kind) + metrics.Tracker.TrackDenial(s.name, kind) + + // A 403 through the error-page machinery, echoing nothing about the rule. + SetErrorResponse(w, r, http.StatusForbidden, nil) + + return true +} + +func (s *Service) logDenied(r *http.Request, kind string) { + if !s.denyRules.denied.TryTake() { + return + } + + slog.Warn("Denied by deny rules", "service", s.name, "rule", kind, "peer", r.RemoteAddr, "path", r.URL.Path) +} diff --git a/internal/server/deny_list_service_test.go b/internal/server/deny_list_service_test.go new file mode 100644 index 00000000..f5f1844b --- /dev/null +++ b/internal/server/deny_list_service_test.go @@ -0,0 +1,382 @@ +package server + +import ( + "encoding/json" + "net/http" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// testDeniedService deploys a service behind deny rules and returns a handler +// wired the way the real server wires one. +func testDeniedService(t *testing.T, options ServiceOptions, handler http.HandlerFunc) http.Handler { + t.Helper() + + router := testRouter(t) + _, target := testBackendWithHandler(t, handler) + + if options.DenyIPs == nil && options.DenyUserAgents == nil { + options.DenyIPs = []string{"203.0.113.0/24"} + } + + require.NoError(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, + options, defaultTargetOptions, defaultDeploymentOptions)) + + return testRoutedHandler(t, router) +} + +func TestDenyListService_EmptyOptionServesEveryone(t *testing.T) { + options := defaultServiceOptions + options.DenyIPs = []string{} + options.DenyUserAgents = []string{} + + handler := testDeniedService(t, options, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + }) + + resp := testAuthRequest(handler, testRequestFromPeer(testDeniedPeer, "http://example.com/")) + + assert.Equal(t, http.StatusOK, resp.StatusCode) +} + +func TestDenyListService_RejectsDeniedPeer(t *testing.T) { + var reachedTarget atomic.Int64 + + handler := testDeniedService(t, defaultServiceOptions, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != DefaultHealthCheckPath { + reachedTarget.Add(1) + } + w.Write([]byte("secret")) + }) + + resp := testAuthRequest(handler, testRequestFromPeer(testDeniedPeer, "http://example.com/")) + + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + assert.Zero(t, reachedTarget.Load(), "the target must never see a denied request") +} + +func TestDenyListService_ServesEveryoneElse(t *testing.T) { + handler := testDeniedService(t, defaultServiceOptions, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + }) + + resp := testAuthRequest(handler, testRequestFromPeer(testAllowedPeer, "http://example.com/")) + + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "ok", testAuthBody(t, resp)) +} + +func TestDenyListService_DenyBeatsAllow(t *testing.T) { + // An address matching both lists is denied: the deny list runs first. + options := defaultServiceOptions + options.AllowIPs = []string{"10.0.0.0/8"} + options.DenyIPs = []string{"10.0.0.5"} + + handler := testDeniedService(t, options, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + }) + + resp := testAuthRequest(handler, testRequestFromPeer("10.0.0.5:44321", "http://example.com/")) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + + resp = testAuthRequest(handler, testRequestFromPeer("10.0.0.6:44321", "http://example.com/")) + assert.Equal(t, http.StatusOK, resp.StatusCode) +} + +func TestDenyListService_RejectsBeforeRedirecting(t *testing.T) { + // Same stance as the allow list: a 403 solicits nothing, so a denied peer + // is refused rather than redirected to HTTPS first. + options := defaultServiceOptions + options.TLSEnabled = true + options.TLSRedirect = true + options.Hosts = []string{"example.com"} + + handler := testDeniedService(t, options, func(w http.ResponseWriter, r *http.Request) {}) + + resp := testAuthRequest(handler, testRequestFromPeer(testDeniedPeer, "http://example.com/")) + + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + assert.Empty(t, resp.Header.Get("Location")) +} + +func TestDenyListService_RejectsWithoutSpendingRateLimitBudget(t *testing.T) { + // A denied client is refused with a 403 every time, never a 429: it must + // not spend rate-limit budget, and repeated denials must not change the + // answer. + options := defaultServiceOptions + options.RateLimit = 1 + options.RateLimitBurst = 1 + + handler := testDeniedService(t, options, func(w http.ResponseWriter, r *http.Request) {}) + + for range 3 { + resp := testAuthRequest(handler, testRequestFromPeer(testDeniedPeer, "http://example.com/")) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + } +} + +func TestDenyListService_RejectsBeforeChallengingBasicAuth(t *testing.T) { + options := defaultServiceOptions + options.BasicAuth = testEncodedCredential(t, testAuthUser, testAuthPassword) + + handler := testDeniedService(t, options, func(w http.ResponseWriter, r *http.Request) {}) + + resp := testAuthRequest(handler, testRequestFromPeer(testDeniedPeer, "http://example.com/")) + + // A denied network never learns that the service wants credentials. + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + assert.Empty(t, resp.Header.Get("WWW-Authenticate")) +} + +func TestDenyListService_RejectsDeniedUserAgent(t *testing.T) { + options := defaultServiceOptions + options.DenyUserAgents = []string{`BadBot/.*`} + + handler := testDeniedService(t, options, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + }) + + tests := []struct { + name string + userAgent string + expectedStatus int + }{ + {"matching agent", "BadBot/1.0", http.StatusForbidden}, + {"other agent", "Mozilla/5.0", http.StatusOK}, + {"missing agent", "", http.StatusOK}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := testRequestFromPeer(testAllowedPeer, "http://example.com/") + if tt.userAgent != "" { + req.Header.Set("User-Agent", tt.userAgent) + } + + assert.Equal(t, tt.expectedStatus, testAuthRequest(handler, req).StatusCode) + }) + } +} + +func TestDenyListService_ResolvesClientThroughTrustedProxies(t *testing.T) { + options := defaultServiceOptions + options.DenyIPs = []string{"203.0.113.0/24"} + options.TrustedProxies = []string{"10.0.0.0/8"} + + handler := testDeniedService(t, options, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + }) + + // The peer is a trusted proxy; the denied client is in the forwarded chain. + req := testRequestFromPeer(testAllowedPeer, "http://example.com/") + req.Header.Set("X-Forwarded-For", "203.0.113.9") + + resp := testAuthRequest(handler, req) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + + // Without a trusted peer the same header buys nothing in either direction: + // a client cannot deny itself into a 403 for someone else, nor be denied on + // a header it wrote. + req = testRequestFromPeer(testDeniedPeer, "http://example.com/") + req.Header.Set("X-Forwarded-For", "10.0.0.1") + + resp = testAuthRequest(handler, req) + assert.Equal(t, http.StatusForbidden, resp.StatusCode) +} + +func TestDenyListService_ExemptsHealthCheckRequests(t *testing.T) { + handler := testDeniedService(t, defaultServiceOptions, func(w http.ResponseWriter, r *http.Request) {}) + + tests := []struct { + name string + method string + path string + expectedStatus int + }{ + {"GET on the health check path", http.MethodGet, DefaultHealthCheckPath, http.StatusOK}, + {"HEAD on the health check path", http.MethodHead, DefaultHealthCheckPath, http.StatusOK}, + {"POST on the health check path", http.MethodPost, DefaultHealthCheckPath, http.StatusForbidden}, + {"any other path", http.MethodGet, "/", http.StatusForbidden}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := testRequestFromPeer(testDeniedPeer, "http://example.com"+tt.path) + req.Method = tt.method + + assert.Equal(t, tt.expectedStatus, testAuthRequest(handler, req).StatusCode) + }) + } +} + +func TestDenyListService_ExemptsInternalRequests(t *testing.T) { + handler := testDeniedService(t, defaultServiceOptions, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + }) + + req := testRequestFromPeer(testDeniedPeer, "http://example.com/") + req = req.WithContext(markInternalRequest(req.Context())) + + assert.Equal(t, http.StatusOK, testAuthRequest(handler, req).StatusCode) +} + +func TestDenyListService_RejectsRootHealthCheckPath(t *testing.T) { + router := testRouter(t) + _, target := testBackendWithHandler(t, func(w http.ResponseWriter, r *http.Request) {}) + + options := defaultServiceOptions + options.DenyIPs = []string{"203.0.113.0/24"} + + targetOptions := defaultTargetOptions + targetOptions.HealthCheckConfig.Path = "/" + + err := router.DeployService("service1", []string{target}, defaultEmptyReaders, + options, targetOptions, defaultDeploymentOptions) + + require.ErrorIs(t, err, ErrServiceOptionsInvalid) + require.ErrorContains(t, err, "health-check-path") +} + +func TestDenyListService_RejectionRendersCustomErrorPage(t *testing.T) { + pagesDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(pagesDir, "403.html"), []byte("

go away

"), 0644)) + + options := defaultServiceOptions + options.ErrorPagePath = pagesDir + + handler := testDeniedService(t, options, func(w http.ResponseWriter, r *http.Request) {}) + + resp := testAuthRequest(handler, testRequestFromPeer(testDeniedPeer, "http://example.com/")) + + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + assert.Contains(t, testAuthBody(t, resp), "go away") +} + +func TestDenyListService_TracksDenialsByRuleKind(t *testing.T) { + fake := installFakeTracker(t) + + options := defaultServiceOptions + options.DenyIPs = []string{"203.0.113.0/24"} + options.DenyUserAgents = []string{`BadBot/.*`} + + handler := testDeniedService(t, options, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + }) + + // IP checks run first, so a request matching both counts as an IP denial. + req := testRequestFromPeer(testDeniedPeer, "http://example.com/") + req.Header.Set("User-Agent", "BadBot/1.0") + testAuthRequest(handler, req) + + req = testRequestFromPeer(testAllowedPeer, "http://example.com/") + req.Header.Set("User-Agent", "BadBot/1.0") + testAuthRequest(handler, req) + + assert.Equal(t, 1, fake.denialCount("service1", "ip")) + assert.Equal(t, 1, fake.denialCount("service1", "user_agent")) +} + +func TestDenyListService_StateWrittenBeforeTheOptionStaysOpen(t *testing.T) { + state := ` + { + "name": "my-app", + "hosts": ["app.example.com"], + "active_target": "localhost:3000", + "options": {}, + "target_options": { + "health_check_config": {"path": "/up", "interval": 1000000000, "timeout": 5000000000}, + "response_timeout": 30000000000 + }, + "pause_controller": {"state": 0, "stop_message": "", "fail_after": 0}, + "rollout_controller": null + } + ` + + var service Service + require.NoError(t, json.NewDecoder(strings.NewReader(state)).Decode(&service)) + t.Cleanup(service.Dispose) + + // An upgrade must never start refusing traffic an older proxy served. + assert.Empty(t, service.options.DenyIPs) + assert.Empty(t, service.options.DenyUserAgents) + assert.Nil(t, service.denyRules) +} + +func TestDenyListService_UnreadableStoredRulesFailClosed(t *testing.T) { + state := ` + { + "name": "my-app", + "active_target": "localhost:3000", + "options": {"deny_ips": ["not-an-ip"]}, + "target_options": { + "health_check_config": {"path": "/up", "interval": 1000000000, "timeout": 5000000000}, + "response_timeout": 30000000000 + }, + "pause_controller": {"state": 0, "stop_message": "", "fail_after": 0}, + "rollout_controller": null + } + ` + + // Decoding must succeed, or one bad entry takes down every other service in + // the state file. + var service Service + require.NoError(t, json.NewDecoder(strings.NewReader(state)).Decode(&service)) + t.Cleanup(service.Dispose) + + // A block that cannot be read back must hold, not silently lapse: the + // service denies everyone until it is redeployed with readable rules. + require.NotNil(t, service.denyRules) + assert.True(t, service.denyRules.denyAll) +} + +func TestDenyListService_SurvivesStateRoundTrip(t *testing.T) { + options := defaultServiceOptions + options.DenyIPs = []string{"203.0.113.0/24"} + options.DenyUserAgents = []string{`BadBot/.*`} + + service := testCreateService(t, options, defaultTargetOptions) + t.Cleanup(service.Dispose) + + encoded, err := json.Marshal(service) + require.NoError(t, err) + + var restored Service + require.NoError(t, json.Unmarshal(encoded, &restored)) + t.Cleanup(restored.Dispose) + + require.NotNil(t, restored.denyRules) + assert.True(t, restored.denyRules.deniesAddr(parseHostAddr(testDeniedPeer))) + assert.False(t, restored.denyRules.deniesAddr(parseHostAddr(testAllowedPeer))) + assert.True(t, restored.denyRules.deniesUserAgent("BadBot/1.0")) + assert.Equal(t, []string{"203.0.113.0/24"}, restored.options.DenyIPs) + assert.Equal(t, []string{`BadBot/.*`}, restored.options.DenyUserAgents) +} + +func TestDenyListService_RedeployWithoutTheFlagRemovesBlock(t *testing.T) { + router := testRouter(t) + _, target := testBackendWithHandler(t, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + }) + + blocked := defaultServiceOptions + blocked.DenyIPs = []string{"203.0.113.0/24"} + + require.NoError(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, + blocked, defaultTargetOptions, defaultDeploymentOptions)) + + handler := testRoutedHandler(t, router) + resp := testAuthRequest(handler, testRequestFromPeer(testDeniedPeer, "http://example.com/")) + require.Equal(t, http.StatusForbidden, resp.StatusCode) + + require.NoError(t, router.DeployService("service1", []string{target}, defaultEmptyReaders, + defaultServiceOptions, defaultTargetOptions, defaultDeploymentOptions)) + + resp = testAuthRequest(handler, testRequestFromPeer(testDeniedPeer, "http://example.com/")) + assert.Equal(t, http.StatusOK, resp.StatusCode) +} diff --git a/internal/server/deny_list_test.go b/internal/server/deny_list_test.go new file mode 100644 index 00000000..9577d833 --- /dev/null +++ b/internal/server/deny_list_test.go @@ -0,0 +1,221 @@ +package server + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDenyList_ParseErrors(t *testing.T) { + tests := []struct { + name string + denyIPs []string + denyUserAgents []string + expectedError string + }{ + { + name: "invalid address", + denyIPs: []string{"not-an-ip"}, + expectedError: "deny-ip", + }, + { + name: "invalid CIDR", + denyIPs: []string{"10.0.0.0/33"}, + expectedError: "deny-ip", + }, + { + name: "empty address entry", + denyIPs: []string{""}, + expectedError: "deny-ip", + }, + { + name: "invalid regex", + denyUserAgents: []string{"BadBot("}, + expectedError: "deny-user-agent", + }, + { + name: "empty pattern", + denyUserAgents: []string{""}, + expectedError: "deny-user-agent", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := newDenyList(tt.denyIPs, tt.denyUserAgents, nil, "") + + require.ErrorIs(t, err, ErrServiceOptionsInvalid) + require.ErrorContains(t, err, tt.expectedError) + }) + } +} + +func TestDenyList_DeniesAddr(t *testing.T) { + list, err := newDenyList([]string{"203.0.113.0/24", "198.51.100.7", "2001:db8::1"}, nil, nil, "") + require.NoError(t, err) + + tests := []struct { + name string + addr string + denied bool + }{ + {"address inside a denied range", "203.0.113.9", true}, + {"address outside every range", "10.0.0.5", false}, + {"exactly the denied address", "198.51.100.7", true}, + {"neighbour of the denied address", "198.51.100.8", false}, + {"exactly the denied IPv6 address", "2001:db8::1", true}, + {"same /64 as the denied IPv6 address", "2001:db8::2", false}, + {"IPv4-mapped form of a denied address", "::ffff:203.0.113.9", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.denied, list.deniesAddr(parseHostAddr(tt.addr))) + }) + } +} + +func TestDenyList_ZeroAddrMatchesNothing(t *testing.T) { + // A deny names exactly what the operator wrote; an unresolvable client is + // not any of those things. The allow list still refuses it when one is set. + list, err := newDenyList([]string{"0.0.0.0/0", "::/0"}, nil, nil, "") + require.NoError(t, err) + + assert.False(t, list.deniesAddr(parseHostAddr("not-an-address"))) +} + +func TestDenyList_DeniesUserAgent(t *testing.T) { + list, err := newDenyList(nil, []string{`BadBot/.*`, `^$`, `(?i)evilcrawler`}, nil, "") + require.NoError(t, err) + + tests := []struct { + name string + userAgent string + denied bool + }{ + {"full match on the pattern", "BadBot/1.0", true}, + {"different agent", "GoodBot/1.0", false}, + {"pattern is anchored, prefix junk escapes it", "xBadBot/1.0", false}, + {"pattern is anchored, matching only a substring is not enough", "EvilCrawler plus trailing junk", false}, + {"case-insensitive when the pattern says so", "eViLcRaWlEr", true}, + {"missing agent matches the explicit empty pattern", "", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.denied, list.deniesUserAgent(tt.userAgent)) + }) + } +} + +func TestDenyList_MissingUserAgentIsNotACrime(t *testing.T) { + // .* matches the empty string, but absence only matches an explicit ^$. + list, err := newDenyList(nil, []string{`.*`}, nil, "") + require.NoError(t, err) + + assert.False(t, list.deniesUserAgent("")) + assert.True(t, list.deniesUserAgent("anything at all")) +} + +func TestDenyList_ValidateDeny(t *testing.T) { + tests := []struct { + name string + options ServiceOptions + expectedError string + }{ + { + name: "deny-ip alone is valid", + options: ServiceOptions{DenyIPs: []string{"203.0.113.0/24"}}, + }, + { + name: "deny-user-agent alone is valid", + options: ServiceOptions{DenyUserAgents: []string{`BadBot/.*`}}, + }, + { + name: "deny-ip justifies trusted-proxy on its own", + options: ServiceOptions{DenyIPs: []string{"203.0.113.0/24"}, TrustedProxies: []string{"10.0.0.0/8"}}, + }, + { + name: "trusted-proxy still needs a consumer", + options: ServiceOptions{TrustedProxies: []string{"10.0.0.0/8"}}, + expectedError: "trusted-proxy requires", + }, + { + name: "deny-ip with client-ip-header requires trusted-proxy", + options: ServiceOptions{DenyIPs: []string{"203.0.113.0/24"}, ClientIPHeader: "CF-Connecting-IP"}, + expectedError: "requires trusted-proxy", + }, + { + name: "invalid deny-ip entry", + options: ServiceOptions{DenyIPs: []string{"not-an-ip"}}, + expectedError: "deny-ip", + }, + { + name: "invalid deny-user-agent entry", + options: ServiceOptions{DenyUserAgents: []string{"("}}, + expectedError: "deny-user-agent", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.options.Validate() + + if tt.expectedError == "" { + require.NoError(t, err) + } else { + require.ErrorIs(t, err, ErrServiceOptionsInvalid) + require.ErrorContains(t, err, tt.expectedError) + } + }) + } +} + +func TestDenyList_ValidateDenyHealthCheck(t *testing.T) { + tests := []struct { + name string + options ServiceOptions + path string + expectError bool + }{ + { + name: "deny-ip with a root health check path", + options: ServiceOptions{DenyIPs: []string{"203.0.113.0/24"}}, + path: "/", + expectError: true, + }, + { + name: "deny-user-agent with a root health check path", + options: ServiceOptions{DenyUserAgents: []string{`BadBot/.*`}}, + path: "/", + expectError: true, + }, + { + name: "deny rules with a dedicated health check path", + options: ServiceOptions{DenyIPs: []string{"203.0.113.0/24"}}, + path: "/up", + }, + { + name: "no deny rules with a root health check path", + options: ServiceOptions{}, + path: "/", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + targetOptions := defaultTargetOptions + targetOptions.HealthCheckConfig.Path = tt.path + + err := validateDenyHealthCheck(tt.options, targetOptions) + + if tt.expectError { + require.ErrorIs(t, err, ErrServiceOptionsInvalid) + require.ErrorContains(t, err, "health-check-path") + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/internal/server/ip_allow_list.go b/internal/server/ip_allow_list.go index 551f3f04..72fe7d25 100644 --- a/internal/server/ip_allow_list.go +++ b/internal/server/ip_allow_list.go @@ -251,10 +251,10 @@ func parseIPPrefixes(entries []string, flagName string) ([]netip.Prefix, error) } func (so ServiceOptions) validateAllowIPs() error { - // Rate limiting resolves the client the same way, so it is a second - // legitimate reason to declare the proxies in front of this one. - if len(so.TrustedProxies) > 0 && len(so.AllowIPs) == 0 && so.RateLimit <= 0 { - return fmt.Errorf("%w: trusted-proxy requires allow-ip or rate-limit", ErrServiceOptionsInvalid) + // Rate limiting and the deny list resolve the client the same way, so each + // is its own legitimate reason to declare the proxies in front of this one. + if len(so.TrustedProxies) > 0 && len(so.AllowIPs) == 0 && so.RateLimit <= 0 && len(so.DenyIPs) == 0 { + return fmt.Errorf("%w: trusted-proxy requires allow-ip, deny-ip or rate-limit", ErrServiceOptionsInvalid) } if len(so.AllowIPs) > 0 && so.ClientIPHeader != "" && len(so.TrustedProxies) == 0 { diff --git a/internal/server/router.go b/internal/server/router.go index 8c81a410..63a63360 100644 --- a/internal/server/router.go +++ b/internal/server/router.go @@ -361,6 +361,10 @@ func (r *Router) DeployService(name string, targetURLs, readerURLs []string, opt return err } + if err := validateDenyHealthCheck(options, targetOptions); err != nil { + return err + } + if err := validateRateLimitHealthCheck(options, targetOptions); err != nil { return err } diff --git a/internal/server/service.go b/internal/server/service.go index 47b0891f..78467eb6 100644 --- a/internal/server/service.go +++ b/internal/server/service.go @@ -153,6 +153,13 @@ type ServiceOptions struct { // Empty (the default) serves everyone. See ip_allow_list.go for which // address is matched and why. AllowIPs []string `json:"allow_ips,omitempty"` + // DenyIPs refuses the given addresses and CIDR ranges, checked before + // AllowIPs: an address on both lists is denied. Empty (the default) denies + // nobody. See deny_list.go for which address is matched and why. + DenyIPs []string `json:"deny_ips,omitempty"` + // DenyUserAgents refuses requests whose full User-Agent matches one of + // these RE2 patterns, compiled once at deploy. Checked after the IP rules. + DenyUserAgents []string `json:"deny_user_agents,omitempty"` // TrustedProxies names the proxies in front of this one, allowing AllowIPs // and the rate limit to be matched against the forwarded chain rather than // the connecting peer. @@ -288,6 +295,10 @@ func (so ServiceOptions) Validate() error { return err } + if err := so.validateDeny(); err != nil { + return err + } + if err := so.validateRateLimit(); err != nil { return err } @@ -362,6 +373,7 @@ type Service struct { middleware http.Handler basicAuth *basicAuthCredential allowedIPs *ipAllowList + denyRules *denyList rateLimiter *rateLimiter redirects *pathRuleSet rewrites *pathRuleSet @@ -696,6 +708,7 @@ func (s *Service) initialize(options ServiceOptions, targetOptions TargetOptions s.middleware = middleware s.basicAuth = s.resolveBasicAuth(options) s.allowedIPs = s.resolveIPAllowList(options) + s.denyRules = s.resolveDenyList(options) s.rateLimiter = s.resolveRateLimiter(options) return nil @@ -866,6 +879,11 @@ func (s *Service) createMiddleware(options ServiceOptions, targetOptions TargetO func (s *Service) serviceRequestWithTarget(w http.ResponseWriter, r *http.Request) { LoggingRequestContext(r).Service = s.name + // First, even before the allow list: an address on both lists is denied. + if s.rejectDenied(w, r) { + return + } + if s.rejectDisallowedIP(w, r) { return }