From 802ab61c82933a5b6f2e51b97dd34f80d3fd986e Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Thu, 14 Apr 2022 09:21:45 +0000 Subject: [PATCH] chore(userspace/engine): renamings and code polishing in rule_loader and rule_reader Signed-off-by: Jason Dellaluce --- userspace/engine/falco_engine.cpp | 26 +++---- userspace/engine/rule_loader.cpp | 113 ++++++++++++++++-------------- userspace/engine/rule_loader.h | 40 +++++------ userspace/engine/rule_reader.cpp | 50 ++++++------- userspace/engine/rule_reader.h | 2 +- 5 files changed, 118 insertions(+), 113 deletions(-) diff --git a/userspace/engine/falco_engine.cpp b/userspace/engine/falco_engine.cpp index 45f455c2623..1a03d59f10a 100644 --- a/userspace/engine/falco_engine.cpp +++ b/userspace/engine/falco_engine.cpp @@ -150,33 +150,33 @@ void falco_engine::load_rules(const string &rules_content, bool verbose, bool al void falco_engine::load_rules(const string &rules_content, bool verbose, bool all_events, uint64_t &required_engine_version) { - rule_loader::context ctx(rules_content); - ctx.engine = this; - ctx.min_priority = m_min_priority; - ctx.output_extra = m_extra; - ctx.replace_output_container_info = m_replace_container_info; + rule_loader::configuration cfg(rules_content); + cfg.engine = this; + cfg.min_priority = m_min_priority; + cfg.output_extra = m_extra; + cfg.replace_output_container_info = m_replace_container_info; std::ostringstream os; rule_reader reader; - bool success = reader.load(ctx, m_rule_loader); + bool success = reader.load(cfg, m_rule_loader); if (success) { clear_filters(); m_rules.clear(); - success = m_rule_loader.compile(ctx, m_rules); + success = m_rule_loader.compile(cfg, m_rules); } - if (!ctx.errors.empty()) + if (!cfg.errors.empty()) { - os << ctx.errors.size() << " errors:" << std::endl; - for(auto &err : ctx.errors) + os << cfg.errors.size() << " errors:" << std::endl; + for(auto &err : cfg.errors) { os << err << std::endl; } } - if (!ctx.warnings.empty()) + if (!cfg.warnings.empty()) { - os << ctx.warnings.size() << " warnings:" << std::endl; - for(auto &warn : ctx.warnings) + os << cfg.warnings.size() << " warnings:" << std::endl; + for(auto &warn : cfg.warnings) { os << warn << std::endl; } diff --git a/userspace/engine/rule_loader.cpp b/userspace/engine/rule_loader.cpp index a523df3fc60..81d05135a0b 100644 --- a/userspace/engine/rule_loader.cpp +++ b/userspace/engine/rule_loader.cpp @@ -36,7 +36,7 @@ static void quote_item(string& e) } } -static void paren_item(std::string& e) +static void paren_item(string& e) { if(e[0] != '(') { @@ -44,7 +44,8 @@ static void paren_item(std::string& e) } } -static bool is_field_defined(falco_engine *engine, string source, string field) +static bool is_field_defined( + falco_engine *engine, const string& source, string field) { auto factory = engine->get_filter_factory(source); if(factory) @@ -60,7 +61,7 @@ static bool is_field_defined(falco_engine *engine, string source, string field) } // todo(jasondellaluce): add an helper in libsinsp for this -static bool is_operator_defined(std::string op) +static bool is_operator_defined(const string& op) { static vector ops = {"=", "==", "!=", "<=", ">=", "<", ">", "contains", "icontains", "bcontains", "glob", "bstartswith", @@ -69,7 +70,7 @@ static bool is_operator_defined(std::string op) } // todo(jasondellaluce): add an helper in libsinsp for this -static bool is_operator_for_list(std::string op) +static bool is_operator_for_list(const string& op) { return op == "in" || op == "intersects" || op == "pmatch"; } @@ -90,7 +91,6 @@ static bool is_format_valid( } } - template static inline void define_info(indexed_vector& infos, T& info, uint32_t id) { @@ -113,11 +113,11 @@ template static inline void append_info(T* prev, T& info, uint32_t id) { prev->visibility = id; - prev->context.append(info.context); + prev->ctx.append(info.ctx); } static void validate_exception_info( - rule_loader::context& ctx, + rule_loader::configuration& cfg, rule_loader::rule_exception_info &ex, const string& source) { @@ -142,7 +142,7 @@ static void validate_exception_info( } for (auto &v : ex.fields.items) { - THROW(!is_field_defined(ctx.engine, source, v.item), + THROW(!is_field_defined(cfg.engine, source, v.item), "Rule exception item " + ex.name + ": field name " + v.item + " is not a supported filter field"); } @@ -159,14 +159,14 @@ static void validate_exception_info( THROW(!is_operator_defined(ex.comps.item), "Rule exception item " + ex.name + ": comparison operator " + ex.comps.item + " is not a supported comparison operator"); - THROW(!is_field_defined(ctx.engine, source, ex.fields.item), + THROW(!is_field_defined(cfg.engine, source, ex.fields.item), "Rule exception item " + ex.name + ": field name " + ex.fields.item + " is not a supported filter field"); } } static void build_rule_exception_infos( - vector exceptions, + vector& exceptions, set& exception_fields, string& condition) { @@ -370,8 +370,11 @@ static shared_ptr parse_condition( } static shared_ptr compile_condition( - falco_engine* engine, uint32_t id, shared_ptr cnd, - string src, string& err) + falco_engine* engine, + uint32_t id, + shared_ptr cnd, + string src, + string& err) { try { @@ -392,18 +395,20 @@ static shared_ptr compile_condition( return nullptr; } -void apply_output_substitutions(rule_loader::context ctx, string& out) +static void apply_output_substitutions( + rule_loader::configuration& cfg, + string& out) { if (out.find(s_container_info_fmt) != string::npos) { - if (ctx.replace_output_container_info) + if (cfg.replace_output_container_info) { - out = replace(out, s_container_info_fmt, ctx.output_extra); + out = replace(out, s_container_info_fmt, cfg.output_extra); return; } out = replace(out, s_container_info_fmt, s_default_extra_fmt); } - out += ctx.output_extra.empty() ? "" : " " + ctx.output_extra; + out += cfg.output_extra.empty() ? "" : " " + cfg.output_extra; } void rule_loader::clear() @@ -443,24 +448,24 @@ bool rule_loader::is_plugin_compatible( return true; } -void rule_loader::define(context& ctx, engine_version_info& info) +void rule_loader::define(configuration& cfg, engine_version_info& info) { auto v = falco_engine::engine_version(); THROW(v < info.version, "Rules require engine version " + to_string(info.version) + ", but engine version is " + to_string(v)); } -void rule_loader::define(context& ctx, plugin_version_info& info) +void rule_loader::define(configuration& cfg, plugin_version_info& info) { m_required_plugin_versions[info.name].insert(info.version); } -void rule_loader::define(context& ctx, list_info& info) +void rule_loader::define(configuration& cfg, list_info& info) { define_info(m_list_infos, info, m_cur_index++); } -void rule_loader::append(context& ctx, list_info& info) +void rule_loader::append(configuration& cfg, list_info& info) { auto prev = m_list_infos.at(info.name); THROW(!prev, "List " + info.name + @@ -469,11 +474,11 @@ void rule_loader::append(context& ctx, list_info& info) append_info(prev, info, m_cur_index++); } -void rule_loader::define(context& ctx, macro_info& info) +void rule_loader::define(configuration& cfg, macro_info& info) { - if (!ctx.engine->is_source_valid(info.source)) + if (!cfg.engine->is_source_valid(info.source)) { - ctx.warnings.push_back("Macro " + info.name + cfg.warnings.push_back("Macro " + info.name + ": warning (unknown-source): unknown source " + info.source + ", skipping"); return; @@ -481,7 +486,7 @@ void rule_loader::define(context& ctx, macro_info& info) define_info(m_macro_infos, info, m_cur_index++); } -void rule_loader::append(context& ctx, macro_info& info) +void rule_loader::append(configuration& cfg, macro_info& info) { auto prev = m_macro_infos.at(info.name); THROW(!prev, "Macro " + info.name @@ -491,11 +496,11 @@ void rule_loader::append(context& ctx, macro_info& info) append_info(prev, info, m_cur_index++); } -void rule_loader::define(context& ctx, rule_info& info) +void rule_loader::define(configuration& cfg, rule_info& info) { - if (!ctx.engine->is_source_valid(info.source)) + if (!cfg.engine->is_source_valid(info.source)) { - ctx.warnings.push_back("Rule " + info.name + cfg.warnings.push_back("Rule " + info.name + ": warning (unknown-source): unknown source " + info.source + ", skipping"); return; @@ -509,13 +514,13 @@ void rule_loader::define(context& ctx, rule_info& info) { THROW(!ex.fields.is_valid(), "Rule exception item " + ex.name + ": must have fields property with a list of fields"); - validate_exception_info(ctx, ex, info.source); + validate_exception_info(cfg, ex, info.source); } define_info(m_rule_infos, info, m_cur_index++); } -void rule_loader::append(context& ctx, rule_info& info) +void rule_loader::append(configuration& cfg, rule_info& info) { auto prev = m_rule_infos.at(info.name); THROW(!prev, "Rule " + info.name @@ -540,7 +545,7 @@ void rule_loader::append(context& ctx, rule_info& info) + ex.name + ": must have fields property with a list of fields"); THROW(ex.values.empty(), "Rule exception new item " + ex.name + ": must have fields property with a list of values"); - validate_exception_info(ctx, ex, prev->source); + validate_exception_info(cfg, ex, prev->source); prev->exceptions.push_back(ex); } else @@ -556,7 +561,7 @@ void rule_loader::append(context& ctx, rule_info& info) append_info(prev, info, m_cur_index++); } -void rule_loader::enable(context& ctx, rule_info& info) +void rule_loader::enable(configuration& cfg, rule_info& info) { auto prev = m_rule_infos.at(info.name); THROW(!prev, "Rule " + info.name @@ -564,7 +569,7 @@ void rule_loader::enable(context& ctx, rule_info& info) prev->enabled = info.enabled; } -void rule_loader::compile_list_infos(context& ctx, indexed_vector& out) +void rule_loader::compile_list_infos(configuration& cfg, indexed_vector& out) { string tmp; vector used; @@ -598,7 +603,7 @@ void rule_loader::compile_list_infos(context& ctx, indexed_vector& ou } catch (exception& e) { - throw falco_exception(list.context.error(e.what())); + throw falco_exception(list.ctx.error(e.what())); } } for (auto &v : used) @@ -609,17 +614,17 @@ void rule_loader::compile_list_infos(context& ctx, indexed_vector& ou // note: there is a visibility ordering between macros void rule_loader::compile_macros_infos( - context& ctx, + configuration& cfg, indexed_vector& lists, indexed_vector& out) { set used; - mark* info_ctx = NULL; + context* info_ctx = NULL; try { for (auto &m : m_macro_infos) { - info_ctx = &m.context; + info_ctx = &m.ctx; macro_info entry = m; entry.cond_ast = parse_condition(m.cond, lists); entry.used = false; @@ -627,7 +632,7 @@ void rule_loader::compile_macros_infos( } for (auto &m : out) { - info_ctx = &m.context; + info_ctx = &m.ctx; resolve_macros(out, m.cond_ast, m.visibility, "Compilation error when compiling \"" + m.cond + "\": "); } @@ -640,7 +645,7 @@ void rule_loader::compile_macros_infos( void rule_loader::compile_rule_infos( - context& ctx, + configuration& cfg, indexed_vector& lists, indexed_vector& macros, indexed_vector& out) @@ -650,7 +655,7 @@ void rule_loader::compile_rule_infos( { try { - if (r.priority > ctx.min_priority) + if (r.priority > cfg.min_priority) { continue; } @@ -668,9 +673,9 @@ void rule_loader::compile_rule_infos( rule.output = r.output; if (r.source == falco_common::syscall_source) { - apply_output_substitutions(ctx, rule.output); + apply_output_substitutions(cfg, rule.output); } - THROW(!is_format_valid(ctx.engine, r.source, rule.output, err), + THROW(!is_format_valid(cfg.engine, r.source, rule.output, err), "Invalid output format '" + rule.output + "': '" + err + "'"); @@ -681,13 +686,13 @@ void rule_loader::compile_rule_infos( rule.tags = r.tags; // note: indexes are 0-based, but 0 is not an acceptable rule_id auto id = out.insert(rule, rule.name) + 1; - auto filter = compile_condition(ctx.engine, id, ast, rule.source, err); + auto filter = compile_condition(cfg.engine, id, ast, rule.source, err); if (!filter) { if (r.skip_if_unknown_filter && err.find("nonexistent field") != string::npos) { - ctx.warnings.push_back( + cfg.warnings.push_back( "Rule " + rule.name + ": warning (unknown-field):"); continue; } @@ -696,28 +701,28 @@ void rule_loader::compile_rule_infos( throw falco_exception("Rule " + rule.name + ": error " + err); } } - ctx.engine->add_filter(filter, rule.name, rule.source, rule.tags); + cfg.engine->add_filter(filter, rule.name, rule.source, rule.tags); if (rule.source == falco_common::syscall_source && r.warn_evttypes) { auto evttypes = filter->evttypes(); if (evttypes.size() == 0 || evttypes.size() > 100) { - ctx.warnings.push_back( + cfg.warnings.push_back( "Rule " + rule.name + ": warning (no-evttype):\n" + + " matches too many evt.type values.\n" + " This has a significant performance penalty."); } } - ctx.engine->enable_rule(rule.name, r.enabled); + cfg.engine->enable_rule(rule.name, r.enabled); } catch (exception& e) { - throw falco_exception(r.context.error(e.what())); + throw falco_exception(r.ctx.error(e.what())); } } } -bool rule_loader::compile(context& ctx, indexed_vector& out) +bool rule_loader::compile(configuration& cfg, indexed_vector& out) { indexed_vector lists; indexed_vector macros; @@ -725,13 +730,13 @@ bool rule_loader::compile(context& ctx, indexed_vector& out) // expand all lists, macros, and rules try { - compile_list_infos(ctx, lists); - compile_macros_infos(ctx, lists, macros); - compile_rule_infos(ctx, lists, macros, out); + compile_list_infos(cfg, lists); + compile_macros_infos(cfg, lists, macros); + compile_rule_infos(cfg, lists, macros, out); } catch (exception& e) { - ctx.errors.push_back(e.what()); + cfg.errors.push_back(e.what()); return false; } @@ -740,7 +745,7 @@ bool rule_loader::compile(context& ctx, indexed_vector& out) { if (!m.used) { - ctx.warnings.push_back("macro " + m.name + cfg.warnings.push_back("macro " + m.name + " not referred to by any rule/macro"); } } @@ -748,7 +753,7 @@ bool rule_loader::compile(context& ctx, indexed_vector& out) { if (!l.used) { - ctx.warnings.push_back("list " + l.name + cfg.warnings.push_back("list " + l.name + " not referred to by any rule/macro/list"); } } diff --git a/userspace/engine/rule_loader.h b/userspace/engine/rule_loader.h index 204099ce245..1fa032c8247 100644 --- a/userspace/engine/rule_loader.h +++ b/userspace/engine/rule_loader.h @@ -37,7 +37,7 @@ class rule_loader \brief Represents a section of text from which a certain info struct has been decoded */ - struct mark + struct context { std::string content; @@ -55,7 +55,7 @@ class rule_loader /*! \brief Appends another text section info to this one */ - inline void append(mark& m) + inline void append(context& m) { content += "\n\n"; content += m.content; @@ -65,9 +65,9 @@ class rule_loader /*! \brief Contains the info required to load rule definitions */ - struct context + struct configuration { - context(const std::string& cont): content(cont) {} + configuration(const std::string& cont): content(cont) {} const std::string& content; std::string output_extra; bool replace_output_container_info; @@ -99,7 +99,7 @@ class rule_loader */ struct list_info { - mark context; + context ctx; bool used; size_t index; size_t visibility; @@ -112,7 +112,7 @@ class rule_loader */ struct macro_info { - mark context; + context ctx; bool used; size_t index; size_t visibility; @@ -156,7 +156,7 @@ class rule_loader */ struct rule_info { - mark context; + context ctx; size_t index; size_t visibility; std::string name; @@ -190,43 +190,43 @@ class rule_loader /*! \brief Uses the internal state to compile a list of falco_rules */ - bool compile(context& ctx, indexed_vector& out); + bool compile(configuration& cfg, indexed_vector& out); /*! \brief Defines an info block. If a similar info block is found in the internal state (e.g. another rule with same name), then the previous definition gets overwritten */ - virtual void define(context& ctx, engine_version_info& info); - virtual void define(context& ctx, plugin_version_info& info); - virtual void define(context& ctx, list_info& info); - virtual void define(context& ctx, macro_info& info); - virtual void define(context& ctx, rule_info& info); + virtual void define(configuration& cfg, engine_version_info& info); + virtual void define(configuration& cfg, plugin_version_info& info); + virtual void define(configuration& cfg, list_info& info); + virtual void define(configuration& cfg, macro_info& info); + virtual void define(configuration& cfg, rule_info& info); /*! \brief Appends an info block to an existing one. An exception is thrown if no existing definition can be matched with the appended one */ - virtual void append(context& ctx, list_info& info); - virtual void append(context& ctx, macro_info& info); - virtual void append(context& ctx, rule_info& info); + virtual void append(configuration& cfg, list_info& info); + virtual void append(configuration& cfg, macro_info& info); + virtual void append(configuration& cfg, rule_info& info); /*! \brief Updates the 'enabled' flag of an existing definition */ - virtual void enable(context& ctx, rule_info& info); + virtual void enable(configuration& cfg, rule_info& info); private: void compile_list_infos( - context& ctx, + configuration& cfg, indexed_vector& out); void compile_macros_infos( - context& ctx, + configuration& cfg, indexed_vector& lists, indexed_vector& out); void compile_rule_infos( - context& ctx, + configuration& cfg, indexed_vector& lists, indexed_vector& macros, indexed_vector& out); diff --git a/userspace/engine/rule_reader.cpp b/userspace/engine/rule_reader.cpp index c4abd24e44f..3edddd72267 100644 --- a/userspace/engine/rule_reader.cpp +++ b/userspace/engine/rule_reader.cpp @@ -18,13 +18,13 @@ limitations under the License. #define THROW(cond, err) { if (cond) { throw falco_exception(err); } } -static rule_loader::mark yaml_get_mark( +static rule_loader::context yaml_get_context( const string& content, const vector& docs, vector::iterator doc, YAML::iterator node) { - rule_loader::mark m; + rule_loader::context m; YAML::Node item = *node++; YAML::Node cur_doc = *doc++; // include the "- " sequence mark @@ -151,17 +151,17 @@ static void read_rule_exceptions( } static void read_item( - rule_loader::context& ctx, + rule_loader::configuration& cfg, rule_loader& loader, const YAML::Node& item, - const rule_loader::mark& m) + const rule_loader::context& ctx) { if (item["required_engine_version"].IsDefined()) { rule_loader::engine_version_info v; THROW(!decode_val(item["required_engine_version"], v.version), "Value of required_engine_version must be a number"); - loader.define(ctx, v); + loader.define(cfg, v); } else if(item["required_plugin_versions"].IsDefined()) { @@ -175,13 +175,13 @@ static void read_item( "required_plugin_versions item must have name property"); THROW(!decode_val(plugin["version"], v.version) || v.version.empty(), "required_plugin_versions item must have version property"); - loader.define(ctx, v); + loader.define(cfg, v); } } else if(item["list"].IsDefined()) { rule_loader::list_info v; - v.context = m; + v.ctx = ctx; bool append = false; THROW(!decode_val(item["list"], v.name) || v.name.empty(), "List name is empty"); @@ -189,17 +189,17 @@ static void read_item( "List must have property items"); if(decode_val(item["append"], append) && append) { - loader.append(ctx, v); + loader.append(cfg, v); } else { - loader.define(ctx, v); + loader.define(cfg, v); } } else if(item["macro"].IsDefined()) { rule_loader::macro_info v; - v.context = m; + v.ctx = ctx; bool append = false; v.source = falco_common::syscall_source; THROW(!decode_val(item["macro"], v.name) || v.name.empty(), @@ -209,17 +209,17 @@ static void read_item( decode_val(item["source"], v.source); if(decode_val(item["append"], append) && append) { - loader.append(ctx, v); + loader.append(cfg, v); } else { - loader.define(ctx, v); + loader.define(cfg, v); } } else if(item["rule"].IsDefined()) { rule_loader::rule_info v; - v.context = m; + v.ctx = ctx; bool append = false; v.enabled = true; v.warn_evttypes = true; @@ -233,7 +233,7 @@ static void read_item( { read_rule_exceptions(item["exceptions"], v); } - loader.append(ctx, v); + loader.append(cfg, v); } else { @@ -246,7 +246,7 @@ static void read_item( if (!has_defs) { THROW(!has_enabled, "Rule must have properties 'condition', 'output', 'desc', and 'priority'"); - loader.enable(ctx, v); + loader.enable(cfg, v); } else { @@ -262,26 +262,26 @@ static void read_item( { read_rule_exceptions(item["exceptions"], v); } - loader.define(ctx, v); + loader.define(cfg, v); } } } else { - ctx.warnings.push_back("Unknown top level object"); + cfg.warnings.push_back("Unknown top level object"); } } -bool rule_reader::load(rule_loader::context& ctx, rule_loader& loader) +bool rule_reader::load(rule_loader::configuration& cfg, rule_loader& loader) { std::vector docs; try { - docs = YAML::LoadAll(ctx.content); + docs = YAML::LoadAll(cfg.content); } catch(const exception& e) { - ctx.errors.push_back("Could not load YAML file: " + string(e.what())); + cfg.errors.push_back("Could not load YAML file: " + string(e.what())); return false; } @@ -291,12 +291,12 @@ bool rule_reader::load(rule_loader::context& ctx, rule_loader& loader) { if(!doc->IsMap() && !doc->IsSequence()) { - ctx.errors.push_back("Rules content is not yaml"); + cfg.errors.push_back("Rules content is not yaml"); return false; } if(!doc->IsSequence()) { - ctx.errors.push_back( + cfg.errors.push_back( "Rules content is not yaml array of objects"); return false; } @@ -304,17 +304,17 @@ bool rule_reader::load(rule_loader::context& ctx, rule_loader& loader) { if (!it->IsNull()) { - rule_loader::mark m = yaml_get_mark(ctx.content, docs, doc, it); + auto ctx = yaml_get_context(cfg.content, docs, doc, it); YAML::Node item = *it; try { THROW(!item.IsMap(), "Unexpected element type. " "Each element should be a yaml associative array."); - read_item(ctx, loader, item, m); + read_item(cfg, loader, item, ctx); } catch(const exception& e) { - ctx.errors.push_back(m.error(e.what())); + cfg.errors.push_back(ctx.error(e.what())); return false; } } diff --git a/userspace/engine/rule_reader.h b/userspace/engine/rule_reader.h index 7214ff3add7..5abd4b35655 100644 --- a/userspace/engine/rule_reader.h +++ b/userspace/engine/rule_reader.h @@ -31,5 +31,5 @@ class rule_reader \brief Reads the contents of a ruleset and uses a loader to store thew new definitions */ - virtual bool load(rule_loader::context& ctx, rule_loader& loader); + virtual bool load(rule_loader::configuration& cfg, rule_loader& loader); };