From: Michael Altizer (mialtize) <mialtize@cisco.com>
Date: Mon, 18 May 2020 22:00:24 +0000 (+0000)
Subject: Merge pull request #2211 in SNORT/snort3 from ~SMINUT/snort3:hpq_timeout_config to... 
X-Git-Tag: 3.0.1-4~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fa9f51667c828bca73c40f0bcfad28d38e1f5cff;p=thirdparty%2Fsnort3.git

Merge pull request #2211 in SNORT/snort3 from ~SMINUT/snort3:hpq_timeout_config to master

Squashed commit of the following:

commit 2fc74253788d479c939aa17354fdc44c24c4540f
Author: Silviu Minut <sminut@cisco.com>
Date:   Tue May 12 11:57:14 2020 -0400

    stream: move held packet timeout to Stream and support changing it on reload
---

diff --git a/src/stream/base/stream_base.cc b/src/stream/base/stream_base.cc
index 889a0bb69..9d6b08097 100644
--- a/src/stream/base/stream_base.cc
+++ b/src/stream/base/stream_base.cc
@@ -34,6 +34,7 @@
 #include "protocols/packet.h"
 #include "protocols/tcp.h"
 #include "stream/flush_bucket.h"
+#include "stream/tcp/tcp_stream_tracker.h"
 
 #include "stream_ha.h"
 #include "stream_module.h"
@@ -197,6 +198,8 @@ void StreamBase::tinit()
     if ( config.flow_cache_cfg.max_flows > 0 )
         flow_con->init_exp(config.flow_cache_cfg.max_flows);
 
+    TcpStreamTracker::set_held_packet_timeout(config.held_packet_timeout);
+
 #ifdef REG_TEST
     FlushBucket::set(config.footprint);
 #else
@@ -303,11 +306,17 @@ static void base_dtor(Inspector* p)
     delete p;
 }
 
+static void base_tinit()
+{
+    TcpStreamTracker::thread_init();
+}
+
 static void base_tterm()
 {
     // this can't happen sooner because the counts haven't been harvested yet
     delete flow_con;
     flow_con = nullptr;
+    TcpStreamTracker::thread_term();
 }
 
 static const InspectApi base_api =
@@ -330,7 +339,7 @@ static const InspectApi base_api =
     nullptr, // service
     nullptr, // init
     nullptr, // term
-    nullptr, // tinit
+    base_tinit,
     base_tterm,
     base_ctor,
     base_dtor,
diff --git a/src/stream/base/stream_module.cc b/src/stream/base/stream_module.cc
index 0a7df8a8e..0a8450ec9 100644
--- a/src/stream/base/stream_module.cc
+++ b/src/stream/base/stream_module.cc
@@ -29,6 +29,8 @@
 #include "main/snort.h"
 #include "main/snort_config.h"
 #include "stream/flush_bucket.h"
+#include "stream/tcp/tcp_stream_tracker.h"
+#include "time/packet_time.h"
 #include "trace/trace.h"
 
 using namespace snort;
@@ -79,6 +81,9 @@ static const Parameter s_params[] =
     { "pruning_timeout", Parameter::PT_INT, "1:max32", "30",
                     "minimum inactive time before being eligible for pruning" },
 
+    { "held_packet_timeout", Parameter::PT_INT, "1:max32", "1000",
+      "timeout in milliseconds for held packets" },
+
     FLOW_TYPE_TABLE("ip_cache",   "ip",   ip_params),
     FLOW_TYPE_TABLE("icmp_cache", "icmp", icmp_params),
     FLOW_TYPE_TABLE("tcp_cache",  "tcp",  tcp_params),
@@ -168,6 +173,11 @@ bool StreamModule::set(const char* fqn, Value& v, SnortConfig* c)
         config.flow_cache_cfg.pruning_timeout = v.get_uint32();
         return true;
     }
+    else if ( v.is("held_packet_timeout") )
+    {
+        config.held_packet_timeout = v.get_uint32();
+        return true;
+    }
     else if ( strstr(fqn, "ip_cache") )
         type = PktType::IP;
     else if ( strstr(fqn, "icmp_cache") )
