Pull request #4995: appid: ignore empty strings in ssl lookup api

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

Squashed commit of the following:

commit 5e6a9c0b9cea6e476cee6369e79d168807b5db2d
Author: Oleksandr Stepanov <ostepano@cisco.com>
Date:   Fri Nov 14 09:54:37 2025 -0500

    appid: ignore empty strings in ssl lookup api

diff --git a/src/network_inspectors/appid/appid_api.cc b/src/network_inspectors/appid/appid_api.cc
index 5bc73dc8b2148898a8b1fc42d2a31625a327940d..688e9f5f0be8da4351c432ba9816dcb74f739c83 100644 (file)
--- a/src/network_inspectors/appid/appid_api.cc
+++ b/src/network_inspectors/appid/appid_api.cc
@@ -149,28 +149,44 @@ bool AppIdApi::ssl_app_group_id_lookup(Flow* flow, const char* server_name,
 
         if (org_unit)
         {
-            asd->tsession->set_tls_org_unit(org_unit, strlen(org_unit));
-            asd->scan_flags |= SCAN_SSL_ORG_UNIT_FLAG;
+            auto org_unit_len = strlen(org_unit);
+            if (org_unit_len > 0)
+            {
+                asd->tsession->set_tls_org_unit(org_unit, org_unit_len);
+                asd->scan_flags |= SCAN_SSL_ORG_UNIT_FLAG;
+            }
         }
 
         if (server_name)
         {
-            asd->tsession->set_tls_sni(server_name, strlen(server_name));
-            if (!sni_mismatch)
-                asd->scan_flags |= SCAN_SSL_HOST_FLAG;
+            auto sni_len = strlen(server_name);
+            if (sni_len > 0)
+            {
+                asd->tsession->set_tls_sni(server_name, sni_len);
+                if (!sni_mismatch)
+                    asd->scan_flags |= SCAN_SSL_HOST_FLAG;
+            }
         }
 
         if (first_alt_name)
         {
-            asd->tsession->set_tls_first_alt_name(first_alt_name, strlen(first_alt_name));
-            asd->scan_flags |= SCAN_SSL_ALT_NAME;
+            auto first_alt_name_len = strlen(first_alt_name);
+            if (first_alt_name_len > 0)
+            {
+                asd->tsession->set_tls_first_alt_name(first_alt_name, first_alt_name_len);
+                asd->scan_flags |= SCAN_SSL_ALT_NAME;
+            }
         }
 
         if (common_name)
         {
-            asd->tsession->set_tls_cname(common_name, strlen(common_name));
-            asd->scan_flags |= SCAN_SSL_CERTIFICATE_FLAG;
-            asd->tsession->set_tls_handshake_done();
+            auto common_name_len = strlen(common_name);
+            if (common_name_len > 0)
+            {
+                asd->tsession->set_tls_cname(common_name, common_name_len);
+                asd->scan_flags |= SCAN_SSL_CERTIFICATE_FLAG;
+                asd->tsession->set_tls_handshake_done();
+            }
         }
 
         asd->scan_flags |= SCAN_CERTVIZ_ENABLED_FLAG;

diff --git a/src/network_inspectors/appid/appid_session.h b/src/network_inspectors/appid/appid_session.h
index f694b903b2638be71b1dcd2207ac18f5637ecf42..dcdf4958a24bb27540b650eeb0c9d3f6e4bcef27 100644 (file)
--- a/src/network_inspectors/appid/appid_session.h
+++ b/src/network_inspectors/appid/appid_session.h
@@ -152,18 +152,14 @@ public:
     void set_tls_sni(const char* new_tls_sni, uint32_t len)
     {
         if (tls_sni)
-        {
             snort_free(tls_sni);
-        }
-        if (new_tls_sni)
-        {
-            tls_sni = len ? snort::snort_strndup(new_tls_sni, len) :
-                const_cast<char*>(new_tls_sni);
-        }
-        else
+        if (!new_tls_sni or *new_tls_sni == '\0')
         {
             tls_sni = nullptr;
+            return;
         }
+        tls_sni = len ? snort::snort_strndup(new_tls_sni, len) :
+            const_cast<char*>(new_tls_sni);
     }
 
     void set_tls_first_alt_name(const char* new_tls_first_alt_name, uint32_t len)

diff --git a/src/network_inspectors/appid/test/appid_api_test.cc b/src/network_inspectors/appid/test/appid_api_test.cc
index 15f42f04a137c52532356401c2a5ba313bd2d716..a3ee9657ebdbfa7eb798a13795e6da1996459cb8 100644 (file)
--- a/src/network_inspectors/appid/test/appid_api_test.cc
+++ b/src/network_inspectors/appid/test/appid_api_test.cc
@@ -376,6 +376,33 @@ TEST(appid_api, ssl_app_group_id_lookup_sni_mismatch)
     mock().checkExpectations();
 }
 
