From e08d627964600fe210e341ef8421b32f6035e088 Mon Sep 17 00:00:00 2001 From: Max Leske <250711+theseion@users.noreply.github.com> Date: Sun, 5 Jan 2025 19:13:23 +0100 Subject: [PATCH] fix: put back support for encoded_request (#439) * fix: put back support for encoded_request * fix: test cleanup --- ftwhttp/connection.go | 12 ++- ftwhttp/request.go | 12 ++- ftwhttp/types.go | 2 + runner/run.go | 27 ++++--- runner/run_test.go | 74 ++++++++++++++----- runner/testdata/TestEncodedRequest.yaml | 18 +++++ .../TestEncodedRequest_InvalidEncoding.yaml | 18 +++++ test/defaults.go | 20 ----- 8 files changed, 130 insertions(+), 53 deletions(-) create mode 100644 runner/testdata/TestEncodedRequest.yaml create mode 100644 runner/testdata/TestEncodedRequest_InvalidEncoding.yaml diff --git a/ftwhttp/connection.go b/ftwhttp/connection.go index 8075e22..d16e0ca 100644 --- a/ftwhttp/connection.go +++ b/ftwhttp/connection.go @@ -84,9 +84,15 @@ func (c *Connection) receive() (io.Reader, error) { // Request will use all the inputs and send a raw http request to the destination func (c *Connection) Request(request *Request) error { // Build request first, then connect and send, so timers are accurate - data, err := BuildRequest(request) - if err != nil { - return fmt.Errorf("ftw/http: fatal error building request: %w", err) + var data []byte + var err error + if request.isRaw { + data = request.rawRequest + } else { + data, err = BuildRequest(request) + if err != nil { + return fmt.Errorf("ftw/http: fatal error building request: %w", err) + } } log.Debug().Msgf("ftw/http: sending data:\n%s\n", data) diff --git a/ftwhttp/request.go b/ftwhttp/request.go index f5d6e50..f69b141 100644 --- a/ftwhttp/request.go +++ b/ftwhttp/request.go @@ -26,14 +26,22 @@ func (rl RequestLine) ToString() string { // NewRequest creates a new request, an initial request line, and headers func NewRequest(reqLine *RequestLine, h Header, data []byte, autocompleteHeaders bool) *Request { - r := &Request{ + return &Request{ requestLine: reqLine, headers: h.Clone(), cookies: nil, data: data, autoCompleteHeaders: autocompleteHeaders, + isRaw: false, + } +} + +// NewRawRequest creates a new request from raw data +func NewRawRequest(data []byte) *Request { + return &Request{ + rawRequest: data, + isRaw: true, } - return r } // SetAutoCompleteHeaders sets the value to the corresponding bool diff --git a/ftwhttp/types.go b/ftwhttp/types.go index 2cd0102..088dcbf 100644 --- a/ftwhttp/types.go +++ b/ftwhttp/types.go @@ -76,6 +76,8 @@ type Request struct { cookies http.CookieJar data []byte autoCompleteHeaders bool + isRaw bool + rawRequest []byte } // Response represents the http response received from the server/waf diff --git a/runner/run.go b/runner/run.go index 59e86dd..a06f85a 100644 --- a/runner/run.go +++ b/runner/run.go @@ -4,6 +4,7 @@ package runner import ( + "encoding/base64" "errors" "fmt" "time" @@ -158,8 +159,6 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase sch return nil } - var req *ftwhttp.Request - // Destination is needed for a request dest := &ftwhttp.Destination{ DestAddr: testInput.GetDestAddr(), @@ -175,9 +174,12 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase sch ftwCheck.SetStartMarker(startMarker) } - req = getRequestFromTest(testInput) + req, err := getRequestFromTest(testInput) + if err != nil { + return fmt.Errorf("failed to read request from test specification: %w", err) + } - err := runContext.Client.NewConnection(*dest) + err = runContext.Client.NewConnection(*dest) if err != nil && !expectErr { return fmt.Errorf("can't connect to destination %+v: %w", dest, err) @@ -399,8 +401,14 @@ func checkResult(c *check.FTWCheck, response *ftwhttp.Response, responseError er return Success } -func getRequestFromTest(testInput test.Input) *ftwhttp.Request { - var req *ftwhttp.Request +func getRequestFromTest(testInput test.Input) (*ftwhttp.Request, error) { + if utils.IsNotEmpty(testInput.EncodedRequest) { + data, err := base64.StdEncoding.DecodeString(testInput.EncodedRequest) + if err != nil { + return nil, err + } + return ftwhttp.NewRawRequest(data), nil + } rline := &ftwhttp.RequestLine{ Method: testInput.GetMethod(), @@ -409,11 +417,8 @@ func getRequestFromTest(testInput test.Input) *ftwhttp.Request { } data := testInput.GetData() - // create a new request - req = ftwhttp.NewRequest(rline, testInput.Headers, - data, *testInput.AutocompleteHeaders) - - return req + return ftwhttp.NewRequest(rline, testInput.Headers, + data, *testInput.AutocompleteHeaders), nil } func notRunningInCloudMode(c *check.FTWCheck) bool { diff --git a/runner/run_test.go b/runner/run_test.go index c838fa3..02396f6 100644 --- a/runner/run_test.go +++ b/runner/run_test.go @@ -24,6 +24,7 @@ import ( "github.com/coreruleset/go-ftw/ftwhttp" "github.com/coreruleset/go-ftw/output" "github.com/coreruleset/go-ftw/test" + "github.com/coreruleset/go-ftw/waflog" ) var logText = `[Tue Jan 05 02:21:09.637165 2021] [:error] [pid 76:tid 139683434571520] [client 172.23.0.1:58998] [client 172.23.0.1] ModSecurity: Warning. Pattern match "\\\\b(?:keep-alive|close),\\\\s?(?:keep-alive|close)\\\\b" at REQUEST_HEADERS:Connection. [file "/etc/modsecurity.d/owasp-crs/rules/REQUEST-920-PROTOCOL-ENFORCEMENT.conf"] [line "339"] [id "920210"] [msg "Multiple/Conflicting Connection Header Data Found"] [data "close,close"] [severity "WARNING"] [ver "OWASP_CRS/3.3.0"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-protocol"] [tag "paranoia-level/1"] [tag "OWASP_CRS"] [tag "capec/1000/210/272"] [hostname "localhost"] [uri "/"] [unique_id "X-PNFSe1VwjCgYRI9FsbHgAAAIY"] @@ -233,8 +234,7 @@ func (s *runTestSuite) BeforeTest(_ string, name string) { } // create a temporary file to hold the test - tempDir := s.T().TempDir() - testFileContents, err := os.CreateTemp(tempDir, "mock-test-*.yaml") + testFileContents, err := os.CreateTemp(s.T().TempDir(), "mock-test-*.yaml") s.Require().NoError(err, "cannot create temporary file") err = tmpl.Execute(testFileContents, vars) s.Require().NoError(err, "cannot execute template") @@ -247,17 +247,6 @@ func (s *runTestSuite) BeforeTest(_ string, name string) { s.tempFileName = testFileContents.Name() } -func (s *runTestSuite) AfterTest(_ string, _ string) { - err := os.Remove(s.logFilePath) - s.Require().NoError(err, "cannot remove log file") - log.Info().Msgf("Deleting temporary file '%s'", s.logFilePath) - if s.tempFileName != "" { - err = os.Remove(s.tempFileName) - s.Require().NoError(err, "cannot remove test file") - s.tempFileName = "" - } -} - func TestRunTestsTestSuite(t *testing.T) { suite.Run(t, new(runTestSuite)) } @@ -409,7 +398,8 @@ func (s *runTestSuite) TestGetRequestFromTestWithAutocompleteHeaders() { Port: &s.dest.Port, Protocol: &s.dest.Protocol, } - request := getRequestFromTest(input) + request, err := getRequestFromTest(input) + s.Require().NoError(err) client, err := ftwhttp.NewClient(ftwhttp.NewClientConfig()) s.Require().NoError(err) @@ -439,7 +429,8 @@ func (s *runTestSuite) TestGetRequestFromTestWithoutAutocompleteHeaders() { Port: &s.dest.Port, Protocol: &s.dest.Protocol, } - request := getRequestFromTest(input) + request, err := getRequestFromTest(input) + s.Require().NoError(err) client, err := ftwhttp.NewClient(ftwhttp.NewClientConfig()) s.Require().NoError(err) @@ -617,7 +608,8 @@ func (s *runTestSuite) TestGetRequestFromData() { Protocol: &s.dest.Protocol, Data: &data, } - request := getRequestFromTest(input) + request, err := getRequestFromTest(input) + s.Require().NoError(err) s.Equal(data, string(request.Data())) } @@ -635,7 +627,8 @@ func (s *runTestSuite) TestGetRequestFromEncodedData() { Protocol: &s.dest.Protocol, Data: &data, } - request := getRequestFromTest(input) + request, err := getRequestFromTest(input) + s.Require().NoError(err) s.Equal(data, string(request.Data())) } @@ -663,3 +656,50 @@ func (s *runTestSuite) TestTriggeredRules() { }}} s.Equal(triggeredRules, res.Stats.TriggeredRules, "Oops, triggered rules don't match expectation") } + +func (s *runTestSuite) TestEncodedRequest() { + client, err := ftwhttp.NewClient(ftwhttp.NewClientConfig()) + s.Require().NoError(err) + ll, err := waflog.NewFTWLogLines(s.cfg) + s.T().Cleanup(func() { _ = ll.Cleanup() }) + s.Require().NoError(err) + + context := &TestRunContext{ + Config: s.cfg, + Client: client, + LogLines: ll, + Stats: NewRunStats(), + Output: s.out, + } + stage := s.ftwTests[0].Tests[0].Stages[0] + _check, err := check.NewCheck(s.cfg) + s.T().Cleanup(func() { _ = _check.Close() }) + s.Require().NoError(err) + + err = RunStage(context, _check, types.Test{}, stage) + s.Require().NoError(err) + s.Equal(Success, context.Result) +} + +func (s *runTestSuite) TestEncodedRequest_InvalidEncoding() { + client, err := ftwhttp.NewClient(ftwhttp.NewClientConfig()) + s.Require().NoError(err) + ll, err := waflog.NewFTWLogLines(s.cfg) + s.T().Cleanup(func() { _ = ll.Cleanup() }) + s.Require().NoError(err) + + context := &TestRunContext{ + Config: s.cfg, + Client: client, + LogLines: ll, + Stats: NewRunStats(), + Output: s.out, + } + stage := s.ftwTests[0].Tests[0].Stages[0] + _check, err := check.NewCheck(s.cfg) + s.T().Cleanup(func() { _ = _check.Close() }) + s.Require().NoError(err) + + err = RunStage(context, _check, types.Test{}, stage) + s.Error(err, "failed to read request from test specification: illegal base64 data at input byte 4") +} diff --git a/runner/testdata/TestEncodedRequest.yaml b/runner/testdata/TestEncodedRequest.yaml new file mode 100644 index 0000000..40dd6e1 --- /dev/null +++ b/runner/testdata/TestEncodedRequest.yaml @@ -0,0 +1,18 @@ +--- +meta: + author: "tester" + description: "Tests for verifying encoded_request" +rule_id: 123456 +tests: + - test_id: 1 + stages: + - input: + dest_addr: "{{ .TestAddr }}" + port: {{ .TestPort }} + headers: + User-Agent: "ModSecurity CRS 3 Tests" + Accept: "*/*" + Host: "none.host" + encoded_request: "UE9TVCAvIEhUVFAvMS4xDQpBY2NlcHQ6ICovKg0KSG9zdDogbG9jYWxob3N0DQpUcmFuc2Zlci1F\nbmNvZGluZzogY2h1bmtlZA0KVXNlci1BZ2VudDogTW9kU2VjdXJpdHkgQ1JTIDMgVGVzdHMNCg0K\nMw0KSGkgDQozDQpDUlMNCjANCg0K" + output: + status: 200 diff --git a/runner/testdata/TestEncodedRequest_InvalidEncoding.yaml b/runner/testdata/TestEncodedRequest_InvalidEncoding.yaml new file mode 100644 index 0000000..2279456 --- /dev/null +++ b/runner/testdata/TestEncodedRequest_InvalidEncoding.yaml @@ -0,0 +1,18 @@ +--- +meta: + author: "tester" + description: "Tests for verifying encoded_request" +rule_id: 123456 +tests: + - test_id: 1 + stages: + - input: + dest_addr: "{{ .TestAddr }}" + port: {{ .TestPort }} + headers: + User-Agent: "ModSecurity CRS 3 Tests" + Accept: "*/*" + Host: "none.host" + encoded_request: "garbage" + output: + status: 200 diff --git a/test/defaults.go b/test/defaults.go index 2aea607..4edc894 100644 --- a/test/defaults.go +++ b/test/defaults.go @@ -4,12 +4,9 @@ package test import ( - "encoding/base64" - schema "github.com/coreruleset/ftw-tests-schema/v2/types" "github.com/coreruleset/go-ftw/ftwhttp" - "github.com/coreruleset/go-ftw/utils" ) type Input schema.Input @@ -74,20 +71,3 @@ func (i *Input) GetHeaders() ftwhttp.Header { } return ftwhttp.Header(i.Headers) } - -// GetRawRequest returns the proper raw data, and error if there was none -func (i *Input) GetRawRequest() ([]byte, error) { - if utils.IsNotEmpty(i.EncodedRequest) { - // if Encoded, first base64 decode, then dump - return base64.StdEncoding.DecodeString(i.EncodedRequest) - } - return nil, nil -} - -// GetAutocompleteHeaders returns the autocompleteHeaders value, defaults to true -func (i *Input) GetAutocompleteHeaders() bool { - if i.AutocompleteHeaders == nil { - return true - } - return *i.AutocompleteHeaders -}