From 99466ec646c450949d8f335c12cc5b8f12c3338e Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Fri, 2 Sep 2022 15:09:20 +0900 Subject: [PATCH] Process body phases even without actual bodies and populate Host header (#16) --- ftw/tests.sh | 2 +- main.go | 42 ++++++++++++++++++++-- main_test.go | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 4 deletions(-) diff --git a/ftw/tests.sh b/ftw/tests.sh index 410104dc70fdf..8923633916de9 100755 --- a/ftw/tests.sh +++ b/ftw/tests.sh @@ -31,4 +31,4 @@ echo -e "\n[Ok] Got status code $status_code, expected 200. Ready to start." # Protocol violations often get treated by Envoy itself, exclude them for now while investigating # what works. Also currently HTTP/1.0 seems to have an issue so we exclude any tests using it. -go-ftw run -d coreruleset/tests/regression/tests --config ftw.yml --exclude '920.*|921.*|93.*|True.*|94.*|95.*' +go-ftw run -d coreruleset/tests/regression/tests --config ftw.yml --exclude '920.*|9323.*' diff --git a/main.go b/main.go index 8cdbd2709d052..acb1869e3bd17 100644 --- a/main.go +++ b/main.go @@ -147,9 +147,11 @@ type httpContext struct { // Embed the default http context here, // so that we don't need to reimplement all the methods. types.DefaultHttpContext - contextID uint32 - tx *coraza.Transaction - httpProtocol string + contextID uint32 + tx *coraza.Transaction + httpProtocol string + processedRequestBody bool + processedResponseBody bool } // Override types.DefaultHttpContext. @@ -193,6 +195,12 @@ func (ctx *httpContext) OnHttpRequestHeaders(numHeaders int, endOfStream bool) t tx.AddRequestHeader(h[0], h[1]) } + // CRS rules tend to expect Host even with HTTP/2 + authority, err := proxywasm.GetHttpRequestHeader(":authority") + if err == nil { + tx.AddRequestHeader("Host", authority) + } + interruption := tx.ProcessRequestHeaders() if interruption != nil { return ctx.handleInterruption(interruption) @@ -222,6 +230,7 @@ func (ctx *httpContext) OnHttpRequestBody(bodySize int, endOfStream bool) types. return types.ActionContinue } + ctx.processedRequestBody = true interruption, err := tx.ProcessRequestBody() if err != nil { proxywasm.LogCriticalf("failed to process request body: %v", err) @@ -237,6 +246,20 @@ func (ctx *httpContext) OnHttpRequestBody(bodySize int, endOfStream bool) types. func (ctx *httpContext) OnHttpResponseHeaders(numHeaders int, endOfStream bool) types.Action { tx := ctx.tx + // Requests without body won't call OnHttpRequestBody, but there are rules in the request body + // phase that still need to be executed. If they haven't been executed yet, now is the time. + if !ctx.processedRequestBody { + ctx.processedRequestBody = true + interruption, err := tx.ProcessRequestBody() + if err != nil { + proxywasm.LogCriticalf("failed to process request body: %v", err) + return types.ActionContinue + } + if interruption != nil { + return ctx.handleInterruption(interruption) + } + } + status, err := proxywasm.GetHttpResponseHeader(":status") if err != nil { proxywasm.LogCriticalf("failed to get :status: %v", err) @@ -288,6 +311,7 @@ func (ctx *httpContext) OnHttpResponseBody(bodySize int, endOfStream bool) types // We have already sent response headers so cannot now send an unauthorized response. // The error will have been logged by Coraza though. + ctx.processedResponseBody = true _, err := tx.ProcessResponseBody() if err != nil { proxywasm.LogCriticalf("failed to process response body: %v", err) @@ -299,6 +323,18 @@ func (ctx *httpContext) OnHttpResponseBody(bodySize int, endOfStream bool) types // Override types.DefaultHttpContext. func (ctx *httpContext) OnHttpStreamDone() { + tx := ctx.tx + + // Responses without body won't call OnHttpResponseBody, but there are rules in the response body + // phase that still need to be executed. If they haven't been executed yet, now is the time. + if !ctx.processedResponseBody { + ctx.processedResponseBody = true + _, err := tx.ProcessResponseBody() + if err != nil { + proxywasm.LogCriticalf("failed to process response body: %v", err) + } + } + ctx.tx.ProcessLogging() _ = ctx.tx.Clean() proxywasm.LogInfof("%d finished", ctx.contextID) diff --git a/main_test.go b/main_test.go index ab1896c41ef20..b380d3b8ac84c 100644 --- a/main_test.go +++ b/main_test.go @@ -614,6 +614,105 @@ func TestParseCRS(t *testing.T) { }) } +func TestBodyRulesWithoutBody(t *testing.T) { + reqHdrs := [][2]string{ + {":path", "/hello"}, + {":method", "GET"}, + {":authority", "localhost"}, + {"User-Agent", "gotest"}, + {"Content-Type", "application/x-www-form-urlencoded"}, + {"Content-Length", "32"}, + } + respHdrs := [][2]string{ + {":status", "200"}, + {"Server", "gotest"}, + {"Content-Length", "11"}, + {"Content-Type", "text/plain"}, + } + tests := []struct { + name string + rules string + responseHdrsAction types.Action + responded403 bool + }{ + { + name: "url accepted in request body phase", + rules: ` +SecRuleEngine On\nSecRule REQUEST_URI \"@streq /admin\" \"id:101,phase:2,t:lowercase,deny\" +`, + responseHdrsAction: types.ActionContinue, + responded403: false, + }, + { + name: "url denied in request body phase", + rules: ` +SecRuleEngine On\nSecRule REQUEST_URI \"@streq /hello\" \"id:101,phase:2,t:lowercase,deny\" +`, + responseHdrsAction: types.ActionPause, + responded403: true, + }, + { + name: "url accepted in response body phase", + rules: ` +SecRuleEngine On\nSecRule REQUEST_URI \"@streq /admin\" \"id:101,phase:4,t:lowercase,deny\" +`, + responseHdrsAction: types.ActionContinue, + responded403: false, + }, + { + name: "url denied in response body phase", + rules: ` +SecRuleEngine On\nSecRule REQUEST_URI \"@streq /hello\" \"id:101,phase:4,t:lowercase,deny\" +`, + responseHdrsAction: types.ActionContinue, + responded403: false, + }, + } + + vmTest(t, func(t *testing.T, vm types.VMContext) { + for _, tc := range tests { + tt := tc + + t.Run(tt.name, func(t *testing.T) { + conf := fmt.Sprintf(` + { + "rules" : "%s", + "include_core_rule_set": false + } + `, strings.TrimSpace(tt.rules)) + opt := proxytest. + NewEmulatorOption(). + WithVMContext(vm). + WithPluginConfiguration([]byte(conf)) + + host, reset := proxytest.NewHostEmulator(opt) + defer reset() + + require.Equal(t, types.OnPluginStartStatusOK, host.StartPlugin()) + + id := host.InitializeHttpContext() + + requestHdrsAction := host.CallOnRequestHeaders(id, reqHdrs, false) + require.Equal(t, types.ActionContinue, requestHdrsAction) + + responseHdrsAction := host.CallOnResponseHeaders(id, respHdrs, false) + require.Equal(t, tt.responseHdrsAction, responseHdrsAction) + + // Call OnHttpStreamDone. + host.CompleteHttpContext(id) + + pluginResp := host.GetSentLocalResponse(id) + if tt.responded403 { + require.NotNil(t, pluginResp) + require.EqualValues(t, 403, pluginResp.StatusCode) + } else { + require.Nil(t, pluginResp) + } + }) + } + }) +} + func vmTest(t *testing.T, f func(*testing.T, types.VMContext)) { t.Helper()