Merge pull request #2107 in SNORT/snort3 from ~BBANTWAL/snort3:latency_updates to...

Squashed commit of the following:

commit 99e8356b5e645aebb676d58acc22462948cab5b8
Author: Bhagya Tholpady <bbantwal@cisco.com>
Date:   Wed Mar 25 10:01:54 2020 -0400

    latency: remove action config option and convert the log handler to trace_log message

commit d9ce00ad8447b8f376077b249f1a03c7f0c2acbc
Author: Bhagya Tholpady <bbantwal@cisco.com>
Date:   Wed Mar 25 09:05:41 2020 -0400

    snort2lua: remove conversion of deprecated options pkt-log and rule-log

diff --git a/src/latency/latency_config.h b/src/latency/latency_config.h
index c3ac7ea6029f10a62ac96ef95d18d9a860ebe39c..c580c4495d474e78bb0ac2a139e13057dab8652a 100644 (file)
--- a/src/latency/latency_config.h
+++ b/src/latency/latency_config.h
@@ -21,6 +21,8 @@
 #ifndef LATENCY_CONFIG_H
 #define LATENCY_CONFIG_H
 
+#include "main/snort_debug.h"
+
 #include "packet_latency_config.h"
 #include "rule_latency_config.h"
 
@@ -30,4 +32,6 @@ struct LatencyConfig
     RuleLatencyConfig rule_latency;
 };
 
+extern snort::Trace latency_trace;
+
 #endif

diff --git a/src/latency/latency_module.cc b/src/latency/latency_module.cc
index f331636e013aa5eb09ca9146fb843e8b7ab06309..2a5bee245dca6192baab13615e4aba3bf16d1ace 100644 (file)
--- a/src/latency/latency_module.cc
+++ b/src/latency/latency_module.cc
@@ -42,6 +42,8 @@ using namespace snort;
 #define s_help \
     "packet and rule latency monitoring and control"
 
+Trace latency_trace(s_name);
+
 static const Parameter s_packet_params[] =
 {
     { "max_time", Parameter::PT_INT, "0:max53", "500",
@@ -50,9 +52,6 @@ static const Parameter s_packet_params[] =
     { "fastpath", Parameter::PT_BOOL, nullptr, "false",
         "fastpath expensive packets (max_time exceeded)" },
 
-    { "action", Parameter::PT_ENUM, "none | alert | log | alert_and_log", "none",
-        "event action if packet times out and is fastpathed" },
-
     { nullptr, Parameter::PT_MAX, nullptr, nullptr, nullptr }
 };
 
@@ -72,9 +71,6 @@ static const Parameter s_rule_params[] =
     { "max_suspend_time", Parameter::PT_INT, "0:max32", "30000",
         "set max time for suspending a rule (ms, 0 means permanently disable rule)" },
 
-    { "action", Parameter::PT_ENUM, "none | alert | log | alert_and_log", "none",
-        "event action for rule latency enable and suspend events" },
-
     { nullptr, Parameter::PT_MAX, nullptr, nullptr, nullptr }
 };
 
@@ -126,9 +122,6 @@ static inline bool latency_set(Value& v, PacketLatencyConfig& config)
     else if ( v.is("fastpath") )
         config.fastpath = v.get_bool();
 
-    else if ( v.is("action") )
-        config.action =
-            static_cast<decltype(config.action)>(v.get_uint8());
     else
         return false;
 
@@ -153,9 +146,6 @@ static inline bool latency_set(Value& v, RuleLatencyConfig& config)
         long t = clock_ticks(v.get_uint32());
         config.max_suspend_time = TO_DURATION(config.max_time, t);
     }
-    else if ( v.is("action") )
-        config.action =
-            static_cast<decltype(config.action)>(v.get_uint8());
     else
         return false;
 
@@ -163,7 +153,7 @@ static inline bool latency_set(Value& v, RuleLatencyConfig& config)
 }
 
 LatencyModule::LatencyModule() :
-    Module(s_name, s_help, s_params)
+    Module(s_name, s_help, s_params, false, &latency_trace)
 { }
 
 bool LatencyModule::set(const char* fqn, Value& v, SnortConfig* sc)
@@ -177,7 +167,7 @@ bool LatencyModule::set(const char* fqn, Value& v, SnortConfig* sc)
     else if ( !strncmp(fqn, slr, strlen(slr)) )
         return latency_set(v, sc->latency->rule_latency);
 
