waf: decide every visitor request within rproxy's timeout (closes #394)

The Core Rule Set is the one check whose time a visitor can stretch.
At paranoia level 4 it took over 2 s on a request with an 8 KiB
request line, three 8 KiB Cookie lines and a 64 KiB form body.
SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE now takes at most 8K, which
brings that request under 1 s.

A test per kind of input times the Core Rule Set on the worst request
within the limits, at the slowest settings. SPEC.md lists the limits
that bound it.

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` cannot be reached or does not answer in time: `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` 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_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 `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`.                                                                                                                                     |

SPEC.md

@@ -704,13 +704,13 @@ 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
+  to that size. It takes at most `8K`, so that `waf` answers in time (see
-  that (see "The answer"). Form data and multipart bodies are read up to the
+  "Answering in time"). Form data and multipart bodies are read up to the size.
-  size. JSON and XML bodies are read only when no larger than the size, since
+  JSON and XML bodies are read only when no larger than the size, since those
-  those cannot be read in part. Any other body is not read: the Core Rule Set
+  cannot be read in part. Any other body is not read: the Core Rule Set would
-  would read it as form data, where binary content such as a git push trips
+  read it as form data, where binary content such as a git push trips rules
-  rules written for text. A body the Core Rule Set cannot parse adds 5 to the
+  written for text. A body the Core Rule Set cannot parse adds 5 to the score,
-  score, and so does a multipart body cut by the limit in the middle of a part's
+  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.
 - `SWW_WAF_TRAP_PATHS` names paths the app never serves and only scanners ask
   for, such as `/wp-login.php` in front of an app that is not WordPress. This is
@@ -2397,9 +2397,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
+`SWW_WAF_CORE_RULE_SET_BODY_INSPECTION_SIZE` is set, and at most 8 KiB of it.
-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
 
@@ -2474,12 +2472,20 @@ 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
+The Core Rule Set is the one check a visitor can keep busy for long. It runs
-cookies would keep it busy for over a second. One 30 KB cookie or query string
+each of its rules on each part of a request, so thousands of small cookies would
-would keep it busy for a third of a second or more. A request with several long
+keep it busy for over a second. These fixed limits bound what it reads:
-`Cookie` lines, each within the limits, can keep it as busy. Enough such
+
-requests at once can still overload `waf` past
+- the header limits above;
-`SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`.
+- `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.
+
+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
 
@@ -2584,11 +2590,10 @@ 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`. When
+thing it keeps between requests is its circuit breaker for asks to `waf`.
-`waf` cannot be connected to, does not answer at all, or does not answer within
+`SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION` decides what is done with a
-`SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`,
+request when `waf` gives no answer in time, and while the circuit breaker is
-`SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION` decides. Its default, `allow`,
+open. Its default, `allow`, keeps the site answering.
-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
@@ -2606,7 +2611,8 @@ 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"). It speaks HTTP/1.1 to
+6. It sends `waf` the question (see "The question"), unless the circuit breaker
+   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`.
@@ -2694,10 +2700,10 @@ decides, without asking `waf`, while the circuit breaker is open.
 Either way `rproxy` writes a process warning, its log line has
 `waf_backend_no_answer`, and it sends no report.
 
-A visitor can make `waf` slow. A request that keeps `waf` busy past
+`waf` taking longer than `SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS` to decide a request
-`SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`, like any request while `waf` is overloaded,
+is a bug in `waf`. Such a request gets
-gets `SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION`. With `deny`, such a request
+`SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION`, like any request while `waf` is
-gets `503` instead of reaching the app.
+overloaded. With `deny`, it gets `503` instead of reaching the app.
 
 A path under `/_sww/` that `allow` would pass on is answered `404` instead, so
 that the app never sees that prefix.
@@ -2715,10 +2721,10 @@ 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` cannot be connected to, does not answer at all, or does not
+ask fails when `waf` gives no answer in time (see "Without an answer from
-answer within `SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`. Any answer in time succeeds,
+`waf`"), or an answer `rproxy` cannot pass on. Any other answer in time
-whatever it decides, and so does an answer `rproxy` cannot pass on. A visitor
+succeeds, whatever it decides. A visitor that goes away while `waf` is being
-that goes away while `waf` is being asked counts as neither.
+asked counts as neither.
 
 The breaker works like this:
 
@@ -2728,11 +2734,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`.
-- The first ask that succeeds closes it, and the failures before it no longer
+- Each ask that succeeds clears the failures counted so far, and closes the
-  count.
+  breaker if it is open.
-
-A request that `waf` cannot decide within `SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`
-counts as a failure too. Such a request is a bug in `waf`.
 
 The breaker lives in memory, and a restart of `rproxy` closes it. It has no
 settings.

docs/request-log.md

