From 2b058159f8bd2d9c6c4364415bfd6ad4d0a62546 Mon Sep 17 00:00:00 2001 From: Anil Mahtani <929854+Anilm3@users.noreply.github.com> Date: Tue, 18 Jun 2024 15:25:50 +0100 Subject: [PATCH] Prevent default rule tags from being overridden, add extra test --- src/event.cpp | 3 +++ src/rule.hpp | 32 ++++++++++------------- src/ruleset_builder.cpp | 8 ++++-- tests/interface_test.cpp | 56 +++++++++++++++++++++++++++++++++++----- 4 files changed, 73 insertions(+), 26 deletions(-) diff --git a/src/event.cpp b/src/event.cpp index 6de443950..c67233328 100644 --- a/src/event.cpp +++ b/src/event.cpp @@ -159,6 +159,9 @@ void serialize_rule(const ddwaf::rule &rule, ddwaf_object &rule_map) for (const auto &[key, value] : rule.get_tags()) { ddwaf_object_map_addl(&tags_map, key.c_str(), key.size(), to_object(tmp, value)); } + for (const auto &[key, value] : rule.get_ancillary_tags()) { + ddwaf_object_map_addl(&tags_map, key.c_str(), key.size(), to_object(tmp, value)); + } ddwaf_object_map_add(&rule_map, "tags", &tags_map); } diff --git a/src/rule.hpp b/src/rule.hpp index c085b278b..3f9b32cd1 100644 --- a/src/rule.hpp +++ b/src/rule.hpp @@ -42,23 +42,8 @@ class rule { rule(const rule &) = delete; rule &operator=(const rule &) = delete; - rule(rule &&rhs) noexcept - : enabled_(rhs.enabled_), source_(rhs.source_), id_(std::move(rhs.id_)), - name_(std::move(rhs.name_)), tags_(std::move(rhs.tags_)), expr_(std::move(rhs.expr_)), - actions_(std::move(rhs.actions_)) - {} - - rule &operator=(rule &&rhs) noexcept - { - enabled_ = rhs.enabled_; - source_ = rhs.source_; - id_ = std::move(rhs.id_); - name_ = std::move(rhs.name_); - tags_ = std::move(rhs.tags_); - expr_ = std::move(rhs.expr_); - actions_ = std::move(rhs.actions_); - return *this; - } + rule(rule &&rhs) noexcept = default; + rule &operator=(rule &&rhs) noexcept = default; virtual ~rule() = default; @@ -94,8 +79,18 @@ class rule { } const std::unordered_map &get_tags() const { return tags_; } + const std::unordered_map &get_ancillary_tags() const + { + return ancillary_tags_; + } - void set_tag(const std::string &key, const std::string &value) { tags_[key] = value; } + void set_ancillary_tag(const std::string &key, const std::string &value) + { + // Ancillary tags aren't allowed to overlap with standard tags + if (!tags_.contains(key)) { + ancillary_tags_[key] = value; + } + } const std::vector &get_actions() const { return actions_; } @@ -112,6 +107,7 @@ class rule { std::string id_; std::string name_; std::unordered_map tags_; + std::unordered_map ancillary_tags_; std::shared_ptr expr_; std::vector actions_; }; diff --git a/src/ruleset_builder.cpp b/src/ruleset_builder.cpp index 86bcb000f..d6cac8413 100644 --- a/src/ruleset_builder.cpp +++ b/src/ruleset_builder.cpp @@ -98,7 +98,9 @@ std::shared_ptr ruleset_builder::build(parameter::map &root, base_rules rule_ptr->set_actions(*ovrd.actions); } - for (const auto &[tag, value] : ovrd.tags) { rule_ptr->set_tag(tag, value); } + for (const auto &[tag, value] : ovrd.tags) { + rule_ptr->set_ancillary_tag(tag, value); + } } } @@ -113,7 +115,9 @@ std::shared_ptr ruleset_builder::build(parameter::map &root, base_rules rule_ptr->set_actions(*ovrd.actions); } - for (const auto &[tag, value] : ovrd.tags) { rule_ptr->set_tag(tag, value); } + for (const auto &[tag, value] : ovrd.tags) { + rule_ptr->set_ancillary_tag(tag, value); + } } } } diff --git a/tests/interface_test.cpp b/tests/interface_test.cpp index e9c8fa59f..8d22bbf9f 100644 --- a/tests/interface_test.cpp +++ b/tests/interface_test.cpp @@ -780,7 +780,7 @@ TEST(TestInterface, UpdateTagsByID) EXPECT_EVENTS(result2, {.id = "1", .name = "rule1", - .tags = {{"type", "flow1"}, {"category", "new_category"}, {"confidence", "0"}, + .tags = {{"type", "flow1"}, {"category", "category1"}, {"confidence", "1"}, {"new_tag", "value"}}, .matches = {{.op = "match_regex", .op_value = "rule1", @@ -834,7 +834,7 @@ TEST(TestInterface, UpdateTagsByID) EXPECT_EVENTS(result2, {.id = "1", .name = "rule1", - .tags = {{"type", "flow1"}, {"category", "new_category"}, {"confidence", "0"}, + .tags = {{"type", "flow1"}, {"category", "category1"}, {"confidence", "1"}, {"new_tag", "value"}}, .matches = {{.op = "match_regex", .op_value = "rule1", @@ -907,7 +907,7 @@ TEST(TestInterface, UpdateTagsByTags) EXPECT_EVENTS(result2, {.id = "1", .name = "rule1", - .tags = {{"type", "flow1"}, {"category", "category1"}, {"confidence", "0"}, + .tags = {{"type", "flow1"}, {"category", "category1"}, {"confidence", "1"}, {"new_tag", "value"}}, .matches = {{.op = "match_regex", .op_value = "rule1", @@ -1000,7 +1000,7 @@ TEST(TestInterface, UpdateTagsByTags) EXPECT_EVENTS(result2, {.id = "3", .name = "rule3", - .tags = {{"type", "flow2"}, {"category", "category3"}, {"confidence", "0"}, + .tags = {{"type", "flow2"}, {"category", "category3"}, {"confidence", "1"}, {"new_tag", "value"}}, .matches = {{.op = "match_regex", .op_value = "rule3", @@ -1055,7 +1055,7 @@ TEST(TestInterface, UpdateTagsByTags) EXPECT_EVENTS(result2, {.id = "3", .name = "rule3", - .tags = {{"type", "flow2"}, {"category", "category3"}, {"confidence", "0"}, + .tags = {{"type", "flow2"}, {"category", "category3"}, {"confidence", "1"}, {"new_tag", "value"}}, .matches = {{.op = "match_regex", .op_value = "rule3", @@ -1088,7 +1088,7 @@ TEST(TestInterface, UpdateOverrideByIDAndTag) ddwaf_handle handle2; { auto overrides = yaml_to_object( - R"({rules_override: [{rules_target: [{tags: {type: flow1}}], on_match: ["block"], enabled: false}, {rules_target: [{rule_id: 1}], enabled: true}]})"); + R"({rules_override: [{rules_target: [{tags: {type: flow1}}], tags: {new_tag: old_value}, on_match: ["block"], enabled: false}, {rules_target: [{rule_id: 1}], tags: {new_tag: new_value}, enabled: true}]})"); handle2 = ddwaf_update(handle1, &overrides, nullptr); ddwaf_object_free(&overrides); } @@ -1110,6 +1110,28 @@ TEST(TestInterface, UpdateOverrideByIDAndTag) EXPECT_EQ(ddwaf_run(context1, ¶meter, nullptr, &result1, LONG_TIME), DDWAF_MATCH); EXPECT_EQ(ddwaf_run(context2, ¶meter, nullptr, &result2, LONG_TIME), DDWAF_MATCH); + EXPECT_EVENTS(result1, + {.id = "1", + .name = "rule1", + .tags = {{"type", "flow1"}, {"category", "category1"}, {"confidence", "1"}}, + .matches = {{.op = "match_regex", + .op_value = "rule1", + .highlight = "rule1", + .args = { + {.name = "input", .value = "rule1", .address = "value1", .path = {}}}}}}); + + EXPECT_EVENTS(result2, + {.id = "1", + .name = "rule1", + .tags = {{"type", "flow1"}, {"category", "category1"}, {"confidence", "1"}, + {"new_tag", "new_value"}}, + .actions = {"block"}, + .matches = {{.op = "match_regex", + .op_value = "rule1", + .highlight = "rule1", + .args = { + {.name = "input", .value = "rule1", .address = "value1", .path = {}}}}}}); + EXPECT_ACTIONS(result1, {}); EXPECT_ACTIONS( result2, {{"block_request", @@ -1149,6 +1171,28 @@ TEST(TestInterface, UpdateOverrideByIDAndTag) EXPECT_EQ(ddwaf_run(context2, ¶meter, nullptr, &result2, LONG_TIME), DDWAF_MATCH); EXPECT_EQ(ddwaf_run(context3, ¶meter, nullptr, &result3, LONG_TIME), DDWAF_MATCH); + EXPECT_EVENTS(result2, + {.id = "1", + .name = "rule1", + .tags = {{"type", "flow1"}, {"category", "category1"}, {"confidence", "1"}, + {"new_tag", "new_value"}}, + .actions = {"block"}, + .matches = {{.op = "match_regex", + .op_value = "rule1", + .highlight = "rule1", + .args = { + {.name = "input", .value = "rule1", .address = "value1", .path = {}}}}}}); + + EXPECT_EVENTS(result3, + {.id = "1", + .name = "rule1", + .tags = {{"type", "flow1"}, {"category", "category1"}, {"confidence", "1"}}, + .matches = {{.op = "match_regex", + .op_value = "rule1", + .highlight = "rule1", + .args = { + {.name = "input", .value = "rule1", .address = "value1", .path = {}}}}}}); + EXPECT_ACTIONS( result2, {{"block_request", {{"status_code", "403"}, {"grpc_status_code", "10"}, {"type", "auto"}}}});