@@ -196,6 +206,9 @@ bool StreamModule::end(const char*, int, SnortConfig* sc)
     if ( reload_resource_manager.initialize(config) )
         sc->register_reload_resource_tuner(reload_resource_manager);
 
+    if ( hpq_rrt.initialize(config.held_packet_timeout) )
+        sc->register_reload_resource_tuner(hpq_rrt);
+
     return true;
 }
 
@@ -237,6 +250,7 @@ bool StreamReloadResourceManager::tinit()
 {
     int max_flows_change =
         config.flow_cache_cfg.max_flows - flow_con->get_flow_cache_config().max_flows;
+
     if ( max_flows_change )
     {
         if ( max_flows_change < 0 )
@@ -294,3 +308,25 @@ void StreamModuleConfig::show() const
         ConfigLogger::log_value(flow_type_names[i], tmp.c_str());
     }
 }
+
+bool HPQReloadTuner::initialize(uint32_t new_timeout_ms)
+{
+    held_packet_timeout = new_timeout_ms;
+    return Snort::is_reloading();
+}
+
+bool HPQReloadTuner::tinit()
+{
+    packet_gettimeofday(&reload_time);
+    return TcpStreamTracker::adjust_expiration(held_packet_timeout, reload_time);
+}
+
+bool HPQReloadTuner::tune_packet_context()
+{
+    return !TcpStreamTracker::release_held_packets(reload_time, max_work);
+}
+
+bool HPQReloadTuner::tune_idle_context()
+{
+    return !TcpStreamTracker::release_held_packets(reload_time, max_work_idle);
+}
diff --git a/src/stream/base/stream_module.h b/src/stream/base/stream_module.h
index 671103f31..7b494ef60 100644
--- a/src/stream/base/stream_module.h
+++ b/src/stream/base/stream_module.h
@@ -79,6 +79,7 @@ struct StreamModuleConfig
 #ifdef REG_TEST
     unsigned footprint = 0;
 #endif
+    uint32_t held_packet_timeout = 1000;  // in milliseconds
 
     void show() const;
 };
@@ -101,6 +102,21 @@ private:
     StreamModuleConfig config;
 };
 
+class HPQReloadTuner : public snort::ReloadResourceTuner
+{
+public:
+    HPQReloadTuner() = default;
+
+    bool tinit() override;
+    bool tune_packet_context() override;
+    bool tune_idle_context() override;
+    bool initialize(uint32_t new_timeout_ms);
+
+private:
+    uint32_t held_packet_timeout;
+    timeval reload_time;
+};
+
 class StreamModule : public snort::Module
 {
 public:
@@ -135,6 +151,7 @@ public:
 private:
     StreamModuleConfig config;
     StreamReloadResourceManager reload_resource_manager;
+    HPQReloadTuner hpq_rrt;
 };
 
 extern void base_prep();
diff --git a/src/stream/tcp/held_packet_queue.cc b/src/stream/tcp/held_packet_queue.cc
index 2c5656509..889a3f25e 100644
--- a/src/stream/tcp/held_packet_queue.cc
+++ b/src/stream/tcp/held_packet_queue.cc
@@ -31,6 +31,16 @@
 
 using namespace snort;
 
+void HeldPacket::adjust_expiration(const timeval& delta, bool up)
+{
+    timeval new_expiration;
+    if ( up )
+        timeradd(&expiration, &delta, &new_expiration);
+    else
+        timersub(&expiration, &delta, &new_expiration);
+    expiration = new_expiration;
+}
+
 HeldPacket::HeldPacket(DAQ_Msg_h msg, uint32_t seq, const timeval& exp, TcpStreamTracker& trk)
     : daq_msg(msg), seq_num(seq), expiration(exp), tracker(trk), expired(false)
 {
@@ -52,7 +62,7 @@ void HeldPacketQueue::erase(iter_t it)
     q.erase(it);
 }
 
