Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject mismatched sources rules changes #3383

Merged
merged 3 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 105 additions & 0 deletions unit_tests/engine/test_rule_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1222,3 +1222,108 @@ TEST_F(test_falco_engine, exceptions_fields_transformer_space_quoted) {
EXPECT_EQ(get_compiled_rule_condition("test_rule"),
"(evt.type = open and not tolower(proc.name) = test)");
}

TEST_F(test_falco_engine, redefine_rule_different_source) {
auto rules_content = R"END(
- rule: LD_PRELOAD trick
desc: Some desc
condition: ka.verb = GET
output: some output
priority: INFO
source: k8s_audit

- rule: LD_PRELOAD trick
desc: Some desc
condition: and 1 = 2
output: Some output
priority: INFO
source: syscall
)END";

ASSERT_FALSE(load_rules(rules_content, "rules.yaml"));
ASSERT_TRUE(check_error_message("Rule has been re-defined with a different source"));
}

TEST_F(test_falco_engine, append_across_sources) {
auto rules_content = R"END(
- rule: LD_PRELOAD trick
desc: Some desc
condition: ka.verb = GET
output: some output
priority: INFO
source: k8s_audit

- rule: LD_PRELOAD trick
desc: Some desc
condition: and 1 = 2
output: Some output
priority: INFO
source: syscall
append: true
)END";

ASSERT_FALSE(load_rules(rules_content, "rules.yaml"));
ASSERT_TRUE(check_error_message("Rule has been re-defined with a different source"));
}

TEST_F(test_falco_engine, selective_replace_across_sources) {
auto rules_content = R"END(
- rule: LD_PRELOAD trick
desc: Some desc
condition: ka.verb = GET
output: some output
priority: INFO
source: k8s_audit

- rule: LD_PRELOAD trick
condition: 1 = 2
override:
condition: replace
source: syscall
)END";

ASSERT_FALSE(load_rules(rules_content, "rules.yaml"));
ASSERT_TRUE(check_error_message("Rule has been re-defined with a different source"));
}

TEST_F(test_falco_engine, empty_source_addl_rule) {
auto rules_content = R"END(
- rule: LD_PRELOAD trick
desc: Some desc
condition: evt.type=execve
output: some output
priority: INFO
source: syscall

- rule: LD_PRELOAD trick
desc: Some desc
condition: and proc.name=apache
output: Some output
priority: INFO
source:
append: true
)END";

EXPECT_TRUE(load_rules(rules_content, "rules.yaml"));
}

TEST_F(test_falco_engine, empty_string_source_addl_rule) {
auto rules_content = R"END(
- rule: LD_PRELOAD trick
desc: Some desc
condition: evt.type=execve
output: some output
priority: INFO
source: syscall

- rule: LD_PRELOAD trick
desc: Some desc
condition: and proc.name=apache
output: Some output
priority: INFO
source: ""
append: true
)END";

