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; SWWAF_WAF_DISABLED_RULES refuses
900000 to 900999, smallwebwaf's own rules among them. A request with more
query parameters than Coraza reads, 1000, adds 5 (rule 900300). 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,18 +205,20 @@ 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 score at or over
+  level `SWWAF_WAF_PARANOIA_LEVEL` sets. A request with more than 1000 query
-  `SWWAF_WAF_ANOMALY_THRESHOLD`, 5 by default, is a match: in `block` mode, the
+  parameters adds 5 (rule 900300), as a rule rated critical does, since Coraza
-  default, the request is refused with `403`, and in `detect` mode it goes on to
+  reads only the first 1000. A score at or over `SWWAF_WAF_ANOMALY_THRESHOLD`, 5
-  the app. Either way its log line names the rules and the score (see
+  by default, is a match: in `block` mode, the default, the request is refused
-  `waf_rule_ids` and `waf_score` in "Request log" below), and it raises a
+  with `403`, and in `detect` mode it goes on to the app. Either way its log
-  `waf_block` alert. A refusal bans no one by itself, since the Core Rule Set
+  line names the rules and the score (see `waf_rule_ids` and `waf_score` in
-  takes some ordinary requests for attacks, but it is an offence the client's
+  "Request log" below), and it raises a `waf_block` alert. A refusal bans no one
-  history counts, and it counts toward the error burst; a match in `detect` mode
+  by itself, since the Core Rule Set takes some ordinary requests for attacks,
-  is neither. A request a rule file refuses, one for a path
+  but it is an offence the client's history counts, and it counts toward the
-  `SWWAF_WAF_EXEMPT_PATHS` exempts, and one from a client in `SWWAF_ALLOW_NETS`
+  error burst; a match in `detect` mode is neither. A request a rule file
-  are not inspected. `smallwebwaf` changes the Core Rule Set in six ways, so
+  refuses, one for a path `SWWAF_WAF_EXEMPT_PATHS` exempts, and one from a
-  that gitea's ordinary requests get through, and no setting undoes them:
+  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
@@ -686,7 +688,9 @@ 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, and the id of no rule switches nothing off.
+  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.
 - `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,6 +352,12 @@ 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 = "********"
@@ -417,6 +423,9 @@ 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, " +
@@ -1644,7 +1653,8 @@ 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.
+// rules, each a whole number above zero and outside firstSetupRuleID to
+// lastSetupRuleID.
 func parseRuleIDs(value string) ([]int, error) {
 	items, err := parseList(value)
 	if err != nil {
@@ -1658,6 +1668,10 @@ 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,7 +611,11 @@ func TestCoreRuleSetSettings(t *testing.T) {
 func TestInvalidCoreRuleSetSettingStopsTheStart(t *testing.T) {
 	t.Parallel()
 
-	const notParanoiaLevel = " is not a paranoia level, from 1 to 4"
+	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"
+	)
 
 	for _, tc := range []struct{ name, value, want string }{
 		{wafMode, "enforce", `"enforce" is not off, detect or block`},
@@ -632,6 +636,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, 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,6 +205,7 @@ func TestBannedClientIsRefusedBeforeItsCountryIsLookedUp(t *testing.T) {
 
 	geojsURL, asked := startGeoJS(t)
 	s, _, _ := startWithClock(t, geojsURL, map[string]string{
+		lookupTimeout:      "1h",
 		rateLimitPerMinute: "1",
 		banScopeV4Prefix:   "24",
 		deniedCountries:    "kp",
@@ -243,6 +244,7 @@ func TestBanResponseAnswersEveryRefusalButTheSizeLimits(t *testing.T) {
 
 			geojsURL, _ := startGeoJS(t)
 			env := map[string]string{
+				lookupTimeout:      "1h",
 				rateLimitPerMinute: "1",
 				denyNets:           denied,
 				deniedCountries:    "kp",
@@ -268,6 +270,7 @@ 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,6 +4,7 @@ import (
 	"net/http"
 	"net/netip"
 	"slices"
+	"strings"
 	"testing"
 
 	"sneak.berlin/go/smallwebwaf/internal/alerts"
@@ -69,6 +70,12 @@ 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}
+			env := map[string]string{trustedProxies: trustLocalhost, lookupTimeout: "1h"}
 			maps.Copy(env, tc.env)
 			addr, out := startProxyWithGeoJS(t, app.URL, geojsURL, env)
 
@@ -101,6 +101,7 @@ func TestCountryRefusalComesBeforeTheBody(t *testing.T) {
 	geojsURL, _ := startGeoJS(t)
 	addr, out := startProxyWithGeoJS(t, app.URL, geojsURL, map[string]string{
 		trustedProxies:  trustLocalhost,
+		lookupTimeout:   "1h",
 		deniedCountries: "kp",
 	})
 
@@ -147,6 +148,7 @@ 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",
 	})
@@ -194,7 +196,7 @@ func TestPrivateAddressIsNeverLookedUp(t *testing.T) {
 
 			app := startApp(t, func(http.ResponseWriter, *http.Request) {})
 			geojsURL, asked := startGeoJS(t)
-			env := map[string]string{trustedProxies: trustLocalhost}
+			env := map[string]string{trustedProxies: trustLocalhost, lookupTimeout: "1h"}
 			maps.Copy(env, tc.env)
 			addr, out := startProxyWithGeoJS(t, app.URL, geojsURL, env)
 
@@ -280,7 +282,11 @@ 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.
+// 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.
 func startGeoJS(t *testing.T) (string, func() []string) {
 	t.Helper()
 

internal/proxy/errorburst_test.go

@@ -194,6 +194,7 @@ 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,6 +18,7 @@ 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,6 +249,7 @@ 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,6 +39,7 @@ 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,6 +144,7 @@ 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,8 +20,11 @@ import (
 )
 
 // directives are the Core Rule Set as smallwebwaf runs it, with the
-// paranoia level for %d. Coraza joins a line ending in \ to the next,
+// paranoia level for %d. Each rule smallwebwaf adds has an id from 900000
-// without the spaces at the start of the next.
+// 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.
 const directives = `
 # The engine only detects. smallwebwaf compares the request's anomaly
 # score with SWWAF_WAF_ANOMALY_THRESHOLD itself, in block and detect mode