@@ -238,7 +238,8 @@ 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, as for the health check;
+    - `waf` was not asked and `SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION` did
+      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,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
+  much of a request body the Core Rule Set reads, a size of at most `8K`. A
-  question carries no more of a body than 64 KiB, and the Core Rule Set reads at
+  larger size would let one request keep it busy past `rproxy`'s timeout (see
-  most 64 KiB less one byte of it, whatever the value (see "The answer" in
+  "Answering in time" in [`SPEC.md`](../SPEC.md)).
-  [`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
@@ -453,11 +452,9 @@ start, and an invalid value stops the start. Any can be given as a file, as a
   for `waf`'s answer, to a question and to a report. A time in milliseconds;
   never `off`.
 - `SWW_RPROXY_WAF_BACKEND_NO_ANSWER_DISPOSITION` (default `allow`): what is done
-  with a request when `waf` cannot be connected to, does not answer at all, or
+  with a request when `waf` gives no answer in time, and while the circuit
-  does not answer within `SWW_RPROXY_WAF_BACKEND_TIMEOUT_MS`, and while the
+  breaker for asks to `waf` is open. `allow` passes it to the app, and `deny`
-  circuit breaker for asks to `waf` is open. `allow` passes it to the app, and
+  answers `503`. It does not apply to an answer `waf` gives, such as an error.
-  `deny` answers `503`. It does not apply to an answer `waf` gives, such as an
-  error.
 - `SWW_RPROXY_MAX_REQUEST_SIZE` (default `100M`): the largest request body. A
   size or `off`.
 - `SWW_RPROXY_MAX_RESPONSE_SIZE` (default `1G`): the largest response body. A

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(
+	errOver8K       = errors.New(
-		"is more than 64K, the most waf gets of a body")
+		"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
+// reads: a size as parseSize reads it, or off, but at most 8K, so that one
-// gets no more of a body than that.
+// 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 {
+	if limit > 8*kibibyte {
-		return 0, fmt.Errorf("%q %w", value, errOver64K)
+		return 0, fmt.Errorf("%q %w", value, errOver8K)
 	}
 
 	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: "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 {
+	if cfg.CoreRuleSetBodyInspectionSize != 8<<10 {
-		t.Errorf("64K read as %d", cfg.CoreRuleSetBodyInspectionSize)
+		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",
+			coreRuleSetBodyInspectionSize, "9K",
-			`"65537" is more than 64K, the most waf gets of a body`,
+			`"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) {

internal/coreruleset/coreruleset_test.go

@@ -1512,13 +1512,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 +1584,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 +1695,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 +1921,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 +2145,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 +2264,7 @@ func TestWordPressExclusionsAreNarrow(t *testing.T) {
 		length  = "Content-Length: 2"
 	)
 
-	crs := readingBodiesUpTo64K(t)
+	crs := readingBodiesUpTo8K(t)
 	saving := request{
 		http.MethodPost,
 		wpPost,

internal/coreruleset/inspectiontime_test.go

@@ -0,0 +1,345 @@
+package coreruleset_test
+
+import (
+	"maps"
+	"net/http"
+	"net/http/httptest"
+	"net/netip"
+	"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
+)
+
+// 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 mostBodyRead bytes
+// holding as many elements as fit, each the number 1.
+func jsonArrayOfMostElements() string {
+	return "[1" + strings.Repeat(
+		",1",
+		(mostBodyRead-len("[1]"))/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 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: jsonArrayOfMostElements(),
+		},
+	)
+}
+
+// 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 = (mostBodyRead - 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(
+				mostBodyRead-len(`{"a":""}`),
+			) + `"}`,
+		},
+		{
+			name: "a JSON body of the most elements", target: "/",
+			contentType: jsonType, body: jsonArrayOfMostElements(),
+		},
+		{
+			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.
+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()
+	_, 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))
+	}
+
+	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,8 +21,9 @@ 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. While it is open, at most one request each breakerAskInterval
+// answer in time, or an answer rproxy cannot pass on. While it is open, at
-// asks waf. The first answer closes it, whatever it decides.
+// most one request each breakerAskInterval asks waf. Any usable answer closes
+// 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
@@ -52,16 +53,14 @@ func (b *breaker) mayAsk(now time.Time) bool {
 	return true
 }
 
-// noteAnswer notes that an ask got an answer in time. It closes the breaker,
+// noteAnswer notes that an ask got an answer rproxy can use. It closes the
-// and forgets the failures before it.
+// breaker 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

internal/rproxy/breaker_test.go

@@ -63,17 +63,17 @@ func TestFiveFailedAsksOverMoreThanFiveSecondsLeaveTheBreakerClosed(
 	}
 }
 
