Properly support syscalls in filter conditions (#352)

* Properly support syscalls in filter conditions

Syscalls have their own numbers but they weren't really handled within
falco.  This meant that there wasn't a way to handle filters with
evt.type=xxx clauses where xxx was a value that didn't have a
corresponding event entry (like "madvise", for examples), or where a
syscall like open could also be done indirectly via syscall(__NR_open,
...).

First, add a new top-level global syscalls that maps from a string like
"madvise" to all the syscall nums for that id, just as we do for event
names/numbers.

In the compiler, when traversing the AST for evt.type=XXX or evt.type in
(XXX, ...) clauses, also try to match XXX against the global syscalls
table, and return any ids in a standalone table.

Also throw an error if an XXX doesn't match any event name or syscall name.

The syscall numbers are passed as an argument to sinsp_evttype_filter so
it can preindex the filters by syscall number.

This depends on https://github.com/draios/sysdig/pull/1100

* Add unit test for syscall support

This does a madvise, which doesn't have a ppm event type, both directly
and indirectly via syscall(__NR_madvise, ...), as well as an open
directly + indirectly. The corresponding rules file matches on madvise
and open.

The test ensures that both opens and both madvises are detected.

test/falco_tests.yaml

@@ -689,3 +689,13 @@ trace_files: !mux
     rules_file:
       - rules/detect_connect_using_in.yaml
     trace_file: trace_files/connect_localhost.scap
+
+  syscalls:
+    detect: True
+    detect_level: INFO
+    rules_file:
+      - rules/syscalls.yaml
+    detect_counts:
+      - detect_madvise: 2
+      - detect_open: 2
+    trace_file: trace_files/syscall.scap

test/rules/syscalls.yaml

@@ -0,0 +1,11 @@
+- rule: detect_madvise
+  desc: Detect any call to madvise
+  condition: evt.type=madvise and evt.dir=<
+  output: A madvise syscall was seen (command=%proc.cmdline evt=%evt.type)
+  priority: INFO
+
+- rule: detect_open
+  desc: Detect any call to open
+  condition: evt.type=open and evt.dir=< and fd.name=/dev/null
+  output: An open syscall was seen (command=%proc.cmdline evt=%evt.type file=%fd.name)
+  priority: INFO

userspace/engine/falco_engine.cpp

@@ -162,6 +162,13 @@ void falco_engine::evttypes_for_ruleset(std::vector<bool> &evttypes, const std::
 	return m_evttype_filter->evttypes_for_ruleset(evttypes, ruleset_id);
 }
 
+void falco_engine::syscalls_for_ruleset(std::vector<bool> &syscalls, const std::string &ruleset)
+{
+	uint16_t ruleset_id = find_ruleset_id(ruleset);
+
+	return m_evttype_filter->syscalls_for_ruleset(syscalls, ruleset_id);
+}
+
 unique_ptr<falco_engine::rule_result> falco_engine::process_event(sinsp_evt *ev, uint16_t ruleset_id)
 {
 	if(should_drop_evt())
@@ -237,10 +244,11 @@ void falco_engine::print_stats()
 
 void falco_engine::add_evttype_filter(string &rule,
 				      set<uint32_t> &evttypes,
+				      set<uint32_t> &syscalls,
 				      set<string> &tags,
 				      sinsp_filter* filter)
 {
-	m_evttype_filter->add(rule, evttypes, tags, filter);
+	m_evttype_filter->add(rule, evttypes, syscalls, tags, filter);
 }
 
 void falco_engine::clear_filters()

userspace/engine/falco_engine.h

@@ -91,6 +91,12 @@ public:
 	//
 	void evttypes_for_ruleset(std::vector<bool> &evttypes, const std::string &ruleset);
 
+	//
+	// Given a ruleset, fill in a bitset containing the syscalls
+	// for which this ruleset can run.
+	//
+	void syscalls_for_ruleset(std::vector<bool> &syscalls, const std::string &ruleset);
+
 	//
 	// Given an event, check it against the set of rules in the
 	// engine and if a matching rule is found, return details on
@@ -122,10 +128,11 @@ public:
 
 	//
 	// Add a filter, which is related to the specified set of
-	// event types, to the engine.
+	// event types/syscalls, to the engine.
 	//
 	void add_evttype_filter(std::string &rule,
 				std::set<uint32_t> &evttypes,
+				std::set<uint32_t> &syscalls,
 				std::set<std::string> &tags,
 				sinsp_filter* filter);
 

userspace/engine/lua/compiler.lua

@@ -191,19 +191,20 @@ function check_for_ignored_syscalls_events(ast, filter_type, source)
    parser.traverse_ast(ast, {BinaryRelOp=1}, cb)
 end
 
--- Examine the ast and find the event types for which the rule should
+-- Examine the ast and find the event types/syscalls for which the
--- run. All evt.type references are added as event types up until the
+-- rule should run. All evt.type references are added as event types
--- first "!=" binary operator or unary not operator. If no event type
+-- up until the first "!=" binary operator or unary not operator. If
--- checks are found afterward in the rule, the rule is considered
+-- no event type checks are found afterward in the rule, the rule is
--- optimized and is associated with the event type(s).
+-- considered optimized and is associated with the event type(s).
 --
 -- Otherwise, the rule is associated with a 'catchall' category and is
--- run for all event types. (Also, a warning is printed).
+-- run for all event types/syscalls. (Also, a warning is printed).
 --
 
-function get_evttypes(name, ast, source)
+function get_evttypes_syscalls(name, ast, source)
 
    local evttypes = {}
+   local syscallnums = {}
    local evtnames = {}
    local found_event = false
    local found_not = false
@@ -226,17 +227,45 @@ function get_evttypes(name, ast, source)
 	    if node.operator == "in" or node.operator == "pmatch" then
 	       for i, v in ipairs(node.right.elements) do
 		  if v.type == "BareString" then
+
+                     -- The event must be a known event
+                     if events[v.value] == nil and syscalls[v.value] == nil then
+                        error("Unknown event/syscall \""..v.value.."\" in filter: "..source)
+                     end
+
 		     evtnames[v.value] = 1
-		     for id in string.gmatch(events[v.value], "%S+") do
+		     if events[v.value] ~= nil then
-			evttypes[id] = 1
+			for id in string.gmatch(events[v.value], "%S+") do
+			   evttypes[id] = 1
+			end
+		     end
+
+		     if syscalls[v.value] ~= nil then
+			for id in string.gmatch(syscalls[v.value], "%S+") do
+			   syscallnums[id] = 1
+			end
 		     end
 		  end
 	       end
 	    else
 	       if node.right.type == "BareString" then
+
+		  -- The event must be a known event
+		  if events[node.right.value] == nil and syscalls[node.right.value] == nil then
+		     error("Unknown event/syscall \""..node.right.value.."\" in filter: "..source)
+		  end
+
 		  evtnames[node.right.value] = 1
-		  for id in string.gmatch(events[node.right.value], "%S+") do
+		  if events[node.right.value] ~= nil then
-		     evttypes[id] = 1
+		     for id in string.gmatch(events[node.right.value], "%S+") do
+			evttypes[id] = 1
+		     end
+		  end
+
+		  if syscalls[node.right.value] ~= nil then
+		     for id in string.gmatch(syscalls[node.right.value], "%S+") do
+			syscallnums[id] = 1
+		     end
 		  end
 	       end
 	    end
@@ -252,6 +281,7 @@ function get_evttypes(name, ast, source)
       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")
       evttypes = {}
+      syscallnums = {}
       evtnames = {}
    end
 
@@ -264,6 +294,7 @@ function get_evttypes(name, ast, source)
       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")
       evttypes = {}
+      syscallnums = {}
       evtnames = {}
    end
 
@@ -281,10 +312,10 @@ function get_evttypes(name, ast, source)
    table.sort(evtnames_only)
 
    if compiler.verbose then
-      io.stderr:write("Event types for rule "..name..": "..table.concat(evtnames_only, ",").."\n")
+      io.stderr:write("Event types/Syscalls for rule "..name..": "..table.concat(evtnames_only, ",").."\n")
    end
 
-   return evttypes
+   return evttypes, syscallnums
 end
 
 function compiler.expand_lists_in(source, list_defs)
@@ -371,9 +402,9 @@ function compiler.compile_filter(name, source, macro_defs, list_defs)
       error("Unexpected top-level AST type: "..ast.type)
    end
 
-   evttypes = get_evttypes(name, ast, source)
+   evttypes, syscallnums = get_evttypes_syscalls(name, ast, source)
 
-   return ast, evttypes
+   return ast, evttypes, syscallnums
 end
 
 

userspace/engine/lua/rule_loader.lua

@@ -373,8 +373,8 @@ function load_rules(rules_content, rules_mgr, verbose, all_events, extra, replac
 
       local v = state.rules_by_name[name]
 
-      local filter_ast, evttypes = compiler.compile_filter(v['rule'], v['condition'],
+      local filter_ast, evttypes, syscallnums = compiler.compile_filter(v['rule'], v['condition'],
-							   state.macros, state.lists)
+								     state.macros, state.lists)
 
       if (filter_ast.type == "Rule") then
 	 state.n_rules = state.n_rules + 1
@@ -395,7 +395,7 @@ function load_rules(rules_content, rules_mgr, verbose, all_events, extra, replac
 	 end
 
 	 -- Pass the filter and event types back up
-	 falco_rules.add_filter(rules_mgr, v['rule'], evttypes, v['tags'])
+	 falco_rules.add_filter(rules_mgr, v['rule'], evttypes, syscallnums, v['tags'])
 
 	 -- Rule ASTs are merged together into one big AST, with "OR" between each
 	 -- rule.

userspace/engine/rules.cpp

@@ -66,8 +66,9 @@ void falco_rules::clear_filters()
 
 int falco_rules::add_filter(lua_State *ls)
 {
-	if (! lua_islightuserdata(ls, -4) ||
+	if (! lua_islightuserdata(ls, -5) ||
-	    ! lua_isstring(ls, -3) ||
+	    ! lua_isstring(ls, -4) ||
+	    ! lua_istable(ls, -3) ||
 	    ! lua_istable(ls, -2) ||
 	    ! lua_istable(ls, -1))
 	{
@@ -75,16 +76,28 @@ int falco_rules::add_filter(lua_State *ls)
 		lua_error(ls);
 	}
 
-	falco_rules *rules = (falco_rules *) lua_topointer(ls, -4);
+	falco_rules *rules = (falco_rules *) lua_topointer(ls, -5);
-	const char *rulec = lua_tostring(ls, -3);
+	const char *rulec = lua_tostring(ls, -4);
 
 	set<uint32_t> evttypes;
 
+	lua_pushnil(ls);  /* first key */
+	while (lua_next(ls, -4) != 0) {
+                // key is at index -2, value is at index
+                // -1. We want the keys.
+		evttypes.insert(luaL_checknumber(ls, -2));
+
+		// Remove value, keep key for next iteration
+		lua_pop(ls, 1);
+	}
+
+	set<uint32_t> syscalls;
+
 	lua_pushnil(ls);  /* first key */
 	while (lua_next(ls, -3) != 0) {
                 // key is at index -2, value is at index
                 // -1. We want the keys.
-		evttypes.insert(luaL_checknumber(ls, -2));
+		syscalls.insert(luaL_checknumber(ls, -2));
 
 		// Remove value, keep key for next iteration
 		lua_pop(ls, 1);
@@ -95,7 +108,7 @@ int falco_rules::add_filter(lua_State *ls)
 	lua_pushnil(ls);  /* first key */
 	while (lua_next(ls, -2) != 0) {
                 // key is at index -2, value is at index
-                // -1. We want the keys.
+                // -1. We want the values.
 		tags.insert(lua_tostring(ls, -1));
 
 		// Remove value, keep key for next iteration
@@ -103,19 +116,19 @@ int falco_rules::add_filter(lua_State *ls)
 	}
 
 	std::string rule = rulec;
-	rules->add_filter(rule, evttypes, tags);
+	rules->add_filter(rule, evttypes, syscalls, tags);
 
 	return 0;
 }
 
-void falco_rules::add_filter(string &rule, set<uint32_t> &evttypes, set<string> &tags)
+void falco_rules::add_filter(string &rule, set<uint32_t> &evttypes, set<uint32_t> &syscalls, set<string> &tags)
 {
 	// While the current rule was being parsed, a sinsp_filter
 	// object was being populated by lua_parser. Grab that filter
 	// and pass it to the engine.
 	sinsp_filter *filter = m_lua_parser->get_filter(true);
 
-	m_engine->add_evttype_filter(rule, evttypes, tags, filter);
+	m_engine->add_evttype_filter(rule, evttypes, syscalls, tags, filter);
 }
 
 int falco_rules::enable_rule(lua_State *ls)
@@ -183,6 +196,35 @@ void falco_rules::load_rules(const string &rules_content,
 
 		lua_setglobal(m_ls, m_lua_events.c_str());
 
+		map<string,string> syscalls_by_name;
+		for(uint32_t j = 0; j < PPM_SC_MAX; j++)
+		{
+			auto it = syscalls_by_name.find(stable[j].name);
+
+			if (it == syscalls_by_name.end())
+			{
+				syscalls_by_name[stable[j].name] = to_string(j);
+			}
+			else
+			{
+				string cur = it->second;
+				cur += " ";
+				cur += to_string(j);
+				syscalls_by_name[stable[j].name] = cur;
+			}
+		}
+
+		lua_newtable(m_ls);
+
+		for( auto kv : syscalls_by_name)
+		{
+			lua_pushstring(m_ls, kv.first.c_str());
+			lua_pushstring(m_ls, kv.second.c_str());
+			lua_settable(m_ls, -3);
+		}
+
+		lua_setglobal(m_ls, m_lua_syscalls.c_str());
+
 		// Create a table containing the syscalls/events that
 		// are ignored by the kernel module. load_rules will
 		// return an error if any rule references one of these

userspace/engine/rules.h

@@ -45,7 +45,7 @@ class falco_rules
 
  private:
 	void clear_filters();
-	void add_filter(string &rule, std::set<uint32_t> &evttypes, std::set<string> &tags);
+	void add_filter(string &rule, std::set<uint32_t> &evttypes, std::set<uint32_t> &syscalls, std::set<string> &tags);
 	void enable_rule(string &rule, bool enabled);
 
 	lua_parser* m_lua_parser;
@@ -57,5 +57,6 @@ class falco_rules
 	string m_lua_ignored_syscalls = "ignored_syscalls";
 	string m_lua_ignored_events = "ignored_events";
 	string m_lua_events = "events";
+	string m_lua_syscalls = "syscalls";
 	string m_lua_describe_rule = "describe_rule";
 };