From: Mike Stepanek (mstepane) <mstepane@cisco.com>
Date: Fri, 9 Nov 2018 14:50:14 +0000 (-0500)
Subject: Merge pull request #1425 in SNORT/snort3 from ~MASHASAN/snort3:databus_clone to master
X-Git-Tag: 3.0.0-250~27
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a3c89006e415c99d8741601dc9cac39c5de2b253;p=thirdparty%2Fsnort3.git

Merge pull request #1425 in SNORT/snort3 from ~MASHASAN/snort3:databus_clone to master

Squashed commit of the following:

commit 8ba948e060cad592234fc4b0786a0942fec30dde
Author: Masud Hasan <mashasan@cisco.com>
Date:   Thu Nov 1 00:02:28 2018 -0400

    framework: Cloning databus to new config during module reload
---

diff --git a/src/file_api/file_log.cc b/src/file_api/file_log.cc
index d91e1daa4..b2cc88281 100644
--- a/src/file_api/file_log.cc
+++ b/src/file_api/file_log.cc
@@ -83,7 +83,7 @@ static void dl_tterm()
 class LogHandler : public DataHandler
 {
 public:
-    LogHandler(FileLogConfig& conf)
+    LogHandler(FileLogConfig& conf) : DataHandler(s_name)
     { config = conf; }
 
     void handle(DataEvent&, Flow*) override;
diff --git a/src/framework/data_bus.cc b/src/framework/data_bus.cc
index 7793e9f3b..16b2c3176 100644
--- a/src/framework/data_bus.cc
+++ b/src/framework/data_bus.cc
@@ -69,7 +69,33 @@ DataBus::~DataBus()
 {
     for ( auto& p : map )
         for ( auto* h : p.second )
-            delete h;
+        {
+            // If the object is cloned, pass the ownership to the next config.
+            // When the object is no further cloned (e.g., the last config), delete it.
+            if ( h->cloned )
+                h->cloned = false;
+            else
+                delete h;
+        }
+
+    mapped_module.clear();
+}
+
+void DataBus::add_mapped_module(const char* name)
+{
+    if ( name )
+        mapped_module.emplace(name);
+}
+
+void DataBus::clone(DataBus& from)
+{
+    for ( auto& p : from.map )
+        for ( auto* h : p.second )
+            if ( mapped_module.count(h->module_name) == 0 )
+            {
+                h->cloned = true;
+                _subscribe(p.first.c_str(), h);
+            }
 }
 
 // add handler to list of handlers to be notified upon
@@ -137,6 +163,10 @@ void DataBus::_subscribe(const char* key, DataHandler* h)
 {
     DataList& v = map[key];
     v.emplace_back(h);
+
+    // Track fresh subscriptions to distinguish during cloning
+    if ( !h->cloned )
+        add_mapped_module(h->module_name);
 }
 
 void DataBus::_unsubscribe(const char* key, DataHandler* h)
diff --git a/src/framework/data_bus.h b/src/framework/data_bus.h
index c37a36640..204b9a80b 100644
--- a/src/framework/data_bus.h
+++ b/src/framework/data_bus.h
@@ -29,6 +29,7 @@
 
 #include <map>
 #include <string>
+#include <unordered_set>
 #include <vector>
 
 #include "main/snort_types.h"
@@ -72,14 +73,18 @@ public:
     virtual ~DataHandler() = default;
 
     virtual void handle(DataEvent&, Flow*) { }
+    const char* module_name;
+    bool cloned;
 
 protected:
-    DataHandler() = default;
+    DataHandler(std::nullptr_t) = delete;
+    DataHandler(const char* mod_name) : module_name(mod_name), cloned(false) { }
 };
 
 // FIXIT-P evaluate perf; focus is on correctness
 typedef std::vector<DataHandler*> DataList;
 typedef std::map<std::string, DataList> DataMap;
+typedef std::unordered_set<const char*> DataModule;
 
 class SO_PUBLIC DataBus
 {
@@ -87,6 +92,9 @@ public:
     DataBus();
     ~DataBus();
 
+    void clone(DataBus& from);
+    void add_mapped_module(const char*);
+
     static void subscribe(const char* key, DataHandler*);
     static void subscribe_default(const char* key, DataHandler*);
     static void unsubscribe(const char* key, DataHandler*);
@@ -105,6 +113,7 @@ private:
 
 private:
     DataMap map;
+    DataModule mapped_module;
 };
 
 class SO_PUBLIC DaqMetaEvent : public DataEvent
