[closed in favor of a new follow up PR] cleanup/new/feat(load-rules): expand rules append support and feature partial overrides

unit_tests/engine/engine_helper.h

@@ -0,0 +1,53 @@
+/*
+Copyright (C) 2023 The Falco Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+*/
+
+#pragma once
+#include <gtest/gtest.h>
+
+// When updating unit_tests/falco_rules_test.yaml bump this
+#define N_VALID_TEST_RULES_FALCO_RULES_TEST_YAML 6
+
+#define ASSERT_CONTAINS(a, b)            \
+    {                                    \
+        auto a1 = a;                     \
+        auto b1 = b;                     \
+        uint32_t prev_size = a1.size();  \
+        for(const auto& val : b1)        \
+        {                                \
+            a1.insert(val);              \
+        }                                \
+        ASSERT_EQ(prev_size, a1.size()); \
+    }
+
+#define ASSERT_NOT_CONTAINS(a, b)                    \
+	{                                                \
+		auto a1 = a;                                 \
+		auto b1 = b;                                 \
+		uint32_t prev_size = a1.size();              \
+		for(const auto& val : b1)                    \
+		{                                            \
+			a1.insert(val);                          \
+		}                                            \
+		ASSERT_EQ(prev_size + b1.size(), a1.size()); \
+	}
+
+#define ASSERT_STRING_EQUAL(a, b)        \
+    {                                    \
+        auto a1 = a;                     \
+        auto b1 = b;                     \
+        ASSERT_EQ(a1.compare(b1), 0);    \
+    }

unit_tests/engine/test_rule_loader_reader.cpp

@@ -0,0 +1,110 @@
+/*
+Copyright (C) 2023 The Falco Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+
+*/
+
+#include <falco_engine.h>
+#include <falco/app/app.h>
+#include "engine_helper.h"
+#include <gtest/gtest.h>
+#include "../falco/app/actions/app_action_helpers.h"
+
+static std::shared_ptr<falco_engine> mock_engine()
+{
+	// Create a mock Falco engine
+	std::shared_ptr<falco_engine> engine(new falco_engine());
+	auto filter_factory = std::shared_ptr<gen_event_filter_factory>(
+			new sinsp_filter_factory(nullptr));
+	auto formatter_factory = std::shared_ptr<gen_event_formatter_factory>(
+			new sinsp_evt_formatter_factory(nullptr));
+	engine->add_source("syscall", filter_factory, formatter_factory);
+	return engine;
+}
+
+TEST(RuleLoaderReader, append_merge_override_enabled)
+{
+    falco::app::state s;
+    s.engine = mock_engine();
+    s.options.rules_filenames.push_back("../unit_tests/falco_rules_test1.yaml");
+
+    auto result = falco::app::actions::load_rules_files(s);
+    ASSERT_TRUE(result.success);
+
+    auto rules1 = s.engine->get_rules();
+    std::unordered_set<std::string> rules_names = {};
+    std::unordered_set<std::string> expected_rules_names = {"Dummy Rule 0", "Dummy Rule 1", \
+    "Dummy Rule 2", "Dummy Rule 4 Disabled", "Dummy Rule 5", "Dummy Rule 6"};
+    std::unordered_set<std::string> not_expected_rules_names = {"Dummy Rule 3 Invalid"};
+    ASSERT_EQ(rules1.size(), N_VALID_TEST_RULES_FALCO_RULES_TEST_YAML);
+
+    for(const auto& r : rules1)
+    {
+        rules_names.insert(r.name);
+        if (r.name.compare(std::string("Dummy Rule 0")) == 0)
+        {
+            // Test condition where we append to tags, cond and output
+            ASSERT_TRUE(r.enabled);
+            std::set<std::string> some_desired_tags = {"maturity_stable", "test1", "test2"};
+            ASSERT_CONTAINS(r.tags, some_desired_tags);
+            ASSERT_STRING_EQUAL(r.cond, std::string("evt.type in (execve, execveat) and proc.name=cat and proc.cmdline contains test"));
+            ASSERT_STRING_EQUAL(r.output, std::string("%evt.type %evt.num %proc.aname[5] %proc.name %proc.tty %proc.exepath %fd.name proc_exeline=%proc.exeline proc_exepath=%proc.exepath"));
+            ASSERT_EQ(r.priority, falco_common::priority_type::PRIORITY_CRITICAL);
+        }
+        else if (r.name.compare(std::string("Dummy Rule 1")) == 0)
+        {
+            // Test rules merging aka override only re-defined keys, else keep old keys
+            std::set<std::string> some_desired_tags = {"maturity_incubating", "host", "container"};  // ensure prev definition
+            ASSERT_STRING_EQUAL(r.desc, std::string("My test desc 1"));  // ensure prev definition
+            ASSERT_EQ(r.priority, falco_common::priority_type::PRIORITY_CRITICAL);  // ensure prev definition
+            ASSERT_CONTAINS(r.tags, some_desired_tags);
+            ASSERT_STRING_EQUAL(r.cond, std::string("evt.type in (ptrace)"));  // ensure new definition
+            ASSERT_STRING_EQUAL(r.output, std::string("%evt.type %evt.num"));  // ensure new definition
+            ASSERT_FALSE(r.enabled);  // ensure new definition
+        }
+        else if (r.name.compare(std::string("Dummy Rule 2")) == 0)
+        {
+            // Test where we have overridden a rule to ONLY NOT be enabled
+            ASSERT_EQ(r.priority, falco_common::priority_type::PRIORITY_NOTICE);
+            ASSERT_FALSE(r.enabled);
+        }
+        else if (r.name.compare(std::string("Dummy Rule 4 Disabled")) == 0)
+        {
+            // Test if entire rule defined just once is disabled
+            ASSERT_FALSE(r.enabled);
+        }
+        else if (r.name.compare(std::string("Dummy Rule 5")) == 0)
+        {
+            // Test if we correctly support append mode with override for enabled and priority
+            std::set<std::string> some_desired_tags = {"maturity_sandbox", "host", "container"};
+            ASSERT_TRUE(r.enabled); // ensure new definition
+            ASSERT_EQ(r.priority, falco_common::priority_type::PRIORITY_CRITICAL); // ensure new definition
+            ASSERT_STRING_EQUAL(r.cond, std::string("evt.type in (open, openat, openat2) and proc.name=cat"));  // ensure correct append
+            ASSERT_STRING_EQUAL(r.output, std::string("%evt.type %evt.num %proc.cmdline %container.ip")); // ensure correct append
+            ASSERT_CONTAINS(r.tags, some_desired_tags); // ensure correct append
+        }
+        else if (r.name.compare(std::string("Dummy Rule 6")) == 0)
+        {
+            // Test if we correctly support append mode when used only to override enabled and priority
+            std::set<std::string> some_desired_tags = {"maturity_sandbox", "host"};
+            ASSERT_STRING_EQUAL(r.cond, std::string("evt.type in (ptrace)"));  // ensure prev definition
+            ASSERT_STRING_EQUAL(r.output, std::string("%evt.type %proc.cmdline"));  // ensure prev definition
+            ASSERT_TRUE(r.enabled); // ensure new definition
+            ASSERT_EQ(r.priority, falco_common::priority_type::PRIORITY_CRITICAL); // ensure new definition
+            ASSERT_CONTAINS(r.tags, some_desired_tags); // ensure prev definition
+        }
+    }
+    ASSERT_CONTAINS(rules_names, expected_rules_names);
+    ASSERT_NOT_CONTAINS(rules_names, not_expected_rules_names);
+}

