Merge pull request #2091 in SNORT/snort3 from ~MIALTIZE/snort3:detained_fixups to...

Squashed commit of the following:

commit 1444a851fb7c3714995441a3cd6385f38e733e5e
Author: Michael Altizer <mialtize@cisco.com>
Date:   Tue Mar 24 12:03:33 2020 -0400

    stream_tcp: Cancel hold requests on the current packet when flushing

commit 16ab4c97c5342e893a2ab5dc6b50b4c7a909fd79
Author: Michael Altizer <mialtize@cisco.com>
Date:   Thu Mar 19 13:52:16 2020 -0400

    active: Move packet hold realization for Stream detainment to verdict handling

commit 3e5d373c511b04dd2fcd61937c75e5ae490bd407
Author: Michael Altizer <mialtize@cisco.com>
Date:   Thu Mar 19 11:24:46 2020 -0400

    active: Base hold_packet() decision on DAQ message pool usage

    This change cascades into TcpStreamTracker's hold packet logic.

commit d61ce2dc2ba3d30bd8347ed6b7885e5bd5699e8a
Author: Michael Altizer <mialtize@cisco.com>
Date:   Thu Mar 19 09:34:10 2020 -0400

    stream_tcp: Finalize held packets in TcpSession::clear_session()

    This ensures that held packets are released even if a flow is cleared
    without cleanup (as in prune conditions).

diff --git a/src/main/analyzer.cc b/src/main/analyzer.cc
index bb37d83a743bed60d944a198a76c6b2a3c584aab..a2023e74571e03ce3fba170a8c3e17cd767673c3 100644 (file)
--- a/src/main/analyzer.cc
+++ b/src/main/analyzer.cc
@@ -339,7 +339,7 @@ void Analyzer::post_process_daq_pkt_msg(Packet* p)
         retry_queue->put(p->daq_msg);
         daq_stats.retries_queued++;
     }
-    else if (!p->active->is_packet_held())
+    else if (!p->active->is_packet_held() || !Stream::set_packet_action_to_hold(p))
         verdict = distill_verdict(p);
 
     if (PacketTracer::is_active())

diff --git a/src/main/modules.cc b/src/main/modules.cc
index b21043ce1d114f7eec9548ef4b83b26a0ff6b871..2e348583defaa128e11518fc7b70ff40bdfc9f90 100644 (file)
--- a/src/main/modules.cc
+++ b/src/main/modules.cc
@@ -831,6 +831,8 @@ static PegInfo active_pegs[]
     { CountType::SUM, "failed_injects", "total crafted packet encode + injects that failed" },
     { CountType::SUM, "direct_injects", "total crafted packets directly injected" },
     { CountType::SUM, "failed_direct_injects", "total crafted packet direct injects that failed" },
+    { CountType::SUM, "holds_denied", "total number of packet hold requests denied" },
+    { CountType::SUM, "holds_canceled", "total number of packet hold requests canceled" },
     { CountType::END, nullptr, nullptr }
 };
 

diff --git a/src/packet_io/active.cc b/src/packet_io/active.cc
index 1ca1a1a3decb10d1fb5711f402b3205619561548..452c3bdb745e57932a1b6a796b066508370f51de 100644 (file)
--- a/src/packet_io/active.cc
+++ b/src/packet_io/active.cc
@@ -586,15 +586,28 @@ bool Active::retry_packet(const Packet* p)
     return true;
 }
 
-bool Active::hold_packet(const Packet*)
+bool Active::hold_packet(const Packet* p)
 {
-    if ( active_action < ACT_HOLD )
+    if (active_action >= ACT_HOLD)
+        return false;
+
+    // FIXIT-L same semi-arbitrary heuristic as the retry queue logic - reevaluate later
+    if (!p->daq_instance || p->daq_instance->get_pool_available() < p->daq_instance->get_batch_size())
     {
-        active_action = ACT_HOLD;
-        return true;
+        active_counts.holds_denied++;
+        return false;
     }
 
-    return false;
+    active_action = ACT_HOLD;
+
+    return true;
+}
+
+void Active::cancel_packet_hold()
+{
+    assert(active_action == ACT_HOLD);
+    active_counts.holds_canceled++;
+    active_action = ACT_PASS;
 }
 
 void Active::allow_session(Packet* p)