-    return false;
+    return Module::set(fqn, v, sc);
 }
 
 const RuleMap* LatencyModule::get_rules() const

diff --git a/src/latency/packet_latency.cc b/src/latency/packet_latency.cc
index 4ff4fa6f8aab5eed104f0a2e965fa4c9e1641d01..938987beb4af72cb99e25187947c29e279234288 100644 (file)
--- a/src/latency/packet_latency.cc
+++ b/src/latency/packet_latency.cc
@@ -72,7 +72,7 @@ using EventHandler = EventingWrapper<Event>;
 
 static inline std::ostream& operator<<(std::ostream& os, const Event& e)
 {
-    os << "latency: " << e.packet->context->packet_number << " packet";
+    os << "packet " << e.packet->context->packet_number;
 
     if ( e.fastpathed )
         os << " fastpathed: ";
@@ -102,7 +102,7 @@ template<typename Clock = SnortClock>
 class Impl
 {
 public:
-    Impl(const ConfigWrapper&, EventHandler&, EventHandler&);
+    Impl(const ConfigWrapper&, EventHandler&);
 
     void push();
     bool pop(const Packet*);
@@ -114,12 +114,11 @@ private:
     std::vector<PacketTimer<Clock>> timers;
     const ConfigWrapper& config;
     EventHandler& event_handler;
-    EventHandler& log_handler;
 };
 
 template<typename Clock>
-inline Impl<Clock>::Impl(const ConfigWrapper& cfg, EventHandler& eh, EventHandler& lh) :
-    config(cfg), event_handler(eh), log_handler(lh)
+inline Impl<Clock>::Impl(const ConfigWrapper& cfg, EventHandler& eh) :
+    config(cfg), event_handler(eh)
 { }
 
 template<typename Clock>
@@ -143,11 +142,7 @@ inline bool Impl<Clock>::pop(const Packet* p)
         // timer.mark implies fastpath-related timeout
         Event e { p, timer.marked_as_fastpathed, timer.elapsed() };
 
-        if ( config->action & PacketLatencyConfig::LOG )
-            log_handler.handle(e);
-
-        if ( config->action & PacketLatencyConfig::ALERT )
-            event_handler.handle(e);
+        event_handler.handle(e);
     }
 
     elapsed = clock_usecs(TO_USECS(timer.elapsed()));
@@ -186,21 +181,17 @@ static struct SnortConfigWrapper : public ConfigWrapper
 } config;
 
 static struct SnortEventHandler : public EventHandler
-{
-    void handle(const Event&) override
-    { DetectionEngine::queue_event(GID_LATENCY, LATENCY_EVENT_PACKET_FASTPATHED); }
-} event_handler;
-
-static struct SnortLogHandler : public EventHandler
 {
     void handle(const Event& e) override
     {
         assert(e.packet);
         std::ostringstream ss;
         ss << e;
-        LogMessage("%s\n", ss.str().c_str());
+        debug_logf(latency_trace, "%s\n", ss.str().c_str());
+
+        DetectionEngine::queue_event(GID_LATENCY, LATENCY_EVENT_PACKET_FASTPATHED);
     }
-} log_handler;
+} event_handler;
 
 static THREAD_LOCAL Impl<>* impl = nullptr;
 
@@ -208,7 +199,7 @@ static THREAD_LOCAL Impl<>* impl = nullptr;
 static inline Impl<>& get_impl()
 {
     if ( !impl )
-        impl = new Impl<>(config, event_handler, log_handler);
+        impl = new Impl<>(config, event_handler);
 
     return *impl;
 }
@@ -312,15 +303,12 @@ TEST_CASE ( "packet latency impl", "[latency]" )
 
     MockConfigWrapper config;
     EventHandlerSpy event_handler;
-    EventHandlerSpy log_handler;
 
     MockClock::reset();
 
-    packet_latency::Impl<MockClock> impl(config, event_handler, log_handler);
+    packet_latency::Impl<MockClock> impl(config, event_handler);
 
     config.config.max_time = 2_ticks;
-    config.config.action = PacketLatencyConfig::ALERT_AND_LOG;
-
     SECTION( "fastpath enabled" )
     {
         config.config.fastpath = true;
@@ -336,7 +324,6 @@ TEST_CASE ( "packet latency impl", "[latency]" )
             CHECK( impl.pop(nullptr) );
 
             CHECK( event_handler.count == 1 );
-            CHECK( log_handler.count == 1 );
         }
 
         SECTION( "no timeout" )
