Core Rule Set reads request bodies up to SWWAF_WAF_BODY_LIMIT (closes #116)

SWWAF_WAF_BODY_LIMIT (default off) has the Core Rule Set read form data
and multipart up to the limit, the rest streaming on, and JSON and XML
no larger than it; other bodies pass uninspected. The part read is held
and sent to the app first. A size or time limit met while it is read
ends the request before it reaches the app. Content-Encoding is refused
again on those four kinds. Rule 900300 moves to phase 2, to count form
and JSON fields past Coraza's 1000 too.

Judgement call: Content-Encoding is refused on a JSON or XML body too
large to be read, which SPEC.md allows.

Model: opus-5-5

Dockerfile

@@ -36,12 +36,11 @@ RUN go mod tidy -diff || \
     { echo "go.mod or go.sum is not tidy: run make tidy" >&2; exit 1; }
 
 # Go's build cache is kept on a tmpfs, out of the image: nothing uses it
-# after this step, and writing it into the image takes seconds. The tests
-# are built with the no_fs_access tag, as the binary is in the build stage.
+# after this step, and writing it into the image takes seconds.
 RUN --mount=type=tmpfs,target=/root/.cache/go-build \
-    go test -tags no_fs_access -timeout 90s -race -cover ./... || \
+    go test -timeout 90s -race -cover ./... || \
     { echo "--- Rerunning with -v for details ---"; \
-      go test -tags no_fs_access -timeout 90s -race -v ./...; exit 1; }
+      go test -timeout 90s -race -v ./...; exit 1; }
 
 # Tidy stage: `go mod tidy` in the test phase's Go, so that the files it
 # writes pass the test phase's check. Nothing else depends on it, so only
@@ -85,10 +84,7 @@ COPY . .
 # The VERSION build arg when one is given, otherwise
 # `git describe --tags --always` on the .git in the build context. With
 # .git present, a version that is still empty, dev or unknown fails the
-# build: git is missing or could not read the checkout. The no_fs_access
-# tag keeps Coraza from writing the files of a multipart body to the
-# system's temporary directory, since smallwebwaf writes only to its state
-# directory.
+# build: git is missing or could not read the checkout.
 ARG VERSION
 RUN VERSION="${VERSION:-$(git describe --tags --always)}"; \
     if [ -e .git ]; then \
@@ -97,7 +93,7 @@ RUN VERSION="${VERSION:-$(git describe --tags --always)}"; \
             exit 1 ;; \
         esac; \
     fi; \
-    CGO_ENABLED=0 go build -tags no_fs_access -trimpath \
+    CGO_ENABLED=0 go build -trimpath \
         -ldflags="-s -w -X main.Version=${VERSION}" \
         -o /usr/local/bin/smallwebwaf ./cmd/smallwebwaf
 

README.md

@@ -197,12 +197,11 @@ in `bin/state` unless `SWWAF_STATE_DIR` is set, and the default rule file of
   rate limits and before the rule files, which it does not need. A request for
   one is refused with `SWWAF_BAN_RESPONSE` and bans the client's netblock for a
   clear sign of attack, as a `ban` rule's match does. A trap path is matched
-  against the path a `path` rule sees: without the query, as the client sent it
-  and once percent-decoded, the whole of it, character for character.
-  `/wp-login.php` matches `/wp-login.php?redirect_to=x` and `/%77p-login.php`,
-  but not `/wp-login.php/`, `/WP-LOGIN.PHP`, `/blog/wp-login.php` or
-  `/%2577p-login.php`. The ban's notes give the path as the client sent it. A
-  client in `SWWAF_ALLOW_NETS` is not checked.
+  against the path a `path` rule sees: as the client sent it, before any
+  decoding and without the query, the whole of it, character for character.
+  `/wp-login.php` matches `/wp-login.php?redirect_to=x`, but not
+  `/wp-login.php/`, `/WP-LOGIN.PHP`, `/blog/wp-login.php` or `/%77p-login.php`.
+  A client in `SWWAF_ALLOW_NETS` is not checked.
 - Inspects each request with the OWASP Core Rule Set 4.25.0, run by Coraza,
   after the rule files, unless `SWWAF_WAF_MODE` is `off`: its method, its URL
   with the query, and its headers, and, once `SWWAF_WAF_BODY_LIMIT` is set, its
@@ -712,38 +711,29 @@ effective settings are logged at start, unless `SWWAF_LOG_LEVEL` is `warn` or
   `SWWAF_RATE_LIMIT_EXEMPT_PATHS` matches its own: a request whose path holds
   `..`, a backslash or an encoded slash is inspected whatever its prefix.
 - `SWWAF_WAF_BODY_LIMIT` (default `off`): `off` has the Core Rule Set read no
