From: Russ Combs (rucombs) Date: Fri, 4 Aug 2017 13:25:26 +0000 (-0400) Subject: Merge pull request #970 in SNORT/snort3 from port_reload_performance_fixes to master X-Git-Tag: 3.0.0-240~76 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=82808abfa706fe65006e598496d698ea1f161db6;p=thirdparty%2Fsnort3.git Merge pull request #970 in SNORT/snort3 from port_reload_performance_fixes to master Squashed commit of the following: commit def9b84906c82d9ebb5f8a85f4227e51520327f3 Author: Victor Roemer Date: Thu Aug 3 13:37:54 2017 -0400 dce_rpc: remove connection-oriented rules from dce_smb module commit 324a91db775c65c34a5b0cf435768d199368c20c Author: Victor Roemer Date: Thu Aug 3 12:44:41 2017 -0400 snort: keep tracking rtn after merging duplicate otns commit b79a20e1a45d052177ff78dff1fd4778dd956b41 Author: Victor Roemer Date: Wed Aug 2 14:20:50 2017 -0400 dce_rpc: Fix --enable-debug-msgs without --enable-debug commit e901cb79cb608603e109d199cc804e8eab285e83 Author: Victor Roemer Date: Wed Aug 2 14:20:21 2017 -0400 snort: Fix --enable-debug-msgs commit 733037605b804fd1a1585ff170e5da36dc39400f Author: Victor Roemer Date: Wed Jul 26 08:43:41 2017 -0400 snort: optimize mSplit performance commit f48509d9ce999a91fa1ade6009235ed32353338c Author: Victor Roemer Date: Tue Jul 25 10:26:09 2017 -0400 snort: Parse time RuleTreeNode hash table commit 85c3cd141b1578200c89ee451382508e9d83732a Author: Victor Roemer Date: Mon Jul 24 13:40:38 2017 -0400 snort: Stop iterating over ports earlier --- diff --git a/src/main/snort_config.cc b/src/main/snort_config.cc index 8979c0abd..44be55320 100644 --- a/src/main/snort_config.cc +++ b/src/main/snort_config.cc @@ -35,6 +35,7 @@ #include "filters/rate_filter.h" #include "filters/sfrf.h" #include "filters/sfthreshold.h" +#include "hash/sfxhash.h" #include "helpers/process.h" #include "ips_options/ips_pcre.h" #include "latency/latency_config.h" @@ -222,6 +223,9 @@ SnortConfig::~SnortConfig() fpDeleteFastPacketDetection(this); + if (rtn_hash_table) + sfxhash_delete(rtn_hash_table); + if (eth_dst ) snort_free(eth_dst); diff --git a/src/main/snort_config.h b/src/main/snort_config.h index 42914aed4..c2199f746 100644 --- a/src/main/snort_config.h +++ b/src/main/snort_config.h @@ -318,6 +318,7 @@ public: SFXHASH* detection_option_hash_table = nullptr; SFXHASH* detection_option_tree_hash_table = nullptr; + SFXHASH* rtn_hash_table = nullptr; PolicyMap* policy_map = nullptr; struct VarNode* var_list = nullptr; diff --git a/src/parser/mstring.cc b/src/parser/mstring.cc index 1be111f91..754943d2c 100644 --- a/src/parser/mstring.cc +++ b/src/parser/mstring.cc @@ -24,6 +24,8 @@ #include "mstring.h" +#include + #include "utils/util.h" static char* mSplitAddTok(const char*, const int, const char*, const char); @@ -75,45 +77,45 @@ char** mSplit(const char* str, const char* sep_chars, const int max_toks, char** retstr; const char* whitespace = " \t"; - if (num_toks == NULL) - return NULL; + assert(num_toks); + assert(str); *num_toks = 0; + size_t str_length = strlen(str); - if ((str == NULL) || (strlen(str) == 0) || - ((sep_chars != NULL) && (strlen(sep_chars) == 0))) - { + if (str_length == 0) return NULL; - } if (sep_chars == NULL) sep_chars = whitespace; + size_t sep_length = strlen(sep_chars); + /* Meta char cannot also be a separator char */ - for (i = 0; i < strlen(sep_chars); i++) + for (i = 0; i < sep_length; i++) { if (sep_chars[i] == meta_char) return NULL; } /* Move past initial separator characters and whitespace */ - for (i = 0; i < strlen(str); i++) + for (i = 0; i < str_length; i++) { - for (j = 0; j < strlen(sep_chars); j++) + if (isspace((int)str[i])) + continue; + + for (j = 0; j < sep_length; j++) { - if ((str[i] == sep_chars[j]) || - isspace((int)str[i])) - { + if (str[i] == sep_chars[j]) break; - } } /* Not a separator character or whitespace */ - if (j == strlen(sep_chars)) + if (j == sep_length) break; } - if (i == strlen(str)) + if (i == str_length) { /* Nothing but separator characters or whitespace in string */ return NULL; @@ -124,14 +126,14 @@ char** mSplit(const char* str, const char* sep_chars, const int max_toks, if ((cur_tok + 1) == (size_t)max_toks) { retstr = (char**)snort_calloc(sizeof(char*)); - retstr[cur_tok] = snort_strndup(&str[i], strlen(str) - i); + retstr[cur_tok] = snort_strndup(&str[i], str_length - i); *num_toks = cur_tok + 1; return retstr; } /* Mark the beginning of the next tok */ tok_start = i; - for (; i < strlen(str); i++) + for (; i < str_length; i++) { if (!escaped) { @@ -144,14 +146,14 @@ char** mSplit(const char* str, const char* sep_chars, const int max_toks, } /* See if the current character is a separator */ - for (j = 0; j < strlen(sep_chars); j++) + for (j = 0; j < sep_length; j++) { if (str[i] == sep_chars[j]) break; } /* It's a normal character */ - if (j == strlen(sep_chars)) + if (j == sep_length) continue; /* Current character matched a separator character. Trim off @@ -171,24 +173,24 @@ char** mSplit(const char* str, const char* sep_chars, const int max_toks, cur_tok++; /* Move past any more separator characters or whitespace */ - for (; i < strlen(str); i++) + for (; i < str_length; i++) { - for (j = 0; j < strlen(sep_chars); j++) + if (isspace((int)str[i])) + continue; + + for (j = 0; j < sep_length; j++) { - if ((str[i] == sep_chars[j]) || - isspace((int)str[i])) - { + if (str[i] == sep_chars[j]) break; - } } /* Not a separator character or whitespace */ - if (j == strlen(sep_chars)) + if (j == sep_length) break; } /* Nothing but separator characters or whitespace left in the string */ - if (i == strlen(str)) + if (i == str_length) { *num_toks = cur_tok; @@ -248,7 +250,7 @@ char** mSplit(const char* str, const char* sep_chars, const int max_toks, } /* Trim whitespace at end of last tok */ - for (j = strlen(str); j > tok_start; j--) + for (j = str_length; j > tok_start; j--) { if (!isspace((int)str[j - 1])) break; @@ -313,6 +315,7 @@ static char* mSplitAddTok( char* tok; int tok_len = 0; int got_meta = 0; + size_t sep_length = strlen(sep_chars); /* Get the length of the returned tok * Could have a maximum token length and use a fixed sized array and @@ -330,7 +333,7 @@ static char* mSplitAddTok( else { /* See if the current character is a separator */ - for (j = 0; j < strlen(sep_chars); j++) + for (j = 0; j < sep_length; j++) { if (str[i] == sep_chars[j]) break; @@ -338,7 +341,7 @@ static char* mSplitAddTok( /* It's a non-separator character, so include * the meta character in the return tok */ - if (j == strlen(sep_chars)) + if (j == sep_length) tok_len++; got_meta = 0; @@ -363,7 +366,7 @@ static char* mSplitAddTok( else { /* See if the current character is a separator */ - for (j = 0; j < strlen(sep_chars); j++) + for (j = 0; j < sep_length; j++) { if (str[i] == sep_chars[j]) break; @@ -371,7 +374,7 @@ static char* mSplitAddTok( /* It's a non-separator character, so include * the meta character in the return tok */ - if (j == strlen(sep_chars)) + if (j == sep_length) tok[k++] = meta_char; got_meta = 0; diff --git a/src/parser/parse_rule.cc b/src/parser/parse_rule.cc index 40f2427d2..61aa0f43e 100644 --- a/src/parser/parse_rule.cc +++ b/src/parser/parse_rule.cc @@ -29,6 +29,7 @@ #include "detection/rtn_checks.h" #include "detection/treenodes.h" #include "framework/decode_data.h" +#include "hash/sfxhash.h" #include "log/messages.h" #include "main/snort_config.h" #include "main/snort_debug.h" @@ -645,7 +646,7 @@ static int ParsePortList( return 0; } -static bool same_headers(RuleTreeNode* rule, RuleTreeNode* rtn) +bool same_headers(RuleTreeNode* rule, RuleTreeNode* rtn) { if ( !rule or !rtn ) return false; @@ -685,17 +686,10 @@ static bool same_headers(RuleTreeNode* rule, RuleTreeNode* rtn) static RuleTreeNode* findHeadNode( SnortConfig* sc, RuleTreeNode* testNode, PolicyId policyId) { - SFGHASH_NODE* hashNode; - - for (hashNode = sfghash_findfirst(sc->otn_map); - hashNode; - hashNode = sfghash_findnext(sc->otn_map)) + if ( sc->rtn_hash_table ) { - OptTreeNode* otn = (OptTreeNode*)hashNode->data; - RuleTreeNode* rtn = getRtnFromOtn(otn, policyId); - - if (same_headers(rtn, testNode)) - return rtn; + RuleTreeNodeKey key { testNode, policyId }; + return (RuleTreeNode*)sfxhash_find(sc->rtn_hash_table, &key); } return nullptr; @@ -933,7 +927,7 @@ static int mergeDuplicateOtn( { // current OTN is newer version. Keep current and discard the new one. // OTN is for new policy group, salvage RTN - deleteRtnFromOtn(otn_new); + deleteRtnFromOtn(otn_new, sc); ParseWarning(WARN_RULES, "%u:%u duplicates previous rule. Using revision %u.", otn_cur->sigInfo.gid, otn_cur->sigInfo.sid, otn_cur->sigInfo.rev); @@ -947,7 +941,7 @@ static int mergeDuplicateOtn( // otherwise ignore it if ( !rtn_cur ) { - addRtnToOtn(otn_cur, rtn_new); + addRtnToOtn(sc, otn_cur, rtn_new); } else { @@ -960,11 +954,11 @@ static int mergeDuplicateOtn( for ( unsigned i = 0; i < otn_cur->proto_node_num; ++i ) { - RuleTreeNode* rtnTmp2 = deleteRtnFromOtn(otn_cur, i); + RuleTreeNode* rtnTmp2 = deleteRtnFromOtn(otn_cur, i, sc, (rtn_cur != rtn_new)); if ( rtnTmp2 and (i != get_ips_policy()->policy_id) ) { - addRtnToOtn(otn_new, rtnTmp2, i); + addRtnToOtn(sc, otn_new, rtnTmp2, i); } } @@ -1233,7 +1227,7 @@ const char* parse_rule_close(SnortConfig* sc, RuleTreeNode& rtn, OptTreeNode* ot * port var table is freed */ RuleTreeNode* new_rtn = ProcessHeadNode(sc, &rtn, rtn.listhead); - addRtnToOtn(otn, new_rtn); + addRtnToOtn(sc, otn, new_rtn); OptTreeNode* otn_dup = OtnLookup(sc->otn_map, otn->sigInfo.gid, otn->sigInfo.sid); diff --git a/src/parser/parser.cc b/src/parser/parser.cc index 133184bfe..de5b7901c 100644 --- a/src/parser/parser.cc +++ b/src/parser/parser.cc @@ -33,6 +33,8 @@ #include "filters/detection_filter.h" #include "filters/rate_filter.h" #include "filters/sfthreshold.h" +#include "hash/sfhashfcn.h" +#include "hash/sfxhash.h" #include "log/messages.h" #include "main/shell.h" #include "main/snort_config.h" @@ -749,25 +751,70 @@ void OrderRuleLists(SnortConfig* sc, const char* order) * * @return pointer to deleted RTN, NULL otherwise. */ -RuleTreeNode* deleteRtnFromOtn(OptTreeNode* otn, PolicyId policyId) +RuleTreeNode* deleteRtnFromOtn(OptTreeNode* otn, PolicyId policyId, SnortConfig* sc, bool remove) { - RuleTreeNode* rtn = NULL; + RuleTreeNode* rtn = nullptr; if (otn->proto_nodes && (otn->proto_node_num >= (policyId+1))) { rtn = getRtnFromOtn(otn, policyId); - otn->proto_nodes[policyId] = NULL; + otn->proto_nodes[policyId] = nullptr; + + if ( remove && rtn ) + { + RuleTreeNodeKey key{ rtn, policyId }; + if ( sc && sc->rtn_hash_table ) + sfxhash_remove(sc->rtn_hash_table, &key); + } return rtn; } - return NULL; + return nullptr; +} + +RuleTreeNode* deleteRtnFromOtn(OptTreeNode* otn, SnortConfig* sc) +{ + return deleteRtnFromOtn(otn, get_ips_policy()->policy_id, sc); +} + +uint32_t rtn_hash_func(SFHASHFCN*, unsigned char *k, int) +{ + uint32_t a,b,c; + RuleTreeNodeKey* rtnk = (RuleTreeNodeKey*)k; + RuleTreeNode* rtn = rtnk->rtn; + + a = rtn->type; + b = rtn->flags; + c = (uint32_t)(uintptr_t)rtn->listhead; + + mix(a,b,c); + + a += (uint32_t)(uintptr_t)rtn->src_portobject; + b += (uint32_t)(uintptr_t)rtn->dst_portobject; + c += (uint32_t)(uintptr_t)rtnk->policyId; + + finalize(a,b,c); + + return c; } -RuleTreeNode* deleteRtnFromOtn(OptTreeNode* otn) +int rtn_compare_func(const void *k1, const void *k2, size_t) { - return deleteRtnFromOtn(otn, get_ips_policy()->policy_id); + RuleTreeNodeKey* rtnk1 = (RuleTreeNodeKey*)k1; + RuleTreeNodeKey* rtnk2 = (RuleTreeNodeKey*)k2; + + if (!rtnk1 || !rtnk2) + return 1; + + if (rtnk1->policyId != rtnk2->policyId) + return 1; + + if (same_headers(rtnk1->rtn, rtnk2->rtn)) + return 0; + + return 1; } /**Add RTN to OTN for a particular OTN. @@ -778,7 +825,7 @@ RuleTreeNode* deleteRtnFromOtn(OptTreeNode* otn) * @return 0 if successful, * -ve otherwise */ -int addRtnToOtn(OptTreeNode* otn, RuleTreeNode* rtn, PolicyId policyId) +int addRtnToOtn(SnortConfig* sc, OptTreeNode* otn, RuleTreeNode* rtn, PolicyId policyId) { if (otn->proto_node_num <= policyId) { @@ -810,12 +857,32 @@ int addRtnToOtn(OptTreeNode* otn, RuleTreeNode* rtn, PolicyId policyId) otn->proto_nodes[policyId] = rtn; + // Optimized for parsing only, this check avoids adding run time rtn + if (!sc) + return 0; + + if (!sc->rtn_hash_table) + { + sc->rtn_hash_table = sfxhash_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"); + + sfxhash_set_keyops(sc->rtn_hash_table, rtn_hash_func, rtn_compare_func); + } + + RuleTreeNodeKey key; + memset(&key, 0, sizeof(key)); + key.rtn = rtn; + key.policyId = policyId; + sfxhash_add(sc->rtn_hash_table, &key, rtn); + return 0; //success } -int addRtnToOtn(OptTreeNode* otn, RuleTreeNode* rtn) +int addRtnToOtn(SnortConfig*sc, OptTreeNode* otn, RuleTreeNode* rtn) { - return addRtnToOtn(otn, rtn, get_ips_policy()->policy_id); + return addRtnToOtn(sc, otn, rtn, get_ips_policy()->policy_id); } void rule_index_map_print_index(int index, char* buf, int bufsize) diff --git a/src/parser/parser.h b/src/parser/parser.h index a450ec25e..54daa8769 100644 --- a/src/parser/parser.h +++ b/src/parser/parser.h @@ -53,11 +53,12 @@ void parser_append_rules(const char*); int ParseBool(const char* arg); -int addRtnToOtn(struct OptTreeNode*, RuleTreeNode*); -int addRtnToOtn(struct OptTreeNode*, RuleTreeNode*, PolicyId); +int addRtnToOtn(SnortConfig*, struct OptTreeNode*, RuleTreeNode*); +int addRtnToOtn(SnortConfig*, struct OptTreeNode*, RuleTreeNode*, PolicyId); -RuleTreeNode* deleteRtnFromOtn(struct OptTreeNode*); -RuleTreeNode* deleteRtnFromOtn(struct OptTreeNode*, PolicyId); +bool same_headers(RuleTreeNode*, RuleTreeNode*); +RuleTreeNode* deleteRtnFromOtn(OptTreeNode*, SnortConfig* sc = nullptr); +RuleTreeNode* deleteRtnFromOtn(struct OptTreeNode*, PolicyId, SnortConfig* sc = nullptr, bool remove = true); inline RuleTreeNode* getRtnFromOtn(const struct OptTreeNode* otn, PolicyId policyId) { @@ -87,5 +88,11 @@ int parser_get_rule_index(unsigned gid, unsigned sid); void parser_get_rule_ids(int index, unsigned& gid, unsigned& sid); void rule_index_map_print_index(int index, char* buf, int); +struct RuleTreeNodeKey +{ + RuleTreeNode* rtn; + PolicyId policyId; +}; + #endif diff --git a/src/ports/port_table.cc b/src/ports/port_table.cc index 5fa49189b..b1229fbc9 100644 --- a/src/ports/port_table.cc +++ b/src/ports/port_table.cc @@ -718,7 +718,7 @@ static int PortTableCompileMergePortObjects(PortTable* p) // consistency check - part 1 // make sure each port is only in one composite port object -static void PortTableConsistencyCheck(PortTable* p) +static bool PortTableConsistencyCheck(PortTable* p) { std::unique_ptr upA(new char[SFPO_MAX_PORTS]); char* parray = upA.get(); @@ -732,7 +732,8 @@ static void PortTableConsistencyCheck(PortTable* p) if ( !po ) { - FatalError("PortObject consistency Check failed, hash table problem\n"); + DebugMessage(DEBUG_PORTLISTS, "PortObject consistency Check failed, hash table problem\n"); + return false; } if ( !po->port_cnt ) /* port object is not used ignore it */ @@ -744,16 +745,16 @@ static void PortTableConsistencyCheck(PortTable* p) { if ( parray[i] ) { - FatalError("PortTableCompile: failed consistency check, " - "multiple objects reference port %d\n", i); + DebugFormat(DEBUG_PORTLISTS, "PortTableCompile: failed consistency check, " + "multiple objects reference port %d\n", i); + return false; } parray[i] = 1; } } } - DebugMessage(DEBUG_PORTLISTS, - "***\n***Port Table Compiler Consistency Check Phase-I Passed !\n"); + return true; } // consistency check - part 2 @@ -765,7 +766,7 @@ static void PortTableConsistencyCheck(PortTable* p) * check that each port it reference has all of the rules * referenced to that port in the composite object */ -static void PortTableConsistencyCheck2(PortTable* p) +static bool PortTableConsistencyCheck2(PortTable* p) { SF_LNODE* pos; PortObject2* lastpo = nullptr; @@ -796,8 +797,9 @@ static void PortTableConsistencyCheck2(PortTable* p) { if ( _po2_include_po_rules(p->pt_port_object[i], ipo) ) { - FatalError("InputPortObject<->CompositePortObject " - "consistency Check II failed\n"); + DebugMessage(DEBUG_PORTLISTS, + "InputPortObject<->CompositePortObject consistency Check II failed\n"); + return false; } lastpo = p->pt_port_object[i]; } @@ -805,9 +807,7 @@ static void PortTableConsistencyCheck2(PortTable* p) } } - DebugMessage(DEBUG_PORTLISTS, - "***\n***Port Table Compiler Consistency Check Phase-II Passed !!!" - " - Good to go Houston\n****\n"); + return true; } //------------------------------------------------------------------------- @@ -934,8 +934,8 @@ int PortTableCompile(PortTable* p) DEBUG_WRAP(DebugMessage(DEBUG_PORTLISTS, "Done\n"); fflush(stdout); ); - PortTableConsistencyCheck(p); - PortTableConsistencyCheck2(p); + assert(PortTableConsistencyCheck(p)); + assert(PortTableConsistencyCheck2(p)); return 0; } diff --git a/src/ports/port_utils.cc b/src/ports/port_utils.cc index a0e4c879a..68d939884 100644 --- a/src/ports/port_utils.cc +++ b/src/ports/port_utils.cc @@ -123,18 +123,15 @@ int PortObjectBits(PortBitSet& parray, PortObject* po) return cnt; } -/* - * Make a list of ports form the char array, each char is either - * on or off. - */ SF_LIST* PortObjectItemListFromBits(const PortBitSet& parray, int n) { SF_LIST* plist = sflist_new(); + int nports = parray.count(); if ( !plist ) return 0; - for (int i = 0; i < n; i++) + for (int i = 0; i < n && nports > 0; i++) { if ( parray[i] == 0 ) continue; @@ -143,12 +140,14 @@ SF_LIST* PortObjectItemListFromBits(const PortBitSet& parray, int n) /* Either a port or the start of a range */ lport = hport = i; + nports--; for (i++; i