From: Russ Combs (rucombs) Date: Mon, 8 Apr 2019 22:15:17 +0000 (-0400) Subject: Merge pull request #1570 in SNORT/snort3 from ~RUCOMBS/snort3:rule_state to master X-Git-Tag: 3.0.0-252~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d49a88af09a446703f9c594ccf18aa64e1f648ce;p=thirdparty%2Fsnort3.git Merge pull request #1570 in SNORT/snort3 from ~RUCOMBS/snort3:rule_state to master Squashed commit of the following: commit 8af3fc4d5d0e7d1a6ac213cf92635b4dba74b500 Author: russ Date: Sat Apr 6 11:32:27 2019 -0400 rules: remove cruft from tree nodes commit f1190a2475f7b560c3016b4a0d8801c276846e6f Author: russ Date: Fri Apr 5 11:30:40 2019 -0400 rule_state: rule_state: do not require rules in all policies --- diff --git a/src/detection/rules.cc b/src/detection/rules.cc index f77f27efd..58cb12ba3 100644 --- a/src/detection/rules.cc +++ b/src/detection/rules.cc @@ -24,6 +24,8 @@ #include "rules.h" +#include + #include "log/messages.h" #include "hash/xhash.h" #include "main/policy.h" @@ -64,68 +66,25 @@ void RuleState::apply(SnortConfig* sc) void RuleState::apply(SnortConfig* sc, OptTreeNode* otn, unsigned ips_num) { RuleTreeNode* rtn = getRtnFromOtn(otn, ips_num); - - if ( !rtn ) - return; - - if ( rtn->otnRefCount > 1 ) - { - // duplicate to avoid blanket setting behavior of multiple OTNs - rtn = find_updated_rtn(rtn, sc, ips_num); - if ( !rtn ) - { - rtn = dup_rtn(otn, sc, ips_num); - replace_rtn(otn, rtn, sc, ips_num); - } - - else if ( rtn != getRtnFromOtn(otn, ips_num) ) - replace_rtn(otn, rtn, sc, ips_num); - update_rtn(getRtnFromOtn(otn, ips_num)); - } - else - { - RuleTreeNode* existing_rtn = find_updated_rtn(rtn, sc, ips_num); - - // dedup to avoid wasting memory when transitioning RTN to behavior of existing one - if ( existing_rtn ) - replace_rtn(otn, existing_rtn, sc, ips_num); - else - update_rtn(getRtnFromOtn(otn, ips_num)); - } -} - -RuleTreeNode* RuleState::find_updated_rtn(RuleTreeNode* rtn, SnortConfig* sc, unsigned ips_num) -{ - RuleTreeNode test_rtn(*rtn); - update_rtn(&test_rtn); - - RuleTreeNodeKey key { &test_rtn, ips_num }; - return (RuleTreeNode*)xhash_find(sc->rtn_hash_table, &key); -} + if ( !rtn ) + rtn = getRtnFromOtn(otn, 0); -void RuleState::replace_rtn(OptTreeNode* otn, RuleTreeNode* replacement, SnortConfig* sc, unsigned ips_num) -{ - RuleTreeNode* rtn = getRtnFromOtn(otn, ips_num); - rtn->otnRefCount--; + if ( !rtn ) + return; - deleteRtnFromOtn(otn, ips_num, sc, rtn->otnRefCount == 0); - addRtnToOtn(snort::SnortConfig::get_conf(), otn, replacement, ips_num); + rtn = dup_rtn(rtn); + update_rtn(rtn); + addRtnToOtn(sc, otn, rtn, ips_num); } -RuleTreeNode* RuleState::dup_rtn(OptTreeNode* otn, SnortConfig* sc, unsigned ips_num) +RuleTreeNode* RuleState::dup_rtn(RuleTreeNode* rtn) { - RuleTreeNode* rtn = getRtnFromOtn(otn, ips_num); RuleTreeNode* ret = new RuleTreeNode(*rtn); - ret->otnRefCount = 1; - - auto ip_vartable = sc->policy_map->get_ips_policy(ips_num)->ip_vartable; - - if ( rtn->sip ) - ret->sip = sfvt_lookup_var(ip_vartable, rtn->sip->name); - if ( rtn->dip ) - ret->dip = sfvt_lookup_var(ip_vartable, rtn->dip->name); + ret->otnRefCount = 0; + ret->sip = sfvar_deep_copy(rtn->sip); + ret->dip = sfvar_deep_copy(rtn->dip); RuleFpList* from = rtn->rule_func; @@ -146,7 +105,7 @@ RuleTreeNode* RuleState::dup_rtn(OptTreeNode* otn, SnortConfig* sc, unsigned ips return ret; } -void RuleStateAction::update_rtn(RuleTreeNode* rtn) +void RuleState::update_rtn(RuleTreeNode* rtn) { switch ( action ) { @@ -158,11 +117,7 @@ void RuleStateAction::update_rtn(RuleTreeNode* rtn) case IpsPolicy::RESET: rtn->action = snort::Actions::Type::RESET; break; case IpsPolicy::INHERIT_ACTION: break; } -} - -void RuleStateEnable::update_rtn(RuleTreeNode* rtn) -{ - switch( enable ) + switch ( enable ) { case IpsPolicy::DISABLED: rtn->clear_enabled(); break; case IpsPolicy::ENABLED: rtn->set_enabled(); break; diff --git a/src/detection/rules.h b/src/detection/rules.h index 83687b0fd..be76f1944 100644 --- a/src/detection/rules.h +++ b/src/detection/rules.h @@ -62,48 +62,28 @@ struct RuleListNode RuleListNode* next; /* the next RuleListNode */ }; -// for separately overriding rule type class RuleState { public: - RuleState(unsigned g, unsigned s) : gid(g), sid(s) + RuleState(unsigned g, unsigned s, IpsPolicy::Action a, IpsPolicy::Enable e) : + gid(g), sid(s), action(a), enable(e) { policy = snort::get_ips_policy()->policy_id; } virtual ~RuleState() = default; void apply(snort::SnortConfig*); - virtual void update_rtn(RuleTreeNode*) = 0; + void update_rtn(RuleTreeNode*); private: unsigned gid; unsigned sid; unsigned policy; - void apply(snort::SnortConfig*, OptTreeNode* otn, unsigned ips_num); - RuleTreeNode* find_updated_rtn(RuleTreeNode*, snort::SnortConfig*, unsigned ips_num); - void replace_rtn(OptTreeNode*, RuleTreeNode*, snort::SnortConfig*, unsigned ips_num); - RuleTreeNode* dup_rtn(OptTreeNode*, snort::SnortConfig*, unsigned ips_num); -}; - -class RuleStateAction : public RuleState -{ -public: - RuleStateAction(unsigned g, unsigned s, IpsPolicy::Action a) : RuleState(g, s), action(a) { } - virtual void update_rtn(RuleTreeNode*) override; - -private: IpsPolicy::Action action; - -}; - -class RuleStateEnable : public RuleState -{ -public: - RuleStateEnable(unsigned g, unsigned s, IpsPolicy::Enable e) : RuleState(g, s), enable(e) { } - virtual void update_rtn(RuleTreeNode*) override; - -private: IpsPolicy::Enable enable; + + void apply(snort::SnortConfig*, OptTreeNode* otn, unsigned ips_num); + RuleTreeNode* dup_rtn(RuleTreeNode*); }; #endif diff --git a/src/detection/signature.cc b/src/detection/signature.cc index a7dc523a9..251cf90c6 100644 --- a/src/detection/signature.cc +++ b/src/detection/signature.cc @@ -182,8 +182,7 @@ void OtnFree(void* data) if ( otn->sigInfo.message ) { - if ( !otn->generated() ) - snort_free(otn->sigInfo.message); + snort_free(otn->sigInfo.message); } for (unsigned svc_idx = 0; svc_idx < otn->sigInfo.num_services; svc_idx++) { @@ -209,19 +208,6 @@ void OtnFree(void* data) if ( otn->soid ) snort_free(otn->soid); - /* RTN was generated on the fly. Don't necessarily know which policy - * at this point so go through all RTNs and delete them */ - if ( otn->generated() ) - { - for (int i = 0; i < otn->proto_node_num; i++) - { - RuleTreeNode* rtn = deleteRtnFromOtn(otn, i); - - if ( rtn ) - delete rtn; - } - } - if (otn->proto_nodes) snort_free(otn->proto_nodes); diff --git a/src/detection/treenodes.h b/src/detection/treenodes.h index 1618f9a54..26ae256af 100644 --- a/src/detection/treenodes.h +++ b/src/detection/treenodes.h @@ -135,11 +135,8 @@ struct RuleTreeNode struct OptTreeNode { using Flag = uint8_t; - static constexpr Flag GENERATED = 0x01; - static constexpr Flag WARNED_FP = 0x02; - static constexpr Flag ESTABLISHED = 0x04; - static constexpr Flag UNESTABLISHED = 0x08; - static constexpr Flag STATELESS = 0x10; + static constexpr Flag WARNED_FP = 0x01; + static constexpr Flag STATELESS = 0x02; /* metadata about signature */ SigInfo sigInfo; @@ -169,40 +166,20 @@ struct OptTreeNode // Added for integrity checks during rule parsing. SnortProtocolId snort_protocol_id = 0; - /**number of proto_nodes. */ unsigned short proto_node_num = 0; - uint16_t longestPatternLen = 0; Flag flags = 0; - void set_generated() - { flags |= GENERATED; } - - bool generated() const - { return flags & GENERATED; } - void set_warned_fp() { flags |= WARNED_FP; } bool warned_fp() const { return flags & WARNED_FP; } - void set_established() - { flags |= ESTABLISHED; } - - void set_unestablished() - { flags |= UNESTABLISHED; } - void set_stateless() { flags |= STATELESS; } - bool established() const - { return flags & ESTABLISHED; } - - bool unestablished() const - { return flags & UNESTABLISHED; } - bool stateless() const { return flags & STATELESS; } diff --git a/src/ips_options/ips_flow.cc b/src/ips_options/ips_flow.cc index a4c492752..eea65841d 100644 --- a/src/ips_options/ips_flow.cc +++ b/src/ips_options/ips_flow.cc @@ -412,12 +412,6 @@ static IpsOption* flow_ctor(Module* p, OptTreeNode* otn) if ( m->data.stateless ) otn->set_stateless(); - if ( m->data.established ) - otn->set_established(); - - if ( m->data.unestablished ) - otn->set_unestablished(); - if (otn->snort_protocol_id == SNORT_PROTO_ICMP) { if ( (m->data.only_reassembled != ONLY_FRAG) && diff --git a/src/main/modules.cc b/src/main/modules.cc index db95f977a..3ccdff864 100755 --- a/src/main/modules.cc +++ b/src/main/modules.cc @@ -1727,7 +1727,8 @@ bool RateFilterModule::end(const char*, int idx, SnortConfig* sc) static const Parameter single_rule_state_params[] = { - { "action", Parameter::PT_ENUM, "log | pass | alert | drop | block | reset | inherit", "inherit", + { "action", Parameter::PT_ENUM, + "log | pass | alert | drop | block | reset | inherit", "inherit", "apply action if rule matches or inherit from rule definition" }, { "enable", Parameter::PT_ENUM, "false | true | inherit", "inherit", @@ -1739,7 +1740,7 @@ static const Parameter single_rule_state_params[] = 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_TABLE, single_rule_state_params, nullptr, + { rule_state_gid_sid_regex, Parameter::PT_LIST, single_rule_state_params, nullptr, "defines rule state parameters for gid:sid", true }, { nullptr, Parameter::PT_MAX, nullptr, nullptr, nullptr } @@ -1752,13 +1753,21 @@ class RuleStateModule : public Module { public: RuleStateModule() : Module("rule_state", rule_state_help, rule_state_params, false) { } + bool set(const char*, Value&, SnortConfig*) override; + bool begin(const char*, int, SnortConfig*) override; + bool end(const char*, int, SnortConfig*) override; Usage get_usage() const override { return DETECT; } + +private: + unsigned gid, sid; + IpsPolicy::Action action; + IpsPolicy::Enable enable; }; -bool RuleStateModule::set(const char* fqn, Value& v, SnortConfig* sc) +bool RuleStateModule::set(const char* fqn, Value& v, SnortConfig*) { static regex gid_sid(rule_state_gid_sid_regex); @@ -1770,19 +1779,15 @@ bool RuleStateModule::set(const char* fqn, Value& v, SnortConfig* sc) if ( regex_search(fqn, match, gid_sid) ) { - unsigned gid = strtoul(match[1].str().c_str(), nullptr, 10); - unsigned sid = strtoul(match[2].str().c_str(), nullptr, 10); + gid = strtoul(match[1].str().c_str(), nullptr, 10); + sid = strtoul(match[2].str().c_str(), nullptr, 10); if ( v.is("action") ) - { - sc->rule_states.emplace_back( - new RuleStateAction(gid, sid, IpsPolicy::Action(v.get_uint8()))); - } + action = IpsPolicy::Action(v.get_uint8()); + else if ( v.is("enable") ) - { - sc->rule_states.emplace_back( - new RuleStateEnable(gid, sid, IpsPolicy::Enable(v.get_uint8()))); - } + enable = IpsPolicy::Enable(v.get_uint8()); + else return false; } @@ -1792,6 +1797,22 @@ bool RuleStateModule::set(const char* fqn, Value& v, SnortConfig* sc) return true; } +bool RuleStateModule::begin(const char*, int, SnortConfig*) +{ + gid = sid = 0; + action = IpsPolicy::Action::INHERIT_ACTION; + enable = IpsPolicy::Enable::INHERIT_ENABLE; + return true; +} + +bool RuleStateModule::end(const char*, int, SnortConfig* sc) +{ + if ( gid ) + sc->rule_states.emplace_back(new RuleState(gid, sid, action, enable)); + gid = 0; + return true; +} + //------------------------------------------------------------------------- // hosts module //------------------------------------------------------------------------- diff --git a/src/parser/parse_rule.cc b/src/parser/parse_rule.cc index 91587bed1..11aed2470 100644 --- a/src/parser/parse_rule.cc +++ b/src/parser/parse_rule.cc @@ -816,23 +816,13 @@ static RuleTreeNode* ProcessHeadNode( if ( !rtn ) { head_count++; - rtn = new RuleTreeNode; - rtn->otnRefCount++; - - /* copy the prototype header info into the new header block */ XferHeader(test_node, rtn); - - /* initialize the function list for the new RTN */ SetupRTNFuncList(rtn); - - /* add link to parent listhead */ rtn->listhead = list; - } else { - rtn->otnRefCount++; FreeRuleTreeNode(test_node); } @@ -878,13 +868,10 @@ static int mergeDuplicateOtn( // Add rtn to current otn for the first rule instance in a policy, // otherwise ignore it if ( !rtn_cur ) - { addRtnToOtn(sc, otn_cur, rtn_new); - } else - { DestroyRuleTreeNode(rtn_new); - } + return true; } @@ -913,9 +900,9 @@ static int mergeDuplicateOtn( ParseWarning(WARN_RULES, "%u:%u duplicates previous rule. Using revision %u.", otn_new->sigInfo.gid, otn_new->sigInfo.sid, otn_new->sigInfo.rev); } + DestroyRuleTreeNode(rtn_cur); } OtnRemove(sc->otn_map, otn_cur); - DestroyRuleTreeNode(rtn_cur); return false; } @@ -1180,7 +1167,6 @@ void parse_rule_close(SnortConfig* sc, RuleTreeNode& rtn, OptTreeNode* otn) * already a matching RTN. The portobjects will get freed when the * port var table is freed */ RuleTreeNode* new_rtn = ProcessHeadNode(sc, &rtn, rtn.listhead); - addRtnToOtn(sc, otn, new_rtn); OptTreeNode* otn_dup = diff --git a/src/parser/parser.cc b/src/parser/parser.cc index 2967fd8b8..1c3c97dda 100644 --- a/src/parser/parser.cc +++ b/src/parser/parser.cc @@ -70,31 +70,22 @@ static std::string s_aux_rules; static void FreeRuleTreeNodes(SnortConfig* sc) { - RuleTreeNode* rtn; - PolicyId policyId; - GHashNode* hashNode; - - if (sc->otn_map == nullptr) + if ( !sc->otn_map ) return; - for (hashNode = ghash_findfirst(sc->otn_map); + for ( GHashNode* hashNode = ghash_findfirst(sc->otn_map); hashNode; - hashNode = ghash_findnext(sc->otn_map)) + hashNode = ghash_findnext(sc->otn_map) ) { OptTreeNode* otn = (OptTreeNode*)hashNode->data; - /* Autogenerated OTNs along with their respective pseudo RTN - * will get cleaned up when the OTN is freed */ - if ( otn->generated() ) - continue; - - for (policyId = 0; - policyId < otn->proto_node_num; - policyId++) + for ( PolicyId policyId = 0; policyId < otn->proto_node_num; policyId++ ) { - rtn = getRtnFromOtn(otn, policyId); + RuleTreeNode* rtn = getRtnFromOtn(otn, policyId); + if ( !rtn ) + continue; + rtn->otnRefCount--; DestroyRuleTreeNode(rtn); - otn->proto_nodes[policyId] = nullptr; } } @@ -345,18 +336,13 @@ SnortConfig* ParseSnortConf(const SnortConfig* boot_conf, const char* fname, boo void FreeRuleTreeNode(RuleTreeNode* rtn) { - if (!rtn) - return; + assert(rtn and rtn->otnRefCount == 0); if (rtn->sip) - { sfvar_free(rtn->sip); - } if (rtn->dip) - { sfvar_free(rtn->dip); - } RuleFpList* idx = rtn->rule_func; @@ -370,15 +356,12 @@ void FreeRuleTreeNode(RuleTreeNode* rtn) void DestroyRuleTreeNode(RuleTreeNode* rtn) { - if (!rtn) - return; + assert(rtn); - rtn->otnRefCount--; if (rtn->otnRefCount != 0) return; FreeRuleTreeNode(rtn); - delete rtn; } @@ -572,31 +555,26 @@ void OrderRuleLists(SnortConfig* sc) sc->rule_lists = ordered_list; } -/**Delete rtn from OTN. - * - * @param otn pointer to structure OptTreeNode. - * @param policyId policy id - * - * @return pointer to deleted RTN, NULL otherwise. - */ RuleTreeNode* deleteRtnFromOtn(OptTreeNode* otn, PolicyId policyId, SnortConfig* sc, bool remove) { - if (otn->proto_nodes - && (otn->proto_node_num >= (policyId+1))) + if (otn->proto_nodes and (otn->proto_node_num >= (policyId+1))) { RuleTreeNode* rtn = getRtnFromOtn(otn, policyId); otn->proto_nodes[policyId] = nullptr; - if ( remove && rtn ) + if ( rtn ) { - RuleTreeNodeKey key { rtn, policyId }; - if ( sc && sc->rtn_hash_table ) + rtn->otnRefCount--; + + if ( remove ) + { + assert(sc and sc->rtn_hash_table); + RuleTreeNodeKey key { rtn, policyId }; xhash_remove(sc->rtn_hash_table, &key); + } } - return rtn; } - return nullptr; } @@ -643,14 +621,6 @@ static int rtn_compare_func(const void* k1, const void* k2, size_t) return 1; } -/**Add RTN to OTN for a particular OTN. - * @param otn pointer to structure OptTreeNode. - * @param policyId policy id - * @param rtn pointer to RuleTreeNode structure - * - * @return 0 if successful, - * -ve otherwise - */ int addRtnToOtn(SnortConfig* sc, OptTreeNode* otn, RuleTreeNode* rtn, PolicyId policyId) { if (otn->proto_node_num <= policyId) @@ -674,22 +644,20 @@ int addRtnToOtn(SnortConfig* sc, OptTreeNode* otn, RuleTreeNode* rtn, PolicyId p otn->proto_nodes = tmpNodeArray; } - //add policyId - if (otn->proto_nodes[policyId]) + RuleTreeNode* curr = otn->proto_nodes[policyId]; + + if ( curr ) { - DestroyRuleTreeNode(rtn); - return -1; + deleteRtnFromOtn(otn, policyId, sc, (curr->otnRefCount == 1)); + DestroyRuleTreeNode(curr); } - otn->proto_nodes[policyId] = rtn; - - // Optimized for parsing only, this check avoids adding run time rtn - if (!sc) - return 0; + rtn->otnRefCount++; if (!sc->rtn_hash_table) { - sc->rtn_hash_table = xhash_new(10000, sizeof(RuleTreeNodeKey), 0, 0, 0, nullptr, nullptr, 1); + sc->rtn_hash_table = xhash_new( + 10000, sizeof(RuleTreeNodeKey), 0, 0, 0, nullptr, nullptr, 1); if (sc->rtn_hash_table == nullptr) FatalError("Failed to create rule tree node hash table\n"); @@ -703,7 +671,7 @@ int addRtnToOtn(SnortConfig* sc, OptTreeNode* otn, RuleTreeNode* rtn, PolicyId p key.policyId = policyId; xhash_add(sc->rtn_hash_table, &key, rtn); - return 0; //success + return 0; } int addRtnToOtn(SnortConfig* sc, OptTreeNode* otn, RuleTreeNode* rtn) diff --git a/src/sfip/sf_ipvar.cc b/src/sfip/sf_ipvar.cc index be6b3fe2c..ab56e53dc 100644 --- a/src/sfip/sf_ipvar.cc +++ b/src/sfip/sf_ipvar.cc @@ -211,7 +211,7 @@ static inline sfip_node_t* _sfvar_deep_copy_list(const sfip_node_t* idx) } /* Deep copy. Returns identical, new, linked list of sfipnodes. */ -static sfip_var_t* sfvar_deep_copy(const sfip_var_t* var) +sfip_var_t* sfvar_deep_copy(const sfip_var_t* var) { sfip_var_t* ret; diff --git a/src/sfip/sf_ipvar.h b/src/sfip/sf_ipvar.h index 7952dcc56..d45bb18db 100644 --- a/src/sfip/sf_ipvar.h +++ b/src/sfip/sf_ipvar.h @@ -99,6 +99,7 @@ struct vartable_t /* Creates a new variable that is an alias of another variable * Does a "deep" copy so it owns it's own pointers */ +sfip_var_t* sfvar_deep_copy(const sfip_var_t*); sfip_var_t* sfvar_create_alias(const sfip_var_t* alias_from, const char* alias_to); /* Allocates a new variable as according to "str" */