EXPECT_TRUE(load_rules(rules_content, "rules.yaml"));
}
1 change: 1 addition & 0 deletions userspace/engine/rule_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ struct rule_update_info {
context cond_ctx;
std::string name;
std::optional<std::string> cond;
std::string source;
std::optional<std::string> output;
std::optional<std::string> desc;
std::optional<std::set<std::string>> tags;
Expand Down
23 changes: 17 additions & 6 deletions userspace/engine/rule_loader_collector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,8 @@ void rule_loader::collector::append(configuration& cfg, macro_info& info) {
}

void rule_loader::collector::define(configuration& cfg, rule_info& info) {
const auto* prev = m_rule_infos.at(info.name);
THROW(prev && prev->source != info.source,
"Rule has been re-defined with a different source",
info.ctx);
auto prev = find_prev_rule(info);
(void)prev;

const auto* source = cfg.sources.at(info.source);
if(!source) {
Expand All @@ -205,7 +203,7 @@ void rule_loader::collector::define(configuration& cfg, rule_info& info) {
}

void rule_loader::collector::append(configuration& cfg, rule_update_info& info) {
auto prev = m_rule_infos.at(info.name);
auto prev = find_prev_rule(info);

THROW(!prev, ERROR_NO_PREVIOUS_RULE_APPEND, info.ctx);
THROW(!info.has_any_value(),
Expand Down Expand Up @@ -275,7 +273,7 @@ void rule_loader::collector::append(configuration& cfg, rule_update_info& info)
}

void rule_loader::collector::selective_replace(configuration& cfg, rule_update_info& info) {
auto prev = m_rule_infos.at(info.name);
auto prev = find_prev_rule(info);

THROW(!prev, ERROR_NO_PREVIOUS_RULE_REPLACE, info.ctx);
THROW(!info.has_any_value(),
Expand Down Expand Up @@ -330,6 +328,19 @@ void rule_loader::collector::selective_replace(configuration& cfg, rule_update_i
replace_info(prev, info, m_cur_index++);
}

template<typename ruleInfo>
rule_loader::rule_info* rule_loader::collector::find_prev_rule(ruleInfo& info) {
auto ret = m_rule_infos.at(info.name);

// Throw an error if both the original rule and current rule
// have the same name and explicitly have different sources.
THROW(ret && (ret->source != "" && info.source != "" && ret->source != info.source),
"Rule has been re-defined with a different source",
info.ctx);

return ret;
}

void rule_loader::collector::enable(configuration& cfg, rule_info& info) {
auto prev = m_rule_infos.at(info.name);
THROW(!prev, "Rule has 'enabled' key but no rule by that name already exists", info.ctx);
Expand Down
3 changes: 3 additions & 0 deletions userspace/engine/rule_loader_collector.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ class collector {
virtual void selective_replace(configuration& cfg, rule_update_info& info);

private:
template<typename ruleInfo>
rule_info* find_prev_rule(ruleInfo& info);

uint32_t m_cur_index;
indexed_vector<rule_info> m_rule_infos;
indexed_vector<macro_info> m_macro_infos;
Expand Down
40 changes: 35 additions & 5 deletions userspace/engine/rule_loader_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,28 @@ static void decode_val_generic(const YAML::Node& item,
const char* key,
T& out,
const rule_loader::context& ctx,
bool optional) {
bool optional,
bool can_be_empty) {
const YAML::Node& val = item[key];

if(!val.IsDefined() && optional) {
return;
}

THROW(!val.IsDefined(), std::string("Item has no mapping for key '") + key + "'", ctx);
if(val.IsNull() && can_be_empty) {
return;
}

THROW(val.IsNull(), std::string("Mapping for key '") + key + "' is empty", ctx);

rule_loader::context valctx(val, rule_loader::context::VALUE_FOR, key, ctx);
THROW(!val.IsScalar(), "Value is not a scalar value", valctx);

if(val.Scalar().empty() && can_be_empty) {
return;
}

THROW(val.Scalar().empty(), "Value must be non-empty", valctx);

THROW(!YAML::convert<T>::decode(val, out), "Can't decode YAML scalar value", valctx);
Expand All @@ -75,9 +85,10 @@ static void decode_val_generic(const YAML::Node& item,
const char* key,
std::optional<T>& out,
const rule_loader::context& ctx,
bool optional) {
bool optional,
bool can_be_empty) {
T decoded;
decode_val_generic(item, key, decoded, ctx, optional);
decode_val_generic(item, key, decoded, ctx, optional, can_be_empty);
out = decoded;
}

Expand All @@ -87,8 +98,9 @@ void rule_loader::reader::decode_val(const YAML::Node& item,
T& out,
const rule_loader::context& ctx) {
bool optional = false;
bool can_be_empty = false;

decode_val_generic(item, key, out, ctx, optional);
decode_val_generic(item, key, out, ctx, optional, can_be_empty);
}

template void rule_loader::reader::decode_val<std::string>(const YAML::Node& item,
Expand All @@ -102,8 +114,20 @@ void rule_loader::reader::decode_optional_val(const YAML::Node& item,
T& out,
const rule_loader::context& ctx) {
bool optional = true;
bool can_be_empty = false;

decode_val_generic(item, key, out, ctx, optional);
decode_val_generic(item, key, out, ctx, optional, can_be_empty);
}

template<typename T>
void rule_loader::reader::decode_optional_empty_val(const YAML::Node& item,
const char* key,
T& out,
const rule_loader::context& ctx) {
bool optional = true;
bool can_be_empty = true;

decode_val_generic(item, key, out, ctx, optional, can_be_empty);
}

template void rule_loader::reader::decode_optional_val<std::string>(
Expand Down Expand Up @@ -591,6 +615,9 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg,

rule_loader::context ctx(item, rule_loader::context::RULE, name, parent);

std::string source = "";
decode_optional_empty_val(item, "source", source, ctx);

bool has_append_flag = false;
decode_optional_val(item, "append", has_append_flag, ctx);
if(has_append_flag) {
Expand Down Expand Up @@ -648,6 +675,7 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg,
"append",
"condition",
ctx)) {
v.source = source;
decode_val(item, "condition", v.cond, ctx);
}

Expand Down Expand Up @@ -682,6 +710,7 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg,
"replace",
"condition",
ctx)) {
v.source = source;
decode_val(item, "condition", v.cond, ctx);
}

Expand Down Expand Up @@ -765,6 +794,7 @@ void rule_loader::reader::read_item(rule_loader::configuration& cfg,
} else if(has_append_flag) {
rule_loader::rule_update_info v(ctx);
v.name = name;
v.source = source;

if(item["condition"].IsDefined()) {
v.cond_ctx = rule_loader::context(item["condition"],
Expand Down
5 changes: 5 additions & 0 deletions userspace/engine/rule_loader_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ class reader {
const char* key,
T& out,
const rule_loader::context& ctx);
template<typename T>
static void decode_optional_empty_val(const YAML::Node& item,
const char* key,
T& out,
const rule_loader::context& ctx);

protected:
virtual void read_item(rule_loader::configuration& cfg,
Expand Down
Loading