Merge pull request #1330 in SNORT/snort3 from smtp_early_detection to master

Squashed commit of the following:

commit bf17fcb491b8e017d4b37123e951b2d404159c9e
Author: Pratik Shinde <pshinde2@cisco.com>
Date:   Fri Aug 10 18:06:03 2018 -0400

    appid: Declare SMTPS early in STARTTLS state on success response code.

diff --git a/src/network_inspectors/appid/detector_plugins/detector_smtp.cc b/src/network_inspectors/appid/detector_plugins/detector_smtp.cc
index 2294c4181008bf7025c70b16d28a3d573de2f111..b4a6f43662ff24dda8fdd51336b096054e35a33f 100644 (file)
--- a/src/network_inspectors/appid/detector_plugins/detector_smtp.cc
+++ b/src/network_inspectors/appid/detector_plugins/detector_smtp.cc
@@ -47,7 +47,6 @@ enum SMTPClientState
 
 /* flag values for ClientSMTPData */
 #define CLIENT_FLAG_STARTTLS_SUCCESS    0x01
-#define CLIENT_FLAG_SMTPS               0x02
 
 #define MAX_VERSION_SIZE    64
 #define SSL_WAIT_PACKETS    8  // This many un-decrypted packets without a HELO and we quit.
@@ -210,7 +209,7 @@ int SmtpClientDetector::identify_client_version(ClientSMTPData* const fd, const
     const uint8_t* data_end, AppIdSession& asd, snort::Packet*)
 {
     const uint8_t* p;
-    AppId appId = (fd->flags & CLIENT_FLAG_SMTPS) ?  APP_ID_SMTPS : APP_ID_SMTP;
+    AppId appId = APP_ID_SMTP;
     uint8_t* v_end = fd->version + MAX_VERSION_SIZE - 1;
     unsigned len = data_end - product;
     if (len >= sizeof(MICROSOFT) && memcmp(product, MICROSOFT, sizeof(MICROSOFT)-1) == 0)
@@ -385,8 +384,6 @@ int SmtpClientDetector::validate(AppIdDiscoveryArgs& args)
             fd->decryption_countdown--;
             if (!fd->decryption_countdown)
             {
-                fd->flags |= CLIENT_FLAG_SMTPS; // report as SMTPS
-                args.asd.clear_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS);
                 /* Because we can't see any further info without decryption we settle for
                    plain APP_ID_SMTPS instead of perhaps finding data that would make calling
                    ExtractVersion() worthwhile, So set the appid and call it good. */
@@ -591,6 +588,10 @@ int SmtpClientDetector::validate(AppIdDiscoveryArgs& args)
 
 done:
     dd->need_continue = 0;
+    if(args.asd.get_session_flags(APPID_SESSION_SERVICE_DETECTED))
+        args.asd.clear_session_flags(APPID_SESSION_CONTINUE | APPID_SESSION_CLIENT_GETS_SERVER_PACKETS);
+    else
+        args.asd.clear_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS); 
     args.asd.set_client_detected();
     return APPID_SUCCESS;
 }
@@ -763,17 +764,11 @@ int SmtpServiceDetector::validate(AppIdDiscoveryArgs& args)
 
     args.asd.clear_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS);
 
-    // Whether this is bound for the client detector or not, if client doesn't care
-    //  then clear the APPID_SESSION_CONTINUE flag and we will be done sooner.
-    if (dd->need_continue == 0)
+    if (args.asd.get_session_flags(APPID_SESSION_SERVICE_DETECTED))
     {
-        dd->need_continue--; // don't come through again.
-        args.asd.clear_session_flags(APPID_SESSION_CONTINUE);
-        if ( dd->client.flags & CLIENT_FLAG_SMTPS )    // encrypted session client side gave up
-            return add_service(args.asd, args.pkt, args.dir,  APP_ID_SMTPS);
-        else if ( args.asd.is_service_detected() )    // client done, so we are too
-            return APPID_SUCCESS;
-        // We arrive here because the service side is not done yet.
+        if(!dd->need_continue)
+            args.asd.clear_session_flags(APPID_SESSION_CONTINUE);
+        return APPID_SUCCESS;
     }
 
     if (args.dir != APP_ID_FROM_RESPONDER)
@@ -835,11 +830,26 @@ int SmtpServiceDetector::validate(AppIdDiscoveryArgs& args)
             fd->state = SMTP_SERVICE_STATE_HELO;
             if (fd->code == 220)
             {
-                args.asd.set_session_flags(APPID_SESSION_ENCRYPTED);
-                // Now we wonder if the decryption mechanism is in place, so...
                 dd->client.flags |= CLIENT_FLAG_STARTTLS_SUCCESS;
-                dd->client.decryption_countdown = SSL_WAIT_PACKETS; // start a countdown
-                goto inprocess;
+                //FIXIT-M: FIXIT-M: Revisit SSL decryption countdown after isSSLPolicyEnabled() is ported.
+                //Can we use Flow::is_proxied() here?
+#if 0
+                if (_dpd.isSSLPolicyEnabled(NULL))
+#endif
+                    dd->client.decryption_countdown = SSL_WAIT_PACKETS; // start a countdown
+#if 0
+                else
+                    dd->client.decryption_countdown = 1
+#endif
+
+                add_service(args.asd, args.pkt, args.dir,  APP_ID_SMTPS);
+
+                if(dd->need_continue > 0)
+                    args.asd.set_session_flags(APPID_SESSION_ENCRYPTED | APPID_SESSION_STICKY_SERVICE | APPID_SESSION_CONTINUE);
+                else
+                    args.asd.set_session_flags(APPID_SESSION_ENCRYPTED | APPID_SESSION_STICKY_SERVICE);
+
+                return APPID_SUCCESS;
             }
             /* STARTTLS failed. */
             break;

diff --git a/src/network_inspectors/appid/service_plugins/service_ftp.cc b/src/network_inspectors/appid/service_plugins/service_ftp.cc
index 9966eddea3541013daf4723fa54ec29d7b189e73..ca8030b160f62756a2158db121da028339433f24 100644 (file)
--- a/src/network_inspectors/appid/service_plugins/service_ftp.cc
+++ b/src/network_inspectors/appid/service_plugins/service_ftp.cc
@@ -986,7 +986,6 @@ int FtpServiceDetector::validate(AppIdDiscoveryArgs& args)
                     break;
                 case 234:
                 {
-                    args.asd.set_session_flags(APPID_SESSION_CONTINUE);
                     retval = APPID_SUCCESS;
                     /*
                     // we do not set the state to FTP_STATE_MONITOR here because we don't know