Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

appsec: do not attempt to deduplicate native modsec rules #3347

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

appsec: do not attempt to deduplicate native modsec rules #3347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8f03169
fix #3343
buixor Nov 27, 2024
40c2a7a
deal with unterminated multi-line rules
buixor Nov 27, 2024
259b6be
linter
buixor Nov 27, 2024
78453b4
Merge branch 'master' into fix_3343
buixor Nov 28, 2024
2453241
fix #3350
buixor Nov 29, 2024
f48d208
fix #3350
buixor Nov 29, 2024
91cffb7
up
buixor Nov 29, 2024
3bcf8cf
fix
buixor Nov 29, 2024
595e792
Merge branch 'master' into fix_3343
blotus Dec 16, 2024
ee349b6
Merge branch 'master' into fix_3343
buixor Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/acquisition/modules/appsec/appsec_runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func (r *AppsecRunner) MergeDedupRules(collections []appsec.AppsecCollection, lo
dedupRules := make(map[string]struct{})

for _, collection := range collections {
// Dedup *our* rules
for _, rule := range collection.Rules {
if _, ok := dedupRules[rule]; !ok {
rulesArr = append(rulesArr, rule)
Expand All @@ -45,6 +46,8 @@ func (r *AppsecRunner) MergeDedupRules(collections []appsec.AppsecCollection, lo
logger.Debugf("Discarding duplicate rule : %s", rule)
}
}
// 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))
Expand Down
70 changes: 61 additions & 9 deletions pkg/acquisition/modules/appsec/appsec_runner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,29 @@ import (
"github.com/stretchr/testify/require"
)

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 @@ -26,33 +46,65 @@ 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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
type AppsecCollection struct {
collectionName string
Rules []string
NativeRules []string
}

var APPSEC_RULE = "appsec-rule"
Expand Down Expand Up @@ -88,14 +89,14 @@
if strings.TrimSpace(line) == "" {
continue
}
appsecCol.Rules = append(appsecCol.Rules, line)
appsecCol.NativeRules = append(appsecCol.NativeRules, line)

Check warning on line 92 in pkg/appsec/appsec_rules_collection.go

View check run for this annotation

Codecov / codecov/patch

pkg/appsec/appsec_rules_collection.go#L92 

Added line #L92 was not covered by tests
}
}
}

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...)

Check warning on line 99 in pkg/appsec/appsec_rules_collection.go

View check run for this annotation

Codecov / codecov/patch

pkg/appsec/appsec_rules_collection.go#L99 

Added line #L99 was not covered by tests
}

if appsecRule.Rules != nil {
Expand Down
Loading