rproxy: circuit breaker for asks to waf (closes #392)

Five asks that get no answer in time within five seconds open the
breaker. While open, a request takes
SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION without asking waf, except
one ask a second; the first answer closes it. Slow answers count as
failures. SPEC.md describes it.

The sww test that stops and restarts waf now waits for the breaker to
close before it expects the ban again.

Model: opus-5-5

README.md

@@ -298,7 +298,7 @@ Every setting the code reads, with its default:
 | `SWW_RPROXY_LISTEN_ADDR`                          | `:8080`                                                                                  | Where `rproxy` listens; leave the host part empty.                                                                                                                                                                                                              |
 | `SWW_RPROXY_TRUSTED_PROXIES`                      | `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`                                                | The netblocks whose `X-Forwarded-For` `rproxy` believes; set but empty, it believes none.                                                                                                                                                                       |
 | `SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`               | `2000`                                                                                   | How long `rproxy` waits for `waf`'s answer to a question and to a report, in milliseconds.                                                                                                                                                                      |
-| `SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION`    | `allow`                                                                                  | What `rproxy` does with a request when `waf` gives no answer in time, and while the circuit breaker for asks to `waf` is open: `allow` passes it to the app, `deny` answers `503`.                                                                              |
+| `SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION`    | `allow`                                                                                  | What `rproxy` does with a request when `waf` cannot be reached or does not answer in time: `allow` passes it to the app, `deny` answers `503`.                                                                                                                  |
 | `SWW_RPROXY_MAX_REQUEST_SIZE`                     | `100M`                                                                                   | The largest request body, a size or `off`; a larger one is answered `413`.                                                                                                                                                                                      |
 | `SWW_RPROXY_MAX_RESPONSE_SIZE`                    | `1G`                                                                                     | The largest response body, a size or `off`. The visitor gets `502` in place of a larger one, or it is cut off if it has already started.                                                                                                                        |
 | `SWW_RPROXY_MAX_REQUEST_TIME_MS`                  | `60000`                                                                                  | Two time limits, each this long, in milliseconds or `off`: one for the visitor to send its request line and headers, then one for the whole request to reach the app.                                                                                           |
@@ -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 `8K`; `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 `64K`; `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`.                                                                                                                                     |

SPEC.md

@@ -704,12 +704,11 @@ 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 `8K`, so that `waf` answers in time (see
+  to that size. It takes at most `64K`, since `waf` gets no more of a body than
-  "Answering in time"). Form data and multipart bodies are read up to the size.
+  that (see "The answer"). Form data and multipart bodies are read up to the
-  JSON and XML bodies are read only when no larger than the size, since those
+  size. JSON and XML bodies are read only when no larger than the size, since
-  cannot be read in part. A JSON body over 1 KiB is not read either, whatever
+  those cannot be read in part. Any other body is not read: the Core Rule Set
-  the size (see "Answering in time"). Any other body is not read: the Core Rule
+  would read it as form data, where binary content such as a git push trips
-  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.
@@ -2398,7 +2397,9 @@ 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, and at most 8 KiB of it.
+`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.
 
 ### Answering in time
 
@@ -2473,26 +2474,12 @@ 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 is the one check a visitor can keep busy for long. It runs
+The Core Rule Set runs each of its rules on each cookie, so thousands of small
-each of its rules on each part of a request, so thousands of small cookies would
+cookies would keep it busy for over a second. One 30 KB cookie or query string
-keep it busy for over a second. These fixed limits bound what it reads:
+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
-- the header limits above;
+requests at once can still overload `waf` past
-- `rproxy`'s limit of 32 KiB on the request line and headers together (see
+`SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`.
-  "`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
 
@@ -2597,10 +2584,11 @@ request and ban notes name the site by its host.
 
 `sww rproxy` is the reverse proxy in front of one app. It asks `waf` about each
 request, and passes on what `waf` allows. It writes nothing to disk. The only
-thing it keeps between requests is its circuit breaker for asks to `waf`.
+thing it keeps between requests is its circuit breaker for asks to `waf`. When
-`SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION` decides what is done with a
+`waf` cannot be connected to, does not answer at all, or does not answer within
-request when `waf` gives no answer in time, and while the circuit breaker is
+`SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`,
-open. Its default, `allow`, keeps the site answering.
+`SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION` decides. Its default, `allow`,
+keeps the site answering.
 
 `rproxy` reads only its `SWW_RPROXY_` settings, which `docs/settings.md` lists.
 One of them is required: `SWW_RPROXY_WAF_BACKEND`, `waf`'s URL. It has no
@@ -2618,8 +2606,7 @@ only. The image runs it by default (see "Deployment").
 4. It refuses a request with a method it does not pass on, or one it cannot put
    to `waf` as the visitor sent it (see "Requests `rproxy` refuses itself").
 5. It reads up to the first 64 KiB of the body into memory.
-6. It sends `waf` the question (see "The question"), unless the circuit breaker
+6. It sends `waf` the question (see "The question"). It speaks HTTP/1.1 to
-   keeps it from asking (see "The circuit breaker"). It speaks HTTP/1.1 to
    `waf`, whatever `waf` offers. Go's HTTP/2 client would refuse to send a
    visitor's `Upgrade` header, and most `Connection` headers. The question never
    carries the visitor's `Trailer`.
@@ -2728,10 +2715,9 @@ status `0` and no `disposition`.
 ### The circuit breaker
 
 `rproxy` stops asking `waf` for a while when `waf` keeps failing to answer. An
-ask fails when `waf` gives no answer in time (see "Without an answer from
+ask fails in each case that "Without an answer from `waf`" lists. Any answer in
-`waf`"), or an answer `rproxy` cannot pass on. Any other answer in time
+time succeeds, whatever it decides, and so does an answer `rproxy` cannot pass
-succeeds, whatever it decides. A visitor that goes away while `waf` is being
+on. A visitor that goes away while `waf` is being asked counts as neither.
-asked counts as neither.
 
 The breaker works like this:
 
@@ -2741,8 +2727,8 @@ The breaker works like this:
   It gets the same process warning and `waf_backend_no_answer` as a request
   whose ask failed.
 - While it is open, one request a second still asks `waf`.
-- Each ask that succeeds clears the failures counted so far, and closes the
+- The first ask that succeeds closes it, and the failures before it no longer
-  breaker if it is open.
+  count.
 
 The breaker lives in memory, and a restart of `rproxy` closes it. It has no
 settings.

docs/request-log.md

@@ -238,8 +238,7 @@ meanings where they differ:
 - `disposition`: what was done with the request: `allow`, `deny` or `close`. It
   is `waf`'s answer, or the value of
   `SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION`. It is left out when:
-    - `waf` was not asked and `SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION` did
+    - `rproxy` answered the request itself, as for the health check;
-      not apply, as for the health check;
     - the visitor went away while `waf` was being asked;
     - `waf`'s answer had no valid `X-Sww-Disposition`;
     - `rproxy` could not pass on `waf`'s answer, and answered `502`.

docs/settings.md

@@ -251,9 +251,10 @@ 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 `8K`. A
+  much of a request body the Core Rule Set reads, a size of at most `64K`. A
-  larger size would let one request keep it busy past `rproxy`'s timeout (see
+  question carries no more of a body than 64 KiB, and the Core Rule Set reads at
-  "Answering in time" in [`SPEC.md`](../SPEC.md)).
+  most 64 KiB less one byte of it, whatever the value (see "The answer" 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

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")
-	errOver8K       = errors.New(
+	errOver64K      = errors.New(
-		"is more than 8K, the most the Core Rule Set can read in time")
+		"is more than 64K, the most waf gets of a body")
 	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 8K, so that one
+// reads: a size as parseSize reads it, or off, but at most 64K, since waf
-// request cannot keep the Core Rule Set busy past rproxy's timeout.
+// gets no more of a body than that.
 func parseCoreRuleSetBodyInspectionSize(value string) (int64, error) {
 	limit, err := parseSize(value)
 	if err != nil {
 		return 0, err
 	}
 
-	if limit > 8*kibibyte {
+	if limit > 64*kibibyte {
-		return 0, fmt.Errorf("%q %w", value, errOver8K)
+		return 0, fmt.Errorf("%q %w", value, errOver64K)
 	}
 
 	return limit, nil

internal/config/waf_test.go

@@ -616,7 +616,7 @@ func TestCoreRuleSetSettings(t *testing.T) {
 				coreRuleSetScoreThreshold:     "10",
 				coreRuleSetDisabledRules:      "942100, 920350",
 				coreRuleSetExemptPaths:        "/api/, /static/",
-				coreRuleSetBodyInspectionSize: "4K",
+				coreRuleSetBodyInspectionSize: "32K",
 				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: 4 << 10,
+				CoreRuleSetBodyInspectionSize: 32 << 10,
 				CoreRuleSetExclusionsFile:     "/etc/sww/crs-exclusions.conf",
 			},
 		},
@@ -762,15 +762,15 @@ func TestInvalidAppsStopsTheStart(t *testing.T) {
 	}
 }
 
-func TestCoreRuleSetBodyInspectionSizeOf8K(t *testing.T) {
+func TestCoreRuleSetBodyInspectionSizeOf64K(t *testing.T) {
 	t.Parallel()
 
 	cfg := wafFromEnvironment(
 		t,
-		environment{coreRuleSetBodyInspectionSize: "8K"},
+		environment{coreRuleSetBodyInspectionSize: "64K"},
 	)
-	if cfg.CoreRuleSetBodyInspectionSize != 8<<10 {
+	if cfg.CoreRuleSetBodyInspectionSize != 64<<10 {
-		t.Errorf("8K read as %d", cfg.CoreRuleSetBodyInspectionSize)
+		t.Errorf("64K read as %d", cfg.CoreRuleSetBodyInspectionSize)
 	}
 }
 
@@ -819,13 +819,11 @@ 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, "9K",
+			coreRuleSetBodyInspectionSize, "65537",
-			`"9K" is more than 8K, the most the Core Rule Set can read in time`,
+			`"65537" is more than 64K, the most waf gets of a body`,
-		},
-		{
-			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) {

internal/coreruleset/coreruleset.go

@@ -481,21 +481,12 @@ 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. The Core Rule Set reads no JSON body
+// since they cannot be read in part.
-// 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.
@@ -508,8 +499,6 @@ 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
@@ -517,16 +506,14 @@ func (c *CoreRuleSet) readBody(
 	switch state.Variables().RequestBodyProcessor().Get() {
 	case "URLENCODED", "MULTIPART":
 		inPart = true
-	case "JSON":
+	case "JSON", "XML":
-		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)) <= largest) {
+	if inPart || (err == nil && int64(len(read)) <= c.bodyInspectionSize) {
 		// Coraza holds what it reads of the body in memory, up to the
 		// limit, so this cannot fail.
 		_, _, _ = tx.WriteRequestBody(read)

internal/coreruleset/coreruleset_test.go

@@ -390,15 +390,22 @@ func TestMoreParametersThanCorazaKeepsIsAMatch(t *testing.T) {
 		matched(900300),
 	)
 
-	// So it is with the fields of a form data body. A JSON body the Core
+	// So it is with the fields of a form data or JSON body.
-	// Rule Set reads, of at most mostJSONRead bytes, cannot hold that many.
 	crs = readingBodies(t)
 
-	body := strings.Repeat("a=1&", 999) + attack
+	for _, tc := range []struct{ header, body string }{
-	wantBody(t, crs, post(formData), body, matched(942100), body)
+		{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&", 1000) + attack
+	for _, tc := range []struct{ header, body string }{
-	wantBody(t, crs, post(formData), body, matched(900300), body)
+		{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)
+	}
 }
 
 func TestRedirectURIMayNameALocalAddress(t *testing.T) {
@@ -1505,15 +1512,13 @@ const (
 	wpTags    = "/wp-json/wp/v2/tags"
 )
 
-// readingBodiesUpTo8K returns the Core Rule Set as sww runs it by default,
+// readingBodiesUpTo64K 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 readingBodiesUpTo8K(t *testing.T) *coreruleset.CoreRuleSet {
+func readingBodiesUpTo64K(t *testing.T) *coreruleset.CoreRuleSet {
 	t.Helper()
 
 	return shared(t, coreruleset.Params{
-		ParanoiaLevel:      1,
+		ParanoiaLevel: 1, DisabledRules: defaultDisabledRules, BodyInspectionSize: 64<<10 - 1,
-		DisabledRules:      defaultDisabledRules,
-		BodyInspectionSize: mostBodyRead,
 	})
 }
 
@@ -1577,7 +1582,7 @@ func TestWordPressAttacksAreRefused(t *testing.T) {
 
 	const threshold = 5 // SWW_WAF_CORE_RULE_SET_SCORE_THRESHOLD's default
 
-	defaults, bodies := atDefaults(t), readingBodiesUpTo8K(t)
+	defaults, bodies := atDefaults(t), readingBodiesUpTo64K(t)
 
 	for _, attack := range []string{
 		sqlInjection,
@@ -1688,7 +1693,7 @@ func wordpressRESTAddresses(route string) []string {
 func TestWordPressExclusionsFollowEveryRESTAddress(t *testing.T) {
 	t.Parallel()
 
-	crs := readingBodiesUpTo8K(t)
+	crs := readingBodiesUpTo64K(t)
 	content := `{"content":"` + wordpressBlock + `"}`
 
 	for _, route := range []string{
@@ -1914,7 +1919,7 @@ func TestWordPressCommandAfterASeparatorIsRefusedOnSearchesAndNames(
 		wantResult(t, crs, get(target+command), matched(932235))
 	}
 
-	bodies := readingBodiesUpTo8K(t)
+	bodies := readingBodiesUpTo64K(t)
 
 	for _, p := range wordpressBodyFields(command) {
 		r := request{http.MethodPost, p.target, []string{p.contentType}}
@@ -2138,7 +2143,7 @@ func TestWordPressBodyFieldsAreCheckedInTheQuery(t *testing.T) {
 func TestWordPressBodyNamesAndAddressesAreExcludedOnlyThere(t *testing.T) {
 	t.Parallel()
 
-	crs := readingBodiesUpTo8K(t)
+	crs := readingBodiesUpTo64K(t)
 
 	for _, v := range wordpressWords() {
 		for _, field := range []string{"title", "excerpt", "name"} {
@@ -2257,7 +2262,7 @@ func TestWordPressExclusionsAreNarrow(t *testing.T) {
 		length  = "Content-Length: 2"
 	)
 
-	crs := readingBodiesUpTo8K(t)
+	crs := readingBodiesUpTo64K(t)
 	saving := request{
 		http.MethodPost,
 		wpPost,
@@ -2468,35 +2473,19 @@ func TestFormDataAndMultipartAreReadUpToTheLimit(t *testing.T) {
 func TestJSONAndXMLAreReadOnlyWhenNoLargerThanTheLimit(t *testing.T) {
 	t.Parallel()
 
-	// The limit is the body inspection size, but at most mostJSONRead for
+	crs := readingBodies(t)
-	// JSON. smallSize is a body inspection size under mostJSONRead.
-	const smallSize = 512
 
-	bodies, smallBodies := readingBodies(t), shared(t, coreruleset.Params{
+	for _, tc := range []struct{ header, head, tail string }{
-		ParanoiaLevel: 1, DisabledRules: defaultDisabledRules,
+		{jsonBody, `{"q":"` + injection + `","pad":"`, `"}`},
-		BodyInspectionSize: smallSize,
+		{xmlBody, "<r><q>" + injection + "</q><pad>", "</pad></r>"},
-	})
-	jsonHead, jsonTail := `{"q":"`+injection+`","pad":"`, `"}`
-
-	for _, tc := range []struct {
-		crs                *coreruleset.CoreRuleSet
-		header, head, tail string
-		limit              int
 	} {
-		{bodies, jsonBody, jsonHead, jsonTail, mostJSONRead},
+		fits := padded(tc.head, tc.tail, bodyInspectionSize)
-		{smallBodies, jsonBody, jsonHead, jsonTail, smallSize},
+		wantBody(t, crs, post(tc.header), fits, matched(942100), fits)
-		{
-			bodies, xmlBody, "<r><q>" + injection + "</q><pad>", "</pad></r>",
-			bodyInspectionSize,
-		},
-	} {
-		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, tc.limit+1)
+		larger := padded(tc.head, tc.tail, bodyInspectionSize+1)
 		wantBody(
 			t,
-			tc.crs,
+			crs,
 			post(tc.header),
 			larger,
 			coreruleset.Result{},

internal/coreruleset/inspectiontime_test.go

@@ -1,371 +0,0 @@
-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 <target>
-// 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: <value>", 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, "<name>: <value>", 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.<key>.<index>", 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: "<a>" + strings.Repeat(
-				"<b>x</b>",
-				(mostBodyRead-len("<a></a>"))/len("<b>x</b>"),
-			) + "</a>",
-		},
-		{
-			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,
-			)
-		})
-	}
-}

internal/rproxy/breaker.go

@@ -21,9 +21,8 @@ var errBreakerOpen = errors.New(
 
 // breaker is the circuit breaker for asks to waf. It opens when
 // breakerFailures asks fail within breakerWindow. An ask fails when it gets no
-// answer in time, or an answer rproxy cannot pass on. While it is open, at
+// answer in time. While it is open, at most one request each breakerAskInterval
-// most one request each breakerAskInterval asks waf. Any usable answer closes
+// asks waf. The first answer closes it, whatever it decides.
-// it, whatever it decides, and forgets the failures before it.
 type breaker struct {
 	mu sync.Mutex
 	// failures holds the times of the latest failed asks, oldest first, at most
@@ -53,15 +52,17 @@ func (b *breaker) mayAsk(now time.Time) bool {
 	return true
 }
 
-// noteAnswer notes that an ask got an answer rproxy can use. It closes the
+// noteAnswer notes that an ask got an answer in time. It closes the breaker,
-// breaker and forgets the failures before it.
+// and forgets the failures before it.
 func (b *breaker) noteAnswer() {
 	b.mu.Lock()
 	defer b.mu.Unlock()
 
+	if b.open {
 		b.open = false
 		b.failures = nil
 	}
+}
 
 // noteFailure notes that an ask that ended at now got no answer in time, and
 // opens the breaker if breakerFailures asks have failed within breakerWindow.

internal/rproxy/breaker_test.go

@@ -63,17 +63,17 @@ func TestFiveFailedAsksOverMoreThanFiveSecondsLeaveTheBreakerClosed(
 	}
 }
 
-func TestAnAnswerBetweenFailedAsksKeepsTheBreakerClosed(t *testing.T) {
+func TestAnAnswerBeforeTheBreakerOpensDoesNotForgetFailures(t *testing.T) {
 	t.Parallel()
 
 	now := time.Now()
 	b := &rproxy.Breaker{}
-	fail(b, now, 4, 100*time.Millisecond)
+	fail(b, now, 4, time.Millisecond)
 	b.NoteAnswer()
-	fail(b, now.Add(500*time.Millisecond), 4, 100*time.Millisecond)
+	b.NoteFailure(now.Add(time.Second))
 
-	if !b.MayAsk(now.Add(900 * time.Millisecond)) {
+	if b.MayAsk(now.Add(time.Second)) {
-		t.Error("a request may not ask, want it to")
+		t.Error("a request may ask, want it not to")
 	}
 }
 

internal/rproxy/export_test.go

@@ -12,7 +12,7 @@ func (b *Breaker) MayAsk(now time.Time) bool {
 	return b.b.mayAsk(now)
 }
 
-// NoteAnswer notes that an ask got an answer rproxy can use.
+// NoteAnswer notes that an ask got an answer in time.
 func (b *Breaker) NoteAnswer() {
 	b.b.noteAnswer()
 }

internal/rproxy/rproxy_test.go

@@ -1490,7 +1490,7 @@ func TestVisitorsThatGoAwayWhileWAFIsAskedLeaveTheBreakerClosed(
 	standIn.question(t)
 }
 
-func TestAnswersThatCannotBePassedOnOpenTheBreaker(t *testing.T) {
+func TestAnswersThatCannotBePassedOnLeaveTheBreakerClosed(t *testing.T) {
 	t.Parallel()
 
 	var answering atomic.Bool
@@ -1506,10 +1506,11 @@ func TestAnswersThatCannotBePassedOnOpenTheBreaker(t *testing.T) {
 		tooLarge(w, r)
 	})
 	app := startApp(t, greet)
-	addr, out := startRProxy(t, standIn.URL, app.URL, map[string]string{
+	addr, _ := startRProxy(t, standIn.URL, app.URL, map[string]string{
 		wafBackendTimeout: "60000",
 	})
 
+	// Five of these would open the breaker if they counted as failures.
 	for range 5 {
 		if got := get(t, addr, "/"); got.status != http.StatusBadGateway {
 			t.Errorf("answered %d, want 502", got.status)
@@ -1520,13 +1521,7 @@ func TestAnswersThatCannotBePassedOnOpenTheBreaker(t *testing.T) {
 
 	answering.Store(true)
 	wantGreeting(t, get(t, addr, "/"))
-	out.waitFor(t, "the circuit breaker is open")
+	standIn.question(t)
-
-	select {
-	case <-standIn.questions:
-		t.Error("waf was asked, want it not to be")
-	case <-time.After(10 * pollInterval):
-	}
 }
 
 func TestAPathUnderOwnPrefixNeverReachesTheApp(t *testing.T) {

internal/rproxy/waf.go

@@ -90,7 +90,7 @@ func (rq *request) handleNoUsableAnswer(ctx context.Context, err error) {
 
 // askUnlessBreakerOpen asks waf, unless the circuit breaker is open and it is
 // not yet time for a request to ask. It then tells the breaker whether waf
-// answered in time. An answer rproxy cannot pass on counts as a failure. A
+// answered in time. An answer rproxy cannot pass on still came in time. A
 // visitor that goes away tells the breaker nothing. ctx is the request's own
 // context.
 func (rq *request) askUnlessBreakerOpen(
@@ -104,7 +104,7 @@ func (rq *request) askUnlessBreakerOpen(
 	answer, err := rq.ask(ctx)
 
 	switch {
-	case err == nil:
+	case err == nil, errors.Is(err, errCannotPassOn):
 		b.noteAnswer()
 	case ctx.Err() == nil:
 		b.noteFailure(time.Now())

internal/waf/proxy.go

@@ -228,9 +228,12 @@ 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. An exclusions file that
+// or nil while SWW_WAF_CORE_RULE_SET_MODE is off. It
-// coreruleset.New refuses is an error in every mode. While the mode is off, a
+// reads at most QuestionBodyMaxBytes less one byte: a question's body of
-// set file is loaded with the Core Rule Set only to check it.
+// 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.
 func NewCoreRuleSet(cfg *config.WAF) (*coreruleset.CoreRuleSet, error) {
 	if cfg.CoreRuleSetMode == config.CoreRuleSetModeOff &&
 		cfg.CoreRuleSetExclusionsFile == "" {
@@ -240,7 +243,10 @@ func NewCoreRuleSet(cfg *config.WAF) (*coreruleset.CoreRuleSet, error) {
 	coreRuleSet, err := coreruleset.New(coreruleset.Params{
 		ParanoiaLevel: cfg.CoreRuleSetParanoiaLevel,
 		DisabledRules: cfg.CoreRuleSetDisabledRules,
-		BodyInspectionSize: cfg.CoreRuleSetBodyInspectionSize,
+		BodyInspectionSize: min(
+			cfg.CoreRuleSetBodyInspectionSize,
+			QuestionBodyMaxBytes-1,
+		),
 		ExclusionsFile: cfg.CoreRuleSetExclusionsFile,
 		Apps:           cfg.Apps,
 	})

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: "8K",
+		coreRuleSetMode: block, coreRuleSetBodyInspectionSize: "64K",
 	})
 
 	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 8 KiB of a longer JSON body cannot be parsed, which is no
+	// The first 64 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", 8<<10)
+	cut := `{"text":"` + strings.Repeat("x", waf.QuestionBodyMaxBytes)
-	got = ask("application/json", cut)
+	got = ask("application/json", cut[:waf.QuestionBodyMaxBytes])
 	wantDisposition(t, got, http.StatusOK, waf.DispositionAllow)
 }