Merge pull request #1866 in SNORT/snort3 from ~KAMURTHI/snort3:shell-application...

Squashed commit of the following:

commit 28d4e06573721c4a1beb02dbb3e100d050d61e14
Author: Kanimozhi Murthi <kamurthi@cisco.com>
Date:   Wed Nov 20 19:57:27 2019 -0500

    appid: Fix for better classification on pinholed data session and control session for Rshell/rexec.

diff --git a/src/network_inspectors/appid/appid_discovery.cc b/src/network_inspectors/appid/appid_discovery.cc
index 4f070ae505839d7de19bfea9a073fb985dd8af09..168de593ccf075fcac1faee038894e7db0944a69 100644 (file)
--- a/src/network_inspectors/appid/appid_discovery.cc
+++ b/src/network_inspectors/appid/appid_discovery.cc
@@ -990,19 +990,14 @@ bool AppIdDiscovery::do_discovery(Packet* p, AppIdSession& asd, IpProtocol proto
     if (asd.get_session_flags(APPID_SESSION_REXEC_STDERR))
     {
         ServiceDiscovery::get_instance().identify_service(asd, p, direction, change_bits);
-        AppIdDnsSession* dsession = asd.get_dns_session();
 
-        if (asd.service.get_id() == APP_ID_DNS &&
-            asd.config->mod_config->dns_host_reporting && dsession->get_host() )
+        if (asd.get_session_flags(APPID_SESSION_SERVICE_DETECTED |
+            APPID_SESSION_CONTINUE) == APPID_SESSION_SERVICE_DETECTED)
         {
-            dns_host_scan_hostname((const uint8_t*)dsession->get_host(), dsession->get_host_len(),
-                &client_id, &payload_id);
-            asd.set_client_appid_data(client_id, change_bits);
+            asd.service_disco_state = APPID_DISCO_STATE_FINISHED;
+            if ( asd.payload.get_id() == APP_ID_NONE)
+                asd.payload.set_id(APP_ID_UNKNOWN);
         }
-        else if (asd.service.get_id() == APP_ID_RTMP)
-            asd.examine_rtmp_metadata(change_bits);
-        else if (asd.get_session_flags(APPID_SESSION_SSL_SESSION) && asd.tsession)
-            asd.examine_ssl_metadata(p, change_bits);
     }
     // FIXIT-M - snort 2.x has added a check for midstream pickup to this, do we need that?
     else if (protocol != IpProtocol::TCP || (p->packet_flags & PKT_STREAM_ORDER_OK))

diff --git a/src/network_inspectors/appid/service_plugins/service_rexec.cc b/src/network_inspectors/appid/service_plugins/service_rexec.cc
index b48d372e55354515f42d22172cb066fad7b10e73..b13d7104b38ef76a0602e8273c0adc58fc1a9c88 100644 (file)
--- a/src/network_inspectors/appid/service_plugins/service_rexec.cc
+++ b/src/network_inspectors/appid/service_plugins/service_rexec.cc
@@ -25,6 +25,7 @@
 
 #include "service_rexec.h"
 
+#include "appid_debug.h"
 #include "appid_inspector.h"
 #include "app_info_table.h"
 #include "protocols/packet.h"
@@ -43,8 +44,11 @@ enum REXECState
     REXEC_STATE_COMMAND,
     REXEC_STATE_REPLY,
     REXEC_STATE_DONE,
+    REXEC_STATE_BAIL,
     REXEC_STATE_STDERR_CONNECT_SYN,
-    REXEC_STATE_STDERR_CONNECT_SYN_ACK
+    REXEC_STATE_STDERR_CONNECT_SYN_ACK,
+    REXEC_STATE_STDERR_WAIT,
+    REXEC_STATE_STDERR_DONE
 };
 
 struct ServiceREXECData
@@ -55,8 +59,8 @@ struct ServiceREXECData
 };
 
 static const uint64_t REXEC_EXPECTED_SESSION_FLAGS = APPID_SESSION_CONTINUE |
-    APPID_SESSION_REXEC_STDERR | APPID_SESSION_NO_TPI | APPID_SESSION_SERVICE_DETECTED |
-    APPID_SESSION_NOT_A_SERVICE | APPID_SESSION_PORT_SERVICE_DONE;
+    APPID_SESSION_REXEC_STDERR | APPID_SESSION_NO_TPI | APPID_SESSION_NOT_A_SERVICE |
+    APPID_SESSION_PORT_SERVICE_DONE;
 
 RexecServiceDetector::RexecServiceDetector(ServiceDiscovery* sd)
 {
@@ -99,6 +103,18 @@ static void rexec_free_state(void* data)
     }
 }
 
