From dd3d235d7fa9ab071c2c0416197512049c4caf8e Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Wed, 6 Apr 2022 08:07:26 +0000 Subject: [PATCH] refactor(tests): adapting test_rulesets to new method signatures At the same time, this also simplifies the unit test cases by using the SCENARIO construct of catch2, which allows sharing a setup phases between different unit tests, and removes a bunch of repeated LOC in our case. Signed-off-by: Jason Dellaluce --- tests/engine/test_rulesets.cpp | 340 +++++++++++++-------------------- userspace/engine/ruleset.cpp | 17 +- userspace/engine/ruleset.h | 13 +- 3 files changed, 142 insertions(+), 228 deletions(-) diff --git a/tests/engine/test_rulesets.cpp b/tests/engine/test_rulesets.cpp index e4d6b9d5..ca03c5d0 100644 --- a/tests/engine/test_rulesets.cpp +++ b/tests/engine/test_rulesets.cpp @@ -25,6 +25,7 @@ static uint16_t default_ruleset = 0; static uint16_t non_default_ruleset = 3; static uint16_t other_non_default_ruleset = 2; static std::set tags = {"some_tag", "some_other_tag"}; +static std::set evttypes = { ppm_event_type::PPME_GENERIC_E }; static std::shared_ptr create_filter() { @@ -37,234 +38,159 @@ static std::shared_ptr create_filter() return ret; } -TEST_CASE("Should enable/disable for exact match w/ default ruleset", "[rulesets]") +TEST_CASE("Should enable/disable on ruleset", "[rulesets]") { falco_ruleset r; std::shared_ptr filter = create_filter(); string rule_name = "one_rule"; string source = "syscall"; - r.add(source, rule_name, tags, filter); + r.add(source, rule_name, tags, evttypes, filter); - r.enable("one_rule", exact_match, enabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); + SECTION("Should enable/disable for exact match w/ default ruleset") + { + r.enable("one_rule", exact_match, enabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); + + r.enable("one_rule", exact_match, disabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + } + + SECTION("Should enable/disable for exact match w/ specific ruleset") + { + r.enable("one_rule", exact_match, enabled, non_default_ruleset); + REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 1); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); + + r.enable("one_rule", exact_match, disabled, non_default_ruleset); + REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 0); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); + } + + SECTION("Should not enable for exact match different rule name") + { + r.enable("some_other_rule", exact_match, enabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + } + + SECTION("Should enable/disable for exact match w/ substring and default ruleset") + { + r.enable("one_rule", substring_match, enabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); + + r.enable("one_rule", substring_match, disabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + } + + SECTION("Should not enable for substring w/ exact_match") + { + r.enable("one_", exact_match, enabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + } + + SECTION("Should enable/disable for prefix match w/ default ruleset") + { + r.enable("one_", substring_match, enabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); + + r.enable("one_", substring_match, disabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + } + + SECTION("Should enable/disable for suffix match w/ default ruleset") + { + r.enable("_rule", substring_match, enabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); + + r.enable("_rule", substring_match, disabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + } + + SECTION("Should enable/disable for substring match w/ default ruleset") + { + r.enable("ne_ru", substring_match, enabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); + + r.enable("ne_ru", substring_match, disabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + } + + SECTION("Should enable/disable for substring match w/ specific ruleset") + { + r.enable("ne_ru", substring_match, enabled, non_default_ruleset); + REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 1); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); + + r.enable("ne_ru", substring_match, disabled, non_default_ruleset); + REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 0); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); + } + + SECTION("Should enable/disable for tags w/ default ruleset") + { + std::set want_tags = {"some_tag"}; + + r.enable_tags(want_tags, enabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); + + r.enable_tags(want_tags, disabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + } + + SECTION("Should enable/disable for tags w/ specific ruleset") + { + std::set want_tags = {"some_tag"}; + + r.enable_tags(want_tags, enabled, non_default_ruleset); + REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 1); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); + + r.enable_tags(want_tags, disabled, non_default_ruleset); + REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 0); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); + } + + SECTION("Should not enable for different tags") + { + std::set want_tags = {"some_different_tag"}; + + r.enable_tags(want_tags, enabled); + REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 0); + } + + SECTION("Should enable/disable for overlapping tags") + { + std::set want_tags = {"some_tag", "some_different_tag"}; + + r.enable_tags(want_tags, enabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); + + r.enable_tags(want_tags, disabled); + REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); + } - r.enable("one_rule", exact_match, disabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); } -TEST_CASE("Should enable/disable for exact match w/ specific ruleset", "[rulesets]") +TEST_CASE("Should enable/disable on ruleset for incremental adding tags", "[rulesets]") { falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; string source = "syscall"; - r.add(source, rule_name, tags, filter); - - r.enable("one_rule", exact_match, enabled, non_default_ruleset); - REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 1); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); - REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); - - r.enable("one_rule", exact_match, disabled, non_default_ruleset); - REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 0); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); - REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); -} - -TEST_CASE("Should not enable for exact match different rule name", "[rulesets]") -{ - falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; - string source = "syscall"; - - r.add(source, rule_name, tags, filter); - - r.enable("some_other_rule", exact_match, enabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); -} - -TEST_CASE("Should enable/disable for exact match w/ substring and default ruleset", "[rulesets]") -{ - falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; - string source = "syscall"; - - r.add(source, rule_name, tags, filter); - - r.enable("one_rule", substring_match, enabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); - - r.enable("one_rule", substring_match, disabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); -} - -TEST_CASE("Should not enable for substring w/ exact_match", "[rulesets]") -{ - falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; - string source = "syscall"; - - r.add(source, rule_name, tags, filter); - - r.enable("one_", exact_match, enabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); -} - -TEST_CASE("Should enable/disable for prefix match w/ default ruleset", "[rulesets]") -{ - falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; - string source = "syscall"; - - r.add(source, rule_name, tags, filter); - - r.enable("one_", substring_match, enabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); - - r.enable("one_", substring_match, disabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); -} - -TEST_CASE("Should enable/disable for suffix match w/ default ruleset", "[rulesets]") -{ - falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; - string source = "syscall"; - - r.add(source, rule_name, tags, filter); - - r.enable("_rule", substring_match, enabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); - - r.enable("_rule", substring_match, disabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); -} - -TEST_CASE("Should enable/disable for substring match w/ default ruleset", "[rulesets]") -{ - falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; - string source = "syscall"; - - r.add(source, rule_name, tags, filter); - - r.enable("ne_ru", substring_match, enabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); - - r.enable("ne_ru", substring_match, disabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); -} - -TEST_CASE("Should enable/disable for substring match w/ specific ruleset", "[rulesets]") -{ - falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; - string source = "syscall"; - - r.add(source, rule_name, tags, filter); - - r.enable("ne_ru", substring_match, enabled, non_default_ruleset); - REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 1); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); - REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); - - r.enable("ne_ru", substring_match, disabled, non_default_ruleset); - REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 0); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); - REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); -} - -TEST_CASE("Should enable/disable for tags w/ default ruleset", "[rulesets]") -{ - falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; - string source = "syscall"; - std::set want_tags = {"some_tag"}; - - r.add(source, rule_name, tags, filter); - - r.enable_tags(want_tags, enabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); - - r.enable_tags(want_tags, disabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); -} - -TEST_CASE("Should enable/disable for tags w/ specific ruleset", "[rulesets]") -{ - falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; - string source = "syscall"; - std::set want_tags = {"some_tag"}; - - r.add(source, rule_name, tags, filter); - - r.enable_tags(want_tags, enabled, non_default_ruleset); - REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 1); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); - REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); - - r.enable_tags(want_tags, disabled, non_default_ruleset); - REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 0); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); - REQUIRE(r.num_rules_for_ruleset(other_non_default_ruleset) == 0); -} - -TEST_CASE("Should not enable for different tags", "[rulesets]") -{ - falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; - string source = "syscall"; - std::set want_tags = {"some_different_tag"}; - - r.add(source, rule_name, tags, filter); - - r.enable_tags(want_tags, enabled); - REQUIRE(r.num_rules_for_ruleset(non_default_ruleset) == 0); -} - -TEST_CASE("Should enable/disable for overlapping tags", "[rulesets]") -{ - falco_ruleset r; - std::shared_ptr filter = create_filter(); - string rule_name = "one_rule"; - string source = "syscall"; - std::set want_tags = {"some_tag", "some_different_tag"}; - - r.add(source, rule_name, tags, filter); - - r.enable_tags(want_tags, enabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 1); - - r.enable_tags(want_tags, disabled); - REQUIRE(r.num_rules_for_ruleset(default_ruleset) == 0); -} - -TEST_CASE("Should enable/disable for incremental adding tags", "[rulesets]") -{ - string source = "syscall"; - falco_ruleset r; std::shared_ptr rule1_filter = create_filter(); string rule1_name = "one_rule"; std::set rule1_tags = {"rule1_tag"}; - r.add(source, rule1_name, rule1_tags, rule1_filter); + r.add(source, rule1_name, rule1_tags, evttypes, rule1_filter); std::shared_ptr rule2_filter = create_filter(); string rule2_name = "two_rule"; std::set rule2_tags = {"rule2_tag"}; - r.add(source, rule2_name, rule2_tags, rule2_filter); + r.add(source, rule2_name, rule2_tags, evttypes, rule2_filter); std::set want_tags; diff --git a/userspace/engine/ruleset.cpp b/userspace/engine/ruleset.cpp index d3aad9e4..00b8838b 100644 --- a/userspace/engine/ruleset.cpp +++ b/userspace/engine/ruleset.cpp @@ -66,16 +66,14 @@ void falco_ruleset::ruleset_filters::remove_wrapper_from_list(filter_wrapper_lis void falco_ruleset::ruleset_filters::add_filter(std::shared_ptr wrap) { - std::set fevttypes = wrap->evttypes(); - - if(fevttypes.empty()) + if(wrap->evttypes.empty()) { // Should run for all event types add_wrapper_to_list(m_filter_all_event_types, wrap); } else { - for(auto &etype : fevttypes) + for(auto &etype : wrap->evttypes) { if(m_filter_by_event_type.size() <= etype) { @@ -91,15 +89,13 @@ void falco_ruleset::ruleset_filters::add_filter(std::shared_ptr void falco_ruleset::ruleset_filters::remove_filter(std::shared_ptr wrap) { - std::set fevttypes = wrap->evttypes(); - - if(fevttypes.empty()) + if(wrap->evttypes.empty()) { remove_wrapper_from_list(m_filter_all_event_types, wrap); } else { - for(auto &etype : fevttypes) + for(auto &etype : wrap->evttypes) { if( etype < m_filter_by_event_type.size() ) { @@ -147,14 +143,14 @@ void falco_ruleset::ruleset_filters::evttypes_for_ruleset(std::set &ev for(auto &wrap : m_filters) { - auto fevttypes = wrap->evttypes(); - evttypes.insert(fevttypes.begin(), fevttypes.end()); + evttypes.insert(wrap->evttypes.begin(), wrap->evttypes.end()); } } void falco_ruleset::add(string &source, string &name, set &tags, + set &evttypes, std::shared_ptr filter) { std::shared_ptr wrap(new filter_wrapper()); @@ -162,6 +158,7 @@ void falco_ruleset::add(string &source, wrap->name = name; wrap->tags = tags; wrap->filter = filter; + wrap->evttypes = evttypes; m_filters.insert(wrap); } diff --git a/userspace/engine/ruleset.h b/userspace/engine/ruleset.h index 1d31e85e..fe70a6c4 100644 --- a/userspace/engine/ruleset.h +++ b/userspace/engine/ruleset.h @@ -37,6 +37,7 @@ public: void add(string &source, std::string &name, std::set &tags, + set &evttypes, std::shared_ptr filter); // rulesets are arbitrary numbers and should be managed by the caller. @@ -77,18 +78,8 @@ private: std::string source; std::string name; std::set tags; + std::set evttypes; std::shared_ptr filter; - std::set evttypes() - { - // todo(jasondellaluce,leogr): temp workaround, remove when fixed in libs - if(source == "syscall" || source == "k8s_audit") - { - return filter->evttypes(); - } - // else assume plugins - return {ppm_event_type::PPME_PLUGINEVENT_E}; - // workaround end - } }; typedef std::list> filter_wrapper_list;