@@ -345,7 +332,6 @@ TEST_CASE ( "packet latency impl", "[latency]" )
             CHECK_FALSE( impl.pop(nullptr) );
 
             CHECK( event_handler.count == 0 );
-            CHECK( log_handler.count == 0 );
         }
     }
 
@@ -364,7 +350,6 @@ TEST_CASE ( "packet latency impl", "[latency]" )
             CHECK( impl.pop(nullptr) );
 
             CHECK( event_handler.count == 1 );
-            CHECK( log_handler.count == 1 );
         }
 
         SECTION( "no timeout" )
@@ -373,7 +358,6 @@ TEST_CASE ( "packet latency impl", "[latency]" )
             CHECK_FALSE( impl.pop(nullptr) );
 
             CHECK( event_handler.count == 0 );
-            CHECK( log_handler.count == 0 );
         }
     }
 }

diff --git a/src/latency/packet_latency_config.h b/src/latency/packet_latency_config.h
index 0821d891d2bce25de2c93ca5b18366d38d88a78d..18501dfe6965cddd7567cc257a3e5afefe71f5d0 100644 (file)
--- a/src/latency/packet_latency_config.h
+++ b/src/latency/packet_latency_config.h
@@ -25,17 +25,8 @@
 
 struct PacketLatencyConfig
 {
-    enum Action
-    {
-        NONE = 0x00,
-        ALERT = 0x01,
-        LOG = 0x02,
-        ALERT_AND_LOG = ALERT | LOG
-    };
-
     hr_duration max_time = CLOCK_ZERO;
     bool fastpath = false;
-    Action action = NONE;
 
     bool enabled() const { return max_time > CLOCK_ZERO; }
 };

diff --git a/src/latency/rule_latency.cc b/src/latency/rule_latency.cc
index f045b7964bf9beed1dcb60bb96d3a9e9b2a347b2..182f32f2a3300e2cf8ad197f707fc9a7274b9ce7 100644 (file)
--- a/src/latency/rule_latency.cc
+++ b/src/latency/rule_latency.cc
@@ -86,7 +86,7 @@ static inline std::ostream& operator<<(std::ostream& os, const Event& e)
     using std::chrono::duration_cast;
     using std::chrono::microseconds;
 
-    os << "latency: " << e.packet->context->packet_number << " rule tree ";
+    os << "packet " << e.packet->context->packet_number << " rule tree ";
 
     switch ( e.type )
     {
@@ -186,24 +186,21 @@ template<typename Clock = SnortClock, typename RuleTree = DefaultRuleInterface>
 class Impl
 {
 public:
-    Impl(const ConfigWrapper&, EventHandler&, EventHandler&);
+    Impl(const ConfigWrapper&, EventHandler&);
 
     bool push(detection_option_tree_root_t*, Packet*);
     bool pop();
     bool suspended() const;
 
 private:
-    void handle(const Event&);
-
     std::vector<RuleTimer<Clock>> timers;
     const ConfigWrapper& config;
     EventHandler& event_handler;
-    EventHandler& log_handler;
 };
 
 template<typename Clock, typename RuleTree>
-inline Impl<Clock, RuleTree>::Impl(const ConfigWrapper& cfg, EventHandler& eh, EventHandler& lh) :
-    config(cfg), event_handler(eh), log_handler(lh)
+inline Impl<Clock, RuleTree>::Impl(const ConfigWrapper& cfg, EventHandler& eh) :
+    config(cfg), event_handler(eh)
 { }
 
 template<typename Clock, typename RuleTree>
@@ -219,7 +216,7 @@ inline bool Impl<Clock, RuleTree>::push(detection_option_tree_root_t* root, Pack
         if ( RuleTree::reenable(*root, config->max_suspend_time, Clock::now()) )
         {
             Event e { Event::EVENT_ENABLED, config->max_suspend_time, root, p };
-            handle(e);
+            event_handler.handle(e);
             return true;
         }
     }
@@ -250,7 +247,7 @@ inline bool Impl<Clock, RuleTree>::pop()
                 timer.elapsed(), timer.root, timer.packet
             };
 
-            handle(e);
+            event_handler.handle(e);
         }
     }
 
@@ -268,16 +265,6 @@ inline bool Impl<Clock, RuleTree>::suspended() const
     return RuleTree::is_suspended(*timers.back().root);
 }
 
