Pull request #3741: appid: publish tls host set in eve process event handler only...

Merge in SNORT/snort3 from ~SATHIRKA/snort3:quic_tls_host to master

Squashed commit of the following:

commit 47919a2706736d804c76dc493c61441d027e6824
Author: Sreeja Athirkandathil Narayanan <sathirka@cisco.com>
Date:   Wed Jan 18 10:33:31 2023 -0500

    appid: publish tls host set in eve process event handler only when appid discovery is complete

diff --git a/src/network_inspectors/appid/appid_discovery.cc b/src/network_inspectors/appid/appid_discovery.cc
index 7b03355b3adbc6ab882929c19291545a3ed54bac..e46763ee217eeb54d8a5c6fabdfbc27320114d06 100644 (file)
--- a/src/network_inspectors/appid/appid_discovery.cc
+++ b/src/network_inspectors/appid/appid_discovery.cc
@@ -882,6 +882,17 @@ void AppIdDiscovery::do_post_discovery(Packet* p, AppIdSession& asd,
     asd.set_ss_application_ids(service_id, client_id, payload_id, misc_id,
         asd.pick_ss_referred_payload_app_id(), change_bits);
     asd.set_tls_host(change_bits);
+    if (asd.tsession and asd.tsession->is_tls_host_unpublished())
+    {
+        change_bits.set(APPID_TLSHOST_BIT);
+        asd.tsession->set_tls_host_unpublished(false);
+    }
+
+    if (asd.is_client_info_unpublished())
+    {
+        change_bits.set(APPID_CLIENT_INFO_BIT);
+        asd.set_client_info_unpublished(false);
+    }
 
     if (PacketTracer::is_daq_activated())
         populate_trace_data(asd); 

diff --git a/src/network_inspectors/appid/appid_eve_process_event_handler.cc b/src/network_inspectors/appid/appid_eve_process_event_handler.cc
index 0493f584dc1f79442d32b0207af6406a2cb5d186..bfcca1ae5f5b90dedb947c7bfd7d8630371ce63f 100644 (file)
--- a/src/network_inspectors/appid/appid_eve_process_event_handler.cc
+++ b/src/network_inspectors/appid/appid_eve_process_event_handler.cc
@@ -75,12 +75,12 @@ void AppIdEveProcessEventHandler::handle(DataEvent& event, Flow* flow)
     const bool is_quic = eve_process_event.is_flow_quic();
     const bool is_client_process_flag = eve_process_event.is_client_process_mapping();
 
-    AppidChangeBits change_bits;
+    OdpContext& odp_ctxt = asd->get_odp_ctxt();
 
     if (is_quic && alpn_vec.size())
     {
         AppId service_id = APP_ID_NONE;
-        service_id = asd->get_odp_ctxt().get_alpn_matchers().match_alpn_pattern(alpn_vec[0]);
+        service_id = odp_ctxt.get_alpn_matchers().match_alpn_pattern(alpn_vec[0]);
         if (service_id)
         {
             asd->set_alpn_service_app_id(service_id);
@@ -88,7 +88,7 @@ void AppIdEveProcessEventHandler::handle(DataEvent& event, Flow* flow)
         }
         else
         {
-            asd->set_service_appid_data(APP_ID_QUIC, change_bits);
+            asd->set_service_id(APP_ID_QUIC, odp_ctxt);
             asd->set_session_flags(APPID_SESSION_SERVICE_DETECTED);
         }
     }
@@ -99,18 +99,17 @@ void AppIdEveProcessEventHandler::handle(DataEvent& event, Flow* flow)
         char* version = nullptr;
         AppId service_id = APP_ID_NONE;
 
-        asd->get_odp_ctxt().get_http_matchers().identify_user_agent(user_agent.c_str(),
+        odp_ctxt.get_http_matchers().identify_user_agent(user_agent.c_str(),
             user_agent.size(), service_id, client_id, &version);
 
         if (client_id != APP_ID_NONE)
-            asd->set_client_appid_data(client_id, change_bits, version);
+            asd->set_client_appid_data(client_id, version);
 
         snort_free(version);
     }
     else if (!name.empty() and is_client_process_flag)
     {
-        client_id = asd->get_odp_ctxt().get_eve_ca_matchers().match_eve_ca_pattern(name,
-            conf);
+        client_id = odp_ctxt.get_eve_ca_matchers().match_eve_ca_pattern(name, conf);
 
         asd->set_eve_client_app_id(client_id);
     }
@@ -123,10 +122,10 @@ void AppIdEveProcessEventHandler::handle(DataEvent& event, Flow* flow)
         if (!asd->tsession)
             asd->tsession = new TlsSession();
 
-        asd->tsession->set_tls_host(server_name.c_str(), server_name.length(), change_bits);
-        asd->set_tls_host(change_bits);
+        asd->tsession->set_tls_host(server_name.c_str(), server_name.length());
+        asd->set_tls_host();
 
-        asd->get_odp_ctxt().get_ssl_matchers().scan_hostname(reinterpret_cast<const uint8_t*>(server_name.c_str()),
+        odp_ctxt.get_ssl_matchers().scan_hostname(reinterpret_cast<const uint8_t*>(server_name.c_str()),
             server_name.length(), client_id, payload_id);
         asd->set_payload_id(payload_id);
     }
@@ -156,7 +155,4 @@ void AppIdEveProcessEventHandler::handle(DataEvent& event, Flow* flow)
         LogMessage("AppIdDbg %s %s\n",
             appidDebug->get_debug_session(), debug_str.c_str());
     }
