diff --git a/README.md b/README.md index ee52a25..c19488d 100644 --- a/README.md +++ b/README.md @@ -543,9 +543,10 @@ request reaches the app. A refused visitor is answered with `SWW_WAF_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_WAF_REFUSALS_LIMIT_PER_MINUTE` -instead. A 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. +instead. A request over the header limits of step 3 is answered `414` for a +request line over 8 KiB and `431` otherwise. It 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. ### Clients and bans diff --git a/SPEC.md b/SPEC.md index ad75904..dc2b63c 100644 --- a/SPEC.md +++ b/SPEC.md @@ -151,11 +151,11 @@ field gives that name. `refusal_limit` and `byte_limits` give theirs in 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 header limits (`header_limits`; see "Answering in time"). They come first - after `SWW_WAF_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_WAF_ROUTEWATCH_URL` is set. A client in - `SWW_WAF_ALLOW_NETS` skips the header limits and gets no tag. + after `SWW_WAF_ALLOW_NETS`, since they only measure lines and 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_WAF_ROUTEWATCH_URL` is set. + A client in `SWW_WAF_ALLOW_NETS` skips the header limits and gets no tag. 4. The static lists. A client in `SWW_WAF_ALLOW_NETS` skips steps 5 to 15, the byte limits and the refusal limit. A client in `SWW_WAF_DENY_NETS` is refused (`deny_nets`); a client in both lists is let through. @@ -253,9 +253,9 @@ 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. +A request over the header limits is refused with `414` or `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: @@ -2403,7 +2403,7 @@ answer but the last below carries `X-Sww-Disposition`. of: - `SWW_WAF_BAN_RESPONSE`, for a refusal; - `403`, for a `block` rule or the Core Rule Set; - - `431`, for a request over the header limits; + - `414` or `431`, for a request over the header limits; - `404`, for a path under `/_sww/` that passes the checks, so that the app never sees that prefix. - `X-Sww-Disposition: close`, status `200`, no body. This takes the place of a @@ -2465,22 +2465,31 @@ never wait. These can still wait for GeoJS: - a rule file; - the Core Rule Set. -The header limits are fixed, with no setting. `waf` refuses a request over -either one: +The header limits are fixed, with no setting. A line's length is in bytes and +does not count its line ending. `waf` refuses a request whose request line is +longer than 8 KiB with `414`. An example is `GET /search?q=` followed by 30,000 +characters. +It refuses a request with `431` for any of: + +- a header line longer than 8 KiB. Each value of a header sent twice is a line + of its own. `Host` counts as a line. - more than 50 cookies. Each part of its `Cookie` headers between semicolons counts as one, unless it is empty or only spaces and tabs. - more than 100 header fields. A header sent twice counts twice. `Host` and `Transfer-Encoding` do not count. `X-Request-ID` and `X-Forwarded-Proto` do. -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. +Real clients rarely reach these limits. nginx refuses lines over 8 KiB by +default, and Apache's default limit is about the same. The answer is `deny`. The +log line's action is `request_line_too_long` for `414` and +`request_header_too_large` for `431`. The refusal makes no ban and is no +offence. The header limits are the first check after `SWW_WAF_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_WAF_DENY_NETS` also gets `431` for such a request. +client under a ban or in `SWW_WAF_DENY_NETS` also gets `414` or `431` for such a +request. The limits apply on every path and in every `SWW_WAF_CORE_RULE_SET_MODE`. A client in `SWW_WAF_ALLOW_NETS` skips them, as it skips the Core Rule Set. In @@ -2488,10 +2497,10 @@ client in `SWW_WAF_ALLOW_NETS` skips them, as it skips the Core Rule Set. In 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 -or query string can still keep it busy for a fraction of a second. So enough -such requests at once can overload `waf` past -`SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`. +cookies would keep it busy for over a second. One 30 KB cookie or query string +would keep it busy for a third of a second or more. A request within the limits +keeps the Core Rule Set busy for less time. Enough such requests at once can +still overload `waf` past `SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`. ### The report diff --git a/docs/request-log.md b/docs/request-log.md index f3a1209..6ae0027 100644 --- a/docs/request-log.md +++ b/docs/request-log.md @@ -102,7 +102,8 @@ Every one of these lines has: ban this request made or made permanent; - `country_denied`, `client_is_rate_limited`, `blocked_by_block_rule` and `blocked_by_core_rule_set`: refused by that check; - - `request_header_too_large`: over a header limit; + - `request_line_too_long`: a request line over the header limits; + - `request_header_too_large`: headers over the header limits; - `own_endpoint`: answered at one of `waf`'s own endpoints, or `404` for a path under `/_sww/`. - `would_action`: in `observe` mode, what `enforce` mode would have done with a @@ -126,7 +127,10 @@ Every one of these lines has: - `rate_limits`: `window`, `count` and `limit`; - `trap_paths`: `path`, the trap path asked for; - `rule_files`: `rule_id` and `rule_action`, of the rule that refused it; - - `header_limits`: `fields`, `fields_limit`, `cookies` and `cookies_limit`; + - `header_limits`: for a request line, `request_line_bytes` and + `request_line_bytes_limit`; for headers, `longest_header_line_bytes`, + `header_line_bytes_limit`, `fields`, `fields_limit`, `cookies` and + `cookies_limit`; - `core_rule_set`: `score`, and `threshold`, `SWW_WAF_CORE_RULE_SET_SCORE_THRESHOLD`; - `refusal_limit`: `window`, `count` and `limit`, of the refusal limit; diff --git a/docs/settings.md b/docs/settings.md index 3a08ca1..7160bd4 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -432,8 +432,9 @@ A few limits are fixed rather than settings: - at most 20,000 anomaly counters are kept; - each alert destination's queue holds at most 1,000 alerts; - `waf` holds at most 10,000 answered requests awaiting their reports; -- `waf` refuses a request with more than 50 cookies or more than 100 header - fields (see "Answering in time" under "`waf`" in [`SPEC.md`](../SPEC.md)); +- `waf` refuses a request with a request line or header line over 8 KiB, more + than 50 cookies or more than 100 header fields (see "Answering in time" under + "`waf`" in [`SPEC.md`](../SPEC.md)); - `rproxy` takes a request line and headers of at most 32 KiB, and closes a kept-open connection idle for 120 seconds; - `rproxy` reads at most 1 MiB of the body of `waf`'s answer, and answers `502` diff --git a/internal/requestlog/requestlog.go b/internal/requestlog/requestlog.go index c8676f5..1f730b6 100644 --- a/internal/requestlog/requestlog.go +++ b/internal/requestlog/requestlog.go @@ -21,7 +21,12 @@ const ( // the app. ActionForward = "forward" - // ActionRequestHeaderTooLarge is a request over the header limits. + // ActionRequestLineTooLong is a request whose request line is over the + // header limits' longest line. + ActionRequestLineTooLong = "request_line_too_long" + + // ActionRequestHeaderTooLarge is a request whose headers are over the + // header limits. ActionRequestHeaderTooLarge = "request_header_too_large" // ActionClientIsRateLimited is a request refused because it took its @@ -144,7 +149,7 @@ type Line struct { // taken with a request it would have refused: ActionClientDenied, // ActionTemporarilyBanned, ActionPermanentlyBanned, ActionCountryDenied, // ActionClientIsRateLimited, ActionBlockedByBlockRule, - // ActionBlockedByCoreRuleSet, or, in waf, + // ActionBlockedByCoreRuleSet, or, in waf, ActionRequestLineTooLong or // ActionRequestHeaderTooLarge for the header limits. WouldAction string `json:"would_action,omitempty"` // Heuristic is the name of the heuristic that refused the request, in diff --git a/internal/waf/answertime_test.go b/internal/waf/answertime_test.go index 197b3d2..3e50108 100644 --- a/internal/waf/answertime_test.go +++ b/internal/waf/answertime_test.go @@ -39,6 +39,7 @@ const ( // The header limits SPEC.md gives. cookieLimit = 50 headerFieldLimit = 100 + lineLimit = 8192 // cookieHeader is the header the cookie limit counts. cookieHeader = "Cookie" // tooLarge is how waf refuses a request over the header limits. @@ -107,11 +108,119 @@ func TestHeaderLimitRefusalNamesItsHeuristicCountsAndLimits(t *testing.T) { fillHeaders(req, headerFieldLimit+1) do(t, req) + // The longest header line is X-Request-ID's, with its 26-character id. wantHeuristic( t, s.out.requestLine(t), "header_limits", - `{"fields": 101, "fields_limit": 100, "cookies": 0, "cookies_limit": 50}`, + `{"longest_header_line_bytes": 40, "header_line_bytes_limit": 8192, + "fields": 101, "fields_limit": 100, "cookies": 0, "cookies_limit": 50}`, + ) +} + +// longQuery returns a path whose request line, sent by Go's client as +// "GET HTTP/1.1", is lineBytes long. +func longQuery(lineBytes int) string { + const around = len("GET /?q= HTTP/1.1") + + return "/?q=" + strings.Repeat("a", lineBytes-around) +} + +func TestARequestLineOverTheLimitIsRefusedWith414BeforeAnyOtherCheck( + t *testing.T, +) { + t.Parallel() + + for _, tc := range []struct { + name string + lineBytes int + status int + action string + }{ + {"at the limit", lineLimit, http.StatusOK, requestlog.ActionForward}, + { + "over the limit", lineLimit + 1, http.StatusRequestURITooLong, + requestlog.ActionRequestLineTooLong, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + s := startWAF(t, "", map[string]string{coreRuleSetMode: block}) + got := do(t, s.question(siteA, client, longQuery(tc.lineBytes))) + line := s.out.requestLine(t) + + if tc.status == http.StatusOK { + wantDisposition(t, got, tc.status, waf.DispositionAllow) + } else { + wantDisposition(t, got, tc.status, waf.DispositionDeny) + } + + wantLine(t, line, tc.status, tc.action) + + refused := tc.status != http.StatusOK + if refused != (line.DurationCoreRuleSetMS == nil) { + t.Errorf( + "Core Rule Set time %v for a request refused: %t", + line.DurationCoreRuleSetMS, + refused, + ) + } + }) + } +} + +func TestAHeaderLineOverTheLimitIsRefusedWith431(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + make func(*http.Request) + // refused is true for a request over the limit, and false for one at it. + refused bool + }{ + {"a header line at the limit", func(req *http.Request) { + req.Header.Set("X-Long", strings.Repeat("a", lineLimit-len("X-Long: "))) + }, false}, + {"a header line over the limit", func(req *http.Request) { + req.Header.Set("X-Long", strings.Repeat("a", lineLimit+1-len("X-Long: "))) + }, true}, + {"one cookie over the limit", func(req *http.Request) { + req.Header.Set(cookieHeader, "a="+strings.Repeat("a", lineLimit)) + }, true}, + {"a Host line over the limit", func(req *http.Request) { + req.Host = strings.Repeat("a", lineLimit+1-len("Host: ")) + }, true}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + s := startWAF(t, "", nil) + req := s.question(siteA, client, "/") + tc.make(req) + + got := do(t, req) + + if tc.refused { + wantDisposition(t, got, tooLarge, waf.DispositionDeny) + } else { + wantDisposition(t, got, http.StatusOK, waf.DispositionAllow) + } + }) + } +} + +func TestRequestLineRefusalNamesItsHeuristicLengthAndLimit(t *testing.T) { + t.Parallel() + + s := startWAF(t, "", nil) + do(t, s.question(siteA, client, longQuery(lineLimit+1))) + + wantHeuristic( + t, + s.out.requestLine(t), + "header_limits", + `{"request_line_bytes": 8193, "request_line_bytes_limit": 8192}`, ) } diff --git a/internal/waf/headerlimits.go b/internal/waf/headerlimits.go index 2857e2f..19c9b09 100644 --- a/internal/waf/headerlimits.go +++ b/internal/waf/headerlimits.go @@ -10,16 +10,22 @@ import ( "sneak.berlin/go/smallwebwaf/internal/requestlog" ) -// The header limits: the most cookies and header fields a request may have. -// Real clients never reach them. +// The header limits: the most cookies and header fields a request may have, +// and the longest its request line and each header line may be, in bytes, not +// counting the line ending. Real clients rarely reach them. maxLineBytes is +// what nginx allows by default. const ( maxCookies = 50 maxHeaderFields = 100 + maxLineBytes = 8 << 10 ) -// 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. +// headerLimits refuses a request with a request line longer than maxLineBytes, +// with 414. It refuses a request with a header line longer than maxLineBytes, +// more than maxHeaderFields header fields or more than maxCookies cookies, with +// 431. Its figures are the request line's length and its limit for 414. For +// 431 they are the longest header line, the header fields and the cookies, +// each with its limit. type headerLimits struct{} func (headerLimits) name() string { return "header_limits" } @@ -29,20 +35,59 @@ func (headerLimits) check( rq *request, _ time.Time, ) *verdict { + requestLine := rq.requestLineBytes() + if requestLine > maxLineBytes { + return &verdict{ + action: requestlog.ActionRequestLineTooLong, + figures: map[string]any{ + "request_line_bytes": requestLine, + "request_line_bytes_limit": maxLineBytes, + }, + } + } + + longest := rq.longestHeaderLineBytes() fields, cookies := rq.headerCounts() - if fields <= maxHeaderFields && cookies <= maxCookies { + + if longest <= maxLineBytes && fields <= maxHeaderFields && + cookies <= maxCookies { return nil } return &verdict{ action: requestlog.ActionRequestHeaderTooLarge, figures: map[string]any{ - "fields": fields, "fields_limit": maxHeaderFields, + "longest_header_line_bytes": longest, + "header_line_bytes_limit": maxLineBytes, + "fields": fields, "fields_limit": maxHeaderFields, "cookies": cookies, "cookies_limit": maxCookies, }, } } +// requestLineBytes returns the length of the request's request line, such as +// "GET /search?q=x HTTP/1.1", not counting its line ending. The target is the +// one the visitor sent, which rproxy's question carries unchanged. +func (rq *request) requestLineBytes() int { + return len(rq.in.Method) + len(" ") + len(rq.in.RequestURI) + + len(" ") + len(rq.in.Proto) +} + +// longestHeaderLineBytes returns the length of the request's longest header +// line, such as "Cookie: a=1", not counting its line ending. Each value of a +// header sent twice is a line of its own, and Host is a line too. +func (rq *request) longestHeaderLineBytes() int { + longest := len("Host: ") + len(rq.in.Host) + + for name, values := range rq.in.Header { + for _, value := range values { + longest = max(longest, len(name)+len(": ")+len(value)) + } + } + + return longest +} + // 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 b928eab..5879c3c 100644 --- a/internal/waf/heuristics.go +++ b/internal/waf/heuristics.go @@ -41,8 +41,8 @@ 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. +// header_limits comes first: it only measures and counts, and a request it +// refuses causes no other work. func beforeLookup() []heuristic { return []heuristic{ headerLimits{}, diff --git a/internal/waf/request.go b/internal/waf/request.go index 5c94377..c945d32 100644 --- a/internal/waf/request.go +++ b/internal/waf/request.go @@ -177,7 +177,7 @@ func requestHeaders(r *http.Request, names []string) map[string]string { // check is the one place where a request can be refused. It returns nil to let // the request through. The heuristics of checkClient decide: a request they // refuse is answered with SWW_WAF_BAN_RESPONSE, 403 for a block rule or the Core -// Rule Set, or 431 for the header limits. In observe mode nothing is refused, +// Rule Set, or 414 or 431 for the header limits. In observe mode nothing is refused, // and the log line names what enforce mode would have done. ctx is the // request's own context. func (rq *request) check(ctx context.Context) *refusal { @@ -196,6 +196,8 @@ func (rq *request) check(ctx context.Context) *refusal { case action == requestlog.ActionBlockedByBlockRule || action == requestlog.ActionBlockedByCoreRuleSet: return &refusal{status: http.StatusForbidden, action: action} + case action == requestlog.ActionRequestLineTooLong: + return &refusal{status: http.StatusRequestURITooLong, action: action} case action == requestlog.ActionRequestHeaderTooLarge: return &refusal{ status: http.StatusRequestHeaderFieldsTooLarge, diff --git a/internal/waf/waf_test.go b/internal/waf/waf_test.go index 8d9c441..45e08bc 100644 --- a/internal/waf/waf_test.go +++ b/internal/waf/waf_test.go @@ -1690,7 +1690,12 @@ func TestALargeBodyAnnouncedAndLongHeadersAreNoReasonToRefuse(t *testing.T) { strings.NewReader("a body")) req.Header.Set(waf.ContentLengthHeader, "1073741824") req.Header.Set(forwardedFor, otherClient) - req.Header.Set("X-Long", strings.Repeat("x", 40<<10)) + + // 40,000 bytes of headers, over rproxy's 32 KiB, in lines within the + // header limits. + for i := range 5 { + req.Header.Set("X-Long-"+strconv.Itoa(i), strings.Repeat("x", 8000)) + } got := do(t, req) wantDisposition(t, got, http.StatusOK, waf.DispositionAllow)