Pull request #3237: Single finish2

Merge in SNORT/snort3 from ~SMINUT/snort3:single_finish2 to master

Squashed commit of the following:

commit 56d6b7e2091d7752f955af1a2d4cc97c18e19bd0
Author: Silviu Minut <sminut@cisco.com>
Date:   Thu Jan 13 20:15:50 2022 -0500

    stream_tcp: ensure that we call splitter finish() only once per flow, per direction

diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc
index c8ffe48ee208e70b7c9f9f350554a0c7df855d63..f46f3defe6ee896161dcbb1ea35a45b9d3941d60 100644 (file)
--- a/src/stream/tcp/tcp_reassembler.cc
+++ b/src/stream/tcp/tcp_reassembler.cc
@@ -787,7 +787,7 @@ void TcpReassembler::flush_queued_segments(
     TcpReassemblerState& trs, Flow* flow, bool clear, Packet* p)
 {
     bool pending = clear and paf_initialized(&trs.paf_state)
-        and (!trs.tracker->get_splitter() || trs.tracker->get_splitter()->finish(flow) );
+        and trs.tracker->splitter_finish(flow);
 
     if ( pending and !(flow->ssn_state.ignore_direction & trs.ignore_dir) )
         final_flush(trs, p, trs.packet_dir);
@@ -1145,7 +1145,7 @@ int TcpReassembler::flush_on_ack_policy(TcpReassemblerState& trs, Packet* p)
                 break;
 
             if ( trs.paf_state.paf == StreamSplitter::ABORT )
-                trs.tracker->get_splitter()->finish(p->flow);
+                trs.tracker->splitter_finish(p->flow);
 
             // for consistency with other cases, should return total
             // but that breaks flushing pipelined pdus

diff --git a/src/stream/tcp/tcp_stream_tracker.cc b/src/stream/tcp/tcp_stream_tracker.cc
index d370b008a74926b673834e17c5ebd9aa1b64ba47..156f683f165b489164503bb896bfc76a4f456503 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.cc
+++ b/src/stream/tcp/tcp_stream_tracker.cc
@@ -222,6 +222,7 @@ void TcpStreamTracker::init_tcp_state()
     held_packet = null_iterator;
     flush_policy = STREAM_FLPOLICY_IGNORE;
     reassembler.reset();
+    splitter_finish_flag = false;
 }
 
 //-------------------------------------------------------------------------
@@ -264,6 +265,21 @@ void TcpStreamTracker::set_splitter(const Flow* flow)
         set_splitter(new AtomSplitter(!client_tracker) );
 }
 
+bool TcpStreamTracker::splitter_finish(snort::Flow* flow)
+{
+    if (!splitter)
+        return true;
+
+    if (!splitter_finish_flag)
+    {
+        splitter_finish_flag = true;
+        return splitter->finish(flow);
+    }
+    // there shouldn't be any un-flushed data beyond this point,
+    // returning false here, discards it
+    return false;
+}
+
 void TcpStreamTracker::init_on_syn_sent(TcpSegmentDescriptor& tsd)
 {
     tsd.get_flow()->set_session_flags(SSNFLAG_SEEN_CLIENT);

diff --git a/src/stream/tcp/tcp_stream_tracker.h b/src/stream/tcp/tcp_stream_tracker.h
index b0f0708fc2dc593cb8125e13162d04890e9f0070..5d5b8a5c14be457aa8af6edca070a84178694cc7 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.h
+++ b/src/stream/tcp/tcp_stream_tracker.h
@@ -261,6 +261,8 @@ public:
     bool is_splitter_paf() const
     { return splitter && splitter->is_paf(); }
 
+    bool splitter_finish(snort::Flow* flow);
+
     bool is_reassembly_enabled() const
     { return  ( splitter and (flush_policy != STREAM_FLPOLICY_IGNORE) ); }
 
@@ -354,6 +356,7 @@ protected:
     FlushPolicy flush_policy = STREAM_FLPOLICY_IGNORE;
     bool mac_addr_valid = false;
     bool fin_seq_set = false;  // FIXIT-M should be obviated by tcp state
+    bool splitter_finish_flag = false;
 };
 
 // <--- note -- the 'state' parameter must be a reference