From: Sreeja Athirkandathil Narayanan (sathirka) Date: Tue, 31 Jan 2023 21:51:15 +0000 (+0000) Subject: Pull request #3751: appid: use packet thread's odp context instead of inspector's... X-Git-Tag: 3.1.55.0~8 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=497d21ca94759f737e637b6f95c60e49eee09ab1;p=thirdparty%2Fsnort3.git Pull request #3751: appid: use packet thread's odp context instead of inspector's context for packet processing Merge in SNORT/snort3 from ~SATHIRKA/snort3:reload_fixes to master Squashed commit of the following: commit fb0d3790437f4b3974552ca94aa68b186b282fd2 Author: Sreeja Athirkandathil Narayanan Date: Fri Jan 20 10:24:30 2023 -0500 appid: use packet thread's odp context instead of inspector's context for packet processing --- diff --git a/src/network_inspectors/appid/appid_api.cc b/src/network_inspectors/appid/appid_api.cc index f06316261..cf33b6b32 100644 --- a/src/network_inspectors/appid/appid_api.cc +++ b/src/network_inspectors/appid/appid_api.cc @@ -124,14 +124,16 @@ bool AppIdApi::ssl_app_group_id_lookup(Flow* flow, const char* server_name, client_id = APP_ID_NONE; payload_id = APP_ID_NONE; + if (!pkt_thread_odp_ctxt) + return false; + if (flow) asd = get_appid_session(*flow); if (asd) { // Skip detection for sessions using old odp context after odp reload - if (!pkt_thread_odp_ctxt or - pkt_thread_odp_ctxt->get_version() != asd->get_odp_ctxt_version()) + if (pkt_thread_odp_ctxt->get_version() != asd->get_odp_ctxt_version()) return false; AppidChangeBits change_bits; @@ -213,24 +215,20 @@ bool AppIdApi::ssl_app_group_id_lookup(Flow* flow, const char* server_name, } else { - AppIdInspector* inspector = (AppIdInspector*) InspectorManager::get_inspector(MOD_NAME, true); - if (inspector) - { - SslPatternMatchers& ssl_matchers = inspector->get_ctxt().get_odp_ctxt().get_ssl_matchers(); + SslPatternMatchers& ssl_matchers = pkt_thread_odp_ctxt->get_ssl_matchers(); - if (server_name and !sni_mismatch) - ssl_matchers.scan_hostname((const uint8_t*)server_name, strlen(server_name), - client_id, payload_id); - if (first_alt_name and client_id == APP_ID_NONE and payload_id == APP_ID_NONE) - ssl_matchers.scan_hostname((const uint8_t*)first_alt_name, strlen(first_alt_name), - client_id, payload_id); - if (common_name and client_id == APP_ID_NONE and payload_id == APP_ID_NONE) - ssl_matchers.scan_cname((const uint8_t*)common_name, strlen(common_name), client_id, - payload_id); - if (org_unit and 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 (server_name and !sni_mismatch) + ssl_matchers.scan_hostname((const uint8_t*)server_name, strlen(server_name), + client_id, payload_id); + if (first_alt_name and client_id == APP_ID_NONE and payload_id == APP_ID_NONE) + ssl_matchers.scan_hostname((const uint8_t*)first_alt_name, strlen(first_alt_name), + client_id, payload_id); + if (common_name and client_id == APP_ID_NONE and payload_id == APP_ID_NONE) + ssl_matchers.scan_cname((const uint8_t*)common_name, strlen(common_name), client_id, + payload_id); + if (org_unit and 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) diff --git a/src/network_inspectors/appid/appid_ha.cc b/src/network_inspectors/appid/appid_ha.cc index 0b1a8ddfa..196a0f4ab 100644 --- a/src/network_inspectors/appid/appid_ha.cc +++ b/src/network_inspectors/appid/appid_ha.cc @@ -49,7 +49,7 @@ static AppIdSession* create_appid_session(Flow& flow, const FlowKey* key, AppIdSession* asd = new AppIdSession(static_cast(key->ip_protocol), flow.flags.client_initiated ? &flow.client_ip : &flow.server_ip, flow.flags.client_initiated ? flow.client_port : flow.server_port, inspector, - inspector.get_ctxt().get_odp_ctxt(), key->addressSpaceId); + *pkt_thread_odp_ctxt, key->addressSpaceId); if (appidDebug->is_active()) LogMessage("AppIdDbg %s high-avail - New AppId session created in consume\n", appidDebug->get_debug_session()); @@ -70,7 +70,7 @@ bool AppIdHAAppsClient::consume(Flow*& flow, const FlowKey* key, HAMessage& msg, AppIdInspector* inspector = static_cast( InspectorManager::get_inspector(MOD_NAME, MOD_USAGE, appid_inspector_api.type)); - if (!inspector) + if (!inspector or !pkt_thread_odp_ctxt) return false; AppIdSession* asd = (AppIdSession*)(flow->get_flow_data(AppIdSession::inspector_id)); @@ -107,7 +107,7 @@ bool AppIdHAAppsClient::consume(Flow*& flow, const FlowKey* key, HAMessage& msg, asd->service_disco_state = APPID_DISCO_STATE_FINISHED; asd->client_disco_state = APPID_DISCO_STATE_FINISHED; - if (asd->get_tp_appid_ctxt()) + if (asd->get_tp_appid_ctxt() and !ThirdPartyAppIdContext::get_tp_reload_in_progress()) { const TPLibHandler* tph = TPLibHandler::get(); TpAppIdCreateSession tpsf = tph->tpsession_factory(); @@ -232,7 +232,7 @@ bool AppIdHAHttpClient::consume(Flow*& flow, const FlowKey* key, HAMessage& msg, AppIdInspector* inspector = static_cast( InspectorManager::get_inspector(MOD_NAME, MOD_USAGE, appid_inspector_api.type)); - if (!inspector) + if (!inspector or !pkt_thread_odp_ctxt) return false; AppIdSession* asd = appid_api.get_appid_session(*flow); @@ -322,7 +322,7 @@ bool AppIdHATlsHostClient::consume(Flow*& flow, const FlowKey* key, HAMessage& m AppIdInspector* inspector = static_cast( InspectorManager::get_inspector(MOD_NAME, MOD_USAGE, appid_inspector_api.type)); - if (!inspector) + if (!inspector or !pkt_thread_odp_ctxt) return false; AppIdSession* asd = appid_api.get_appid_session(*flow); diff --git a/src/network_inspectors/appid/appid_session.h b/src/network_inspectors/appid/appid_session.h index bb41d4a91..15639ded3 100644 --- a/src/network_inspectors/appid/appid_session.h +++ b/src/network_inspectors/appid/appid_session.h @@ -168,7 +168,8 @@ public: 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); + if (new_tls_host and *new_tls_host != '\0') + change_bits.set(APPID_TLSHOST_BIT); } void set_tls_first_alt_name(const char* new_tls_first_alt_name, uint32_t len, AppidChangeBits& change_bits) diff --git a/src/network_inspectors/appid/lua_detector_api.cc b/src/network_inspectors/appid/lua_detector_api.cc index c55bb33c9..678b5c031 100644 --- a/src/network_inspectors/appid/lua_detector_api.cc +++ b/src/network_inspectors/appid/lua_detector_api.cc @@ -545,7 +545,7 @@ static int service_add_service(lua_State* L) /*Phase2 - discuss AppIdServiceSubtype will be maintained on lua side therefore the last parameter on the following call is nullptr. Subtype is not displayed on DC at present. */ unsigned int retValue = ud->sd->add_service(*lsd->ldp.change_bits, *lsd->ldp.asd, lsd->ldp.pkt, - lsd->ldp.dir, ud->get_odp_ctxt().get_app_info_mgr().get_appid_by_service_id(service_id), + lsd->ldp.dir, lsd->ldp.asd->get_odp_ctxt().get_app_info_mgr().get_appid_by_service_id(service_id), vendor, version, nullptr); lua_pushnumber(L, retValue); @@ -944,9 +944,10 @@ static int client_add_application(lua_State* L) unsigned int service_id = lua_tonumber(L, 2); unsigned int productId = lua_tonumber(L, 4); const char* version = lua_tostring(L, 5); + OdpContext& odp_ctxt = lsd->ldp.asd->get_odp_ctxt(); ud->cd->add_app(*lsd->ldp.pkt, *lsd->ldp.asd, lsd->ldp.dir, - ud->get_odp_ctxt().get_app_info_mgr().get_appid_by_service_id(service_id), - ud->get_odp_ctxt().get_app_info_mgr().get_appid_by_client_id(productId), version, + odp_ctxt.get_app_info_mgr().get_appid_by_service_id(service_id), + odp_ctxt.get_app_info_mgr().get_appid_by_client_id(productId), version, *lsd->ldp.change_bits); lua_pushnumber(L, 0); @@ -968,7 +969,7 @@ static int client_add_user(lua_State* L) const char* userName = lua_tostring(L, 2); unsigned int service_id = lua_tonumber(L, 3); ud->cd->add_user(*lsd->ldp.asd, userName, - ud->get_odp_ctxt().get_app_info_mgr().get_appid_by_service_id(service_id), true, + lsd->ldp.asd->get_odp_ctxt().get_app_info_mgr().get_appid_by_service_id(service_id), true, *lsd->ldp.change_bits); lua_pushnumber(L, 0); return 1; @@ -982,7 +983,7 @@ static int client_add_payload(lua_State* L) unsigned int payloadId = lua_tonumber(L, 2); ud->cd->add_payload(*lsd->ldp.asd, - ud->get_odp_ctxt().get_app_info_mgr().get_appid_by_payload_id(payloadId)); + lsd->ldp.asd->get_odp_ctxt().get_app_info_mgr().get_appid_by_payload_id(payloadId)); lua_pushnumber(L, 0); return 1; @@ -1302,9 +1303,9 @@ static int detector_add_host_port_dynamic(lua_State* L) { auto& ud = *UserData::check(L, DETECTOR, 1); // Verify detector user data and that we are in packet context - ud->validate_lua_state(true); + LuaStateDescriptor* lsd = ud->validate_lua_state(true); - if (!ud->get_odp_ctxt().is_host_port_app_cache_runtime) + if (!lsd->ldp.asd->get_odp_ctxt().is_host_port_app_cache_runtime) return 0; SfIp ip_address; @@ -2635,16 +2636,16 @@ static int create_future_flow(lua_State* L) AppId client_id = lua_tointeger(L, 8); AppId payload_id = lua_tointeger(L, 9); AppId app_id_to_snort = lua_tointeger(L, 10); + OdpContext& odp_ctxt = lsd->ldp.asd->get_odp_ctxt(); if (app_id_to_snort > APP_ID_NONE) { - AppInfoTableEntry* entry = ud->get_odp_ctxt().get_app_info_mgr().get_app_info_entry( + AppInfoTableEntry* entry = odp_ctxt.get_app_info_mgr().get_app_info_entry( app_id_to_snort); if (!entry) return 0; snort_protocol_id = entry->snort_protocol_id; } - OdpContext& odp_ctxt = lsd->ldp.asd->get_odp_ctxt(); AppIdSession* fp = AppIdSession::create_future_session(lsd->ldp.pkt, &client_addr, client_port, &server_addr, server_port, proto, snort_protocol_id, odp_ctxt); if (fp) diff --git a/src/network_inspectors/appid/test/appid_api_test.cc b/src/network_inspectors/appid/test/appid_api_test.cc index 635f95f0a..55a196e61 100644 --- a/src/network_inspectors/appid/test/appid_api_test.cc +++ b/src/network_inspectors/appid/test/appid_api_test.cc @@ -49,6 +49,7 @@ using namespace snort; static SnortProtocolId dummy_http2_protocol_id = 1; +char const* APPID_UT_ORG_UNIT = "Google"; namespace snort { @@ -111,9 +112,14 @@ bool SslPatternMatchers::scan_cname(const uint8_t* cname, size_t, AppId& client_ { if (((const char*)cname) == APPID_UT_TLS_HOST) { - client_id = APPID_UT_ID + 2;; + client_id = APPID_UT_ID + 2; payload_id = APPID_UT_ID + 2; } + else if (((const char*)cname) == APPID_UT_ORG_UNIT) + { + client_id = APPID_UT_ID + 3; + payload_id = APPID_UT_ID + 3; + } else { client_id = 0; @@ -242,7 +248,7 @@ TEST(appid_api, get_application_id) TEST(appid_api, ssl_app_group_id_lookup) { - mock().expectNCalls(5, "publish"); + mock().expectNCalls(6, "publish"); AppId service, client, payload = APP_ID_NONE; bool val = false; @@ -257,6 +263,7 @@ TEST(appid_api, ssl_app_group_id_lookup) CHECK_EQUAL(payload, APPID_UT_ID); STRCMP_EQUAL("Published change_bits == 00000000000000000000", test_log); + // Server name based detection service = APP_ID_NONE; client = APP_ID_NONE; payload = APP_ID_NONE; @@ -270,6 +277,7 @@ 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 == 00000000000100000000", test_log); + // Common name based detection mock_session->tsession->set_tls_host("www.cisco.com", 13, change_bits); mock_session->tsession->set_tls_cname("www.cisco.com", 13, change_bits); mock_session->tsession->set_tls_org_unit("Cisco", 5); @@ -286,22 +294,37 @@ TEST(appid_api, ssl_app_group_id_lookup) STRCMP_EQUAL(mock_session->tsession->get_tls_org_unit(), "Cisco"); STRCMP_EQUAL("Published change_bits == 00000000000100000000", test_log); - string host = ""; - val = appid_api.ssl_app_group_id_lookup(flow, (const char*)(host.c_str()), nullptr, - (const char*)APPID_UT_TLS_HOST, (const char*)"Google", false, service, client, payload); + // First alt name based detection + change_bits.reset(); + mock_session->tsession->set_tls_host("", 0, change_bits); + val = appid_api.ssl_app_group_id_lookup(flow, nullptr, (const char*)APPID_UT_TLS_HOST, + nullptr, nullptr, false, service, client, payload); CHECK_TRUE(val); - CHECK_EQUAL(client, APPID_UT_ID + 2); - CHECK_EQUAL(payload, APPID_UT_ID + 2); + CHECK_EQUAL(client, APPID_UT_ID + 1); + CHECK_EQUAL(payload, APPID_UT_ID + 1); STRCMP_EQUAL(mock_session->tsession->get_tls_host(), APPID_UT_TLS_HOST); - STRCMP_EQUAL(mock_session->tsession->get_tls_cname(), APPID_UT_TLS_HOST); - STRCMP_EQUAL(mock_session->tsession->get_tls_org_unit(), "Google"); + STRCMP_EQUAL(mock_session->tsession->get_tls_first_alt_name(), APPID_UT_TLS_HOST); STRCMP_EQUAL("Published change_bits == 00000000000100000000", test_log); + // Org unit based detection + 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, + nullptr, (const char*)APPID_UT_ORG_UNIT, false, service, client, payload); + CHECK_TRUE(val); + CHECK_EQUAL(client, APPID_UT_ID + 3); + CHECK_EQUAL(payload, APPID_UT_ID + 3); + STRCMP_EQUAL(mock_session->tsession->get_tls_org_unit(), APPID_UT_ORG_UNIT); + STRCMP_EQUAL("Published change_bits == 00000000000000000000", test_log); + // Override client id found by SSL pattern matcher with the client id provided by // Encrypted Visibility Engine if available service = APP_ID_NONE; client = APP_ID_NONE; payload = APP_ID_NONE; + change_bits.reset(); + mock_session->tsession->set_tls_host("", 0, change_bits); mock_session->set_client_id(APP_ID_NONE); mock_session->set_eve_client_app_id(APPID_UT_ID + 100); val = appid_api.ssl_app_group_id_lookup(flow, (const char*)APPID_UT_TLS_HOST, (const char*)APPID_UT_TLS_HOST, @@ -315,6 +338,48 @@ TEST(appid_api, ssl_app_group_id_lookup) STRCMP_EQUAL("Published change_bits == 00000000000100000000", test_log); mock().checkExpectations(); + + // When appid session is not existing + // 1. Match based on server name + Flow* f = new Flow; + flow->set_flow_data(nullptr); + service = APP_ID_NONE; + client = APP_ID_NONE; + payload = APP_ID_NONE; + val = appid_api.ssl_app_group_id_lookup(f, (const char*)APPID_UT_TLS_HOST, (const char*)APPID_UT_TLS_HOST, + (const char*)APPID_UT_TLS_HOST, (const char*)APPID_UT_TLS_HOST, false, service, client, payload); + CHECK_TRUE(val); + CHECK_EQUAL(client, APPID_UT_ID + 1); + CHECK_EQUAL(payload, APPID_UT_ID + 1); + + // 2. First alt name match + client = APP_ID_NONE; + payload = APP_ID_NONE; + val = appid_api.ssl_app_group_id_lookup(f, nullptr, (const char*)APPID_UT_TLS_HOST, + (const char*)APPID_UT_TLS_HOST, (const char*)APPID_UT_TLS_HOST, false, service, client, payload); + CHECK_TRUE(val); + CHECK_EQUAL(client, APPID_UT_ID + 1); + CHECK_EQUAL(payload, APPID_UT_ID + 1); + + // 3. CN match + client = APP_ID_NONE; + payload = APP_ID_NONE; + val = appid_api.ssl_app_group_id_lookup(f, nullptr, nullptr, (const char*)APPID_UT_TLS_HOST, + (const char*)APPID_UT_TLS_HOST, false, service, client, payload); + CHECK_TRUE(val); + CHECK_EQUAL(client, APPID_UT_ID + 2); + CHECK_EQUAL(payload, APPID_UT_ID + 2); + + // 4. Org unit match + client = APP_ID_NONE; + payload = APP_ID_NONE; + val = appid_api.ssl_app_group_id_lookup(f, nullptr, nullptr, nullptr, (const char*)APPID_UT_TLS_HOST, + false, service, client, payload); + CHECK_TRUE(val); + CHECK_EQUAL(client, APPID_UT_ID + 2); + CHECK_EQUAL(payload, APPID_UT_ID + 2); + + delete f; } TEST(appid_api, is_inspection_needed)