diff --git a/README.md b/README.md index d725014..594a480 100644 --- a/README.md +++ b/README.md @@ -348,7 +348,7 @@ Every setting the code reads, with its default: | `SWW_WAF_CORE_RULE_SET_SCORE_THRESHOLD` | `5` | The Core Rule Set score at which a request is a match, a whole number or `off`; one critical rule scores 5. | | `SWW_WAF_CORE_RULE_SET_DISABLED_RULES` | `920340,920420,920440,920640,930130,930140` | Core Rule Set rule ids switched off; a list you give replaces the default, so repeat these six to keep them off. | | `SWW_WAF_CORE_RULE_SET_EXEMPT_PATHS` | empty | Path prefixes the Core Rule Set does not inspect. | -| `SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE` | `off` | How much of a request body the Core Rule Set reads, a size up to `64K`; `off` reads none. | +| `SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE` | `off` | How much of a request body the Core Rule Set reads, a size up to `8K`; `off` reads none. | | `SWW_WAF_CORE_RULE_SET_EXCLUSIONS_FILE` | unset | A file of Core Rule Set exclusions, as Coraza directives, that switch rules off for some fields or paths only. | | `SWW_WAF_APPS` | unset | The apps each hostname serves, such as `git.example.com:gitea;blog.example.com:wordpress`; only those apps' fixes apply there. "Attack detection" lists the apps. Listing `forgejo` brings gitea's fixes too. Unset, every app's fixes apply to every hostname. | | `SWW_WAF_BLOCKLIST_URLS` | empty | Blocklists to fetch, text files of addresses and netblocks one to a line, such as `https://www.spamhaus.org/drop/drop.txt`. | diff --git a/SPEC.md b/SPEC.md index 4cf27b5..8a2a71f 100644 --- a/SPEC.md +++ b/SPEC.md @@ -704,11 +704,12 @@ files, the limits and the bans still apply to the client that sends it. rules is loaded yet. - `SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE` switches body inspection on, up - to that size. It takes at most `64K`, since `waf` gets no more of a body than - that (see "The answer"). Form data and multipart bodies are read up to the - size. JSON and XML bodies are read only when no larger than the size, since - those cannot be read in part. Any other body is not read: the Core Rule Set - would read it as form data, where binary content such as a git push trips + to that size. It takes at most `8K`, so that `waf` answers in time (see + "Answering in time"). Form data and multipart bodies are read up to the size. + JSON and XML bodies are read only when no larger than the size, since those + cannot be read in part. A JSON body over 1 KiB is not read either, whatever + the size (see "Answering in time"). Any other body is not read: the Core Rule + Set would read it as form data, where binary content such as a git push trips rules written for text. A body the Core Rule Set cannot parse adds 5 to the score, and so does a multipart body cut by the limit in the middle of a part's header. Body inspection suits apps whose forms carry no code. @@ -2397,9 +2398,7 @@ In `observe` mode a request that `enforce` mode would refuse is answered answered `404`. The Core Rule Set inspects the body a question carries only while -`SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE` is set. It inspects at most 64 KiB -less one byte. So it does not inspect a JSON or XML body of 64 KiB or more, -which may have been cut short. +`SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE` is set, and at most 8 KiB of it. ### Answering in time @@ -2474,12 +2473,26 @@ client in `SWW_WAF_ALLOW_NETS` skips them, as it skips the Core Rule Set. In `observe` mode 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. One 30 KB cookie or query string -would keep it busy for a third of a second or more. A request with several long -`Cookie` lines, each within the limits, can keep it as busy. Enough such -requests at once can still overload `waf` past -`SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`. +The Core Rule Set is the one check a visitor can keep busy for long. It runs +each of its rules on each part of a request, so thousands of small cookies would +keep it busy for over a second. These fixed limits bound what it reads: + +- the header limits above; +- `rproxy`'s limit of 32 KiB on the request line and headers together (see + "`rproxy`'s limits"); +- 8 KiB of a body, the most `SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE` takes, + or 1 KiB of a JSON body. + +Coraza names each value in a JSON body by the whole path to it, such as +`json.users.0.name`. So the names and values the Core Rule Set reads in a JSON +body can add up to more than the body. From 1 KiB of JSON, Coraza makes at most +8 KiB of them, no more than the largest body of another kind. + +A request can reach all of these at once. At paranoia level 4, with no rule +switched off, the Core Rule Set takes under a second on one core of a current +server to inspect it. So `waf` answers it within the default 2 seconds, even +after the default 800 milliseconds of waiting for GeoJS. Enough such requests at +once can still overload `waf` past `SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`. ### The report diff --git a/docs/settings.md b/docs/settings.md index 39e6531..9a62b76 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -251,10 +251,9 @@ tag that no request can get stops the start. A request can get only these: - `SWW_WAF_CORE_RULE_SET_EXEMPT_PATHS` (default empty): path prefixes the Core Rule Set does not inspect. - `SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE` (default `off`): `off`, or how - much of a request body the Core Rule Set reads, a size of at most `64K`. A - question carries no more of a body than 64 KiB, and the Core Rule Set reads at - most 64 KiB less one byte of it, whatever the value (see "The answer" in - [`SPEC.md`](../SPEC.md)). + much of a request body the Core Rule Set reads, a size of at most `8K`. A + larger size would let one request keep it busy past `rproxy`'s timeout (see + "Answering in time" in [`SPEC.md`](../SPEC.md)). - `SWW_WAF_CORE_RULE_SET_EXCLUSIONS_FILE` (default unset): a file of Core Rule Set exclusions, written as Coraza directives. It is loaded before the Core Rule Set's rules, so it holds only `SecAction` and `SecRule` lines that diff --git a/internal/config/waf.go b/internal/config/waf.go index cc166e3..513d253 100644 --- a/internal/config/waf.go +++ b/internal/config/waf.go @@ -365,8 +365,8 @@ var ( "it names the file to look clients up in", ) errDBPathUnused = errors.New("only file reads it") - errOver64K = errors.New( - "is more than 64K, the most waf gets of a body") + errOver8K = errors.New( + "is more than 8K, the most the Core Rule Set can read in time") errNotDurationAboveZero = errors.New( "is not a duration above zero, such as 1h or 7d") errNotNumberAboveZero = errors.New( @@ -1601,16 +1601,16 @@ func durationOrDays(value string) (time.Duration, error) { } // parseCoreRuleSetBodyInspectionSize reads the most of a request body the Core Rule Set -// reads: a size as parseSize reads it, or off, but at most 64K, since waf -// gets no more of a body than that. +// reads: a size as parseSize reads it, or off, but at most 8K, so that one +// request cannot keep the Core Rule Set busy past rproxy's timeout. func parseCoreRuleSetBodyInspectionSize(value string) (int64, error) { limit, err := parseSize(value) if err != nil { return 0, err } - if limit > 64*kibibyte { - return 0, fmt.Errorf("%q %w", value, errOver64K) + if limit > 8*kibibyte { + return 0, fmt.Errorf("%q %w", value, errOver8K) } return limit, nil diff --git a/internal/config/waf_test.go b/internal/config/waf_test.go index c90c0f2..4fe64f4 100644 --- a/internal/config/waf_test.go +++ b/internal/config/waf_test.go @@ -616,7 +616,7 @@ func TestCoreRuleSetSettings(t *testing.T) { coreRuleSetScoreThreshold: "10", coreRuleSetDisabledRules: "942100, 920350", coreRuleSetExemptPaths: "/api/, /static/", - coreRuleSetBodyInspectionSize: "32K", + coreRuleSetBodyInspectionSize: "4K", exclusionsFile: "/etc/sww/crs-exclusions.conf", }, config.WAF{ @@ -625,7 +625,7 @@ func TestCoreRuleSetSettings(t *testing.T) { CoreRuleSetScoreThreshold: 10, CoreRuleSetDisabledRules: []int{942100, 920350}, CoreRuleSetExemptPaths: []string{"/api/", "/static/"}, - CoreRuleSetBodyInspectionSize: 32 << 10, + CoreRuleSetBodyInspectionSize: 4 << 10, CoreRuleSetExclusionsFile: "/etc/sww/crs-exclusions.conf", }, }, @@ -762,15 +762,15 @@ func TestInvalidAppsStopsTheStart(t *testing.T) { } } -func TestCoreRuleSetBodyInspectionSizeOf64K(t *testing.T) { +func TestCoreRuleSetBodyInspectionSizeOf8K(t *testing.T) { t.Parallel() cfg := wafFromEnvironment( t, - environment{coreRuleSetBodyInspectionSize: "64K"}, + environment{coreRuleSetBodyInspectionSize: "8K"}, ) - if cfg.CoreRuleSetBodyInspectionSize != 64<<10 { - t.Errorf("64K read as %d", cfg.CoreRuleSetBodyInspectionSize) + if cfg.CoreRuleSetBodyInspectionSize != 8<<10 { + t.Errorf("8K read as %d", cfg.CoreRuleSetBodyInspectionSize) } } @@ -819,11 +819,13 @@ func TestInvalidCoreRuleSetSettingStopsTheStart(t *testing.T) { coreRuleSetBodyInspectionSize, "128KB", `"128KB" is not a size such as 512K, 100M or 5G, or off`, }, - {coreRuleSetBodyInspectionSize, "65K", `"65K" is more than 64K, the most waf ` + - `gets of a body`}, { - coreRuleSetBodyInspectionSize, "65537", - `"65537" is more than 64K, the most waf gets of a body`, + coreRuleSetBodyInspectionSize, "9K", + `"9K" is more than 8K, the most the Core Rule Set can read in time`, + }, + { + coreRuleSetBodyInspectionSize, "8193", + `"8193" is more than 8K, the most the Core Rule Set can read in time`, }, } { t.Run(tc.name+"="+tc.value, func(t *testing.T) { diff --git a/internal/coreruleset/coreruleset.go b/internal/coreruleset/coreruleset.go index 4a3b9ed..83ae46a 100644 --- a/internal/coreruleset/coreruleset.go +++ b/internal/coreruleset/coreruleset.go @@ -481,12 +481,21 @@ func (c *CoreRuleSet) Inspect( return Result{RuleIDs: ids, Score: score(tx)}, read, err } +// mostJSONRead is the largest JSON body the Core Rule Set reads, even while +// SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE is larger. Coraza names each +// value in a JSON body by the whole path to it, so what it gives the Core +// Rule Set to read can outgrow the body: up to 8 times its size, and up to +// 4 KiB for a small body. 1 KiB of JSON thus gives at most 8 KiB, no more +// than the largest body of another kind. +const mostJSONRead = 1 << 10 + // readBody reads body, the body of the request in tx, which has run on // the request's headers, while SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE is // set and the body is of a kind the Core Rule Set reads: form data and // multipart, of which it reads the first c.bodyInspectionSize bytes, and // JSON and XML, which it reads only when they are no longer than that, -// since they cannot be read in part. +// since they cannot be read in part. The Core Rule Set reads no JSON body +// longer than mostJSONRead either. // readBody reads one byte past c.bodyInspectionSize, to tell which they // are, gives the Core Rule Set what it reads, and returns what it read and // the error that ended the reading early, if one did. @@ -499,6 +508,8 @@ func (c *CoreRuleSet) readBody( } inPart := false + // largest is the largest JSON or XML body the Core Rule Set reads. + largest := c.bodyInspectionSize // How the body is read is a variable of the transaction, which only // Coraza's interface for plugins reads. state := tx.(plugintypes.TransactionState) //nolint:forcetypeassert // every one is @@ -506,14 +517,16 @@ func (c *CoreRuleSet) readBody( switch state.Variables().RequestBodyProcessor().Get() { case "URLENCODED", "MULTIPART": inPart = true - case "JSON", "XML": + case "JSON": + largest = min(largest, mostJSONRead) + case "XML": default: return nil, nil } read, err := io.ReadAll(io.LimitReader(body, c.bodyInspectionSize+1)) - if inPart || (err == nil && int64(len(read)) <= c.bodyInspectionSize) { + if inPart || (err == nil && int64(len(read)) <= largest) { // Coraza holds what it reads of the body in memory, up to the // limit, so this cannot fail. _, _, _ = tx.WriteRequestBody(read) diff --git a/internal/coreruleset/coreruleset_test.go b/internal/coreruleset/coreruleset_test.go index 7db965d..52a3146 100644 --- a/internal/coreruleset/coreruleset_test.go +++ b/internal/coreruleset/coreruleset_test.go @@ -390,22 +390,15 @@ func TestMoreParametersThanCorazaKeepsIsAMatch(t *testing.T) { matched(900300), ) - // So it is with the fields of a form data or JSON body. + // So it is with the fields of a form data body. A JSON body the Core + // Rule Set reads, of at most mostJSONRead bytes, cannot hold that many. crs = readingBodies(t) - for _, tc := range []struct{ header, body string }{ - {formData, strings.Repeat("a=1&", 999) + attack}, - {jsonBody, `{"a":[` + strings.Repeat("1,", 998) + `1],"id":"` + injection + `"}`}, - } { - wantBody(t, crs, post(tc.header), tc.body, matched(942100), tc.body) - } + body := strings.Repeat("a=1&", 999) + attack + wantBody(t, crs, post(formData), body, matched(942100), body) - for _, tc := range []struct{ header, body string }{ - {formData, strings.Repeat("a=1&", 1000) + attack}, - {jsonBody, `{"a":[` + strings.Repeat("1,", 999) + `1],"id":"` + injection + `"}`}, - } { - wantBody(t, crs, post(tc.header), tc.body, matched(900300), tc.body) - } + body = strings.Repeat("a=1&", 1000) + attack + wantBody(t, crs, post(formData), body, matched(900300), body) } func TestRedirectURIMayNameALocalAddress(t *testing.T) { @@ -1512,13 +1505,15 @@ const ( wpTags = "/wp-json/wp/v2/tags" ) -// readingBodiesUpTo64K returns the Core Rule Set as sww runs it by default, +// readingBodiesUpTo8K returns the Core Rule Set as sww runs it by default, // but reading bodies up to the largest SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE. -func readingBodiesUpTo64K(t *testing.T) *coreruleset.CoreRuleSet { +func readingBodiesUpTo8K(t *testing.T) *coreruleset.CoreRuleSet { t.Helper() return shared(t, coreruleset.Params{ - ParanoiaLevel: 1, DisabledRules: defaultDisabledRules, BodyInspectionSize: 64<<10 - 1, + ParanoiaLevel: 1, + DisabledRules: defaultDisabledRules, + BodyInspectionSize: mostBodyRead, }) } @@ -1582,7 +1577,7 @@ func TestWordPressAttacksAreRefused(t *testing.T) { const threshold = 5 // SWW_WAF_CORE_RULE_SET_SCORE_THRESHOLD's default - defaults, bodies := atDefaults(t), readingBodiesUpTo64K(t) + defaults, bodies := atDefaults(t), readingBodiesUpTo8K(t) for _, attack := range []string{ sqlInjection, @@ -1693,7 +1688,7 @@ func wordpressRESTAddresses(route string) []string { func TestWordPressExclusionsFollowEveryRESTAddress(t *testing.T) { t.Parallel() - crs := readingBodiesUpTo64K(t) + crs := readingBodiesUpTo8K(t) content := `{"content":"` + wordpressBlock + `"}` for _, route := range []string{ @@ -1919,7 +1914,7 @@ func TestWordPressCommandAfterASeparatorIsRefusedOnSearchesAndNames( wantResult(t, crs, get(target+command), matched(932235)) } - bodies := readingBodiesUpTo64K(t) + bodies := readingBodiesUpTo8K(t) for _, p := range wordpressBodyFields(command) { r := request{http.MethodPost, p.target, []string{p.contentType}} @@ -2143,7 +2138,7 @@ func TestWordPressBodyFieldsAreCheckedInTheQuery(t *testing.T) { func TestWordPressBodyNamesAndAddressesAreExcludedOnlyThere(t *testing.T) { t.Parallel() - crs := readingBodiesUpTo64K(t) + crs := readingBodiesUpTo8K(t) for _, v := range wordpressWords() { for _, field := range []string{"title", "excerpt", "name"} { @@ -2262,7 +2257,7 @@ func TestWordPressExclusionsAreNarrow(t *testing.T) { length = "Content-Length: 2" ) - crs := readingBodiesUpTo64K(t) + crs := readingBodiesUpTo8K(t) saving := request{ http.MethodPost, wpPost, @@ -2473,19 +2468,35 @@ func TestFormDataAndMultipartAreReadUpToTheLimit(t *testing.T) { func TestJSONAndXMLAreReadOnlyWhenNoLargerThanTheLimit(t *testing.T) { t.Parallel() - crs := readingBodies(t) + // The limit is the body inspection size, but at most mostJSONRead for + // JSON. smallSize is a body inspection size under mostJSONRead. + const smallSize = 512 - for _, tc := range []struct{ header, head, tail string }{ - {jsonBody, `{"q":"` + injection + `","pad":"`, `"}`}, - {xmlBody, "" + injection + "", ""}, + bodies, smallBodies := readingBodies(t), shared(t, coreruleset.Params{ + ParanoiaLevel: 1, DisabledRules: defaultDisabledRules, + BodyInspectionSize: smallSize, + }) + jsonHead, jsonTail := `{"q":"`+injection+`","pad":"`, `"}` + + for _, tc := range []struct { + crs *coreruleset.CoreRuleSet + header, head, tail string + limit int + }{ + {bodies, jsonBody, jsonHead, jsonTail, mostJSONRead}, + {smallBodies, jsonBody, jsonHead, jsonTail, smallSize}, + { + bodies, xmlBody, "" + injection + "", "", + bodyInspectionSize, + }, } { - fits := padded(tc.head, tc.tail, bodyInspectionSize) - wantBody(t, crs, post(tc.header), fits, matched(942100), fits) + fits := padded(tc.head, tc.tail, tc.limit) + wantBody(t, tc.crs, post(tc.header), fits, matched(942100), fits) - larger := padded(tc.head, tc.tail, bodyInspectionSize+1) + larger := padded(tc.head, tc.tail, tc.limit+1) wantBody( t, - crs, + tc.crs, post(tc.header), larger, coreruleset.Result{}, diff --git a/internal/coreruleset/inspectiontime_test.go b/internal/coreruleset/inspectiontime_test.go new file mode 100644 index 0000000..a6bf51d --- /dev/null +++ b/internal/coreruleset/inspectiontime_test.go @@ -0,0 +1,371 @@ +package coreruleset_test + +import ( + "maps" + "net/http" + "net/http/httptest" + "net/netip" + "slices" + "strconv" + "strings" + "testing" + "time" + + "sneak.berlin/go/smallwebwaf/internal/coreruleset" +) + +// The bounds on what a visitor can send, as SPEC.md gives them. +const ( + // lineLimit is the longest request line or header line. + lineLimit = 8 << 10 + // headerLimit is the most bytes rproxy reads of a request line and + // headers together. + headerLimit = 32 << 10 + // mostCookies and mostHeaderFields are waf's limits on cookies and header + // fields. + mostCookies = 50 + mostHeaderFields = 100 + // mostBodyRead is the largest SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE. + mostBodyRead = 8 << 10 + // mostJSONRead is the largest JSON body the Core Rule Set reads. + mostJSONRead = 1 << 10 +) + +// inspectionTimeLimit is the most time the Core Rule Set may take on one +// request. It is the default of rproxy's SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS, 2 +// seconds, less the default of SWW_WAF_ASN_AND_COUNTRY_LOOKUP_TIMEOUT_MS, 800 +// milliseconds, which a question can also wait for GeoJS, and less 200 +// milliseconds for waf's other checks. +const inspectionTimeLimit = time.Second + +// text returns n bytes of letters, digits and spaces, which the Core Rule Set +// takes longer over than one letter repeated. +func text(n int) string { + const words = "select 0x2f from tbl_1 where name like abc_9 " + + return strings.Repeat(words, n/len(words)+1)[:n] +} + +// visitorRequest is a request a visitor can send, at the most of one or more +// of the bounds above. +type visitorRequest struct { + name string + target string + // header holds the headers besides Host. + header http.Header + // contentType is the body's type, "" for no body. + contentType string + body string +} + +// longestTarget returns a target whose request line, "POST +// HTTP/1.1", is lineLimit bytes long. +func longestTarget() string { + const around = len("POST /?q= HTTP/1.1") + + return "/?q=" + strings.ReplaceAll(text(lineLimit-around), " ", "+") +} + +// cookieLine returns a Cookie value whose line, "Cookie: ", is +// lineLimit bytes long, made of count cookies. +func cookieLine(count int) string { + const linePrefix = len("Cookie: ") + + cookies := make([]string, count) + for i := range cookies { + cookies[i] = "c" + strconv.Itoa(i) + "=" + } + + // Each cookie is followed by "; ", but for the last. + room := lineLimit - linePrefix - len(strings.Join(cookies, "; ")) + for i := range cookies { + share := room / count + if i == 0 { + share += room % count + } + + cookies[i] += strings.ReplaceAll(text(share), " ", "_") + } + + return strings.Join(cookies, "; ") +} + +// headerValue returns a value whose line, ": ", is n bytes long. +func headerValue(name string, n int) string { + return text(n - len(name) - len(": ")) +} + +// formFields returns a form body of mostBodyRead bytes holding count fields. +func formFields(count int) string { + fields := make([]string, count) + for i := range fields { + fields[i] = "f" + strconv.Itoa(i) + "=" + } + + room := mostBodyRead - len(strings.Join(fields, "&")) + for i := range fields { + share := room / count + if i == 0 { + share += room % count + } + + fields[i] += strings.ReplaceAll(text(share), " ", "+") + } + + return strings.Join(fields, "&") +} + +// multipartBody returns a multipart body of mostBodyRead bytes, with the +// boundary "b", holding fields of about size bytes each. +func multipartBody(size int) string { + var body strings.Builder + + const end = "--b--\r\n" + + for i := 0; ; i++ { + head := "--b\r\nContent-Disposition: form-data; name=\"f" + + strconv.Itoa(i) + "\"\r\n\r\n" + + room := mostBodyRead - body.Len() - len( + end, + ) - len( + head, + ) - len( + "\r\n", + ) + if room <= 0 { + break + } + + body.WriteString(head + text(min(size, room)) + "\r\n") + } + + body.WriteString(end) + + return body.String() +} + +// jsonArrayOfMostElements returns a JSON array of at most mostJSONRead bytes +// holding as many elements as fit, each the number 1. +func jsonArrayOfMostElements() string { + return "[1" + strings.Repeat( + ",1", + (mostJSONRead-len("[1]"))/len(",1"), + ) + "]" +} + +// jsonOfMostElementsUnderAKey returns a JSON body of mostJSONRead bytes +// holding as many elements as fit, each the number 1, in an array under a +// key of 6 bytes. Coraza names each element "json..", so a +// longer key makes longer names. 6 bytes is the longest key that keeps the +// names and values of this body within the 8 KiB Coraza allows. +func jsonOfMostElementsUnderAKey() string { + head := `{"` + text(6) + `":[1` + + return head + strings.Repeat( + ",1", + (mostJSONRead-len(head+"]}"))/len(",1"), + ) + "]}" +} + +// The kinds of body the Core Rule Set reads, and the header that names a +// request's cookies. +const ( + formType = "application/x-www-form-urlencoded" + jsonType = "application/json" + multipartType = "multipart/form-data; boundary=b" + cookieHeader = "Cookie" +) + +// fullLine is the length of each of the request line and the three or four +// header lines that together fill rproxy's limit, with room for the line +// endings and the other headers. +const fullLine = lineLimit - 64 + +// worstCaseRequests are, for each kind of thing a visitor sends, the requests +// that keep the Core Rule Set busiest, within the bounds above. The last +// holds the most of everything at once: Cookie lines keep it busier than +// other header lines, and the JSON body of the most elements under a key +// keeps it busier than the other bodies. +func worstCaseRequests() []visitorRequest { + fullCookieLine := cookieLine(1)[:fullLine-len("Cookie: ")] + + return append( + append(worstCaseHeaders(), worstCaseBodies()...), + visitorRequest{ + name: "the longest request line, three Cookie lines and body", + target: longestTarget()[:fullLine-len("POST HTTP/1.1")], + header: http.Header{cookieHeader: { + fullCookieLine, fullCookieLine, fullCookieLine, + }}, + contentType: jsonType, body: jsonOfMostElementsUnderAKey(), + }, + ) +} + +// worstCaseHeaders are the requests of worstCaseRequests with no body. +func worstCaseHeaders() []visitorRequest { + headerFields := http.Header{} + + for i := range mostHeaderFields - 1 { + name := "X-Field-" + strconv.Itoa(i) + headerFields.Set( + name, + headerValue(name, headerLimit/mostHeaderFields), + ) + } + + return []visitorRequest{ + {name: "the longest request line", target: longestTarget()}, + { + name: "the longest request line, as query parameters", + target: "/?" + strings.Repeat( + "a=1&", + (lineLimit-len("POST /? HTTP/1.1"))/len("a=1&"), + ), + }, + { + name: "the longest Cookie line, as one cookie", + target: "/", + header: http.Header{cookieHeader: {cookieLine(1)}}, + }, + { + name: "the longest Cookie line, as the most cookies", + target: "/", + header: http.Header{cookieHeader: {cookieLine(mostCookies)}}, + }, + {name: "the most header fields", target: "/", header: headerFields}, + { + name: "the longest header lines", + target: "/", + header: http.Header{ + "User-Agent": {headerValue("User-Agent", fullLine)}, + "Referer": {headerValue("Referer", fullLine)}, + "Accept": {headerValue("Accept", fullLine)}, + "X-Long": {headerValue("X-Long", fullLine)}, + }, + }, + } +} + +// worstCaseBodies are the requests of worstCaseRequests whose body holds +// the most the Core Rule Set reads. +func worstCaseBodies() []visitorRequest { + const nested = (mostJSONRead - len(`{"a":1}`)) / len(`{"a":}`) + + return []visitorRequest{ + { + name: "a form body of one field", target: "/", + contentType: formType, body: formFields(1), + }, + { + name: "a form body of the most fields Coraza keeps", target: "/", + contentType: formType, body: formFields(1000), + }, + { + name: "a JSON body of one string", target: "/", + contentType: jsonType, + body: `{"a":"` + text( + mostJSONRead-len(`{"a":""}`), + ) + `"}`, + }, + { + name: "a JSON body of the most elements", target: "/", + contentType: jsonType, body: jsonArrayOfMostElements(), + }, + { + name: "a JSON body of the most elements under a key", target: "/", + contentType: jsonType, body: jsonOfMostElementsUnderAKey(), + }, + { + name: "a JSON body nested deep", target: "/", + contentType: jsonType, + body: strings.Repeat(`{"a":`, nested) + "1" + + strings.Repeat("}", nested), + }, + { + name: "an XML body of many elements", target: "/", + contentType: "application/xml", + body: "" + strings.Repeat( + "x", + (mostBodyRead-len(""))/len("x"), + ) + "", + }, + { + name: "a multipart body of one field", target: "/", + contentType: multipartType, body: multipartBody(mostBodyRead), + }, + { + name: "a multipart body of many fields", target: "/", + contentType: multipartType, body: multipartBody(1), + }, + } +} + +// inspectionTime returns how long crs takes to inspect tc, and fails t unless +// it reads all of tc's body and Coraza parses it. +func inspectionTime( + t *testing.T, crs *coreruleset.CoreRuleSet, tc visitorRequest, +) time.Duration { + t.Helper() + + req := httptest.NewRequestWithContext( + t.Context(), http.MethodPost, tc.target, nil, + ) + maps.Copy(req.Header, tc.header) + + if tc.contentType != "" { + req.Header.Set("Content-Type", tc.contentType) + } + + began := time.Now() + result, read, _ := crs.Inspect( + req, netip.MustParseAddr("192.0.2.1"), strings.NewReader(tc.body), + ) + took := time.Since(began) + + if string(read) != tc.body { + t.Errorf("read %d bytes of %d", len(read), len(tc.body)) + } + + // Rule 900440 matches a body Coraza could not parse. + if slices.Contains(result.RuleIDs, 900440) { + t.Errorf("Coraza could not parse the body") + } + + return took +} + +// TestTheCoreRuleSetInspectsEachWorstCaseRequestInTime runs one at a time, so +// that the tests running in parallel do not slow it. Other work on the host +// can still slow one try, so a request fails only when three tries in a row +// take too long. +// +//nolint:paralleltest // one at a time, as above +func TestTheCoreRuleSetInspectsEachWorstCaseRequestInTime(t *testing.T) { + const tries = 3 + + // The settings that make the Core Rule Set slowest: the highest paranoia + // level, no rule switched off, and the most of a body read. + crs := shared(t, coreruleset.Params{ + ParanoiaLevel: 4, BodyInspectionSize: mostBodyRead, + }) + + for _, tc := range worstCaseRequests() { + t.Run(tc.name, func(t *testing.T) { + var took time.Duration + + for range tries { + took = inspectionTime(t, crs, tc) + if took <= inspectionTimeLimit { + return + } + } + + t.Errorf( + "took %s on the last of %d tries, over %s", + took, tries, inspectionTimeLimit, + ) + }) + } +} diff --git a/internal/waf/proxy.go b/internal/waf/proxy.go index 59500fe..c79153d 100644 --- a/internal/waf/proxy.go +++ b/internal/waf/proxy.go @@ -228,12 +228,9 @@ func newReputation( // without the rules SWW_WAF_CORE_RULE_SET_DISABLED_RULES switches off, reading bodies // up to SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE, with the exclusions in // SWW_WAF_CORE_RULE_SET_EXCLUSIONS_FILE and the fixes of the apps in SWW_WAF_APPS, -// or nil while SWW_WAF_CORE_RULE_SET_MODE is off. It -// reads at most QuestionBodyMaxBytes less one byte: a question's body of -// QuestionBodyMaxBytes may be cut short, and the Core Rule Set does not read a -// JSON or XML body over its limit. An exclusions file that coreruleset.New refuses is -// an error in every mode. While the mode is off, a set file is loaded with the -// Core Rule Set only to check it. +// or nil while SWW_WAF_CORE_RULE_SET_MODE is off. An exclusions file that +// coreruleset.New refuses is an error in every mode. While the mode is off, a +// set file is loaded with the Core Rule Set only to check it. func NewCoreRuleSet(cfg *config.WAF) (*coreruleset.CoreRuleSet, error) { if cfg.CoreRuleSetMode == config.CoreRuleSetModeOff && cfg.CoreRuleSetExclusionsFile == "" { @@ -241,14 +238,11 @@ func NewCoreRuleSet(cfg *config.WAF) (*coreruleset.CoreRuleSet, error) { } coreRuleSet, err := coreruleset.New(coreruleset.Params{ - ParanoiaLevel: cfg.CoreRuleSetParanoiaLevel, - DisabledRules: cfg.CoreRuleSetDisabledRules, - BodyInspectionSize: min( - cfg.CoreRuleSetBodyInspectionSize, - QuestionBodyMaxBytes-1, - ), - ExclusionsFile: cfg.CoreRuleSetExclusionsFile, - Apps: cfg.Apps, + ParanoiaLevel: cfg.CoreRuleSetParanoiaLevel, + DisabledRules: cfg.CoreRuleSetDisabledRules, + BodyInspectionSize: cfg.CoreRuleSetBodyInspectionSize, + ExclusionsFile: cfg.CoreRuleSetExclusionsFile, + Apps: cfg.Apps, }) if err != nil { return nil, fmt.Errorf( diff --git a/internal/waf/waf_test.go b/internal/waf/waf_test.go index f93f88f..93a0e15 100644 --- a/internal/waf/waf_test.go +++ b/internal/waf/waf_test.go @@ -1787,7 +1787,7 @@ func TestCoreRuleSetReadsTheBodyAQuestionCarries(t *testing.T) { t.Parallel() s := startWAF(t, "", map[string]string{ - coreRuleSetMode: block, coreRuleSetBodyInspectionSize: "64K", + coreRuleSetMode: block, coreRuleSetBodyInspectionSize: "8K", }) ask := func(contentType, body string) answer { @@ -1802,10 +1802,10 @@ func TestCoreRuleSetReadsTheBodyAQuestionCarries(t *testing.T) { got := ask(formData, "id=1'%20OR%20'1'='1") wantDisposition(t, got, http.StatusForbidden, waf.DispositionDeny) - // The first 64 KiB of a longer JSON body cannot be parsed, which is no + // The first 8 KiB of a longer JSON body cannot be parsed, which is no // attack, so the Core Rule Set does not read it. - cut := `{"text":"` + strings.Repeat("x", waf.QuestionBodyMaxBytes) - got = ask("application/json", cut[:waf.QuestionBodyMaxBytes]) + cut := `{"text":"` + strings.Repeat("x", 8<<10) + got = ask("application/json", cut) wantDisposition(t, got, http.StatusOK, waf.DispositionAllow) }