-template<typename Clock, typename RuleTree>
-inline void Impl<Clock, RuleTree>::handle(const Event& e)
-{
-    if ( config->action & RuleLatencyConfig::LOG )
-        log_handler.handle(e);
-
-    if ( config->action & RuleLatencyConfig::ALERT )
-        event_handler.handle(e);
-}
-
 // -----------------------------------------------------------------------------
 // static variables
 // -----------------------------------------------------------------------------
@@ -293,6 +280,10 @@ static struct SnortEventHandler : public EventHandler
 {
     void handle(const Event& e) override
     {
+        std::ostringstream ss;
+        ss << e;
+        debug_logf(latency_trace, "%s\n", ss.str().c_str());
+
         switch ( e.type )
         {
             case Event::EVENT_ENABLED:
@@ -309,23 +300,13 @@ static struct SnortEventHandler : public EventHandler
     }
 } event_handler;
 
-static struct SnortLogHandler : public EventHandler
-{
-    void handle(const Event& e) override
-    {
-        std::ostringstream ss;
-        ss << e;
-        LogMessage("%s\n", ss.str().c_str());
-    }
-} log_handler;
-
 static THREAD_LOCAL Impl<>* impl = nullptr;
 
 // FIXIT-L this should probably be put in a tinit
 static inline Impl<>& get_impl()
 {
     if ( !impl )
-        impl = new Impl<>(config, event_handler, log_handler);
+        impl = new Impl<>(config, event_handler);
 
     return *impl;
 }
@@ -461,17 +442,14 @@ TEST_CASE ( "rule latency impl", "[latency]" )
 
     MockConfigWrapper config;
     EventHandlerSpy event_handler;
-    EventHandlerSpy log_handler;
 
     MockClock::reset();
     RuleInterfaceSpy::reset();
 
-    config.config.action = RuleLatencyConfig::ALERT_AND_LOG;
-
     detection_option_tree_root_t root;
     Packet pkt(false);
 
-    rule_latency::Impl<MockClock, RuleInterfaceSpy> impl(config, event_handler, log_handler);
+    rule_latency::Impl<MockClock, RuleInterfaceSpy> impl(config, event_handler);
 
     SECTION( "push" )
     {
@@ -482,7 +460,6 @@ TEST_CASE ( "rule latency impl", "[latency]" )
             SECTION( "push rule" )
             {
                 CHECK_FALSE( impl.push(&root, &pkt) );
-                CHECK( log_handler.count == 0 );
                 CHECK( event_handler.count == 0 );
                 CHECK( RuleInterfaceSpy::reenable_called );
             }
@@ -492,7 +469,6 @@ TEST_CASE ( "rule latency impl", "[latency]" )
                 RuleInterfaceSpy::reenable_result = true;
 
                 CHECK( impl.push(&root, &pkt) );
-                CHECK( log_handler.count == 1 );
                 CHECK( event_handler.count == 1 );
                 CHECK( RuleInterfaceSpy::reenable_called );
             }
@@ -505,7 +481,6 @@ TEST_CASE ( "rule latency impl", "[latency]" )
             SECTION( "push rule" )
             {
                 CHECK_FALSE( impl.push(&root, &pkt) );
-                CHECK( log_handler.count == 0 );
                 CHECK( event_handler.count == 0 );
                 CHECK_FALSE( RuleInterfaceSpy::reenable_called );
             }
@@ -550,7 +525,6 @@ TEST_CASE ( "rule latency impl", "[latency]" )
                 RuleInterfaceSpy::is_suspended_result = true;
 
                 CHECK_FALSE( impl.pop() );
-                CHECK( log_handler.count == 0 );
                 CHECK( event_handler.count == 0 );
                 CHECK_FALSE( RuleInterfaceSpy::timeout_and_suspend_called );
             }
@@ -561,7 +535,6 @@ TEST_CASE ( "rule latency impl", "[latency]" )
                 RuleInterfaceSpy::timeout_and_suspend_result = true;
 
                 CHECK( impl.pop() );
-                CHECK( log_handler.count == 1 );
                 CHECK( event_handler.count == 1 );
                 CHECK( RuleInterfaceSpy::timeout_and_suspend_called );
             }
@@ -571,7 +544,6 @@ TEST_CASE ( "rule latency impl", "[latency]" )
                 RuleInterfaceSpy::timeout_and_suspend_result = false;
 
                 CHECK( impl.pop() );