-  request body. A size, such as `128K`, at most `1G`, has it read a body of form
-  data or multipart up to that size, the rest of a longer one passing on to the
-  app as it arrives, without being held, and a JSON or XML body no larger than
-  that size, since those cannot be read in part. A body is JSON when its type is
-  `application/json` or `text/json`, or an `application/` or `text/` type ending
-  in `+json`, and XML when its type is `application/xml` or `text/xml`, or an
-  `application/` or `text/` type ending in `+xml`. Any other body reaches the
-  app uninspected, and so does a larger JSON or XML body: the Core Rule Set
-  would read any other body as form data, where binary content such as a git
-  push trips rules written for text. A body it reads that Coraza cannot parse
-  (rule 900440), and a multipart body that fails Coraza's strict checks (rule
-  900450), add 5 to the score, as a rule rated critical does, since no rule
-  reads what comes after the fault. So does a multipart body the limit cuts
-  before the colon of a part's header line, or between the carriage return and
-  the line feed that end a part's header line or the empty line after its
-  headers, since Coraza takes the line the limit cuts for a malformed header.
-  The client has until `SWWAF_CLIENT_REQUEST_TIMEOUT` runs out to send the part
-  that is read, and a request whose body passes `SWWAF_REQUEST_MAX_BYTES` within
-  it is refused before anything reaches the app. Of a file in a multipart body,
-  Coraza counts the bytes and writes nothing. Body inspection suits apps whose
-  forms carry no code. In front of gitea it refuses issue and comment text, wiki
-  pages and files saved in the web editor that hold shell commands or code
-  (932125, 932235, 932250 and others), package descriptions that show code, PyPI
-  uploads (922130), and attachments named like `debug.log` or `config.yml`
-  (932180), until the rule ids the request log names are added to
-  `SWWAF_WAF_DISABLED_RULES`.
+  request body. A size, such as `128K`, has it read a body of form data or
+  multipart up to that size, the rest of a longer one passing on to the app as
+  it arrives, without being held, and a JSON or XML body no larger than that
+  size, since those cannot be read in part. Any other body reaches the app
+  uninspected, and so does a larger JSON or XML body: the Core Rule Set would
+  read any other body as form data, where binary content such as a git push
+  trips rules written for text. The client has until
+  `SWWAF_CLIENT_REQUEST_TIMEOUT` runs out to send the part that is read, and a
+  request whose body passes `SWWAF_REQUEST_MAX_BYTES` within it is refused
+  before anything reaches the app. Coraza writes what it reads of each file in a
+  multipart body to the system's temporary directory, and removes it once the
+  request is inspected. Body inspection suits apps whose forms carry no code. In
+  front of gitea it refuses issue and comment text, wiki pages and files saved
+  in the web editor that hold shell commands or code (932125, 932235, 932250 and
+  others), package descriptions that show code, PyPI uploads (922130), and
+  attachments named like `debug.log` or `config.yml` (932180), until the rule
+  ids the request log names are added to `SWWAF_WAF_DISABLED_RULES`.
 - `SWWAF_TRAP_PATHS` (default empty): paths the app never serves and only
   scanners ask for, such as `/wp-login.php,/xmlrpc.php` in front of gitea; a
   request for one bans its client for a clear sign of attack, as a `ban` rule
   does, with or without rule files (see "What it does so far" above). A path
-  that does not start with `/`, or holds a `?`, as one written with a query
-  does, stops the start.
+  that does not start with `/`, or holds a `?`, which no path a `path` rule sees
+  holds, stops the start.
 - `SWWAF_ERROR_BURST_THRESHOLD` (default `30`): the most requests of a client in
   a minute that `smallwebwaf` may refuse after a rule file match, a trap path or
   a Core Rule Set match, or for a missing or wrong token; one more breaks the
@@ -1523,15 +1513,14 @@ scanner-agent  user_agent  ban     (?i)\b(sqlmap|nikto|nuclei|wpscan)\b
 
 - The id is letters, digits, `-` and `_`, and no two rules share one. The
   request log, the metrics and a ban's notes name the rule by it.
-- The target is what the regex is matched against: `path`, the path without the
-  query, and `uri`, the path and the query together, each both as sent and once
-  percent-decoded, so that an encoded probe does not slip past; `query`, as the
-  client sent it, before any decoding; `method`; `host`; `user_agent`;
-  `referer`; or `header:<Name>`, any one request header but `Host` and
-  `Transfer-Encoding`, which Go's HTTP server takes out of every request; the
-  request's host is the target `host`. A header sent more than once is matched
-  with its values joined by `, `, and one not sent as empty text. No body is
-  read.
+- The target is what the regex is matched against: `path` or `query`, as the
+  client sent it, before any decoding; `uri`, the path and the query together,
+  both as sent and once percent-decoded, so that an encoded probe does not slip
+  past; `method`; `host`; `user_agent`; `referer`; or `header:<Name>`, any one
+  request header but `Host` and `Transfer-Encoding`, which Go's HTTP server
+  takes out of every request; the request's host is the target `host`. A header
+  sent more than once is matched with its values joined by `, `, and one not
+  sent as empty text. No body is read.
 - The action is `log`, `block` or `ban` (see "What it does so far" above). Keep
   `ban` for requests no real visitor sends, and anchor a path at the site root
   with `^/`: a file of the same name deeper in a site can be ordinary content,

