Skip to content

Commit

Permalink
appsec: do not attempt to deduplicate native modsec rules (#3347)
Browse files Browse the repository at this point in the history 
* fix #3343

* fix #3350

* fix #3350

---------

Co-authored-by: blotus <[email protected]>
Co-authored-by: Laurence Jones <[email protected]>
  • Loading branch information
@buixor @blotus @LaurenceJJones
3 people authored Jan 27, 2025
1 parent 5b90dfb commit fdd3737
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 23 deletions.
19 changes: 12 additions & 7 deletions pkg/acquisition/modules/appsec/appsec_runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,24 @@ type AppsecRunner struct {
func (r *AppsecRunner) MergeDedupRules(collections []appsec.AppsecCollection, logger *log.Entry) string {
var rulesArr []string
dedupRules := make(map[string]struct{})
discarded := 0

for _, collection := range collections {
// Dedup *our* rules
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 _, ok := dedupRules[rule]; ok {
discarded++
logger.Debugf("Discarding duplicate rule : %s", rule)
continue
}
rulesArr = append(rulesArr, rule)
dedupRules[rule] = struct{}{}
}
// Don't mess up with native modsec rules
rulesArr = append(rulesArr, collection.NativeRules...)
}
if len(rulesArr) != len(dedupRules) {
logger.Warningf("%d rules were discarded as they were duplicates", len(rulesArr)-len(dedupRules))
if discarded > 0 {
logger.Warningf("%d rules were discarded as they were duplicates", discarded)
}

return strings.Join(rulesArr, "\n")
Expand Down
72 changes: 62 additions & 10 deletions pkg/acquisition/modules/appsec/appsec_runner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,29 @@ import (
"github.com/crowdsecurity/crowdsec/pkg/appsec/appsec_rule"
)

func TestAppsecRuleLoad(t *testing.T) {
func TestAppsecConflictRuleLoad(t *testing.T) {
log.SetLevel(log.TraceLevel)

tests := []appsecRuleTest{
{
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"`,
`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: "id conflict on native rule load",
expected_load_ok: false,
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"`,
},
},
{
name: "simple rule load",
expected_load_ok: true,
Expand All @@ -28,33 +47,66 @@ func TestAppsecRuleLoad(t *testing.T) {
},
},
{
name: "simple native rule load",
name: "duplicate 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"`,
inband_rules: []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: "toto"},
},
},
afterload_asserts: func(runner AppsecRunner) {
require.Len(t, runner.AppsecInbandEngine.GetRuleGroup().GetRules(), 1)
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
loadAppSecEngine(test, t)
})
}
}

func TestAppsecRuleLoad(t *testing.T) {
log.SetLevel(log.TraceLevel)

tests := []appsecRuleTest{
{
name: "simple native rule load (2)",
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"`,
`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)
require.Len(t, runner.AppsecInbandEngine.GetRuleGroup().GetRules(), 1)
},
},
{
name: "simple native rule load + dedup",
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"`,
`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)
Expand Down
10 changes: 6 additions & 4 deletions pkg/acquisition/modules/appsec/appsec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ func loadAppSecEngine(test appsecRuleTest, t *testing.T) {
log.SetLevel(log.WarnLevel)
}
inbandRules := []string{}
nativeInbandRules := []string{}
outofbandRules := []string{}
nativeOutofbandRules := []string{}
InChan := make(chan appsec.ParsedRequest)
OutChan := make(chan types.Event)

Expand All @@ -56,8 +58,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...)
nativeInbandRules = append(nativeInbandRules, test.inband_native_rules...)
nativeOutofbandRules = append(nativeOutofbandRules, test.outofband_native_rules...)
for ridx, rule := range test.outofband_rules {
strRule, _, err := rule.Convert(appsec_rule.ModsecurityRuleType, rule.Name)
if err != nil {
Expand All @@ -82,8 +84,8 @@ func loadAppSecEngine(test appsecRuleTest, t *testing.T) {
if err != nil {
t.Fatalf("unable to build appsec runtime : %s", err)
}
AppsecRuntime.InBandRules = []appsec.AppsecCollection{{Rules: inbandRules}}
AppsecRuntime.OutOfBandRules = []appsec.AppsecCollection{{Rules: outofbandRules}}
AppsecRuntime.InBandRules = []appsec.AppsecCollection{{Rules: inbandRules, NativeRules: nativeInbandRules}}
AppsecRuntime.OutOfBandRules = []appsec.AppsecCollection{{Rules: outofbandRules, NativeRules: nativeOutofbandRules}}
appsecRunnerUUID := uuid.New().String()
//we copy AppsecRutime for each runner
wrt := *AppsecRuntime
Expand Down
5 changes: 3 additions & 2 deletions pkg/appsec/appsec_rules_collection.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
type AppsecCollection struct {
collectionName string
Rules []string
NativeRules []string
}

var APPSEC_RULE = "appsec-rule"
Expand Down Expand Up @@ -88,14 +89,14 @@ func LoadCollection(pattern string, logger *log.Entry) ([]AppsecCollection, erro
if strings.TrimSpace(line) == "" {
continue
}
appsecCol.Rules = append(appsecCol.Rules, line)
appsecCol.NativeRules = append(appsecCol.NativeRules, line)
}
}
}

if appsecRule.SecLangRules != nil {
logger.Tracef("Adding inline rules %+v", appsecRule.SecLangRules)
appsecCol.Rules = append(appsecCol.Rules, appsecRule.SecLangRules...)
appsecCol.NativeRules = append(appsecCol.NativeRules, appsecRule.SecLangRules...)
}

if appsecRule.Rules != nil {
Expand Down

0 comments on commit fdd3737

Please sign in to comment.