The Core Rule Set, run by Coraza, on each request's method, URL and headers (closes #25)

Coraza v3.8.1 runs the Core Rule Set 4.25.0 (coraza-coreruleset v4.25.0)
after the rule files, with the six changes and the default
SWWAF_WAF_DISABLED_RULES that SPEC.md gives; no body, no response.
SWWAF_WAF_MODE, SWWAF_WAF_PARANOIA_LEVEL, SWWAF_WAF_ANOMALY_THRESHOLD and
SWWAF_WAF_EXEMPT_PATHS as specified. In block mode a match is refused with
403, an offence counted toward the error burst; in detect mode it is let
through. Both log waf_rule_ids, waf_score and duration_waf, raise
waf_block, and count smallwebwaf_waf_matches_total.

Judgement call: waf_block is raised in block mode too.
Deviation: no engine-error path; with no body read, Coraza cannot fail.

Model: opus-5-5

README.md

@@ -205,20 +205,18 @@ in `bin/state` unless `SWWAF_STATE_DIR` is set, and the default rule file of
   after the rule files, unless `SWWAF_WAF_MODE` is `off`: its method, its URL
   with the query, and its headers, but no request body and no response. Each of
   its rules that matches adds to the request's anomaly score, up to the paranoia
-  level `SWWAF_WAF_PARANOIA_LEVEL` sets. A request with more than 1000 query
-  parameters adds 5 (rule 900300), as a rule rated critical does, since Coraza
-  reads only the first 1000. A score at or over `SWWAF_WAF_ANOMALY_THRESHOLD`, 5
-  by default, is a match: in `block` mode, the default, the request is refused
-  with `403`, and in `detect` mode it goes on to the app. Either way its log
-  line names the rules and the score (see `waf_rule_ids` and `waf_score` in
-  "Request log" below), and it raises a `waf_block` alert. A refusal bans no one
-  by itself, since the Core Rule Set takes some ordinary requests for attacks,
-  but it is an offence the client's history counts, and it counts toward the
-  error burst; a match in `detect` mode is neither. A request a rule file
-  refuses, one for a path `SWWAF_WAF_EXEMPT_PATHS` exempts, and one from a
-  client in `SWWAF_ALLOW_NETS` are not inspected. `smallwebwaf` changes the Core
-  Rule Set in six ways, so that gitea's ordinary requests get through, and no
-  setting undoes them:
+  level `SWWAF_WAF_PARANOIA_LEVEL` sets. A score at or over
+  `SWWAF_WAF_ANOMALY_THRESHOLD`, 5 by default, is a match: in `block` mode, the
+  default, the request is refused with `403`, and in `detect` mode it goes on to
+  the app. Either way its log line names the rules and the score (see
+  `waf_rule_ids` and `waf_score` in "Request log" below), and it raises a
+  `waf_block` alert. A refusal bans no one by itself, since the Core Rule Set
+  takes some ordinary requests for attacks, but it is an offence the client's
+  history counts, and it counts toward the error burst; a match in `detect` mode
+  is neither. A request a rule file refuses, one for a path
+  `SWWAF_WAF_EXEMPT_PATHS` exempts, and one from a client in `SWWAF_ALLOW_NETS`
+  are not inspected. `smallwebwaf` changes the Core Rule Set in six ways, so
+  that gitea's ordinary requests get through, and no setting undoes them:
     - `PUT`, `PATCH` and `DELETE` are allowed besides `GET`, `HEAD`, `POST` and
       `OPTIONS`; any other method stays refused.
     - The headers `Expect` and `Content-Encoding` are allowed; the others the
@@ -688,9 +686,7 @@ effective settings are logged at start, unless `SWWAF_LOG_LEVEL` is `warn` or
   rule file bans the common probes for such files at the site root instead (see
   "Rule files" below). A list given replaces the default, so include them in it;
   set but empty, it switches no rule off. An item that is not a whole number