+void RexecServiceDetector::rexec_bail(ServiceREXECData* rd)
+{
+    if (!rd) 
+        return;
+    rd->state = REXEC_STATE_BAIL;
+    if(rd->child)
+        rd->child->state = REXEC_STATE_BAIL;
+
+    if(rd->parent)
+        rd->parent->state = REXEC_STATE_BAIL;
+}
+
 int RexecServiceDetector::validate(AppIdDiscoveryArgs& args)
 {
     int i = 0;
@@ -117,6 +133,8 @@ int RexecServiceDetector::validate(AppIdDiscoveryArgs& args)
         data_add(args.asd, rd, &rexec_free_state);
         rd->state = REXEC_STATE_PORT;
     }
+    if (appidDebug->is_active())
+        LogMessage("AppIdDbg %s rexec state %d\n", appidDebug->get_debug_session(), rd->state);
 
     switch (rd->state)
     {
@@ -164,12 +182,14 @@ int RexecServiceDetector::validate(AppIdDiscoveryArgs& args)
                     tmp_rd->parent = nullptr;
                     return APPID_ENULL;
                 }
-                rd->child = tmp_rd;
-                rd->state = REXEC_STATE_SERVER_CONNECT;
                 pf->service_disco_state = APPID_DISCO_STATE_STATEFUL;
                 pf->scan_flags |= SCAN_HOST_PORT_FLAG;
                 initialize_expected_session(args.asd, *pf, REXEC_EXPECTED_SESSION_FLAGS, APP_ID_FROM_RESPONDER);
                 pf->service_disco_state = APPID_DISCO_STATE_STATEFUL;
+                rd->child = tmp_rd;
+                rd->state = REXEC_STATE_SERVER_CONNECT;
+                args.asd.set_session_flags(APPID_SESSION_CONTINUE);
+                goto success;
             }
             else
                 rd->state = REXEC_STATE_USERNAME;
@@ -251,8 +271,15 @@ int RexecServiceDetector::validate(AppIdDiscoveryArgs& args)
             goto fail;
         if (size != 1)
             goto fail;
+        if (rd->child)
+        {
+            if(rd->child->state == REXEC_STATE_STDERR_WAIT)
+                rd->child->state = REXEC_STATE_STDERR_DONE;
+            else
+                goto fail;
+        }
+        args.asd.clear_session_flags(APPID_SESSION_CONTINUE);
         goto success;
-        break;
     case REXEC_STATE_STDERR_CONNECT_SYN:
         rd->state = REXEC_STATE_STDERR_CONNECT_SYN_ACK;
         break;
@@ -260,32 +287,40 @@ int RexecServiceDetector::validate(AppIdDiscoveryArgs& args)
         if (rd->parent && rd->parent->state == REXEC_STATE_SERVER_CONNECT)
         {
             rd->parent->state = REXEC_STATE_USERNAME;
-            args.asd.clear_session_flags(APPID_SESSION_REXEC_STDERR);
+            rd->state = REXEC_STATE_STDERR_WAIT;
+            break;
         }
         goto bail;
+    case REXEC_STATE_STDERR_WAIT:
+        if(!size) 
+            break;
+        goto bail;
+    case REXEC_STATE_STDERR_DONE:
+        args.asd.clear_session_flags(APPID_SESSION_REXEC_STDERR | APPID_SESSION_CONTINUE);
+        goto success;
+    case REXEC_STATE_BAIL:
     default:
         goto bail;
     }
 
 inprocess:
-    if (!args.asd.is_service_detected())
-        service_inprocess(args.asd, args.pkt, args.dir);
+    service_inprocess(args.asd, args.pkt, args.dir);
     return APPID_INPROCESS;
 
 success:
-    if (!args.asd.is_service_detected())
-        return add_service(args.change_bits, args.asd, args.pkt, args.dir, APP_ID_EXEC);
+    return add_service(args.change_bits, args.asd, args.pkt, args.dir, APP_ID_EXEC);
 
 bail:
-    if (!args.asd.is_service_detected())
-        incompatible_data(args.asd, args.pkt, args.dir);
-    args.asd.clear_session_flags(APPID_SESSION_CONTINUE);
+    args.asd.clear_session_flags(APPID_SESSION_REXEC_STDERR);
+    rexec_bail(rd);
+    incompatible_data(args.asd, args.pkt, args.dir);
+    args.asd.clear_session_flags(APPID_SESSION_REXEC_STDERR);
     return APPID_NOT_COMPATIBLE;
 
 fail:
-    if (!args.asd.is_service_detected())
-        fail_service(args.asd, args.pkt, args.dir);
-    args.asd.clear_session_flags(APPID_SESSION_CONTINUE);
+    args.asd.clear_session_flags(APPID_SESSION_REXEC_STDERR);
+    rexec_bail(rd);
+    fail_service(args.asd, args.pkt, args.dir);
     return APPID_NOMATCH;
 }
 