diff --git a/src/packet_io/active.h b/src/packet_io/active.h
index f67360390ce32893479bc046d00fa4333a7d4d43..1ae4ba37b3d1ae68700495e6206637677f442881 100644 (file)
--- a/src/packet_io/active.h
+++ b/src/packet_io/active.h
@@ -42,6 +42,8 @@ public:
         PegCount failed_injects;
         PegCount direct_injects;
         PegCount failed_direct_injects;
+        PegCount holds_denied;
+        PegCount holds_canceled;
     };
 
     enum ActiveStatus : uint8_t
@@ -94,6 +96,7 @@ public:
     void daq_drop_packet(const Packet*);
     bool retry_packet(const Packet*);
     bool hold_packet(const Packet*);
+    void cancel_packet_hold();
 
     void allow_session(Packet*);
     void block_session(Packet*, bool force = false);

diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc
index 587fd7eafd4bfcce8e93af380cf140a7d7b07153..d9ed4e59bb2a42617b3434eb30c6df6e587ee15b 100644 (file)
--- a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc
+++ b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc
@@ -21,6 +21,8 @@
 #include "config.h"
 #endif
 
+#include "packet_io/active.h"
+
 #include "http_common.h"
 #include "http_cutter.h"
 #include "http_enum.h"
@@ -28,7 +30,6 @@
 #include "http_module.h"
 #include "http_stream_splitter.h"
 #include "http_test_input.h"
-#include "stream/stream.h"
 
 using namespace snort;
 using namespace HttpCommon;
@@ -125,7 +126,7 @@ void HttpStreamSplitter::detain_packet(Packet* pkt)
     }
     if (!HttpTestManager::use_test_input(HttpTestManager::IN_HTTP))
 #endif
-    Stream::set_packet_action_to_hold(pkt);
+    pkt->active->hold_packet(pkt);
     HttpModule::increment_peg_counts(PEG_DETAINED);
 }
 

diff --git a/src/stream/tcp/tcp_module.cc b/src/stream/tcp/tcp_module.cc
index c8a709667305d20c9a1db1ece94225f85df703b1..0b339f2ba4833e14502515e41e6f550cd23368b3 100644 (file)
--- a/src/stream/tcp/tcp_module.cc
+++ b/src/stream/tcp/tcp_module.cc
@@ -83,7 +83,6 @@ const PegInfo tcp_pegs[] =
     { CountType::SUM, "held_packets_passed", "number of held packets passed" },
     { CountType::NOW, "cur_packets_held", "number of packets currently held" },
     { CountType::MAX, "max_packets_held", "maximum number of packets held simultaneously" },
-    { CountType::SUM, "held_packet_limit_exceeded", "number of times limit of max held packets exceeded" },
     { CountType::SUM, "partial_flushes", "number of partial flushes initiated" },
     { CountType::SUM, "partial_flush_bytes", "partial flush total bytes" },
     { CountType::END, nullptr, nullptr }

diff --git a/src/stream/tcp/tcp_module.h b/src/stream/tcp/tcp_module.h
index 4587358fe7f0cfde9089b1adc1ad0de573827c07..566e3f0a4b2fad761dc3c0f928e7c9ac6ba6da00 100644 (file)
--- a/src/stream/tcp/tcp_module.h
+++ b/src/stream/tcp/tcp_module.h
@@ -97,7 +97,6 @@ struct TcpStats
     PegCount held_packets_passed;
     PegCount current_packets_held;
     PegCount max_packets_held;
-    PegCount held_packet_limit_exceeded;
     PegCount partial_flushes;
     PegCount partial_flush_bytes;
 };

diff --git a/src/stream/tcp/tcp_session.cc b/src/stream/tcp/tcp_session.cc
index a6803c02e1afc6885508114c5d27f6b032e98ffc..d186d3cf94eebe194700e3e12b382a9bc38427de 100644 (file)
--- a/src/stream/tcp/tcp_session.cc
+++ b/src/stream/tcp/tcp_session.cc
@@ -169,6 +169,17 @@ void TcpSession::clear_session(bool free_flow_data, bool flush_segments, bool re
         server.reassembler.flush_queued_segments(flow, true, p);
     }
 
