Merge pull request #2617 in SNORT/snort3 from ~SHRARANG/snort3:appid_http_fixes to...

Squashed commit of the following:

commit b7ab85456eef818f937b46a2451a2de19c1961cc
Author: Shravan Rangaraju <shrarang@cisco.com>
Date:   Fri Nov 13 16:35:59 2020 -0500

    appid: do not override http fields with empty values

commit adcccb07de640c0298b5cf4c89da19fe36d6a436
Author: Shravan Rangaraju <shrarang@cisco.com>
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 5d348016749ddba0a21a311efb64e0be97febe18..5607cc9886cb1beaa1c85aea236c03cde44c4dfa 100644 (file)
--- 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 4898d44e7fda1ca44112850a2d9bf394db0e5470..bf3593d14e32b9b8ee414355662fef1fb1b034af 100644 (file)
--- 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 bfef312cf74f5997b13de73cb60b4481ef05a6b1..796680ae7807bdc3da8343be6c3df3030111f258 100644 (file)
--- 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 5d9bab02266dac139810d00072b0c79e531d8229..23708855b7e301a9d0bdf03e8bca3ab38ecc0ecc 100644 (file)
--- 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;