Merge pull request #2636 in SNORT/snort3 from ~SATHIRKA/snort3:reload_tp_fix to master

Squashed commit of the following:

commit 01f04c1fc93fe47a35064224fb695d1dbf8d54b1
Author: Sreeja Athirkandathil Narayanan <sathirka@cisco.com>
Date:   Fri Nov 20 10:52:28 2020 -0500

    appid: Mark reload third-party complete after unloading old library and creating new third-party context

diff --git a/src/network_inspectors/appid/appid_discovery.cc b/src/network_inspectors/appid/appid_discovery.cc
index 56549e82b0ead2edb625aef6257610c6676c9d14..b6c4caef0ebd452fdc56537957cd743b7c982da8 100644 (file)
--- a/src/network_inspectors/appid/appid_discovery.cc
+++ b/src/network_inspectors/appid/appid_discovery.cc
@@ -761,7 +761,7 @@ bool AppIdDiscovery::do_discovery(Packet* p, AppIdSession& asd, IpProtocol proto
             else
                 appid_stats.tp_reload_ignored_pkts++;
         }
-        else if (!tp_appid_ctxt->get_tp_reload_in_progress())
+        else if (!ThirdPartyAppIdContext::get_tp_reload_in_progress())
             is_discovery_done = do_tp_discovery(*tp_appid_ctxt, asd, protocol, p,
                 direction, change_bits);
     }

diff --git a/src/network_inspectors/appid/appid_http_event_handler.cc b/src/network_inspectors/appid/appid_http_event_handler.cc
index 9cfbced434a0c0d8d28fd12e97ac34acbd93614e..874190e2249146c672c7a3231af6f9394525ee8c 100644 (file)
--- a/src/network_inspectors/appid/appid_http_event_handler.cc
+++ b/src/network_inspectors/appid/appid_http_event_handler.cc
@@ -61,7 +61,8 @@ void HttpEventHandler::handle(DataEvent& event, Flow* flow)
     HttpEvent* http_event = (HttpEvent*)&event;
     AppidChangeBits change_bits;
 
-    if (asd->get_tp_appid_ctxt() && !http_event->get_is_http2())
+    if ((asd->get_tp_appid_ctxt() or ThirdPartyAppIdContext::get_tp_reload_in_progress()) and
+        !http_event->get_is_http2())
         return;
 
     if (appidDebug->is_active())

diff --git a/src/network_inspectors/appid/appid_module.cc b/src/network_inspectors/appid/appid_module.cc
index b551686db372f04ce93b64c862ced317c77c7bc0..fa52e059259a425290c919124358133079fcb69a 100644 (file)
--- a/src/network_inspectors/appid/appid_module.cc
+++ b/src/network_inspectors/appid/appid_module.cc
@@ -131,20 +131,15 @@ class ACThirdPartyAppIdContextSwap : public AnalyzerCommand
 {
 public:
     bool execute(Analyzer&, void**) override;
-    ACThirdPartyAppIdContextSwap(const AppIdInspector& inspector,
-        Request& current_request, bool from_shell): inspector(inspector),
-        request(current_request), from_shell(from_shell)
+    ACThirdPartyAppIdContextSwap(const AppIdInspector& inspector): inspector(inspector)
     {
         LogMessage("== swapping third-party configuration\n");
-        request.respond("== swapping third-party configuration\n", from_shell, true);
     }
 
     ~ACThirdPartyAppIdContextSwap() override;
     const char* stringify() override { return "THIRD-PARTY_CONTEXT_SWAP"; }
 private:
     const AppIdInspector& inspector;
-    Request& request;
-    bool from_shell;
 };
 
 bool ACThirdPartyAppIdContextSwap::execute(Analyzer&, void**)
@@ -152,16 +147,14 @@ bool ACThirdPartyAppIdContextSwap::execute(Analyzer&, void**)
     assert(!pkt_thread_tp_appid_ctxt);
     pkt_thread_tp_appid_ctxt = inspector.get_ctxt().get_tp_appid_ctxt();
     pkt_thread_tp_appid_ctxt->tinit();
