From 09afcbe93a5a36ce25553f7ebc8c42cee0b98098 Mon Sep 17 00:00:00 2001 From: blotus Date: Fri, 24 May 2024 14:42:17 +0200 Subject: [PATCH] appsec: respect on_success parameter in hooks (#3017) --- .../modules/appsec/appsec_hooks_test.go | 198 +++++++++++++++++- pkg/appsec/appsec.go | 34 ++- 2 files changed, 221 insertions(+), 11 deletions(-) diff --git a/pkg/acquisition/modules/appsec/appsec_hooks_test.go b/pkg/acquisition/modules/appsec/appsec_hooks_test.go index 3cb2fcfde29..65fba33ae81 100644 --- a/pkg/acquisition/modules/appsec/appsec_hooks_test.go +++ b/pkg/acquisition/modules/appsec/appsec_hooks_test.go @@ -274,6 +274,64 @@ func TestAppsecOnMatchHooks(t *testing.T) { require.Equal(t, appsec.BanRemediation, responses[0].Action) }, }, + { + name: "on_match: on_success break", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule42", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "regex", Value: "^toto"}, + Transform: []string{"lowercase"}, + }, + }, + on_match: []appsec.Hook{ + {Filter: "IsInBand == true", Apply: []string{"CancelEvent()"}, OnSuccess: "break"}, + {Filter: "IsInBand == true", Apply: []string{"SetRemediation('captcha')"}}, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/urllll", + Args: url.Values{"foo": []string{"toto"}}, + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 1) + require.Equal(t, types.APPSEC, events[0].Type) + require.Len(t, responses, 1) + require.Equal(t, appsec.BanRemediation, responses[0].Action) + }, + }, + { + name: "on_match: on_success continue", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule42", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "regex", Value: "^toto"}, + Transform: []string{"lowercase"}, + }, + }, + on_match: []appsec.Hook{ + {Filter: "IsInBand == true", Apply: []string{"CancelEvent()"}, OnSuccess: "continue"}, + {Filter: "IsInBand == true", Apply: []string{"SetRemediation('captcha')"}}, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/urllll", + Args: url.Values{"foo": []string{"toto"}}, + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 1) + require.Equal(t, types.APPSEC, events[0].Type) + require.Len(t, responses, 1) + require.Equal(t, appsec.CaptchaRemediation, responses[0].Action) + }, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -286,7 +344,7 @@ func TestAppsecPreEvalHooks(t *testing.T) { tests := []appsecRuleTest{ { - name: "Basic on_load hook to disable inband rule", + name: "Basic pre_eval hook to disable inband rule", expected_load_ok: true, inband_rules: []appsec_rule.CustomRule{ { @@ -314,7 +372,7 @@ func TestAppsecPreEvalHooks(t *testing.T) { }, }, { - name: "Basic on_load fails to disable rule", + name: "Basic pre_eval fails to disable rule", expected_load_ok: true, inband_rules: []appsec_rule.CustomRule{ { @@ -349,7 +407,7 @@ func TestAppsecPreEvalHooks(t *testing.T) { }, }, { - name: "on_load : disable inband by tag", + name: "pre_eval : disable inband by tag", expected_load_ok: true, inband_rules: []appsec_rule.CustomRule{ { @@ -377,7 +435,7 @@ func TestAppsecPreEvalHooks(t *testing.T) { }, }, { - name: "on_load : disable inband by ID", + name: "pre_eval : disable inband by ID", expected_load_ok: true, inband_rules: []appsec_rule.CustomRule{ { @@ -405,7 +463,7 @@ func TestAppsecPreEvalHooks(t *testing.T) { }, }, { - name: "on_load : disable inband by name", + name: "pre_eval : disable inband by name", expected_load_ok: true, inband_rules: []appsec_rule.CustomRule{ { @@ -433,7 +491,7 @@ func TestAppsecPreEvalHooks(t *testing.T) { }, }, { - name: "on_load : outofband default behavior", + name: "pre_eval : outofband default behavior", expected_load_ok: true, outofband_rules: []appsec_rule.CustomRule{ { @@ -464,7 +522,7 @@ func TestAppsecPreEvalHooks(t *testing.T) { }, }, { - name: "on_load : set remediation by tag", + name: "pre_eval : set remediation by tag", expected_load_ok: true, inband_rules: []appsec_rule.CustomRule{ { @@ -491,7 +549,7 @@ func TestAppsecPreEvalHooks(t *testing.T) { }, }, { - name: "on_load : set remediation by name", + name: "pre_eval : set remediation by name", expected_load_ok: true, inband_rules: []appsec_rule.CustomRule{ { @@ -518,7 +576,7 @@ func TestAppsecPreEvalHooks(t *testing.T) { }, }, { - name: "on_load : set remediation by ID", + name: "pre_eval : set remediation by ID", expected_load_ok: true, inband_rules: []appsec_rule.CustomRule{ { @@ -546,6 +604,62 @@ func TestAppsecPreEvalHooks(t *testing.T) { require.Equal(t, http.StatusForbidden, appsecResponse.HTTPStatus) }, }, + { + name: "pre_eval : on_success continue", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rulez", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "regex", Value: "^toto"}, + Transform: []string{"lowercase"}, + }, + }, + pre_eval: []appsec.Hook{ + {Filter: "1==1", Apply: []string{"SetRemediationByName('rulez', 'foobar')"}, OnSuccess: "continue"}, + {Filter: "1==1", Apply: []string{"SetRemediationByName('rulez', 'foobar2')"}}, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/urllll", + Args: url.Values{"foo": []string{"toto"}}, + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Len(t, responses, 1) + require.Equal(t, "foobar2", responses[0].Action) + }, + }, + { + name: "pre_eval : on_success break", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rulez", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "regex", Value: "^toto"}, + Transform: []string{"lowercase"}, + }, + }, + pre_eval: []appsec.Hook{ + {Filter: "1==1", Apply: []string{"SetRemediationByName('rulez', 'foobar')"}, OnSuccess: "break"}, + {Filter: "1==1", Apply: []string{"SetRemediationByName('rulez', 'foobar2')"}}, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/urllll", + Args: url.Values{"foo": []string{"toto"}}, + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + require.Len(t, events, 2) + require.Len(t, responses, 1) + require.Equal(t, "foobar", responses[0].Action) + }, + }, } for _, test := range tests { @@ -705,6 +819,72 @@ func TestOnMatchRemediationHooks(t *testing.T) { require.Equal(t, http.StatusForbidden, statusCode) }, }, + { + name: "on_match: on_success break", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule42", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "regex", Value: "^toto"}, + Transform: []string{"lowercase"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/urllll", + Args: url.Values{"foo": []string{"toto"}}, + }, + DefaultRemediation: appsec.AllowRemediation, + on_match: []appsec.Hook{ + {Filter: "IsInBand == true", Apply: []string{"SetRemediation('captcha')", "SetReturnCode(418)"}, OnSuccess: "break"}, + {Filter: "IsInBand == true", Apply: []string{"SetRemediation('ban')"}}, + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + spew.Dump(responses) + spew.Dump(appsecResponse) + + log.Errorf("http status : %d", statusCode) + require.Equal(t, appsec.CaptchaRemediation, appsecResponse.Action) + require.Equal(t, http.StatusTeapot, appsecResponse.HTTPStatus) + require.Equal(t, http.StatusForbidden, statusCode) + }, + }, + { + name: "on_match: on_success continue", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule42", + Zones: []string{"ARGS"}, + Variables: []string{"foo"}, + Match: appsec_rule.Match{Type: "regex", Value: "^toto"}, + Transform: []string{"lowercase"}, + }, + }, + input_request: appsec.ParsedRequest{ + RemoteAddr: "1.2.3.4", + Method: "GET", + URI: "/urllll", + Args: url.Values{"foo": []string{"toto"}}, + }, + DefaultRemediation: appsec.AllowRemediation, + on_match: []appsec.Hook{ + {Filter: "IsInBand == true", Apply: []string{"SetRemediation('captcha')", "SetReturnCode(418)"}, OnSuccess: "continue"}, + {Filter: "IsInBand == true", Apply: []string{"SetRemediation('ban')"}}, + }, + output_asserts: func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) { + spew.Dump(responses) + spew.Dump(appsecResponse) + + log.Errorf("http status : %d", statusCode) + require.Equal(t, appsec.BanRemediation, appsecResponse.Action) + require.Equal(t, http.StatusTeapot, appsecResponse.HTTPStatus) + require.Equal(t, http.StatusForbidden, statusCode) + }, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/pkg/appsec/appsec.go b/pkg/appsec/appsec.go index 554fc3b7123..2c971fb36c5 100644 --- a/pkg/appsec/appsec.go +++ b/pkg/appsec/appsec.go @@ -259,6 +259,9 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) { //load hooks for _, hook := range wc.OnLoad { + if hook.OnSuccess != "" && hook.OnSuccess != "continue" && hook.OnSuccess != "break" { + return nil, fmt.Errorf("invalid 'on_success' for on_load hook : %s", hook.OnSuccess) + } err := hook.Build(hookOnLoad) if err != nil { return nil, fmt.Errorf("unable to build on_load hook : %s", err) @@ -267,6 +270,9 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) { } for _, hook := range wc.PreEval { + if hook.OnSuccess != "" && hook.OnSuccess != "continue" && hook.OnSuccess != "break" { + return nil, fmt.Errorf("invalid 'on_success' for pre_eval hook : %s", hook.OnSuccess) + } err := hook.Build(hookPreEval) if err != nil { return nil, fmt.Errorf("unable to build pre_eval hook : %s", err) @@ -275,6 +281,9 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) { } for _, hook := range wc.PostEval { + if hook.OnSuccess != "" && hook.OnSuccess != "continue" && hook.OnSuccess != "break" { + return nil, fmt.Errorf("invalid 'on_success' for post_eval hook : %s", hook.OnSuccess) + } err := hook.Build(hookPostEval) if err != nil { return nil, fmt.Errorf("unable to build post_eval hook : %s", err) @@ -283,6 +292,9 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) { } for _, hook := range wc.OnMatch { + if hook.OnSuccess != "" && hook.OnSuccess != "continue" && hook.OnSuccess != "break" { + return nil, fmt.Errorf("invalid 'on_success' for on_match hook : %s", hook.OnSuccess) + } err := hook.Build(hookOnMatch) if err != nil { return nil, fmt.Errorf("unable to build on_match hook : %s", err) @@ -302,6 +314,7 @@ func (wc *AppsecConfig) Build() (*AppsecRuntimeConfig, error) { } func (w *AppsecRuntimeConfig) ProcessOnLoadRules() error { + has_match := false for _, rule := range w.CompiledOnLoad { if rule.FilterExpr != nil { output, err := exprhelpers.Run(rule.FilterExpr, GetOnLoadEnv(w), w.Logger, w.Logger.Level >= log.DebugLevel) @@ -318,6 +331,7 @@ func (w *AppsecRuntimeConfig) ProcessOnLoadRules() error { w.Logger.Errorf("Filter must return a boolean, can't filter") continue } + has_match = true } for _, applyExpr := range rule.ApplyExpr { o, err := exprhelpers.Run(applyExpr, GetOnLoadEnv(w), w.Logger, w.Logger.Level >= log.DebugLevel) @@ -332,12 +346,15 @@ func (w *AppsecRuntimeConfig) ProcessOnLoadRules() error { default: } } + if has_match && rule.OnSuccess == "break" { + break + } } return nil } func (w *AppsecRuntimeConfig) ProcessOnMatchRules(request *ParsedRequest, evt types.Event) error { - + has_match := false for _, rule := range w.CompiledOnMatch { if rule.FilterExpr != nil { output, err := exprhelpers.Run(rule.FilterExpr, GetOnMatchEnv(w, request, evt), w.Logger, w.Logger.Level >= log.DebugLevel) @@ -354,6 +371,7 @@ func (w *AppsecRuntimeConfig) ProcessOnMatchRules(request *ParsedRequest, evt ty w.Logger.Errorf("Filter must return a boolean, can't filter") continue } + has_match = true } for _, applyExpr := range rule.ApplyExpr { o, err := exprhelpers.Run(applyExpr, GetOnMatchEnv(w, request, evt), w.Logger, w.Logger.Level >= log.DebugLevel) @@ -368,12 +386,15 @@ func (w *AppsecRuntimeConfig) ProcessOnMatchRules(request *ParsedRequest, evt ty default: } } + if has_match && rule.OnSuccess == "break" { + break + } } return nil } func (w *AppsecRuntimeConfig) ProcessPreEvalRules(request *ParsedRequest) error { - w.Logger.Debugf("processing %d pre_eval rules", len(w.CompiledPreEval)) + has_match := false for _, rule := range w.CompiledPreEval { if rule.FilterExpr != nil { output, err := exprhelpers.Run(rule.FilterExpr, GetPreEvalEnv(w, request), w.Logger, w.Logger.Level >= log.DebugLevel) @@ -390,6 +411,7 @@ func (w *AppsecRuntimeConfig) ProcessPreEvalRules(request *ParsedRequest) error w.Logger.Errorf("Filter must return a boolean, can't filter") continue } + has_match = true } // here means there is no filter or the filter matched for _, applyExpr := range rule.ApplyExpr { @@ -405,12 +427,16 @@ func (w *AppsecRuntimeConfig) ProcessPreEvalRules(request *ParsedRequest) error default: } } + if has_match && rule.OnSuccess == "break" { + break + } } return nil } func (w *AppsecRuntimeConfig) ProcessPostEvalRules(request *ParsedRequest) error { + has_match := false for _, rule := range w.CompiledPostEval { if rule.FilterExpr != nil { output, err := exprhelpers.Run(rule.FilterExpr, GetPostEvalEnv(w, request), w.Logger, w.Logger.Level >= log.DebugLevel) @@ -427,6 +453,7 @@ func (w *AppsecRuntimeConfig) ProcessPostEvalRules(request *ParsedRequest) error w.Logger.Errorf("Filter must return a boolean, can't filter") continue } + has_match = true } // here means there is no filter or the filter matched for _, applyExpr := range rule.ApplyExpr { @@ -444,6 +471,9 @@ func (w *AppsecRuntimeConfig) ProcessPostEvalRules(request *ParsedRequest) error default: } } + if has_match && rule.OnSuccess == "break" { + break + } } return nil