From 351a41cf0bd60714e9f1ec34d6b4afe92430922b Mon Sep 17 00:00:00 2001 From: Thibault Koechlin Date: Mon, 4 Nov 2024 15:40:59 +0100 Subject: [PATCH 1/6] support multiple appsec configs --- pkg/acquisition/modules/appsec/appsec.go | 16 ++++- pkg/appsec/appsec.go | 74 +++++++++++++++++++++--- 2 files changed, 80 insertions(+), 10 deletions(-) diff --git a/pkg/acquisition/modules/appsec/appsec.go b/pkg/acquisition/modules/appsec/appsec.go index a6dcffe89a2..75d2531c547 100644 --- a/pkg/acquisition/modules/appsec/appsec.go +++ b/pkg/acquisition/modules/appsec/appsec.go @@ -41,6 +41,7 @@ type AppsecSourceConfig struct { Path string `yaml:"path"` Routines int `yaml:"routines"` AppsecConfig string `yaml:"appsec_config"` + AppsecConfigs []string `yaml:"appsec_configs"` AppsecConfigPath string `yaml:"appsec_config_path"` AuthCacheDuration *time.Duration `yaml:"auth_cache_duration"` configuration.DataSourceCommonCfg `yaml:",inline"` @@ -120,7 +121,7 @@ func (w *AppsecSource) UnmarshalConfig(yamlConfig []byte) error { w.config.Routines = 1 } - if w.config.AppsecConfig == "" && w.config.AppsecConfigPath == "" { + if w.config.AppsecConfig == "" && w.config.AppsecConfigPath == "" && len(w.config.AppsecConfigs) == 0 { return errors.New("appsec_config or appsec_config_path must be set") } @@ -172,6 +173,9 @@ func (w *AppsecSource) Configure(yamlConfig []byte, logger *log.Entry, MetricsLe w.InChan = make(chan appsec.ParsedRequest) appsecCfg := appsec.AppsecConfig{Logger: w.logger.WithField("component", "appsec_config")} + //we keep the datasource name + appsecCfg.Name = w.config.Name + // let's load the associated appsec_config: if w.config.AppsecConfigPath != "" { err := appsecCfg.LoadByPath(w.config.AppsecConfigPath) @@ -183,10 +187,20 @@ func (w *AppsecSource) Configure(yamlConfig []byte, logger *log.Entry, MetricsLe if err != nil { return fmt.Errorf("unable to load appsec_config: %w", err) } + } else if len(w.config.AppsecConfigs) > 0 { + for _, appsecConfig := range w.config.AppsecConfigs { + err := appsecCfg.Load(appsecConfig) + if err != nil { + return fmt.Errorf("unable to load appsec_config: %w", err) + } + } } else { return errors.New("no appsec_config provided") } + // Now we can set up the logger + appsecCfg.SetUpLogger() + w.AppsecRuntime, err = appsecCfg.Build() if err != nil { return fmt.Errorf("unable to build appsec_config: %w", err) diff --git a/pkg/appsec/appsec.go b/pkg/appsec/appsec.go index 30784b23db0..553db205b5d 100644 --- a/pkg/appsec/appsec.go +++ b/pkg/appsec/appsec.go @@ -1,7 +1,6 @@ package appsec import ( - "errors" "fmt" "net/http" "os" @@ -150,6 +149,18 @@ func (w *AppsecRuntimeConfig) ClearResponse() { w.Response.SendAlert = true } +func (wc *AppsecConfig) SetUpLogger() { + if wc.LogLevel == nil { + lvl := wc.Logger.Logger.GetLevel() + wc.LogLevel = &lvl + } + + /* wc.Name is actually the datasource name.*/ + wc.Logger = wc.Logger.Dup().WithField("name", wc.Name) + wc.Logger.Logger.SetLevel(*wc.LogLevel) + +} + func (wc *AppsecConfig) LoadByPath(file string) error { wc.Logger.Debugf("loading config %s", file) @@ -157,20 +168,65 @@ func (wc *AppsecConfig) LoadByPath(file string) error { if err != nil { return fmt.Errorf("unable to read file %s : %s", file, err) } - err = yaml.UnmarshalStrict(yamlFile, wc) + + //as LoadByPath can be called several time, we append rules/hooks, but override other options + var tmp AppsecConfig + + err = yaml.UnmarshalStrict(yamlFile, &tmp) if err != nil { return fmt.Errorf("unable to parse yaml file %s : %s", file, err) } - if wc.Name == "" { - return errors.New("name cannot be empty") + if wc.Name == "" && tmp.Name != "" { + wc.Name = tmp.Name } - if wc.LogLevel == nil { - lvl := wc.Logger.Logger.GetLevel() - wc.LogLevel = &lvl + + //We can append rules/hooks + if tmp.OutOfBandRules != nil { + wc.OutOfBandRules = append(wc.OutOfBandRules, tmp.OutOfBandRules...) } - wc.Logger = wc.Logger.Dup().WithField("name", wc.Name) - wc.Logger.Logger.SetLevel(*wc.LogLevel) + if tmp.InBandRules != nil { + wc.InBandRules = append(wc.InBandRules, tmp.InBandRules...) + } + if tmp.OnLoad != nil { + wc.OnLoad = append(wc.OnLoad, tmp.OnLoad...) + } + if tmp.PreEval != nil { + wc.PreEval = append(wc.PreEval, tmp.PreEval...) + } + if tmp.PostEval != nil { + wc.PostEval = append(wc.PostEval, tmp.PostEval...) + } + if tmp.OnMatch != nil { + wc.OnMatch = append(wc.OnMatch, tmp.OnMatch...) + } + if tmp.VariablesTracking != nil { + wc.VariablesTracking = append(wc.VariablesTracking, tmp.VariablesTracking...) + } + + //override other options + wc.LogLevel = tmp.LogLevel + + wc.DefaultRemediation = tmp.DefaultRemediation + wc.DefaultPassAction = tmp.DefaultPassAction + wc.BouncerBlockedHTTPCode = tmp.BouncerBlockedHTTPCode + wc.BouncerPassedHTTPCode = tmp.BouncerPassedHTTPCode + wc.UserBlockedHTTPCode = tmp.UserBlockedHTTPCode + wc.UserPassedHTTPCode = tmp.UserPassedHTTPCode + + if tmp.InbandOptions.DisableBodyInspection { + wc.InbandOptions.DisableBodyInspection = true + } + if tmp.InbandOptions.RequestBodyInMemoryLimit != nil { + wc.InbandOptions.RequestBodyInMemoryLimit = tmp.InbandOptions.RequestBodyInMemoryLimit + } + if tmp.OutOfBandOptions.DisableBodyInspection { + wc.OutOfBandOptions.DisableBodyInspection = true + } + if tmp.OutOfBandOptions.RequestBodyInMemoryLimit != nil { + wc.OutOfBandOptions.RequestBodyInMemoryLimit = tmp.OutOfBandOptions.RequestBodyInMemoryLimit + } + return nil } From f699308e2e066ca1516d59c40b629de52b86a817 Mon Sep 17 00:00:00 2001 From: Thibault Koechlin Date: Thu, 14 Nov 2024 11:45:18 +0100 Subject: [PATCH 2/6] auto dedup rules --- .../modules/appsec/appsec_runner.go | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/pkg/acquisition/modules/appsec/appsec_runner.go b/pkg/acquisition/modules/appsec/appsec_runner.go index de34b62d704..a8db4b9e016 100644 --- a/pkg/acquisition/modules/appsec/appsec_runner.go +++ b/pkg/acquisition/modules/appsec/appsec_runner.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "slices" + "strings" "time" "github.com/prometheus/client_golang/prometheus" @@ -31,23 +32,38 @@ type AppsecRunner struct { logger *log.Entry } +func (r *AppsecRunner) MergeDedupRules(collections []appsec.AppsecCollection, logger *log.Entry) string { + var rulesArr []string + dedupRules := make(map[string]struct{}) + + for _, collection := range collections { + for _, rule := range collection.Rules { + if _, ok := dedupRules[rule]; !ok { + rulesArr = append(rulesArr, rule) + dedupRules[rule] = struct{}{} + } else { + logger.Debugf("Discarding duplicate rule : %s", rule) + } + } + } + if len(rulesArr) != len(dedupRules) { + logger.Warningf("%d rules were discarded as they were duplicates", len(rulesArr)-len(dedupRules)) + } + + return strings.Join(rulesArr, "\n") +} + func (r *AppsecRunner) Init(datadir string) error { var err error fs := os.DirFS(datadir) - inBandRules := "" - outOfBandRules := "" - - for _, collection := range r.AppsecRuntime.InBandRules { - inBandRules += collection.String() - } - - for _, collection := range r.AppsecRuntime.OutOfBandRules { - outOfBandRules += collection.String() - } inBandLogger := r.logger.Dup().WithField("band", "inband") outBandLogger := r.logger.Dup().WithField("band", "outband") + //While loading rules, we dedup rules based on their content, while keeping the order + inBandRules := r.MergeDedupRules(r.AppsecRuntime.InBandRules, inBandLogger) + outOfBandRules := r.MergeDedupRules(r.AppsecRuntime.OutOfBandRules, outBandLogger) + //setting up inband engine inbandCfg := coraza.NewWAFConfig().WithDirectives(inBandRules).WithRootFS(fs).WithDebugLogger(appsec.NewCrzLogger(inBandLogger)) if !r.AppsecRuntime.Config.InbandOptions.DisableBodyInspection { From 63ab4a58d25813d951ab823e0b7cd9813c34b9af Mon Sep 17 00:00:00 2001 From: Thibault Koechlin Date: Thu, 14 Nov 2024 16:44:08 +0100 Subject: [PATCH 3/6] tests --- .../modules/appsec/appsec_runner_test.go | 139 ++++++++++++++++++ pkg/acquisition/modules/appsec/appsec_test.go | 12 ++ 2 files changed, 151 insertions(+) create mode 100644 pkg/acquisition/modules/appsec/appsec_runner_test.go diff --git a/pkg/acquisition/modules/appsec/appsec_runner_test.go b/pkg/acquisition/modules/appsec/appsec_runner_test.go new file mode 100644 index 00000000000..2027cf1d2c0 --- /dev/null +++ b/pkg/acquisition/modules/appsec/appsec_runner_test.go @@ -0,0 +1,139 @@ +package appsecacquisition + +import ( + "testing" + + "github.com/crowdsecurity/crowdsec/pkg/appsec/appsec_rule" + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/require" +) + +func TestAppsecRuleLoad(t *testing.T) { + log.SetLevel(log.TraceLevel) + tests := []appsecRuleTest{ + { + name: "simple rule load", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"ARGS"}, + Match: appsec_rule.Match{Type: "equals", Value: "toto"}, + }, + }, + afterload_asserts: func(runner AppsecRunner) { + require.Len(t, runner.AppsecInbandEngine.GetRuleGroup().GetRules(), 1) + }, + }, + { + name: "simple native rule load", + expected_load_ok: true, + inband_native_rules: []string{ + `Secrule REQUEST_HEADERS:Content-Type "@rx ^application/x-www-form-urlencoded" "id:100,phase:1,pass,nolog,noauditlog,ctl:requestBodyProcessor=URLENCODED"`, + }, + afterload_asserts: func(runner AppsecRunner) { + require.Len(t, runner.AppsecInbandEngine.GetRuleGroup().GetRules(), 1) + }, + }, + { + name: "simple native rule load (2)", + expected_load_ok: true, + inband_native_rules: []string{ + `Secrule REQUEST_HEADERS:Content-Type "@rx ^application/x-www-form-urlencoded" "id:100,phase:1,pass,nolog,noauditlog,ctl:requestBodyProcessor=URLENCODED"`, + `Secrule REQUEST_HEADERS:Content-Type "@rx ^multipart/form-data" "id:101,phase:1,pass,nolog,noauditlog,ctl:requestBodyProcessor=MULTIPART"`, + }, + afterload_asserts: func(runner AppsecRunner) { + require.Len(t, runner.AppsecInbandEngine.GetRuleGroup().GetRules(), 2) + }, + }, + { + name: "simple native rule load + dedup", + expected_load_ok: true, + inband_native_rules: []string{ + `Secrule REQUEST_HEADERS:Content-Type "@rx ^application/x-www-form-urlencoded" "id:100,phase:1,pass,nolog,noauditlog,ctl:requestBodyProcessor=URLENCODED"`, + `Secrule REQUEST_HEADERS:Content-Type "@rx ^multipart/form-data" "id:101,phase:1,pass,nolog,noauditlog,ctl:requestBodyProcessor=MULTIPART"`, + `Secrule REQUEST_HEADERS:Content-Type "@rx ^application/x-www-form-urlencoded" "id:100,phase:1,pass,nolog,noauditlog,ctl:requestBodyProcessor=URLENCODED"`, + }, + afterload_asserts: func(runner AppsecRunner) { + require.Len(t, runner.AppsecInbandEngine.GetRuleGroup().GetRules(), 2) + }, + }, + { + name: "multi simple rule load", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"ARGS"}, + Match: appsec_rule.Match{Type: "equals", Value: "toto"}, + }, + { + Name: "rule2", + Zones: []string{"ARGS"}, + Match: appsec_rule.Match{Type: "equals", Value: "toto"}, + }, + }, + afterload_asserts: func(runner AppsecRunner) { + require.Len(t, runner.AppsecInbandEngine.GetRuleGroup().GetRules(), 2) + }, + }, + { + name: "multi simple rule load", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + Zones: []string{"ARGS"}, + Match: appsec_rule.Match{Type: "equals", Value: "toto"}, + }, + { + Name: "rule2", + Zones: []string{"ARGS"}, + Match: appsec_rule.Match{Type: "equals", Value: "toto"}, + }, + }, + afterload_asserts: func(runner AppsecRunner) { + require.Len(t, runner.AppsecInbandEngine.GetRuleGroup().GetRules(), 2) + }, + }, + { + name: "imbricated rule load", + expected_load_ok: true, + inband_rules: []appsec_rule.CustomRule{ + { + Name: "rule1", + + Or: []appsec_rule.CustomRule{ + { + //Name: "rule1", + Zones: []string{"ARGS"}, + Match: appsec_rule.Match{Type: "equals", Value: "toto"}, + }, + { + //Name: "rule1", + Zones: []string{"ARGS"}, + Match: appsec_rule.Match{Type: "equals", Value: "tutu"}, + }, + { + //Name: "rule1", + Zones: []string{"ARGS"}, + Match: appsec_rule.Match{Type: "equals", Value: "tata"}, + }, { + //Name: "rule1", + Zones: []string{"ARGS"}, + Match: appsec_rule.Match{Type: "equals", Value: "titi"}, + }, + }, + }, + }, + afterload_asserts: func(runner AppsecRunner) { + require.Len(t, runner.AppsecInbandEngine.GetRuleGroup().GetRules(), 4) + }, + }, + } + 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 d2079b43726..1534f5cb7fa 100644 --- a/pkg/acquisition/modules/appsec/appsec_test.go +++ b/pkg/acquisition/modules/appsec/appsec_test.go @@ -18,6 +18,8 @@ type appsecRuleTest struct { expected_load_ok bool inband_rules []appsec_rule.CustomRule outofband_rules []appsec_rule.CustomRule + inband_native_rules []string + outofband_native_rules []string on_load []appsec.Hook pre_eval []appsec.Hook post_eval []appsec.Hook @@ -28,6 +30,7 @@ type appsecRuleTest struct { DefaultRemediation string DefaultPassAction string input_request appsec.ParsedRequest + afterload_asserts func(runner AppsecRunner) output_asserts func(events []types.Event, responses []appsec.AppsecTempResponse, appsecResponse appsec.BodyResponse, statusCode int) } @@ -53,6 +56,8 @@ func loadAppSecEngine(test appsecRuleTest, t *testing.T) { inbandRules = append(inbandRules, strRule) } + inbandRules = append(inbandRules, test.inband_native_rules...) + outofbandRules = append(outofbandRules, test.outofband_native_rules...) for ridx, rule := range test.outofband_rules { strRule, _, err := rule.Convert(appsec_rule.ModsecurityRuleType, rule.Name) if err != nil { @@ -94,6 +99,13 @@ func loadAppSecEngine(test appsecRuleTest, t *testing.T) { t.Fatalf("unable to initialize runner : %s", err) } + if test.afterload_asserts != nil { + //afterload asserts are just to evaluate the state of the runner after the rules have been loaded + //if it's present, don't try to process requests + test.afterload_asserts(runner) + return + } + input := test.input_request input.ResponseChannel = make(chan appsec.AppsecTempResponse) OutputEvents := make([]types.Event, 0) From a9ccf722a97f24c43069445d21ee55b3d8fc8fac Mon Sep 17 00:00:00 2001 From: Thibault Koechlin Date: Fri, 15 Nov 2024 14:56:47 +0100 Subject: [PATCH 4/6] make those mutually exclusive --- pkg/acquisition/modules/appsec/appsec.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/acquisition/modules/appsec/appsec.go b/pkg/acquisition/modules/appsec/appsec.go index 75d2531c547..76ebe54d4dc 100644 --- a/pkg/acquisition/modules/appsec/appsec.go +++ b/pkg/acquisition/modules/appsec/appsec.go @@ -125,6 +125,10 @@ func (w *AppsecSource) UnmarshalConfig(yamlConfig []byte) error { return errors.New("appsec_config or appsec_config_path must be set") } + if (w.config.AppsecConfig != "" || w.config.AppsecConfigPath != "") && len(w.config.AppsecConfigs) != 0 { + return errors.New("appsec_config and appsec_config_path are mutually exclusive with appsec_configs") + } + if w.config.Name == "" { if w.config.ListenSocket != "" && w.config.ListenAddr == "" { w.config.Name = w.config.ListenSocket From 7fa5929bafdc3311aedd1c9c5ce30427f5ca8785 Mon Sep 17 00:00:00 2001 From: Thibault Koechlin Date: Fri, 15 Nov 2024 16:01:03 +0100 Subject: [PATCH 5/6] use alt method --- pkg/acquisition/modules/appsec/appsec_rules_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/acquisition/modules/appsec/appsec_rules_test.go b/pkg/acquisition/modules/appsec/appsec_rules_test.go index 1a52df31714..00093c5a5ad 100644 --- a/pkg/acquisition/modules/appsec/appsec_rules_test.go +++ b/pkg/acquisition/modules/appsec/appsec_rules_test.go @@ -373,7 +373,7 @@ toto { name: "Basic matching IP address", expected_load_ok: true, - seclang_rules: []string{ + inband_native_rules: []string{ "SecRule REMOTE_ADDR \"@ipMatch 1.2.3.4\" \"id:1,phase:1,log,deny,msg: 'block ip'\"", }, input_request: appsec.ParsedRequest{ From 16af35804235e179078ffe12cac70a1be1ffa7b7 Mon Sep 17 00:00:00 2001 From: Thibault Koechlin Date: Fri, 15 Nov 2024 16:15:56 +0100 Subject: [PATCH 6/6] remove seclang_rules --- pkg/acquisition/modules/appsec/appsec_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/acquisition/modules/appsec/appsec_test.go b/pkg/acquisition/modules/appsec/appsec_test.go index 56458721a26..1534f5cb7fa 100644 --- a/pkg/acquisition/modules/appsec/appsec_test.go +++ b/pkg/acquisition/modules/appsec/appsec_test.go @@ -20,7 +20,6 @@ type appsecRuleTest struct { outofband_rules []appsec_rule.CustomRule inband_native_rules []string outofband_native_rules []string - seclang_rules []string on_load []appsec.Hook pre_eval []appsec.Hook post_eval []appsec.Hook @@ -67,8 +66,6 @@ func loadAppSecEngine(test appsecRuleTest, t *testing.T) { outofbandRules = append(outofbandRules, strRule) } - inbandRules = append(inbandRules, test.seclang_rules...) - appsecCfg := appsec.AppsecConfig{Logger: logger, OnLoad: test.on_load, PreEval: test.pre_eval,