diff --git a/src/loggers/log_hext.cc b/src/loggers/log_hext.cc
index 77ea327c0..818470928 100644
--- a/src/loggers/log_hext.cc
+++ b/src/loggers/log_hext.cc
@@ -43,7 +43,7 @@ static THREAD_LOCAL unsigned s_pkt_num = 0;
 class DaqMetaEventHandler : public DataHandler
 {
 public:
-    DaqMetaEventHandler() = default;
+    DaqMetaEventHandler() : DataHandler(S_NAME) { }
     void handle(DataEvent&, Flow*) override;
 };
 
diff --git a/src/main/policy.cc b/src/main/policy.cc
index 36103c911..cb6399fde 100644
--- a/src/main/policy.cc
+++ b/src/main/policy.cc
@@ -60,7 +60,7 @@ NetworkPolicy::NetworkPolicy(PolicyId id)
 class AltPktHandler : public DataHandler
 {
 public:
-    AltPktHandler() = default;
+    AltPktHandler() : DataHandler("detection") { }
 
     void handle(DataEvent& e, Flow*) override
     { DetectionEngine::detect(const_cast<Packet*>(e.get_packet())); }
@@ -93,6 +93,14 @@ void InspectionPolicy::configure()
     dbus.subscribe(PACKET_EVENT, new AltPktHandler);
 }
 
+void InspectionPolicy::clone_dbus(SnortConfig* from, const char* exclude_module)
+{
+    // Clone subscriptions from another config if they are not excluded
+    // (e.g., reloading module) and not already subscribed
+    dbus.add_mapped_module(exclude_module);
+    dbus.clone(from->policy_map->get_inspection_policy()->dbus);
+}
+
 //-------------------------------------------------------------------------
 // detection policy
 //-------------------------------------------------------------------------
diff --git a/src/main/policy.h b/src/main/policy.h
index cc7d9718c..97c9c0aff 100644
--- a/src/main/policy.h
+++ b/src/main/policy.h
@@ -40,6 +40,7 @@ typedef unsigned char uuid_t[16];
 namespace snort
 {
 struct GHash;
+struct SnortConfig;
 }
 
 struct PortTable;
@@ -111,6 +112,7 @@ public:
     ~InspectionPolicy();
 
     void configure();
+    void clone_dbus(snort::SnortConfig*, const char*);
 
 public:
     PolicyId policy_id;
