From: Masud Hasan (mashasan) Date: Tue, 12 Apr 2022 15:10:40 +0000 (+0000) Subject: Pull request #3330: smtp: STARTTLS command injection event processing X-Git-Tag: 3.1.28.0~20 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=19ef61f89150358d1b2ed8d5566f9f636c57d9f9;p=thirdparty%2Fsnort3.git 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 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 41726ef29..412dc0e74 100644 --- 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 728964c35..e1a3490f2 100644 --- 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 93ed6950c..0e266ffbe 100644 --- 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 f0e32e78e..bf2e62dba 100644 --- 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"