-    pkt_thread_tp_appid_ctxt->set_tp_reload_in_progress(false);
+    ThirdPartyAppIdContext::set_tp_reload_in_progress(false);
 
     return true;
 }
 
 ACThirdPartyAppIdContextSwap::~ACThirdPartyAppIdContextSwap()
 {
-    Swapper::set_reload_in_progress(false);
-    LogMessage("== reload third-party complete\n");
-    request.respond("== reload third-party complete\n", from_shell, true);
+    LogMessage("== third-party configuration swap complete\n");
 }
 
 class ACThirdPartyAppIdContextUnload : public AnalyzerCommand
@@ -183,7 +176,7 @@ private:
 bool ACThirdPartyAppIdContextUnload::execute(Analyzer& ac, void**)
 {
     assert(pkt_thread_tp_appid_ctxt);
-    pkt_thread_tp_appid_ctxt->set_tp_reload_in_progress(true);
+    ThirdPartyAppIdContext::set_tp_reload_in_progress(true);
     bool reload_in_progress;
     if (ac.is_idling())
         reload_in_progress = pkt_thread_tp_appid_ctxt->tfini(true, true);
@@ -201,8 +194,10 @@ ACThirdPartyAppIdContextUnload::~ACThirdPartyAppIdContextUnload()
     delete tp_ctxt;
     AppIdContext& ctxt = inspector.get_ctxt();
     ctxt.create_tp_appid_ctxt();
-    main_broadcast_command(new ACThirdPartyAppIdContextSwap(inspector,
-        request, from_shell), from_shell);
+    main_broadcast_command(new ACThirdPartyAppIdContextSwap(inspector));
+    LogMessage("== reload third-party complete\n");
+    request.respond("== reload third-party complete\n", from_shell, true);
+    Swapper::set_reload_in_progress(false);
 }
 
 class ACOdpContextSwap : public AnalyzerCommand