@@ -249,8 +251,6 @@ private:
 // FIXIT-L SO_PUBLIC required because SnortConfig::inline_mode(), etc. uses the function
 namespace snort
 {
-struct SnortConfig;
-
 SO_PUBLIC NetworkPolicy* get_network_policy();
 SO_PUBLIC InspectionPolicy* get_inspection_policy();
 SO_PUBLIC IpsPolicy* get_ips_policy();
diff --git a/src/main/snort.cc b/src/main/snort.cc
index 7f643aebc..73c6bd6bc 100644
--- a/src/main/snort.cc
+++ b/src/main/snort.cc
@@ -674,7 +674,7 @@ SnortConfig* Snort::get_updated_policy(SnortConfig* other_conf, const char* fnam
     }
 
     other_conf->cloned = true;
-
+    sc->policy_map->get_inspection_policy()->clone_dbus(other_conf, iname);
     InspectorManager::update_policy(sc);
     reloading = false;
     return sc;
@@ -688,6 +688,7 @@ SnortConfig* Snort::get_updated_module(SnortConfig* other_conf, const char* name
 
     if ( name )
     {
+        ModuleManager::reset_errors();
         ModuleManager::reload_module(name, sc);
         if ( ModuleManager::get_errors() || !sc->verify() )
         {
@@ -711,7 +712,7 @@ SnortConfig* Snort::get_updated_module(SnortConfig* other_conf, const char* name
     }
 
     other_conf->cloned = true;
-
+    sc->policy_map->get_inspection_policy()->clone_dbus(other_conf, name);
     InspectorManager::update_policy(sc);
     reloading = false;
     return sc;
diff --git a/src/managers/module_manager.cc b/src/managers/module_manager.cc
index 1e25eb6e5..331660062 100644
--- a/src/managers/module_manager.cc
+++ b/src/managers/module_manager.cc
@@ -1079,6 +1079,7 @@ void ModuleManager::reload_module(const char* name, snort::SnortConfig* sc)
     {
         cout << "Module " << name <<" doesn't exist";
         cout << endl;
+        ++s_errors;
     }
 }
 
diff --git a/src/network_inspectors/appid/appid_http_event_handler.h b/src/network_inspectors/appid/appid_http_event_handler.h
index 0560f11eb..03b964c4a 100644
--- a/src/network_inspectors/appid/appid_http_event_handler.h
+++ b/src/network_inspectors/appid/appid_http_event_handler.h
@@ -26,6 +26,8 @@
 
 #include "pub_sub/http_events.h"
 
+#include "appid_module.h"
+
 namespace snort
 {
 class Flow;
@@ -40,7 +42,7 @@ public:
         RESPONSE_EVENT,
     };
 
-    HttpEventHandler(HttpEventType type)
+    HttpEventHandler(HttpEventType type) : DataHandler(MOD_NAME)
     {
         event_type = type;
     }
diff --git a/src/network_inspectors/appid/detector_plugins/detector_sip.h b/src/network_inspectors/appid/detector_plugins/detector_sip.h
index 0a42bccde..d2edc0636 100644
--- a/src/network_inspectors/appid/detector_plugins/detector_sip.h
+++ b/src/network_inspectors/appid/detector_plugins/detector_sip.h
@@ -27,6 +27,8 @@
 #include "framework/data_bus.h"
 #include "pub_sub/sip_events.h"
 
+#include "appid_module.h"
+
 namespace snort
 {
 class Flow;
@@ -105,7 +107,7 @@ public:
     void handle(snort::DataEvent&, snort::Flow*) override;
 
 private:
-    SipEventHandler() = default;
+    SipEventHandler() : DataHandler(MOD_NAME) { }
     void client_handler(SipEvent&, AppIdSession&, AppidChangeBits&);
     void service_handler(SipEvent&, AppIdSession&, AppidChangeBits&);
 
diff --git a/src/network_inspectors/binder/binder.cc b/src/network_inspectors/binder/binder.cc
index d57e2936c..4c63ae0ac 100644
--- a/src/network_inspectors/binder/binder.cc
+++ b/src/network_inspectors/binder/binder.cc
@@ -599,7 +599,7 @@ private:
 class FlowStateSetupHandler : public DataHandler
 {
 public:
-    FlowStateSetupHandler() = default;
+    FlowStateSetupHandler() : DataHandler(BIND_NAME) { }
 
     void handle(DataEvent& event, Flow* flow) override
     {
@@ -613,7 +613,7 @@ public:
 class FlowServiceChangeHandler : public DataHandler
 {
 public:
-    FlowServiceChangeHandler() = default;
+    FlowServiceChangeHandler() : DataHandler(BIND_NAME) { }
 
     void handle(DataEvent&, Flow* flow) override
     {
@@ -626,7 +626,7 @@ public:
 class StreamHANewFlowHandler : public DataHandler
 {
 public:
-    StreamHANewFlowHandler() = default;
+    StreamHANewFlowHandler() : DataHandler(BIND_NAME) { }
 
     void handle(DataEvent&, Flow* flow) override
     {
diff --git a/src/network_inspectors/perf_monitor/perf_monitor.cc b/src/network_inspectors/perf_monitor/perf_monitor.cc
index 5fa927419..3c09d1321 100644
--- a/src/network_inspectors/perf_monitor/perf_monitor.cc
+++ b/src/network_inspectors/perf_monitor/perf_monitor.cc
@@ -86,7 +86,7 @@ private:
 class PerfIdleHandler : public DataHandler
 {
 public:
-    PerfIdleHandler(PerfMonitor& p) : perf_monitor(p)
+    PerfIdleHandler(PerfMonitor& p) : DataHandler(PERF_NAME), perf_monitor(p)
     { DataBus::subscribe_default(THREAD_IDLE_EVENT, this); }
 
     void handle(DataEvent&, Flow*) override
@@ -99,7 +99,7 @@ private:
 class PerfRotateHandler : public DataHandler
 {
 public:
-    PerfRotateHandler(PerfMonitor& p) : perf_monitor(p)
+    PerfRotateHandler(PerfMonitor& p) : DataHandler(PERF_NAME), perf_monitor(p)
     { DataBus::subscribe_default(THREAD_ROTATE_EVENT, this); }
 
     void handle(DataEvent&, Flow*) override
@@ -112,7 +112,7 @@ private:
 class FlowIPDataHandler : public DataHandler
 {
 public:
-    FlowIPDataHandler(PerfMonitor& p) : perf_monitor(p)
+    FlowIPDataHandler(PerfMonitor& p) : DataHandler(PERF_NAME), perf_monitor(p)
     { DataBus::subscribe_default(FLOW_STATE_EVENT, this); }
 
     void handle(DataEvent&, Flow* flow) override