Pull request #4466: smtp: Fixing the processing of SMTP response in case of encrypted...

Merge in SNORT/snort3 from ~BSACHDEV/snort3:smtp_alert_fix to master

Squashed commit of the following:

commit 615507541e02cf81c25c210449f82d37bd41b9e6
Author: bsachdev <bsachdev@cisco.com>
Date:   Wed Oct 2 16:12:52 2024 -0400

    smtp: Fixing the processing of SMTP response in case of encrypted traffic

diff --git a/src/service_inspectors/smtp/smtp.cc b/src/service_inspectors/smtp/smtp.cc
index 0879d8885605627b78574b02c9c9e3a3c581262c..4b785c593887043913a451c9f4839401ddc35a07 100644 (file)
--- a/src/service_inspectors/smtp/smtp.cc
+++ b/src/service_inspectors/smtp/smtp.cc
@@ -1157,79 +1157,82 @@ static void SMTP_ProcessServerPacket(
         /* Check for response code */
         smtp_current_search = &smtp_resp_search[0];
 
-        int resp_found = smtp_resp_search_mpse->find(
-            (const char*)ptr, resp_line_len, SMTP_SearchStrFound);
-
-        if (resp_found > 0)
+        if (smtp_ssn->state != STATE_TLS_DATA or p->flow->flags.data_decrypted)
         {
-            switch (smtp_search_info.id)
-            {
-            case RESP_220:
-                /* This is either an initial server response or a STARTTLS response */
-                if (smtp_ssn->state == STATE_CONNECT)
-                    smtp_ssn->state = STATE_COMMAND;
-                break;
+            int resp_found = smtp_resp_search_mpse->find(
+                (const char*)ptr, resp_line_len, SMTP_SearchStrFound);
 
-            case RESP_250:
-            case RESP_221:
-            case RESP_334:
-            case RESP_354:
-                if ((smtp_ssn->state == STATE_DATA or smtp_ssn->state == STATE_BDATA)
-                    and !p->flow->flags.data_decrypted
-                    and !(smtp_ssn->state_flags & SMTP_FLAG_ABANDON_EVT))
+            if (resp_found > 0)
+            {
+                switch (smtp_search_info.id)
                 {
-                    smtp_ssn->state_flags |= SMTP_FLAG_ABANDON_EVT;
-                    DataBus::publish(intrinsic_pub_id, IntrinsicEventIds::SSL_SEARCH_ABANDONED, p);
-                    ++smtpstats.ssl_search_abandoned;
-                }
-                break;
+                case RESP_220:
+                    /* This is either an initial server response or a STARTTLS response */
+                    if (smtp_ssn->state == STATE_CONNECT)
+                        smtp_ssn->state = STATE_COMMAND;
+                    break;
 
-            case RESP_235:
-                // Auth done
-                *next_state = STATE_COMMAND;
-                break;
+                case RESP_250:
+                case RESP_221:
+                case RESP_334:
+                case RESP_354:
+                    if ((smtp_ssn->state == STATE_DATA or smtp_ssn->state == STATE_BDATA)
+                        and !p->flow->flags.data_decrypted
+                        and !(smtp_ssn->state_flags & SMTP_FLAG_ABANDON_EVT))
+                    {
+                        smtp_ssn->state_flags |= SMTP_FLAG_ABANDON_EVT;
+                        DataBus::publish(intrinsic_pub_id, IntrinsicEventIds::SSL_SEARCH_ABANDONED, p);
+                        ++smtpstats.ssl_search_abandoned;
+                    }
+                    break;
 
-            default:
-                if (smtp_ssn->state != STATE_COMMAND and smtp_ssn->state != STATE_TLS_DATA)
-                {
+                case RESP_235:
+                    // Auth done
                     *next_state = STATE_COMMAND;
+                    break;
+
+                default:
+                    if (smtp_ssn->state != STATE_COMMAND and smtp_ssn->state != STATE_TLS_DATA)
+                    {
+                        *next_state = STATE_COMMAND;
+                    }
+                    break;
+                }
+                //Count responses of client commands, reset starttls waiting flag if response to STARTTLS is not 220
+                if (smtp_ssn->pipelined_command_counter > 0 and --smtp_ssn->pipelined_command_counter == 0 and smtp_ssn->client_requested_starttls)
+                {
+                    if (smtp_search_info.id != RESP_220)
+                    {
+                        smtp_ssn->client_requested_starttls = false;
+                        smtp_ssn->server_accepted_starttls = false;
+                    }
+                    else
+                    {
+                        smtp_ssn->server_accepted_starttls = true;
+                        smtp_ssn->state = STATE_TLS_CLIENT_PEND;
+
+                        OpportunisticTlsEvent event(p, p->flow->service);
+                        DataBus::publish(intrinsic_pub_id, IntrinsicEventIds::OPPORTUNISTIC_TLS, event, p->flow);
+                        ++smtpstats.starttls;
+                        if (smtp_ssn->state_flags & SMTP_FLAG_ABANDON_EVT)
+                            ++smtpstats.ssl_search_abandoned_too_soon;
+                    }
                 }
-                break;
             }
-            //Count responses of client commands, reset starttls waiting flag if response to STARTTLS is not 220
-            if (smtp_ssn->pipelined_command_counter > 0 and --smtp_ssn->pipelined_command_counter == 0 and smtp_ssn->client_requested_starttls)
+            else
             {
-                if (smtp_search_info.id != RESP_220)
+                if ((smtp_ssn->session_flags & SMTP_FLAG_CHECK_SSL) &&
+                    (IsSSL(ptr, end - ptr, p->packet_flags)))
                 {
-                    smtp_ssn->client_requested_starttls = false;
-                    smtp_ssn->server_accepted_starttls = false;
+                    smtp_ssn->state = STATE_TLS_DATA;
+                    return;
                 }
-                else
+                else if (smtp_ssn->session_flags & SMTP_FLAG_CHECK_SSL)
                 {
-                    smtp_ssn->server_accepted_starttls = true;
-                    smtp_ssn->state = STATE_TLS_CLIENT_PEND;
-
-                    OpportunisticTlsEvent event(p, p->flow->service);
-                    DataBus::publish(intrinsic_pub_id, IntrinsicEventIds::OPPORTUNISTIC_TLS, event, p->flow);
-                    ++smtpstats.starttls;
-                    if (smtp_ssn->state_flags & SMTP_FLAG_ABANDON_EVT)
-                        ++smtpstats.ssl_search_abandoned_too_soon;
+                    smtp_ssn->session_flags &= ~SMTP_FLAG_CHECK_SSL;
                 }
             }
         }
-        else
-        {
-            if ((smtp_ssn->session_flags & SMTP_FLAG_CHECK_SSL) &&
-                (IsSSL(ptr, end - ptr, p->packet_flags)))
-            {
-                smtp_ssn->state = STATE_TLS_DATA;
-                return;
-            }
-            else if (smtp_ssn->session_flags & SMTP_FLAG_CHECK_SSL)
-            {
-                smtp_ssn->session_flags &= ~SMTP_FLAG_CHECK_SSL;
-            }
-        }
 
         if (smtp_ssn->state != STATE_TLS_DATA)
         {