From: Russ Combs (rucombs) Date: Mon, 9 Sep 2019 22:04:40 +0000 (-0400) Subject: Merge pull request #1732 in SNORT/snort3 from ~RUCOMBS/snort3:rule_statez to master X-Git-Tag: 3.0.0-261~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=52b6e4e871a4293b49d458550451fee99a14e7f6;p=thirdparty%2Fsnort3.git Merge pull request #1732 in SNORT/snort3 from ~RUCOMBS/snort3:rule_statez to master Squashed commit of the following: commit 8f66afffc52f4eecc0436d23359f2eccd3ff18f2 Author: russ Date: Wed Sep 4 17:53:18 2019 -0400 doc: add bullets for $var parameter names and maxXX limits. commit ff4bca6a07a6b5446332ce0d41272b9299f08998 Author: russ Date: Wed Sep 4 16:59:12 2019 -0400 rule_state: switch from regex parameter names to simpler parsing Performance when loading large rule sets (20K+ rules) with regex is unacceptable. Switch from regex to $var parameter names with name matching delegated to module. In this case, $gid_sid is used for rule_state["1:23456"] type configurations. As you might have guessed, $ indicates parameters with variable names. --- diff --git a/doc/params.txt b/doc/params.txt index 9fd4231c8..59110bfdd 100644 --- a/doc/params.txt +++ b/doc/params.txt @@ -32,6 +32,8 @@ information about the type and use of the parameter: * IPS rules may also have a wild card parameter, which is indicated by a *. Used for unquoted, comma-separated lists such as service and metadata. * The snort module has command line options starting with a -. +* $ denotes variable names, eg rule_state.$gid_sid which would be used + like rule_state["1:23456"] = { }. Some additional details to note: @@ -47,4 +49,6 @@ Some additional details to note: * interval takes the form [operator]i, j<>k, or j<=>k where i,j,k are integers and operator is one of =, !, != (same as !), <, <=, >, >=. j<>k means j < int < k and j<=>k means j <= int <= k. +* Ranges may use maxXX like { 1:max32 } since max32 is easier to read + than 4294967295. To get the values of maxXX, use snort --help-limits. diff --git a/src/detection/rules.cc b/src/detection/rules.cc index 58cb12ba3..94abe2e97 100644 --- a/src/detection/rules.cc +++ b/src/detection/rules.cc @@ -47,7 +47,7 @@ void RuleState::apply(SnortConfig* sc) OptTreeNode* otn = OtnLookup(sc->otn_map, gid, sid); if ( otn == nullptr ) - ParseError("Rule state specified for invalid SID: %u GID: %u", sid, gid); + ParseError("Rule state specified for invalid rule %u:%u", gid, sid); else { if ( sc->global_rule_state ) diff --git a/src/framework/module.h b/src/framework/module.h index 61e883e1d..7c89d06b6 100644 --- a/src/framework/module.h +++ b/src/framework/module.h @@ -92,6 +92,10 @@ public: virtual bool set(const char*, Value&, SnortConfig*); + // used to match parameters with $var names like for rule_state + virtual bool matches(const char* /*param_name*/, std::string& /*lua_name*/) + { return false; } + // ips events: virtual unsigned get_gid() const { return 0; } diff --git a/src/framework/parameter.h b/src/framework/parameter.h index 2b0327801..7779d7c54 100644 --- a/src/framework/parameter.h +++ b/src/framework/parameter.h @@ -67,10 +67,6 @@ struct SO_PUBLIC Parameter const void* range; // nullptr|const char*|RangeQuery*|const Parameter* const char* deflt; const char* help; - bool regex = false; // for name resolution - - Parameter(const char* n, Type t, const void* r, const char* d, const char* h, bool re) : - name(n), type(t), range(r), deflt(d), help(h), regex(re) { } Parameter(const char* n, Type t, const void* r, const char* d, const char* h) : name(n), type(t), range(r), deflt(d), help(h) { } diff --git a/src/main/modules.cc b/src/main/modules.cc index 420340c07..d449ed77c 100644 --- a/src/main/modules.cc +++ b/src/main/modules.cc @@ -24,7 +24,6 @@ #include "modules.h" -#include #include #include "codecs/codec_module.h" @@ -1841,11 +1840,10 @@ static const Parameter single_rule_state_params[] = { nullptr, Parameter::PT_MAX, nullptr, nullptr, nullptr } }; -static const char* rule_state_gid_sid_regex = "([0-9]+):([0-9]+)"; static const Parameter rule_state_params[] = { - { rule_state_gid_sid_regex, Parameter::PT_LIST, single_rule_state_params, nullptr, - "defines rule state parameters for gid:sid", true }, + { "$gid_sid", Parameter::PT_LIST, single_rule_state_params, nullptr, + "defines rule state parameters for gid:sid" }, { nullptr, Parameter::PT_MAX, nullptr, nullptr, nullptr } }; @@ -1862,6 +1860,8 @@ public: bool begin(const char*, int, SnortConfig*) override; bool end(const char*, int, SnortConfig*) override; + bool matches(const char*, std::string&) override; + Usage get_usage() const override { return DETECT; } @@ -1871,21 +1871,30 @@ private: IpsPolicy::Enable enable; }; -bool RuleStateModule::set(const char* fqn, Value& v, SnortConfig*) +bool RuleStateModule::matches(const char* param, std::string& name) { - static regex gid_sid(rule_state_gid_sid_regex); + if ( strcmp(param, "$gid_sid") ) + return false; + + std::stringstream ss(name); + char sep; - // the regex itself is passed as the fqn when declaring rule_state = { } - if ( strstr(fqn, rule_state_gid_sid_regex) ) + ss >> gid >> sep >> sid; + + if ( gid and sid and sep == ':' ) return true; - cmatch match; + return false; +} - if ( regex_search(fqn, match, gid_sid) ) - { - gid = strtoul(match[1].str().c_str(), nullptr, 10); - sid = strtoul(match[2].str().c_str(), nullptr, 10); +bool RuleStateModule::set(const char* fqn, Value& v, SnortConfig*) +{ + // the name itself is passed as the fqn when declaring rule_state = { } + if ( !strcmp(fqn, "$gid_sid") ) + return true; + if ( gid and sid ) + { if ( v.is("action") ) action = IpsPolicy::Action(v.get_uint8()); @@ -1911,9 +1920,9 @@ bool RuleStateModule::begin(const char*, int, SnortConfig*) bool RuleStateModule::end(const char*, int, SnortConfig* sc) { - if ( gid ) + if ( gid and sid ) sc->rule_states.emplace_back(new RuleState(gid, sid, action, enable)); - gid = 0; + gid = sid = 0; return true; } diff --git a/src/managers/module_manager.cc b/src/managers/module_manager.cc index dc37b2a94..cffc586c3 100644 --- a/src/managers/module_manager.cc +++ b/src/managers/module_manager.cc @@ -29,7 +29,6 @@ #include #include #include -#include #include #include "framework/base_api.h" @@ -345,7 +344,7 @@ static void dump_table(string& key, const char* pfx, const Parameter* p, bool li //------------------------------------------------------------------------- static const Parameter* get_params( - const string& sfx, const Parameter* p, int idx = 1) + const string& sfx, Module* m, const Parameter* p, int idx = 1) { size_t pos = sfx.find_first_of('.'); std::string new_fqn; @@ -363,9 +362,10 @@ static const Parameter* get_params( } string name = new_fqn.substr(0, new_fqn.find_first_of('.')); + while ( p->name ) { - if ( p->regex && regex_match(name, regex(p->name)) ) + if ( *p->name == '$' and m->matches(p->name, name) ) break; else if ( name == p->name ) @@ -396,7 +396,7 @@ static const Parameter* get_params( } p = (const Parameter*)p->range; - return get_params(new_fqn, p, idx); + return get_params(new_fqn, m, p, idx); } // FIXIT-M vars may have been defined on command line. that mechanism will @@ -508,10 +508,10 @@ static bool set_value(const char* fqn, Value& v) // now we must traverse the mod params to get the leaf string s = fqn; - const Parameter* p = get_params(s, mod->get_parameters()); + const Parameter* p = get_params(s, mod, mod->get_parameters()); if ( !p ) - p = get_params(s, mod->get_default_parameters()); + p = get_params(s, mod, mod->get_default_parameters()); if ( !p ) { @@ -729,10 +729,10 @@ SO_PUBLIC bool open_table(const char* s, int idx) if ( strcmp(m->get_name(), s) ) { std::string sfqn = s; - p = get_params(sfqn, m->get_parameters(), idx); + p = get_params(sfqn, m, m->get_parameters(), idx); if ( !p ) - p = get_params(sfqn, m->get_default_parameters(), idx); + p = get_params(sfqn, m, m->get_default_parameters(), idx); if ( !p ) {