SPEC.md

@@ -825,8 +825,7 @@ a file edited, added or removed there takes effect while it runs.
 - `id`: a short name of letters, digits, `-` and `_`, unique across all files.
   It appears in the request log, metrics, alerts and ban notes.
 - `target`: what the regex is matched against.
-    - `path`: the URL path without the query, both as received and once
-      percent-decoded, so an encoded probe cannot slip past.
+    - `path`: the URL path as received, before any decoding.
     - `query`: the raw query string.
     - `uri`: path and query together, both as received and once percent-decoded,
       so an encoded probe cannot slip past.

internal/bans/bans.go

@@ -123,8 +123,7 @@ type Notes struct {
 	LimitPercentSetting string `json:"limit_percent_setting,omitempty"`
 	// RuleID and Target are, for a ban for a clear sign of attack, the id
 	// of the rule file rule that matched, and its target; TrapPath is, for
-	// one for a request for a path in SWWAF_TRAP_PATHS, that path as the
-	// client sent it.
+	// one for a request for a path in SWWAF_TRAP_PATHS, that path.
 	RuleID   string `json:"rule_id,omitempty"`
 	Target   string `json:"target,omitempty"`
 	TrapPath string `json:"trap_path,omitempty"`

internal/config/config.go

@@ -398,7 +398,6 @@ var (
 	errNeedsDBPath          = errors.New("it names the file to look clients up in")
 	errDBPathUnused         = errors.New("only file reads it")
 	errNotOver4K            = errors.New("is not a size of more than 4K, such as 32K")
-	errOver1G               = errors.New("is more than 1G, the most Coraza reads")
 	errNotDurationAboveZero = errors.New(
 		"is not a duration above zero, such as 1h or 7d")
 	errNotNumberAboveZero = errors.New(
@@ -562,7 +561,7 @@ func FromEnvironment(lookupEnv func(string) (string, bool)) (*Config, error) {
 		WAFDisabledRules: env.ruleIDs("SWWAF_WAF_DISABLED_RULES",
 			"920340,920420,920440,920640,930130,930140"),
 		WAFExemptPaths:       env.pathPrefixes("SWWAF_WAF_EXEMPT_PATHS", ""),
-		WAFBodyLimit:         env.wafBodyLimit("SWWAF_WAF_BODY_LIMIT", off),
+		WAFBodyLimit:         env.size("SWWAF_WAF_BODY_LIMIT", off),
 		TrapPaths:            env.trapPaths("SWWAF_TRAP_PATHS"),
 		ErrorBurstThreshold:  env.count("SWWAF_ERROR_BURST_THRESHOLD", "30"),
 		LogRemoteURL:         env.logRemoteURL("SWWAF_LOG_REMOTE_URL"),
@@ -768,15 +767,6 @@ func (e *environment) size(name, defaultValue string) int64 {
 	return size
 }
 
-// wafBodyLimit reads the setting that is the most of a request body the
-// Core Rule Set reads.
-func (e *environment) wafBodyLimit(name, defaultValue string) int64 {
-	limit, err := parseWAFBodyLimit(e.value(name, defaultValue))
-	e.check(name, err)
-
-	return limit
-}
-
 // headerSize reads the setting that is the largest request line and
 // headers.
 func (e *environment) headerSize(name, defaultValue string) int64 {
@@ -1445,22 +1435,6 @@ func parseHeaderSize(value string) (int64, error) {
 	return size, nil
 }
 
-// parseWAFBodyLimit reads the most of a request body the Core Rule Set
-// reads: a size as parseSize reads it, or off, but at most 1G, since
-// Coraza, which runs the Core Rule Set, refuses to load with more.
-func parseWAFBodyLimit(value string) (int64, error) {
-	limit, err := parseSize(value)
-	if err != nil {
-		return 0, err
-	}
-
-	if limit > gibibyte {
-		return 0, fmt.Errorf("%q %w", value, errOver1G)
-	}
-
-	return limit, nil
-}
-
 // splitUnit splits a size into its number and the bytes its suffix
 // stands for.
 func splitUnit(value string) (string, int64) {
@@ -1665,8 +1639,7 @@ func parsePathPrefixes(value string) ([]string, error) {
 
 // parseTrapPaths reads a comma-separated list of trap paths. Each is
 // matched against a request's path as a path rule is, without the query,
-// so a path that does not start with / would never match, and one that
-// holds a ? is written with a query.
+// so a path that does not start with / or holds a ? would never match.
 func parseTrapPaths(value string) ([]string, error) {
 	paths, err := parseList(value)
 	if err != nil {

internal/config/config_test.go

@@ -615,15 +615,6 @@ func TestCoreRuleSetSettings(t *testing.T) {
 	}
 }
 
-func TestWAFBodyLimitOf1G(t *testing.T) {
-	t.Parallel()
-
-	cfg := fromEnvironment(t, environment{wafBodyLimit: "1G"})
-	if cfg.WAFBodyLimit != 1<<30 {
-		t.Errorf("1G read as %d", cfg.WAFBodyLimit)
-	}
-}
-
 func TestInvalidCoreRuleSetSettingStopsTheStart(t *testing.T) {
 	t.Parallel()
 
@@ -652,15 +643,12 @@ func TestInvalidCoreRuleSetSettingStopsTheStart(t *testing.T) {
 			`"-942100" is not the id of a Core Rule Set rule, ` +
 				`a whole number such as 942100`,
 		},
-		// The paranoia level, the allowed methods, the headers refused, a
-		// request with more query parameters than Coraza keeps, and a body
-		// Coraza cannot parse or that fails its strict checks.
+		// The paranoia level, the allowed methods, the headers refused, and
+		// a request with more query parameters than Coraza keeps.
 		{wafDisabledRules, "942100,900000", `"900000"` + setupRule},
 		{wafDisabledRules, "942100,900200", `"900200"` + setupRule},
 		{wafDisabledRules, "942100,900250", `"900250"` + setupRule},
 		{wafDisabledRules, "942100,900300", `"900300"` + setupRule},
-		{wafDisabledRules, "942100,900440", `"900440"` + setupRule},
-		{wafDisabledRules, "942100,900450", `"900450"` + setupRule},
 		{
 			wafExemptPaths, "api/",
 			`"api/" is not a path prefix starting with /, such as /assets/`,
@@ -669,11 +657,6 @@ func TestInvalidCoreRuleSetSettingStopsTheStart(t *testing.T) {
 			wafBodyLimit, "128KB",
 			`"128KB" is not a size such as 512K, 100M or 5G, or off`,
 		},
-		{wafBodyLimit, "2G", `"2G" is more than 1G, the most Coraza reads`},
-		{
-			wafBodyLimit, "1073741825",
-			`"1073741825" is more than 1G, the most Coraza reads`,
-		},
 	} {
 		t.Run(tc.name+"="+tc.value, func(t *testing.T) {
 			t.Parallel()

internal/proxy/proxy.go

@@ -245,8 +245,8 @@ func newCoreRuleSet(cfg *config.Config) *waf.CoreRuleSet {
 	})
 	if err != nil {
 		// The Core Rule Set is built in, and the settings cannot break it:
-		// the paranoia level is from 1 to 4, the body limit at most 1G, and
-		// the id of no rule switches nothing off.
+		// the paranoia level is from 1 to 4, and the id of no rule switches
+		// nothing off.
 		panic(err)
 	}
 

internal/proxy/request.go

@@ -789,19 +789,9 @@ func (rq *request) responseTimedOut() {
 	}
 }
 
-// responseReceived is called once the app has sent its whole answer. An
-// app may answer before the client has sent its whole body, as with a 404
-// it sends without reading it. The timeouts then run on: until the
-// transport's read of the body returns, Go's server cannot send the
-// answer, and SWWAF_CLIENT_REQUEST_TIMEOUT is what ends that read for a
-// client that stops sending.
+// responseReceived is called once the app has sent its whole answer.
 func (rq *request) responseReceived() {
 	rq.complete = true
-
-	if rq.body != nil && !rq.body.received.Load() {
-		return
-	}
-
 	rq.stopTimers()
 }
 

internal/proxy/rules.go

@@ -10,15 +10,13 @@ import (
 )
 
 // trapPath reports whether the request asks for a path in
-// SWWAF_TRAP_PATHS: its path as a path rule sees it, without the query,
-// as sent or once percent-decoded, is one of them. Such a request is a
-// clear sign of attack, as a ban rule's match is: it bans the client's
-// netblock, or in observe mode raises the alert for the ban it would have
-// made. The ban's notes give the path as sent.
+// SWWAF_TRAP_PATHS: its path as a path rule sees it, before any decoding
+// and without the query, is one of them. Such a request is a clear sign of
+// attack, as a ban rule's match is: it bans the client's netblock, or in
+// observe mode raises the alert for the ban it would have made.
 func (rq *request) trapPath(now time.Time) bool {
 	path := rules.Path(rq.in)
-	if !slices.Contains(rq.h.config.TrapPaths, path) &&
-		!slices.Contains(rq.h.config.TrapPaths, rules.DecodeOnce(path)) {
+	if !slices.Contains(rq.h.config.TrapPaths, path) {
 		return false
 	}
 

internal/proxy/timeouts_test.go

@@ -194,28 +194,6 @@ func sendLargeBody(t *testing.T, addr string) net.Conn {
 	return conn
 }
 
-func TestClientStopsSendingAfterTheAppHasAnswered(t *testing.T) {
-	t.Parallel()
-
-	// The app answers at once without reading the body, as an app does
-	// for a path it does not serve.
-	app := startApp(t, http.NotFound)
-	addr, out := startProxy(t, app.URL, map[string]string{
-		clientRequestTimeout: shortTimeoutSetting,
-		metricsToken:         token,
-	})
-
-	start := time.Now()
-	got := readResponse(t, sendPartOfBody(t, addr))
-	wantTimedOut(t, start)
-
-	// The app's answer came before the time ran out, so the client is
-	// sent that answer, not smallwebwaf's 408.
-	wantStatus(t, got, http.StatusNotFound)
-	wantLine(t, out.requestLine(t), http.StatusNotFound, requestlog.ActionTimedOut)
-	wantLimitHits(t, addr, clientRequestTimeout, 1)
-}
-
 func TestAppTooSlowToAnswer(t *testing.T) {
 	t.Parallel()
 

internal/proxy/trappaths_test.go

@@ -32,9 +32,10 @@ func TestTrapPathBansAsABanRuleDoes(t *testing.T) {
 	})
 	start := clk.Now()
 
-	// Only the path itself, the whole of it in the same case, is a trap
-	// path.
-	for _, path := range []string{"/wp-login.php/", "/WP-LOGIN.PHP", "/blog/xmlrpc.php"} {
+	// Only the path itself, as the client sent it, is a trap path.
+	for _, path := range []string{
+		"/wp-login.php/", "/WP-LOGIN.PHP", "/blog/xmlrpc.php", "/%77p-login.php",
+	} {
 		s.request(otherClient, path, http.StatusOK, requestlog.ActionForward)
 	}
 
@@ -83,22 +84,6 @@ func TestTrapPathBansAsABanRuleDoes(t *testing.T) {
 	}
 }
 
-func TestTrapPathMatchesThePathDecodedOnce(t *testing.T) {
-	t.Parallel()
-
-	s, _, server := startWithClock(t, "", map[string]string{trapPaths: trapPathList})
-
-	// %2577 decodes once to %77, not to w.
-	s.request(otherClient, "/%2577p-login.php", http.StatusOK, requestlog.ActionForward)
-	s.request(client, "/%77p-login.php", http.StatusForbidden, requestlog.ActionBanned)
-
-	// The ban's notes give the path as the client sent it.
-	got := server.Ledger.Bans(netip.MustParsePrefix(client + "/32"))
-	if len(got) != 1 || got[0].Notes.TrapPath != "/%77p-login.php" {
-		t.Fatalf("bans %+v, want one whose notes give the trap path /%%77p-login.php", got)
-	}
-}
-
 func TestTrapPathsNeedNoRuleFiles(t *testing.T) {
 	t.Parallel()
 

internal/rules/rules.go

@@ -50,13 +50,6 @@ const quietTime = 2 * time.Second
 // header:<Name>.
 const headerTarget = "header:"
 
-// pathTarget and uriTarget are the targets matched both as received and
-// once percent-decoded: the path, and the path and query together.
-const (
-	pathTarget = "path"
-	uriTarget  = "uri"
-)
-
 // escapeLength is the length of a percent escape, such as %2e.
 const escapeLength = 3
 
@@ -381,7 +374,7 @@ func parse(line string) (Rule, bool, error) {
 // isTarget reports whether target is one a rule may have.
 func isTarget(target string) bool {
 	switch target {
-	case pathTarget, "query", uriTarget, "method", "host", "user_agent", "referer":
+	case "path", "query", "uri", "method", "host", "user_agent", "referer":
 		return true
 	}
 
@@ -391,38 +384,35 @@ func isTarget(target string) bool {
 }
 
 // matches reports whether the rule's regex matches its target in r. For
-// path and uri it is matched against the target as received, and against
-// it once percent-decoded, so that an encoded probe cannot slip past.
+// uri it is matched against the path and query as received, and against
+// them once percent-decoded, so that an encoded probe cannot slip past.
 func (rule Rule) matches(r *http.Request) bool {
-	target := value(rule.Target, r)
+	if rule.Target == "uri" {
+		uri := pathAndQuery(r)
 
-	if rule.Target == pathTarget || rule.Target == uriTarget {
-		return rule.regex.MatchString(target) || rule.regex.MatchString(DecodeOnce(target))
+		return rule.regex.MatchString(uri) || rule.regex.MatchString(decodeOnce(uri))
 	}
 
-	return rule.regex.MatchString(target)
+	return rule.regex.MatchString(value(rule.Target, r))
 }
 
 // Path returns r's path as the client sent it, before any decoding or
-// re-encoding, up to the first ?: what a path rule is matched against, as
-// it is and once percent-decoded.
+// re-encoding, up to the first ?: what a path rule is matched against.
 func Path(r *http.Request) string {
 	path, _, _ := strings.Cut(pathAndQuery(r), "?")
 
 	return path
 }
 
-// value returns what a rule with target is matched against in r: the
-// path, as Path gives it, the path and query, as pathAndQuery gives
-// them, and the query as the client sent it, before any decoding or
-// re-encoding, after the first ?, and a header's values joined by ", ",
-// as HTTP joins those of a header sent more than once.
+// value returns what a rule with target, other than uri, is matched
+// against in r: the path, as Path gives it, and the query as the client
+// sent it, before any decoding or re-encoding, after the first ?, and a
+// header's values joined by ", ", as HTTP joins those of a header sent
+// more than once.
 func value(target string, r *http.Request) string {
 	switch target {
-	case pathTarget:
+	case "path":
 		return Path(r)
-	case uriTarget:
-		return pathAndQuery(r)
 	case "query":
 		_, query, _ := strings.Cut(pathAndQuery(r), "?")
 
@@ -474,11 +464,11 @@ func header(r *http.Request, name string) string {
 	return strings.Join(r.Header.Values(name), ", ")
 }
 
-// DecodeOnce returns s with each percent escape, such as %2e, replaced by
+// decodeOnce returns s with each percent escape, such as %2e, replaced by
 // the byte it stands for. A % that is not followed by two hex digits is
 // left as it is, so that a malformed escape cannot keep the rest of s
 // from being decoded.
-func DecodeOnce(s string) string {
+func decodeOnce(s string) string {
 	var decoded strings.Builder
 
 	for i := 0; i < len(s); i++ {

internal/rules/rules_test.go

@@ -49,9 +49,7 @@ func TestEachTargetMatchesWhatItNames(t *testing.T) {
 		want   bool
 	}{
 		{"path as received", `path log ^/%2eenv$`, "/%2eenv", nil, true},
-		{"path decoded", `path log ^/\.env$`, "/%2Eenv", nil, true},
-		{"path decoded only once", `path log ^/\.env$`, "/%252eenv", nil, false},
-		{"path decoded without the query", `path log ^/a\?b$`, "/a%3fb?c", nil, true},
+		{"path not decoded", `path log ^/\.env$`, "/%2eenv", nil, false},
 		{"path without the query", `path log ^/a$`, "/a?b=c", nil, true},
 		{"query as received", `query log ^b=%2e$`, "/a?b=%2e", nil, true},
 		{"uri as received", `uri log ^/a\?b=%2e$`, "/a?b=%2e", nil, true},
@@ -420,7 +418,7 @@ func TestDefaultFileBansProbesAtTheSiteRootAlone(t *testing.T) {
 
 	// Probes sent by a browser, by the rule that refuses them.
 	for rule, targets := range map[string][]string{
-		"env-file":    {"/.env", "/.env.production", "/.ENV", "/%2eenv"},
+		"env-file":    {"/.env", "/.env.production", "/.ENV"},
 		"vcs-dir":     {"/.git/config", "/.git", "/.svn/entries"},
 		"secrets-dir": {"/.aws/credentials", "/.ssh/id_rsa"},
 		"secret-file": {"/.htpasswd", "/.DS_Store", "/.git-credentials"},

internal/waf/waf.go

@@ -3,11 +3,6 @@
 // body while SWWAF_WAF_BODY_LIMIT is set, with the six changes smallwebwaf
 // makes to it, as "Attack detection" under "Configuration surface" in
 // SPEC.md describes them. It reads no response.
-//
-// smallwebwaf writes only to its state directory, so Coraza is built with
-// its no_fs_access tag, as the Dockerfile and script/build build it: of a
-// file in a multipart body, Coraza then counts the bytes instead of
-// writing them to the system's temporary directory.
 package waf
 
 import (
@@ -105,20 +100,16 @@ SecRuleUpdateTargetById 944110 "!REQUEST_HEADERS:Referer"
 // bodyDirectives have the Core Rule Set read the part of a request body
 // Inspect gives it, which is at most one byte longer than the limit, up to
 // the limit, %d bytes, and read JSON and XML as Coraza's recommended
-// configuration has it in its rules 200000, 200001 and 200006, with
-// text/json, and any application or text type ending in +xml or +json,
-// besides; form data and multipart Coraza knows by itself. %% stands for
-// a % Coraza reads.
+// configuration has it in its rules 200000, 200001 and 200006; form data
+// and multipart Coraza knows by itself. %% stands for a % Coraza reads.
 const bodyDirectives = `
 SecRequestBodyAccess On
 SecRequestBodyLimit %d
 SecRequestBodyLimitAction ProcessPartial
 
-SecRule REQUEST_HEADERS:Content-Type \
-    "@rx ^(?:application|text)/(?:[a-z0-9.-]+[+])?xml" \
+SecRule REQUEST_HEADERS:Content-Type "@rx ^(?:application(?:/soap[+]|/)|text/)xml" \
     "id:900410,phase:1,pass,nolog,t:none,t:lowercase,ctl:requestBodyProcessor=XML"
-SecRule REQUEST_HEADERS:Content-Type \
-    "@rx ^(?:application|text)/(?:[a-z0-9.-]+[+])?json" \
+SecRule REQUEST_HEADERS:Content-Type "@rx ^application/(?:[a-z0-9.-]+[+])?json" \
     "id:900420,phase:1,pass,nolog,t:none,t:lowercase,ctl:requestBodyProcessor=JSON"
 
 # The rest of the second change: Content-Encoding is refused again on a
@@ -128,20 +119,6 @@ SecRule REQBODY_PROCESSOR "@rx ^(?:URLENCODED|MULTIPART|JSON|XML)$" \
     "id:900260,phase:1,pass,nolog,\
     setvar:'tx.restricted_headers_basic=%%{tx.restricted_headers_basic} \
     /content-encoding/'"
-
-# A body Coraza fails to parse (900440), and a multipart body that fails
-# its strict checks (900450), each add 5 to the score, as a rule the Core
-# Rule Set rates critical does: no rule reads what comes after the fault,
-# which the app may still read. Coraza's recommended configuration refuses
-# them in its rules 200002 and 200003. A multipart body the limit cuts
-# before the colon of a part's header line, or between the carriage return
-# and the line feed that end a part's header line or the empty line after
-# its headers, adds 5 too, since Coraza takes the line the limit cuts for a
-# malformed header. Coraza parses any form data body.
-SecRule REQBODY_ERROR "!@eq 0" "id:900440,phase:2,pass,severity:'CRITICAL',\
-    setvar:'tx.inbound_anomaly_score_pl1=+5'"
-SecRule MULTIPART_STRICT_ERROR "!@eq 0" "id:900450,phase:2,pass,\
-    severity:'CRITICAL',setvar:'tx.inbound_anomaly_score_pl1=+5'"
 `
 
 // cookiesNotRead are the cookies the Core Rule Set reads a request
@@ -218,7 +195,9 @@ func (c *CoreRuleSet) Inspect(
 	r *http.Request, client netip.Addr, body io.Reader,
 ) (Result, []byte, error) {
 	tx := c.waf.NewTransaction()
-	// Closing would remove the files Coraza wrote, and it writes none.
+	// Closing removes the files Coraza writes, in the system's temporary
+	// directory, for the file parts of a multipart body it reads. One it
+	// cannot remove is left there, and changes nothing in what was found.
 	defer func() { _ = tx.Close() }()
 
 	tx.ProcessConnection(client.String(), 0, "", 0)

internal/waf/waf_test.go

@@ -5,7 +5,6 @@ import (
 	"net/http/httptest"
 	"net/netip"
 	"net/url"
-	"path/filepath"
 	"reflect"
 	"strconv"
 	"strings"
@@ -517,13 +516,8 @@ func TestContentEncodingIsRefusedOnTheKindsOfBodyTheCoreRuleSetReads(t *testing.
 
 	crs := readingBodies(t)
 
-	for _, tc := range []struct{ header, body string }{
-		{formData, "a=1"},
-		{multipart, field("a", "1") + end},
-		{jsonBody, `{"a":1}`},
-		{xmlBody, "<a>1</a>"},
-	} {
-		wantBody(t, crs, post(tc.header, gzip), tc.body, matched(920450), tc.body)
+	for _, header := range []string{formData, multipart, jsonBody, xmlBody} {
+		wantBody(t, crs, post(header, gzip), "a", matched(920450), "a")
 	}
 
 	// Whatever its size: a JSON body larger than the limit is not read, but
@@ -584,105 +578,3 @@ func TestGiteaBodiesTheCoreRuleSetRefuses(t *testing.T) {
 		"filename=\"debug.log\"\r\nContent-Type: text/plain\r\n\r\nstarted\r\n" + end
 	wantBody(t, crs, post(multipart), attachment, matched(932180), attachment)
 }
-
-func TestTypesEndingInXMLOrJSONAndTextJSONAreRead(t *testing.T) {
-	t.Parallel()
-
-	crs := readingBodies(t)
-
-	for _, tc := range []struct{ contentType, body string }{
-		{"application/atom+xml", "<q>" + injection + "</q>"},
-		{"application/vnd.example+xml", "<q>" + injection + "</q>"},
-		{"application/vnd.example+json", `{"q":"` + injection + `"}`},
-		{"text/json", `{"q":"` + injection + `"}`},
-	} {
-		wantBody(t, crs, post("Content-Type: "+tc.contentType), tc.body,
-			matched(942100), tc.body)
-	}
-}
-
-func TestBodyCorazaCannotParseIsAMatch(t *testing.T) {
-	t.Parallel()
-
-	crs := readingBodies(t)
-
-	// An end tag after the root element, past which Coraza reads none of
-	// the body, while an app may still read the attack before it.
-	body := "<q>" + injection + "</q></r>"
-	wantBody(t, crs, post(xmlBody), body, matched(900440), body)
-
-	// The multipart bodies Coraza cannot parse fail its strict checks too:
-	// one whose type names its boundary twice, and one with a part header
-	// that has no colon, before the attack. They do so padded past the
-	// limit too, which cuts them in the padding.
-	noColon := "--b\r\nContent-Disposition form-data; name=\"a\"\r\n\r\n1\r\n"
-	pad := field("pad", strings.Repeat("a", bodyLimit))
-
-	for _, tc := range []struct{ header, head string }{
-		{multipart + "; boundary=c", ""},
-		{multipart, noColon},
-	} {
-		body = tc.head + field("q", injection) + end
-		wantBody(t, crs, post(tc.header), body, matched(900440, 900450), body)
-
-		body = tc.head + field("q", injection) + pad + end
-		wantBody(t, crs, post(tc.header), body, matched(900440, 900450),
-			body[:bodyLimit+1])
-	}
-}
-
-func TestMultipartBodyCutBeforeAPartHeadersColonIsAMatch(t *testing.T) {
-	t.Parallel()
-
-	// The limit falls in the middle of the name of the second part's
-	// header, which Coraza, reading up to the limit, cannot tell from a
-	// header without a colon.
-	cut := "--b\r\nContent-Di"
-	first := field("pad", strings.Repeat("a", bodyLimit-len(field("pad", ""))-len(cut)))
-	body := first + cut + "sposition: form-data; name=\"q\"\r\n\r\n1\r\n" + end
-
-	wantBody(t, readingBodies(t), post(multipart), body, matched(900440, 900450),
-		body[:bodyLimit+1])
-}
-
-func TestMultipartBodyCutBeforeALineFeedInAPartsHeadersIsAMatch(t *testing.T) {
-	t.Parallel()
-
-	crs := readingBodies(t)
-	headerLine := "--b\r\nContent-Disposition: form-data; name=\"q\"\r"
-
-	// The limit falls between the carriage return and the line feed that
-	// end the second part's header line, and then between those that end
-	// the empty line after it. Coraza, reading up to the limit, takes the
-	// line ending in a lone carriage return for a malformed header.
-	for _, cut := range []int{len(headerLine), len(headerLine + "\n\r")} {
-		first := field("pad", strings.Repeat("a", bodyLimit-len(field("pad", ""))-cut))
-		body := first + field("q", "1") + end
-
-		wantBody(t, crs, post(multipart), body, matched(900440, 900450),
-			body[:bodyLimit+1])
-	}
-}
-
-// TestCorazaWritesNoFile is not parallel, since it sets TMPDIR, the
-// system's temporary directory, for the whole test process.
-func TestCorazaWritesNoFile(t *testing.T) {
-	// The system's temporary directory is one that does not exist, so that
-	// Coraza could write nothing there: built without no_fs_access, it
-	// refuses to load, and could not write a file of a multipart body.
-	t.Setenv("TMPDIR", filepath.Join(t.TempDir(), "missing"))
-
-	body := "--b\r\nContent-Disposition: form-data; name=\"file\"; " +
-		"filename=\"notes.txt\"\r\nContent-Type: text/plain\r\n\r\n" +
-		strings.Repeat("a", 1000) + "\r\n" + field("q", injection) + end
-	wantBody(t, readingBodies(t), post(multipart), body, matched(942100), body)
-}
-
-func TestBodyLimitOf1GLoads(t *testing.T) {
-	t.Parallel()
-
-	_, err := waf.New(waf.Params{ParanoiaLevel: 1, BodyLimit: 1 << 30})
-	if err != nil {
-		t.Errorf("load the Core Rule Set reading bodies up to 1G: %v", err)
-	}
-}

script/build

@@ -1,7 +1,7 @@
 #!/bin/sh
 # script/build: build bin/smallwebwaf on the host, with Go installed, for
 # working on the code by hand. The version it reports comes from git, as
-# in script/docker, and the no_fs_access tag is the Dockerfile's.
+# in script/docker.
 set -eu
 
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
@@ -11,7 +11,7 @@ main() {
     cd "$ROOT"
     version="$(git describe --tags --always --dirty 2>/dev/null || true)"
     [ -n "$version" ] || version="unknown"
-    go build -tags no_fs_access -trimpath -ldflags "-X main.Version=$version" \
+    go build -trimpath -ldflags "-X main.Version=$version" \
         -o bin/smallwebwaf ./cmd/smallwebwaf
 }