From 512a36dfe18ddb81d7775ce9def180abb16a5c73 Mon Sep 17 00:00:00 2001 From: Mark Stemm Date: Thu, 3 May 2018 14:24:32 -0700 Subject: [PATCH] Conditional rules (#364) * Add ability to skip rules for unknown filters Add the ability to skip a rule if its condition refers to a filtercheck that doesn't exist. This allows defining a rules file that contains new conditions that can still has limited backward compatibility with older falco versions. When compiling a filter, return a list of filtercheck names that are present in the ast (which also includes filterchecks from any macros). This set of filtercheck names is matched against the set of filterchecks known to sinsp, expressed as lua patterns, and in the global table defined_filters. If no match is found, the rule loader throws an error. The pattern changes slightly depending on whether the filter has arguments or not. Two filters (proc.apid/proc.aname) can work with or without arguments, so both styles of patterns are used. If the rule has an attribute "skip-if-unknown-filter", the rule will be skipped instead. * Unit tests for skipping unknown filter New unit test for skipping unknown filter. Test cases: - A rule that refers to an unknown filter results in an error. - A rule that refers to an unknown filter, but has "skip-if-unknown-filter: true", can be read, but doesn't match any events. - A rule that refers to an unknown filter, but has "skip-if-unknown-filter: false", returns an error. Also test the case of a filtercheck like evt.arg.xxx working properly with the embedded patterns as well as proc.aname/apid which work both ways. --- test/falco_tests.yaml | 27 +++++++++++++ test/rules/skip_unknown_error.yaml | 6 +++ test/rules/skip_unknown_evt.yaml | 6 +++ test/rules/skip_unknown_prefix.yaml | 8 ++++ test/rules/skip_unknown_unspec.yaml | 5 +++ userspace/engine/lua/compiler.lua | 19 +++++++++- userspace/engine/lua/rule_loader.lua | 39 +++++++++++++++++-- userspace/engine/rules.cpp | 57 ++++++++++++++++++++++++++++ userspace/engine/rules.h | 1 + 9 files changed, 164 insertions(+), 4 deletions(-) create mode 100644 test/rules/skip_unknown_error.yaml create mode 100644 test/rules/skip_unknown_evt.yaml create mode 100644 test/rules/skip_unknown_prefix.yaml create mode 100644 test/rules/skip_unknown_unspec.yaml diff --git a/test/falco_tests.yaml b/test/falco_tests.yaml index 6c5afd1460e..62d681cda95 100644 --- a/test/falco_tests.yaml +++ b/test/falco_tests.yaml @@ -713,3 +713,30 @@ trace_files: !mux - open_dev_null: 1 dev_null: 0 trace_file: trace_files/cat_write.scap + + skip_unknown_noevt: + detect: False + stdout_contains: Skipping rule "Contains Unknown Event And Skipping" that contains unknown filter proc.nobody + rules_file: + - rules/skip_unknown_evt.yaml + trace_file: trace_files/cat_write.scap + + skip_unknown_prefix: + detect: False + rules_file: + - rules/skip_unknown_prefix.yaml + trace_file: trace_files/cat_write.scap + + skip_unknown_error: + exit_status: 1 + stderr_contains: Rule "Contains Unknown Event And Not Skipping" contains unknown filter proc.nobody. Exiting. + rules_file: + - rules/skip_unknown_error.yaml + trace_file: trace_files/cat_write.scap + + skip_unknown_unspec_error: + exit_status: 1 + stderr_contains: Rule "Contains Unknown Event And Unspecified" contains unknown filter proc.nobody. Exiting. + rules_file: + - rules/skip_unknown_unspec.yaml + trace_file: trace_files/cat_write.scap diff --git a/test/rules/skip_unknown_error.yaml b/test/rules/skip_unknown_error.yaml new file mode 100644 index 00000000000..8d5a63cb373 --- /dev/null +++ b/test/rules/skip_unknown_error.yaml @@ -0,0 +1,6 @@ +- rule: Contains Unknown Event And Not Skipping + desc: Contains an unknown event + condition: proc.nobody=cat + output: Never + skip-if-unknown-filter: false + priority: INFO diff --git a/test/rules/skip_unknown_evt.yaml b/test/rules/skip_unknown_evt.yaml new file mode 100644 index 00000000000..46919ad3c4c --- /dev/null +++ b/test/rules/skip_unknown_evt.yaml @@ -0,0 +1,6 @@ +- rule: Contains Unknown Event And Skipping + desc: Contains an unknown event + condition: evt.type=open and proc.nobody=cat + output: Never + skip-if-unknown-filter: true + priority: INFO \ No newline at end of file diff --git a/test/rules/skip_unknown_prefix.yaml b/test/rules/skip_unknown_prefix.yaml new file mode 100644 index 00000000000..bee6eaea8c1 --- /dev/null +++ b/test/rules/skip_unknown_prefix.yaml @@ -0,0 +1,8 @@ +- rule: Contains Prefix of Filter + desc: Testing matching filter prefixes + condition: > + evt.type=open and evt.arg.path="foo" and evt.arg[0]="foo" + and proc.aname="ls" and proc.aname[1]="ls" + and proc.apid=10 and proc.apid[1]=10 + output: Never + priority: INFO \ No newline at end of file diff --git a/test/rules/skip_unknown_unspec.yaml b/test/rules/skip_unknown_unspec.yaml new file mode 100644 index 00000000000..50a5240314e --- /dev/null +++ b/test/rules/skip_unknown_unspec.yaml @@ -0,0 +1,5 @@ +- rule: Contains Unknown Event And Unspecified + desc: Contains an unknown event + condition: proc.nobody=cat + output: Never + priority: INFO diff --git a/userspace/engine/lua/compiler.lua b/userspace/engine/lua/compiler.lua index 0ce49ef28cf..da898c00693 100644 --- a/userspace/engine/lua/compiler.lua +++ b/userspace/engine/lua/compiler.lua @@ -322,6 +322,21 @@ function get_evttypes_syscalls(name, ast, source, warn_evttypes) return evttypes, syscallnums end +function get_filters(ast) + + local filters = {} + + function cb(node) + if node.type == "FieldName" then + filters[node.value] = 1 + end + end + + parser.traverse_ast(ast.filter.value, {FieldName=1} , cb) + + return filters +end + function compiler.expand_lists_in(source, list_defs) for name, def in pairs(list_defs) do @@ -408,7 +423,9 @@ function compiler.compile_filter(name, source, macro_defs, list_defs, warn_evtty evttypes, syscallnums = get_evttypes_syscalls(name, ast, source, warn_evttypes) - return ast, evttypes, syscallnums + filters = get_filters(ast) + + return ast, evttypes, syscallnums, filters end diff --git a/userspace/engine/lua/rule_loader.lua b/userspace/engine/lua/rule_loader.lua index 292e32cdf95..a5ba9b1e01b 100644 --- a/userspace/engine/lua/rule_loader.lua +++ b/userspace/engine/lua/rule_loader.lua @@ -275,6 +275,12 @@ function load_rules(rules_content, rules_mgr, verbose, all_events, extra, replac error ("Missing name in rule") end + -- By default, if a rule's condition refers to an unknown + -- filter like evt.type, etc the loader throws an error. + if v['skip-if-unknown-filter'] == nil then + v['skip-if-unknown-filter'] = false + end + -- Possibly append to the condition field of an existing rule append = false @@ -378,9 +384,34 @@ function load_rules(rules_content, rules_mgr, verbose, all_events, extra, replac warn_evttypes = v['warn_evttypes'] end - local filter_ast, evttypes, syscallnums = compiler.compile_filter(v['rule'], v['condition'], - state.macros, state.lists, - warn_evttypes) + local filter_ast, evttypes, syscallnums, filters = compiler.compile_filter(v['rule'], v['condition'], + state.macros, state.lists, + warn_evttypes) + + -- If a filter in the rule doesn't exist, either skip the rule + -- or raise an error, depending on the value of + -- skip-if-unknown-filter. + for filter, _ in pairs(filters) do + found = false + + for pat, _ in pairs(defined_filters) do + if string.match(filter, pat) ~= nil then + found = true + break + end + end + + if not found then + if v['skip-if-unknown-filter'] then + if verbose then + print("Skipping rule \""..v['rule'].."\" that contains unknown filter "..filter) + end + goto next_rule + else + error("Rule \""..v['rule'].."\" contains unknown filter "..filter) + end + end + end if (filter_ast.type == "Rule") then state.n_rules = state.n_rules + 1 @@ -452,6 +483,8 @@ function load_rules(rules_content, rules_mgr, verbose, all_events, extra, replac else error ("Unexpected type in load_rule: "..filter_ast.type) end + + ::next_rule:: end if verbose then diff --git a/userspace/engine/rules.cpp b/userspace/engine/rules.cpp index db7c2534384..0e29bba7762 100644 --- a/userspace/engine/rules.cpp +++ b/userspace/engine/rules.cpp @@ -258,6 +258,63 @@ void falco_rules::load_rules(const string &rules_content, lua_setglobal(m_ls, m_lua_ignored_syscalls.c_str()); + // Create a table containing all filtercheck names. + lua_newtable(m_ls); + + vector fc_plugins; + sinsp::get_filtercheck_fields_info(&fc_plugins); + + for(uint32_t j = 0; j < fc_plugins.size(); j++) + { + const filter_check_info* fci = fc_plugins[j]; + + if(fci->m_flags & filter_check_info::FL_HIDDEN) + { + continue; + } + + for(int32_t k = 0; k < fci->m_nfields; k++) + { + const filtercheck_field_info* fld = &fci->m_fields[k]; + + if(fld->m_flags & EPF_TABLE_ONLY || + fld->m_flags & EPF_PRINT_ONLY) + { + continue; + } + + // Some filters can work with or without an argument + std::set flexible_filters = { + "^proc.aname", + "^proc.apid" + }; + + std::list fields; + std::string field_base = string("^") + fld->m_name; + + if(fld->m_flags & EPF_REQUIRES_ARGUMENT || + flexible_filters.find(field_base) != flexible_filters.end()) + { + fields.push_back(field_base + "[%[%.]"); + } + + if(!(fld->m_flags & EPF_REQUIRES_ARGUMENT) || + flexible_filters.find(field_base) != flexible_filters.end()) + { + fields.push_back(field_base + "$"); + } + + for(auto &field : fields) + { + lua_pushstring(m_ls, field.c_str()); + lua_pushnumber(m_ls, 1); + lua_settable(m_ls, -3); + } + } + } + + lua_setglobal(m_ls, m_lua_defined_filters.c_str()); + lua_pushstring(m_ls, rules_content.c_str()); lua_pushlightuserdata(m_ls, this); lua_pushboolean(m_ls, (verbose ? 1 : 0)); diff --git a/userspace/engine/rules.h b/userspace/engine/rules.h index 75d7cb074d5..1c1f0b447a9 100644 --- a/userspace/engine/rules.h +++ b/userspace/engine/rules.h @@ -56,6 +56,7 @@ class falco_rules string m_lua_load_rules = "load_rules"; string m_lua_ignored_syscalls = "ignored_syscalls"; string m_lua_ignored_events = "ignored_events"; + string m_lua_defined_filters = "defined_filters"; string m_lua_events = "events"; string m_lua_syscalls = "syscalls"; string m_lua_describe_rule = "describe_rule";