diff --git a/src/network_inspectors/appid/appid_session.cc b/src/network_inspectors/appid/appid_session.cc
index 2814c1a66f3aed2be2620caaad799ceef6aa61c5..ec6196f1e4f67a8936e6fee749e5e4b30f1fe7ea 100644 (file)
--- a/src/network_inspectors/appid/appid_session.cc
+++ b/src/network_inspectors/appid/appid_session.cc
@@ -99,7 +99,7 @@ AppIdSession::AppIdSession(IpProtocol proto, const SfIp* ip, uint16_t port,
         initiator_port(port), asid(asid), protocol(proto),
         api(*(new AppIdSessionApi(this, *ip))), odp_ctxt(odp_ctxt),
         odp_ctxt_version(odp_ctxt.get_version()),
-        tp_appid_ctxt(inspector.get_ctxt().get_tp_appid_ctxt())
+        tp_appid_ctxt(pkt_thread_tp_appid_ctxt)
 {
     service_ip.clear();
 
@@ -262,7 +262,7 @@ void AppIdSession::initialize_future_session(AppIdSession& expected, uint64_t fl
 }
 
 void AppIdSession::reinit_session_data(AppidChangeBits& change_bits,
-    ThirdPartyAppIdContext* tp_appid_ctxt)
+    ThirdPartyAppIdContext* curr_tp_appid_ctxt)
 {
     misc_app_id = APP_ID_NONE;
 
@@ -296,8 +296,8 @@ void AppIdSession::reinit_session_data(AppidChangeBits& change_bits,
     free_flow_data_by_mask(APPID_SESSION_DATA_CLIENT_MODSTATE_BIT);
 
     //3rd party cleaning
-    if (tpsession and tp_appid_ctxt and
-        (tpsession->get_ctxt_version() == tp_appid_ctxt->get_version()))
+    if (tpsession and curr_tp_appid_ctxt and
+        (tpsession->get_ctxt_version() == curr_tp_appid_ctxt->get_version()))
         tpsession->reset();
     else if (tpsession)
         tpsession->set_state(TP_STATE_TERMINATED);
@@ -359,7 +359,7 @@ void AppIdSession::sync_with_snort_protocol_id(AppId newAppId, Packet* p)
 }
 
 void AppIdSession::check_ssl_detection_restart(AppidChangeBits& change_bits,
-    ThirdPartyAppIdContext* tp_appid_ctxt)
+    ThirdPartyAppIdContext* curr_tp_appid_ctxt)
 {
     if (get_session_flags(APPID_SESSION_DECRYPTED) or !flow->is_proxied())
         return;
@@ -392,7 +392,7 @@ void AppIdSession::check_ssl_detection_restart(AppidChangeBits& change_bits,
         if (encrypted.payload_id > APP_ID_NONE)
             api.payload.set_overwritten_id(encrypted.payload_id);
 
-        reinit_session_data(change_bits, tp_appid_ctxt);
+        reinit_session_data(change_bits, curr_tp_appid_ctxt);
         if (appidDebug->is_active())
             LogMessage("AppIdDbg %s SSL decryption is available, restarting app detection\n",
                 appidDebug->get_debug_session());
@@ -444,9 +444,9 @@ void AppIdSession::check_tunnel_detection_restart()
 }
 
 void AppIdSession::check_app_detection_restart(AppidChangeBits& change_bits,
-    ThirdPartyAppIdContext* tp_appid_ctxt)
+    ThirdPartyAppIdContext* curr_tp_appid_ctxt)
 {
-    check_ssl_detection_restart(change_bits, tp_appid_ctxt);
+    check_ssl_detection_restart(change_bits, curr_tp_appid_ctxt);
     check_tunnel_detection_restart();
 }
 
@@ -991,20 +991,15 @@ AppIdDnsSession* AppIdSession::get_dns_session() const
 
 bool AppIdSession::is_tp_appid_done() const
 {
-    if (tp_appid_ctxt)
-    {
-        if (get_session_flags(APPID_SESSION_FUTURE_FLOW))
-            return true;
-
-        if (!tpsession)
-            return false;
+    if (get_session_flags(APPID_SESSION_FUTURE_FLOW) or !tp_appid_ctxt)
+        return true;
 
-        unsigned state = tpsession->get_state();
-        return (state == TP_STATE_CLASSIFIED || state == TP_STATE_TERMINATED ||
-            state == TP_STATE_HA);
-    }
+    if (!tpsession)
+        return false;
 
-    return true;
+    unsigned state = tpsession->get_state();
+    return (state == TP_STATE_CLASSIFIED || state == TP_STATE_TERMINATED ||
+        state == TP_STATE_HA);
 }
 
 bool AppIdSession::is_tp_processing_done() const

diff --git a/src/network_inspectors/appid/test/appid_http_event_test.cc b/src/network_inspectors/appid/test/appid_http_event_test.cc
index 71f0c52f6c11eea573e2060e1dcb799677d418b5..f0ad62ff79cb9e5c90ad42bf612c7afe272c70f8 100644 (file)
--- a/src/network_inspectors/appid/test/appid_http_event_test.cc
+++ b/src/network_inspectors/appid/test/appid_http_event_test.cc
@@ -42,6 +42,7 @@
 
 THREAD_LOCAL AppIdDebug* appidDebug = nullptr;
 ThirdPartyAppIdContext* AppIdContext::tp_appid_ctxt = nullptr;
+THREAD_LOCAL bool ThirdPartyAppIdContext::tp_reload_in_progress = false;
 void AppIdDebug::activate(const Flow*, const AppIdSession*, bool) { active = true; }
 void ApplicationDescriptor::set_id(const Packet&, AppIdSession&, AppidSessionDirection, AppId, AppidChangeBits&) { }
 

diff --git a/src/network_inspectors/appid/tp_appid_module_api.h b/src/network_inspectors/appid/tp_appid_module_api.h
index 8c4ede4dafe50ca8fdb8f56524471bf3e2d590fa..d9cd46bc74218c8b68e06bcb5ef09c625b8e1297 100644 (file)
--- a/src/network_inspectors/appid/tp_appid_module_api.h
+++ b/src/network_inspectors/appid/tp_appid_module_api.h
@@ -64,8 +64,8 @@ public:
 
     virtual const ThirdPartyConfig& get_config() const { return cfg; }
 
-    void set_tp_reload_in_progress(bool value) { tp_reload_in_progress = value; }
-    bool get_tp_reload_in_progress() { return tp_reload_in_progress; }
+    static void set_tp_reload_in_progress(bool value) { tp_reload_in_progress = value; }
+    static bool get_tp_reload_in_progress() { return tp_reload_in_progress; }
 
 protected:
     const uint32_t api_version;