diff --git a/Dockerfile b/Dockerfile index b4d8690..9b72b0d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -43,6 +43,12 @@ RUN --mount=type=tmpfs,target=/root/.cache/go-build \ { echo "--- Rerunning with -v for details ---"; \ go test -tags no_fs_access -timeout 90s -race -v ./...; exit 1; } +# Second pass: the recorded requests, too slow for make test under -race. +RUN --mount=type=tmpfs,target=/root/.cache/go-build \ + go test -tags no_fs_access,recorded -timeout 90s -cover ./internal/coreruleset/... || \ + { echo "--- Rerunning with -v for details ---"; \ + go test -tags no_fs_access,recorded -timeout 90s -v ./internal/coreruleset/...; exit 1; } + # Tidy stage: `go mod tidy` in the test phase's Go, so that the files it # writes pass the test phase's check. Nothing else depends on it, so only # script/tidy, which names the stage after it, builds it. diff --git a/README.md b/README.md index de8e2ed..17d5876 100644 --- a/README.md +++ b/README.md @@ -695,7 +695,8 @@ on the host. - `script/projectname`: prints `smallwebwaf`, which the other scripts tag their images with. - `script/test`: checks that `go.mod` and `go.sum` are tidy, then runs the - tests, as the `test` phase of the `Dockerfile`. `make test`. + tests, as the `test` phase of the `Dockerfile`. It runs them twice: once with + the race detector, and once without it for the recorded requests. `make test`. - `script/tidy`: writes `go.mod` and `go.sum` as `go mod tidy` writes them, in the `tidy` stage of the `Dockerfile`. `make tidy`. - `script/lint`: runs golangci-lint, as the `lint` phase of the `Dockerfile`. diff --git a/internal/coreruleset/coreruleset_test.go b/internal/coreruleset/coreruleset_test.go index 6422e12..f960d06 100644 --- a/internal/coreruleset/coreruleset_test.go +++ b/internal/coreruleset/coreruleset_test.go @@ -1,7 +1,6 @@ package coreruleset_test import ( - "encoding/json" "fmt" "net/http" "net/http/httptest" @@ -573,38 +572,6 @@ func TestExclusionsFileThatCannotBeUsedIsAnError(t *testing.T) { } } -// giteaRequests returns the recorded ordinary gitea requests in testdata. -// Each has the Referer of a page that names the same, as a page opened -// from it has. -func giteaRequests(t *testing.T) []request { - t.Helper() - - text, err := os.ReadFile(filepath.Join("testdata", "gitea-requests.txt")) - if err != nil { - t.Fatal(err) - } - - var requests []request - - for line := range strings.Lines(string(text)) { - line = strings.TrimSpace(line) - if line == "" || strings.HasPrefix(line, "#") { - continue - } - - 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 -} - // An SQL injection, a script injection and a shell command after a ";", as // query values. Every app's tests check that its fields still refuse the // first two. @@ -624,61 +591,13 @@ const passwordField = "password=" //nolint:gochecknoglobals // a constant cannot be a list var hardValues = []string{"python3", "powershell", "ls%20-la", ".htaccess"} -// wantRecordedRequestsPass checks that none of the recorded ordinary -// requests in the file name in testdata is refused, at the defaults and -// while bodies are read. A request with neither a body nor a Content-Type -// header is checked only at the defaults: with neither, the set that reads -// bodies has nothing more to check. Each request runs as its own parallel -// subtest. -func wantRecordedRequestsPass(t *testing.T, name string) { - t.Helper() - - const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default - - defaults, bodies := atDefaults(t), readingBodies(t) - - for _, recorded := range recordedRequests(t, name) { - t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) { - t.Parallel() - - r := request{recorded.Method, recorded.Target, recorded.Headers} - - got, _ := inspectBody(t, defaults, r, recorded.Body) - if got.Score >= threshold { - t.Errorf("%s %s refused: %+v", r.method, r.target, got) - } - - typed := slices.ContainsFunc(recorded.Headers, func(header string) bool { - name, _, _ := strings.Cut(header, ":") - - return strings.EqualFold(name, "Content-Type") - }) - if recorded.Body == "" && !typed { - return - } - - got, _ = inspectBody(t, bodies, r, recorded.Body) - if got.Score >= threshold { - t.Errorf("%s %s %.200q refused while bodies are read: %+v", - r.method, r.target, recorded.Body, got) - } - }) - } -} - -func TestOrdinaryGiteaRequestsPassAndAttacksAreRefused(t *testing.T) { +func TestGiteaAttacksAreRefused(t *testing.T) { t.Parallel() const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default crs := atDefaults(t) - for _, r := range giteaRequests(t) { - if got := inspect(t, crs, r); got.Score >= threshold { - t.Errorf("%s %q refused: %+v", r.target, r.headers[1:], got) - } - } - // In q too, off gitea's search pages. for _, parameter := range []string{"id", "q"} { for _, attack := range []string{ @@ -731,46 +650,6 @@ const ( wpTags = "/wp-json/wp/v2/tags" ) -// recordedRequest is a line of a file of recorded requests in testdata. -type recordedRequest struct { - Method string `json:"method"` - Target string `json:"target"` - Headers []string `json:"headers"` - Body string `json:"body"` -} - -// recordedRequests returns the recorded ordinary requests in the file name -// in testdata, each with its body. -func recordedRequests(t *testing.T, name string) []recordedRequest { - t.Helper() - - path := filepath.Join("testdata", name) - - text, err := os.ReadFile(path) //nolint:gosec // a file in testdata - if err != nil { - t.Fatal(err) - } - - var requests []recordedRequest - - for line := range strings.Lines(string(text)) { - if strings.HasPrefix(line, "#") { - continue - } - - var r recordedRequest - - err := json.Unmarshal([]byte(line), &r) - if err != nil { - t.Fatalf("%q: %v", line, err) - } - - requests = append(requests, r) - } - - return requests -} - // readingBodiesUpTo64K returns the Core Rule Set as sww runs it by default, // but reading bodies up to the largest SWW_WAF_BODY_LIMIT. func readingBodiesUpTo64K(t *testing.T) *coreruleset.CoreRuleSet { @@ -836,33 +715,13 @@ func wordpressAttackTargets(attack string) []string { } } -func TestOrdinaryWordPressRequestsPassAndAttacksAreRefused(t *testing.T) { +func TestWordPressAttacksAreRefused(t *testing.T) { t.Parallel() const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default defaults, bodies := atDefaults(t), readingBodiesUpTo64K(t) - for _, recorded := range recordedRequests(t, "wordpress-requests.txt") { - t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) { - t.Parallel() - - r := request{recorded.Method, recorded.Target, recorded.Headers} - - crsSets := []*coreruleset.CoreRuleSet{defaults} - if recorded.Body != "" { - crsSets = append(crsSets, bodies) - } - - for _, crs := range crsSets { - got, _ := inspectBody(t, crs, r, recorded.Body) - if got.Score >= threshold { - t.Errorf("%s %s %.200q refused: %+v", r.method, r.target, recorded.Body, got) - } - } - }) - } - for _, attack := range []string{ sqlInjection, "..%2F..%2Fwp-config.php", diff --git a/internal/coreruleset/drupal_test.go b/internal/coreruleset/drupal_test.go index 35aaa7e..cf9befb 100644 --- a/internal/coreruleset/drupal_test.go +++ b/internal/coreruleset/drupal_test.go @@ -31,12 +31,6 @@ const drupalEditAccount = "/user/2/edit" // drupalName is the field for a person's name, as the start of a form body. const drupalName = "name=" -func TestOrdinaryDrupalRequestsPass(t *testing.T) { - t.Parallel() - - wantRecordedRequestsPass(t, "drupal-requests.txt") -} - // drupalAttacks are an SQL injection, a path traversal and a script // injection, as a query value. // diff --git a/internal/coreruleset/mastodon_test.go b/internal/coreruleset/mastodon_test.go index 4cf28b9..3f78b42 100644 --- a/internal/coreruleset/mastodon_test.go +++ b/internal/coreruleset/mastodon_test.go @@ -54,12 +54,6 @@ const ( mastodonStatus = "status=" ) -func TestOrdinaryMastodonRequestsPass(t *testing.T) { - t.Parallel() - - wantRecordedRequestsPass(t, "mastodon-requests.txt") -} - // mastodonQueryFields sends each field that skips 930120, 932120, 932250 // and 932260 in the query, on a path where it skips them, ending in "=". // diff --git a/internal/coreruleset/mediawiki_test.go b/internal/coreruleset/mediawiki_test.go index 66a8137..390e6ef 100644 --- a/internal/coreruleset/mediawiki_test.go +++ b/internal/coreruleset/mediawiki_test.go @@ -42,12 +42,6 @@ const ( mediawikiUploadPage = "/index.php/Special:Upload" ) -func TestOrdinaryMediaWikiRequestsPass(t *testing.T) { - t.Parallel() - - wantRecordedRequestsPass(t, "mediawiki-requests.txt") -} - // mediawikiQueryFields and mediawikiBodyFields together hold every field that // skips 930120, 932120, 932250 and 932260, other than wikitext, wpTitles and // the JSON fields of /rest.php/. mediawikiQueryFields sends each in the query, on a diff --git a/internal/coreruleset/nextcloud_test.go b/internal/coreruleset/nextcloud_test.go index c54ab95..c91c6c2 100644 --- a/internal/coreruleset/nextcloud_test.go +++ b/internal/coreruleset/nextcloud_test.go @@ -48,12 +48,6 @@ const ( // nextcloudOther is a JSON field no exclusion names. const nextcloudOther = `{"other":"python3"}` -func TestOrdinaryNextcloudRequestsPass(t *testing.T) { - t.Parallel() - - wantRecordedRequestsPass(t, "nextcloud-requests.txt") -} - // nextcloudQueryFields are the query fields that skip 930120, 932120, 932250 // and 932260, each on a path where it skips them, ending in "=". // diff --git a/internal/coreruleset/recorded_test.go b/internal/coreruleset/recorded_test.go new file mode 100644 index 0000000..0b27942 --- /dev/null +++ b/internal/coreruleset/recorded_test.go @@ -0,0 +1,204 @@ +//go:build recorded + +// The tests that check the apps' recorded requests in testdata. The recorded +// tag keeps them out of the test phase's pass with the race detector, and +// its second pass, without the race detector, runs them. + +package coreruleset_test + +import ( + "encoding/json" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "sneak.berlin/go/smallwebwaf/internal/coreruleset" +) + +// giteaRequests returns the recorded ordinary gitea requests in testdata. +// Each has the Referer of a page that names the same, as a page opened +// from it has. +func giteaRequests(t *testing.T) []request { + t.Helper() + + text, err := os.ReadFile(filepath.Join("testdata", "gitea-requests.txt")) + if err != nil { + t.Fatal(err) + } + + var requests []request + + for line := range strings.Lines(string(text)) { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + + 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 +} + +func TestOrdinaryGiteaRequestsPass(t *testing.T) { + t.Parallel() + + const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default + + crs := atDefaults(t) + + for _, r := range giteaRequests(t) { + if got := inspect(t, crs, r); got.Score >= threshold { + t.Errorf("%s %q refused: %+v", r.target, r.headers[1:], got) + } + } +} + +// recordedRequest is a line of a file of recorded requests in testdata. +type recordedRequest struct { + Method string `json:"method"` + Target string `json:"target"` + Headers []string `json:"headers"` + Body string `json:"body"` +} + +// recordedRequests returns the recorded ordinary requests in the file name +// in testdata, each with its body. +func recordedRequests(t *testing.T, name string) []recordedRequest { + t.Helper() + + path := filepath.Join("testdata", name) + + text, err := os.ReadFile(path) //nolint:gosec // a file in testdata + if err != nil { + t.Fatal(err) + } + + var requests []recordedRequest + + for line := range strings.Lines(string(text)) { + if strings.HasPrefix(line, "#") { + continue + } + + var r recordedRequest + + err := json.Unmarshal([]byte(line), &r) + if err != nil { + t.Fatalf("%q: %v", line, err) + } + + requests = append(requests, r) + } + + return requests +} + +// wantRecordedRequestsPass checks that none of the recorded ordinary +// requests in the file name in testdata is refused, at the defaults and +// while bodies are read. A request with neither a body nor a Content-Type +// header is checked only at the defaults: with neither, the set that reads +// bodies has nothing more to check. Each request runs as its own parallel +// subtest. +func wantRecordedRequestsPass(t *testing.T, name string) { + t.Helper() + + const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default + + defaults, bodies := atDefaults(t), readingBodies(t) + + for _, recorded := range recordedRequests(t, name) { + t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) { + t.Parallel() + + r := request{recorded.Method, recorded.Target, recorded.Headers} + + got, _ := inspectBody(t, defaults, r, recorded.Body) + if got.Score >= threshold { + t.Errorf("%s %s refused: %+v", r.method, r.target, got) + } + + typed := slices.ContainsFunc(recorded.Headers, func(header string) bool { + name, _, _ := strings.Cut(header, ":") + + return strings.EqualFold(name, "Content-Type") + }) + if recorded.Body == "" && !typed { + return + } + + got, _ = inspectBody(t, bodies, r, recorded.Body) + if got.Score >= threshold { + t.Errorf("%s %s %.200q refused while bodies are read: %+v", + r.method, r.target, recorded.Body, got) + } + }) + } +} + +func TestOrdinaryDrupalRequestsPass(t *testing.T) { + t.Parallel() + + wantRecordedRequestsPass(t, "drupal-requests.txt") +} + +func TestOrdinaryMastodonRequestsPass(t *testing.T) { + t.Parallel() + + wantRecordedRequestsPass(t, "mastodon-requests.txt") +} + +func TestOrdinaryMediaWikiRequestsPass(t *testing.T) { + t.Parallel() + + wantRecordedRequestsPass(t, "mediawiki-requests.txt") +} + +func TestOrdinaryNextcloudRequestsPass(t *testing.T) { + t.Parallel() + + wantRecordedRequestsPass(t, "nextcloud-requests.txt") +} + +func TestOrdinaryVaultwardenRequestsPass(t *testing.T) { + t.Parallel() + + wantRecordedRequestsPass(t, "vaultwarden-requests.txt") +} + +func TestOrdinaryWordPressRequestsPass(t *testing.T) { + t.Parallel() + + const threshold = 5 // SWW_WAF_ANOMALY_THRESHOLD's default + + defaults, bodies := atDefaults(t), readingBodiesUpTo64K(t) + + for _, recorded := range recordedRequests(t, "wordpress-requests.txt") { + t.Run(recorded.Method+" "+recorded.Target, func(t *testing.T) { + t.Parallel() + + r := request{recorded.Method, recorded.Target, recorded.Headers} + + crsSets := []*coreruleset.CoreRuleSet{defaults} + if recorded.Body != "" { + crsSets = append(crsSets, bodies) + } + + for _, crs := range crsSets { + got, _ := inspectBody(t, crs, r, recorded.Body) + if got.Score >= threshold { + t.Errorf("%s %s %.200q refused: %+v", r.method, r.target, recorded.Body, got) + } + } + }) + } +} diff --git a/internal/coreruleset/vaultwarden_test.go b/internal/coreruleset/vaultwarden_test.go index c7c8590..0d9f58f 100644 --- a/internal/coreruleset/vaultwarden_test.go +++ b/internal/coreruleset/vaultwarden_test.go @@ -27,12 +27,6 @@ const ( // vaultwardenEmail is a JSON body that sends an address in email. const vaultwardenEmail = `{"email":"VALUE"}` -func TestOrdinaryVaultwardenRequestsPass(t *testing.T) { - t.Parallel() - - wantRecordedRequestsPass(t, "vaultwarden-requests.txt") -} - // vaultwardenField is a field Vaultwarden receives in plain text: the // request that sends it, and its body, with VALUE where the field's value // goes.