diff --git a/src/network_inspectors/appid/service_plugins/service_rexec.h b/src/network_inspectors/appid/service_plugins/service_rexec.h
index 9b9888ab77ec2d11112db55ab0d80625d262d786..53a85dde88410bbb7b46f6e872b5a9c605230c82 100644 (file)
--- a/src/network_inspectors/appid/service_plugins/service_rexec.h
+++ b/src/network_inspectors/appid/service_plugins/service_rexec.h
@@ -25,6 +25,7 @@
 #include "service_detector.h"
 
 class ServiceDiscovery;
+struct ServiceREXECData;
 
 class RexecServiceDetector : public ServiceDetector
 {
@@ -32,6 +33,8 @@ public:
     RexecServiceDetector(ServiceDiscovery*);
 
     int validate(AppIdDiscoveryArgs&) override;
+private:
+    void rexec_bail(ServiceREXECData* rd);
 };
 
 #endif

diff --git a/src/network_inspectors/appid/service_plugins/service_rshell.cc b/src/network_inspectors/appid/service_plugins/service_rshell.cc
index 716eb43fde6da0000cc4c3bd6fb185b5d3c307f2..88302f27f25d1687b0b18bd836d0bdacf9add2ac 100644 (file)
--- a/src/network_inspectors/appid/service_plugins/service_rshell.cc
+++ b/src/network_inspectors/appid/service_plugins/service_rshell.cc
@@ -45,8 +45,11 @@ enum RSHELLState
     RSHELL_STATE_COMMAND,
     RSHELL_STATE_REPLY,
     RSHELL_STATE_DONE,
+    RSHELL_STATE_BAIL,
     RSHELL_STATE_STDERR_CONNECT_SYN,
-    RSHELL_STATE_STDERR_CONNECT_SYN_ACK
+    RSHELL_STATE_STDERR_CONNECT_SYN_ACK,
+    RSHELL_STATE_STDERR_WAIT,
+    RSHELL_STATE_STDERR_DONE
 };
 
 struct ServiceRSHELLData
@@ -97,6 +100,17 @@ static void rshell_free_state(void* data)
     }
 }
 
+void RshellServiceDetector::rshell_bail(ServiceRSHELLData* rd)
+{
+    if (!rd)
+        return;
+    rd->state = RSHELL_STATE_BAIL;
+    if(rd->child)
+        rd->child->state = RSHELL_STATE_BAIL;
+    if(rd->parent)
+        rd->parent->state = RSHELL_STATE_BAIL;
+}
+
 int RshellServiceDetector::validate(AppIdDiscoveryArgs& args)
 {
     int i = 0;
@@ -143,16 +157,16 @@ int RshellServiceDetector::validate(AppIdDiscoveryArgs& args)
             if(rsh_error_snort_protocol_id == UNKNOWN_PROTOCOL_ID)
                 rsh_error_snort_protocol_id = SnortConfig::get_conf()->proto_ref->find("rsh-error");
 
-            ServiceRSHELLData* tmp_rd = (ServiceRSHELLData*)snort_calloc(
-                sizeof(ServiceRSHELLData));
-            tmp_rd->state = RSHELL_STATE_STDERR_CONNECT_SYN;
-            tmp_rd->parent = rd;
             const SfIp* dip = args.pkt->ptrs.ip_api.get_dst();
             const SfIp* sip = args.pkt->ptrs.ip_api.get_src();
             AppIdSession* pf = AppIdSession::create_future_session(args.pkt, dip, 0, sip,
                 (uint16_t)port, IpProtocol::TCP, rsh_error_snort_protocol_id, APPID_EARLY_SESSION_FLAG_FW_RULE);
             if (pf)
             {
+                ServiceRSHELLData* tmp_rd = (ServiceRSHELLData*)snort_calloc(
+                    sizeof(ServiceRSHELLData));
+                tmp_rd->state = RSHELL_STATE_STDERR_CONNECT_SYN;
+                tmp_rd->parent = rd;
                 pf->client_disco_state = APPID_DISCO_STATE_FINISHED;
                 data_add(*pf, tmp_rd, &rshell_free_state);
                 if (pf->add_flow_data_id((uint16_t)port, this))
@@ -165,17 +179,16 @@ int RshellServiceDetector::validate(AppIdDiscoveryArgs& args)
                 pf->scan_flags |= SCAN_HOST_PORT_FLAG;
                 initialize_expected_session(args.asd, *pf,
                     APPID_SESSION_CONTINUE | APPID_SESSION_REXEC_STDERR | APPID_SESSION_NO_TPI |
-                    APPID_SESSION_SERVICE_DETECTED | APPID_SESSION_NOT_A_SERVICE |
-                    APPID_SESSION_PORT_SERVICE_DONE, APP_ID_FROM_RESPONDER);
+                    APPID_SESSION_NOT_A_SERVICE | APPID_SESSION_PORT_SERVICE_DONE,
+                    APP_ID_FROM_RESPONDER);
                 pf->service_disco_state = APPID_DISCO_STATE_STATEFUL;
+                rd->child = tmp_rd;
+                rd->state = RSHELL_STATE_SERVER_CONNECT;
+                args.asd.set_session_flags(APPID_SESSION_CONTINUE);
+                goto success;
             }
             else
