Pull request #3330: smtp: STARTTLS command injection event processing

Merge in SNORT/snort3 from ~OSTEPANO/snort3:smtp_starttls_command_injection_alert to master

Squashed commit of the following:

commit 73e2e3cef812a0a9e93b327ef0c9d713ba9e8c27
Author: Oleksandr Stepanov <ostepano@cisco.com>
Date:   Mon Mar 21 11:01:55 2022 -0400

    smtp: STARTTLS command injection event processing

diff --git a/src/service_inspectors/smtp/smtp.cc b/src/service_inspectors/smtp/smtp.cc
index 41726ef29141013b9a6132573ddd1c80e2f8a31f..412dc0e7457fcfc06ff7cdcf995b660d4fcbb2c8 100644 (file)
--- a/src/service_inspectors/smtp/smtp.cc
+++ b/src/service_inspectors/smtp/smtp.cc
@@ -121,6 +121,7 @@ const SMTPToken smtp_resps[] =
     { "450",  3,  RESP_450, SMTP_CMD_TYPE_NORMAL },  /* Mailbox unavailable */
     { "451",  3,  RESP_451, SMTP_CMD_TYPE_NORMAL },  /* Local error in processing */
     { "452",  3,  RESP_452, SMTP_CMD_TYPE_NORMAL },  /* Insufficient system storage */
+    { "454",  3,  RESP_454, SMTP_CMD_TYPE_NORMAL },  /* TLS not available due to temporary reason */
     { "500",  3,  RESP_500, SMTP_CMD_TYPE_NORMAL },  /* Command unrecognized */
     { "501",  3,  RESP_501, SMTP_CMD_TYPE_NORMAL },  /* Syntax error in parameters or arguments */
     { "502",  3,  RESP_502, SMTP_CMD_TYPE_NORMAL },  /* Command not implemented */
