From 9821800f92cbe627783b5a35ddd1b170e1600682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustavo=20I=C3=B1iguez=20Goia?= Date: Fri, 12 Nov 2021 12:08:31 +0100 Subject: [PATCH] rules: don't load rules that fail to compile Be sure that we don't load invalid regexp rules. related: #536 --- daemon/rule/loader.go | 4 +++ daemon/rule/loader_test.go | 34 +++++++++++++++++++ daemon/rule/operator_test.go | 17 ++++++++++ daemon/rule/testdata/invalid-regexp-list.json | 31 +++++++++++++++++ daemon/rule/testdata/invalid-regexp.json | 16 +++++++++ 5 files changed, 102 insertions(+) create mode 100644 daemon/rule/testdata/invalid-regexp-list.json create mode 100644 daemon/rule/testdata/invalid-regexp.json diff --git a/daemon/rule/loader.go b/daemon/rule/loader.go index d463b09c63..3d02811552 100644 --- a/daemon/rule/loader.go +++ b/daemon/rule/loader.go @@ -116,11 +116,13 @@ func (l *Loader) loadRule(fileName string) error { if r.Enabled { if err := r.Operator.Compile(); err != nil { log.Warning("Operator.Compile() error: %s: %s", err, r.Operator.Data) + return fmt.Errorf("(1) Error compiling rule: %s", err) } if r.Operator.Type == List { for i := 0; i < len(r.Operator.List); i++ { if err := r.Operator.List[i].Compile(); err != nil { log.Warning("Operator.Compile() error: %s: ", err) + return fmt.Errorf("(1) Error compiling list rule: %s", err) } } } @@ -275,6 +277,7 @@ func (l *Loader) replaceUserRule(rule *Rule) (err error) { if rule.Enabled { if err := rule.Operator.Compile(); err != nil { log.Warning("Operator.Compile() error: %s: %s", err, rule.Operator.Data) + return fmt.Errorf("(2) Error compiling rule: %s", err) } if rule.Operator.Type == List { @@ -286,6 +289,7 @@ func (l *Loader) replaceUserRule(rule *Rule) (err error) { for i := 0; i < len(rule.Operator.List); i++ { if err := rule.Operator.List[i].Compile(); err != nil { log.Warning("Operator.Compile() error: %s: ", err) + return fmt.Errorf("(2) Error compiling list rule: %s", err) } } } diff --git a/daemon/rule/loader_test.go b/daemon/rule/loader_test.go index 8f5d61cb38..29fa796468 100644 --- a/daemon/rule/loader_test.go +++ b/daemon/rule/loader_test.go @@ -16,6 +16,7 @@ func TestMain(m *testing.M) { defer os.RemoveAll(tmpDir) os.Exit(m.Run()) } + func TestRuleLoader(t *testing.T) { t.Parallel() t.Log("Test rules loader") @@ -61,6 +62,39 @@ func TestRuleLoader(t *testing.T) { testDurationChange(t, l) } +func TestRuleLoaderInvalidRegexp(t *testing.T) { + t.Parallel() + t.Log("Test rules loader: invalid regexp") + + l, err := NewLoader(true) + if err != nil { + t.Fail() + } + t.Run("loadRule() from disk test (simple)", func(t *testing.T) { + if err := l.loadRule("testdata/invalid-regexp.json"); err == nil { + t.Error("invalid regexp rule loaded: loadRule()") + } + }) + + t.Run("loadRule() from disk test (list)", func(t *testing.T) { + if err := l.loadRule("testdata/invalid-regexp-list.json"); err == nil { + t.Error("invalid regexp rule loaded: loadRule()") + } + }) + + var list []Operator + dur30m := Duration("30m") + opListData := `[{"type": "regexp", "operand": "process.path", "sensitive": false, "data": "^(/di(rmngr)$"}, {"type": "simple", "operand": "dest.port", "data": "53", "sensitive": false}]` + invalidRegexpOp, _ := NewOperator(List, false, OpList, opListData, list) + invalidRegexpRule := Create("invalid-regexp", true, false, Allow, dur30m, invalidRegexpOp) + + t.Run("replaceUserRule() test list", func(t *testing.T) { + if err := l.replaceUserRule(invalidRegexpRule); err == nil { + t.Error("invalid regexp rule loaded: replaceUserRule()") + } + }) +} + func TestLiveReload(t *testing.T) { t.Parallel() t.Log("Test rules loader with live reload") diff --git a/daemon/rule/operator_test.go b/daemon/rule/operator_test.go index f16ac505f5..92fd814ce9 100644 --- a/daemon/rule/operator_test.go +++ b/daemon/rule/operator_test.go @@ -276,6 +276,23 @@ func TestNewOperatorRegexp(t *testing.T) { restoreConnection() } +func TestNewOperatorInvalidRegexp(t *testing.T) { + t.Log("Test NewOperator() invalid regexp") + var dummyList []Operator + + opRE, err := NewOperator(Regexp, false, OpProto, "^TC(P$", dummyList) + if err != nil { + t.Error("NewOperator regexp.err should be nil: ", err) + t.Fail() + } + if err = opRE.Compile(); err == nil { + t.Error("NewOperator() invalid regexp. It should fail: ", err) + t.Fail() + } + + restoreConnection() +} + func TestNewOperatorRegexpSensitive(t *testing.T) { t.Log("Test NewOperator() regexp sensitive") var dummyList []Operator diff --git a/daemon/rule/testdata/invalid-regexp-list.json b/daemon/rule/testdata/invalid-regexp-list.json new file mode 100644 index 0000000000..bd8973f2bc --- /dev/null +++ b/daemon/rule/testdata/invalid-regexp-list.json @@ -0,0 +1,31 @@ +{ + "created": "2020-12-13T18:06:52.209804547+01:00", + "updated": "2020-12-13T18:06:52.209857713+01:00", + "name": "invalid-regexp-list", + "enabled": true, + "precedence": true, + "action": "allow", + "duration": "always", + "operator": { + "type": "list", + "operand": "list", + "sensitive": false, + "data": "[{\"type\": \"regexp\", \"operand\": \"process.path\", \"sensitive\": false, \"data\": \"^(/di(rmngr$\"}, {\"type\": \"simple\", \"operand\": \"dest.port\", \"data\": \"53\", \"sensitive\": false}]", + "list": [ + { + "type": "regexp", + "operand": "process.path", + "sensitive": false, + "data": "^(/di(rmngr)$", + "list": null + }, + { + "type": "simple", + "operand": "dest.port", + "sensitive": false, + "data": "53", + "list": null + } + ] + } +} diff --git a/daemon/rule/testdata/invalid-regexp.json b/daemon/rule/testdata/invalid-regexp.json new file mode 100644 index 0000000000..d296098b90 --- /dev/null +++ b/daemon/rule/testdata/invalid-regexp.json @@ -0,0 +1,16 @@ +{ + "created": "2020-12-13T18:06:52.209804547+01:00", + "updated": "2020-12-13T18:06:52.209857713+01:00", + "name": "invalid-regexp", + "enabled": true, + "precedence": true, + "action": "allow", + "duration": "always", + "operator": { + "type": "regexp", + "operand": "process.path", + "sensitive": false, + "data": "/opt/((.*)google/chrome/chrome", + "list": [] + } +}