Pull request #5137: appid: address FIXIT comments in detector plugins

Merge in SNORT/snort3 from ~BHRYNIV/snort3:fixit_detector_plugins to master

Squashed commit of the following:

commit b672e1cdc0f89cb5b008a5a6fc7f39c15fe45f2f
Author: Bohdan Hryniv <bhryniv@cisco>
Date:   Wed Jan 28 10:54:39 2026 -0500

    appid: address FIXIT comments in detector plugins

diff --git a/src/network_inspectors/appid/appid_session_api.h b/src/network_inspectors/appid/appid_session_api.h
index e526a0a7afae4a165c4487fbed9d9ac6f01204ac..bc834b1f5e155471465445e19adfc8fd75814685 100644 (file)
--- a/src/network_inspectors/appid/appid_session_api.h
+++ b/src/network_inspectors/appid/appid_session_api.h
@@ -94,14 +94,15 @@ namespace snort
 #define APPID_SESSION_OPPORTUNISTIC_TLS     (1ULL << 44)
 #define APPID_SESSION_FIRST_PKT_CACHE_MATCHED    (1ULL << 45)
 #define APPID_SESSION_DO_NOT_DECRYPT        (1ULL << 46)
-#define APPID_SESSION_WAIT_FOR_EXTERNAL       (1ULL << 47)
+#define APPID_SESSION_WAIT_FOR_EXTERNAL     (1ULL << 47)
 #define APPID_SESSION_IGNORE_ID_FLAGS \
     (APPID_SESSION_FUTURE_FLOW | \
     APPID_SESSION_NOT_A_SERVICE | \
     APPID_SESSION_NO_TPI | \
     APPID_SESSION_SERVICE_DETECTED | \
     APPID_SESSION_PORT_SERVICE_DONE)
-const uint64_t APPID_SESSION_ALL_FLAGS = 0xFFFFFFFFFFFFFFFFULL;
+
+#define APPID_SESSION_ALL_FLAGS 0xFFFFFFFFFFFFFFFFULL
 
 class SO_PUBLIC AppIdSessionApi : public StashGenericObject
 {

diff --git a/src/network_inspectors/appid/detector_plugins/detector_imap.cc b/src/network_inspectors/appid/detector_plugins/detector_imap.cc
index c863732c88f8598fab9e76055adc506c48de8a2a..ade55ed3cd7973d04c898dee043aa2d1dee030b3 100644 (file)
--- a/src/network_inspectors/appid/detector_plugins/detector_imap.cc
+++ b/src/network_inspectors/appid/detector_plugins/detector_imap.cc
@@ -37,6 +37,8 @@ static const unsigned IMAP_USER_NAME_MAX_LEN = 32;
 static const unsigned IMAP_TAG_MAX_LEN = 6;
 static const unsigned MIN_CMDS = 3;
 
+#define SSL_WAIT_PACKETS 8
+
 static const char NO_LOGIN[] = " Login failed.";
 
 static const uint8_t CAPA[] = "CAPABILITY\x00d\x00a";
@@ -135,6 +137,7 @@ struct ImapClientData
     int detected;
     int got_user;
     int auth;
+    int decryption_countdown;
     char username[IMAP_USER_NAME_MAX_LEN+1];
     char imapCmdTag[IMAP_TAG_MAX_LEN+1];
 };
@@ -431,11 +434,7 @@ static int imap_server_validate(ImapDetectorData* dd, const uint8_t* data, uint1
     if (dd->client.state == IMAP_CLIENT_STATE_STARTTLS_CMD)
     {
         if (id->flags & IMAP_FLAG_RESULT_OK)
-        {
-            // FIXIT-L - this may be called from server side
-            detector->add_app(asd, APP_ID_IMAPS, APP_ID_IMAPS, nullptr, change_bits);
             asd.clear_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS);
-        }
         else
             dd->client.state = IMAP_CLIENT_STATE_NON_AUTH;
     }
@@ -588,6 +587,23 @@ int ImapClientDetector::validate(AppIdDiscoveryArgs& args)
     ImapDetectorData* dd = get_common_data(args.asd);
     ImapClientData* fd = &dd->client;
 
