From: Shravan Rangarajuvenkata (shrarang) Date: Thu, 19 Nov 2020 16:29:06 +0000 (+0000) Subject: Merge pull request #2617 in SNORT/snort3 from ~SHRARANG/snort3:appid_http_fixes to... X-Git-Tag: 3.0.3-6~43 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f997e954bb002a201e3b81b14cc97ddf46ba00a3;p=thirdparty%2Fsnort3.git Merge pull request #2617 in SNORT/snort3 from ~SHRARANG/snort3:appid_http_fixes to master Squashed commit of the following: commit b7ab85456eef818f937b46a2451a2de19c1961cc Author: Shravan Rangaraju Date: Fri Nov 13 16:35:59 2020 -0500 appid: do not override http fields with empty values commit adcccb07de640c0298b5cf4c89da19fe36d6a436 Author: Shravan Rangaraju Date: Fri Nov 13 16:35:29 2020 -0500 appid: for http2 flow, return service id as http2 when no streams are yet created --- diff --git a/src/network_inspectors/appid/appid_http_session.cc b/src/network_inspectors/appid/appid_http_session.cc index 5d3480167..5607cc988 100644 --- a/src/network_inspectors/appid/appid_http_session.cc +++ b/src/network_inspectors/appid/appid_http_session.cc @@ -754,31 +754,31 @@ void AppIdHttpSession::clear_all_fields() void AppIdHttpSession::set_field(HttpFieldIds id, const std::string* str, AppidChangeBits& change_bits) { - delete meta_data[id]; - meta_data[id] = str; - if (str) + if (str and !str->empty()) { + delete meta_data[id]; + meta_data[id] = str; set_http_change_bits(change_bits, id); if (appidDebug->is_active()) print_field(id, str); } + else if (str) + delete str; } void AppIdHttpSession::set_field(HttpFieldIds id, const uint8_t* str, int32_t len, AppidChangeBits& change_bits) { - delete meta_data[id]; if (str and len) { + delete meta_data[id]; meta_data[id] = new std::string((const char*)str, len); set_http_change_bits(change_bits, id); if (appidDebug->is_active()) print_field(id, meta_data[id]); } - else - meta_data[id] = nullptr; } void AppIdHttpSession::print_field(HttpFieldIds id, const std::string* field) diff --git a/src/network_inspectors/appid/appid_session_api.cc b/src/network_inspectors/appid/appid_session_api.cc index 4898d44e7..bf3593d14 100644 --- a/src/network_inspectors/appid/appid_session_api.cc +++ b/src/network_inspectors/appid/appid_session_api.cc @@ -129,7 +129,10 @@ void AppIdSessionApi::get_app_id(AppId& service, AppId& client, if (get_service_app_id() == APP_ID_HTTP2) { if ((stream_index != 0) and (stream_index >= get_hsessions_size())) + { service = client = payload = misc = referred = APP_ID_UNKNOWN; + return; + } else if (AppIdHttpSession* hsession = get_hsession(stream_index)) { service = get_service_app_id(); @@ -137,13 +140,12 @@ void AppIdSessionApi::get_app_id(AppId& service, AppId& client, payload = hsession->payload.get_id(); misc = hsession->misc_app_id; referred = hsession->referred_payload_app_id; + return; } } - else - { - get_first_stream_app_ids(service, client, payload, misc); - referred = get_referred_app_id(); - } + + get_first_stream_app_ids(service, client, payload, misc); + referred = get_referred_app_id(); } void AppIdSessionApi::get_app_id(AppId* service, AppId* client, diff --git a/src/network_inspectors/appid/test/appid_http_session_test.cc b/src/network_inspectors/appid/test/appid_http_session_test.cc index bfef312cf..796680ae7 100644 --- a/src/network_inspectors/appid/test/appid_http_session_test.cc +++ b/src/network_inspectors/appid/test/appid_http_session_test.cc @@ -252,6 +252,20 @@ TEST(appid_http_session, http_field_ids_enum_order) CHECK_EQUAL(change_bits.test(APPID_USERAGENT_BIT), true); CHECK_EQUAL(change_bits.test(APPID_RESPONSE_BIT), true); CHECK_EQUAL(change_bits.test(APPID_REFERER_BIT), true); + + // verify empty fields do not override existing fields + mock_hsession->set_field(REQ_AGENT_FID, nullptr, change_bits); + field = mock_hsession->get_field(REQ_AGENT_FID); + STRCMP_EQUAL(field->c_str(), "agent"); + mock_hsession->set_field(REQ_AGENT_FID, new std::string(""), change_bits); + field = mock_hsession->get_field(REQ_AGENT_FID); + STRCMP_EQUAL(field->c_str(), "agent"); + mock_hsession->set_field(REQ_AGENT_FID, nullptr, 0, change_bits); + field = mock_hsession->get_field(REQ_AGENT_FID); + STRCMP_EQUAL(field->c_str(), "agent"); + mock_hsession->set_field(REQ_AGENT_FID, (const uint8_t*)"", 0, change_bits); + field = mock_hsession->get_field(REQ_AGENT_FID); + STRCMP_EQUAL(field->c_str(), "agent"); } TEST(appid_http_session, set_tun_dest) 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 5d9bab022..23708855b 100644 --- a/src/network_inspectors/appid/test/appid_session_api_test.cc +++ b/src/network_inspectors/appid/test/appid_session_api_test.cc @@ -127,6 +127,34 @@ TEST(appid_session_api, get_referred_app_id) CHECK_EQUAL(APP_ID_NONE, id); } +TEST(appid_session_api, get_app_id) +{ + SfIp ip; + AppIdSession asd(IpProtocol::TCP, &ip, 1492, dummy_appid_inspector, odpctxt); + AppidChangeBits change_bits; + asd.set_application_ids_service(APP_ID_HTTP2, change_bits); + + AppId service, client, payload, misc, referred; + asd.get_api().get_app_id(service, client, payload, misc, referred, 0); + + CHECK_EQUAL(service, APP_ID_HTTP2); + CHECK_EQUAL(client, APP_ID_NONE); + CHECK_EQUAL(payload, APP_ID_NONE); + CHECK_EQUAL(misc, APP_ID_NONE); + CHECK_EQUAL(referred, APP_ID_NONE); + + service = client = payload = misc = referred = APPID_UT_ID; + asd.get_api().get_app_id(&service, &client, &payload, &misc, &referred, 0); + + CHECK_EQUAL(service, APP_ID_HTTP2); + CHECK_EQUAL(client, APP_ID_NONE); + CHECK_EQUAL(payload, APP_ID_NONE); + CHECK_EQUAL(misc, APP_ID_NONE); + CHECK_EQUAL(referred, APP_ID_NONE); + + delete &asd.get_api(); +} + TEST(appid_session_api, get_tls_host) { AppidChangeBits change_bits;