Pull request #4319: appid: Store and retrieve only SNI in AppIdSession

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

Squashed commit of the following:

commit 6a591a048bc22e8d5fa99d6876613443dabf8352
Author: Oleksandr Stepanov <ostepano@cisco.com>
Date:   Fri May 3 11:01:52 2024 -0400

    appid: Store and retrieve only SNI in AppIdSession

diff --git a/src/network_inspectors/appid/appid_api.cc b/src/network_inspectors/appid/appid_api.cc
index f7ccfec6660825fb4c5c73b27f3f696b41eb1c4c..623a99c56a4d2cf1963911b02561c13c8599192b 100644 (file)
--- a/src/network_inspectors/appid/appid_api.cc
+++ b/src/network_inspectors/appid/appid_api.cc
@@ -141,7 +141,10 @@ bool AppIdApi::ssl_app_group_id_lookup(Flow* flow, const char* server_name,
         if (!asd->tsession)
             asd->tsession = new TlsSession();
         else if (sni_mismatch)
-            asd->tsession->set_tls_host(nullptr, 0, change_bits);
+        {
+            asd->tsession->process_sni_mismatch();
+        }
+            
 
         if (sni_mismatch)
             asd->scan_flags |= SCAN_SPOOFED_SNI_FLAG;

diff --git a/src/network_inspectors/appid/appid_session.h b/src/network_inspectors/appid/appid_session.h
index 715d3f20d04302a170a3c48016334efdbf5a191c..fe6e2fd1155339ceca9ac1a2ead98072784a14b2 100644 (file)
--- a/src/network_inspectors/appid/appid_session.h
+++ b/src/network_inspectors/appid/appid_session.h
@@ -118,6 +118,8 @@ public:
             snort_free(tls_cname);
         if (tls_org_unit)
             snort_free(tls_org_unit);
+        if (tls_host_mismatch)
+            snort_free(tls_host_mismatch);
     }
 
     const char* get_tls_host() const
@@ -141,6 +143,22 @@ public:
         return nullptr;
     }
 
+    const char* get_tls_sni() const
+    {
+        return tls_host_mismatch ? tls_host_mismatch : tls_host;
+    }
+
+    void process_sni_mismatch()
+    {
+        if(tls_host)
+        {
+            if(tls_host_mismatch)
+                snort_free(tls_host_mismatch);
+            tls_host_mismatch = tls_host;
+            tls_host = nullptr;
+        }
+    }
+
     const char* get_tls_first_alt_name() const { return tls_first_alt_name; }
 
     const char* get_tls_cname() const { return tls_cname; }
@@ -228,6 +246,7 @@ public:
 
 private:
     char* tls_host = nullptr;
+    char* tls_host_mismatch = nullptr;
     char* tls_first_alt_name = nullptr;
     char* tls_cname = nullptr;
     char* tls_org_unit = nullptr;
@@ -601,7 +620,10 @@ public:
     void set_tls_host(const AppidChangeBits& change_bits)
     {
         if (tsession and change_bits[APPID_TLSHOST_BIT])
+        {
             api.set_tls_host(tsession->get_tls_host());
+            api.set_tls_sni(tsession->get_tls_sni());
+        }
     }
 
     void set_tls_host(const char* tls_host)
@@ -612,7 +634,10 @@ public:
     void set_tls_host()
     {
         if (tsession and tsession->is_tls_host_unpublished())
+        {
             api.set_tls_host(tsession->get_tls_host());
+            api.set_tls_sni(tsession->get_tls_sni());
+        }
     }
 
     void set_netbios_name(AppidChangeBits& change_bits, const char *name)

diff --git a/src/network_inspectors/appid/appid_session_api.h b/src/network_inspectors/appid/appid_session_api.h
index fd6c4e12fea951c645300665c1ddbb261f3c6cb6..e2e0669a0a222bf866fffb1a70fde92809f7a4cd 100644 (file)
--- a/src/network_inspectors/appid/appid_session_api.h
+++ b/src/network_inspectors/appid/appid_session_api.h
@@ -157,6 +157,8 @@ public:
 
     void clear_user_logged_in() { flags.user_logged_in = false; }
 
+    const char* get_tls_sni() const { return tls_sni; }
+
 protected:
     AppIdSessionApi(const AppIdSession* asd, const SfIp& ip);
 
@@ -176,6 +178,7 @@ private:
     snort::SfIp initiator_ip;
     ServiceAppDescriptor service;
     char* tls_host = nullptr;
+    char* tls_sni = nullptr;
     char* netbios_name = nullptr;
     char* netbios_domain = nullptr;
     std::string session_id;
