From: Russ Combs (rucombs) <rucombs@cisco.com>
Date: Mon, 29 Jan 2018 22:57:13 +0000 (-0500)
Subject: Merge pull request #1101 in SNORT/snort3 from fp_select to master
X-Git-Tag: 3.0.0-243~4
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=79659b76f1c0af338e15e41a97c99e247a6c2a2d;p=thirdparty%2Fsnort3.git

Merge pull request #1101 in SNORT/snort3 from fp_select to master

Squashed commit of the following:

commit 6a2533921c36f5e481c42f22106686880e540536
Author: Russ Combs (rucombs) <rucombs@cisco.com>
Date:   Mon Jan 29 10:52:43 2018 -0500

    detection: trace fast pattern searches with 0x20

commit ce128ff0b0c8c91423dfaba45e473eecbc4a78bd
Author: Russ Combs (rucombs) <rucombs@cisco.com>
Date:   Sun Jan 28 12:08:18 2018 -0500

    rules: update fast pattern selection to exclude redundant port groups when service groups are present
---

diff --git a/src/detection/detect_trace.h b/src/detection/detect_trace.h
index f90a0f4c4..9039e1a61 100644
--- a/src/detection/detect_trace.h
+++ b/src/detection/detect_trace.h
@@ -35,7 +35,8 @@ enum
     TRACE_RULE_EVAL = 0x2,
     TRACE_BUFFER_MINIMAL = 0x4,
     TRACE_BUFFER_VERBOSE = 0x8,
-    TRACE_RULE_VARS = 0x10
+    TRACE_RULE_VARS = 0x10,
+    TRACE_FP_SEARCH = 0x20
 };
 
 void clear_trace_cursor_info();
