From: Steve Chew (stechew) <stechew@cisco.com>
Date: Fri, 24 Jul 2020 19:09:08 +0000 (+0000)
Subject: Merge pull request #2333 in SNORT/snort3 from ~DERAMADA/snort3:init_unpriv_during_sta... 
X-Git-Tag: 3.0.2-3~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a7e556d0ff0d784c19080b555832db7f2d6d3007;p=thirdparty%2Fsnort3.git

Merge pull request #2333 in SNORT/snort3 from ~DERAMADA/snort3:init_unpriv_during_startup to master

Squashed commit of the following:

commit 9dff164defbc45e84ddd61715252d0a8562e0442
Author: deramada <deramada@cisco.com>
Date:   Wed Jul 15 14:49:21 2020 -0400

    active: Move Active enabled flag into SnortConfig

    This fixes potential race conditions between reloads in the main thread
    changing the Active state while packet threads are directly accessing
    it.
---

diff --git a/src/actions/act_react.cc b/src/actions/act_react.cc
index c0befba6e..40fa375ef 100644
--- a/src/actions/act_react.cc
+++ b/src/actions/act_react.cc
@@ -52,6 +52,7 @@
 #include "framework/ips_action.h"
 #include "framework/module.h"
 #include "log/messages.h"
+#include "main/snort_config.h"
 #include "packet_io/active.h"
 #include "profiler/profiler.h"
 #include "protocols/packet.h"
@@ -208,9 +209,10 @@ bool ReactModule::getpage(const char* file)
     return true;
 }
 
-bool ReactModule::begin(const char*, int, SnortConfig*)
+bool ReactModule::begin(const char*, int, SnortConfig* sc)
 {
     page.clear();
+    sc->set_active_enabled();
     return true;
 }
 
@@ -236,7 +238,6 @@ static IpsAction* react_ctor(Module* p)
 {
     ReactModule* m = (ReactModule*)p;
     ReactData* rd = new ReactData(m->page);
-    Active::set_enabled();
 
     return new ReactAction(rd);
 }
diff --git a/src/actions/act_reject.cc b/src/actions/act_reject.cc
index 082c166db..f9eb2e315 100644
--- a/src/actions/act_reject.cc
+++ b/src/actions/act_reject.cc
@@ -50,6 +50,7 @@
 
 #include "framework/ips_action.h"
 #include "framework/module.h"
+#include "main/snort_config.h"
 #include "packet_io/active.h"
 #include "profiler/profiler.h"
 
@@ -180,9 +181,10 @@ public:
     uint32_t flags;
 };
 
-bool RejectModule::begin(const char*, int, SnortConfig*)
+bool RejectModule::begin(const char*, int, SnortConfig* sc)
 {
     flags = 0;
+    sc->set_active_enabled();
     return true;
 }
 
@@ -241,7 +243,6 @@ static void mod_dtor(Module* m)
 static IpsAction* rej_ctor(Module* p)
 {
     RejectModule* m = (RejectModule*)p;
-    Active::set_enabled();
     return new RejectAction(m->flags);
 }
 
diff --git a/src/actions/act_replace.cc b/src/actions/act_replace.cc
index 388ab381b..1ad01da12 100644
--- a/src/actions/act_replace.cc
+++ b/src/actions/act_replace.cc
@@ -24,6 +24,7 @@
 #include "detection/detection_engine.h"
 #include "framework/ips_action.h"
 #include "framework/module.h"
+#include "main/snort_config.h"
 #include "packet_io/active.h"
 #include "protocols/packet.h"
 
@@ -88,7 +89,6 @@ public:
     ReplaceModule() : Module(s_name, s_help, s_params) { }
     bool set(const char*, Value&, SnortConfig*) override;
     bool begin(const char*, int, SnortConfig*) override;
-    bool end(const char*, int, SnortConfig*) override;
 
     Usage get_usage() const override
     { return DETECT; }
@@ -107,14 +107,10 @@ bool ReplaceModule::set(const char*, Value& v, SnortConfig*)
     return true;
 }
 
-bool ReplaceModule::begin(const char*, int, SnortConfig*)
+bool ReplaceModule::begin(const char*, int, SnortConfig* sc)
 {
     disable_replace = false;
-    return true;
-}
-
-bool ReplaceModule::end(const char*, int, SnortConfig*)
-{
+    sc->set_active_enabled();
     return true;
 }
 
@@ -133,7 +129,6 @@ ReplaceAction::ReplaceAction(bool dr) :
     IpsAction(s_name, ACT_RESET)
 {
     disable_replace = dr;
-    Active::set_enabled();
 }
 
 void ReplaceAction::exec(Packet* p)
diff --git a/src/file_api/file_module.cc b/src/file_api/file_module.cc
index 40006d7e7..58bbf21e3 100644
--- a/src/file_api/file_module.cc
+++ b/src/file_api/file_module.cc
@@ -464,12 +464,12 @@ bool FileIdModule::begin(const char* fqn, int idx, SnortConfig*)
     return true;
 }
 
