Remove everything specific to a Gitea deployment (closes #209)

Delete the Gitea compose setup, its document, its two scripts and their
make targets. Drop the Core Rule Set exclusions written for Gitea: the
ten query parameters skipped by rules 930120, 932160 and 932260, and the
gitea_flash and redirect_to cookies. The remaining changes are
renumbered, five in all. Documents and tests use neutral examples.

Model: opus-5-5

README.md

@@ -553,7 +553,7 @@ and the status for a size or time limit, such as `413` for a body too large.
   headers of every request, and no body unless `SWWAF_WAF_BODY_LIMIT` is set. In
   front of a code forge, bodies carry what people write and publish, which the
   rules take for attacks.
-- `smallwebwaf` changes the Core Rule Set in six ways so that it lets ordinary
+- `smallwebwaf` changes the Core Rule Set in five ways so that it lets ordinary
   requests to a code forge through, such as git over HTTP and sign-in through
   Git Credential Manager. No setting undoes them; "Attack detection" in
   [`SPEC.md`](SPEC.md) lists them.

SPEC.md

@@ -81,7 +81,7 @@ to them.
       of bans is full.
     - `internal/rules`: reads the rule files at start and again when they
       change, and tells which rules a request matches.
-    - `internal/waf`: the Core Rule Set, with the six changes listed under
+    - `internal/waf`: the Core Rule Set, with the five changes listed under
       "Attack detection", as Coraza directives.
     - `internal/lookup`: looks clients up through GeoJS or in the lookup
       database.
@@ -610,9 +610,9 @@ files, the limits and the bans still apply to the client that sends it.
     scanner from an ordinary client. In front of a code forge, registry and git
     clients get `404` and `401` by design, hundreds of times a minute.
 
-`smallwebwaf` changes the Core Rule Set 4.25.0 in six ways, since in front of
-gitea it would otherwise refuse ordinary requests. The changes hold in front of
-every app, and no setting undoes them:
+`smallwebwaf` changes the Core Rule Set 4.25.0 in five ways, since in front of a
+code forge it would otherwise refuse ordinary requests. The changes hold in
+front of every app, and no setting undoes them:
 
 1. `PUT`, `PATCH` and `DELETE` are allowed methods besides `GET`, `HEAD`, `POST`
    and `OPTIONS`. APIs, container image pushes and package uploads use them.
@@ -625,52 +625,20 @@ every app, and no setting undoes them:
    body cannot be inspected. The other headers the Core Rule Set refuses, such
    as `Proxy`, stay refused.
 3. The `redirect_uri` parameter is not checked for a URL naming an IP address or
-   `localhost` (931100, 934110). Git Credential Manager, git-credential-oauth
-   and tea, which gitea registers for OAuth sign-in, ask to be sent back to
-   `http://127.0.0.1` on the user's own machine. The server never fetches that
-   address.
-4. Ten query parameters are not checked against the Core Rule Set's lists of
-   system files (930120), shell paths (932160) and command names (932260):
-    - `path`, `files`, `skip-to` and `sub_path`;
-    - `ref`, `sha`, `branch` and `workflow`;
-    - `artifactName` and `redirect_to`.
-
-    gitea puts into them file paths, branch and workflow names, and the page to
-    return to after signing in. It reads these values as names in a repository
-    or in its own records, or as a page of its own site, never as a command. Any
-    name on those lists can be an ordinary file or branch, such as `.gitignore`
-    or `docker-build`. In these parameters, what only these three rules would
-    catch reaches the app, such as the command `|cat /etc/passwd`. Path
-    traversal, SQL and script injection, and PHP, Java and Node.js code are
-    still refused there. Every other parameter keeps all three rules. An app
-    that uses one of these parameters as a file on the server, or passes it to a
-    shell, must check the value itself. A rule file can refuse it instead.
-
-5. Two cookies are left out, and `Referer` keeps fewer rules:
-    - The Core Rule Set reads the request without the `gitea_flash` and
-      `redirect_to` cookies. gitea puts a message naming what was just done into
-      `gitea_flash`. It puts into `redirect_to` the page to return to after
-      signing in. The Core Rule Set takes file names in them, such as
-      `package.json`, for attacks. A browser sends such a cookie with every
-      request until gitea replaces it. gitea cannot replace it while those
-      requests are refused, so one refusal would keep the browser out of the
-      whole site. gitea shows the message with any script removed and returns
-      only to its own pages. Every other cookie is read in full. An app that
-      reads a cookie of either name must check it itself.
-    - `Referer` is not checked for a Unix command given without arguments
-      (932340) or for Java starting a process (944110). `Referer` names the page
-      a request comes from, on everything that page loads and on every link
-      followed from it. The Core Rule Set takes some page addresses for attacks,
-      such as a search for `env`, or OpenJDK's `Runtime.java`, whose path holds
-      both `runtime` and `java.`. All of those requests would be refused.
-      `Referer` keeps the other rules. An app that passes `Referer` to a shell
-      must check it itself.
-6. Responses are not inspected. A raw file from a repository, such as a shell
+   `localhost` (931100, 934110). OAuth clients that run on the user's own
+   machine, such as Git Credential Manager, ask to be sent back to
+   `http://127.0.0.1` there. The server never fetches that address. The
+   parameter's name is matched without regard to case, as Coraza matches it.
+4. `Referer` is not checked for a Unix command given without arguments (932340)
+   or for Java starting a process (944110). `Referer` names the page a request
+   comes from, on everything that page loads and on every link followed from it.
+   The Core Rule Set takes some page addresses for attacks, such as a search for
+   `env`, or OpenJDK's `Runtime.java`, whose path holds both `runtime` and
+   `java.`. All of those requests would be refused. `Referer` keeps the other
+   rules. An app that passes `Referer` to a shell must check it itself.
+5. Responses are not inspected. A raw file from a repository, such as a shell
    script, looks to the response rules like source code leaking from the server.
 
-The parameter and cookie names in these changes are matched without regard to
-case, as Coraza matches them.
-
 ### Ban lengths, response and scope
 
 `SWWAF_BAN_RESPONSE` is `403` by default, which makes a ban easy to recognise

internal/servewaf/coreruleset_test.go

@@ -179,7 +179,7 @@ func TestAnomalyThreshold(t *testing.T) {
 	s.request(client, sqlInjection, http.StatusForbidden, requestlog.ActionWAFBlocked)
 }
 
-func TestDisabledRulesSwitchOffWhatGiteaWouldBeRefused(t *testing.T) {
+func TestDisabledRulesSwitchOffWhatACodeForgeWouldBeRefused(t *testing.T) {
 	t.Parallel()
 
 	for _, request := range []struct {

internal/waf/waf.go

@@ -1,6 +1,6 @@
 // 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, and on its
-// body while SWWAF_WAF_BODY_LIMIT is set, with the six changes smallwebwaf
+// body while SWWAF_WAF_BODY_LIMIT is set, with the five changes smallwebwaf
 // makes to it, as "Attack detection" under "Settings" in SPEC.md describes
 // them. It reads no response.
 //
@@ -15,7 +15,6 @@ import (
 	"io"
 	"net/http"
 	"net/netip"
-	"slices"
 	"strconv"
 	"strings"
 
@@ -69,35 +68,19 @@ SecArgumentsLimit 1000
 SecRule ARGUMENTS_LIMIT_REACHED "@eq 1" "id:900300,phase:2,pass,\
     severity:'CRITICAL',setvar:'tx.inbound_anomaly_score_pl1=+5'"
 
-# The sixth: only the rules for requests are loaded, and no response is
+# The fifth: 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. Coraza matches a parameter name here, and in the fourth,
-# without regard to case. ARGS holds the fields of a form data or multipart
-# body Coraza reads as well as the query parameters, so a field of one of
-# these names is left out too.
+# localhost. Coraza matches a parameter name without regard to case. ARGS
+# holds the fields of a form data or multipart body Coraza reads as well as
+# the query parameters, so a field of this name is left out too.
 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.
+# The fourth: Referer is not checked for a Unix command without arguments,
+# or for Java starting a process. Coraza takes one rule id per directive.
 SecRuleUpdateTargetById 932340 "!REQUEST_HEADERS:Referer"
 SecRuleUpdateTargetById 944110 "!REQUEST_HEADERS:Referer"
 `
@@ -144,12 +127,6 @@ 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
-// 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.
@@ -169,7 +146,7 @@ type CoreRuleSet struct {
 	bodyLimit int64
 }
 
-// New returns the Core Rule Set with the six changes, at params'
+// New returns the Core Rule Set with the five changes, at params'
 // paranoia level, without the rules it switches off, and reading request
 // bodies up to params' limit.
 func New(params Params) (*CoreRuleSet, error) {
@@ -209,10 +186,10 @@ type Result struct {
 }
 
 // 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, and on body, r's body as the
-// caller has it, as readBody reads it. It returns what it found, what it
-// read of body, and the error that ended the reading early, if one did.
+// method, its URL with the query, and its headers, and on body, r's body
+// as the caller has it, as readBody reads it. It returns what it found,
+// what it read of body, and the error that ended the reading early, if
+// one did.
 func (c *CoreRuleSet) Inspect(
 	r *http.Request, client netip.Addr, body io.Reader,
 ) (Result, []byte, error) {
@@ -225,13 +202,6 @@ func (c *CoreRuleSet) Inspect(
 
 	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)
 		}
 	}
@@ -318,18 +288,3 @@ func score(tx types.Transaction) int {
 
 	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

@@ -118,8 +118,8 @@ func atDefaults(t *testing.T) *waf.CoreRuleSet {
 	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
+// wantChange checks that crs lets through passes, a request one of the
+// five 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,
@@ -241,71 +241,25 @@ func TestRedirectURIMayNameALocalAddress(t *testing.T) {
 		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 TestParameterNamesAreMatchedWithoutRegardToCase(t *testing.T) {
 	t.Parallel()
 
-	crs := atDefaults(t)
-
-	wantChange(t, crs, get("/?Path=.gitignore"), get("/?q=.gitignore"), matched(930120))
-	wantChange(t, crs, get("/?REDIRECT_URI=http://127.0.0.1:52341/"),
+	wantChange(t, atDefaults(t), get("/?REDIRECT_URI=http://127.0.0.1:52341/"),
 		get("/?next=http://127.0.0.1:52341/"), matched(931100, 934110))
 }
 
-func TestCookiesGiteaFlashAndRedirectToAreNotRead(t *testing.T) {
+func TestCookiesAreRead(t *testing.T) {
 	t.Parallel()
 
-	const (
-		flash      = "success%3DFile%2Bpackage.json%2Bdeleted"
-		redirectTo = "%2Fowner%2Frepo%2Fsrc%2Fbranch%2Fmain%2Fpackage.json"
-	)
+	// A file on the list of system files, in any cookie.
+	const file = "%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))
+	for _, name := range []string{"flash", "redirect_to"} {
+		wantResult(t, crs, get("/", "Cookie: lang=en-US; "+name+"="+file),
+			matched(930120))
+	}
 }
 
 func TestRefererIsNotCheckedForACommandOrJavaStartingAProcess(t *testing.T) {
@@ -538,37 +492,32 @@ func TestContentEncodingIsRefusedOnTheKindsOfBodyTheCoreRuleSetReads(t *testing.
 	wantBody(t, atDefaults(t), post(formData, gzip), "a", waf.Result{}, "")
 }
 
-func TestParametersGiteaSendsNamesInAreLeftOutAmongFormFieldsToo(t *testing.T) {
+func TestRedirectURIIsLeftOutAmongFormFieldsToo(t *testing.T) {
 	t.Parallel()
 
+	const local = "http://127.0.0.1:52341/"
+
 	crs := readingBodies(t)
-	local := url.QueryEscape("http://127.0.0.1:52341/")
 
 	for _, tc := range []struct {
-		body string
-		want waf.Result
+		header, body string
+		want         waf.Result
 	}{
-		{"path=.gitignore", waf.Result{}},
-		{"q=.gitignore", matched(930120)},
-		{"redirect_uri=" + local, waf.Result{}},
-		{"next=" + local, matched(931100, 934110)},
+		{formData, "redirect_uri=" + url.QueryEscape(local), waf.Result{}},
+		{formData, "next=" + url.QueryEscape(local), matched(931100, 934110)},
+		{multipart, field("redirect_uri", local) + end, waf.Result{}},
+		{multipart, field("next", local) + end, matched(931100, 934110)},
 	} {
-		wantBody(t, crs, post(formData), tc.body, tc.want, tc.body)
+		wantBody(t, crs, post(tc.header), tc.body, tc.want, tc.body)
 	}
 
-	body := field("path", ".gitignore") + end
-	wantBody(t, crs, post(multipart), body, waf.Result{}, body)
-
-	body = field("q", ".gitignore") + end
-	wantBody(t, crs, post(multipart), body, matched(930120), body)
-
-	// A JSON body's field is named by its path, here json.path, and is
-	// checked.
-	body = `{"path":".gitignore"}`
-	wantBody(t, crs, post(jsonBody), body, matched(930120), body)
+	// A JSON body's field is named by its path, here json.redirect_uri, and
+	// is checked.
+	body := `{"redirect_uri":"` + local + `"}`
+	wantBody(t, crs, post(jsonBody), body, matched(931100, 934110), body)
 }
 
-func TestGiteaBodiesTheCoreRuleSetRefuses(t *testing.T) {
+func TestCommentsAndAttachmentsTheCoreRuleSetRefuses(t *testing.T) {
 	t.Parallel()
 
 	crs := readingBodies(t)