From: Shravan Rangarajuvenkata (shrarang) <shrarang@cisco.com>
Date: Fri, 3 Jan 2020 14:46:22 +0000 (+0000)
Subject: Merge pull request #1901 in SNORT/snort3 from ~SHRARANG/snort3:appid_tp_reload_1_thre... 
X-Git-Tag: 3.0.0-268~68
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=06929a82c8cb24bb2dec9c1f16402ace2038502f;p=thirdparty%2Fsnort3.git

Merge pull request #1901 in SNORT/snort3 from ~SHRARANG/snort3:appid_tp_reload_1_thread to master

Squashed commit of the following:

commit cf334063cb9963471a4d3b87267f0e3e72966613
Author: Shravan Rangaraju <shrarang@cisco.com>
Date:   Tue Dec 17 06:27:06 2019 -0500

    appid: support third party reload when snort is running with single packet thread
---

diff --git a/src/network_inspectors/appid/appid_config.cc b/src/network_inspectors/appid/appid_config.cc
index 98ad1d64c..e9eeb9ded 100644
--- a/src/network_inspectors/appid/appid_config.cc
+++ b/src/network_inspectors/appid/appid_config.cc
@@ -265,12 +265,21 @@ bool AppIdConfig::init_appid(SnortConfig* sc)
     }
 
 #ifdef ENABLE_APPID_THIRD_PARTY
-    TPLibHandler::pinit(mod_config);
+    // do not reload third party on reload_config()
+    if (!tp_appid_ctxt)
+        tp_appid_ctxt = TPLibHandler::create_tp_appid_ctxt(*mod_config);
 #endif
     map_app_names_to_snort_ids(sc);
     return true;
 }
 
+#ifdef ENABLE_APPID_THIRD_PARTY
+void AppIdConfig::create_tp_appid_ctxt()
+{
+    tp_appid_ctxt = TPLibHandler::create_tp_appid_ctxt(*mod_config);
+}
+#endif
+
 AppId AppIdConfig::get_port_service_id(IpProtocol proto, uint16_t port)
 {
     AppId appId;
diff --git a/src/network_inspectors/appid/appid_config.h b/src/network_inspectors/appid/appid_config.h
index e6f2a52eb..a1b219198 100644
--- a/src/network_inspectors/appid/appid_config.h
+++ b/src/network_inspectors/appid/appid_config.h
@@ -32,6 +32,9 @@
 #include "sfip/sf_ip.h"
 #include "target_based/snort_protocols.h"
 #include "utils/sflsq.h"
+#ifdef ENABLE_APPID_THIRD_PARTY
+#include "tp_appid_module_api.h"
+#endif
 
 #define APP_ID_MAX_DIRS         16
 #define APP_ID_PORT_ARRAY_SIZE  65536
@@ -113,6 +116,20 @@ public:
     AppIdConfig(AppIdModuleConfig* config) : mod_config(config)
     { }
 
+    ~AppIdConfig()
+    {
+#ifdef ENABLE_APPID_THIRD_PARTY
+        delete tp_appid_ctxt;
+#endif
+    }
+
+#ifdef ENABLE_APPID_THIRD_PARTY
+    ThirdPartyAppIDModule* get_tp_appid_ctxt() const
+    { return tp_appid_ctxt; }
+
+    void create_tp_appid_ctxt();
+#endif
+
     bool init_appid(snort::SnortConfig*);
     static void pterm();
     void show();
@@ -137,6 +154,9 @@ private:
     // FIXIT-M: RELOAD - Remove static, once app_info_mgr cleanup is
     // removed from AppIdConfig::pterm
     static AppInfoManager& app_info_mgr;
+#ifdef ENABLE_APPID_THIRD_PARTY
+    ThirdPartyAppIDModule* tp_appid_ctxt = nullptr;
+#endif
 };
 
 #endif
diff --git a/src/network_inspectors/appid/appid_discovery.cc b/src/network_inspectors/appid/appid_discovery.cc
index a13dd5921..b023d979e 100644
--- a/src/network_inspectors/appid/appid_discovery.cc
+++ b/src/network_inspectors/appid/appid_discovery.cc
@@ -136,7 +136,12 @@ int AppIdDiscovery::add_service_port(AppIdDetector*, const ServiceDetectorPort&)
     return APPID_EINVALID;
 }
 