-bool FileIdModule::end(const char* fqn, int idx, SnortConfig*)
+bool FileIdModule::end(const char* fqn, int idx, SnortConfig* sc)
 {
     if (!idx)
     {
         if ( need_active )
-            Active::set_enabled();
+            sc->set_active_enabled();
         return true;
     }
 
diff --git a/src/main/modules.cc b/src/main/modules.cc
index 714c62ff8..4fa9bd0b5 100644
--- a/src/main/modules.cc
+++ b/src/main/modules.cc
@@ -869,8 +869,13 @@ bool ActiveModule::set(const char*, Value& v, SnortConfig* sc)
         sc->set_dst_mac(v.get_string());
 
     else if ( v.is("max_responses") )
+    {
         sc->max_responses = v.get_uint8();
 
+        if ( sc->max_responses )
+            sc->set_active_enabled();
+    }
+
     else if ( v.is("min_interval") )
         sc->min_interval = v.get_uint8();
 
diff --git a/src/main/snort.cc b/src/main/snort.cc
index 0ccdad0e1..e3303a56e 100644
--- a/src/main/snort.cc
+++ b/src/main/snort.cc
@@ -233,8 +233,6 @@ void Snort::init(int argc, char** argv)
 
     parser_term(sc);
 
-    Active::init(sc);
-
     LogMessage("%s\n", LOG_DIV);
 
     SFDAQ::init(sc->daq_config, ThreadConfig::get_instance_max());
diff --git a/src/main/snort_config.h b/src/main/snort_config.h
index 769f7ceee..70ba985f4 100644
--- a/src/main/snort_config.h
+++ b/src/main/snort_config.h
@@ -424,6 +424,7 @@ public:
     Plugins* plugins = nullptr;
     SoRules* so_rules = nullptr;
 private:
+    bool active_enabled = false;
     std::list<ReloadResourceTuner*> reload_tuners;
 
 public:
@@ -626,6 +627,13 @@ public:
     bool assure_established() const
     { return run_flags & RUN_FLAG__ASSURE_EST; }
 
+    // active stuff
+    void set_active_enabled()
+    { active_enabled = true; }
+
+    bool is_active_enabled() const
+    { return active_enabled; }
+
     // other stuff
     uint8_t min_ttl() const
     { return get_network_policy()->min_ttl; }
diff --git a/src/packet_io/active.cc b/src/packet_io/active.cc
index 98de00ca6..14aab9a5e 100644
--- a/src/packet_io/active.cc
+++ b/src/packet_io/active.cc
@@ -65,7 +65,6 @@ const char* Active::act_str[Active::ACT_MAX][Active::AST_MAX] =
     { "block", "cant_block", "would_block", "force_block" },
     { "reset", "cant_reset", "would_reset", "force_reset" },
 };
-bool Active::enabled = false;
 
 THREAD_LOCAL uint8_t Active::s_attempts = 0;
 THREAD_LOCAL bool Active::s_suspend = false;
@@ -176,12 +175,6 @@ void Active::kill_session(Packet* p, EncodeFlags flags)
 
 //--------------------------------------------------------------------
 
-void Active::init(SnortConfig* sc)
-{
-    if (sc->max_responses > 0)
-        Active::set_enabled();
-}
-
 bool Active::thread_init(const SnortConfig* sc)
 {
     s_attempts = sc->respond_attempts;
@@ -189,10 +182,10 @@ bool Active::thread_init(const SnortConfig* sc)
     if ( s_attempts > MAX_ATTEMPTS )
         s_attempts = MAX_ATTEMPTS;
 
-    if ( enabled && !s_attempts )
+    if ( sc->is_active_enabled() && !s_attempts )
         s_attempts = 1;
 
-    if ( enabled && (!SFDAQ::can_inject() || !sc->respond_device.empty()) )
+    if ( sc->is_active_enabled() && (!SFDAQ::can_inject() || !sc->respond_device.empty()) )
     {
         if ( sc->read_mode() || !open(sc->respond_device.c_str()) )
         {
@@ -667,7 +660,7 @@ void Active::reset_session(Packet* p, ActiveAction* reject, bool force)
     if ( force or p->context->conf->inline_mode() or p->context->conf->treat_drop_as_ignore() )
         Stream::drop_flow(p);
 
-    if ( enabled )
+    if ( p->context->conf->is_active_enabled() )
     {
         if (reject)
             Active::queue(reject, p);
diff --git a/src/packet_io/active.h b/src/packet_io/active.h
index 5c9ccc7fa..5e1f79059 100644
--- a/src/packet_io/active.h
+++ b/src/packet_io/active.h
@@ -58,13 +58,9 @@ public:
 
 public:
 
-    static void init(SnortConfig*);
     static bool thread_init(const SnortConfig*);
     static void thread_term();
 
-    static void set_enabled(bool on_off = true)
-    { enabled = on_off; }
-
     static void suspend()
     { s_suspend = true; }
 
@@ -181,7 +177,6 @@ private:
 
 private:
     static const char* act_str[ACT_MAX][AST_MAX];
-    static bool enabled;
     static THREAD_LOCAL uint8_t s_attempts;
     static THREAD_LOCAL bool s_suspend;
 
diff --git a/src/service_inspectors/smtp/smtp_module.cc b/src/service_inspectors/smtp/smtp_module.cc
index 0aaeada21..690407957 100644
--- a/src/service_inspectors/smtp/smtp_module.cc
+++ b/src/service_inspectors/smtp/smtp_module.cc
@@ -24,6 +24,7 @@
 
 #include "smtp_module.h"
 
+#include "main/snort_config.h"
 #include "log/messages.h"
 #include "packet_io/active.h"
 #include "utils/util.h"
@@ -330,10 +331,7 @@ bool SmtpModule::set(const char*, Value& v, SnortConfig*)
     }
 
     else if ( v.is("xlink2state") )
-    {
         config->xlink2state = (SMTPXlinkState)v.get_uint8();
-        Active::set_enabled();
-    }
 
     else
         return false;
@@ -366,8 +364,11 @@ bool SmtpModule::begin(const char*, int, SnortConfig*)
     return true;
 }
 
-bool SmtpModule::end(const char* fqn, int idx, SnortConfig*)
+bool SmtpModule::end(const char* fqn, int idx, SnortConfig* sc)
 {
+    if ( !strcmp(fqn, "smtp") and config->xlink2state )
+        sc->set_active_enabled();
+
     if ( !idx )
         return true;