-func TestAnAnswerBeforeTheBreakerOpensDoesNotForgetFailures(t *testing.T) {
+func TestAnAnswerBetweenFailedAsksKeepsTheBreakerClosed(t *testing.T) {
 	t.Parallel()
 
 	now := time.Now()
 	b := &rproxy.Breaker{}
-	fail(b, now, 4, time.Millisecond)
+	fail(b, now, 4, 100*time.Millisecond)
 	b.NoteAnswer()
-	b.NoteFailure(now.Add(time.Second))
+	fail(b, now.Add(500*time.Millisecond), 4, 100*time.Millisecond)
 
-	if b.MayAsk(now.Add(time.Second)) {
+	if !b.MayAsk(now.Add(900 * time.Millisecond)) {
-		t.Error("a request may ask, want it not to")
+		t.Error("a request may not ask, want it 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 in time.
+// NoteAnswer notes that an ask got an answer rproxy can use.
 func (b *Breaker) NoteAnswer() {
 	b.b.noteAnswer()
 }

internal/rproxy/rproxy_test.go

@@ -1452,6 +1452,83 @@ func TestFiveFailedAsksKeepTheNextRequestFromAskingWAF(t *testing.T) {
 	}
 }
 
+func TestVisitorsThatGoAwayWhileWAFIsAskedLeaveTheBreakerClosed(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	var answering atomic.Bool
+
+	standIn := startWAF(t, func(w http.ResponseWriter, r *http.Request) {
+		if answering.Load() {
+			allow(w, r)
+
+			return
+		}
+
+		<-r.Context().Done()
+	})
+	app := startApp(t, greet)
+	addr, out := 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 {
+		conn := dial(t, addr)
+		send(t, conn, "GET / HTTP/1.1\r\nHost: site.example\r\n\r\n")
+		id := standIn.question(t).header.Get(requestIDHeader)
+
+		_ = conn.Close()
+
+		// rproxy writes the request's log line once it is done with the ask.
+		out.waitFor(t, id)
+	}
+
+	answering.Store(true)
+	wantGreeting(t, get(t, addr, "/"))
+	standIn.question(t)
+}
+
+func TestAnswersThatCannotBePassedOnOpenTheBreaker(t *testing.T) {
+	t.Parallel()
+
+	var answering atomic.Bool
+
+	tooLarge := overOneMiB(dispositionDeny)
+	standIn := startWAF(t, func(w http.ResponseWriter, r *http.Request) {
+		if answering.Load() {
+			allow(w, r)
+
+			return
+		}
+
+		tooLarge(w, r)
+	})
+	app := startApp(t, greet)
+	addr, out := startRProxy(t, standIn.URL, app.URL, map[string]string{
+		wafBackendTimeout: "60000",
+	})
+
+	for range 5 {
+		if got := get(t, addr, "/"); got.status != http.StatusBadGateway {
+			t.Errorf("answered %d, want 502", got.status)
+		}
+
+		standIn.question(t)
+	}
+
+	answering.Store(true)
+	wantGreeting(t, get(t, addr, "/"))
+	out.waitFor(t, "the circuit breaker is open")
+
+	select {
+	case <-standIn.questions:
+		t.Error("waf was asked, want it not to be")
+	case <-time.After(10 * pollInterval):
+	}
+}
+
 func TestAPathUnderOwnPrefixNeverReachesTheApp(t *testing.T) {
 	t.Parallel()
 

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 still came in time. A
+// answered in time. An answer rproxy cannot pass on counts as a failure. 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, errors.Is(err, errCannotPassOn):
+	case err == nil:
 		b.noteAnswer()
 	case ctx.Err() == nil:
 		b.noteFailure(time.Now())

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
+// or nil while SWW_WAF_CORE_RULE_SET_MODE is off. An exclusions file that
-// reads at most QuestionBodyMaxBytes less one byte: a question's body of
+// coreruleset.New refuses is an error in every mode. While the mode is off, a
-// QuestionBodyMaxBytes may be cut short, and the Core Rule Set does not read a
+// set file is loaded with the Core Rule Set only to check it.
-// 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 == "" {
@@ -243,10 +240,7 @@ func NewCoreRuleSet(cfg *config.WAF) (*coreruleset.CoreRuleSet, error) {
 	coreRuleSet, err := coreruleset.New(coreruleset.Params{
 		ParanoiaLevel:      cfg.CoreRuleSetParanoiaLevel,
 		DisabledRules:      cfg.CoreRuleSetDisabledRules,
-		BodyInspectionSize: min(
+		BodyInspectionSize: cfg.CoreRuleSetBodyInspectionSize,
-			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: "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)
+	cut := `{"text":"` + strings.Repeat("x", 8<<10)
-	got = ask("application/json", cut[:waf.QuestionBodyMaxBytes])
+	got = ask("application/json", cut)
 	wantDisposition(t, got, http.StatusOK, waf.DispositionAllow)
 }