Pull request #4144: appid: process organization unit instead of organization name

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

Squashed commit of the following:

commit 1182e2ebd813cc0b6a523438704d44ff95e4691e
Author: Oleksandr Stepanov <ostepano@cisco.com>
Date:   Wed Dec 13 07:24:00 2023 -0500

    appid: process organization unit instead of organization name

diff --git a/src/network_inspectors/appid/appid_api.cc b/src/network_inspectors/appid/appid_api.cc
index ccb7a7456508a002b36c7df8d880bb3164a57110..87b1a9ec1251377b1fa3a7e81fdc11abcf5fae01 100644 (file)
--- a/src/network_inspectors/appid/appid_api.cc
+++ b/src/network_inspectors/appid/appid_api.cc
@@ -146,6 +146,18 @@ bool AppIdApi::ssl_app_group_id_lookup(Flow* flow, const char* server_name,
         if (sni_mismatch)
             asd->scan_flags |= SCAN_SPOOFED_SNI_FLAG;
 
+        if (org_unit)
+        {
+            asd->tsession->set_tls_org_unit(org_unit, strlen(org_unit));
+            if (client_id == APP_ID_NONE and payload_id == APP_ID_NONE)
+            {
+                ssl_matchers.scan_cname((const uint8_t*)org_unit, strlen(org_unit),
+                    client_id, payload_id);
+                if (client_id != APP_ID_NONE or payload_id != APP_ID_NONE)
+                    asd->tsession->set_matched_tls_type(MatchedTlsType::MATCHED_TLS_ORG_UNIT);
+            }
+        }
+
         if (server_name and !sni_mismatch)
         {
             asd->tsession->set_tls_host(server_name, strlen(server_name), change_bits);
@@ -179,18 +191,6 @@ 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));
-            if (client_id == APP_ID_NONE and payload_id == APP_ID_NONE)
-            {
-                ssl_matchers.scan_cname((const uint8_t*)org_unit, strlen(org_unit),
-                    client_id, payload_id);
-                if (client_id != APP_ID_NONE or payload_id != APP_ID_NONE)
-                    asd->tsession->set_matched_tls_type(MatchedTlsType::MATCHED_TLS_ORG_UNIT);
-            }
-        }
-
         asd->scan_flags |= SCAN_CERTVIZ_ENABLED_FLAG;
 
         service_id = asd->get_api().get_service_app_id();