unit_tests/falco_rules_test1.yaml

@@ -0,0 +1,111 @@
+# Test ruleset 1
+
+- list: http_server_binaries
+  items: [nginx, httpd, httpd-foregroun, lighttpd, apache, apache2]
+
+- list: http_server_binaries
+  append: true
+  items: [test]
+
+- macro: interpreted_procs
+  condition: (proc.name in (test1, test2))
+
+- macro: interpreted_procs
+  append: true
+  condition: and evt.num > 0
+
+- rule: Dummy Rule 0
+  desc: My test desc 0
+  condition: evt.type in (execve, execveat)
+  enabled: true
+  output: '%evt.type %evt.num %proc.aname[5] %proc.name %proc.tty %proc.exepath %fd.name'
+  priority: CRITICAL
+  tags: [maturity_stable, host, container, network, mitre_lateral_movement, T1021.004]
+
+- rule: Dummy Rule 1
+  desc: My test desc 1
+  condition: evt.type in (open, openat, openat2)
+  enabled: true
+  output: '%evt.type %evt.num %proc.aname[5] %proc.name %proc.tty %proc.exepath %fd.name'
+  priority: CRITICAL
+  tags: [maturity_incubating, host, container]
+
+- rule: Dummy Rule 2
+  desc: My test desc 2
+  condition: evt.type in (mprotect, mmap)
+  enabled: true
+  output: '%evt.type %evt.num %proc.exepath'
+  priority: NOTICE
+  tags: [maturity_incubating, host, container]
+
+# Test invalid / incomplete rule
+- rule: Dummy Rule 3 Invalid
+  condition: evt.type in (execve, execveat) and proc.name=cat and proc.cmdline contains invalid
+
+- rule: Dummy Rule 4 Disabled
+  desc: My test desc 4
+  condition: evt.type in (mprotect, mmap)
+  enabled: false
+  output: '%evt.type %evt.num %proc.exepath'
+  priority: NOTICE
+  tags: [maturity_incubating, host, container]
+
+- rule: Dummy Rule 5
+  desc: My test desc 5
+  condition: evt.type in (open, openat, openat2)
+  enabled: false
+  output: '%evt.type %evt.num %proc.cmdline'
+  priority: INFORMATIONAL
+  tags: [maturity_sandbox]
+
+- rule: Dummy Rule 6
+  desc: My test desc 6
+  condition: evt.type in (ptrace)
+  enabled: false
+  output: '%evt.type %proc.cmdline'
+  priority: INFORMATIONAL
+  tags: [maturity_sandbox, host]
+
+# Test appending to rule
+- rule: Dummy Rule 0
+  append: true
+  condition: and proc.name=cat and proc.cmdline contains test
+  output: 'proc_exeline=%proc.exeline proc_exepath=%proc.exepath'
+  tags: [test1, test2]
+
+# Test merge / partial override
+- rule: Dummy Rule 1
+  condition: evt.type in (ptrace)
+  output: '%evt.type %evt.num'
+  enabled: false
+
+- rule: Dummy Rule 2
+  enabled: false
+
+- rule: Dummy Rule 2
+  enabled: true
+
+- rule: Dummy Rule 2
+  enabled: false
+
+# Test append for "appendable" fields `condition`, `output`, `tags`
+# + partial override for eligible fields `enabled` and `priority`
+- rule: Dummy Rule 5
+  append: true
+  desc: My test desc 5
+  condition: and proc.name=cat
+  enabled: true
+  priority: CRITICAL
+  tags: [maturity_sandbox, host, container]
+
+- rule: Dummy Rule 5
+  append: true
+  output: '%container.ip'
+
+- rule: Dummy Rule 6
+  append: true
+  priority: CRITICAL
+
+- rule: Dummy Rule 6
+  append: true
+  enabled: true