+    // encrypted session without decryption - use countdown fallback (matches SMTP)
+    if (args.asd.get_session_flags(APPID_SESSION_ENCRYPTED | APPID_SESSION_DECRYPTED) == APPID_SESSION_ENCRYPTED)
+    {
+        if (fd->decryption_countdown > 0)
+        {
+            fd->decryption_countdown--;
+            if (!fd->decryption_countdown)
+            {
+                add_app(args.asd, APP_ID_IMAPS, APP_ID_IMAPS, nullptr, args.change_bits);
+                dd->need_continue = 0;
+                args.asd.clear_session_flags(APPID_SESSION_CLIENT_GETS_SERVER_PACKETS);
+                return APPID_SUCCESS;
+            }
+        }
+        return APPID_INPROCESS;
+    }
+
     if (args.dir == APP_ID_FROM_RESPONDER)
     {
         if (imap_server_validate(dd, args.data, args.size, args.asd, args.change_bits, this))
@@ -897,7 +913,16 @@ int ImapServiceDetector::validate(AppIdDiscoveryArgs& args)
     {
         if ((id->flags & IMAP_FLAG_RESULT_OK) &&
             dd->client.state == IMAP_CLIENT_STATE_STARTTLS_CMD)
+        {
+            if (args.asd.get_session_flags(APPID_SESSION_OPPORTUNISTIC_TLS))
+                dd->client.decryption_countdown = SSL_WAIT_PACKETS;
+            else
+                dd->client.decryption_countdown = 1;
+
+            args.asd.set_session_flags(APPID_SESSION_ENCRYPTED);
+
             return add_service(args.change_bits, args.asd, args.pkt, args.dir, APP_ID_IMAPS);
+        }
 
         if (id->count >= IMAP_COUNT_THRESHOLD && !args.asd.is_service_detected())
             return add_service(args.change_bits, args.asd, args.pkt, args.dir, APP_ID_IMAP);

diff --git a/src/network_inspectors/appid/detector_plugins/detector_sip.cc b/src/network_inspectors/appid/detector_plugins/detector_sip.cc
index 64ea0f72326b3dd272f1737fc249066772c4bc09..80ed7402da921419d33776f85fd68d5944543bfc 100644 (file)
--- a/src/network_inspectors/appid/detector_plugins/detector_sip.cc
+++ b/src/network_inspectors/appid/detector_plugins/detector_sip.cc
@@ -189,10 +189,6 @@ void SipServiceDetector::createRtpFlow(AppIdSession& asd, const Packet* pkt, con
         fp->set_payload_id(asd.get_payload_id());
         fp->set_service_id(APP_ID_RTP, odp_ctxt);
 
-        // FIXIT-M : snort 2.9.x updated the flag to APPID_SESSION_EXPECTED_EVALUATE.
-        // Check if it is needed here as well.
-        // asd.initialize_future_session(*fp, APPID_SESSION_EXPECTED_EVALUATE);
-
         asd.initialize_future_session(*fp, APPID_SESSION_IGNORE_ID_FLAGS);
     }
 
@@ -207,9 +203,6 @@ void SipServiceDetector::createRtpFlow(AppIdSession& asd, const Packet* pkt, con
         fp2->set_payload_id(asd.get_payload_id());
         fp2->set_service_id(APP_ID_RTCP, odp_ctxt);
 
-        // FIXIT-M : same comment as above
-        // asd.initialize_future_session(*fp2, APPID_SESSION_EXPECTED_EVALUATE);
-
         asd.initialize_future_session(*fp2, APPID_SESSION_IGNORE_ID_FLAGS);
     }
 }

diff --git a/src/network_inspectors/appid/detector_plugins/detector_smtp.cc b/src/network_inspectors/appid/detector_plugins/detector_smtp.cc
index 21a56cc6d1b49d6ee54d8c5517452cabce400d5e..34523ee57de771299c519d3bf3f1a3ad427e17c0 100644 (file)
--- a/src/network_inspectors/appid/detector_plugins/detector_smtp.cc
+++ b/src/network_inspectors/appid/detector_plugins/detector_smtp.cc
@@ -190,7 +190,6 @@ SmtpClientDetector::SmtpClientDetector(ClientDiscovery* cdm)
  *    prefix_len - The number of characters that are the prefix to the version,
  *              including the NUL terminating character.
  */
-// FIXIT-M - refactor this to reduce the number of function parameters
 int SmtpClientDetector::extract_version_and_add_client_app(
     AppId clientId, const int prefix_len, const uint8_t* product, const uint8_t* product_end,
     ClientSMTPData* const client_data, AppIdSession& asd, AppId appId,