diff --git a/src/network_inspectors/appid/appid_session.cc b/src/network_inspectors/appid/appid_session.cc
index b3ed6d190595989bbfb6ad88acc3042a1394790c..ec85268e3c7ec8b79bd875075823fdaadbca9df5 100644 (file)
--- a/src/network_inspectors/appid/appid_session.cc
+++ b/src/network_inspectors/appid/appid_session.cc
@@ -496,45 +496,45 @@ void AppIdSession::examine_ssl_metadata(AppidChangeBits& change_bits)
 {
     AppId client_id = 0;
     AppId payload_id = 0;
-    const char* tls_str = tsession->get_tls_host();
+    const char* tls_str = tsession->get_tls_org_unit();
 
     if (scan_flags & SCAN_CERTVIZ_ENABLED_FLAG)
         return;
 
-    if ((scan_flags & SCAN_SSL_HOST_FLAG) and tls_str)
+    if (tls_str)
     {
         size_t size = strlen(tls_str);
-        if (odp_ctxt.get_ssl_matchers().scan_hostname((const uint8_t*)tls_str, size,
+        if (odp_ctxt.get_ssl_matchers().scan_cname((const uint8_t*)tls_str, size,
             client_id, payload_id))
         {
-            if (api.client.get_id() == APP_ID_NONE or api.client.get_id() == APP_ID_SSL_CLIENT)
-                set_client_appid_data(client_id, change_bits);
+            set_client_appid_data(client_id, change_bits);
             set_payload_appid_data(payload_id);
         }
-        scan_flags &= ~SCAN_SSL_HOST_FLAG;
+        tsession->set_tls_org_unit(nullptr, 0);
     }
-    if ((scan_flags & SCAN_SSL_CERTIFICATE_FLAG) and (tls_str = tsession->get_tls_cname()))
+    if ((scan_flags & SCAN_SSL_HOST_FLAG) and (tls_str = tsession->get_tls_host()))
     {
         size_t size = strlen(tls_str);
-        if (odp_ctxt.get_ssl_matchers().scan_cname((const uint8_t*)tls_str, size,
+        if (odp_ctxt.get_ssl_matchers().scan_hostname((const uint8_t*)tls_str, size,
             client_id, payload_id))
         {
             if (api.client.get_id() == APP_ID_NONE or api.client.get_id() == APP_ID_SSL_CLIENT)
                 set_client_appid_data(client_id, change_bits);
             set_payload_appid_data(payload_id);
         }
-        scan_flags &= ~SCAN_SSL_CERTIFICATE_FLAG;
+        scan_flags &= ~SCAN_SSL_HOST_FLAG;
     }
-    if ((tls_str = tsession->get_tls_org_unit()))
+    if ((scan_flags & SCAN_SSL_CERTIFICATE_FLAG) and (tls_str = tsession->get_tls_cname()))
     {
         size_t size = strlen(tls_str);
         if (odp_ctxt.get_ssl_matchers().scan_cname((const uint8_t*)tls_str, size,
             client_id, payload_id))
         {
-            set_client_appid_data(client_id, change_bits);
+            if (api.client.get_id() == APP_ID_NONE or api.client.get_id() == APP_ID_SSL_CLIENT)
+                set_client_appid_data(client_id, change_bits);
             set_payload_appid_data(payload_id);
         }
-        tsession->set_tls_org_unit(nullptr, 0);
+        scan_flags &= ~SCAN_SSL_CERTIFICATE_FLAG;
     }
     if (tsession->get_tls_handshake_done() and
         api.payload.get_id() == APP_ID_NONE)

diff --git a/src/network_inspectors/appid/service_plugins/service_ssl.cc b/src/network_inspectors/appid/service_plugins/service_ssl.cc
index e73d9658dbb22e55578f05d7c3eac3bc79b900a5..1c6f9190bab2a35559f32d1e12e7b8bbf7cf80c6 100644 (file)
--- a/src/network_inspectors/appid/service_plugins/service_ssl.cc
+++ b/src/network_inspectors/appid/service_plugins/service_ssl.cc
@@ -514,7 +514,7 @@ success:
     }
 
     args.asd.set_session_flags(APPID_SESSION_SSL_SESSION);
-    if (ss->client_hello.host_name || ss->server_cert.common_name || ss->server_cert.org_name)
+    if (ss->client_hello.host_name || ss->server_cert.common_name || ss->server_cert.org_unit)
     {
         if (!args.asd.tsession)
             args.asd.tsession = new TlsSession();
@@ -540,10 +540,10 @@ success:
             args.asd.scan_flags |= SCAN_SSL_CERTIFICATE_FLAG;
         }
         /* TLS Org Unit */
-        if (ss->server_cert.org_name)
-            args.asd.tsession->set_tls_org_unit(ss->server_cert.org_name, 0);
+        if (ss->server_cert.org_unit)
+            args.asd.tsession->set_tls_org_unit(ss->server_cert.org_unit, 0);
 
-        ss->client_hello.host_name = ss->server_cert.common_name = ss->server_cert.org_name = nullptr;
+        ss->client_hello.host_name = ss->server_cert.common_name = ss->server_cert.org_unit = nullptr;
         args.asd.tsession->set_tls_handshake_done();
     }
     return add_service(args.change_bits, args.asd, args.pkt, args.dir,

diff --git a/src/network_inspectors/appid/test/appid_api_test.cc b/src/network_inspectors/appid/test/appid_api_test.cc
index a190225c369dae64bb4920c372e6d7939a91c578..13914a0ca6fc1c9b97b9834e0dad41306d6afcdd 100644 (file)
--- a/src/network_inspectors/appid/test/appid_api_test.cc
+++ b/src/network_inspectors/appid/test/appid_api_test.cc
@@ -310,7 +310,7 @@ TEST(appid_api, ssl_app_group_id_lookup)
     string host = "";
     change_bits.reset();
     mock_session->tsession->set_tls_host("", 0, change_bits);
-    val = appid_api.ssl_app_group_id_lookup(flow, (const char*)(host.c_str()), nullptr,
+    val = appid_api.ssl_app_group_id_lookup(flow, nullptr, nullptr,
         nullptr, (const char*)APPID_UT_ORG_UNIT, false, service, client, payload);
     CHECK_TRUE(val);
     CHECK_EQUAL(client, APPID_UT_ID + 3);

diff --git a/src/protocols/ssl.cc b/src/protocols/ssl.cc
index 0f4c162382ac3246c91267828506a18a849b28ee..84a3f4a309364f82d2ce160a0aa15adc458a6c64 100644 (file)
--- a/src/protocols/ssl.cc
+++ b/src/protocols/ssl.cc
@@ -58,7 +58,7 @@ SSLV3ServerCertData::~SSLV3ServerCertData()
 {
     snort_free(certs_data);
     snort_free(common_name);
-    snort_free(org_name);
+    snort_free(org_unit);
 }
 
 void SSLV3ServerCertData::clear()
@@ -69,8 +69,8 @@ void SSLV3ServerCertData::clear()
     snort_free(common_name);
     common_name = nullptr;
 
-    snort_free(org_name);
-    org_name = nullptr;
+    snort_free(org_unit);
+    org_unit = nullptr;
 }
 
 static uint32_t SSL_decode_version_v3(uint8_t major, uint8_t minor)
@@ -699,13 +699,13 @@ bool parse_server_certificates(SSLV3ServerCertData* server_cert_data)
         return false;
 
     char* common_name = nullptr;
-    char* org_name = nullptr;
+    char* org_unit = nullptr;
     const uint8_t* data = server_cert_data->certs_data;
     int len = server_cert_data->certs_len;
     int common_name_len = 0;
-    int org_name_len  = 0;
+    int org_unit_len  = 0;
 
-    while (len > 0 and !(common_name and org_name))
+    while (len > 0 and !(common_name and org_unit))
     {
         X509* cert = nullptr;
         X509_NAME* cert_name = nullptr;
@@ -714,13 +714,13 @@ bool parse_server_certificates(SSLV3ServerCertData* server_cert_data)
         data += 3;
         len -= 3;
         if (len < cert_len)
-            return false;
+            break;
 
         /* d2i_X509() increments the data ptr for us. */
         cert = d2i_X509(nullptr, (const unsigned char**)&data, cert_len);
         len -= cert_len;
         if (!cert)
-            return false;
+            break;
 
         if (nullptr == (cert_name = X509_get_subject_name(cert)))
         {
@@ -744,9 +744,19 @@ bool parse_server_certificates(SSLV3ServerCertData* server_cert_data)
 
                 common_name_len = length;
                 common_name = snort_strndup((const char*)(str_data + (wildcard ? 2 : 0)), common_name_len);
+            }
+        }
 
-                org_name_len = length;
-                org_name = snort_strndup((const char*)(str_data + (wildcard ? 2 : 0)), org_name_len);
+        if (!org_unit)
+        {
+            int lastpos = -1;
+            lastpos = X509_NAME_get_index_by_NID(cert_name, NID_organizationalUnitName, lastpos);
+            if (lastpos != -1)
+            {
+                X509_NAME_ENTRY* e = X509_NAME_get_entry(cert_name, lastpos);
+                const unsigned char* str_data = ASN1_STRING_get0_data(X509_NAME_ENTRY_get_data(e));
+                org_unit_len = strlen((const char*)str_data);
+                org_unit = snort_strndup((const char*)(str_data), org_unit_len);
             }
         }
 
@@ -758,9 +768,12 @@ bool parse_server_certificates(SSLV3ServerCertData* server_cert_data)
     {
         server_cert_data->common_name = common_name;
         server_cert_data->common_name_strlen = common_name_len;
+    }
 
-        server_cert_data->org_name = org_name;
-        server_cert_data->org_name_strlen = org_name_len;
+    if (org_unit)
+    {
+        server_cert_data->org_unit = org_unit;
+        server_cert_data->org_unit_strlen = org_unit_len;
     }
 
     /* No longer need entire certificates. We have what we came for. */

diff --git a/src/protocols/ssl.h b/src/protocols/ssl.h
index 5ab7af96d6ebcc084543c9186431fcd96b70edb9..345f53d91b6554c2e572e5a563714600e76fc68f 100644 (file)
--- a/src/protocols/ssl.h
+++ b/src/protocols/ssl.h
@@ -218,8 +218,8 @@ struct SSLV3ServerCertData
     /* Data collected from certificates afterwards: */
     char* common_name = nullptr;
     int common_name_strlen;
-    char* org_name = nullptr;
-    int org_name_strlen;
+    char* org_unit = nullptr;
+    int org_unit_strlen;
 };
 
 enum class SSLV3RecordType : uint8_t