Pull request #3212: Call splitter finish() on end-of-flow data, on a FIN packet.

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

Squashed commit of the following:

commit 638c0494ccdf566b1f82605d43c29c2c24c58527
Author: Silviu Minut <sminut@cisco.com>
Date:   Thu Dec 9 11:41:14 2021 -0500

    stream_tcp: fix a bug in which in some cases we did not call splitter finish() in each direction, by calling flush_queued_segments() in perform_fin_recv_flush() on FIN with data packets

    stream: defer flush_queued_segments() if flow->clouseau

    stream_tcp: introduce TcpStreamTracker::delayed_finish_flag and call splitter finish from flush_on_data_policy if delayed_finish_flag is true

    stream_tcp: better place for setting delayed_finish_flag
                call flush_queued_segments() rather than splitter_finish() directly, from flush_on_data_policy()

    stream_tcp: wrap flow->clouseau in searching_for_service()

diff --git a/src/flow/flow.h b/src/flow/flow.h
index 36e073bcd140e51c3ad1d0d07a3e20e1d27d2329..356d188bb6f5d5dae3ebf4c46332dad25ee3dc40 100644 (file)
--- a/src/flow/flow.h
+++ b/src/flow/flow.h
@@ -314,6 +314,11 @@ public:
         clouseau = nullptr;
     }
 
