Gitea setup: Core Rule Set blocks attacks, not ordinary Gitea traffic (closes #198)

The Gitea setup runs the Core Rule Set in block mode. Rules that refused
ordinary Gitea requests are switched off only where they misfired:

- deploy/gitea/crs-exclusions.conf, loaded through the new setting
  SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE, covers Gitea's search, file-name
  and branch-name fields, Referer, and file paths;
- smallwebwaf's defaults gain a seventh change: the registry upload
  path's from parameter skips 932260.

A file that cannot be read, does not parse or names an id from 900000
to 900999 stops servewaf at start. internal/waf's tests and
script/gitea-setup send the recorded requests in
internal/waf/testdata/gitea-requests.txt and three attacks.

Model: opus-5-5

SPEC.md

@@ -586,22 +586,12 @@ sends it.
     default, so name these six ids in it to keep them off.
 
 - `SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE` names a file of exclusions, written as
-  Coraza directives. An exclusion switches one rule off for some fields or paths
+  Coraza directives, which `servewaf` loads after the Core Rule Set and the
+  changes below. An exclusion switches one rule off for some fields or paths
   only, where `SWWAF_WAF_DISABLED_RULES` switches it off everywhere.
-  `deploy/gitea/crs-exclusions.conf` holds gitea's.
+  `deploy/gitea/crs-exclusions.conf` holds gitea's. The start stops on a file
-    - `servewaf` loads the file after `smallwebwaf`'s own setup rules and before
+  that cannot be read or does not parse. It stops too on a file that names any
-      the Core Rule Set's rules. The Core Rule Set puts its own file of
+  number from 900000 to 900999, the ids of `smallwebwaf`'s own rules.
-      exclusions that run before the rules in the same place.
-    - So the file holds only `SecAction` and `SecRule` lines that exclude with
-      `ctl:ruleRemoveTargetById` or `ctl:ruleRemoveById`. These run with each
-      request, before the rules they exclude, those of phase 1 included.
-    - A rule that `SWWAF_WAF_DISABLED_RULES` switches off and the file names is
-      simply off.
-    - The start stops on a file that cannot be read or does not parse, whatever
-      `SWWAF_WAF_MODE` is.
-    - It stops too on a file that names a number from 900000 to 900999, the ids
-      of `smallwebwaf`'s own rules, alone or in a range. Comment lines are not
-      checked.
 
 - `SWWAF_WAF_BODY_LIMIT` switches body inspection on, up to that size. It takes
   at most `64K`, since `servewaf` gets no more of a body than that (see "The
@@ -690,9 +680,9 @@ every app, and no setting undoes them:
 6. 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.
 7. The `from` parameter of a container registry's upload path,
-   `/v2/<image>/blobs/uploads/`, is not checked for command names (932260). When
+   `/v2/<image>/blobs/uploads/`, is not checked for command names (932260). A
-   a push reuses a layer, `from` names the image the layer comes from. An image
+   push that reuses a layer names there the image it takes the layer from. An
-   such as `owner/python3-app` would have that push refused.
+   image such as `owner/python3-app` would have that push refused.
 
 The parameter and cookie names in these changes are matched without regard to
 case, as Coraza matches them.

deploy/gitea/crs-exclusions.conf

@@ -1,10 +1,7 @@
 # The Core Rule Set's exclusions for Gitea, which servewaf loads from
-# SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE. servewaf loads this file before the Core
+# SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE. Each switches one rule off for the
-# Rule Set's rules, so it holds only exclusions made while a request is checked:
+# Gitea fields or paths where it refused ordinary Gitea requests. Gitea
-# SecAction and SecRule lines with ctl:ruleRemoveTargetById or
+# sends these in the query string:
-# ctl:ruleRemoveById. Each switches one rule off for the Gitea fields or paths
-# where it refused ordinary Gitea requests. A SecAction applies on every path, a
-# SecRule on the paths it matches. Gitea sends these in the query string:
 #   - q: the text of every search;
 #   - file, path, template, body and redirect_to: a file name, alone or in
 #     JSON or a link;
@@ -13,138 +10,87 @@
 
 # Searches.
 # 930110: a search for ../
-SecAction "id:10001,phase:1,pass,nolog,ctl:ruleRemoveTargetById=930110;ARGS:q"
+SecRuleUpdateTargetById 930110 "!ARGS:q"
 # 931100: a search for http://127.0.0.1:8080
-SecAction "id:10002,phase:1,pass,nolog,ctl:ruleRemoveTargetById=931100;ARGS:q"
+SecRuleUpdateTargetById 931100 "!ARGS:q"
 # 932140: a search for if (a == b) {
-SecAction "id:10003,phase:1,pass,nolog,ctl:ruleRemoveTargetById=932140;ARGS:q"
+SecRuleUpdateTargetById 932140 "!ARGS:q"
 # 932235: a search for `make check`
-SecAction "id:10004,phase:1,pass,nolog,ctl:ruleRemoveTargetById=932235;ARGS:q"
+SecRuleUpdateTargetById 932235 "!ARGS:q"
 # 932250: a search for ls -la
-SecAction "id:10005,phase:1,pass,nolog,ctl:ruleRemoveTargetById=932250;ARGS:q"
+SecRuleUpdateTargetById 932250 "!ARGS:q"
 # 932380: a search for `whoami`
-SecAction "id:10006,phase:1,pass,nolog,ctl:ruleRemoveTargetById=932380;ARGS:q"
+SecRuleUpdateTargetById 932380 "!ARGS:q"
-# 933100: a search for <?php echo
-SecAction "id:10007,phase:1,pass,nolog,ctl:ruleRemoveTargetById=933100;ARGS:q"
-# 933130: a search for $_GET[
-SecAction "id:10008,phase:1,pass,nolog,ctl:ruleRemoveTargetById=933130;ARGS:q"
 # 933135: a search for ${VAR}
-SecAction "id:10009,phase:1,pass,nolog,ctl:ruleRemoveTargetById=933135;ARGS:q"
+SecRuleUpdateTargetById 933135 "!ARGS:q"
 # 934110: a search for http://127.0.0.1:8080
-SecAction "id:10010,phase:1,pass,nolog,ctl:ruleRemoveTargetById=934110;ARGS:q"
+SecRuleUpdateTargetById 934110 "!ARGS:q"
 # 941100: a search for <script>
-SecAction "id:10011,phase:1,pass,nolog,ctl:ruleRemoveTargetById=941100;ARGS:q"
+SecRuleUpdateTargetById 941100 "!ARGS:q"
 # 941110: a search for <script>, or a page opened from its results
-SecAction "id:10012,phase:1,pass,nolog,\
+SecRuleUpdateTargetById 941110 "!ARGS:q|!REQUEST_HEADERS:Referer"
-    ctl:ruleRemoveTargetById=941110;ARGS:q,\
-    ctl:ruleRemoveTargetById=941110;REQUEST_HEADERS:Referer"
-# 941120: a page opened from the results of a search for onClick={
-SecAction "id:10013,phase:1,pass,nolog,\
-    ctl:ruleRemoveTargetById=941120;REQUEST_HEADERS:Referer"
 # 941160: a search for <img src=, or a page opened from its results
-SecAction "id:10014,phase:1,pass,nolog,\
+SecRuleUpdateTargetById 941160 "!ARGS:q|!REQUEST_HEADERS:Referer"
-    ctl:ruleRemoveTargetById=941160;ARGS:q,\
-    ctl:ruleRemoveTargetById=941160;REQUEST_HEADERS:Referer"
 # 941390: a search for fetch(
-SecAction "id:10015,phase:1,pass,nolog,ctl:ruleRemoveTargetById=941390;ARGS:q"
+SecRuleUpdateTargetById 941390 "!ARGS:q"
-# 942100: a search for SELECT * FROM users WHERE, or a page opened from the
+# 942100: a search for SELECT * FROM users WHERE
-# results of a search for LIKE '%foo%'
+SecRuleUpdateTargetById 942100 "!ARGS:q"
-SecAction "id:10016,phase:1,pass,nolog,\
-    ctl:ruleRemoveTargetById=942100;ARGS:q,\
-    ctl:ruleRemoveTargetById=942100;REQUEST_HEADERS:Referer"
 # 942190: a search for union select
-SecAction "id:10017,phase:1,pass,nolog,ctl:ruleRemoveTargetById=942190;ARGS:q"
+SecRuleUpdateTargetById 942190 "!ARGS:q"
 # 942230: a search for if (a == b) {
-SecAction "id:10018,phase:1,pass,nolog,ctl:ruleRemoveTargetById=942230;ARGS:q"
+SecRuleUpdateTargetById 942230 "!ARGS:q"
 # 944100: a search for java.lang.Runtime, or a page opened from its results
-SecAction "id:10019,phase:1,pass,nolog,\
+SecRuleUpdateTargetById 944100 "!ARGS:q|!REQUEST_HEADERS:Referer"
-    ctl:ruleRemoveTargetById=944100;ARGS:q,\
-    ctl:ruleRemoveTargetById=944100;REQUEST_HEADERS:Referer"
 
 # Searches, and file, branch and tag names.
 # 930120: a search for package.json, or the commit graph of .gitignore
-SecAction "id:10020,phase:1,pass,nolog,ctl:ruleRemoveTargetById=930120;\
+SecRuleUpdateTargetById 930120 "!ARGS:q|!ARGS:file|!ARGS:template|!ARGS:body|\
-    ARGS:/^(?:q|file|template|body|refsuburl|name|tag|rule_name)$/"
+    !ARGS:refSubUrl|!ARGS:name|!ARGS:tag|!ARGS:rule_name"
 # 932160: a search for /bin/sh, or the API's contents of bin/docker-entrypoint
-SecAction "id:10021,phase:1,pass,nolog,ctl:ruleRemoveTargetById=932160;\
+SecRuleUpdateTargetById 932160 "!ARGS:q|!ARGS:file|!ARGS:template|!ARGS:body|\
-    ARGS:/^(?:q|file|template|body|refsuburl|name|tag|rule_name)$/"
+    !ARGS:refSubUrl|!ARGS:name|!ARGS:tag|!ARGS:rule_name"
 # 932260: a search for python3, or deleting the branch docker-build
-SecAction "id:10022,phase:1,pass,nolog,ctl:ruleRemoveTargetById=932260;\
+SecRuleUpdateTargetById 932260 "!ARGS:q|!ARGS:file|!ARGS:template|!ARGS:body|\
-    ARGS:/^(?:q|file|template|body|refsuburl|name|tag|rule_name)$/"
+    !ARGS:refSubUrl|!ARGS:name|!ARGS:tag|!ARGS:rule_name"
 
 # Searches, and file names in the query string.
 # 932120: a search for Invoke-WebRequest, or the commits of install-script.sh
-SecAction "id:10023,phase:1,pass,nolog,ctl:ruleRemoveTargetById=932120;\
+SecRuleUpdateTargetById 932120 "!ARGS:q|!ARGS:file|!ARGS:path|!ARGS:template|\
-    ARGS:/^(?:q|file|path|template|body|redirect_to)$/"
+    !ARGS:body|!ARGS:redirect_to"
 # 932130: a search for $(pwd), or an issue opened from the template [draft].md
-SecAction "id:10024,phase:1,pass,nolog,ctl:ruleRemoveTargetById=932130;\
+SecRuleUpdateTargetById 932130 "!ARGS:q|!ARGS:file|!ARGS:path|!ARGS:template|\
-    ARGS:/^(?:q|file|path|template|body|redirect_to)$/"
+    !ARGS:body|!ARGS:redirect_to"
 # 934100: a search for process.env, or the commits of process.env.d.ts
-SecAction "id:10025,phase:1,pass,nolog,ctl:ruleRemoveTargetById=934100;\
+SecRuleUpdateTargetById 934100 "!ARGS:q|!ARGS:file|!ARGS:path|!ARGS:template|\
-    ARGS:/^(?:q|file|path|template|body|redirect_to)$/"
+    !ARGS:body|!ARGS:redirect_to"
 # 934130: a search for __proto__, or the commits of __proto__.json
-SecAction "id:10026,phase:1,pass,nolog,ctl:ruleRemoveTargetById=934130;\
+SecRuleUpdateTargetById 934130 "!ARGS:q|!ARGS:file|!ARGS:path|!ARGS:template|\
-    ARGS:/^(?:q|file|path|template|body|redirect_to)$/"
+    !ARGS:body|!ARGS:redirect_to"
 # 941130: the commits of index.xhtml
-SecAction "id:10027,phase:1,pass,nolog,ctl:ruleRemoveTargetById=941130;\
+SecRuleUpdateTargetById 941130 "!ARGS:file|!ARGS:path|!ARGS:template|\
-    ARGS:/^(?:file|path|template|body|redirect_to)$/"
+    !ARGS:body|!ARGS:redirect_to"
 # 941180: a search for document.cookie, or the commits of document.write.js
-SecAction "id:10028,phase:1,pass,nolog,ctl:ruleRemoveTargetById=941180;\
+SecRuleUpdateTargetById 941180 "!ARGS:q|!ARGS:file|!ARGS:path|!ARGS:template|\
-    ARGS:/^(?:q|file|path|template|body|redirect_to)$/"
+    !ARGS:body|!ARGS:redirect_to"
-# 942140: a search for information_schema, or the commits of
+# 942140: the commits of information_schema.sql
-# information_schema.sql
+SecRuleUpdateTargetById 942140 "!ARGS:file|!ARGS:path|!ARGS:template|\
-SecAction "id:10029,phase:1,pass,nolog,ctl:ruleRemoveTargetById=942140;\
+    !ARGS:body|!ARGS:redirect_to"
-    ARGS:/^(?:q|file|path|template|body|redirect_to)$/"
 # 944110: a search for java.lang.Runtime, or the commits of Runtime.java
-SecAction "id:10030,phase:1,pass,nolog,ctl:ruleRemoveTargetById=944110;\
+SecRuleUpdateTargetById 944110 "!ARGS:q|!ARGS:file|!ARGS:path|!ARGS:template|\
-    ARGS:/^(?:q|file|path|template|body|redirect_to)$/"
+    !ARGS:body|!ARGS:redirect_to"
 # 944130: a search for java.lang.Runtime, the commits of java.lang.String.md,
 # or any page opened from that file's page, which names it in the Referer
-SecAction "id:10031,phase:1,pass,nolog,\
+SecRuleUpdateTargetById 944130 "!ARGS:q|!ARGS:file|!ARGS:path|!ARGS:template|\
-    ctl:ruleRemoveTargetById=944130;\
+    !ARGS:body|!ARGS:redirect_to|!REQUEST_HEADERS:Referer"
-    ARGS:/^(?:q|file|path|template|body|redirect_to)$/,\
-    ctl:ruleRemoveTargetById=944130;REQUEST_HEADERS:Referer"
 
 # File names in the path of a file's page, raw file, blame, history and edit
 # page, and of the API's calls for its contents.
-# 920500: the page of notes.txt~
-SecRule REQUEST_FILENAME "@rx ^/(?:api/v1/repos/)?[^/]+/[^/]+/(?:src|raw|media|blame|commits|_edit|_delete|contents)/" \
-    "id:10032,phase:1,pass,nolog,ctl:ruleRemoveTargetById=920500;REQUEST_FILENAME"
 # 934100: the page of process.env.d.ts
 SecRule REQUEST_FILENAME "@rx ^/(?:api/v1/repos/)?[^/]+/[^/]+/(?:src|raw|media|blame|commits|_edit|_delete|contents)/" \
-    "id:10033,phase:1,pass,nolog,ctl:ruleRemoveTargetById=934100;REQUEST_FILENAME"
+    "id:10001,phase:1,pass,nolog,ctl:ruleRemoveTargetById=934100;REQUEST_FILENAME"
 # 941130: the page of docs/a b.xhtml
 SecRule REQUEST_FILENAME "@rx ^/(?:api/v1/repos/)?[^/]+/[^/]+/(?:src|raw|media|blame|commits|_edit|_delete|contents)/" \
-    "id:10034,phase:1,pass,nolog,ctl:ruleRemoveTargetById=941130;REQUEST_FILENAME"
+    "id:10002,phase:1,pass,nolog,ctl:ruleRemoveTargetById=941130;REQUEST_FILENAME"
 # 944130: the page of java.lang.String.md
 SecRule REQUEST_FILENAME "@rx ^/(?:api/v1/repos/)?[^/]+/[^/]+/(?:src|raw|media|blame|commits|_edit|_delete|contents)/" \
-    "id:10035,phase:1,pass,nolog,ctl:ruleRemoveTargetById=944130;REQUEST_FILENAME"
+    "id:10003,phase:1,pass,nolog,ctl:ruleRemoveTargetById=944130;REQUEST_FILENAME"
-
-# The file name, in name, of an attachment uploaded through the API to a
-# release, an issue or a comment.
-# 932120: the attachment install-script.sh
-SecRule REQUEST_FILENAME "@rx ^/api/v1/repos/[^/]+/[^/]+/(?:releases|issues|issues/comments)/[0-9]+/assets$" \
-    "id:10036,phase:1,pass,nolog,ctl:ruleRemoveTargetById=932120;ARGS:name"
-# 934100: the attachment process.env.d.ts
-SecRule REQUEST_FILENAME "@rx ^/api/v1/repos/[^/]+/[^/]+/(?:releases|issues|issues/comments)/[0-9]+/assets$" \
-    "id:10037,phase:1,pass,nolog,ctl:ruleRemoveTargetById=934100;ARGS:name"
-# 934130: the attachment __proto__.json
-SecRule REQUEST_FILENAME "@rx ^/api/v1/repos/[^/]+/[^/]+/(?:releases|issues|issues/comments)/[0-9]+/assets$" \
-    "id:10038,phase:1,pass,nolog,ctl:ruleRemoveTargetById=934130;ARGS:name"
-# 941130: the attachment index.xhtml
-SecRule REQUEST_FILENAME "@rx ^/api/v1/repos/[^/]+/[^/]+/(?:releases|issues|issues/comments)/[0-9]+/assets$" \
-    "id:10039,phase:1,pass,nolog,ctl:ruleRemoveTargetById=941130;ARGS:name"
-# 941180: the attachment document.write.js
-SecRule REQUEST_FILENAME "@rx ^/api/v1/repos/[^/]+/[^/]+/(?:releases|issues|issues/comments)/[0-9]+/assets$" \
-    "id:10040,phase:1,pass,nolog,ctl:ruleRemoveTargetById=941180;ARGS:name"
-# 942140: the attachment information_schema.sql
-SecRule REQUEST_FILENAME "@rx ^/api/v1/repos/[^/]+/[^/]+/(?:releases|issues|issues/comments)/[0-9]+/assets$" \
-    "id:10041,phase:1,pass,nolog,ctl:ruleRemoveTargetById=942140;ARGS:name"
-# 944130: the attachment java.lang.String.md
-SecRule REQUEST_FILENAME "@rx ^/api/v1/repos/[^/]+/[^/]+/(?:releases|issues|issues/comments)/[0-9]+/assets$" \
-    "id:10042,phase:1,pass,nolog,ctl:ruleRemoveTargetById=944130;ARGS:name"
-
-# The header Content-Range of an artifact upload.
-# 920450: an artifact upload from actions/upload-artifact@v3
-SecRule REQUEST_FILENAME "@rx ^/api/actions_pipeline/_apis/pipelines/workflows/[0-9]+/artifacts/[^/]+/upload$" \
-    "id:10043,phase:1,pass,nolog,ctl:ruleRemoveTargetById=920450;REQUEST_HEADERS_NAMES:Content-Range"

docs/gitea.md

@@ -49,9 +49,8 @@ under a name of its own, and checks:
 - that gitea logs the visitor's address, not `rproxy`'s;
 - that a request from a container on the shared network skips the checks;
 - that the Core Rule Set refuses none of the ordinary gitea requests in
-  `internal/waf/testdata/gitea-requests.txt`, sent by a signed-in user;
+  `internal/waf/testdata/gitea-requests.txt`, sent by a signed-in user, nor an
-- that it refuses none of the API calls with JSON bodies;
+  API call with a JSON body, nor a file saved through the API;
-- that it refuses no fetch;
 - that it refuses SQL injection, path traversal and script injection in a query
   string;
 - that with `servewaf` stopped, gitea still answers through `rproxy`.
@@ -162,8 +161,8 @@ Put the runners' addresses in `SWWAF_RATE_LIMIT_EXEMPT_NETS`.
 
 `deploy/gitea/crs-exclusions.conf` switches single Core Rule Set rules off where
 they refused ordinary gitea requests. `servewaf` loads it from
-`SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE`, before the Core Rule Set's rules. Each
+`SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE`. Each exclusion covers one rule, in these
-exclusion covers one rule, in these places only:
+places only:
 
 - `q`, the text of every search;
 - the query parameters that hold a file name: `file`, `path`, `template`, `body`
@@ -172,31 +171,33 @@ exclusion covers one rule, in these places only:
   `rule_name` and `refSubUrl`;
 - `Referer`, on a page opened from search results or from a file's page;
 - the path of a file's page, raw file, blame, history and edit page, and of the
-  API's call for its contents;
+  API's call for its contents.
-- `name`, the file name of an attachment uploaded through the API;
-- the header `Content-Range` of an Actions artifact upload.
 
-gitea reads these as text to search for, as names in a repository, or, for
+gitea reads these as text to search for, or as names in a repository. It never
-`Content-Range`, as the part of a file an upload carries. It never runs them as
+runs them as SQL, a command or a page's script. So an attack that only the
-SQL, a command or a page's script. So an attack that only the excluded rules
+excluded rules would catch there reaches gitea and does nothing. Every other
-would catch there reaches gitea and does nothing. Every other field and path
+field and path keeps every rule.
-keeps every rule.
 
 To add an exclusion for a request the Core Rule Set refuses:
 
 1. Find the request in `servewaf`'s log. Its line gives the path and query, and
    names the rules in `waf_rule_ids`.
-2. For each rule, add a line with `ctl:ruleRemoveTargetById` for the field it
+2. For each rule, add a `SecRuleUpdateTargetById` line naming the field it
-   matched. For a field on every path, copy one of the file's `SecAction` lines.
+   matched, as the file's other lines do. For a path, copy one of the file's
-   For some paths only, copy one of its `SecRule REQUEST_FILENAME` lines. Give
+   `SecRule REQUEST_FILENAME` rules and give it a new id. An id from 900000 to
-   it a new id. An id from 900000 to 900999 stops the start.
+   900999 stops the start.
 3. Above it, write a one-line comment naming the request it refused.
 4. Add the request to `internal/waf/testdata/gitea-requests.txt`, and run
    `make check`.
 5. Restart `servewaf`.
 
-The Core Rule Set still refuses the page of a file in a directory named `...`,
+The Core Rule Set still refuses these gitea requests:
-which reads as path traversal.
+
+- the page of a file whose name ends in `~`, such as an editor's backup file.
+  Rule 920500 checks the path before any exclusion takes effect.
+- the page of a file in a directory named `...`, which reads as path traversal;
+- an artifact upload from `actions/upload-artifact@v3`, which sends the header
+  `Content-Range`. `actions/upload-artifact@v4` does not.
 
 With `SWWAF_WAF_BODY_LIMIT` set, it also refuses issue text, comments, wiki
 pages and files saved in the web editor that hold shell commands or code.

docs/settings.md

@@ -193,12 +193,9 @@ chooses between the two (see "The image's command" below).
   of a body than 64 KiB, and the Core Rule Set reads at most 64 KiB less one
   byte of it, whatever the value (see "The answer" in [`SPEC.md`](../SPEC.md)).
 - `SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE` (default unset): a file of Core Rule Set
-  exclusions, written as Coraza directives. It is loaded before the Core Rule
+  exclusions, written as Coraza directives, loaded after the Core Rule Set. A
-  Set's rules, so it holds only `SecAction` and `SecRule` lines that exclude
+  file that cannot be read or does not parse stops the start. So does a file
-  with `ctl:ruleRemoveTargetById` or `ctl:ruleRemoveById`. A file that cannot be
+  that names any number from 900000 to 900999. It has no `_FILE` form.
-  read or does not parse stops the start, whatever `SWWAF_WAF_MODE` is. So does
-  a file that names a number from 900000 to 900999 outside a comment, alone or
-  in a range. It has no `_FILE` form.
 - `SWWAF_ERROR_BURST_THRESHOLD` (default `30`): the most refusals of one client
   in a minute (see "Attack detection" in `SPEC.md`). A count or `off`.
 

internal/servewaf/proxy.go

@@ -213,11 +213,9 @@ func newReputation(
 // reads at most QuestionBodyMaxBytes less one byte: a question's body of
 // QuestionBodyMaxBytes may be cut short, and the Core Rule Set does not read a
 // JSON or XML body over its limit. An exclusions file that cannot be read, does
-// not parse or names an id of smallwebwaf's own rules is an error in every
+// not parse or names an id of smallwebwaf's own rules is an error.
-// mode. While the mode is off, a set file is loaded with the Core Rule Set only
-// to check it.
 func NewCoreRuleSet(cfg *config.Config) (*waf.CoreRuleSet, error) {
-	if cfg.WAFMode == config.WAFModeOff && cfg.CoreRuleSetExclusionsFile == "" {
+	if cfg.WAFMode == config.WAFModeOff {
 		return nil, nil //nolint:nilnil // off is no Core Rule Set, and no error
 	}
 
@@ -230,10 +228,6 @@ func NewCoreRuleSet(cfg *config.Config) (*waf.CoreRuleSet, error) {
 		return nil, fmt.Errorf("SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE: %w", err)
 	}
 
-	if cfg.WAFMode == config.WAFModeOff {
-		return nil, nil //nolint:nilnil // off is no Core Rule Set, and no error
-	}
-
 	return coreRuleSet, nil
 }
 

internal/smallwebwaf/smallwebwaf_test.go

@@ -19,7 +19,6 @@ import (
 	"testing"
 	"time"
 
-	"sneak.berlin/go/smallwebwaf/internal/config"
 	"sneak.berlin/go/smallwebwaf/internal/lookup/lookuptest"
 	"sneak.berlin/go/smallwebwaf/internal/servewaf"
 	"sneak.berlin/go/smallwebwaf/internal/smallwebwaf"
@@ -575,33 +574,6 @@ func TestCoreRuleSetExclusionsFileThatCannotBeReadStopsTheStart(t *testing.T) {
 	}
 }
 
-func TestCoreRuleSetExclusionsFileThatCannotBeUsedStopsTheStartWhileTheCoreRuleSetIsOff(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	broken := filepath.Join(t.TempDir(), "broken.conf")
-
-	err := os.WriteFile(broken, []byte("SecNoSuchDirective On\n"), 0o600)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	for _, path := range []string{filepath.Join(t.TempDir(), "missing.conf"), broken} {
-		out := &output{}
-
-		status := run(t.Context(), map[string]string{
-			listenAddr: localhost + ":0", stateDir: t.TempDir(), rulesDir: t.TempDir(),
-			"SWWAF_WAF_MODE": config.WAFModeOff, "SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE": path,
-		}, out)
-		if status != 1 {
-			t.Fatalf("%s: exit status %d, want 1", path, status)
-		}
-
-		out.line(t, "msg", "cannot use the Core Rule Set")
-	}
-}
-
 func TestLookupDatabaseReplacedWhileRunningTakesEffect(t *testing.T) {
 	t.Parallel()
 

internal/waf/testdata/gitea-requests.txt

@@ -1,8 +1,7 @@
-# Ordinary gitea requests that the Core Rule Set must let through with
+# Ordinary gitea requests, one path and query per line, that the Core Rule Set
-# deploy/gitea/crs-exclusions.conf. Each line is a path and query, and may end
+# must let through with deploy/gitea/crs-exclusions.conf. The tests of
-# with one header after a space. The tests of internal/waf inspect each as a GET,
+# internal/waf inspect each, and script/gitea-setup sends each to gitea through
-# and script/gitea-setup sends each to gitea through the setup as a GET. The Core
+# the setup.
-# Rule Set reads the query and the headers the same way for any method.
 
 # Searches with ordinary words and symbols.
 /owner/repo/search?q=package.json
@@ -40,13 +39,6 @@
 /owner/repo/search?q=sudo%20apt%20install
 /owner/repo/search?q=~%2F.ssh%2Fconfig
 /owner/repo/search?q=.htaccess
-/owner/repo/search?q=%24_GET%5B
-/owner/repo/search?q=%24_POST
-/owner/repo/search?q=%24_SERVER%5B%27REQUEST_URI%27%5D
-/owner/repo/search?q=%3C%3Fphp%20echo
-/owner/repo/search?q=information_schema
-/owner/repo/search?q=LIKE%20%27%25foo%25%27
-/owner/repo/search?q=onClick%3D%7B
 /explore/repos?q=SELECT%20%2A%20FROM%20users%20WHERE
 /explore/repos?q=%3Cimg%20src%3D
 /explore/code?q=SELECT%20%2A%20FROM%20users%20WHERE
@@ -150,10 +142,6 @@
 /owner/repo/raw/branch/main/%5Bdraft%5D.md
 /owner/repo/_edit/main/%5Bdraft%5D.md
 /api/v1/repos/owner/repo/contents/%5Bdraft%5D.md
-/owner/repo/src/branch/main/notes.txt~
-/owner/repo/raw/branch/main/notes.txt~
-/owner/repo/_edit/main/notes.txt~
-/api/v1/repos/owner/repo/contents/notes.txt~
 
 # File names in the query string: the commit graph of a file, the API's lists
 # of its commits and of its contents, an issue opened from a template, and the
@@ -244,43 +232,6 @@
 /owner/repo/issues/new?template=.gitea%2FISSUE_TEMPLATE%2F%5Bdraft%5D.md
 /user/login?redirect_to=%2Fowner%2Frepo%2F_edit%2Fmain%2F%5Bdraft%5D.md
 
-# File names of attachments uploaded through the API, to a release, an issue and
-# a comment.
-/api/v1/repos/owner/repo/releases/1/assets?name=package.json
-/api/v1/repos/owner/repo/releases/1/assets?name=.gitignore
-/api/v1/repos/owner/repo/releases/1/assets?name=docker-compose.yml
-/api/v1/repos/owner/repo/releases/1/assets?name=.htaccess
-/api/v1/repos/owner/repo/releases/1/assets?name=docker-entrypoint
-/api/v1/repos/owner/repo/releases/1/assets?name=python3-setup.sh
-/api/v1/repos/owner/repo/releases/1/assets?name=Runtime.java
-/api/v1/repos/owner/repo/releases/1/assets?name=org.apache.commons.logging.LogFactory
-/api/v1/repos/owner/repo/releases/1/assets?name=%5Bdraft%5D.md
-/api/v1/repos/owner/repo/releases/1/assets?name=notes.txt~
-/api/v1/repos/owner/repo/releases/1/assets?name=install-script.sh
-/api/v1/repos/owner/repo/issues/1/assets?name=install-script.sh
-/api/v1/repos/owner/repo/issues/comments/1/assets?name=install-script.sh
-/api/v1/repos/owner/repo/releases/1/assets?name=process.env.d.ts
-/api/v1/repos/owner/repo/issues/1/assets?name=process.env.d.ts
-/api/v1/repos/owner/repo/issues/comments/1/assets?name=process.env.d.ts
-/api/v1/repos/owner/repo/releases/1/assets?name=java.lang.String.md
-/api/v1/repos/owner/repo/issues/1/assets?name=java.lang.String.md
-/api/v1/repos/owner/repo/issues/comments/1/assets?name=java.lang.String.md
-/api/v1/repos/owner/repo/releases/1/assets?name=information_schema.sql
-/api/v1/repos/owner/repo/issues/1/assets?name=information_schema.sql
-/api/v1/repos/owner/repo/issues/comments/1/assets?name=information_schema.sql
-/api/v1/repos/owner/repo/releases/1/assets?name=document.write.js
-/api/v1/repos/owner/repo/issues/1/assets?name=document.write.js
-/api/v1/repos/owner/repo/issues/comments/1/assets?name=document.write.js
-/api/v1/repos/owner/repo/releases/1/assets?name=__proto__.json
-/api/v1/repos/owner/repo/issues/1/assets?name=__proto__.json
-/api/v1/repos/owner/repo/issues/comments/1/assets?name=__proto__.json
-/api/v1/repos/owner/repo/releases/1/assets?name=index.xhtml
-/api/v1/repos/owner/repo/issues/1/assets?name=index.xhtml
-/api/v1/repos/owner/repo/issues/comments/1/assets?name=index.xhtml
-
-# An artifact upload from actions/upload-artifact@v3, which sends Content-Range.
-/api/actions_pipeline/_apis/pipelines/workflows/1/artifacts/9b7c1f3a/upload?itemPath=artifact%2Freport.txt Content-Range: bytes 0-9/10
-
 # Branch and tag names in the query string: deleting a branch, starting a
 # release from a tag, a branch protection rule, a directory listing's last
 # commits, and a container image push that reuses a layer.

internal/waf/waf.go

@@ -30,13 +30,12 @@ import (
 )
 
 // directives are the Core Rule Set as smallwebwaf runs it, with the
-// paranoia level for %d, bodyDirectives for the first %s while
+// paranoia level for %d, and bodyDirectives for %s while
-// SWWAF_WAF_BODY_LIMIT is set, and the exclusions file for the second.
+// SWWAF_WAF_BODY_LIMIT is set. Each rule smallwebwaf adds has an id from
-// Each rule smallwebwaf adds has an id from 900000 to 900999, the ids the
+// 900000 to 900999, the ids the Core Rule Set keeps for the rules that set
-// Core Rule Set keeps for the rules that set it up, which
+// it up, which SWWAF_WAF_DISABLED_RULES and the exclusions file refuse, so
-// SWWAF_WAF_DISABLED_RULES and the exclusions file refuse, so that no
+// that no setting switches one off. Coraza joins a line ending in \ to the
-// setting switches one off. Coraza joins a line ending in \ to the next,
+// next, without the spaces at the start of 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
@@ -74,11 +73,6 @@ SecArgumentsLimit 1000
 SecRule ARGUMENTS_LIMIT_REACHED "@eq 1" "id:900300,phase:2,pass,\
     severity:'CRITICAL',setvar:'tx.inbound_anomaly_score_pl1=+5'"
 
-# The exclusions file comes before the rules, where the Core Rule Set puts
-# its own file of exclusions that run before the rules, so that they run
-# before the rules of phase 1 too.
-%s
-
 # The sixth: only the rules for requests are loaded, and no response is
 # inspected.
 Include @owasp_crs/REQUEST-*.conf
@@ -177,53 +171,20 @@ type Params struct {
 	// BodyLimit is the most of a request body the Core Rule Set reads
 	// (SWWAF_WAF_BODY_LIMIT), 0 while it is off and it reads none.
 	BodyLimit int64
-	// ExclusionsFile is the path of a file of exclusions loaded before the
+	// ExclusionsFile is the path of a file of exclusions loaded after the
-	// Core Rule Set's rules (SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE), "" while
+	// rest (SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE), "" while there is none.
-	// there is none.
 	ExclusionsFile string
 }
 
-// The ids of smallwebwaf's own rules, which an exclusions file may not
+// smallwebwafRuleID matches an id from 900000 to 900999, which an
-// name.
+// exclusions file may not name: those are the ids of smallwebwaf's own
-const (
+// rules.
-	firstOwnRuleID = 900000
+var smallwebwafRuleID = regexp.MustCompile(`\b900\d{3}\b`)
-	lastOwnRuleID  = 900999
-)
-
-// numberOrRange matches a number, or a range of numbers such as
-// 900000-900999.
-var numberOrRange = regexp.MustCompile(`(\d+)(?:-(\d+))?`)
 
 // errSmallwebwafRuleID is the error of an exclusions file that names an
 // id of smallwebwaf's own rules.
 var errSmallwebwafRuleID = errors.New("an id of smallwebwaf's own rules")
 
-// ownRuleNamed returns the first number or range in exclusions, the text
-// of an exclusions file, that takes in an id of smallwebwaf's own rules,
-// or "" if none does. Comment lines are skipped, as Coraza skips them.
-func ownRuleNamed(exclusions string) string {
-	for line := range strings.Lines(exclusions) {
-		if strings.HasPrefix(strings.TrimSpace(line), "#") {
-			continue
-		}
-
-		for _, match := range numberOrRange.FindAllStringSubmatch(line, -1) {
-			first, _ := strconv.Atoi(match[1])
-
-			last := first
-			if match[2] != "" {
-				last, _ = strconv.Atoi(match[2])
-			}
-
-			if first <= lastOwnRuleID && last >= firstOwnRuleID {
-				return match[0]
-			}
-		}
-	}
-
-	return ""
-}
-
 // CoreRuleSet is the Core Rule Set, ready to inspect requests. It is safe
 // for concurrent use.
 type CoreRuleSet struct {
@@ -234,32 +195,15 @@ type CoreRuleSet struct {
 // New returns the Core Rule Set with the seven changes, at params'
 // paranoia level, without the rules it switches off, reading request
 // bodies up to params' limit, and with params' exclusions file loaded
-// before the Core Rule Set's rules. An exclusions file that cannot be
+// last. An exclusions file that cannot be read, does not parse or names
-// read, does not parse or names an id of smallwebwaf's own rules is an
+// an id of smallwebwaf's own rules is an error.
-// error.
 func New(params Params) (*CoreRuleSet, error) {
 	body := ""
 	if params.BodyLimit > 0 {
 		body = fmt.Sprintf(bodyDirectives, params.BodyLimit)
 	}
 
-	exclusions := ""
+	text := fmt.Sprintf(directives, params.ParanoiaLevel, body)
-
-	if params.ExclusionsFile != "" {
-		file, err := os.ReadFile(params.ExclusionsFile)
-		if err != nil {
-			return nil, fmt.Errorf("read the exclusions file: %w", err)
-		}
-
-		exclusions = string(file)
-
-		if named := ownRuleNamed(exclusions); named != "" {
-			return nil, fmt.Errorf("the exclusions file names %s: %w", named,
-				errSmallwebwafRuleID)
-		}
-	}
-
-	text := fmt.Sprintf(directives, params.ParanoiaLevel, body, exclusions)
 
 	if len(params.DisabledRules) > 0 {
 		ids := make([]string, len(params.DisabledRules))
@@ -270,9 +214,23 @@ func New(params Params) (*CoreRuleSet, error) {
 		text += "SecRuleRemoveById " + strings.Join(ids, " ") + "\n"
 	}
 
-	waf, err := coraza.NewWAF(coraza.NewWAFConfig().
+	config := coraza.NewWAFConfig().WithRootFS(coreruleset.FS).WithDirectives(text)
-		WithRootFS(coreruleset.FS).
+
-		WithDirectives(text))
+	if params.ExclusionsFile != "" {
+		exclusions, err := os.ReadFile(params.ExclusionsFile)
+		if err != nil {
+			return nil, fmt.Errorf("read the exclusions file: %w", err)
+		}
+
+		if id := smallwebwafRuleID.Find(exclusions); id != nil {
+			return nil, fmt.Errorf("the exclusions file names %s: %w", id,
+				errSmallwebwafRuleID)
+		}
+
+		config = config.WithDirectives(string(exclusions))
+	}
+
+	waf, err := coraza.NewWAF(config)
 	if err != nil {
 		return nil, fmt.Errorf("load the Core Rule Set: %w", err)
 	}

internal/waf/waf_test.go

@@ -400,44 +400,17 @@ func exclusionsFile(t *testing.T, text string) string {
 	return path
 }
 
-// backupFileExclusion is an exclusions file that switches 920500, a rule
+func TestExclusionsFileIsLoadedAfterTheCoreRuleSet(t *testing.T) {
-// of phase 1, off for the path of a backup file under /files/.
-const backupFileExclusion = `SecRule REQUEST_FILENAME "@beginsWith /files/" ` +
-	`"id:1,phase:1,pass,nolog,ctl:ruleRemoveTargetById=920500;REQUEST_FILENAME"` + "\n"
-
-func TestExclusionsFileIsLoadedBeforeTheRules(t *testing.T) {
 	t.Parallel()
 
-	crs, err := withExclusions(exclusionsFile(t, backupFileExclusion))
+	crs, err := withExclusions(exclusionsFile(t,
+		"SecRuleUpdateTargetById 942100 \"!ARGS:id\"\n"))
 	if err != nil {
 		t.Fatalf("load the exclusions: %v", err)
 	}
 
-	wantChange(t, crs, get("/files/notes.txt~"), get("/notes.txt~"), matched(920500))
+	wantChange(t, crs, get("/?id=1'%20OR%20'1'='1"), get("/?q=1'%20OR%20'1'='1"),
-}
+		matched(942100))
-
-func TestRuleSwitchedOffAndNamedInTheExclusionsFileIsOff(t *testing.T) {
-	t.Parallel()
-
-	crs, err := waf.New(waf.Params{
-		ParanoiaLevel: 1, DisabledRules: []int{920500},
-		ExclusionsFile: exclusionsFile(t, backupFileExclusion),
-	})
-	if err != nil {
-		t.Fatalf("load the exclusions: %v", err)
-	}
-
-	wantResult(t, crs, get("/notes.txt~"), waf.Result{})
-}
-
-func TestExclusionsFileMayNameAnyNumberInAComment(t *testing.T) {
-	t.Parallel()
-
-	_, err := withExclusions(exclusionsFile(t,
-		"# 942100: GET /issues/900123\n"+backupFileExclusion))
-	if err != nil {
-		t.Errorf("the exclusions did not load: %v", err)
-	}
 }
 
 func TestExclusionsFileThatCannotBeUsedIsAnError(t *testing.T) {
@@ -446,16 +419,9 @@ func TestExclusionsFileThatCannotBeUsedIsAnError(t *testing.T) {
 	for _, path := range []string{
 		filepath.Join(t.TempDir(), "missing.conf"),
 		exclusionsFile(t, "SecNoSuchDirective On\n"),
-		// The Core Rule Set's rules are not loaded yet.
+		// smallwebwaf's own rules may not be switched off or changed.
-		exclusionsFile(t, "SecRuleUpdateTargetById 942100 \"!ARGS:id\"\n"),
-		// smallwebwaf's own rules may not be switched off or changed, by
-		// id or in a range.
 		exclusionsFile(t, "SecRuleRemoveById 900300\n"),
-		exclusionsFile(t, "SecRuleRemoveById 899999-901000\n"),
+		exclusionsFile(t, "SecRuleUpdateTargetById 900500 \"!ARGS:x\"\n"),
-		exclusionsFile(t, "SecAction \"id:1,phase:1,pass,nolog,"+
-			"ctl:ruleRemoveById=899999-901000\"\n"),
-		exclusionsFile(t, "SecAction \"id:1,phase:1,pass,nolog,"+
-			"ctl:ruleRemoveTargetById=900500;ARGS:from\"\n"),
 	} {
 		_, err := withExclusions(path)
 		if err == nil {
@@ -464,10 +430,9 @@ func TestExclusionsFileThatCannotBeUsedIsAnError(t *testing.T) {
 	}
 }
 
-// giteaRequests returns the recorded ordinary gitea requests in testdata,
+// giteaRequests returns the targets of the recorded ordinary gitea requests
-// which script/gitea-setup sends too. Each has the Referer of a page that
+// in testdata, which script/gitea-setup sends too.
-// names the same, as a page opened from it has.
+func giteaRequests(t *testing.T) []string {
-func giteaRequests(t *testing.T) []request {
 	t.Helper()
 
 	text, err := os.ReadFile(filepath.Join("testdata", "gitea-requests.txt"))
@@ -475,25 +440,16 @@ func giteaRequests(t *testing.T) []request {
 		t.Fatal(err)
 	}
 
-	var requests []request
+	var targets []string
 
 	for line := range strings.Lines(string(text)) {
 		line = strings.TrimSpace(line)
-		if line == "" || strings.HasPrefix(line, "#") {
+		if line != "" && !strings.HasPrefix(line, "#") {
-			continue
+			targets = append(targets, line)
 		}
-
-		target, header, _ := strings.Cut(line, " ")
-
-		headers := []string{"Referer: http://git.example" + target}
-		if header != "" {
-			headers = append(headers, header)
-		}
-
-		requests = append(requests, get(target, headers...))
 	}
 
-	return requests
+	return targets
 }
 
 func TestGiteaExclusionsLetOrdinaryGiteaRequestsThroughAndRefuseAttacks(t *testing.T) {
@@ -507,9 +463,12 @@ func TestGiteaExclusionsLetOrdinaryGiteaRequestsThroughAndRefuseAttacks(t *testi
 
 	const threshold = 5 // SWWAF_WAF_ANOMALY_THRESHOLD's default
 
-	for _, r := range giteaRequests(t) {
+	// Each with the Referer of a page that names the same, as a page
+	// opened from it has.
+	for _, target := range giteaRequests(t) {
+		r := get(target, "Referer: http://git.example"+target)
 		if got := inspect(t, crs, r); got.Score >= threshold {
-			t.Errorf("%s %q refused: %+v", r.target, r.headers[1:], got)
+			t.Errorf("%s refused: %+v", target, got)
 		}
 	}
 

script/gitea-setup

@@ -6,9 +6,8 @@
 #   - that gitea logs the visitor's address, not rproxy's;
 #   - that a request from a container on the shared network skips the checks;
 #   - that the Core Rule Set refuses none of the ordinary gitea requests in
-#     internal/waf/testdata/gitea-requests.txt;
+#     internal/waf/testdata/gitea-requests.txt, nor a fetch, nor API calls with
-#   - that it refuses none of the API calls with JSON bodies;
+#     JSON bodies;
-#   - that it refuses no fetch;
 #   - that it refuses SQL injection, path traversal and script injection in a
 #     query string;
 #   - that with servewaf stopped, gitea still answers through rproxy.
@@ -150,11 +149,10 @@ refused() {
 
 # ordinary_requests: sign in as the owner, save a file and open an issue
 # through the API with JSON bodies, then send, signed in, each request in
-# internal/waf/testdata/gitea-requests.txt, with the header its line ends with,
+# internal/waf/testdata/gitea-requests.txt, with that request's own address as
-# if any, and that request's own address as its Referer, as a page opened from it
+# its Referer, as a page opened from it has. One a second keeps them under the
-# has. One a second keeps them under the default limit of 120 requests a minute,
+# default limit of 120 requests a minute, since a request refused for a limit is
-# since a request refused for a limit is never inspected. Fail if servewaf
+# never inspected. Fail if servewaf refuses any of them.
-# refuses any of them.
 ordinary_requests() {
     send /user/login --cookie-jar "$WORK/cookies" >/dev/null
     status="$(send /user/login --cookie "$WORK/cookies" --cookie-jar "$WORK/cookies" \
@@ -173,14 +171,9 @@ ordinary_requests() {
     [ "$status" = 201 ] || fail "opening an issue through the API was answered $status"
 
     grep -v -e '^#' -e '^$' "$ROOT/internal/waf/testdata/gitea-requests.txt" |
-        while read -r target header; do
+        while IFS= read -r target; do
-            if [ -n "$header" ]; then
+            send "$target" --globoff --cookie "$WORK/cookies" \
-                send "$target" --globoff --cookie "$WORK/cookies" \
+                --header "Referer: https://$SITE$target" >/dev/null
-                    --header "Referer: https://$SITE$target" --header "$header" >/dev/null
-            else
-                send "$target" --globoff --cookie "$WORK/cookies" \
-                    --header "Referer: https://$SITE$target" >/dev/null
-            fi
             sleep 1
         done