From 63bd31b471e765dd85a2d7760483782111e0fb68 Mon Sep 17 00:00:00 2001 From: "Thibault \"bui\" Koechlin" Date: Fri, 29 Mar 2024 17:57:54 +0100 Subject: [PATCH] Fix REQUEST_URI behavior + fix #2891 (#2917) * fix our behavior to comply more with modsec, REQUEST_URI should be: path+query string * fix #2891 as well * add new transforms * add transform tests --- .../modules/appsec/appsec_others_test.go | 74 +++++++ pkg/acquisition/modules/appsec/appsec_test.go | 200 ++++++++++++++++++ .../modules/appsec/appsec_win_test.go | 46 ++++ pkg/appsec/appsec_rule/modsecurity.go | 13 +- pkg/appsec/request.go | 6 +- 5 files changed, 333 insertions(+), 6 deletions(-) create mode 100644 pkg/acquisition/modules/appsec/appsec_others_test.go create mode 100644 pkg/acquisition/modules/appsec/appsec_win_test.go diff --git a/pkg/acquisition/modules/appsec/appsec_others_test.go b/pkg/acquisition/modules/appsec/appsec_others_test.go new file mode 100644 index 000000000..93edc9d9e --- /dev/null +++ b/pkg/acquisition/modules/appsec/appsec_others_test.go @@ -0,0 +1,74 @@ +//go:build !windows +// +build !windows + +package appsecacquisition + +import ( + "testing" + + "github.com/crowdsecurity/crowdsec/pkg/appsec" + "github.com/crowdsecurity/crowdsec/pkg/appsec/appsec_rule" + "github.com/crowdsecurity/crowdsec/pkg/types" + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/require" +) + +func TestAppsecRuleTransformsOthers(t *testing.T) { + + log.SetLevel(log.TraceLevel) + tests := []appsecRuleTest{ + { + name: "normalizepath", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "equals", Value: "b/c"}, + Transform: []string{"normalizepath"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/?foo=a/../b/c", + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Equal(t, types.APPSEC, events[0].Type) + require.Equal(t, types.LOG, events[1].Type) + require.Equal(t, "rule1", events[1].Appsec.MatchedRules[0]["msg"]) + }, + }, + { + name: "normalizepath #2", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "equals", Value: "b/c/"}, + Transform: []string{"normalizepath"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/?foo=a/../b/c/////././././", + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Equal(t, types.APPSEC, events[0].Type) + require.Equal(t, types.LOG, events[1].Type) + require.Equal(t, "rule1", events[1].Appsec.MatchedRules[0]["msg"]) + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + loadAppSecEngine(test, t) + }) + } +} diff --git a/pkg/acquisition/modules/appsec/appsec_test.go b/pkg/acquisition/modules/appsec/appsec_test.go index 25aea0c78..d98215bf2 100644 --- a/pkg/acquisition/modules/appsec/appsec_test.go +++ b/pkg/acquisition/modules/appsec/appsec_test.go @@ -1284,6 +1284,206 @@ func TestAppsecRuleMatches(t *testing.T) { } } +func TestAppsecRuleTransforms(t *testing.T) { + + log.SetLevel(log.TraceLevel) + tests := []appsecRuleTest{ + { + name: "Basic matching rule", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"URI"}, + Match: appsec_rule.Match{Type: "equals", Value: "/toto"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/toto", + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Equal(t, types.APPSEC, events[0].Type) + require.Equal(t, types.LOG, events[1].Type) + require.Equal(t, "rule1", events[1].Appsec.MatchedRules[0]["msg"]) + }, + }, + { + name: "lowercase", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"URI"}, + Match: appsec_rule.Match{Type: "equals", Value: "/toto"}, + Transform: []string{"lowercase"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/TOTO", + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Equal(t, types.APPSEC, events[0].Type) + require.Equal(t, types.LOG, events[1].Type) + require.Equal(t, "rule1", events[1].Appsec.MatchedRules[0]["msg"]) + }, + }, + { + name: "uppercase", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"URI"}, + Match: appsec_rule.Match{Type: "equals", Value: "/TOTO"}, + Transform: []string{"uppercase"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/toto", + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Equal(t, types.APPSEC, events[0].Type) + require.Equal(t, types.LOG, events[1].Type) + require.Equal(t, "rule1", events[1].Appsec.MatchedRules[0]["msg"]) + }, + }, + { + name: "b64decode", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "equals", Value: "toto"}, + Transform: []string{"b64decode"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/?foo=dG90bw", + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Equal(t, types.APPSEC, events[0].Type) + require.Equal(t, types.LOG, events[1].Type) + require.Equal(t, "rule1", events[1].Appsec.MatchedRules[0]["msg"]) + }, + }, + { + name: "b64decode with extra padding", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "equals", Value: "toto"}, + Transform: []string{"b64decode"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/?foo=dG90bw===", + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Equal(t, types.APPSEC, events[0].Type) + require.Equal(t, types.LOG, events[1].Type) + require.Equal(t, "rule1", events[1].Appsec.MatchedRules[0]["msg"]) + }, + }, + { + name: "length", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "gte", Value: "3"}, + Transform: []string{"length"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/?foo=toto", + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Equal(t, types.APPSEC, events[0].Type) + require.Equal(t, types.LOG, events[1].Type) + require.Equal(t, "rule1", events[1].Appsec.MatchedRules[0]["msg"]) + }, + }, + { + name: "urldecode", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "equals", Value: "BB/A"}, + Transform: []string{"urldecode"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/?foo=%42%42%2F%41", + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Equal(t, types.APPSEC, events[0].Type) + require.Equal(t, types.LOG, events[1].Type) + require.Equal(t, "rule1", events[1].Appsec.MatchedRules[0]["msg"]) + }, + }, + { + name: "trim", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "equals", Value: "BB/A"}, + Transform: []string{"urldecode", "trim"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/?foo=%20%20%42%42%2F%41%20%20", + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Equal(t, types.APPSEC, events[0].Type) + require.Equal(t, types.LOG, events[1].Type) + require.Equal(t, "rule1", events[1].Appsec.MatchedRules[0]["msg"]) + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + loadAppSecEngine(test, t) + }) + } +} + func loadAppSecEngine(test appsecRuleTest, t *testing.T) { if testing.Verbose() { log.SetLevel(log.TraceLevel) diff --git a/pkg/acquisition/modules/appsec/appsec_win_test.go b/pkg/acquisition/modules/appsec/appsec_win_test.go new file mode 100644 index 000000000..e85d75df2 --- /dev/null +++ b/pkg/acquisition/modules/appsec/appsec_win_test.go @@ -0,0 +1,46 @@ +//go:build windows +// +build windows + +package appsecacquisition + +import ( + "testing" + + log "github.com/sirupsen/logrus" +) + +func TestAppsecRuleTransformsWindows(t *testing.T) { + + log.SetLevel(log.TraceLevel) + tests := []appsecRuleTest{ + // { + // name: "normalizepath", + // expected_load_ok: true, + // inband_rules: []appsec_rule.CustomRule{ + // { + // Name: "rule1", + // Zones: []string{"ARGS"}, + // Variables: []string{"foo"}, + // Match: appsec_rule.Match{Type: "equals", Value: "b/c"}, + // Transform: []string{"normalizepath"}, + // }, + // }, + // input_request: appsec.ParsedRequest{ + // RemoteAddr: "1.2.3.4", + // Method: "GET", + // URI: "/?foo=a/../b/c", + // }, + // output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + // require.Len(t, events, 2) + // require.Equal(t, types.APPSEC, events[0].Type) + // require.Equal(t, types.LOG, events[1].Type) + // require.Equal(t, "rule1", events[1].Appsec.MatchedRules[0]["msg"]) + // }, + // }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + loadAppSecEngine(test, t) + }) + } +} diff --git a/pkg/appsec/appsec_rule/modsecurity.go b/pkg/appsec/appsec_rule/modsecurity.go index 0b117cd77..a269384cc 100644 --- a/pkg/appsec/appsec_rule/modsecurity.go +++ b/pkg/appsec/appsec_rule/modsecurity.go @@ -19,7 +19,8 @@ var zonesMap map[string]string = map[string]string{ "HEADERS": "REQUEST_HEADERS", "METHOD": "REQUEST_METHOD", "PROTOCOL": "REQUEST_PROTOCOL", - "URI": "REQUEST_URI", + "URI": "REQUEST_FILENAME", + "URI_FULL": "REQUEST_URI", "RAW_BODY": "REQUEST_BODY", "FILENAMES": "FILES", } @@ -28,8 +29,14 @@ var transformMap map[string]string = map[string]string{ "lowercase": "t:lowercase", "uppercase": "t:uppercase", "b64decode": "t:base64Decode", - "hexdecode": "t:hexDecode", - "length": "t:length", + //"hexdecode": "t:hexDecode", -> not supported by coraza + "length": "t:length", + "urldecode": "t:urlDecode", + "trim": "t:trim", + "normalize_path": "t:normalizePath", + "normalizepath": "t:normalizePath", + "htmlentitydecode": "t:htmlEntityDecode", + "html_entity_decode": "t:htmlEntityDecode", } var matchMap map[string]string = map[string]string{ diff --git a/pkg/appsec/request.go b/pkg/appsec/request.go index a9eb0d372..66b5d797f 100644 --- a/pkg/appsec/request.go +++ b/pkg/appsec/request.go @@ -365,11 +365,11 @@ func NewParsedRequestFromRequest(r *http.Request, logger *logrus.Entry) (ParsedR UUID: uuid.New().String(), ClientHost: clientHost, ClientIP: clientIP, - URI: parsedURL.Path, + URI: clientURI, Method: clientMethod, - Host: r.Host, + Host: clientHost, Headers: r.Header, - URL: r.URL, + URL: parsedURL, Proto: r.Proto, Body: body, Args: ParseQuery(parsedURL.RawQuery),