@@ -76,6 +79,16 @@ 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,11 +55,14 @@ 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, ": ")
-		if name == "Host" {
+		switch name {
-			// Go's server keeps it out of the headers.
+		case "Host":
 			req.Host = value
-		} else {
+		case "Transfer-Encoding":
+			req.TransferEncoding = []string{value}
+		default:
 			req.Header.Set(name, value)
 		}
 	}
@@ -123,28 +126,63 @@ func TestExpectAndContentEncodingAreAllowed(t *testing.T) {
 	t.Parallel()
 
 	const (
-		pushType   = "Content-Type: application/x-git-receive-pack-request"
+		pushType  = "Content-Type: application/x-git-receive-pack-request"
-		fetchType  = "Content-Type: application/x-git-upload-pack-request"
+		fetchType = "Content-Type: application/x-git-upload-pack-request"
-		length     = "Content-Length: 1024"
+		length    = "Content-Length: 1024"
-		push       = "/owner/repo.git/git-receive-pack"
+		push      = "/owner/repo.git/git-receive-pack"
-		fetch      = "/owner/repo.git/git-upload-pack"
+		fetch     = "/owner/repo.git/git-upload-pack"
-		otherProxy = "Proxy: http://proxy.example"
 	)
 
 	crs := atDefaults(t)
 
-	wantChange(t, crs,
+	wantResult(t, crs,
 		request{http.MethodPost, push, []string{pushType, length, "Expect: 100-continue"}},
-		request{http.MethodPost, push, []string{pushType, length, otherProxy}},
+		waf.Result{})
-		matched(920450))
+	wantResult(t, crs,
-	wantChange(t, crs,
+		request{http.MethodPost, fetch, []string{fetchType, length, "Content-Encoding: gzip"}},
-		request{http.MethodPost, fetch, []string{
+		waf.Result{})
-			fetchType, length, "Content-Encoding: gzip",
+
+	// 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, fetch, []string{
+		waf.Result{})
-			fetchType, length, "Content-Range: bytes 0-1023/1024",
+}
-		}},
+
-		matched(920450))
+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))
 }
 
 func TestRedirectURIMayNameALocalAddress(t *testing.T) {