-            {
-                snort_free(tmp_rd);
-                return APPID_ENOMEM;
-            }
-            rd->child = tmp_rd;
-            rd->state = RSHELL_STATE_SERVER_CONNECT;
+                rd->state = RSHELL_STATE_USERNAME;
         }
         else
             rd->state = RSHELL_STATE_USERNAME;
@@ -252,30 +265,48 @@ int RshellServiceDetector::validate(AppIdDiscoveryArgs& args)
             goto inprocess;
         if (args.dir != APP_ID_FROM_RESPONDER)
             goto fail;
-        if (size == 1)
-            goto success;
-        if (*data == 0x01)
+        if (size == 1 || *data == 0x01)
         {
-            data++;
-            size--;
-            for (i=0; i<size && data[i]; i++)
+            if(size !=1) 
             {
-                if (!isprint(data[i]) && data[i] != 0x0A && data[i] != 0x0D && data[i] != 0x09)
-                    goto fail;
+                data++;
+                size--;
+                for (i=0; i<size && data[i]; i++)
+                {
+                    if (!isprint(data[i]) && data[i] != 0x0A && data[i] != 0x0D && data[i] != 0x09)
+                        goto fail;
+                }
             }
+            if(rd->child)
+            { 
+                if(rd->child->state == RSHELL_STATE_STDERR_WAIT)
+                    rd->child->state = RSHELL_STATE_STDERR_DONE;
+                else
+                    goto fail;
+            } 
+            args.asd.clear_session_flags(APPID_SESSION_CONTINUE);
             goto success;
         }
         goto fail;
-        break;
     case RSHELL_STATE_STDERR_CONNECT_SYN:
         rd->state = RSHELL_STATE_STDERR_CONNECT_SYN_ACK;
         break;
     case RSHELL_STATE_STDERR_CONNECT_SYN_ACK:
         if (rd->parent && rd->parent->state == RSHELL_STATE_SERVER_CONNECT)
+        {
             rd->parent->state = RSHELL_STATE_USERNAME;
-        // FIXIT-M why is this set here and not when add_service is called?
-        args.asd.set_service_detected();
-        return APPID_SUCCESS;
+            rd->state = RSHELL_STATE_STDERR_WAIT;
+            break;
+        }
+        goto bail;
+    case RSHELL_STATE_STDERR_WAIT:
+        if(!size)
+            break;
+        goto bail;
+    case RSHELL_STATE_STDERR_DONE:
+        args.asd.clear_session_flags(APPID_SESSION_REXEC_STDERR | APPID_SESSION_CONTINUE);
+        goto success;
+    case RSHELL_STATE_BAIL:
     default:
         goto bail;
     }
@@ -288,10 +319,14 @@ success:
     return add_service(args.change_bits, args.asd, args.pkt, args.dir, APP_ID_SHELL);
 
 bail:
+    args.asd.clear_session_flags(APPID_SESSION_REXEC_STDERR);
+    rshell_bail(rd);
     incompatible_data(args.asd, args.pkt, args.dir);
     return APPID_NOT_COMPATIBLE;
 
 fail:
+    args.asd.clear_session_flags(APPID_SESSION_REXEC_STDERR);
+    rshell_bail(rd);
     fail_service(args.asd, args.pkt, args.dir);
     return APPID_NOMATCH;
 }

diff --git a/src/network_inspectors/appid/service_plugins/service_rshell.h b/src/network_inspectors/appid/service_plugins/service_rshell.h
index 9bfad5cbf40178b7369867e73d8b574f44ca9da1..8d24c3eca8d6c4397b095bfe8be8c99e1b340626 100644 (file)
--- a/src/network_inspectors/appid/service_plugins/service_rshell.h
+++ b/src/network_inspectors/appid/service_plugins/service_rshell.h
@@ -25,6 +25,7 @@
 #include "service_detector.h"
 
 class ServiceDiscovery;
+struct ServiceRSHELLData;
 
 class RshellServiceDetector : public ServiceDetector
 {
@@ -32,6 +33,8 @@ public:
     RshellServiceDetector(ServiceDiscovery*);
 
     int validate(AppIdDiscoveryArgs&) override;
+private:
+    void rshell_bail(ServiceRSHELLData* rd);
 };
 #endif