@@ -703,6 +704,9 @@ static const uint8_t* SMTP_HandleCommand(SmtpProtoConf* config, Packet* p, SMTPD
     /* see if we actually found a command and not a substring */
     if (cmd_found > 0)
     {
+        if (!smtp_ssn->client_requested_starttls)
+            ++smtp_ssn->pipelined_command_counter;
+
         const uint8_t* tmp = ptr;
         const uint8_t* cmd_start = ptr + smtp_search_info.index;
         const uint8_t* cmd_end = cmd_start + smtp_search_info.length;
@@ -795,6 +799,8 @@ static const uint8_t* SMTP_HandleCommand(SmtpProtoConf* config, Packet* p, SMTPD
         DetectionEngine::queue_event(GID_SMTP, SMTP_ILLEGAL_CMD);
     }
 
+    bool alert_on_command_injection = smtp_ssn->client_requested_starttls;
+
     switch (smtp_search_info.id)
     {
     /* unless we do our own parsing of MAIL and RCTP commands we have to assume they
@@ -829,6 +835,10 @@ static const uint8_t* SMTP_HandleCommand(SmtpProtoConf* config, Packet* p, SMTPD
     case CMD_EHLO:
     case CMD_QUIT:
         smtp_ssn->state_flags &= ~(SMTP_FLAG_GOT_MAIL_CMD | SMTP_FLAG_GOT_RCPT_CMD);
+        smtp_ssn->client_requested_starttls = false;
+        smtp_ssn->server_accepted_starttls = false;
+        smtp_ssn->pipelined_command_counter = 0;
+        alert_on_command_injection = false;
 
         break;
 
@@ -837,8 +847,11 @@ static const uint8_t* SMTP_HandleCommand(SmtpProtoConf* config, Packet* p, SMTPD
          * command in reassembled packet and if not reassembled it should be the
          * last line in the packet as you can't pipeline the tls hello */
         if (eol == end)
+        {
             smtp_ssn->state = STATE_TLS_CLIENT_PEND;
-
+            smtp_ssn->client_requested_starttls = true;
+        }
+            
         break;
 
     case CMD_X_LINK2STATE:
@@ -982,6 +995,13 @@ static const uint8_t* SMTP_HandleCommand(SmtpProtoConf* config, Packet* p, SMTPD
             return nullptr;
     }
 
+     /*If client sends another command after STARTTLS raise the alert of command injection
+      STARTTLS command should be the last command when PIPELINING*/
+    if (alert_on_command_injection)
+    {
+       DetectionEngine::queue_event(GID_SMTP, SMTP_STARTTLS_INJECTION_ATTEMPT);
+    }
+
     return eol;
 }
 
@@ -1058,6 +1078,8 @@ static void SMTP_ProcessServerPacket(
         if (IsTlsServerHello(ptr, end))
         {
             smtp_ssn->state = STATE_TLS_DATA;
+            //TLS server hello received, reset flag
+            smtp_ssn->server_accepted_starttls = false;
         }
         else if ( !p->test_session_flags(SSNFLAG_MIDSTREAM)
             && !Stream::missed_packets(p->flow, SSN_DIR_BOTH))
@@ -1095,15 +1117,6 @@ static void SMTP_ProcessServerPacket(
                 if (smtp_ssn->state == STATE_CONNECT)
                     smtp_ssn->state = STATE_COMMAND;
 
-                if (smtp_ssn->state == STATE_TLS_CLIENT_PEND)
-                {
-                    OpportunisticTlsEvent event(p, p->flow->service);
-                    DataBus::publish(OPPORTUNISTIC_TLS_EVENT, event, p->flow);
-                    ++smtpstats.starttls;
-                    if (smtp_ssn->state_flags & SMTP_FLAG_ABANDON_EVT)
-                        ++smtpstats.ssl_search_abandoned_too_soon;
-                }
-
                 break;
 
             case RESP_250:
@@ -1132,6 +1145,25 @@ static void SMTP_ProcessServerPacket(
                 }
                 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;
+                    
+                    OpportunisticTlsEvent event(p, p->flow->service);
+                    DataBus::publish(OPPORTUNISTIC_TLS_EVENT, event, p->flow);
+                    ++smtpstats.starttls;
+                    if (smtp_ssn->state_flags & SMTP_FLAG_ABANDON_EVT)
+                        ++smtpstats.ssl_search_abandoned_too_soon;
+                }
+            }
         }
         else
         {
@@ -1199,18 +1231,16 @@ static void snort_smtp(SmtpProtoConf* config, Packet* p)
     else
     {
         /* This packet should be a tls client hello */
-        if (smtp_ssn->state == STATE_TLS_CLIENT_PEND)
+        if (smtp_ssn->server_accepted_starttls)
         {
             if (IsTlsClientHello(p->data, p->data + p->dsize))
             {
                 smtp_ssn->state = STATE_TLS_SERVER_PEND;
             }
-            else
-            {
-                /* reset state - server may have rejected STARTTLS command */
-                smtp_ssn->state = STATE_COMMAND;
-            }
         }
+        
+        if(smtp_ssn->state == STATE_TLS_CLIENT_PEND)
+            smtp_ssn->state = STATE_COMMAND;
 
         if ((smtp_ssn->state == STATE_TLS_DATA)
             || (smtp_ssn->state == STATE_TLS_SERVER_PEND))

diff --git a/src/service_inspectors/smtp/smtp.h b/src/service_inspectors/smtp/smtp.h
index 728964c352a615fa0d1d70e64a5b381611082ab6..e1a3490f29ff0e85626774db3681971d1a121858 100644 (file)
--- a/src/service_inspectors/smtp/smtp.h
+++ b/src/service_inspectors/smtp/smtp.h
@@ -91,6 +91,7 @@ enum SMTPRespEnum
     RESP_450,
     RESP_451,
     RESP_452,
+    RESP_454,
     RESP_500,
     RESP_501,
     RESP_502,
@@ -163,6 +164,9 @@ struct SMTPData
     uint32_t dat_chunk;
     SmtpMime* mime_ssn;
     SMTPAuthName* auth_name;
+    bool client_requested_starttls = false;
+    size_t pipelined_command_counter = 0;
+    bool server_accepted_starttls = false;
 };
 
 class SmtpFlowData : public snort::FlowData

diff --git a/src/service_inspectors/smtp/smtp_module.cc b/src/service_inspectors/smtp/smtp_module.cc
index 93ed6950cb5bf5c2edd3620c7d6a023b18294558..0e266ffbed9288e115a1071e10954f2ca001c03d 100644 (file)
--- a/src/service_inspectors/smtp/smtp_module.cc
+++ b/src/service_inspectors/smtp/smtp_module.cc
@@ -170,6 +170,8 @@ static const RuleMap smtp_rules[] =
     { SMTP_AUTH_ABORT_AUTH, "Cyrus SASL authentication attack" },
     { SMTP_AUTH_COMMAND_OVERFLOW, "attempted authentication command buffer overflow" },
     { SMTP_FILE_DECOMP_FAILED, "file decompression failed" },
+    { SMTP_STARTTLS_INJECTION_ATTEMPT, "STARTTLS command injection attempt"},
+    
 
     { 0, nullptr }
 };

diff --git a/src/service_inspectors/smtp/smtp_module.h b/src/service_inspectors/smtp/smtp_module.h
index f0e32e78ef7c4310615d7a5dc20933c943f04916..bf2e62dba6b65c88eeccb75dfc6a7d286db5db13 100644 (file)
--- a/src/service_inspectors/smtp/smtp_module.h
+++ b/src/service_inspectors/smtp/smtp_module.h
@@ -45,6 +45,7 @@
 #define SMTP_AUTH_ABORT_AUTH        14
 #define SMTP_AUTH_COMMAND_OVERFLOW  15
 #define SMTP_FILE_DECOMP_FAILED     16
+#define SMTP_STARTTLS_INJECTION_ATTEMPT 17
 
 #define SMTP_NAME "smtp"
 #define SMTP_HELP "smtp inspection"