-                CHECK( log_handler.count == 1 );
                 CHECK( event_handler.count == 1 );
                 CHECK( RuleInterfaceSpy::timeout_and_suspend_called );
             }
@@ -584,7 +556,6 @@ TEST_CASE ( "rule latency impl", "[latency]" )
             RuleInterfaceSpy::is_suspended_result = false;
 
             CHECK_FALSE( impl.pop() );
-            CHECK( log_handler.count == 0 );
             CHECK( event_handler.count == 0 );
             CHECK_FALSE( RuleInterfaceSpy::timeout_and_suspend_called );
         }

diff --git a/src/latency/rule_latency_config.h b/src/latency/rule_latency_config.h
index f88f79100a97ba8d3ad034095d443c5d60d44045..e47afd446e7b6332618ff9022ca1d2afb21efa37 100644 (file)
--- a/src/latency/rule_latency_config.h
+++ b/src/latency/rule_latency_config.h
@@ -25,19 +25,10 @@
 
 struct RuleLatencyConfig
 {
-    enum Action
-    {
-        NONE = 0x00,
-        ALERT = 0x01,
-        LOG = 0x02,
-        ALERT_AND_LOG = ALERT | LOG
-    };
-
     hr_duration max_time = 0_ticks;
     bool suspend = false;
     unsigned suspend_threshold = 0;
     hr_duration max_suspend_time = 0_ticks;
-    Action action = NONE;
 
     bool enabled() const { return max_time > 0_ticks; }
     bool allow_reenable() const { return max_suspend_time > 0_ticks; }

diff --git a/tools/snort2lua/config_states/config_ppm.cc b/tools/snort2lua/config_states/config_ppm.cc
index b06aeb9a580cd3c34d269ea21acd47bebff832e3..9c352dd7693be224f8e33ccb55a18e123752727e 100644 (file)
--- a/tools/snort2lua/config_states/config_ppm.cc
+++ b/tools/snort2lua/config_states/config_ppm.cc
@@ -47,17 +47,12 @@ bool Ppm::convert(std::istringstream& data_stream)
     while (data_stream >> keyword)
     {
         bool tmpval = true;
-        bool popped_comma;
-
+        bool popped_comma = false;
         if (keyword.back() == ',')
         {
             keyword.pop_back();
             popped_comma = true;
         }
-        else
-        {
-            popped_comma = false;
-        }
 
         if (keyword.empty())
             continue;
@@ -125,67 +120,14 @@ bool Ppm::convert(std::istringstream& data_stream)
             table_api.close_table();
         }
 
-        else if (keyword == "pkt-log")
-        {
-            table_api.add_diff_option_comment("pkt-log", "packet.action");
-            table_api.open_table("packet");
-
-            std::string opt1;
-            std::string opt2;
-
-            if (popped_comma)
-                table_api.add_option("action", "log");
-
-            else if (!(data_stream >> opt1))
-                table_api.add_option("action", "log");
-
-            else if (opt1.back() == ',')
-            {
-                opt1.pop_back();
-                tmpval = table_api.add_option("action", opt1);
-            }
-
-            else if (!(data_stream >> opt2))
-                tmpval = table_api.add_option("action", opt1);
-
-            else
-            {
-                table_api.add_diff_option_comment("'both'", "'alert_and_log'");
-                tmpval = table_api.add_option("action", "alert_and_log");
-            }
-
-            table_api.close_table();
-        }
-
-        else if (keyword == "rule-log")
+        else if ((keyword == "pkt-log") or (keyword == "rule-log"))
         {
-            table_api.add_diff_option_comment("rule-log", "rule.action");
-            table_api.open_table("rule");
-
-            std::string opt1;
-            std::string opt2;
-
-            if (!(data_stream >> opt1))
-                tmpval = false;
-
-            else if (opt1.back() == ',')
+            table_api.add_deleted_comment(keyword);
+            if (!popped_comma)
             {
-                opt1.pop_back();
-                tmpval = table_api.add_option("action", opt1);
+                while ((data_stream >> keyword) && (keyword.back() != ','));
             }
-
-            else if (!(data_stream >> opt2))
-                tmpval = table_api.add_option("action", opt1);
-
-            else
-            {
-                table_api.add_diff_option_comment("'both'", "'alert_and_log'");
-                tmpval = table_api.add_option("action", "alert_and_log");
-            }
-
-            table_api.close_table();
         }
-
         else
             tmpval = false;