diff --git a/src/detection/fp_create.cc b/src/detection/fp_create.cc
index d568caa91..228530457 100644
--- a/src/detection/fp_create.cc
+++ b/src/detection/fp_create.cc
@@ -513,8 +513,8 @@ static int fpAddPortGroupRule(
         return -1;
 
     OptFpList* next = nullptr;
-    bool only_literal = !MpseManager::is_regex_capable(fp->get_search_api());
-    pmv = get_fp_content(otn, next, srvc, only_literal);
+    bool exclude, only_literal = !MpseManager::is_regex_capable(fp->get_search_api());
+    pmv = get_fp_content(otn, next, srvc, only_literal, exclude);
 
     if ( !pmv.empty() )
     {
@@ -540,6 +540,9 @@ static int fpAddPortGroupRule(
         }
     }
 
+    if ( exclude )
+        return 0;
+
     // no fast pattern added
     if (fpFinishPortGroupRule(sc, pg, otn, nullptr, fp) != 0)
         return -1;
@@ -1183,7 +1186,7 @@ static void fpBuildServicePortGroupByServiceOtnList(
     s_group = srvc;
 
     /*
-     * add each rule to the port group pattern matchers,
+     * add each rule to the service group pattern matchers,
      * or to the no-content rule list
      */
     SF_LNODE* cursor;
@@ -1192,8 +1195,7 @@ static void fpBuildServicePortGroupByServiceOtnList(
         otn;
         otn = (OptTreeNode*)sflist_next(&cursor) )
     {
-        if (fpAddPortGroupRule(sc, pg, otn, fp, true) != 0)
-            continue;
+        fpAddPortGroupRule(sc, pg, otn, fp, true);
     }
 
     if (fpFinishPortGroup(sc, pg, fp) != 0)
diff --git a/src/detection/fp_detect.cc b/src/detection/fp_detect.cc
index e51c127fe..7be9a84f5 100644
--- a/src/detection/fp_detect.cc
+++ b/src/detection/fp_detect.cc
@@ -886,9 +886,8 @@ static inline int search_buffer(
     {
         if ( Mpse* so = omd->pg->mpse[pmt] )
         {
-            trace_logf(detection, TRACE_RULE_EVAL,
-                "inspector %s, buffer type %s\n",
-                gadget->get_name(),pm_type_strings[pmt]);
+            trace_logf(detection, TRACE_FP_SEARCH, "%ld fp %s.%s[%d]\n",
+                pc.total_from_daq, gadget->get_name(), pm_type_strings[pmt], buf.len);
 
             search_data(so, omd, buf.data, buf.len, cnt);
         }
@@ -921,10 +920,13 @@ static int fp_search(
                 pattern_match_size = p->alt_dsize;
 
             if ( pattern_match_size )
-                search_data(so, omd, p->data, pattern_match_size, pc.pkt_searches);
+            {
+                trace_logf(detection, TRACE_FP_SEARCH, "%ld fp %s[%d]\n",
+                    pc.total_from_daq, pm_type_strings[PM_TYPE_PKT], pattern_match_size);
 
-            if ( pattern_match_size )
+                search_data(so, omd, p->data, pattern_match_size, pc.pkt_searches);
                 p->is_cooked() ?  pc.cooked_searches++ : pc.raw_searches++;
+            }
         }
     }
 
@@ -958,7 +960,9 @@ static int fp_search(
 
             if ( file_data.len )
             {
-                trace_log(detection, TRACE_RULE_EVAL, "Searching file data\n");
+                trace_logf(detection, TRACE_FP_SEARCH, "%ld fp search %s[%d]\n",
+                    pc.total_from_daq, pm_type_strings[PM_TYPE_FILE], file_data.len);
+
                 search_data(so, omd, file_data.data, file_data.len, pc.file_searches);
             }
         }
diff --git a/src/detection/fp_utils.cc b/src/detection/fp_utils.cc
index 2efbef810..db05595ee 100644
--- a/src/detection/fp_utils.cc
+++ b/src/detection/fp_utils.cc
@@ -240,7 +240,7 @@ FpSelector::FpSelector(CursorActionType c, PatternMatchData* p)
 }
 
 bool FpSelector::is_better_than(
-    FpSelector& rhs, bool srvc, RuleDirection dir, bool only_literals)
+    FpSelector& rhs, bool /*srvc*/, RuleDirection /*dir*/, bool only_literals)
 {
     if ( !pmd_can_be_fp(pmd, cat, only_literals) )
     {
@@ -255,22 +255,6 @@ bool FpSelector::is_better_than(
     if ( !rhs.pmd )
         return true;
 
-    if ( !srvc )
-    {
-        if ( cat == CAT_SET_RAW and rhs.cat != CAT_SET_RAW )
-            return true;
-
-        if ( cat != CAT_SET_RAW and rhs.cat == CAT_SET_RAW )
-            return false;
-    }
-    else if ( dir == RULE_FROM_SERVER )
-    {
-        if ( cat != CAT_SET_KEY and rhs.cat == CAT_SET_KEY )
-            return true;
-
-        if ( cat == CAT_SET_KEY and rhs.cat != CAT_SET_KEY )
-            return false;
-    }
     if ( pmd->is_fast_pattern() )
     {
         if ( rhs.pmd->is_fast_pattern() )
@@ -299,7 +283,7 @@ bool FpSelector::is_better_than(
 //--------------------------------------------------------------------------
 
 PatternMatchVector get_fp_content(
-    OptTreeNode* otn, OptFpList*& next, bool srvc, bool only_literals)
+    OptTreeNode* otn, OptFpList*& next, bool srvc, bool only_literals, bool& exclude)
 {
     CursorActionType curr_cat = CAT_SET_RAW;
     FpSelector best;
@@ -349,6 +333,14 @@ PatternMatchVector get_fp_content(
         }
     }
 
+    if ( best.pmd and best.cat != CAT_SET_RAW and !srvc and otn->sigInfo.num_services > 0 )
+    {
+        pmds.clear();  // just include in service group
+        exclude = true;
+    }
+    else
+        exclude = false;
+
     if ( best.pmd and otn->proto == SNORT_PROTO_FILE and best.cat != CAT_SET_FILE )
     {
         ParseWarning(WARN_RULES, "file rule %u:%u does not have file_data fast pattern",
@@ -494,7 +486,7 @@ TEST_CASE("fp_cat2", "[FastPatternSelect]")
     set_pmd(p1, 0x0, "foo");
     FpSelector s1(CAT_SET_FILE, &p1);
 
-    CHECK(s0.is_better_than(s1, false, RULE_WO_DIR));
+    CHECK(!s0.is_better_than(s1, false, RULE_WO_DIR));
     CHECK(!s1.is_better_than(s0, false, RULE_WO_DIR));
 }
 
@@ -534,7 +526,7 @@ TEST_CASE("fp_pkt_key_port", "[FastPatternSelect]")
     set_pmd(p1, 0x0, "longer");
     FpSelector s1(CAT_SET_KEY, &p1);
 
-    CHECK(s0.is_better_than(s1, false, RULE_WO_DIR));
+    CHECK(!s0.is_better_than(s1, false, RULE_WO_DIR));
 }
 
 TEST_CASE("fp_pkt_key_port_user", "[FastPatternSelect]")
@@ -612,8 +604,8 @@ TEST_CASE("fp_pkt_key_srvc_rsp", "[FastPatternSelect]")
     set_pmd(p1, 0x0, "longer");
     FpSelector s1(CAT_SET_KEY, &p1);
 
-    CHECK(s0.is_better_than(s1, true, RULE_FROM_SERVER));
-    CHECK(!s1.is_better_than(s0, true, RULE_FROM_SERVER));
+    CHECK(!s0.is_better_than(s1, true, RULE_FROM_SERVER));
+    CHECK(s1.is_better_than(s0, true, RULE_FROM_SERVER));
 }
 #endif
 
diff --git a/src/detection/fp_utils.h b/src/detection/fp_utils.h
index 3e0f1ff2b..70a96c9ec 100644
--- a/src/detection/fp_utils.h
+++ b/src/detection/fp_utils.h
@@ -36,7 +36,7 @@ int flp_trim(const char* p, int plen, const char** buff);
 bool set_fp_content(OptTreeNode*);
 
 std::vector <PatternMatchData*> get_fp_content(
-    OptTreeNode*, OptFpList*&, bool srvc, bool only_literals);
+    OptTreeNode*, OptFpList*&, bool srvc, bool only_literals, bool& exclude);
 
 #endif