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

internal/waf/waf.go

@@ -0,0 +1,227 @@
+// Package waf runs the OWASP Core Rule Set 4.25.0, through Coraza, on the
+// method, the URL with its query and the headers of a request, with the
+// six changes smallwebwaf makes to it, as "Attack detection" under
+// "Configuration surface" in SPEC.md describes them. It reads no request
+// body and no response.
+package waf
+
+import (
+	"fmt"
+	"net/http"
+	"net/netip"
+	"slices"
+	"strconv"
+	"strings"
+
+	coreruleset "github.com/corazawaf/coraza-coreruleset/v4"
+	"github.com/corazawaf/coraza/v3"
+	"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes"
+	"github.com/corazawaf/coraza/v3/types"
+)
+
+// 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.
+const directives = `
+# The engine only detects. smallwebwaf compares the request's anomaly
+# score with SWWAF_WAF_ANOMALY_THRESHOLD itself, in block and detect mode
+# alike. It reads no body.
+SecRuleEngine DetectionOnly
+SecRequestBodyAccess Off
+SecResponseBodyAccess Off
+
+Include @crs-setup.conf.example
+
+SecAction "id:900000,phase:1,pass,nolog,\
+    setvar:tx.blocking_paranoia_level=%d"
+
+# The first change: PUT, PATCH and DELETE are allowed besides GET, HEAD,
+# POST and OPTIONS.
+SecAction "id:900200,phase:1,pass,nolog,\
+    setvar:'tx.allowed_methods=GET HEAD POST OPTIONS PUT PATCH DELETE'"
+
+# The second: Expect and Content-Encoding are taken off the Core Rule Set's
+# list of the headers it refuses. Content-Encoding stays refused on a body
+# the Core Rule Set reads, and it reads none.
+SecAction "id:900250,phase:1,pass,nolog,\
+    setvar:'tx.restricted_headers_basic=/proxy/ /lock-token/ /content-range/ \
+    /if/ /x-http-method-override/ /x-http-method/ /x-method-override/ \
+    /x-middleware-subrequest/'"
+
+# The sixth: only the rules for requests are loaded, and no response is
+# inspected.
+Include @owasp_crs/REQUEST-*.conf
+
+# The third: redirect_uri is not checked for a URL naming an IP address or
+# localhost.
+SecRuleUpdateTargetById 931100 "!ARGS:redirect_uri"
+SecRuleUpdateTargetById 934110 "!ARGS:redirect_uri"
+
+# The fourth: the query parameters in which gitea sends names within a
+# repository or its own records, or a page of its own site, are not
+# checked against the lists of system files, shell paths and command
+# names. Coraza takes one rule id per directive.
+SecRuleUpdateTargetById 930120 "!ARGS:path|!ARGS:files|!ARGS:skip-to|\
+    !ARGS:sub_path|!ARGS:ref|!ARGS:sha|!ARGS:branch|!ARGS:workflow|\
+    !ARGS:artifactName|!ARGS:redirect_to"
+SecRuleUpdateTargetById 932160 "!ARGS:path|!ARGS:files|!ARGS:skip-to|\
+    !ARGS:sub_path|!ARGS:ref|!ARGS:sha|!ARGS:branch|!ARGS:workflow|\
+    !ARGS:artifactName|!ARGS:redirect_to"
+SecRuleUpdateTargetById 932260 "!ARGS:path|!ARGS:files|!ARGS:skip-to|\
+    !ARGS:sub_path|!ARGS:ref|!ARGS:sha|!ARGS:branch|!ARGS:workflow|\
+    !ARGS:artifactName|!ARGS:redirect_to"
+
+# The fifth, for Referer: it is not checked for a Unix command without
+# arguments, or for Java starting a process. The cookies are left out in
+# 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
+// without, the rest of the fifth change.
+//
+//nolint:gochecknoglobals // a constant cannot be a list
+var cookiesNotRead = []string{"gitea_flash", "redirect_to"}
+
+// Params are what New needs.
+type Params struct {
+	// ParanoiaLevel is SWWAF_WAF_PARANOIA_LEVEL, from 1 to 4.
+	ParanoiaLevel int
+	// DisabledRules are the ids of the rules switched off
+	// (SWWAF_WAF_DISABLED_RULES).
+	DisabledRules []int
+}
+
+// CoreRuleSet is the Core Rule Set, ready to inspect requests. It is safe
+// for concurrent use.
+type CoreRuleSet struct {
+	waf coraza.WAF
+}
+
+// New returns the Core Rule Set with the six changes, at params'
+// paranoia level and without the rules it switches off.
+func New(params Params) (*CoreRuleSet, error) {
+	text := fmt.Sprintf(directives, params.ParanoiaLevel)
+
+	if len(params.DisabledRules) > 0 {
+		ids := make([]string, len(params.DisabledRules))
+		for i, id := range params.DisabledRules {
+			ids[i] = strconv.Itoa(id)
+		}
+
+		text += "SecRuleRemoveById " + strings.Join(ids, " ") + "\n"
+	}
+
+	waf, err := coraza.NewWAF(coraza.NewWAFConfig().
+		WithRootFS(coreruleset.FS).
+		WithDirectives(text))
+	if err != nil {
+		return nil, fmt.Errorf("load the Core Rule Set: %w", err)
+	}
+
+	return &CoreRuleSet{waf: waf}, nil
+}
+
+// Result is what the Core Rule Set found in a request.
+type Result struct {
+	// RuleIDs are the ids of the rules that matched, in the order they
+	// ran.
+	RuleIDs []int
+	// Score is the request's anomaly score: what those rules add up to.
+	Score int
+}
+
+// Inspect runs the Core Rule Set on r, a request from client: on its
+// method, its URL with the query, and its headers, the Cookie header
+// without the cookies in cookiesNotRead.
+func (c *CoreRuleSet) Inspect(r *http.Request, client netip.Addr) Result {
+	tx := c.waf.NewTransaction()
+	// With no body read, there is nothing whose closing can fail.
+	defer func() { _ = tx.Close() }()
+
+	tx.ProcessConnection(client.String(), 0, "", 0)
+	tx.ProcessURI(r.URL.String(), r.Method, r.Proto)
+
+	for name, values := range r.Header {
+		for _, value := range values {
+			if name == "Cookie" {
+				value = withoutCookiesNotRead(value)
+				if value == "" {
+					continue // it held those cookies alone
+				}
+			}
+
+			tx.AddRequestHeader(name, value)
+		}
+	}
+
+	// Go's server takes these two out of the headers.
+	tx.AddRequestHeader("Host", r.Host)
+
+	for _, encoding := range r.TransferEncoding {
+		tx.AddRequestHeader("Transfer-Encoding", encoding)
+	}
+
+	tx.ProcessRequestHeaders()
+	// With no body read, this runs the rest of the rules, and cannot fail.
+	_, _ = tx.ProcessRequestBody()
+
+	var ids []int
+
+	for _, matched := range tx.MatchedRules() {
+		// The rules that look for attacks have a severity; the others set
+		// the Core Rule Set up and add up the score.
+		rule := matched.Rule()
+		if rule.Severity() != types.RuleSeverityUnset {
+			ids = append(ids, rule.ID())
+		}
+	}
+
+	return Result{RuleIDs: ids, Score: score(tx)}
+}
+
+// score returns the anomaly score the Core Rule Set added up in tx, a
+// transaction it has run, or 0 if a rule that adds it up is switched off.
+func score(tx types.Transaction) int {
+	// The score is in a variable of the transaction, which only Coraza's
+	// interface for plugins reads.
+	state := tx.(plugintypes.TransactionState) //nolint:forcetypeassert // every one is
+
+	values := state.Variables().TX().Get("blocking_inbound_anomaly_score")
+	if len(values) == 0 {
+		return 0
+	}
+
+	n, _ := strconv.Atoi(values[0])
+
+	return n
+}
+
+// withoutCookiesNotRead returns value, a Cookie header's, without the
+// cookies in cookiesNotRead.
+func withoutCookiesNotRead(value string) string {
+	var kept []string
+
+	for cookie := range strings.SplitSeq(value, ";") {
+		name, _, _ := strings.Cut(strings.TrimSpace(cookie), "=")
+		if !slices.Contains(cookiesNotRead, name) {
+			kept = append(kept, cookie)
+		}
+	}
+
+	return strings.Join(kept, ";")
+}

internal/waf/waf_test.go

@@ -0,0 +1,310 @@
+package waf_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/netip"
+	"reflect"
+	"strings"
+	"testing"
+
+	"sneak.berlin/go/smallwebwaf/internal/waf"
+)
+
+// defaultDisabledRules are the rules SWWAF_WAF_DISABLED_RULES switches off
+// by default.
+//
+//nolint:gochecknoglobals // a constant cannot be a list
+var defaultDisabledRules = []int{920340, 920420, 920440, 920640, 930130, 930140}
+
+// newCoreRuleSet returns the Core Rule Set at paranoia level level, with
+// the rules in disabled switched off.
+func newCoreRuleSet(t *testing.T, level int, disabled ...int) *waf.CoreRuleSet {
+	t.Helper()
+
+	crs, err := waf.New(waf.Params{ParanoiaLevel: level, DisabledRules: disabled})
+	if err != nil {
+		t.Fatalf("load the Core Rule Set: %v", err)
+	}
+
+	return crs
+}
+
+// request is a request a test inspects: its method, its target, the path
+// and the query as a client sends them, and its headers, each written
+// "Name: value".
+type request struct {
+	method, target string
+	headers        []string
+}
+
+// get is a GET request for target with headers.
+func get(target string, headers ...string) request {
+	return request{http.MethodGet, target, headers}
+}
+
+// inspect returns what crs finds in r, sent to git.example by a browser,
+// whose Host, User-Agent and Accept r.headers may replace.
+func inspect(t *testing.T, crs *waf.CoreRuleSet, r request) waf.Result {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), r.method,
+		"http://git.example"+r.target, http.NoBody)
+	req.Header.Set("User-Agent", "Mozilla/5.0 (X11; Linux x86_64; rv:131.0) "+
+		"Gecko/20100101 Firefox/131.0")
+	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":
+			req.Host = value
+		case "Transfer-Encoding":
+			req.TransferEncoding = []string{value}
+		default:
+			req.Header.Set(name, value)
+		}
+	}
+
+	return crs.Inspect(req, netip.MustParseAddr("203.0.113.9"))
+}
+
+// wantResult checks what crs finds in r.
+func wantResult(t *testing.T, crs *waf.CoreRuleSet, r request, want waf.Result) {
+	t.Helper()
+
+	if got := inspect(t, crs, r); !reflect.DeepEqual(got, want) {
+		t.Errorf("%s %s %q: %+v, want %+v", r.method, r.target, r.headers, got, want)
+	}
+}
+
+// matched is the result of a request that the rules ids match, each of
+// them a critical one, which adds 5 to the score.
+func matched(ids ...int) waf.Result {
+	const critical = 5
+
+	return waf.Result{RuleIDs: ids, Score: critical * len(ids)}
+}
+
+// atDefaults returns the Core Rule Set as smallwebwaf runs it by default.
+func atDefaults(t *testing.T) *waf.CoreRuleSet {
+	t.Helper()
+
+	return newCoreRuleSet(t, 1, defaultDisabledRules...)
+}
+
+// wantChange checks that crs lets through passes, a gitea request one of
+// the six changes is for, and still finds result in refused, a request
+// like it that the change is not for.
+func wantChange(
+	t *testing.T, crs *waf.CoreRuleSet, passes, refused request, result waf.Result,
+) {
+	t.Helper()
+
+	wantResult(t, crs, passes, waf.Result{})
+	wantResult(t, crs, refused, result)
+}
+
+func TestPutPatchAndDeleteAreAllowed(t *testing.T) {
+	t.Parallel()
+
+	crs := atDefaults(t)
+
+	for _, r := range []request{
+		{http.MethodPut, "/v2/owner/image/blobs/uploads/1?digest=sha256:ab", nil},
+		{http.MethodPatch, "/api/v1/repos/owner/repo/issues/1", nil},
+		{http.MethodDelete, "/api/v1/repos/owner/repo/branches/old", nil},
+	} {
+		wantChange(t, crs, r, request{http.MethodTrace, r.target, nil}, matched(911100))
+	}
+
+	wantChange(t, crs, get("/"), request{"PROPFIND", "/", nil}, matched(911100))
+}
+
+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"
+	)
+
+	crs := atDefaults(t)
+
+	wantResult(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",
+		}},
+		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))
+}
+
+func TestRedirectURIMayNameALocalAddress(t *testing.T) {
+	t.Parallel()
+
+	const oauth = "/login/oauth/authorize?client_id=tea&response_type=code&"
+
+	crs := atDefaults(t)
+
+	wantChange(t, crs, get(oauth+"redirect_uri=http://127.0.0.1:52341/"),
+		get(oauth+"next=http://127.0.0.1:52341/"), matched(931100, 934110))
+	wantChange(t, crs, get(oauth+"redirect_uri=http://localhost:52341/"),
+		get(oauth+"next=http://localhost:52341/"), matched(934110))
+}
+
+func TestParametersGiteaSendsNamesInSkipTheListsOfFilesPathsAndCommands(t *testing.T) {
+	t.Parallel()
+
+	crs := atDefaults(t)
+
+	for _, value := range []struct {
+		name string
+		// result is what a parameter that is not one of gitea's gets.
+		result waf.Result
+	}{
+		// A file on the list of system files.
+		{".gitignore", matched(930120)},
+		// A command's name, after a directory on the list of shell paths.
+		{"bin/docker-entrypoint", matched(932260, 932160)},
+	} {
+		for _, parameter := range []string{
+			"path", "files", "skip-to", "sub_path", "ref", "sha", "branch", "workflow",
+			"artifactName", "redirect_to",
+		} {
+			wantChange(t, crs, get("/?"+parameter+"="+value.name),
+				get("/?q="+value.name), value.result)
+		}
+	}
+
+	// What only those rules refuse gets through there too, but path
+	// traversal and SQL injection are still refused.
+	wantChange(t, crs, get("/?path=|cat%20/etc/passwd"), get("/?q=|cat%20/etc/passwd"),
+		matched(930120, 932160))
+	wantResult(t, crs, get("/?path=../../etc/passwd"),
+		waf.Result{RuleIDs: []int{930100, 930110}, Score: 20})
+	wantResult(t, crs, get("/?path=1'%20OR%20'1'='1"), matched(942100))
+}
+
+func TestCookiesGiteaFlashAndRedirectToAreNotRead(t *testing.T) {
+	t.Parallel()
+
+	const (
+		flash      = "success%3DFile%2Bpackage.json%2Bdeleted"
+		redirectTo = "%2Fowner%2Frepo%2Fsrc%2Fbranch%2Fmain%2Fpackage.json"
+	)
+
+	crs := atDefaults(t)
+
+	wantChange(t, crs, get("/owner/repo", "Cookie: gitea_flash="+flash),
+		get("/owner/repo", "Cookie: flash="+flash), matched(930120))
+	wantChange(t, crs, get("/", "Cookie: redirect_to="+redirectTo),
+		get("/", "Cookie: redirect="+redirectTo), matched(930120))
+
+	// Among other cookies, which are read.
+	wantChange(t, crs,
+		get("/", "Cookie: lang=en-US; gitea_flash="+flash+"; redirect_to="+redirectTo+
+			"; i_like_gitea=abc"),
+		get("/", "Cookie: lang=en-US; gitea_flash="+flash+"; redirect="+redirectTo+
+			"; i_like_gitea=abc"),
+		matched(930120))
+}
+
+func TestRefererIsNotCheckedForACommandOrJavaStartingAProcess(t *testing.T) {
+	t.Parallel()
+
+	const (
+		search      = "https://git.example/explore/repos?q=env"
+		runtimeJava = "https://git.example/openjdk/jdk/src/branch/master/src/" +
+			"java.base/share/classes/java/lang/Runtime.java"
+	)
+
+	crs := atDefaults(t)
+
+	wantChange(t, crs, get("/", "Referer: "+search), get("/", "User-Agent: "+search),
+		matched(932340))
+	wantChange(t, crs, get("/", "Referer: "+runtimeJava),
+		get("/", "X-Page: "+runtimeJava), matched(944110))
+
+	// It is still checked for script and SQL injection.
+	wantResult(t, crs,
+		get("/", "Referer: https://git.example/?q=<script>alert(1)</script>"),
+		matched(941110, 941160))
+	wantResult(t, crs, get("/", "Referer: https://git.example/?q=1' OR '1'='1"),
+		matched(942100))
+}
+
+func TestEmptyHeaderIsRead(t *testing.T) {
+	t.Parallel()
+
+	// An empty User-Agent is a notice, which adds 2.
+	wantResult(t, atDefaults(t), get("/", "User-Agent: "),
+		waf.Result{RuleIDs: []int{920330}, Score: 2})
+}
+
+func TestParanoiaLevel(t *testing.T) {
+	t.Parallel()
+
+	// Accept-Charset is refused from paranoia level 2.
+	r := get("/", "Accept-Charset: utf-8")
+
+	wantResult(t, newCoreRuleSet(t, 1), r, waf.Result{})
+	wantResult(t, newCoreRuleSet(t, 2), r, matched(920451))
+}
+
+func TestEachDisabledRuleIsSwitchedOff(t *testing.T) {
+	t.Parallel()
+
+	// A method not allowed, and a Host that is an IP address, a warning,
+	// which adds 3.
+	r := request{http.MethodTrace, "/", []string{"Host: 192.0.2.1"}}
+
+	wantResult(t, newCoreRuleSet(t, 1), r,
+		waf.Result{RuleIDs: []int{911100, 920350}, Score: 8})
+	wantResult(t, newCoreRuleSet(t, 1, 920350, 911100), r, waf.Result{})
+}