@@ -203,6 +206,7 @@ private:
         snort_free(tls_host);
         snort_free(netbios_name);
         snort_free(netbios_domain);
+        snort_free(tls_sni);
         delete dsession;
     }
 
@@ -223,6 +227,16 @@ private:
         }
     }
 
+    void set_tls_sni(const char* sni)
+    {
+        if (sni and sni != tls_sni)
+        {
+            if (tls_sni)
+                snort_free(tls_sni);
+            tls_sni = snort_strdup(sni);
+        }
+    }
+
     friend AppIdSession;
 };
 

diff --git a/src/network_inspectors/appid/service_plugins/service_ssl.cc b/src/network_inspectors/appid/service_plugins/service_ssl.cc
index 1c6f9190bab2a35559f32d1e12e7b8bbf7cf80c6..0ad0ce70c7eb9955b6903684de44e820ee77472f 100644 (file)
--- a/src/network_inspectors/appid/service_plugins/service_ssl.cc
+++ b/src/network_inspectors/appid/service_plugins/service_ssl.cc
@@ -525,19 +525,13 @@ success:
             args.asd.tsession->set_tls_host(ss->client_hello.host_name, 0, args.change_bits);
             args.asd.scan_flags |= SCAN_SSL_HOST_FLAG;
         }
-        else if (ss->server_cert.common_name)
-        {
-            /* Use common name (from server) if we didn't get host name (from client). */
-            args.asd.tsession->set_tls_host(ss->server_cert.common_name, ss->server_cert.common_name_strlen,
-                args.change_bits);
-            args.asd.scan_flags |= SCAN_SSL_HOST_FLAG;
-        }
 
         /* TLS Common Name */
         if (ss->server_cert.common_name)
         {
             args.asd.tsession->set_tls_cname(ss->server_cert.common_name, 0, args.change_bits);
             args.asd.scan_flags |= SCAN_SSL_CERTIFICATE_FLAG;
+            args.asd.scan_flags |= SCAN_SSL_HOST_FLAG;
         }
         /* TLS Org Unit */
         if (ss->server_cert.org_unit)

diff --git a/src/network_inspectors/appid/test/appid_api_test.cc b/src/network_inspectors/appid/test/appid_api_test.cc
index ec552b19c9065d7ca777ddc7249a0865b1e16f43..eec184f3fe6cb2b50419f732cfb703c037802faa 100644 (file)
--- a/src/network_inspectors/appid/test/appid_api_test.cc
+++ b/src/network_inspectors/appid/test/appid_api_test.cc
@@ -257,7 +257,7 @@ TEST(appid_api, get_application_id)
 
 TEST(appid_api, ssl_app_group_id_lookup)
 {
-    mock().expectNCalls(6, "publish");
+    mock().expectNCalls(7, "publish");
     AppId service, client, payload = APP_ID_NONE;
     bool val = false;
 
@@ -284,6 +284,7 @@ TEST(appid_api, ssl_app_group_id_lookup)
     STRCMP_EQUAL(mock_session->tsession->get_tls_host(), APPID_UT_TLS_HOST);
     STRCMP_EQUAL(mock_session->tsession->get_tls_first_alt_name(), APPID_UT_TLS_HOST);
     STRCMP_EQUAL(mock_session->tsession->get_tls_cname(), APPID_UT_TLS_HOST);
+    STRCMP_EQUAL(mock_session->tsession->get_tls_sni(),  APPID_UT_TLS_HOST);
     STRCMP_EQUAL("Published change_bits == 00000000000100011000", test_log);
 
     // Common name based detection
@@ -346,6 +347,18 @@ TEST(appid_api, ssl_app_group_id_lookup)
     STRCMP_EQUAL(mock_session->tsession->get_tls_cname(), APPID_UT_TLS_HOST);
     STRCMP_EQUAL("Published change_bits == 00000000000100011000", test_log);
 
+    //check for sni mismatch being stored in sni field
+    change_bits.reset();
+    mock_session->tsession->set_tls_host("mismatchedsni.com", 17, change_bits);
+    service = APP_ID_NONE;
+    client = APP_ID_NONE;
+    payload = APP_ID_NONE;
+    val = appid_api.ssl_app_group_id_lookup(flow, (const char*)APPID_UT_TLS_HOST, (const char*)APPID_UT_TLS_HOST,
+        nullptr, nullptr, true, service, client, payload);
+    CHECK_TRUE(val);
+    STRCMP_EQUAL(APPID_UT_TLS_HOST, mock_session->tsession->get_tls_host());
+    STRCMP_EQUAL("mismatchedsni.com", mock_session->tsession->get_tls_sni());
+
     mock().checkExpectations();
 
     // When appid session is not existing