+TEST(appid_api, ssl_app_group_id_lookup_zero_len_data)
+{
+    mock().expectNCalls(1, "publish");
+    AppId service, client, payload = APP_ID_NONE;
+    bool val = false;
+
+    AppidChangeBits change_bits;
+
+    mock_session->set_ss_application_ids(0,0,0,0,0, change_bits);
+    mock_session->tsession->set_tls_sni(nullptr, 0);
+    mock_session->tsession->set_tls_cname(nullptr, 0);
+    mock_session->tsession->set_tls_first_alt_name(nullptr, 0);
+    mock_session->tsession->set_tls_org_unit(nullptr, 0);
+
+    const char* test_zero_tls_data = "";
+
+    val = appid_api.ssl_app_group_id_lookup(flow, test_zero_tls_data, test_zero_tls_data,
+        test_zero_tls_data, test_zero_tls_data, true, service, client, payload);
+
+    CHECK_TRUE(val);
+    CHECK_EQUAL(mock_session->tsession->get_tls_sni(), nullptr);
+    CHECK_EQUAL(mock_session->tsession->get_tls_first_alt_name(), nullptr);
+    CHECK_EQUAL(mock_session->tsession->get_tls_cname(), nullptr);
+    CHECK_EQUAL(mock_session->tsession->get_tls_org_unit(), nullptr);
+    mock().checkExpectations();
+}
+
 TEST(appid_api_no_session, ssl_app_group_id_lookup)
 {
     AppId service, client, payload = APP_ID_NONE;
@@ -431,6 +458,23 @@ TEST(appid_api, is_inspection_needed)
     CHECK_FALSE(appid_api.is_inspection_needed(inspector));
 }
 
+TEST(appid_api, is_inspection_needed_no_appid_inspector)
+{
+    mock_inspector_exist = false;
+    DummyInspector inspector;
+    bool res = appid_api.is_inspection_needed(inspector);
+    CHECK_FALSE(res);
+    mock_inspector_exist = true;
+}
+
+TEST(appid_api, update_shadow_traffic_status_no_appid_inspector)
+{
+    mock_inspector_exist = false;
+    appid_api.update_shadow_traffic_status(false);
+    CHECK_TRUE(true);// no crash
+    mock_inspector_exist = true;
+}
+
 TEST(appid_api, is_service_http_type)
 {
     CHECK_TRUE(appid_api.is_service_http_type(APP_ID_HTTP));

diff --git a/src/network_inspectors/appid/test/appid_session_api_test.cc b/src/network_inspectors/appid/test/appid_session_api_test.cc
index d56f5808f36ff47ae7995c0fd5201f9157533bde..4fede8f6c4993e61b07e81516b49f99a9271b332 100644 (file)
--- a/src/network_inspectors/appid/test/appid_session_api_test.cc
+++ b/src/network_inspectors/appid/test/appid_session_api_test.cc
@@ -393,6 +393,34 @@ TEST(appid_session_api, get_first_stream_appids_for_http2)
     delete &asd.get_api();
 }
 
+TEST(appid_session_api, get_first_stream_appids_for_http3)
+{
+    SfIp ip{};
+    AppIdSession asd(IpProtocol::TCP, &ip, 1492, dummy_appid_inspector, odpctxt, 0
+#ifndef DISABLE_TENANT_ID
+    ,0
+#endif
+    );
+    asd.flow = &flow;
+    AppidChangeBits change_bits;
+    asd.set_ss_application_ids(APP_ID_HTTP3,APP_ID_HTTP3,APP_ID_HTTP3,APP_ID_HTTP3,APP_ID_HTTP3, change_bits);
+
+    AppId service, client, payload, misc;
+    asd.get_api().get_first_stream_app_ids(service, client, payload, misc);
+    CHECK_EQUAL(service, APP_ID_HTTP3);
+    CHECK_EQUAL(client, APP_ID_HTTP3);
+    CHECK_EQUAL(payload, APP_ID_HTTP3);
+    CHECK_EQUAL(misc, APP_ID_HTTP3);
+
+    service = client = payload = APP_ID_NONE;
+    asd.get_api().get_first_stream_app_ids(service, client, payload);
+    CHECK_EQUAL(service, APP_ID_HTTP3);
+    CHECK_EQUAL(client, APP_ID_HTTP3);
+    CHECK_EQUAL(payload, APP_ID_HTTP3);
+
+    delete &asd.get_api();
+}
+
 TEST(appid_session_api, get_tls_host)
 {
     AppidChangeBits change_bits;