-void AppIdDiscovery::do_application_discovery(Packet* p, AppIdInspector& inspector)
+#ifdef ENABLE_APPID_THIRD_PARTY
+void AppIdDiscovery::do_application_discovery(Packet* p, AppIdInspector& inspector,
+    ThirdPartyAppIDModule* tp_appid_ctxt)
+#else
+  void AppIdDiscovery::do_application_discovery(Packet* p, AppIdInspector& inspector)
+#endif
 {
     IpProtocol protocol = IpProtocol::PROTO_NOT_SET;
     AppidSessionDirection direction = APP_ID_FROM_INITIATOR;
@@ -150,8 +155,13 @@ void AppIdDiscovery::do_application_discovery(Packet* p, AppIdInspector& inspect
     AppId payload_id = APP_ID_NONE;
     AppId misc_id = APP_ID_NONE;
     AppidChangeBits change_bits;
-    bool is_discovery_done = do_discovery(p, *asd, protocol, direction, service_id, client_id,
-        payload_id, misc_id, change_bits);
+#ifdef ENABLE_APPID_THIRD_PARTY
+    bool is_discovery_done = do_discovery(p, *asd, protocol, direction, service_id,
+        client_id, payload_id, misc_id, change_bits, tp_appid_ctxt);
+#else
+    bool is_discovery_done = do_discovery(p, *asd, protocol, direction, service_id,
+        client_id, payload_id, misc_id, change_bits);
+#endif
 
     do_post_discovery(p, *asd, direction, is_discovery_done, service_id, client_id, payload_id,
         misc_id, change_bits);
@@ -826,9 +836,16 @@ static inline bool is_check_host_cache_valid(AppIdSession& asd, AppId service_id
     return false;
 }
 
-bool AppIdDiscovery::do_discovery(Packet* p, AppIdSession& asd, IpProtocol protocol,
-    AppidSessionDirection direction, AppId& service_id, AppId& client_id, AppId& payload_id,
-    AppId& misc_id, AppidChangeBits& change_bits)
+#ifdef ENABLE_APPID_THIRD_PARTY
+bool AppIdDiscovery::do_discovery(Packet* p, AppIdSession& asd,
+    IpProtocol protocol, AppidSessionDirection direction, AppId& service_id, AppId& client_id,
+    AppId& payload_id, AppId& misc_id, AppidChangeBits& change_bits,
+    ThirdPartyAppIDModule* tp_appid_ctxt)
+#else
+bool AppIdDiscovery::do_discovery(Packet* p, AppIdSession& asd,
+    IpProtocol protocol, AppidSessionDirection direction, AppId& service_id, AppId& client_id,
+    AppId& payload_id, AppId& misc_id, AppidChangeBits& change_bits)
+#endif
 {
     bool is_discovery_done = false;
 
@@ -860,8 +877,13 @@ bool AppIdDiscovery::do_discovery(Packet* p, AppIdSession& asd, IpProtocol proto
 
     // Third party detection
 #ifdef ENABLE_APPID_THIRD_PARTY
-    if ( TPLibHandler::have_tp() )
-        is_discovery_done = do_tp_discovery(asd, protocol, p, direction, change_bits);
+    if (tp_appid_ctxt)
+    {
+        // Skip third-party inspection for sessions using old config
+        if ((asd.tpsession and asd.tpsession->get_ctxt() == tp_appid_ctxt) || !asd.tpsession)
+            is_discovery_done = do_tp_discovery(*tp_appid_ctxt, asd, protocol, p,
+                direction, change_bits);
+    }
 #endif
 
     // Port-based service detection
diff --git a/src/network_inspectors/appid/appid_discovery.h b/src/network_inspectors/appid/appid_discovery.h
index b89b26771..409704f70 100644
--- a/src/network_inspectors/appid/appid_discovery.h
+++ b/src/network_inspectors/appid/appid_discovery.h
@@ -40,6 +40,7 @@ class AppIdSession;
 class AppIdDetector;
 class ServiceDetector;
 struct ServiceDetectorPort;
+class ThirdPartyAppIDModule;
 
 namespace snort
 {
@@ -111,7 +112,12 @@ public:
         int position, unsigned nocase);
     virtual int add_service_port(AppIdDetector*, const ServiceDetectorPort&);
 
+#ifdef ENABLE_APPID_THIRD_PARTY
+    static void do_application_discovery(snort::Packet* p, AppIdInspector&,
+        ThirdPartyAppIDModule*);
+#else
     static void do_application_discovery(snort::Packet* p, AppIdInspector&);
+#endif
     static void publish_appid_event(AppidChangeBits&, snort::Flow*);
 
     AppIdDetectors* get_tcp_detectors()
@@ -136,9 +142,16 @@ protected:
 private:
     static bool do_pre_discovery(snort::Packet* p, AppIdSession** p_asd, AppIdInspector& inspector,
         IpProtocol& protocol, AppidSessionDirection& direction);
-    static bool do_discovery(snort::Packet* p, AppIdSession& asd, IpProtocol protocol,
-        AppidSessionDirection direction, AppId& service_id, AppId& client_id, AppId& payload_id,
-        AppId& misc_id, AppidChangeBits& change_bits);
+#ifdef ENABLE_APPID_THIRD_PARTY
+    static bool do_discovery(snort::Packet* p, AppIdSession& asd,
+        IpProtocol protocol, AppidSessionDirection direction, AppId& service_id, AppId& client_id,
+        AppId& payload_id, AppId& misc_id, AppidChangeBits& change_bits,
+        ThirdPartyAppIDModule* tp_appid_ctxt);
+#else
+    static bool do_discovery(snort::Packet* p, AppIdSession& asd,
+        IpProtocol protocol, AppidSessionDirection direction, AppId& service_id, AppId& client_id,
+        AppId& payload_id, AppId& misc_id, AppidChangeBits& change_bits);
+#endif
     static void do_post_discovery(snort::Packet* p, AppIdSession& asd,
         AppidSessionDirection direction, bool is_discovery_done, AppId service_id, AppId client_id,
         AppId payload_id, AppId misc_id, AppidChangeBits& change_bits);
diff --git a/src/network_inspectors/appid/appid_inspector.cc b/src/network_inspectors/appid/appid_inspector.cc
index 18428ac41..b73ec9cbc 100644
--- a/src/network_inspectors/appid/appid_inspector.cc
+++ b/src/network_inspectors/appid/appid_inspector.cc
@@ -50,10 +50,14 @@
 #include "service_plugins/service_discovery.h"
 #include "service_plugins/service_ssl.h"
 #ifdef ENABLE_APPID_THIRD_PARTY
+#include "tp_appid_module_api.h"
 #include "tp_lib_handler.h"
 #endif
 
 using namespace snort;
+#ifdef ENABLE_APPID_THIRD_PARTY
+THREAD_LOCAL ThirdPartyAppIDModule* tp_appid_thread_ctxt = nullptr;
+#endif
 static THREAD_LOCAL PacketTracer::TracerMute appid_mute;
 
 // FIXIT-L - appid cleans up openssl now as it is the primary (only) user... eventually this
@@ -116,7 +120,7 @@ bool AppIdInspector::configure(SnortConfig* sc)
     active_config->init_appid(sc);
 
 #ifdef ENABLE_APPID_THIRD_PARTY
-    if (!TPLibHandler::have_tp())
+    if (!active_config->get_tp_appid_ctxt())
 #endif
     {
         DataBus::subscribe_global(HTTP_REQUEST_HEADER_EVENT_KEY, new HttpEventHandler(
@@ -155,9 +159,6 @@ void AppIdInspector::tinit()
     appidDebug = new AppIdDebug();
     if (active_config->mod_config and active_config->mod_config->log_all_sessions)
         appidDebug->set_enabled(true);
-#ifdef ENABLE_APPID_THIRD_PARTY
-    TPLibHandler::tinit();
-#endif
 }
 
 void AppIdInspector::tterm()
@@ -170,7 +171,9 @@ void AppIdInspector::tterm()
     delete appidDebug;
     appidDebug = nullptr;
 #ifdef ENABLE_APPID_THIRD_PARTY
-    TPLibHandler::tterm();
+    ThirdPartyAppIDModule* tp_appid_ctxt = active_config->get_tp_appid_ctxt();
+    if (tp_appid_ctxt)
+        tp_appid_ctxt->tfini();
 #endif
 }
 
@@ -179,9 +182,29 @@ void AppIdInspector::eval(Packet* p)
     Profile profile(appid_perf_stats);
     appid_stats.packets++;
 
+#ifdef ENABLE_APPID_THIRD_PARTY
+    ThirdPartyAppIDModule* tp_appid_ctxt = active_config->get_tp_appid_ctxt();
+    if (tp_appid_thread_ctxt != tp_appid_ctxt)
+    {
+        if (tp_appid_thread_ctxt)
+        {
+            tp_appid_thread_ctxt->tfini();
+
+            // FIXIT-H: Assuming one packet thread
+            delete tp_appid_thread_ctxt;
+        }
+        tp_appid_ctxt->tinit();
+        tp_appid_thread_ctxt = tp_appid_ctxt;
+    }
+#endif
+
     if (p->flow)
     {
+#ifdef ENABLE_APPID_THIRD_PARTY
+        AppIdDiscovery::do_application_discovery(p, *this, tp_appid_thread_ctxt);
+#else
         AppIdDiscovery::do_application_discovery(p, *this);
+#endif
         // FIXIT-L tag verdict reason as appid for daq
         if (PacketTracer::is_active())
             add_appid_to_packet_trace(*p->flow);
@@ -238,6 +261,9 @@ static void appid_inspector_tinit()
 
 static void appid_inspector_tterm()
 {
+#ifdef ENABLE_APPID_THIRD_PARTY
+    TPLibHandler::tfini();
+#endif
     AppIdPegCounts::cleanup_pegs();
 }
 
diff --git a/src/network_inspectors/appid/appid_inspector.h b/src/network_inspectors/appid/appid_inspector.h
index ab1b4704c..6c582eafb 100644
--- a/src/network_inspectors/appid/appid_inspector.h
+++ b/src/network_inspectors/appid/appid_inspector.h
@@ -60,5 +60,9 @@ private:
 
 };
 
+#ifdef ENABLE_APPID_THIRD_PARTY
+extern THREAD_LOCAL ThirdPartyAppIDModule* tp_appid_thread_ctxt;
+#endif
+
 #endif
 
diff --git a/src/network_inspectors/appid/appid_module.cc b/src/network_inspectors/appid/appid_module.cc
index 99f361c5b..fec0ae9bc 100644
--- a/src/network_inspectors/appid/appid_module.cc
+++ b/src/network_inspectors/appid/appid_module.cc
@@ -33,11 +33,13 @@
 #include "main/snort.h"
 #include "main/swapper.h"
 #include "main/thread_config.h"
+#include "managers/inspector_manager.h"
 #include "profiler/profiler.h"
 #include "utils/util.h"
 
 #include "app_info_table.h"
 #include "appid_debug.h"
+#include "appid_inspector.h"
 #include "appid_peg_counts.h"
 #include "service_state.h"
 
@@ -170,6 +172,7 @@ static int disable_debug(lua_State*)
 
 static int reload_third_party(lua_State*)
 {
+#ifdef ENABLE_APPID_THIRD_PARTY
     if (Swapper::get_reload_in_progress())
     {
         LogMessage("== reload pending; retry\n");
@@ -182,8 +185,14 @@ static int reload_third_party(lua_State*)
     {
         Swapper::set_reload_in_progress(true);
         LogMessage(".. reloading third-party");
+        AppIdInspector* inspector = (AppIdInspector*) InspectorManager::get_inspector(MOD_NAME, true);
+        AppIdConfig* config = inspector->get_appid_config();
+        config->create_tp_appid_ctxt();
         Swapper::set_reload_in_progress(false);
     }
+#else
+    LogMessage("== third party is not enabled\n");
+#endif
 
     return 0;
 }
diff --git a/src/network_inspectors/appid/appid_session.cc b/src/network_inspectors/appid/appid_session.cc
index 868f8e4c4..77c531302 100644
--- a/src/network_inspectors/appid/appid_session.cc
+++ b/src/network_inspectors/appid/appid_session.cc
@@ -140,8 +140,10 @@ AppIdSession::~AppIdSession()
 #ifdef ENABLE_APPID_THIRD_PARTY
     if (tpsession)
     {
-        delete tpsession;
-        tpsession = nullptr;
+        if (tpsession->get_ctxt() == tp_appid_thread_ctxt)
+            tpsession->delete_with_ctxt();
+        else
+            delete tpsession;
     }
 #endif
 
@@ -926,7 +928,7 @@ AppIdDnsSession* AppIdSession::get_dns_session()
 bool AppIdSession::is_tp_appid_done() const
 {
 #ifdef ENABLE_APPID_THIRD_PARTY
-    if (TPLibHandler::have_tp())
+    if (config->get_tp_appid_ctxt())
     {
         if (!tpsession)
             return false;
@@ -955,7 +957,7 @@ bool AppIdSession::is_tp_processing_done() const
 bool AppIdSession::is_tp_appid_available() const
 {
 #ifdef ENABLE_APPID_THIRD_PARTY
-    if (TPLibHandler::have_tp())
+    if (config->get_tp_appid_ctxt())
     {
         if (!tpsession)
             return false;
diff --git a/src/network_inspectors/appid/http_xff_fields.h b/src/network_inspectors/appid/http_xff_fields.h
index 073dec622..ebd8012ad 100644
--- a/src/network_inspectors/appid/http_xff_fields.h
+++ b/src/network_inspectors/appid/http_xff_fields.h
@@ -27,9 +27,6 @@
 // FIXIT-L refactor
 #define HTTP_XFF_FIELD_X_FORWARDED_FOR  "X-Forwarded-For"
 #define HTTP_XFF_FIELD_TRUE_CLIENT_IP   "True-Client-IP"
-/* #define HTTP_MAX_XFF_FIELDS             8 */
-/* #define HTTP_XFF_FIELD_X_FORWARDED_FOR "" */
-/* #define HTTP_XFF_FIELD_TRUE_CLIENT_IP "" */
 
 #define HTTP_MAX_XFF_FIELDS 8
 
@@ -39,5 +36,4 @@ struct XffFieldValue
     std::string value;
 };
 
-
 #endif
diff --git a/src/network_inspectors/appid/test/appid_discovery_test.cc b/src/network_inspectors/appid/test/appid_discovery_test.cc
index 82769df2a..50050226c 100644
--- a/src/network_inspectors/appid/test/appid_discovery_test.cc
+++ b/src/network_inspectors/appid/test/appid_discovery_test.cc
@@ -256,7 +256,7 @@ int dns_host_scan_hostname(const uint8_t*, size_t, AppId*, AppId*)
 {
     return 0;
 }
-bool do_tp_discovery(AppIdSession&, IpProtocol,
+bool do_tp_discovery(ThirdPartyAppIDModule& , AppIdSession&, IpProtocol,
     Packet*, AppidSessionDirection&, AppidChangeBits&)
 {
     return true;
@@ -335,7 +335,11 @@ TEST(appid_discovery_tests, event_published_when_ignoring_flow)
     asd->common.initiator_ip.set("1.2.3.4");
     asd->set_session_flags(APPID_SESSION_IGNORE_FLOW);
 
+#ifdef ENABLE_APPID_THIRD_PARTY
+    AppIdDiscovery::do_application_discovery(&p, ins, nullptr);
+#else
     AppIdDiscovery::do_application_discovery(&p, ins);
+#endif
 
     // Detect changes in service, client, payload, and misc appid
     CHECK_EQUAL(databus_publish_called, true);
@@ -366,7 +370,11 @@ TEST(appid_discovery_tests, event_published_when_processing_flow)
     asd->common.initiator_port = 21;
     asd->common.initiator_ip.set("1.2.3.4");
 
+#ifdef ENABLE_APPID_THIRD_PARTY
+    AppIdDiscovery::do_application_discovery(&p, ins, nullptr);
+#else
     AppIdDiscovery::do_application_discovery(&p, ins);
+#endif
 
     // Detect changes in service, client, payload, and misc appid
     CHECK_EQUAL(databus_publish_called, true);
@@ -427,7 +435,11 @@ TEST(appid_discovery_tests, change_bits_for_non_http_appid)
     asd->client.set_id(APP_ID_CURL);
     asd->service.set_id(APP_ID_FTP);
 
+#ifdef ENABLE_APPID_THIRD_PARTY
+    AppIdDiscovery::do_application_discovery(&p, ins, nullptr);
+#else
     AppIdDiscovery::do_application_discovery(&p, ins);
+#endif
 
     // Detect event for FTP service and CURL client
     CHECK_EQUAL(databus_publish_called, true);
@@ -440,7 +452,11 @@ TEST(appid_discovery_tests, change_bits_for_non_http_appid)
     asd->payload.set_id(APP_ID_NONE);
     asd->client.set_id(APP_ID_NONE);
     asd->service.set_id(APP_ID_DNS);
+#ifdef ENABLE_APPID_THIRD_PARTY
+    AppIdDiscovery::do_application_discovery(&p, ins, nullptr);
+#else
     AppIdDiscovery::do_application_discovery(&p, ins);
+#endif
 
     // Detect event for DNS service
     CHECK_EQUAL(databus_publish_called, true);
diff --git a/src/network_inspectors/appid/test/tp_lib_handler_test.cc b/src/network_inspectors/appid/test/tp_lib_handler_test.cc
index ee4c51be8..877f8c227 100644
--- a/src/network_inspectors/appid/test/tp_lib_handler_test.cc
+++ b/src/network_inspectors/appid/test/tp_lib_handler_test.cc
@@ -52,24 +52,25 @@ TEST(tp_lib_handler, load_unload)
     config.tp_appid_config="./tp.config";
 
     tph = TPLibHandler::get();
-    tph->pinit(&config);
-    CHECK_TRUE(tph->have_tp());
+    ThirdPartyAppIDModule* tpm = TPLibHandler::create_tp_appid_ctxt(config);
+    CHECK_TRUE(tpm != nullptr);
 
-    CreateThirdPartyAppIDSession_t asf = tph->tpsession_factory();
-    ThirdPartyAppIDSession* tpsession = asf();
+    TpAppIdCreateSession asf = tph->tpsession_factory();
+    ThirdPartyAppIDSession* tpsession = asf(*tpm);
 
     CHECK_TRUE(tpsession != nullptr);
 
     delete tpsession;
+    delete tpm;
 
-    tph->pfini();
+    TPLibHandler::pfini();
 }
 
 TEST(tp_lib_handler, tp_lib_handler_get)
 {
-    tph=TPLibHandler::get();
-    TPLibHandler* tph2=TPLibHandler::get();
-    CHECK_EQUAL(tph,tph2);
+    tph = TPLibHandler::get();
+    TPLibHandler* tph2 = TPLibHandler::get();
+    CHECK_EQUAL(tph, tph2);
     TPLibHandler::pfini();
 }
 
@@ -79,23 +80,11 @@ TEST(tp_lib_handler, load_error)
     AppIdModuleConfig config;
     config.tp_appid_path="nonexistent.so";
     TPLibHandler::get();
-    TPLibHandler::pinit(&config);
-    CHECK_FALSE(TPLibHandler::have_tp());
+    ThirdPartyAppIDModule* tpm = TPLibHandler::create_tp_appid_ctxt(config);
+    CHECK_TRUE(tpm == nullptr);
     TPLibHandler::pfini();
 }
 
-TEST(tp_lib_handler, tp_appid_module_pinit_error)
-{
-    // Trigger MODULE pinit error:
-    AppIdModuleConfig config;
-    config.tp_appid_path="./libtp_mock.so";
-    config.tp_appid_config="";  // forces MODULE::pinit() to return 1.
-    tph=TPLibHandler::get();
-    tph->pinit(&config);
-    CHECK_FALSE(tph->have_tp());
-    tph->pfini(1);                      // print stats too.
-}
-
 int main(int argc, char** argv)
 {
     int rc = CommandLineTestRunner::RunAllTests(argc, argv);
diff --git a/src/network_inspectors/appid/test/tp_mock.cc b/src/network_inspectors/appid/test/tp_mock.cc
index 3eb1b3423..b21dbd64d 100644
--- a/src/network_inspectors/appid/test/tp_mock.cc
+++ b/src/network_inspectors/appid/test/tp_mock.cc
@@ -42,8 +42,8 @@ using namespace std;
 class ThirdPartyAppIDModuleImpl : public ThirdPartyAppIDModule
 {
 public:
-    ThirdPartyAppIDModuleImpl(uint32_t ver, const char* mname)
-        : ThirdPartyAppIDModule(ver, mname)
+    ThirdPartyAppIDModuleImpl(uint32_t ver, const char* mname, ThirdPartyConfig& config)
+        : ThirdPartyAppIDModule(ver, mname, config)
     {
         cerr << WhereMacro << endl;
     }
@@ -53,32 +53,18 @@ public:
         cerr << WhereMacro << endl;
     }
 
-    // Hack: use cfg to manipulate pinit to return 1, so we can hit the
-    // if (ret != 0) case in tp_lib_handler.cc.
-    int pinit(ThirdPartyConfig& cfg) override
-    {
-        cerr << WhereMacro << endl;
-        return cfg.tp_appid_config.empty() ? 1 : 0;
-    }
-
     int tinit() override { return 0; }
-    int reconfigure(const ThirdPartyConfig&) override { return 0; }
-    int pfini() override
-    {
-        cerr << WhereMacro << endl;
-        return 0;
-    }
-
     int tfini() override { return 0; }
-    int print_stats() override { return 0; }
-    int reset_stats() override { return 0; }
 };
 
 class ThirdPartyAppIDSessionImpl : public ThirdPartyAppIDSession
 {
 public:
-
+    ThirdPartyAppIDSessionImpl(ThirdPartyAppIDModule& ctxt)
+      : ThirdPartyAppIDSession(ctxt)
+    { }
     bool reset() override { return 1; }
+    void delete_with_ctxt() override { delete this; }
     TPState process(const Packet&, AppidSessionDirection, vector<AppId>&,
         ThirdPartyAppIDAttributeData&) override { return TP_STATE_INIT; }
 
@@ -98,17 +84,24 @@ private:
 // once the .so has been loaded.
 extern "C"
 {
-    SO_PUBLIC ThirdPartyAppIDModuleImpl* create_third_party_appid_module();
-    SO_PUBLIC ThirdPartyAppIDSessionImpl* create_third_party_appid_session();
+    SO_PUBLIC ThirdPartyAppIDModuleImpl* tp_appid_create_ctxt(ThirdPartyConfig& config)
+    {
+        return new ThirdPartyAppIDModuleImpl(2,"foobar", config);
+    }
+
+    SO_PUBLIC ThirdPartyAppIDSessionImpl* tp_appid_create_session(ThirdPartyAppIDModule& ctxt)
+    {
+        return new ThirdPartyAppIDSessionImpl(ctxt);
+    }
 
-    SO_PUBLIC ThirdPartyAppIDModuleImpl* create_third_party_appid_module()
+    SO_PUBLIC int tp_appid_pfini()
     {
-        return new ThirdPartyAppIDModuleImpl(1,"foobar");
+        return 0;
     }
 
-    SO_PUBLIC ThirdPartyAppIDSessionImpl* create_third_party_appid_session()
+    SO_PUBLIC int tp_appid_tfini()
     {
-        return new ThirdPartyAppIDSessionImpl;
+        return 0;
     }
 }
 
diff --git a/src/network_inspectors/appid/tp_appid_module_api.h b/src/network_inspectors/appid/tp_appid_module_api.h
index dbc2ecc12..39bbee899 100644
--- a/src/network_inspectors/appid/tp_appid_module_api.h
+++ b/src/network_inspectors/appid/tp_appid_module_api.h
@@ -25,7 +25,7 @@
 #include <string>
 #include "tp_appid_types.h"
 
-#define THIRD_PARTY_APP_ID_API_VERSION 1
+#define THIRD_PARTY_APP_ID_API_VERSION 2
 
 class ThirdPartyConfig
 {
@@ -38,16 +38,10 @@ public:
     unsigned http_response_version_enabled : 1;
     std::string tp_appid_config;
     std::vector<std::string> xff_fields;
-    std::vector<std::string> old_xff_fields;
     bool tp_appid_stats_enable = false;
     bool tp_appid_config_dump = false;
 
     ThirdPartyConfig()
-    {
-        getXffFields();
-    }
-
-    void getXffFields()
     {
         xff_fields.clear();
         xff_fields.emplace_back(HTTP_XFF_FIELD_X_FORWARDED_FOR);
@@ -58,41 +52,28 @@ public:
 class ThirdPartyAppIDModule
 {
 public:
-
-    /* ThirdPartyAppIdConfig tpac; */
-
-    ThirdPartyAppIDModule(uint32_t ver, const char* mname)
-        : version(ver), name(mname) { }
+    ThirdPartyAppIDModule(uint32_t ver, const char* mname, ThirdPartyConfig& config)
+        : version(ver), name(mname), cfg(config) { }
 
     virtual ~ThirdPartyAppIDModule() { }
 
     uint32_t api_version() const { return version; }
     const std::string& module_name() const { return name; }
 
-    virtual int pinit(ThirdPartyConfig&) = 0;
-    virtual int pfini() = 0;
-
     virtual int tinit() = 0;
     virtual int tfini() = 0;
 
-    virtual int reconfigure(const ThirdPartyConfig&) = 0;
-    virtual int print_stats() = 0;
-    virtual int reset_stats() = 0;
+    virtual const ThirdPartyConfig& get_config() const { return cfg; }
 
-private:
+protected:
+    const uint32_t version;
+    const std::string name;
+    ThirdPartyConfig cfg;
 
+private:
     // No implicit constructor as derived classes need to provide
     // version and name.
     ThirdPartyAppIDModule() : version(0), name("") { }
-
-    const uint32_t version;
-    const std::string name;
 };
 
-// Function pointer to object factory that returns a pointer to a newly
-// created object of a derived class.
-// This needs to be exported (SO_PUBLIC) by any third party .so library.
-// Must return NULL if it fails to create the object.
-typedef ThirdPartyAppIDModule* (* CreateThirdPartyAppIDModule_t)();
-
 #endif
diff --git a/src/network_inspectors/appid/tp_appid_session_api.h b/src/network_inspectors/appid/tp_appid_session_api.h
index beed36430..1a904985b 100644
--- a/src/network_inspectors/appid/tp_appid_session_api.h
+++ b/src/network_inspectors/appid/tp_appid_session_api.h
@@ -32,17 +32,17 @@ namespace snort
 struct Packet;
 }
 
-#define THIRD_PARTY_APP_ID_API_VERSION 1
+class ThirdPartyAppIDModule;
 
 class ThirdPartyAppIDSession
 {
 public:
-
-    ThirdPartyAppIDSession()
-        : appid(APP_ID_NONE), confidence(100), state(TP_STATE_INIT) { }
+    ThirdPartyAppIDSession(ThirdPartyAppIDModule& ctxt)
+        : appid(APP_ID_NONE), confidence(100), state(TP_STATE_INIT), ctxt(ctxt) { }
     virtual ~ThirdPartyAppIDSession() { }
 
     virtual bool reset() = 0;            // just reset state
+    virtual void delete_with_ctxt() = 0;
     virtual TPState process(const snort::Packet&,
         AppidSessionDirection direction,
         std::vector<AppId>& proto_list,
@@ -55,18 +55,15 @@ public:
     virtual void set_attr(TPSessionAttr) = 0;
     virtual unsigned get_attr(TPSessionAttr) = 0;
     virtual AppId get_appid(int& conf) { conf=confidence; return appid; }
+    virtual const ThirdPartyAppIDModule* get_ctxt() const
+    { return &ctxt; }
 
 protected:
     AppId appid;
     int confidence;
     TPState state;
+    const ThirdPartyAppIDModule& ctxt;
 };
 
-// Function pointer to object factory that returns a pointer to a newly
-// created object of a derived class.
-// This needs to be exported (SO_PUBLIC) by any third party .so library.
-// Must return NULL if it fails to create the object.
-typedef ThirdPartyAppIDSession* (* CreateThirdPartyAppIDSession_t)();
-
 #endif
 
diff --git a/src/network_inspectors/appid/tp_appid_utils.cc b/src/network_inspectors/appid/tp_appid_utils.cc
index 9b1b9460f..fdd6556c0 100644
--- a/src/network_inspectors/appid/tp_appid_utils.cc
+++ b/src/network_inspectors/appid/tp_appid_utils.cc
@@ -621,7 +621,7 @@ static inline void check_terminate_tp_module(AppIdSession& asd, uint16_t tpPktCo
     }
 }
 
-bool do_tp_discovery(AppIdSession& asd, IpProtocol protocol,
+bool do_tp_discovery(ThirdPartyAppIDModule& tp_module, AppIdSession& asd, IpProtocol protocol,
     Packet* p, AppidSessionDirection& direction, AppidChangeBits& change_bits)
 {
     AppId tp_app_id = asd.get_tp_app_id();
@@ -668,10 +668,10 @@ bool do_tp_discovery(AppIdSession& asd, IpProtocol protocol,
                 if (!asd.tpsession)
                 {
                     const TPLibHandler* tph = TPLibHandler::get();
-                    CreateThirdPartyAppIDSession_t tpsf = tph->tpsession_factory();
-                    if ( !(asd.tpsession = tpsf()) )
-                        FatalError("Could not allocate asd.tpsession data");
-                }      // debug output of packet content
+                    TpAppIdCreateSession tpsf = tph->tpsession_factory();
+                    if ( !(asd.tpsession = tpsf(tp_module)) )
+                        ErrorMessage("Could not allocate asd.tpsession data");
+                }
 
                 TPState current_tp_state = asd.tpsession->process(*p, direction,
                     tp_proto_list, tp_attribute_data);
diff --git a/src/network_inspectors/appid/tp_appid_utils.h b/src/network_inspectors/appid/tp_appid_utils.h
index 05a9bb845..943cb3ff3 100644
--- a/src/network_inspectors/appid/tp_appid_utils.h
+++ b/src/network_inspectors/appid/tp_appid_utils.h
@@ -26,7 +26,7 @@
 
 class AppIdSession;
 
-bool do_tp_discovery(
-    AppIdSession&, IpProtocol, snort::Packet*, AppidSessionDirection&, AppidChangeBits&);
+bool do_tp_discovery(ThirdPartyAppIDModule& tp_module, AppIdSession&, IpProtocol, snort::Packet*,
+    AppidSessionDirection&, AppidChangeBits&);
 
 #endif
diff --git a/src/network_inspectors/appid/tp_lib_handler.cc b/src/network_inspectors/appid/tp_lib_handler.cc
index c3dcade53..50720522a 100644
--- a/src/network_inspectors/appid/tp_lib_handler.cc
+++ b/src/network_inspectors/appid/tp_lib_handler.cc
@@ -34,143 +34,131 @@
 using namespace std;
 using namespace snort;
 
-#define TP_APPID_MODULE_SYMBOL "create_third_party_appid_module"
-#define TP_APPID_SESSION_SYMBOL "create_third_party_appid_session"
-
 TPLibHandler* TPLibHandler::self = nullptr;
 
-int TPLibHandler::LoadCallback(const char* const path, int /* indent */)
+bool TPLibHandler::load_callback(const char* const path)
 {
-    void* handle = 0;
-    ThirdPartyAppIDModule* tp_module = 0;
-    const char* error = nullptr;
-
-    if (tp_appid_module != nullptr)
-    {
-        ErrorMessage("Ignoring additional 3rd party AppID module (%s)!\n", path);
-        return 0;
-    }
-
-    // Load the tp library and get function pointers to the module and
-    // session object factories.
     dlerror();
-    handle = dlopen(path, RTLD_NOW | RTLD_LOCAL);
-    if (handle == nullptr)
+    self->tp_so_handle = dlopen(path, RTLD_NOW | RTLD_LOCAL);
+    if (self->tp_so_handle == nullptr)
     {
         ErrorMessage("Failed to load 3rd party AppID library: %s - %s\n", path, dlerror());
-        return 0;
+        return false;
     }
 
-    CreateThirdPartyAppIDModule_t createThirdPartyAppIDModule=
-        (CreateThirdPartyAppIDModule_t)dlsym(handle, TP_APPID_MODULE_SYMBOL);
-    if ((error=dlerror()) != nullptr)
+    typedef void (*dummyFunc)();
+    struct funcBinding
     {
-        ErrorMessage(
-            "Failed to get 3rd party AppID module object factory: %s - %s\n",
-            TP_APPID_MODULE_SYMBOL, error);
-        dlclose(handle);
-        return 0;
-    }
-
-    createThirdPartyAppIDSession=(CreateThirdPartyAppIDSession_t)dlsym(
-        handle,TP_APPID_SESSION_SYMBOL);
-    if ((error=dlerror()) != nullptr)
+        const char* lib_sym;
+        dummyFunc* local_sym;
+    } bindings[] =
     {
-        ErrorMessage(
-            "Failed to get 3rd party AppID session object factory: %s - %s\n",
-            TP_APPID_SESSION_SYMBOL, error);
-        dlclose(handle);
-        return 0;
-    }
+        { "tp_appid_create_ctxt", (dummyFunc*)&tp_appid_create_ctxt },
+        { "tp_appid_create_session", (dummyFunc*)&tp_appid_create_session },
+        { "tp_appid_pfini", (dummyFunc*)&tp_appid_pfini },
+        { "tp_appid_tfini", (dummyFunc*)&tp_appid_tfini },
+        { nullptr, nullptr }
+    };
 
-    // The tp module object is a singleton and gets created here in main thread.
-    // TP session objects get created per worker thread.
-    tp_module=createThirdPartyAppIDModule();
-    if (tp_module == nullptr)
-    {
-        ErrorMessage("Failed to create third party appId module.\n");
-        dlclose(handle);
-        return 0;
-    }
+    funcBinding* index;
 
-    if ( (tp_module->api_version() != THIRD_PARTY_APP_ID_API_VERSION)
-        || (tp_module->module_name().empty()) )
+    for (index = bindings; index->lib_sym; index++)
     {
-        ErrorMessage("Ignoring incomplete 3rd party AppID module (%s, %u, %s)!\n",
-            path, tp_module->api_version(),
-            tp_module->module_name().empty() ? "empty" : tp_module->module_name().c_str());
-
-        dlclose(handle);
-        delete tp_module;
-        return 0;
+        *(void**)index->local_sym  = dlsym(self->tp_so_handle, index->lib_sym);
+        if (*(index->local_sym) == nullptr)
+        {
+            char* error;
+            ErrorMessage("AppId: Failed to resolve symbol: %s %s\n", index->lib_sym,
+                (error = dlerror()) ? error : "");
+            dlclose(self->tp_so_handle);
+            self->tp_so_handle = nullptr;
+            return false;
+        }
     }
 
-    this->tp_so_handle = handle;
-    tp_appid_module = tp_module;
-    return 0;
+    return true;
 }
 
-void TPLibHandler::pinit(const AppIdModuleConfig* config)
+ThirdPartyAppIDModule* TPLibHandler::create_tp_appid_ctxt(const AppIdModuleConfig& config)
 {
-    int ret;
-
-    if (self->tp_so_handle or config->tp_appid_path.empty())
-        return;
-
-    self->tp_config.tp_appid_config = config->tp_appid_config;
-    self->tp_config.tp_appid_stats_enable = config->tp_appid_stats_enable;
-    self->tp_config.tp_appid_config_dump = config->tp_appid_config_dump;
+    assert(self != nullptr);
 
-    self->LoadCallback(config->tp_appid_path.c_str(),1);
-
-    if (self->tp_appid_module == nullptr)
+    if (!self->tp_so_handle)
     {
-        ErrorMessage("Ignoring third party AppId library\n");
-        return;
+        if (config.tp_appid_path.empty())
+            return nullptr;
+
+        if (!self->load_callback(config.tp_appid_path.c_str()))
+            return nullptr;
     }
 
-    self->tp_config.chp_body_collection_max = config->chp_body_collection_max;
-    self->tp_config.ftp_userid_disabled = config->ftp_userid_disabled;
-    self->tp_config.chp_body_collection_disabled =
-        config->chp_body_collection_disabled;
-    self->tp_config.tp_allow_probes = config->tp_allow_probes;
-    if (config->http2_detection_enabled)
-        self->tp_config.http_upgrade_reporting_enabled = 1;
+    ThirdPartyConfig tp_config;
+    tp_config.tp_appid_config = config.tp_appid_config;
+    tp_config.tp_appid_stats_enable = config.tp_appid_stats_enable;
+    tp_config.tp_appid_config_dump = config.tp_appid_config_dump;
+    tp_config.chp_body_collection_max = config.chp_body_collection_max;
+    tp_config.ftp_userid_disabled = config.ftp_userid_disabled;
+    tp_config.chp_body_collection_disabled =
+        config.chp_body_collection_disabled;
+    tp_config.tp_allow_probes = config.tp_allow_probes;
+    if (config.http2_detection_enabled)
+        tp_config.http_upgrade_reporting_enabled = 1;
     else
-        self->tp_config.http_upgrade_reporting_enabled = 0;
+        tp_config.http_upgrade_reporting_enabled = 0;
+    tp_config.http_response_version_enabled = config.http_response_version_enabled;
 
-    self->tp_config.http_response_version_enabled = config->http_response_version_enabled;
+    ThirdPartyAppIDModule* tp_appid_ctxt = self->tp_appid_create_ctxt(tp_config);
+    if (tp_appid_ctxt == nullptr)
+    {
+        ErrorMessage("Failed to create third party appId context.\n");
+        dlclose(self->tp_so_handle);
+        self->tp_so_handle = nullptr;
+        return nullptr;
+    }
 
-    ret = self->tp_appid_module->pinit(self->tp_config);
-    if (ret != 0)
+    if ( (tp_appid_ctxt->api_version() != THIRD_PARTY_APP_ID_API_VERSION)
+        || (tp_appid_ctxt->module_name().empty()) )
     {
-        ErrorMessage("Unable to initialize 3rd party AppID module (%d)!\n", ret);
-        delete self->tp_appid_module;
+        ErrorMessage("Ignoring incomplete 3rd party AppID module (%s, %u, %s)!\n",
+            config.tp_appid_path.c_str(), tp_appid_ctxt->api_version(),
+            tp_appid_ctxt->module_name().empty() ? "empty" : tp_appid_ctxt->module_name().c_str());
+
+        delete tp_appid_ctxt;
         dlclose(self->tp_so_handle);
         self->tp_so_handle = nullptr;
-        self->tp_appid_module = nullptr;
-        return;
+        return nullptr;
     }
+
+    return tp_appid_ctxt;
 }
 
-void TPLibHandler::pfini(bool print_stats_flag)
+void TPLibHandler::tfini()
 {
-    if (self and self->tp_appid_module != nullptr)
-    {
-        if (print_stats_flag)
-            self->tp_appid_module->print_stats();
+    assert(self != nullptr);
 
-        int ret = self->tp_appid_module->pfini();
+    int ret = 0;
 
-        if (ret != 0)
-            ErrorMessage("Could not finalize 3rd party AppID module (%d)!\n", ret);
+    if (self->tp_appid_tfini)
+        ret = self->tp_appid_tfini();
 
-        delete self->tp_appid_module;
-        self->tp_appid_module = nullptr;
+    if (ret != 0)
+        ErrorMessage("Could not terminate packet thread in 3rd party AppID module (%d)!\n", ret);
+}
 
-        dlclose(self->tp_so_handle); // after delete, otherwise tpam will be dangling
-        self->tp_so_handle = nullptr;
-    }
+void TPLibHandler::pfini()
+{
+    assert(self != nullptr);
+
+    int ret = 0;
+
+    if (self->tp_appid_pfini)
+        ret = self->tp_appid_pfini();
+
+    if (ret != 0)
+        ErrorMessage("Could not terminate 3rd party AppID module (%d)!\n", ret);
+
+    // FIXIT-L: Find the right place to dlclose self->tp_so_handle. dlclose here was causing
+    // segfault
 
     if ( self ) {
         delete self;
diff --git a/src/network_inspectors/appid/tp_lib_handler.h b/src/network_inspectors/appid/tp_lib_handler.h
index 6909a4ed7..e461d4582 100644
--- a/src/network_inspectors/appid/tp_lib_handler.h
+++ b/src/network_inspectors/appid/tp_lib_handler.h
@@ -26,52 +26,46 @@
 
 class AppIdModuleConfig;
 
-// Class responsible for loading/reloading the thirdparty.so library and
-// for holding pointers to objects that live inside thirdparty.so.
+// This needs to be exported by any third party .so library.
+// Must return NULL if it fails to create the object.
+typedef ThirdPartyAppIDModule* (* TpAppIdCreateCtxt)(ThirdPartyConfig& );
+typedef ThirdPartyAppIDSession* (* TpAppIdCreateSession)(ThirdPartyAppIDModule& ctxt);
+typedef int (* TpAppIdPfini)();
+typedef int (* TpAppIdTfini)();
+
+// Class responsible for loading/reloading the thirdparty.so library
 class TPLibHandler
 {
 public:
-
-    static bool have_tp()
-    {
-        return self and self->tp_appid_module != nullptr;
-    }
-
     static TPLibHandler* get()
     {
         if (self)
             return self;
         else
-            return (self=new TPLibHandler());
+            return (self = new TPLibHandler());
     }
 
-    // called from appid_inspector.cc appid_inspector_pinit() / pterm()
-    static void pinit(const AppIdModuleConfig* config);
-    static void pfini(bool print_stats_flag=0);
+    static ThirdPartyAppIDModule* create_tp_appid_ctxt(const AppIdModuleConfig& config);
+    static void tfini();
+    static void pfini();
 
-    // called from AppIdInspector tinit/tterm via
-    // AppIdConfig::tp_appid_module_tinit/tterm.
-    static void tinit() { if ( have_tp() ) self->tp_appid_module->tinit(); }
-    static void tterm() { if ( have_tp() ) self->tp_appid_module->tfini(); }
-
-    CreateThirdPartyAppIDSession_t tpsession_factory() const
+    TpAppIdCreateSession tpsession_factory() const
     {
-        return createThirdPartyAppIDSession;
+        return tp_appid_create_session;
     }
 
 private:
-
     TPLibHandler() = default;
     ~TPLibHandler() = default;
 
     static TPLibHandler* self;
     void* tp_so_handle = nullptr;   // output of dlopen(thirdparty.so)
-    ThirdPartyAppIDModule* tp_appid_module = nullptr;
-    CreateThirdPartyAppIDSession_t createThirdPartyAppIDSession;
-
-    ThirdPartyConfig tp_config;
+    TpAppIdCreateCtxt tp_appid_create_ctxt = nullptr;
+    TpAppIdCreateSession tp_appid_create_session = nullptr;
+    TpAppIdPfini tp_appid_pfini = nullptr;
+    TpAppIdTfini tp_appid_tfini = nullptr;
 
-    int LoadCallback(const char* path, int);
+    bool load_callback(const char* path);
 };
 
 #endif