-
-    if (change_bits.any())
-        asd->publish_appid_event(change_bits, *p);
 }

diff --git a/src/network_inspectors/appid/appid_session.cc b/src/network_inspectors/appid/appid_session.cc
index 398aa734cbf8bde7b17ea598716c8f73ec433fca..575129ceda199f666a98c11925b32e611820abde 100644 (file)
--- a/src/network_inspectors/appid/appid_session.cc
+++ b/src/network_inspectors/appid/appid_session.cc
@@ -616,7 +616,7 @@ void AppIdSession::examine_rtmp_metadata(AppidChangeBits& change_bits)
     }
 }
 
-void AppIdSession::set_client_appid_data(AppId id, AppidChangeBits& change_bits, char* version)
+void AppIdSession::set_client_appid_data(AppId id, char* version, bool published)
 {
     if (id <= APP_ID_NONE or id == APP_ID_HTTP)
         return;
@@ -633,7 +633,16 @@ void AppIdSession::set_client_appid_data(AppId id, AppidChangeBits& change_bits,
     if (!version)
         return;
     api.client.set_version(version);
-    change_bits.set(APPID_CLIENT_INFO_BIT);
+
+    if (!published)
+        client_info_unpublished = true;
+}
+
+void AppIdSession::set_client_appid_data(AppId id, AppidChangeBits& change_bits, char* version)
+{
+    set_client_appid_data(id, version, true);
+    if (version)
+        change_bits.set(APPID_CLIENT_INFO_BIT);
 }
 
 void AppIdSession::set_payload_appid_data(AppId id, char* version)

diff --git a/src/network_inspectors/appid/appid_session.h b/src/network_inspectors/appid/appid_session.h
index a94011c74ae8855706555c2cbda2e32d7f54c7b1..bb41d4a9114c0c543e901410dd3ac9ac1fbf18ac 100644 (file)
--- a/src/network_inspectors/appid/appid_session.h
+++ b/src/network_inspectors/appid/appid_session.h
@@ -150,7 +150,7 @@ public:
     bool get_tls_handshake_done() const { return tls_handshake_done; }
 
     // Duplicate only if len > 0, otherwise simply set (i.e., own the argument)
-    void set_tls_host(const char* new_tls_host, uint32_t len, AppidChangeBits& change_bits)
+    void set_tls_host(const char* new_tls_host, uint32_t len, bool published=false)
     {
         if (tls_host)
             snort_free(tls_host);
@@ -160,6 +160,14 @@ public:
             return;
         }
         tls_host = len? snort::snort_strndup(new_tls_host,len) : const_cast<char*>(new_tls_host);
+
+        if (!published)
+            tls_host_unpublished = true;
+    }
+
+    void set_tls_host(const char* new_tls_host, uint32_t len, AppidChangeBits& change_bits)
+    {
+        set_tls_host(new_tls_host, len, true);
         change_bits.set(APPID_TLSHOST_BIT);
     }
 
