diff --git a/README.md b/README.md index 91460ff..a9c56fd 100644 --- a/README.md +++ b/README.md @@ -509,7 +509,8 @@ request reaches the app. `waf` takes the checks: -3. The tag lists. +3. The header limits, then the tag lists. A client in `SWW_ALLOW_NETS` skips the + header limits. 4. The static lists, `SWW_ALLOW_NETS` and `SWW_DENY_NETS`. 5. The bans. 6. The lookup of the client's AS number and country. @@ -521,7 +522,7 @@ request reaches the app. 11. The rate limits. 12. The trap paths. 13. The rule files. -14. The header limits, and then the Core Rule Set. +14. The Core Rule Set. 15. The tag refusals again, so that `exploitsignature` can refuse too. 16. A path under `/_sww/` is answered `404`, so that the app never sees that prefix. @@ -537,7 +538,7 @@ request reaches the app. A refused visitor is answered with `SWW_BAN_RESPONSE`. A `block` rule or a Core Rule Set match answers `403` and bans nobody, since the Core Rule Set has false positives; such refusals count toward `SWW_ERROR_BURST_THRESHOLD` instead. A -request over the header limits of step 14 is answered `431`, and bans nobody +request over the header limits of step 3 is answered `431`, and bans nobody either. `rproxy` itself answers `405` to a method it does not pass on, and the status for a size or time limit, such as `413` for a body too large. diff --git a/SPEC.md b/SPEC.md index ca33c60..1ed784c 100644 --- a/SPEC.md +++ b/SPEC.md @@ -149,10 +149,12 @@ field gives that name. `refusal_limit` and `byte_limits` give theirs in - An IPv6 client is its group: the netblock of `SWW_IPV6_GROUP_PREFIX` around its address, a /64 by default, since one abuser usually controls a whole /64. A client is therefore one IPv4 address or one IPv6 group. -3. The tag lists (`tag_lists`; see "Request tags"). Each list that holds the - client adds its tag. Next, `routewatch_tags` adds the client's `asn=` and +3. The header limits (`header_limits`; see "Answering in time"). They come first + after `SWW_ALLOW_NETS`, since they only count header fields and cookies. + Next, the tag lists (`tag_lists`; see "Request tags"). Each list that holds + the client adds its tag. Next, `routewatch_tags` adds the client's `asn=` and `prefix=` tags while `SWW_ROUTEWATCH_URL` is set. A client in - `SWW_ALLOW_NETS` gets no tag. + `SWW_ALLOW_NETS` skips the header limits and gets no tag. 4. The static lists. A client in `SWW_ALLOW_NETS` skips steps 5 to 15, the byte limits and the error burst. A client in `SWW_DENY_NETS` is refused (`deny_nets`); a client in both lists is let through. @@ -185,10 +187,9 @@ field gives that name. `refusal_limit` and `byte_limits` give theirs in refused and bans the client. 13. The rule files (`rule_files`; see "Rule files"). Matching stops at the first rule that refuses. -14. The header limits (`header_limits`; see "Answering in time"), and then the - Core Rule Set (`core_rule_set`; see "Attack detection"). A Core Rule Set - match is refused with `403` while `SWW_WAF_MODE` is `block`, and only logged - and alerted while it is `detect`. +14. The Core Rule Set (`core_rule_set`; see "Attack detection"). A match is + refused with `403` while `SWW_WAF_MODE` is `block`, and only logged and + alerted while it is `detect`. 15. The tag refusals again (`tag_deny`), so that `exploitsignature` can refuse too. 16. A path under `/_sww/` is answered `404`, so that the app never sees that @@ -241,11 +242,16 @@ apply. ## Bans Until a ban ends, every request from its netblock is refused with -`SWW_BAN_RESPONSE`. The netblock is the client: its IPv4 address, widened to -`SWW_BAN_SCOPE_V4_PREFIX`, or its IPv6 group. At the defaults a ban covers one -address or one /64, so a permanent ban does not reach neighbours who did -nothing. Requests refused under a ban are not counted against limits. They are -counted in the client's history and in the ban's notes. +`SWW_BAN_RESPONSE`, unless it is over the header limits. The netblock is the +client: its IPv4 address, widened to `SWW_BAN_SCOPE_V4_PREFIX`, or its IPv6 +group. At the defaults a ban covers one address or one /64, so a permanent ban +does not reach neighbours who did nothing. Requests refused under a ban are not +counted against limits. They are counted in the client's history and in the +ban's notes. + +A request over the header limits is refused with `431` before the bans are +checked (see "Answering in time"). It makes no ban, makes no ban permanent and +is not counted in a ban's notes. An offence is a request `waf` holds against the client. It is one of: @@ -2025,7 +2031,6 @@ waits on these, which run in the background: In `enforce` mode, a question does not wait for GeoJS when it is a request: -- over the header limits; - from a client on a fetched blocklist, while `SWW_BLOCKLIST_ACTION` is `deny`; - from a client on the CrowdSec decision list; - that would take its client over a rate limit at 100% of it, since a percentage @@ -2037,8 +2042,8 @@ number and country stay unknown until GeoJS has answered. So while `SWW_UNKNOWN_LIMIT_PERCENT` does not apply to it until then, so not waiting never lowers its limit. -The static lists and bans come before the lookup, so they never wait. These can -still wait for GeoJS: +The header limits, the static lists and bans come before the lookup, so they +never wait. These can still wait for GeoJS: - a country list, which needs the answer; - a rate limit a percentage has lowered, which needs the answer; @@ -2057,13 +2062,17 @@ either one: Real clients never reach these limits. The answer is `deny` with `431`, and the log line's action is `request_header_too_large`. The refusal makes no ban and is -no offence. The rate limits count the request as usual, so a client that keeps -sending such requests breaks a limit. +no offence. + +The header limits are the first check after `SWW_ALLOW_NETS`. A request over +them is refused before any later check runs. So it causes no lookup, no query to +an outside source and no rule match, and the rate limits do not count it. A +client under a ban or in `SWW_DENY_NETS` also gets `431` for such a request. The limits apply on every path and in every `SWW_WAF_MODE`. A client in `SWW_ALLOW_NETS` skips them, as it skips the Core Rule Set. In `observe` mode -such a request is answered `allow` with `would_action`, and the Core Rule Set -does not read it. +such a request is answered `allow` with `would_action`, and no other check runs +on it. The Core Rule Set runs each of its rules on each cookie, so thousands of small cookies would keep it busy for over a second. Within the limits, one long cookie diff --git a/internal/waf/answertime_test.go b/internal/waf/answertime_test.go index c6e7208..bf5bd4c 100644 --- a/internal/waf/answertime_test.go +++ b/internal/waf/answertime_test.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/httptest" "net/netip" + "path/filepath" "slices" "strconv" "strings" @@ -16,6 +17,7 @@ import ( "sneak.berlin/go/smallwebwaf/internal/bans" "sneak.berlin/go/smallwebwaf/internal/lookup" + "sneak.berlin/go/smallwebwaf/internal/lookup/lookuptest" "sneak.berlin/go/smallwebwaf/internal/ratelimit" "sneak.berlin/go/smallwebwaf/internal/reputation" "sneak.berlin/go/smallwebwaf/internal/requestlog" @@ -216,93 +218,199 @@ func TestObserveModeAllowsARequestOverTheHeaderLimitsUnread(t *testing.T) { } } -func TestRequestsOverTheHeaderLimitsCountTowardTheRateLimits(t *testing.T) { +func TestTheRateLimitsDoNotCountRequestsOverTheHeaderLimits(t *testing.T) { t.Parallel() s := startWAF(t, "", map[string]string{rateLimitPerMinute: "1"}) - ask := func() answer { + + for range 2 { req := s.question(siteA, client, "/") req.Header.Set(cookieHeader, cookies(cookieLimit+1)) - - return do(t, req) + wantDisposition(t, do(t, req), tooLarge, waf.DispositionDeny) } - wantDisposition(t, ask(), tooLarge, waf.DispositionDeny) - wantDisposition(t, ask(), http.StatusForbidden, waf.DispositionDeny) - - lines := s.out.requestLines(t, 2) - wantLine(t, lines[1], http.StatusForbidden, requestlog.ActionClientIsRateLimited) + wantDisposition(t, s.ask(client, "/"), http.StatusOK, waf.DispositionAllow) } -func TestTheHeaderLimitsComeAfterTheTrapPathsAndTheRuleFiles(t *testing.T) { +func TestTheHeaderLimitsComeBeforeEveryCheckThatRefuses(t *testing.T) { t.Parallel() + // The lookup database places client in Germany. + lookupDB := filepath.Join(t.TempDir(), "ipinfo_lite.mmdb") + lookuptest.Write(t, lookupDB, map[string]lookuptest.Network{ + client + "/32": {ASN: asnDE, ASName: asNameDE, Country: "DE"}, + }) + for _, tc := range []struct { - name, path, action string + name string + env map[string]string + path string + // prepare gets the server ready, at now, before the question. + prepare func(t *testing.T, server *waf.Server, now time.Time) }{ - {"a trap path", "/wp-login.php", requestlog.ActionTemporarilyBanned}, - {"a block rule", "/blocked", requestlog.ActionBlockedByBlockRule}, + {"SWW_DENY_NETS", map[string]string{denyNets: client}, "/", leaveAsItIs}, + {"a ban", nil, "/", banClient}, + { + "the country list", map[string]string{ + lookupSource: fileSource, lookupDBPath: lookupDB, allowedCountries: "FR", + }, + "/", leaveAsItIs, + }, + {"a blocklist", map[string]string{blocklistURLs: dropURL}, "/", listClient}, + { + "the CrowdSec decision list", + map[string]string{crowdSecURL: lapi, crowdSecKey: bouncerKey}, "/", decideOnClient, + }, + { + "a DNS blocklist", map[string]string{ + dnsblZones: dnsblZone, dnsblResolver: noResolver, reputationAction: actionDeny, + }, + "/", func(_ *testing.T, server *waf.Server, _ time.Time) { + loadVerdicts(server, map[string][]string{client: {dnsblZone}}) + }, + }, + { + "AbuseIPDB", + map[string]string{abuseIPDBKey: accountKey, reputationAction: actionDeny}, + "/", func(_ *testing.T, server *waf.Server, _ time.Time) { + // Over SWW_ABUSEIPDB_MIN_SCORE, 75 by default. + loadScores(server, map[string]int64{client: 80}) + }, + }, + { + "SWW_TAG_DENY", map[string]string{tagLists: "vpn=" + vpnList, tagDeny: "vpn"}, + "/", func(t *testing.T, server *waf.Server, _ time.Time) { + t.Helper() + loadLists(t, server, map[string][]string{vpnList: {client}}) + }, + }, + {"the rate limits", map[string]string{rateLimitPerMinute: "1"}, "/", countClient}, + { + "a trap path", map[string]string{trapPaths: trapPathList}, "/wp-login.php", + leaveAsItIs, + }, + { + "a block rule", map[string]string{rulesDir: writeRules(t, testRules)}, "/blocked", + leaveAsItIs, + }, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - s := startWAF(t, "", map[string]string{ - trapPaths: trapPathList, rulesDir: writeRules(t, testRules), - }) + s := startWAF(t, "", tc.env) + tc.prepare(t, s.server, s.clock.Now()) + req := s.question(siteA, client, tc.path) req.Header.Set(cookieHeader, cookies(cookieLimit+1)) - wantDisposition(t, do(t, req), http.StatusForbidden, waf.DispositionDeny) - wantLine(t, s.out.requestLine(t), http.StatusForbidden, tc.action) + wantDisposition(t, do(t, req), tooLarge, waf.DispositionDeny) + wantLine(t, s.out.requestLine(t), tooLarge, requestlog.ActionRequestHeaderTooLarge) }) } } +func TestARequestOverTheHeaderLimitsRunsNoOtherCheck(t *testing.T) { + t.Parallel() + + // Each check after the header limits leaves a mark on the log line of a + // request it reads: a tag, routewatch_tags_missing, a rule id, the + // counts, or the Core Rule Set's time. + path := filepath.Join(t.TempDir(), "ipinfo_lite.mmdb") + lookuptest.Write(t, path, map[string]lookuptest.Network{ + client + "/32": {ASN: asnDE, ASName: asNameDE, Country: "DE"}, + }) + s := startWAF(t, "", map[string]string{ + lookupSource: fileSource, lookupDBPath: path, + tagLists: "vpn=" + vpnList, + routeWatchURL: startRouteWatch(t, map[string]standInRoute{client: {prefixDE, 64496}}), + dnsblZones: dnsblZone, reputationAction: actionLog, + rulesDir: writeRules(t, "noted path log ^/\n"), + wafMode: block, + }) + loadLists(t, s.server, map[string][]string{vpnList: {client}}) + loadVerdicts(s.server, map[string][]string{client: {dnsblZone}}) + + over := s.question(siteA, client, "/") + over.Header.Set(cookieHeader, cookies(cookieLimit+1)) + wantDisposition(t, do(t, over), tooLarge, waf.DispositionDeny) + + within := s.question(siteA, client, "/") + within.Header.Set(cookieHeader, cookies(cookieLimit)) + wantDisposition(t, do(t, within), http.StatusOK, waf.DispositionAllow) + + lines := s.out.requestLines(t, 2) + + wantTags(t, lines[0]) + wantTagsMissing(t, lines[0], "") + + if lines[0].RuleIDs != nil || lines[0].Counts != (ratelimit.Counts{}) || + lines[0].DurationCoreRuleSetMS != nil { + t.Errorf("rule ids %v, counts %+v and Core Rule Set time %v, want none", + lines[0].RuleIDs, lines[0].Counts, lines[0].DurationCoreRuleSetMS) + } + + // The request within the limits shows that each check leaves its mark. + wantTags(t, lines[1], + "asn=64496", "country=DE", "dnsbl="+dnsblZone, "exploitsignature", "vpn") + wantTagsMissing(t, lines[1], "no_answer_yet") + + if lines[1].RuleIDs == nil || lines[1].Counts.Minute != 1 || + lines[1].DurationCoreRuleSetMS == nil { + t.Errorf("rule ids %v, counts %+v and Core Rule Set time %v, want each", + lines[1].RuleIDs, lines[1].Counts, lines[1].DurationCoreRuleSetMS) + } +} + func TestNotWaitingForGeoJSNeverLowersAClientsLimit(t *testing.T) { t.Parallel() + // The client's next request takes it over the whole limit, so it does + // not wait. geojsURL, _ := startGeoJS(t) s := startWAF(t, geojsURL, map[string]string{ - unknownLimitPercent: "0", lookupTimeout: "1h", + unknownLimitPercent: "0", lookupTimeout: "1h", rateLimitPerMinute: "1", }) - req := s.question(siteA, fromDE, "/") - req.Header.Set(cookieHeader, cookies(cookieLimit+1)) + countClient(t, s.server, s.clock.Now()) - wantDisposition(t, do(t, req), tooLarge, waf.DispositionDeny) + wantDisposition(t, s.ask(fromDE, "/"), http.StatusForbidden, waf.DispositionDeny) line := s.out.requestLine(t) - wantLine(t, line, tooLarge, requestlog.ActionRequestHeaderTooLarge) + wantLine(t, line, http.StatusForbidden, requestlog.ActionClientIsRateLimited) if line.RateLimitPercent != nil { t.Errorf("limit percentage %d, want none", *line.RateLimitPercent) } - - // The client is under no ban, and GeoJS places it in a country. - wantDisposition(t, s.ask(fromDE, "/"), http.StatusOK, waf.DispositionAllow) } func TestAClientGeoJSPlacedInNoCountryCountsAsUnknownWithoutWaiting(t *testing.T) { t.Parallel() + // The client's next request takes it over the whole limit, so it does + // not wait. geojsURL, _ := startGeoJS(t) - s := startWAF(t, geojsURL, map[string]string{unknownLimitPercent: "0"}) + s := startWAF(t, geojsURL, map[string]string{ + unknownLimitPercent: "0", rateLimitPerMinute: "1", + }) s.server.GeoJS.Load([]lookup.Answer{{ Client: netip.MustParsePrefix(client + "/32"), ASN: "AS64512", Answered: s.clock.Now(), Used: s.clock.Now(), }}) + countClient(t, s.server, s.clock.Now()) - req := s.question(siteA, client, "/") - req.Header.Set(cookieHeader, cookies(cookieLimit+1)) + wantDisposition(t, s.ask(client, "/"), http.StatusForbidden, waf.DispositionDeny) - wantDisposition(t, do(t, req), http.StatusForbidden, waf.DispositionDeny) - wantLine(t, s.out.requestLine(t), http.StatusForbidden, - requestlog.ActionClientIsRateLimited) + line := s.out.requestLine(t) + wantLine(t, line, http.StatusForbidden, requestlog.ActionClientIsRateLimited) + + if line.RateLimitPercent == nil || *line.RateLimitPercent != 0 { + t.Errorf("limit percentage %v, want 0", line.RateLimitPercent) + } } -func TestObserveModeWaitsForGeoJSForARequestOverTheHeaderLimits(t *testing.T) { +func TestObserveModeAsksGeoJSNothingForARequestOverTheHeaderLimits(t *testing.T) { t.Parallel() - geojsURL, _ := startGeoJS(t) + geojsURL, asked := startGeoJS(t) s := startWAF(t, geojsURL, map[string]string{ mode: observe, addLookupHeaders: on, lookupTimeout: "1h", }) @@ -312,10 +420,9 @@ func TestObserveModeWaitsForGeoJSForARequestOverTheHeaderLimits(t *testing.T) { got := do(t, req) wantDisposition(t, got, http.StatusOK, waf.DispositionAllow) - asn, country := got.header.Get("X-Client-Asn"), got.header.Get("X-Client-Country") - if asn != asnDE || country != "DE" { - t.Errorf("answered with the AS number %q and the country %q, want %q and DE", - asn, country, asnDE) + if len(asked()) != 0 || got.header.Get("X-Client-Asn") != "" { + t.Errorf("GeoJS was asked about %v, and the answer gave the AS number %q, "+ + "want neither", asked(), got.header.Get("X-Client-Asn")) } } diff --git a/internal/waf/headerlimits.go b/internal/waf/headerlimits.go index c6ab162..c18e990 100644 --- a/internal/waf/headerlimits.go +++ b/internal/waf/headerlimits.go @@ -10,22 +10,23 @@ import ( "sneak.berlin/go/smallwebwaf/internal/requestlog" ) -// The header limits: the most cookies and header fields a request may have -// when the checks reach the Core Rule Set. Real clients never reach them. +// The header limits: the most cookies and header fields a request may have. +// Real clients never reach them. const ( maxCookies = 50 maxHeaderFields = 100 ) -// headerLimits refuses a request over the header limits, with 431. Its figures -// are the request's header fields and cookies, and the limit of each. +// headerLimits refuses a request with more than maxHeaderFields header fields +// or more than maxCookies cookies, with 431. Its figures are the request's +// header fields and cookies, and the limit of each. type headerLimits struct{} func (headerLimits) name() string { return "header_limits" } func (headerLimits) check(_ context.Context, rq *request, _ time.Time) *verdict { fields, cookies := rq.headerCounts() - if !overHeaderLimits(fields, cookies) { + if fields <= maxHeaderFields && cookies <= maxCookies { return nil } @@ -38,13 +39,6 @@ func (headerLimits) check(_ context.Context, rq *request, _ time.Time) *verdict } } -// overHeaderLimits reports whether a request with these counts of header fields -// and cookies, as headerCounts counts them, is over the header limits: it has -// more than maxHeaderFields fields or more than maxCookies cookies. -func overHeaderLimits(fields, cookies int) bool { - return fields > maxHeaderFields || cookies > maxCookies -} - // headerCounts returns the request's header fields, each value of a header sent // twice being one, and its cookies, each part of a Cookie header between // semicolons being one unless it is empty or only spaces and tabs. The Core diff --git a/internal/waf/heuristics.go b/internal/waf/heuristics.go index fbaa571..450cb10 100644 --- a/internal/waf/heuristics.go +++ b/internal/waf/heuristics.go @@ -41,8 +41,11 @@ const ( // beforeLookup are the heuristics that run before the client's AS number and // country are looked up, so that a client they refuse is not looked up. +// header_limits comes first: it only counts, and a request it refuses causes no +// other work. func beforeLookup() []heuristic { return []heuristic{ + headerLimits{}, tagLists{}, routeWatchTags{}, denyNets{}, @@ -64,7 +67,6 @@ func afterLookup() []heuristic { rateLimits{}, trapPaths{}, ruleFiles{}, - headerLimits{}, coreRuleSet{}, tagDeny{}, } diff --git a/internal/waf/waf.go b/internal/waf/waf.go index 17631b5..9d5ed66 100644 --- a/internal/waf/waf.go +++ b/internal/waf/waf.go @@ -143,10 +143,9 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // refusedWhateverTheAnswer reports whether the request is one that does not wait // for GeoJS, since the checks will refuse it at now whatever GeoJS answers about -// its client: the heuristic crowdsec, header_limits, blocklists or rate_limits -// will refuse it, as a function in that heuristic's file says. In observe mode, -// which refuses nothing, none is. rproxy lets a request through when -// waf answers too late. +// its client: the heuristic crowdsec, blocklists or rate_limits will refuse it, +// as a function in that heuristic's file says. In observe mode, which refuses +// nothing, none is. rproxy lets a request through when waf answers too late. func (rq *request) refusedWhateverTheAnswer(now time.Time) bool { if rq.h.config.Observe { return false @@ -154,8 +153,7 @@ func (rq *request) refusedWhateverTheAnswer(now time.Time) bool { _, decided := rq.crowdSecDecisionInForce(now) - return decided || overHeaderLimits(rq.headerCounts()) || - rq.refusedByBlocklists(rq.h.lists.ListedBy(rq.client)) || + return decided || rq.refusedByBlocklists(rq.h.lists.ListedBy(rq.client)) || rq.overAWholeRateLimit(now) }