Merge pull request #1634 in SNORT/snort3 from ~KAMURTHI/snort3:MySQL-Non-std-port...

Squashed commit of the following:

commit 71a85486671a4148e910d1a8d299876fc1f0d080
Author: cljudge <cljudge@cisco.com>
Date:   Thu Jun 6 08:38:17 2019 -0400

    appid: delay port-based detection until a non-zero payload packe is seen for the session

diff --git a/src/network_inspectors/appid/appid_discovery.cc b/src/network_inspectors/appid/appid_discovery.cc
index ced22d2f31696af1d0a49d42be07a96a13225acd..ee81b0ce3591e764db210b54aa1647770b176a35 100644 (file)
--- a/src/network_inspectors/appid/appid_discovery.cc
+++ b/src/network_inspectors/appid/appid_discovery.cc
@@ -765,6 +765,8 @@ bool AppIdDiscovery::do_pre_discovery(Packet* p, AppIdSession** p_asd, AppIdInsp
 
     asd->common.flags = flow_flags;
     asd->common.policyId = asd->config->appIdPolicyId;
+    if (!asd->get_session_flags(APPID_SESSION_PAYLOAD_SEEN) and p->dsize)
+        asd->set_session_flags(APPID_SESSION_PAYLOAD_SEEN);
 
     if (asd->get_session_flags(APPID_SESSION_IGNORE_FLOW))
     {
@@ -859,6 +861,43 @@ bool AppIdDiscovery::do_pre_discovery(Packet* p, AppIdSession** p_asd, AppIdInsp
     return true;
 }
 
+void AppIdDiscovery::do_port_based_discovery(Packet* p, AppIdSession& asd, IpProtocol protocol,
+    AppidSessionDirection direction)
+{
+    // Delay port-based detection until payload data is seen for the session. This ensures
+    // pattern-based detection gets higher priority than port-based detection.
+    // Do port-based detection only for responder packets.
+    if (asd.get_session_flags(APPID_SESSION_PORT_SERVICE_DONE) or
+        !asd.get_session_flags(APPID_SESSION_PAYLOAD_SEEN) or
+        (direction != APP_ID_FROM_RESPONDER))
+        return;
+
+    if (p->ptrs.tcph)
+    {
+        if (asd.get_session_flags(APPID_SESSION_SYN_RST))
+            return;
+
+        // we have to check for SYN/ACK here explicitly in case of TCP Fast Open
+        if (p->ptrs.tcph->is_syn_ack())
+            return;
+    }
+
+    AppId id = asd.config->get_port_service_id(protocol, p->ptrs.sp);
+    if (id > APP_ID_NONE)
+    {
+        asd.service.set_port_service_id(id);
+        if (appidDebug->is_active())
+        {
+            const char *app_name =
+                AppInfoManager::get_instance().get_app_name(asd.service.get_port_service_id());
+            LogMessage("AppIdDbg %s Port service %s (%d) from port\n",
+                appidDebug->get_debug_session(), app_name ? app_name : "unknown",
+                asd.service.get_port_service_id());
+        }
+    }
+    asd.set_session_flags(APPID_SESSION_PORT_SERVICE_DONE);
+}
+
 bool AppIdDiscovery::do_discovery(Packet* p, AppIdSession& asd, IpProtocol protocol,
     AppidSessionDirection direction, AppId& service_id, AppidChangeBits& change_bits)
 {
@@ -905,39 +944,7 @@ bool AppIdDiscovery::do_discovery(Packet* p, AppIdSession& asd, IpProtocol proto
 #endif
 
     // Port-based service detection
-    if ( !asd.get_session_flags(APPID_SESSION_PORT_SERVICE_DONE) )
-    {
-        switch (protocol)
-        {
-        case IpProtocol::TCP:
-            if (asd.get_session_flags(APPID_SESSION_SYN_RST)) // TCP-specific exception
-                break;
-        // fallthrough
-        case IpProtocol::UDP:
-            // Both TCP and UDP need this test to be made
-            // against only the p->src_port of the response.
-            // For all other cases the port parameter is never checked.
-            if (direction != APP_ID_FROM_RESPONDER)
-                break;
-        // fallthrough
-        default:
-        {
-            AppId id = asd.config->get_port_service_id(protocol, p->ptrs.sp);
-            if (id > APP_ID_NONE)
-            {
-                asd.service.set_port_service_id(id);
-                if (appidDebug->is_active())
-                {
-                    const char *app_name = AppInfoManager::get_instance().get_app_name(asd.service.get_port_service_id());
-                    LogMessage("AppIdDbg %s Port service %s (%d) from port\n",
-                        appidDebug->get_debug_session(), app_name ? app_name : "unknown", asd.service.get_port_service_id());
-                }
-            }
-            asd.set_session_flags(APPID_SESSION_PORT_SERVICE_DONE);
-        }
-        break;
-        }
-    }
+    do_port_based_discovery(p, asd, protocol, direction);
 
     // exceptions for rexec and any other service detector that need to see SYN and SYN/ACK
     if (asd.get_session_flags(APPID_SESSION_REXEC_STDERR))

diff --git a/src/network_inspectors/appid/appid_discovery.h b/src/network_inspectors/appid/appid_discovery.h
index bbc3a4322910f94a76cde2750fc743079e7258e0..4a0516aaa0025c3fdacb76194c82958937b6c636 100644 (file)
--- a/src/network_inspectors/appid/appid_discovery.h
+++ b/src/network_inspectors/appid/appid_discovery.h
@@ -139,6 +139,8 @@ private:
     static void do_post_discovery(snort::Packet* p, AppIdSession& asd,
         AppidSessionDirection direction, bool is_discovery_done, AppId service_id,
         AppidChangeBits& change_bits);
+    static void do_port_based_discovery(snort::Packet* p, AppIdSession& asd, IpProtocol protocol,
+        AppidSessionDirection direction);
     static bool handle_unmonitored_session(AppIdSession* asd, const snort::Packet* p,
         IpProtocol protocol, AppidSessionDirection dir, AppIdInspector& inspector,
         uint64_t& flow_flags);

diff --git a/src/network_inspectors/appid/appid_session_api.h b/src/network_inspectors/appid/appid_session_api.h
index d48ad9207668dc1afa817331d8a5732153667105..525a65f180176921a9a48fd231264bde9d9bce70 100644 (file)
--- a/src/network_inspectors/appid/appid_session_api.h
+++ b/src/network_inspectors/appid/appid_session_api.h
@@ -86,6 +86,7 @@ namespace snort
 #define APPID_SESSION_IGNORE_FLOW           (1ULL << 39)
 #define APPID_SESSION_IGNORE_FLOW_IDED      (1ULL << 40)
 #define APPID_SESSION_OOO_CHECK_TP          (1ULL << 41)
+#define APPID_SESSION_PAYLOAD_SEEN          (1ULL << 42)
 #define APPID_SESSION_IGNORE_ID_FLAGS \
     (APPID_SESSION_IGNORE_FLOW | \
     APPID_SESSION_NOT_A_SERVICE | \