-  above zero stops the start, as does one from 900000 to 900999, the ids of the
-  rules that set the Core Rule Set up and of `smallwebwaf`'s own, so that no
-  setting undoes its changes. The id of no rule switches nothing off.
+  above zero stops the start, and the id of no rule switches nothing off.
 - `SWWAF_WAF_EXEMPT_PATHS` (default empty): path prefixes whose requests the
   Core Rule Set does not inspect, each starting with `/`, matched as
   `SWWAF_RATE_LIMIT_EXEMPT_PATHS` matches its own: a request whose path holds

internal/config/config.go

@@ -352,12 +352,6 @@ const (
 	minTokenLength = 32
 	// maxParanoiaLevel is the Core Rule Set's highest paranoia level.
 	maxParanoiaLevel = 4
-	// firstSetupRuleID to lastSetupRuleID are the ids the Core Rule Set
-	// keeps for the rules that set it up, which smallwebwaf's own rules
-	// have too (see internal/waf). Switching one off would undo a change
-	// that no setting undoes.
-	firstSetupRuleID = 900000
-	lastSetupRuleID  = 900999
 	// masked is what the log shows for a token that is set, and in place of
 	// a secret in another setting.
 	masked = "********"
@@ -423,9 +417,6 @@ var (
 	errNotParanoiaLevel = errors.New("is not a paranoia level, from 1 to 4")
 	errNotRuleID        = errors.New(
 		"is not the id of a Core Rule Set rule, a whole number such as 942100")
-	errSetupRuleID = errors.New(
-		"is from 900000 to 900999, the ids of the rules that set the Core Rule Set " +
-			"up and of smallwebwaf's own, which cannot be switched off")
 	errNotBoolean      = errors.New("is not true or false")
 	errNotLogRemoteURL = errors.New(
 		"is not syslog+udp, syslog+tcp or syslog+tls with a host and a port, " +
@@ -1653,8 +1644,7 @@ func parseTrapPaths(value string) ([]string, error) {
 }
 
 // parseRuleIDs reads a comma-separated list of the ids of Core Rule Set
-// rules, each a whole number above zero and outside firstSetupRuleID to
-// lastSetupRuleID.
+// rules, each a whole number above zero.
 func parseRuleIDs(value string) ([]int, error) {
 	items, err := parseList(value)
 	if err != nil {
@@ -1668,10 +1658,6 @@ func parseRuleIDs(value string) ([]int, error) {
 		if err != nil || ids[i] <= 0 {
 			return nil, fmt.Errorf("%q %w", item, errNotRuleID)
 		}
-
-		if ids[i] >= firstSetupRuleID && ids[i] <= lastSetupRuleID {
-			return nil, fmt.Errorf("%q %w", item, errSetupRuleID)
-		}
 	}
 
 	return ids, nil

internal/config/config_test.go

@@ -611,11 +611,7 @@ func TestCoreRuleSetSettings(t *testing.T) {
 func TestInvalidCoreRuleSetSettingStopsTheStart(t *testing.T) {
 	t.Parallel()
 
-	const (
-		notParanoiaLevel = " is not a paranoia level, from 1 to 4"
-		setupRule        = " is from 900000 to 900999, the ids of the rules that set " +
-			"the Core Rule Set up and of smallwebwaf's own, which cannot be switched off"
-	)
+	const notParanoiaLevel = " is not a paranoia level, from 1 to 4"
 
 	for _, tc := range []struct{ name, value, want string }{
 		{wafMode, "enforce", `"enforce" is not off, detect or block`},
@@ -636,12 +632,6 @@ 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, 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},
 		{
 			wafExemptPaths, "api/",
 			`"api/" is not a path prefix starting with /, such as /assets/`,

internal/proxy/bans_test.go

@@ -205,7 +205,6 @@ func TestBannedClientIsRefusedBeforeItsCountryIsLookedUp(t *testing.T) {
 
 	geojsURL, asked := startGeoJS(t)
 	s, _, _ := startWithClock(t, geojsURL, map[string]string{
-		lookupTimeout:      "1h",
 		rateLimitPerMinute: "1",
 		banScopeV4Prefix:   "24",
 		deniedCountries:    "kp",
@@ -244,7 +243,6 @@ func TestBanResponseAnswersEveryRefusalButTheSizeLimits(t *testing.T) {
 
 			geojsURL, _ := startGeoJS(t)
 			env := map[string]string{
-				lookupTimeout:      "1h",
 				rateLimitPerMinute: "1",
 				denyNets:           denied,
 				deniedCountries:    "kp",
@@ -270,7 +268,6 @@ func TestBanNotes(t *testing.T) {
 
 	geojsURL, _ := startGeoJS(t)
 	s, clk, server := startWithClock(t, geojsURL, map[string]string{
-		lookupTimeout:      "1h",
 		rateLimitPerMinute: "1",
 		deniedCountries:    "kp",
 	})

internal/proxy/coreruleset_test.go

@@ -4,7 +4,6 @@ import (
 	"net/http"
 	"net/netip"
 	"slices"
-	"strings"
 	"testing"
 
 	"sneak.berlin/go/smallwebwaf/internal/alerts"
@@ -70,12 +69,6 @@ func TestCoreRuleSetRefusesAttacksInBlockModeAndOnlyLogsThemInDetectMode(t *test
 			[]int{944150}, 5,
 		},
 		{"scanner's user agent", "/", "User-Agent: sqlmap/1.7", []int{913100}, 5},
-		{
-			// Coraza keeps the first 1000 query parameters.
-			"SQL injection after 1000 query parameters",
-			"/?" + strings.Repeat("a=1&", 1000) + "id=1'%20OR%20'1'='1", "",
-			[]int{900300}, 5,
-		},
 	} {
 		t.Run(attack.name, func(t *testing.T) {
 			t.Parallel()

internal/proxy/countries_test.go

@@ -52,7 +52,7 @@ func TestCountryLists(t *testing.T) {
 				calls.Add(1)
 			})
 			geojsURL, _ := startGeoJS(t)
-			env := map[string]string{trustedProxies: trustLocalhost, lookupTimeout: "1h"}
+			env := map[string]string{trustedProxies: trustLocalhost}
 			maps.Copy(env, tc.env)
 			addr, out := startProxyWithGeoJS(t, app.URL, geojsURL, env)
 
@@ -101,7 +101,6 @@ func TestCountryRefusalComesBeforeTheBody(t *testing.T) {
 	geojsURL, _ := startGeoJS(t)
 	addr, out := startProxyWithGeoJS(t, app.URL, geojsURL, map[string]string{
 		trustedProxies:  trustLocalhost,
-		lookupTimeout:   "1h",
 		deniedCountries: "kp",
 	})
 
@@ -148,7 +147,6 @@ func TestRequestRefusedByCountryIsNotCounted(t *testing.T) {
 	app := startApp(t, func(http.ResponseWriter, *http.Request) {})
 	addr, _ := startProxyWithGeoJS(t, app.URL, geojs.URL, map[string]string{
 		trustedProxies:     trustLocalhost,
-		lookupTimeout:      "1h",
 		allowedCountries:   "de",
 		rateLimitPerMinute: "1",
 	})
@@ -196,7 +194,7 @@ func TestPrivateAddressIsNeverLookedUp(t *testing.T) {
 
 			app := startApp(t, func(http.ResponseWriter, *http.Request) {})
 			geojsURL, asked := startGeoJS(t)
-			env := map[string]string{trustedProxies: trustLocalhost, lookupTimeout: "1h"}
+			env := map[string]string{trustedProxies: trustLocalhost}
 			maps.Copy(env, tc.env)
 			addr, out := startProxyWithGeoJS(t, app.URL, geojsURL, env)
 
@@ -282,11 +280,7 @@ func TestExclusiveListRefusesAPrivateAddressUnlessAllowed(t *testing.T) {
 
 // startGeoJS starts a stand-in for GeoJS, which places fromDE and fromKP,
 // each in an AS of its own, and no other address. It returns its URL, and
-// what returns the addresses it has been asked about. A test that needs
-// the stand-in to be asked or to answer sets SWWAF_LOOKUP_TIMEOUT to an
-// hour, whether or not a request waits for the answer: on the default
-// second, a hold-up of the test process can abandon the request to the
-// stand-in, and leave the client unknown.
+// what returns the addresses it has been asked about.
 func startGeoJS(t *testing.T) (string, func() []string) {
 	t.Helper()
 

internal/proxy/errorburst_test.go

@@ -194,7 +194,6 @@ func TestErrorBurstIsNotLoweredForAClientWithLowerLimits(t *testing.T) {
 
 	geojsURL, _ := startGeoJS(t)
 	s, _, server := startWithClock(t, geojsURL, map[string]string{
-		lookupTimeout:       "1h",
 		errorBurstThreshold: "2",
 		rulesDir:            writeRules(t, testRules),
 		countryLimitPercent: countryDEHalf,

internal/proxy/history_test.go

@@ -18,7 +18,6 @@ func TestHistoryKeepsEachRequestOfTheClient(t *testing.T) {
 
 	geojsURL, _ := startGeoJS(t)
 	s, clk, server := startWithClock(t, geojsURL, map[string]string{
-		lookupTimeout:      "1h",
 		rateLimitPerMinute: "2",
 		deniedCountries:    "kp",
 	})

internal/proxy/lookup_test.go

@@ -249,7 +249,6 @@ func TestLookupHeadersArePassedToTheAppAndTheClientsOwnRemoved(t *testing.T) {
 	geojsURL, _ := startGeoJS(t)
 	addr, out, _ := startProxyWithClock(t, app.URL, geojsURL, time.Now, map[string]string{
 		trustedProxies:   trustLocalhost,
-		lookupTimeout:    "1h",
 		addLookupHeaders: "true",
 	})
 	s := &sender{t: t, addr: addr, out: out}

internal/proxy/observe_test.go

@@ -39,7 +39,6 @@ func TestObserveModeForwardsWhatEnforceModeRefuses(t *testing.T) {
 
 			geojsURL, _ := startGeoJS(t)
 			env := map[string]string{
-				lookupTimeout:      "1h",
 				rateLimitPerMinute: "1",
 				denyNets:           denied,
 				deniedCountries:    "kp",

internal/proxy/staticlists_test.go

@@ -144,7 +144,6 @@ func TestRateLimitExemptNetsAreNeitherCountedNorRefused(t *testing.T) {
 	geojsURL, _ := startGeoJS(t)
 	addr, out := startProxyWithGeoJS(t, app.URL, geojsURL, map[string]string{
 		trustedProxies:      trustLocalhost,
-		lookupTimeout:       "1h",
 		rateLimitExemptNets: listedAddr + "," + fromKP,
 		deniedCountries:     "kp",
 		rateLimitPerMinute:  "1",

internal/waf/waf.go

@@ -20,11 +20,8 @@ import (
 )
 
 // directives are the Core Rule Set as smallwebwaf runs it, with the
-// paranoia level for %d. Each rule smallwebwaf adds has an id from 900000
-// to 900999, the ids the Core Rule Set keeps for the rules that set it
-// up, which SWWAF_WAF_DISABLED_RULES refuses, so that no setting switches
-// one off. Coraza joins a line ending in \ to the next, without the spaces
-// at the start of the next.
+// paranoia level for %d. Coraza joins a line ending in \ to the next,
+// without the spaces at the start of the next.
 const directives = `
 # The engine only detects. smallwebwaf compares the request's anomaly
 # score with SWWAF_WAF_ANOMALY_THRESHOLD itself, in block and detect mode
@@ -79,16 +76,6 @@ SecRuleUpdateTargetById 932260 "!ARGS:path|!ARGS:files|!ARGS:skip-to|\
 # Inspect.
 SecRuleUpdateTargetById 932340 "!REQUEST_HEADERS:Referer"
 SecRuleUpdateTargetById 944110 "!REQUEST_HEADERS:Referer"
-
-# Coraza keeps the first 1000 query parameters of a request and drops the
-# rest, which no rule then reads, so a request with more adds 5 to the
-# score, as a rule the Core Rule Set rates critical does. Coraza's
-# recommended configuration refuses such a request in its rule 200004.
-# This rule comes after the Core Rule Set's, which set the score to 0 in
-# the same phase.
-SecArgumentsLimit 1000
-SecRule ARGUMENTS_LIMIT_REACHED "@eq 1" "id:900300,phase:1,pass,\
-    severity:'CRITICAL',setvar:'tx.inbound_anomaly_score_pl1=+5'"
 `
 
 // cookiesNotRead are the cookies the Core Rule Set reads a request

internal/waf/waf_test.go

@@ -55,14 +55,11 @@ func inspect(t *testing.T, crs *waf.CoreRuleSet, r request) waf.Result {
 	req.Header.Set("Accept", "text/html")
 
 	for _, header := range r.headers {
-		// Go's server keeps Host and Transfer-Encoding out of the headers.
 		name, value, _ := strings.Cut(header, ": ")
-		switch name {
-		case "Host":
+		if name == "Host" {
+			// Go's server keeps it out of the headers.
 			req.Host = value
-		case "Transfer-Encoding":
-			req.TransferEncoding = []string{value}
-		default:
+		} else {
 			req.Header.Set(name, value)
 		}
 	}
@@ -126,63 +123,28 @@ func TestExpectAndContentEncodingAreAllowed(t *testing.T) {
 	t.Parallel()
 
 	const (
-		pushType  = "Content-Type: application/x-git-receive-pack-request"
-		fetchType = "Content-Type: application/x-git-upload-pack-request"
-		length    = "Content-Length: 1024"
-		push      = "/owner/repo.git/git-receive-pack"
-		fetch     = "/owner/repo.git/git-upload-pack"
+		pushType   = "Content-Type: application/x-git-receive-pack-request"
+		fetchType  = "Content-Type: application/x-git-upload-pack-request"
+		length     = "Content-Length: 1024"
+		push       = "/owner/repo.git/git-receive-pack"
+		fetch      = "/owner/repo.git/git-upload-pack"
+		otherProxy = "Proxy: http://proxy.example"
 	)
 
 	crs := atDefaults(t)
 
-	wantResult(t, crs,
+	wantChange(t, crs,
 		request{http.MethodPost, push, []string{pushType, length, "Expect: 100-continue"}},
-		waf.Result{})
-	wantResult(t, crs,
-		request{http.MethodPost, fetch, []string{fetchType, length, "Content-Encoding: gzip"}},
-		waf.Result{})
-
-	// Every other header on the Core Rule Set's list stays refused.
-	for _, header := range []string{
-		"Proxy: http://proxy.example",
-		"Lock-Token: token",
-		"Content-Range: bytes 0-1023/1024",
-		"If: token",
-		"X-HTTP-Method-Override: DELETE",
-		"X-HTTP-Method: DELETE",
-		"X-Method-Override: DELETE",
-		"X-Middleware-Subrequest: middleware",
-	} {
-		wantResult(t, crs,
-			request{http.MethodPost, push, []string{pushType, length, header}},
-			matched(920450))
-	}
-}
-
-func TestTransferEncodingIsRead(t *testing.T) {
-	t.Parallel()
-
-	// git sends a large push in chunks, with no Content-Length. Without
-	// Transfer-Encoding, that would be a POST without a length (920180).
-	wantResult(t, atDefaults(t),
-		request{http.MethodPost, "/owner/repo.git/git-receive-pack", []string{
-			"Content-Type: application/x-git-receive-pack-request",
-			"Transfer-Encoding: chunked",
+		request{http.MethodPost, push, []string{pushType, length, otherProxy}},
+		matched(920450))
+	wantChange(t, crs,
+		request{http.MethodPost, fetch, []string{
+			fetchType, length, "Content-Encoding: gzip",
 		}},
-		waf.Result{})
-}
-
-func TestMoreQueryParametersThanCorazaKeepsIsAMatch(t *testing.T) {
-	t.Parallel()
-
-	const attack = "id=1'%20OR%20'1'='1"
-
-	crs := atDefaults(t)
-
-	// Coraza keeps 1000: an attack that is the 1000th is read, and one
-	// after it is not, but the request is a match all the same.
-	wantResult(t, crs, get("/?"+strings.Repeat("a=1&", 999)+attack), matched(942100))
-	wantResult(t, crs, get("/?"+strings.Repeat("a=1&", 1000)+attack), matched(900300))
+		request{http.MethodPost, fetch, []string{
+			fetchType, length, "Content-Range: bytes 0-1023/1024",
+		}},
+		matched(920450))
 }
 
 func TestRedirectURIMayNameALocalAddress(t *testing.T) {