@@ -213,12 +221,17 @@ public:
         matched_tls_type = type;
     }
 
+    void set_tls_host_unpublished(bool val) { tls_host_unpublished = val; }
+
+    bool is_tls_host_unpublished() const { return tls_host_unpublished; }
+
 private:
     char* tls_host = nullptr;
     char* tls_first_alt_name = nullptr;
     char* tls_cname = nullptr;
     char* tls_org_unit = nullptr;
     bool tls_handshake_done = false;
+    bool tls_host_unpublished = false;
     MatchedTlsType matched_tls_type = MATCHED_TLS_NONE;
 };
 
@@ -340,6 +353,7 @@ public:
 
     void examine_ssl_metadata(AppidChangeBits& change_bits);
     void set_client_appid_data(AppId, AppidChangeBits& change_bits, char* version = nullptr);
+    void set_client_appid_data(AppId, char* version = nullptr, bool published=false);
     void set_service_appid_data(AppId, AppidChangeBits& change_bits, char* version = nullptr);
     void set_payload_appid_data(AppId, char* version = nullptr);
     void check_app_detection_restart(AppidChangeBits& change_bits,
@@ -580,6 +594,12 @@ public:
         api.set_tls_host(tls_host);
     }
 
+    void set_tls_host()
+    {
+        if (tsession and tsession->is_tls_host_unpublished())
+            api.set_tls_host(tsession->get_tls_host());
+    }
+
     void set_netbios_name(AppidChangeBits& change_bits, const char *name)
     {
         api.set_netbios_name(change_bits, name);
@@ -663,6 +683,15 @@ public:
         no_service_inspector = true;
     }
 
+    void set_client_info_unpublished(bool val)
+    {
+        client_info_unpublished = val;
+    }
+
+    bool is_client_info_unpublished()
+    {
+        return client_info_unpublished;
+    }
 private:
     uint16_t prev_httpx_raw_packet = 0;
 
@@ -685,6 +714,7 @@ private:
     bool consumed_ha_data = false;
     bool no_service_candidate = false;
     bool no_service_inspector = false;
+    bool client_info_unpublished = false;
 };
 
 #endif

diff --git a/src/network_inspectors/appid/test/appid_eve_process_event_handler_test.cc b/src/network_inspectors/appid/test/appid_eve_process_event_handler_test.cc
index 0819f0ddf705d7628b89c2d4d9510b41b61cbbc9..69a2da54e6430da7dfc4498d1c35bdcb9400c005 100644 (file)
--- a/src/network_inspectors/appid/test/appid_eve_process_event_handler_test.cc
+++ b/src/network_inspectors/appid/test/appid_eve_process_event_handler_test.cc
@@ -82,7 +82,7 @@ void AppIdSession::set_ss_application_ids_payload(AppId, AppidChangeBits&)
     return;
 }
 
-void AppIdSession::set_client_appid_data(AppId, AppidChangeBits&, char*)
+void AppIdSession::set_client_appid_data(AppId, char*, bool)
 {
     set_client_id(APPID_UT_ID);
     return;
@@ -99,10 +99,6 @@ void HttpPatternMatchers::identify_user_agent(const char*, int, AppId&, AppId& c
     client = APPID_UT_ID;
 }
 
-void AppIdSession::set_service_appid_data(AppId, AppidChangeBits&, char*)
-{
-}
-
 AppId AlpnPatternMatchers::match_alpn_pattern(const string& str)
 {
     if (!str.compare("h3"))