userspace/engine/falco_common.h

@@ -64,7 +64,8 @@ namespace falco_common
 		PRIORITY_WARNING = 4,
 		PRIORITY_NOTICE = 5,
 		PRIORITY_INFORMATIONAL = 6,
-		PRIORITY_DEBUG = 7
+		PRIORITY_DEBUG = 7,
+		PRIORITY_INVALID = 8,
 	};
 
 	bool parse_priority(std::string v, priority_type& out);

userspace/engine/falco_engine.h

@@ -231,6 +231,14 @@ class falco_engine
 				  std::set<uint16_t> &evttypes,
 				  const std::string &ruleset = s_default_ruleset);
 
+	//
+	// Return all rules form the rule collector.
+	//
+	inline indexed_vector<rule_loader::rule_info> get_rules() const
+	{
+		return m_rule_collector.rules();
+	}
+
 	//
 	// Given an event source and ruleset, return the set of ppm_sc_codes
 	// for which this ruleset can run and match events.

userspace/engine/rule_loader_collector.cpp

@@ -222,7 +222,41 @@ void rule_loader::collector::define(configuration& cfg, rule_info& info)
 		validate_exception_info(*source, ex);
 	}
 
-	define_info(m_rule_infos, info, m_cur_index++);
+	// Reconstruct prev info if no new info and only merge re-defined fields
+	if (prev)
+	{
+		if (info.desc.empty())
+		{
+			info.desc = prev->desc;
+		}
+		if (info.cond.empty())
+		{
+			info.cond = prev->cond;
+		}
+		if (info.output.empty())
+		{
+			info.output = prev->output;
+		}
+		if (info.tags.empty())
+		{
+			info.tags = prev->tags;
+		}
+		if (info.priority == falco_common::priority_type::PRIORITY_INVALID)
+		{
+			info.priority = prev->priority;
+		}
+	}
+
+	// Only add a valid rule that at least has the rule name plus
+	// desc, condition, output, and priority
+	if (!info.desc.empty() &&
+		!info.cond.empty() &&
+		!info.output.empty() &&
+		info.priority >= falco_common::priority_type::PRIORITY_EMERGENCY && info.priority < falco_common::priority_type::PRIORITY_INVALID
+		)
+	{
+		define_info(m_rule_infos, info, m_cur_index++);
+	}
 }
 
 void rule_loader::collector::append(configuration& cfg, rule_info& info)
@@ -232,22 +266,47 @@ void rule_loader::collector::append(configuration& cfg, rule_info& info)
 	THROW(!prev,
 	       "Rule has 'append' key but no rule by that name already exists",
 	       info.ctx);
-	THROW(info.cond.empty() && info.exceptions.empty(),
-	       "Appended rule must have exceptions or condition property",
-	       info.ctx);
 
 	auto source = cfg.sources.at(prev->source);
 	// note: this is not supposed to happen
 	THROW(!source,
 	      std::string("Unknown source ") + prev->source,
 	      info.ctx);
 
+	// enabled and priority are the cases where we allow override also when using append
+	// for better user experience given the introduction of the rules maturity framework
+	prev->enabled = info.enabled;
+
+	if (info.priority < falco_common::priority_type::PRIORITY_INVALID)
+	{
+		prev->priority = info.priority;
+	}
+
+	// Below fields are fields were we append items
+
 	if (!info.cond.empty())
 	{
 		prev->cond += " ";
 		prev->cond += info.cond;
 	}
 
+	if (!info.output.empty())
+	{
+		prev->output += " ";
+		prev->output += info.output;
+	}
+
+	if (!info.tags.empty())
+	{
+		for (auto itr : info.tags)
+		{
+			if (!itr.empty())
+			{
+				prev->tags.insert(itr);
+			}
+		}
+	}
+
 	for (auto &ex : info.exceptions)
 	{
 		auto prev_ex = find_if(prev->exceptions.begin(), prev->exceptions.end(),