Remove everything specific to a Gitea deployment (closes #209)
check / check (push) Successful in 3m20s
check / check (push) Successful in 3m20s
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
This commit is contained in:
1 parent
f8676debda
commit
1c70104677
22 files changed
+142
-1419
No files matched your search
+12
-57
@@ -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, ";")
|
||||
}
|
||||
+25
-76
@@ -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)
|
||||
|
||||
Reference in new issue
Block a user