-void HeldPacketQueue::execute(const timeval& cur_time, int max_remove)
+bool HeldPacketQueue::execute(const timeval& cur_time, int max_remove)
 {
     while ( !q.empty() && (max_remove < 0 || max_remove--) )
     {
@@ -66,4 +76,38 @@ void HeldPacketQueue::execute(const timeval& cur_time, int max_remove)
         else
             break;
     }
+
+    return !q.empty() && q.front().has_expired(cur_time);
+}
+
+bool HeldPacketQueue::adjust_expiration(uint32_t new_timeout, const timeval& now)
+{
+    if ( q.empty() )
+        return false;
+
+    uint32_t ms;
+    bool up;
+
+    uint32_t old_timeout = get_timeout();
+    set_timeout(new_timeout);
+
+    if ( new_timeout < old_timeout )
+    {
+        ms = old_timeout - new_timeout;
+        up = false;
+    }
+    else if ( new_timeout > old_timeout )
+    {
+        ms = new_timeout - old_timeout;
+        up = true;
+    }
+    else
+        return false;
+
+    timeval delta = { static_cast<time_t>(ms) / 1000, static_cast<suseconds_t>((ms % 1000) * 1000) };
+
+    for ( auto& hp : q )
+        hp.adjust_expiration(delta, up);
+
+    return q.front().has_expired(now);
 }
diff --git a/src/stream/tcp/held_packet_queue.h b/src/stream/tcp/held_packet_queue.h
index ded1c8e70..fae51114a 100644
--- a/src/stream/tcp/held_packet_queue.h
+++ b/src/stream/tcp/held_packet_queue.h
@@ -48,6 +48,7 @@ public:
     TcpStreamTracker& get_tracker() const { return tracker; }
     DAQ_Msg_h get_daq_msg() const { return daq_msg; }
     uint32_t get_seq_num() const { return seq_num; }
+    void adjust_expiration(const timeval& delta, bool up);
 
 private:
     DAQ_Msg_h daq_msg;
@@ -66,15 +67,27 @@ public:
 
     iter_t append(DAQ_Msg_h msg, uint32_t seq, TcpStreamTracker& trk);
     void erase(iter_t it);
-    void execute(const timeval& cur_time, int max_remove);
+
+    // Return whether there still are expired packets in the queue.
+    bool execute(const timeval& cur_time, int max_remove);
 
     void set_timeout(uint32_t ms)
     {
-        timeout.tv_sec = ms / 1000;
-        timeout.tv_usec = static_cast<suseconds_t>((ms % 1000) * 1000);
+        timeout = { static_cast<time_t>(ms) / 1000, static_cast<suseconds_t>((ms % 1000) * 1000) };
+    }
+
+    // Return the timeout in milliseconds.
+    uint32_t get_timeout() const
+    {
+        return timeout.tv_sec * 1000 + timeout.tv_usec / 1000;
     }
+
     bool empty() const { return q.empty(); }
 
+    // This must be called at reload time only, with now = reload time.
+    // Return true if, upon exit, there are expired packets in the queue.
+    bool adjust_expiration(uint32_t new_timeout_ms, const timeval& now);
+
 private:
     timeval timeout = {1, 0};
     list_t q;
diff --git a/src/stream/tcp/stream_tcp.cc b/src/stream/tcp/stream_tcp.cc
index ba647e0d5..5fd23df43 100644
--- a/src/stream/tcp/stream_tcp.cc
+++ b/src/stream/tcp/stream_tcp.cc
@@ -29,7 +29,6 @@
 #include "tcp_session.h"
 #include "tcp_reassemblers.h"
 #include "tcp_state_machine.h"
-#include "tcp_stream_tracker.h"
 
 using namespace snort;
 
@@ -77,7 +76,6 @@ bool StreamTcp::configure(SnortConfig* sc)
 
 void StreamTcp::tinit()
 {
-    TcpStreamTracker::set_held_packet_timeout(config->held_packet_timeout);
     TcpHAManager::tinit();
     TcpSession::sinit();
 }
@@ -119,16 +117,6 @@ static Inspector* tcp_ctor(Module* m)
 static void tcp_dtor(Inspector* p)
 { delete p; }
 
