Skip to content

Commit

Permalink
Add tests catchall order (#355)
Browse files Browse the repository at this point in the history
* Only check whole rule names when matching counts

Tweak the regex so a rule my_great_rule doesn't pick up event counts for
a rule "great_rule: nnn".

* Add ability to skip evttype warnings for rules

A new attribute warn_evttypes, if present, suppresses printing warnings
related to a rule not matching any event type. Useful if you have a rule
where not including an event type is intentional.

* Add test for preserving rule order

Test the fix for #354. A rules
file has a event-specific rule first and a catchall rule second. Without
the changes in draios/sysdig#1103, the first
rule does not match the event.
  • Loading branch information
mstemm authored Apr 19, 2018
1 parent b6b490e commit e922a84
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 16 deletions.
2 changes: 1 addition & 1 deletion test/falco_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ def check_detections_by_rule(self, res):
triggered_rules = match.group(1)

for rule, count in self.detect_counts.iteritems():
expected = '{}: (\d+)'.format(rule)
expected = '\s{}: (\d+)'.format(rule)
match = re.search(expected, triggered_rules)

if match is None:
Expand Down
10 changes: 10 additions & 0 deletions test/falco_tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -699,3 +699,13 @@ trace_files: !mux
- detect_madvise: 2
- detect_open: 2
trace_file: trace_files/syscall.scap

catchall_order:
detect: True
detect_level: INFO
rules_file:
- rules/catchall_order.yaml
detect_counts:
- open_dev_null: 1
dev_null: 0
trace_file: trace_files/cat_write.scap
12 changes: 12 additions & 0 deletions test/rules/catchall_order.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
- rule: open_dev_null
desc: Any open of the file /dev/null
condition: evt.type=open and fd.name=/dev/null
output: An open of /dev/null was seen (command=%proc.cmdline evt=%evt.type %evt.args)
priority: INFO

- rule: dev_null
desc: Anything related to /dev/null
condition: fd.name=/dev/null
output: Something related to /dev/null was seen (command=%proc.cmdline evt=%evt.type %evt.args)
priority: INFO
warn_evttypes: false
32 changes: 18 additions & 14 deletions userspace/engine/lua/compiler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ end
-- run for all event types/syscalls. (Also, a warning is printed).
--

function get_evttypes_syscalls(name, ast, source)
function get_evttypes_syscalls(name, ast, source, warn_evttypes)

local evttypes = {}
local syscallnums = {}
Expand Down Expand Up @@ -276,23 +276,27 @@ function get_evttypes_syscalls(name, ast, source)
parser.traverse_ast(ast.filter.value, {BinaryRelOp=1, UnaryBoolOp=1} , cb)

if not found_event then
io.stderr:write("Rule "..name..": warning (no-evttype):\n")
io.stderr:write(source.."\n")
io.stderr:write(" did not contain any evt.type restriction, meaning it will run for all event types.\n")
io.stderr:write(" This has a significant performance penalty. Consider adding an evt.type restriction if possible.\n")
if warn_evttypes == true then
io.stderr:write("Rule "..name..": warning (no-evttype):\n")
io.stderr:write(source.."\n")
io.stderr:write(" did not contain any evt.type restriction, meaning it will run for all event types.\n")
io.stderr:write(" This has a significant performance penalty. Consider adding an evt.type restriction if possible.\n")
end
evttypes = {}
syscallnums = {}
evtnames = {}
end

if found_event_after_not then
io.stderr:write("Rule "..name..": warning (trailing-evttype):\n")
io.stderr:write(source.."\n")
io.stderr:write(" does not have all evt.type restrictions at the beginning of the condition,\n")
io.stderr:write(" or uses a negative match (i.e. \"not\"/\"!=\") for some evt.type restriction.\n")
io.stderr:write(" This has a performance penalty, as the rule can not be limited to specific event types.\n")
io.stderr:write(" Consider moving all evt.type restrictions to the beginning of the rule and/or\n")
io.stderr:write(" replacing negative matches with positive matches if possible.\n")
if warn_evttypes == true then
io.stderr:write("Rule "..name..": warning (trailing-evttype):\n")
io.stderr:write(source.."\n")
io.stderr:write(" does not have all evt.type restrictions at the beginning of the condition,\n")
io.stderr:write(" or uses a negative match (i.e. \"not\"/\"!=\") for some evt.type restriction.\n")
io.stderr:write(" This has a performance penalty, as the rule can not be limited to specific event types.\n")
io.stderr:write(" Consider moving all evt.type restrictions to the beginning of the rule and/or\n")
io.stderr:write(" replacing negative matches with positive matches if possible.\n")
end
evttypes = {}
syscallnums = {}
evtnames = {}
Expand Down Expand Up @@ -375,7 +379,7 @@ end
--[[
Parses a single filter, then expands macros using passed-in table of definitions. Returns resulting AST.
--]]
function compiler.compile_filter(name, source, macro_defs, list_defs)
function compiler.compile_filter(name, source, macro_defs, list_defs, warn_evttypes)

source = compiler.expand_lists_in(source, list_defs)

Expand All @@ -402,7 +406,7 @@ function compiler.compile_filter(name, source, macro_defs, list_defs)
error("Unexpected top-level AST type: "..ast.type)
end

evttypes, syscallnums = get_evttypes_syscalls(name, ast, source)
evttypes, syscallnums = get_evttypes_syscalls(name, ast, source, warn_evttypes)

return ast, evttypes, syscallnums
end
Expand Down
8 changes: 7 additions & 1 deletion userspace/engine/lua/rule_loader.lua
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,14 @@ function load_rules(rules_content, rules_mgr, verbose, all_events, extra, replac

local v = state.rules_by_name[name]

warn_evttypes = true
if v['warn_evttypes'] ~= nil then
warn_evttypes = v['warn_evttypes']
end

local filter_ast, evttypes, syscallnums = compiler.compile_filter(v['rule'], v['condition'],
state.macros, state.lists)
state.macros, state.lists,
warn_evttypes)

if (filter_ast.type == "Rule") then
state.n_rules = state.n_rules + 1
Expand Down

0 comments on commit e922a84

Please sign in to comment.