From: Oleksandr Stepanov -X (ostepano - SOFTSERVE INC at Cisco) Date: Fri, 21 Nov 2025 19:41:57 +0000 (+0000) Subject: Pull request #4995: appid: ignore empty strings in ssl lookup api X-Git-Tag: 3.10.0.0~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dacbb7c981c476972940894d6234b6359e7670dc;p=thirdparty%2Fsnort3.git 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 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 5bc73dc8b..688e9f5f0 100644 --- 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 f694b903b..dcdf4958a 100644 --- 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(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(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 15f42f04a..a3ee9657e 100644 --- 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 d56f5808f..4fede8f6c 100644 --- 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;