-static void stream_tcp_tinit()
-{
-    TcpStreamTracker::thread_init();
-}
-
-static void stream_tcp_tterm()
-{
-    TcpStreamTracker::thread_term();
-}
-
 static void stream_tcp_pinit()
 {
     TcpStateMachine::initialize();
@@ -166,8 +154,8 @@ static const InspectApi tcp_api =
     nullptr,  // service
     stream_tcp_pinit,  // pinit
     stream_tcp_pterm,  // pterm
-    stream_tcp_tinit,  // tinit,
-    stream_tcp_tterm,  // tterm,
+    nullptr,  // tinit,
+    nullptr,  // tterm,
     tcp_ctor,
     tcp_dtor,
     tcp_ssn,
diff --git a/src/stream/tcp/tcp_module.cc b/src/stream/tcp/tcp_module.cc
index d5bdc687d..4cfcd0744 100644
--- a/src/stream/tcp/tcp_module.cc
+++ b/src/stream/tcp/tcp_module.cc
@@ -197,9 +197,6 @@ static const Parameter s_params[] =
     { "track_only", Parameter::PT_BOOL, nullptr, "false",
       "disable reassembly if true" },
 
-    { "held_packet_timeout", Parameter::PT_INT, "1:max32", "1000",
-      "timeout in milliseconds for held packets" },
-
     { nullptr, Parameter::PT_MAX, nullptr, nullptr, nullptr }
 };
 
@@ -323,10 +320,6 @@ bool StreamTcpModule::set(const char*, Value& v, SnortConfig*)
         else
             config->flags &= ~STREAM_CONFIG_NO_REASSEMBLY;
     }
-    else if ( v.is("held_packet_timeout") )
-    {
-        config->held_packet_timeout = v.get_uint32();
-    }
     else
         return false;
 
diff --git a/src/stream/tcp/tcp_stream_config.h b/src/stream/tcp/tcp_stream_config.h
index 3dd60e6d7..40595a5a3 100644
--- a/src/stream/tcp/tcp_stream_config.h
+++ b/src/stream/tcp/tcp_stream_config.h
@@ -71,8 +71,6 @@ public:
     int hs_timeout = -1;
 
     bool no_ack;
-
-    uint32_t held_packet_timeout = 1000;  // in milliseconds
 };
 
 #endif
diff --git a/src/stream/tcp/tcp_stream_tracker.cc b/src/stream/tcp/tcp_stream_tracker.cc
index 120ef09cc..cd6f86566 100644
--- a/src/stream/tcp/tcp_stream_tracker.cc
+++ b/src/stream/tcp/tcp_stream_tracker.cc
@@ -746,10 +746,12 @@ void TcpStreamTracker::finalize_held_packet(Flow* flow)
     }
 }
 
-void TcpStreamTracker::release_held_packets(const timeval& cur_time, int max_remove)
+bool TcpStreamTracker::release_held_packets(const timeval& cur_time, int max_remove)
 {
+    bool is_front_expired = false;
     if ( hpq )
-        hpq->execute(cur_time, max_remove);
+        is_front_expired = hpq->execute(cur_time, max_remove);
+    return is_front_expired;
 }
 
 void TcpStreamTracker::set_held_packet_timeout(const uint32_t ms)
@@ -758,6 +760,12 @@ void TcpStreamTracker::set_held_packet_timeout(const uint32_t ms)
     hpq->set_timeout(ms);
 }
 
+bool TcpStreamTracker::adjust_expiration(uint32_t new_timeout_ms, const timeval& now)
+{
+    assert(hpq);
+    return hpq->adjust_expiration(new_timeout_ms, now);
+}
+
 void TcpStreamTracker::thread_init()
 {
     assert(!hpq);
diff --git a/src/stream/tcp/tcp_stream_tracker.h b/src/stream/tcp/tcp_stream_tracker.h
index b6dbf44cc..7ccab1a72 100644
--- a/src/stream/tcp/tcp_stream_tracker.h
+++ b/src/stream/tcp/tcp_stream_tracker.h
@@ -292,9 +292,11 @@ public:
     void finalize_held_packet(snort::Flow*);
     uint32_t perform_partial_flush();
 
-    // max_remove < 0 means time out all eligible packets
-    static void release_held_packets(const timeval& cur_time, int max_remove);
+    // max_remove < 0 means time out all eligible packets.
+    // Return whether there are more packets that need to be released.
+    static bool release_held_packets(const timeval& cur_time, int max_remove);
     static void set_held_packet_timeout(const uint32_t ms);
+    static bool adjust_expiration(uint32_t new_timeout_ms, const timeval& now);
     static void thread_init();
     static void thread_term();