+    if ( p )
+    {
+        client.finalize_held_packet(p);
+        server.finalize_held_packet(p);
+    }
+    else
+    {
+        client.finalize_held_packet(flow);
+        server.finalize_held_packet(flow);
+    }
+
     client.reassembler.purge_segment_list();
     server.reassembler.purge_segment_list();
 

diff --git a/src/stream/tcp/tcp_stream_session.cc b/src/stream/tcp/tcp_stream_session.cc
index eb36a9dc891400a321c44ba131cee110f47d02ea..5a203bad0f93653eb31d12f0bad4dbeee5a9807b 100644 (file)
--- a/src/stream/tcp/tcp_stream_session.cc
+++ b/src/stream/tcp/tcp_stream_session.cc
@@ -355,16 +355,6 @@ void TcpStreamSession::cleanup(Packet* p)
     clear_session(true, true, false, p);
     client.normalizer.reset();
     server.reassembler.reset();
-    if ( p )
-    {
-        client.finalize_held_packet(p);
-        server.finalize_held_packet(p);
-    }
-    else
-    {
-        client.finalize_held_packet(flow);
-        server.finalize_held_packet(flow);
-    }
 }
 
 void TcpStreamSession::clear()

diff --git a/src/stream/tcp/tcp_stream_tracker.cc b/src/stream/tcp/tcp_stream_tracker.cc
index dcb2a0df75ea06c855f80744905e2dd21f7386e5..e4226efb864ded0fc8362284ecd9b9bd174b2de4 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.cc
+++ b/src/stream/tcp/tcp_stream_tracker.cc
@@ -655,27 +655,15 @@ bool TcpStreamTracker::is_segment_seq_valid(TcpSegmentDescriptor& tsd)
 
 bool TcpStreamTracker::set_held_packet(Packet* p)
 {
-    // FIXIT-M - limit of packets held per packet thread should be determined based on runtime criteria
-    //           such as # of DAQ Msg buffers, # of threads, etc... for now we use small number like 10
     if ( held_packet )
         return false;
-    if ( tcpStats.current_packets_held >= 50 )
-    {
-        tcpStats.held_packet_limit_exceeded++;
-        return false;
-    }
-
-    if ( p->active->hold_packet(p) )
-    {
-        held_packet = p->daq_msg;
-        held_seq_num = p->ptrs.tcph->seq();
-        tcpStats.total_packets_held++;
-        if ( ++tcpStats.current_packets_held > tcpStats.max_packets_held )
-            tcpStats.max_packets_held = tcpStats.current_packets_held;
-        return true;
-    }
 
-    return false;
+    held_packet = p->daq_msg;
+    held_seq_num = p->ptrs.tcph->seq();
+    tcpStats.total_packets_held++;
+    if ( ++tcpStats.current_packets_held > tcpStats.max_packets_held )
+        tcpStats.max_packets_held = tcpStats.current_packets_held;
+    return true;
 }
 
 bool TcpStreamTracker::is_retransmit_of_held_packet(Packet* cp)
@@ -712,13 +700,17 @@ void TcpStreamTracker::finalize_held_packet(Packet* cp)
         held_seq_num = 0;
         tcpStats.current_packets_held--;
     }
+
+    if (cp->active->is_packet_held())
+        cp->active->cancel_packet_hold();
 }
 
 void TcpStreamTracker::finalize_held_packet(Flow* flow)
 {
     if ( held_packet )
     {
-        if ( flow->ssn_state.session_flags & SSNFLAG_BLOCK )
+        if ( (flow->session_state & STREAM_STATE_BLOCK_PENDING) ||
+             (flow->ssn_state.session_flags & SSNFLAG_BLOCK) )
         {
             Analyzer::get_local_analyzer()->finalize_daq_message(held_packet, DAQ_VERDICT_BLOCK);
             tcpStats.held_packets_dropped++;