+    bool searching_for_service()
+    {
+        return clouseau != nullptr;
+    }
+
     void set_gadget(Inspector* ins)
     {
         gadget = ins;

diff --git a/src/managers/inspector_manager.cc b/src/managers/inspector_manager.cc
index f43453ff701ae1c81a315191bd9fd9b8f91edb48..7d167dfa0ffa50b01ce0d15acb1167a23934c642 100644 (file)
--- a/src/managers/inspector_manager.cc
+++ b/src/managers/inspector_manager.cc
@@ -1676,7 +1676,8 @@ void InspectorManager::full_inspection(Packet* p)
 {
     Flow* flow = p->flow;
 
-    if ( flow->service and flow->clouseau and (!(p->is_cooked()) or p->is_defrag()) )
+    if ( flow->service and flow->searching_for_service()
+         and (!(p->is_cooked()) or p->is_defrag()) )
         bumble(p);
 
     // For reassembled PDUs, a null data buffer signals no detection. Detection can be required

diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc
index 89d6015107ebc8751afaebf0a1065ea6223a37ab..5e463debfc685b5d3c4f570284dbf1f078bc1cb6 100644 (file)
--- a/src/stream/tcp/tcp_reassembler.cc
+++ b/src/stream/tcp/tcp_reassembler.cc
@@ -1055,7 +1055,11 @@ int TcpReassembler::flush_on_data_policy(TcpReassemblerState& trs, Packet* p)
                 flags = get_forward_packet_dir(trs, p);
                 int32_t flush_amt = scan_data_pre_ack(trs, &flags, p);
                 if ( flush_amt <= 0 )
+                {
+                    if (trs.tracker->delayed_finish())
+                        flush_queued_segments(trs, p->flow, true, p);
                     break;
+                }
 
                 flushed += flush_to_seq(trs, flush_amt, p, flags);
             }
@@ -1142,7 +1146,11 @@ int TcpReassembler::flush_on_ack_policy(TcpReassemblerState& trs, Packet* p)
             flags = get_reverse_packet_dir(trs, p);
             flush_amt = scan_data_post_ack(trs, &flags, p);
             if ( flush_amt <= 0 or trs.paf_state.paf == StreamSplitter::SKIP )
+            {
+                if (trs.tracker->delayed_finish())
+                    flush_queued_segments(trs, p->flow, true, p);
                 break;
+            }
 
             if ( trs.paf_state.paf == StreamSplitter::ABORT )
                 trs.tracker->splitter_finish(p->flow);

diff --git a/src/stream/tcp/tcp_state_fin_wait1.cc b/src/stream/tcp/tcp_state_fin_wait1.cc
index 5e6d8140aa36fcd117094f07c33854087551cd11..aaba9c7f322c2e58ec1eb9376d2885d979d8c223 100644 (file)
--- a/src/stream/tcp/tcp_state_fin_wait1.cc
+++ b/src/stream/tcp/tcp_state_fin_wait1.cc
@@ -92,23 +92,22 @@ bool TcpStateFinWait1::fin_sent(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk
 bool TcpStateFinWait1::fin_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk)
 {
     Flow* flow = tsd.get_flow();
+    bool is_ack_valid = false;
 
     trk.update_tracker_ack_recv(tsd);
-    if ( trk.update_on_fin_recv(tsd) )
+    if ( SEQ_GEQ(tsd.get_end_seq(), trk.r_win_base) and
+         check_for_window_slam(tsd, trk, &is_ack_valid) )
     {
-        bool is_ack_valid = false;
-        if ( check_for_window_slam(tsd, trk, &is_ack_valid) )
-        {
-            trk.perform_fin_recv_flush(tsd);
+        trk.perform_fin_recv_flush(tsd);
+        trk.update_on_fin_recv(tsd);
 
-            if ( !flow->two_way_traffic() )
-                trk.set_tf_flags(TF_FORCE_FLUSH);
+        if ( !flow->two_way_traffic() )
+            trk.set_tf_flags(TF_FORCE_FLUSH);
 
-            if ( is_ack_valid )
-                trk.set_tcp_state(TcpStreamTracker::TCP_TIME_WAIT);
-            else
-                trk.set_tcp_state(TcpStreamTracker::TCP_CLOSING);
-        }
+        if ( is_ack_valid )
+            trk.set_tcp_state(TcpStreamTracker::TCP_TIME_WAIT);
+        else
+            trk.set_tcp_state(TcpStreamTracker::TCP_CLOSING);
     }
     return true;
 }

diff --git a/src/stream/tcp/tcp_state_fin_wait2.cc b/src/stream/tcp/tcp_state_fin_wait2.cc
index 10f1bc9bdbbe3f54e1160a3301d42ddbc7a3bdd4..702543ce1b63fafd11f7d4a5def0e0da3e37a0eb 100644 (file)
--- a/src/stream/tcp/tcp_state_fin_wait2.cc
+++ b/src/stream/tcp/tcp_state_fin_wait2.cc
@@ -106,9 +106,10 @@ bool TcpStateFinWait2::fin_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk
     Flow* flow = tsd.get_flow();
 
     trk.update_tracker_ack_recv(tsd);
-    if ( trk.update_on_fin_recv(tsd) )
+    if ( SEQ_GEQ(tsd.get_end_seq(), trk.r_win_base) )
     {
         trk.perform_fin_recv_flush(tsd);
+        trk.update_on_fin_recv(tsd);
 
         if ( !flow->two_way_traffic() )
             trk.set_tf_flags(TF_FORCE_FLUSH);

diff --git a/src/stream/tcp/tcp_stream_tracker.cc b/src/stream/tcp/tcp_stream_tracker.cc
index 4f3281cb0992d5f76e4bf7074f2e7b6db38bd725..19c0fe49985354130985170967fb39b71d6a2582 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.cc
+++ b/src/stream/tcp/tcp_stream_tracker.cc
@@ -223,6 +223,7 @@ void TcpStreamTracker::init_tcp_state()
     flush_policy = STREAM_FLPOLICY_IGNORE;
     reassembler.reset();
     splitter_finish_flag = false;
+    delayed_finish_flag = false;
 }
 
 //-------------------------------------------------------------------------
@@ -664,8 +665,18 @@ void TcpStreamTracker::perform_fin_recv_flush(TcpSegmentDescriptor& tsd)
 {
     if ( tsd.is_data_segment() )
         session->handle_data_segment(tsd);
-    else if ( flush_policy == STREAM_FLPOLICY_ON_DATA and SEQ_EQ(tsd.get_seq(), rcv_nxt) )
+
+    // If the packet is in-sequence, call finish and final flush on it.
+    // FIXIT-L: what do we do about out-of-sequence packets?
+    if ( flush_policy == STREAM_FLPOLICY_ON_DATA and SEQ_EQ(tsd.get_end_seq(), rcv_nxt) )
+    {
+        if (tsd.get_flow()->searching_for_service())
+        {
+            delayed_finish_flag = true;
+            return;
+        }
         reassembler.flush_queued_segments(tsd.get_flow(), true, tsd.get_pkt());
+    }
 }
 
 uint32_t TcpStreamTracker::perform_partial_flush()

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