Pull request #3257: stream_tcp: call flush_queued_segments() from flush_on_ack_policy()

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

Squashed commit of the following:

commit 77304a6d8f435d8491fa6113108dfb331651f386
Author: Silviu Minut <sminut@cisco.com>
Date:   Mon Feb 28 21:15:18 2022 -0500

    stream_tcp: add fin_i_seq and fin_no_gap() and try to use those together with the existing next_no_gap() to determine whether we are on a gap in the seglist or not, when scanning

commit 15eb71a6197aef4e190cd59083bc6cd4012403b3
Author: Silviu Minut <sminut@cisco.com>
Date:   Fri Feb 25 16:27:26 2022 -0500

    stream_tcp: distinguish between the various non-flush cases when returning from scan_on_data_policy(), so we can call final flush only when the seglist has no gaps; if the seglist has gaps, call final_flush only when the gaps have filled or on session teardown

commit e753368af6f64c501dd67f426c4dd40c005fce46
Author: Silviu Minut <sminut@cisco.com>
Date:   Thu Feb 24 19:26:29 2022 -0500

    stream_tcp: introduce TcpStreamTracker::set_fin_seq_status_seen() and call it before using the fin_seq_status flag in perform_fin_recv_flush()

commit 840f71182a9125660b1742fe190abc2d32303873
Author: Silviu Minut <sminut@cisco.com>
Date:   Wed Feb 2 22:00:39 2022 -0500

    stream_tcp: * call flush_queued_segments() from flush_on_ack_policy() when the splitter did not flush but we are on a FIN
                * fix how fin_seq_status is being set in update_tracker_ack_sent()
    	    * make the pre-ack mode work the same way as post-ack by modifying flush_on_data_policy() accordingly

diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc
index 5e463debfc685b5d3c4f570284dbf1f078bc1cb6..92723ee50b3ac9dc830e24671a2444c9364a7ab3 100644 (file)
--- a/src/stream/tcp/tcp_reassembler.cc
+++ b/src/stream/tcp/tcp_reassembler.cc
@@ -107,6 +107,18 @@ bool TcpReassembler::next_acked_no_gap_c(const TcpSegmentNode& tsn, const TcpRea
         and SEQ_LT(tsn.next->c_seq, trs.tracker->r_win_base);
 }
 
+bool TcpReassembler::fin_no_gap(const TcpSegmentNode& tsn, const TcpReassemblerState& trs)
+{
+    return trs.tracker->fin_seq_status >= TcpStreamTracker::FIN_WITH_SEQ_SEEN
+        and SEQ_GEQ(tsn.i_seq + tsn.i_len, trs.tracker->get_fin_i_seq());
+}
+
+bool TcpReassembler::fin_acked_no_gap(const TcpSegmentNode& tsn, const TcpReassemblerState& trs)
+{
+    return trs.tracker->fin_seq_status >= TcpStreamTracker::FIN_WITH_SEQ_ACKED
+        and SEQ_GEQ(tsn.i_seq + tsn.i_len, trs.tracker->get_fin_i_seq());
+}
+
 void TcpReassembler::update_next(TcpReassemblerState& trs, const TcpSegmentNode& tsn)
 {
     trs.sos.seglist.cur_rseg = next_no_gap(tsn) ?  tsn.next : nullptr;
@@ -628,13 +640,8 @@ uint32_t TcpReassembler::get_q_footprint(TcpReassemblerState& trs)
         footprint = trs.tracker->r_win_base - trs.sos.seglist_base_seq;
 
     if ( footprint )
-    {
         sequenced = get_q_sequenced(trs);
 
-        if ( trs.tracker->fin_seq_status == TcpStreamTracker::FIN_WITH_SEQ_ACKED )
-            --footprint;
-    }
-
     return ( footprint > sequenced ) ? sequenced : footprint;
 }
 
@@ -848,17 +855,21 @@ int32_t TcpReassembler::scan_data_pre_ack(TcpReassemblerState& trs, uint32_t* fl
 {
     assert(trs.sos.session->flow == p->flow);
 
+    int32_t ret_val = FINAL_FLUSH_HOLD;
+
     if ( SEQ_GT(trs.sos.seglist.head->c_seq, trs.sos.seglist_base_seq) )
-        return -1;
+        return ret_val;
 
     if ( !trs.sos.seglist.cur_rseg )
         trs.sos.seglist.cur_rseg = trs.sos.seglist.cur_sseg;
 
     if ( !is_q_sequenced(trs) )
-        return -1;
+        return ret_val;
 
     TcpSegmentNode* tsn = trs.sos.seglist.cur_sseg;
     uint32_t total = tsn->c_seq - trs.sos.seglist_base_seq;
+
+    ret_val = FINAL_FLUSH_OK;
     while ( tsn && *flags )
     {
         total += tsn->c_len;
@@ -869,7 +880,10 @@ int32_t TcpReassembler::scan_data_pre_ack(TcpReassemblerState& trs, uint32_t* fl
         if ( paf_initialized(&trs.paf_state) && SEQ_LEQ(end, pos) )
         {
             if ( !next_no_gap(*tsn) )
+            {
+                ret_val = FINAL_FLUSH_HOLD;
                 break;
+            }
 
             tsn = tsn->next;
             continue;
@@ -890,13 +904,17 @@ int32_t TcpReassembler::scan_data_pre_ack(TcpReassemblerState& trs, uint32_t* fl
         }
 
         if (!next_no_gap(*tsn) || (trs.paf_state.paf == StreamSplitter::STOP))
+        {
+            if ( !(next_no_gap(*tsn) || fin_no_gap(*tsn, trs)) )
+                ret_val = FINAL_FLUSH_HOLD;
             break;
+        }
 
         tsn = tsn->next;
     }
 
     trs.sos.seglist.cur_sseg = tsn;
-    return -1;
+    return ret_val;
 }
 
 static inline bool both_splitters_aborted(Flow* flow)
@@ -965,8 +983,10 @@ int32_t TcpReassembler::scan_data_post_ack(TcpReassemblerState& trs, uint32_t* f
 {
     assert(trs.sos.session->flow == p->flow);
 
+    int32_t ret_val = FINAL_FLUSH_HOLD;
+
     if ( !trs.sos.seglist.cur_sseg || SEQ_GEQ(trs.sos.seglist_base_seq, trs.tracker->r_win_base) )
-        return -1;
+        return ret_val ;
 
     if ( !trs.sos.seglist.cur_rseg )
         trs.sos.seglist.cur_rseg = trs.sos.seglist.cur_sseg;
@@ -987,6 +1007,7 @@ int32_t TcpReassembler::scan_data_post_ack(TcpReassemblerState& trs, uint32_t* f
             total = tsn->c_seq - trs.sos.seglist.cur_rseg->c_seq;
     }
 
+    ret_val = FINAL_FLUSH_OK;
     while (tsn && *flags && SEQ_LT(tsn->c_seq, trs.tracker->r_win_base))
     {
         // only flush acked data that fits in pdu reassembly buffer...
@@ -1025,12 +1046,16 @@ int32_t TcpReassembler::scan_data_post_ack(TcpReassemblerState& trs, uint32_t* f
 
         if (flush_len < tsn->c_len || (splitter->is_paf() and !next_no_gap(*tsn)) ||
             (trs.paf_state.paf == StreamSplitter::STOP))
+        {
+            if ( !(next_no_gap(*tsn) || fin_acked_no_gap(*tsn, trs)) )
+                ret_val = FINAL_FLUSH_HOLD;
             break;
+        }
 
         tsn = tsn->next;
     }
 
-    return -1;
+    return ret_val;
 }
 
 int TcpReassembler::flush_on_data_policy(TcpReassemblerState& trs, Packet* p)
@@ -1050,16 +1075,13 @@ int TcpReassembler::flush_on_data_policy(TcpReassemblerState& trs, Packet* p)
         if ( trs.sos.seglist.head )
         {
             uint32_t flags;
+            int32_t flush_amt;
             do
             {
                 flags = get_forward_packet_dir(trs, p);
-                int32_t flush_amt = scan_data_pre_ack(trs, &flags, p);
+                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);
             }
@@ -1070,6 +1092,15 @@ int TcpReassembler::flush_on_data_policy(TcpReassemblerState& trs, Packet* p)
                 fallback(*trs.tracker, trs.server_side);
                 return flush_on_data_policy(trs, p);
             }
+            else if ( trs.tracker->fin_seq_status >= TcpStreamTracker::FIN_WITH_SEQ_SEEN and
+                -1 <= flush_amt and flush_amt <= 0 and
+                trs.paf_state.paf == StreamSplitter::SEARCH and
+                !p->flow->searching_for_service() )
+            {
+                // we are on a FIN, the data has been scanned, it has no gaps,
+                // but somehow we are waiting for more data - do final flush here
+                flush_queued_segments(trs, p->flow, true, p );
+            }
         }
         break;
     }
@@ -1146,11 +1177,7 @@ 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);
@@ -1176,6 +1203,15 @@ int TcpReassembler::flush_on_ack_policy(TcpReassemblerState& trs, Packet* p)
             skip_seglist_hole(trs, p, flags, flush_amt);
             return flush_on_ack_policy(trs, p);
         }
+        else if ( -1 <= flush_amt and flush_amt <= 0 and
+            trs.paf_state.paf == StreamSplitter::SEARCH and
+            trs.tracker->fin_seq_status == TcpStreamTracker::FIN_WITH_SEQ_ACKED and
+            !p->flow->searching_for_service() )
+        {
+            // we are acknowledging a FIN, the data has been scanned, it has no gaps,
+            // but somehow we are waiting for more data - do final flush here
+            flush_queued_segments(trs, p->flow, true, p);
+        }
     }
     break;
 

diff --git a/src/stream/tcp/tcp_reassembler.h b/src/stream/tcp/tcp_reassembler.h
index 8a12f21b7633c940efcc0b9c188af75d6a103f36..83ce6b17b2f74173e84ccfecd0cb366c8a96efd4 100644 (file)
--- a/src/stream/tcp/tcp_reassembler.h
+++ b/src/stream/tcp/tcp_reassembler.h
@@ -28,6 +28,13 @@
 class TcpReassembler : public SegmentOverlapEditor
 {
 public:
+
+    // OK means FIN seen, data scanned, flush point not found, no gaps
+    enum ScanStatus {
+        FINAL_FLUSH_HOLD = -2,
+        FINAL_FLUSH_OK = -1
+    };
+
     virtual void queue_packet_for_reassembly(TcpReassemblerState&, TcpSegmentDescriptor&);
     virtual void purge_segment_list(TcpReassemblerState&);
     virtual void purge_flushed_ackd(TcpReassemblerState&);
@@ -88,6 +95,8 @@ protected:
     bool next_no_gap(const TcpSegmentNode&);
     bool next_no_gap_c(const TcpSegmentNode&);
     bool next_acked_no_gap_c(const TcpSegmentNode&, const TcpReassemblerState&);
+    bool fin_no_gap(const TcpSegmentNode&, const TcpReassemblerState&);
+    bool fin_acked_no_gap(const TcpSegmentNode&, const TcpReassemblerState&);
     void update_next(TcpReassemblerState&, const TcpSegmentNode&);
     void update_skipped_bytes(uint32_t, TcpReassemblerState&);
     bool has_seglist_hole(TcpReassemblerState&, TcpSegmentNode&, PAF_State&, uint32_t& total,

diff --git a/src/stream/tcp/tcp_state_close_wait.cc b/src/stream/tcp/tcp_state_close_wait.cc
index 13b13814723113eacbf5deb48fc9ea803dfbaaa3..d819fb6fae89fc49b8b00eb07127ee363aa69044 100644 (file)
--- a/src/stream/tcp/tcp_state_close_wait.cc
+++ b/src/stream/tcp/tcp_state_close_wait.cc
@@ -90,6 +90,7 @@ bool TcpStateCloseWait::fin_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& tr
 {
     Flow* flow = tsd.get_flow();
 
+    trk.set_fin_seq_status_seen(tsd);
     trk.update_tracker_ack_recv(tsd);
 
     if ( SEQ_GT(tsd.get_seq(), trk.get_fin_final_seq() ) )

diff --git a/src/stream/tcp/tcp_state_established.cc b/src/stream/tcp/tcp_state_established.cc
index 027f556bf1cf2c0cdd79690c6f4b0cb32c70ec6f..3e952704a0971c4e5360b6d972a9fad33521ca96 100644 (file)
--- a/src/stream/tcp/tcp_state_established.cc
+++ b/src/stream/tcp/tcp_state_established.cc
@@ -102,6 +102,7 @@ bool TcpStateEstablished::fin_sent(TcpSegmentDescriptor& tsd, TcpStreamTracker&
 
 bool TcpStateEstablished::fin_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk)
 {
+    trk.set_fin_seq_status_seen(tsd);
     trk.update_tracker_ack_recv(tsd);
     trk.perform_fin_recv_flush(tsd);
 

diff --git a/src/stream/tcp/tcp_state_fin_wait1.cc b/src/stream/tcp/tcp_state_fin_wait1.cc
index aaba9c7f322c2e58ec1eb9376d2885d979d8c223..a26b0391e8c8fe1efbb820ea079d3eaf0c4012b5 100644 (file)
--- a/src/stream/tcp/tcp_state_fin_wait1.cc
+++ b/src/stream/tcp/tcp_state_fin_wait1.cc
@@ -94,6 +94,7 @@ bool TcpStateFinWait1::fin_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk
     Flow* flow = tsd.get_flow();
     bool is_ack_valid = false;
 
+    trk.set_fin_seq_status_seen(tsd);
     trk.update_tracker_ack_recv(tsd);
     if ( SEQ_GEQ(tsd.get_end_seq(), trk.r_win_base) and
          check_for_window_slam(tsd, trk, &is_ack_valid) )

diff --git a/src/stream/tcp/tcp_state_fin_wait2.cc b/src/stream/tcp/tcp_state_fin_wait2.cc
index 702543ce1b63fafd11f7d4a5def0e0da3e37a0eb..4c0bb06f7d102ac788f3956605f481e7d708ab9e 100644 (file)
--- a/src/stream/tcp/tcp_state_fin_wait2.cc
+++ b/src/stream/tcp/tcp_state_fin_wait2.cc
@@ -105,6 +105,7 @@ bool TcpStateFinWait2::fin_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk
 {
     Flow* flow = tsd.get_flow();
 
+    trk.set_fin_seq_status_seen(tsd);
     trk.update_tracker_ack_recv(tsd);
     if ( SEQ_GEQ(tsd.get_end_seq(), trk.r_win_base) )
     {

diff --git a/src/stream/tcp/tcp_state_syn_recv.cc b/src/stream/tcp/tcp_state_syn_recv.cc
index 7af0a2864de73e54811acbfda9f66aeb48ea6423..499882f042599989f9c10c61dd92405fe0ed741f 100644 (file)
--- a/src/stream/tcp/tcp_state_syn_recv.cc
+++ b/src/stream/tcp/tcp_state_syn_recv.cc
@@ -151,6 +151,7 @@ bool TcpStateSynRecv::fin_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk)
     {
         Flow* flow = tsd.get_flow();
 
+        trk.set_fin_seq_status_seen(tsd);
         trk.update_tracker_ack_recv(tsd);
         trk.session->set_pkt_action_flag(trk.normalizer.handle_paws(tsd));
         flow->session_state |= STREAM_STATE_ACK;

diff --git a/src/stream/tcp/tcp_state_syn_sent.cc b/src/stream/tcp/tcp_state_syn_sent.cc
index e92feed95d2d3b72d4ad99f1080a1d4bb4772f4c..e2eacf9af2c097b0c36d4cac2264d7f6c73184b0 100644 (file)
--- a/src/stream/tcp/tcp_state_syn_sent.cc
+++ b/src/stream/tcp/tcp_state_syn_sent.cc
@@ -108,6 +108,7 @@ bool TcpStateSynSent::data_seg_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker&
 
 bool TcpStateSynSent::fin_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk)
 {
+    trk.set_fin_seq_status_seen(tsd);
     trk.perform_fin_recv_flush(tsd);
     return true;
 }

diff --git a/src/stream/tcp/tcp_state_time_wait.cc b/src/stream/tcp/tcp_state_time_wait.cc
index c3d2cc2b73c59322857d708b0870f93d2cbb408e..bdaf5ea33debff7158f3724e9c50cd0813552ae8 100644 (file)
--- a/src/stream/tcp/tcp_state_time_wait.cc
+++ b/src/stream/tcp/tcp_state_time_wait.cc
@@ -66,6 +66,7 @@ bool TcpStateTimeWait::data_seg_sent(TcpSegmentDescriptor& tsd, TcpStreamTracker
 
 bool TcpStateTimeWait::fin_recv(TcpSegmentDescriptor& tsd, TcpStreamTracker& trk)
 {
+    trk.set_fin_seq_status_seen(tsd);
     trk.update_tracker_ack_recv(tsd);
     if ( SEQ_GT(tsd.get_seq(), trk.get_fin_final_seq() ) )
     {

diff --git a/src/stream/tcp/tcp_stream_tracker.cc b/src/stream/tcp/tcp_stream_tracker.cc
index 19c0fe49985354130985170967fb39b71d6a2582..2188082f6679d965451c06d3b58ab63aba1a6f58 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.cc
+++ b/src/stream/tcp/tcp_stream_tracker.cc
@@ -201,6 +201,18 @@ void TcpStreamTracker::cache_mac_address(const TcpSegmentDescriptor& tsd, uint8_
     }
 }
 
+
+void TcpStreamTracker::set_fin_seq_status_seen(const TcpSegmentDescriptor& tsd)
+{
+    if ( !fin_seq_set and SEQ_GEQ(tsd.get_end_seq(), r_win_base) )
+    {
+        fin_i_seq = tsd.get_seq();
+        fin_final_seq = tsd.get_end_seq();
+        fin_seq_set = true;
+        fin_seq_status = TcpStreamTracker::FIN_WITH_SEQ_SEEN;
+    }
+}
+
 void TcpStreamTracker::init_tcp_state()
 {
     tcp_state = ( client_tracker ) ?
@@ -214,6 +226,7 @@ void TcpStreamTracker::init_tcp_state()
     mss = 0;
     tf_flags = 0;
     mac_addr_valid = false;
+    fin_i_seq = 0;
     fin_final_seq = 0;
     fin_seq_status = TcpStreamTracker::FIN_NOT_SEEN;
     fin_seq_set = false;
@@ -223,7 +236,6 @@ void TcpStreamTracker::init_tcp_state()
     flush_policy = STREAM_FLPOLICY_IGNORE;
     reassembler.reset();
     splitter_finish_flag = false;
-    delayed_finish_flag = false;
 }
 
 //-------------------------------------------------------------------------
@@ -526,7 +538,7 @@ void TcpStreamTracker::update_tracker_ack_sent(TcpSegmentDescriptor& tsd)
     }
 
     if ( ( fin_seq_status == TcpStreamTracker::FIN_WITH_SEQ_SEEN )
-        && SEQ_EQ(r_win_base, fin_final_seq) )
+        && SEQ_GEQ(tsd.get_ack(), fin_final_seq + 1) )
     {
         fin_seq_status = TcpStreamTracker::FIN_WITH_SEQ_ACKED;
     }
@@ -595,15 +607,6 @@ bool TcpStreamTracker::update_on_fin_recv(TcpSegmentDescriptor& tsd)
     else
         fin_seq_adjust = 1;
 
-    // set final seq # any packet rx'ed with seq > is bad
-    if ( !fin_seq_set )
-    {
-        fin_final_seq = tsd.get_end_seq();
-        fin_seq_set = true;
-        if( tsd.get_len() == 0 )
-            fin_seq_status = TcpStreamTracker::FIN_WITH_SEQ_SEEN;
-    }
-
     return true;
 }
 
@@ -666,17 +669,9 @@ void TcpStreamTracker::perform_fin_recv_flush(TcpSegmentDescriptor& tsd)
     if ( tsd.is_data_segment() )
         session->handle_data_segment(tsd);
 
-    // 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;
-        }
+    if ( flush_policy == STREAM_FLPOLICY_ON_DATA and SEQ_EQ(tsd.get_end_seq(), rcv_nxt)
+         and !tsd.get_flow()->searching_for_service() )
         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 6cdcd00b12a5b5a7621b99bff01fda466e5b20fb..309fdef2c6e51c224dd556f372a87036a1cb063f 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.h
+++ b/src/stream/tcp/tcp_stream_tracker.h
@@ -180,6 +180,12 @@ public:
     uint32_t get_fin_seq_adjust()
     { return fin_seq_adjust; }
 
+    uint32_t get_fin_i_seq() const
+    { return fin_i_seq; }
+
+    // set final seq # any packet rx'ed with seq > is bad
+    void set_fin_seq_status_seen(const TcpSegmentDescriptor&);
+
     bool is_fin_seq_set() const
     { return fin_seq_set; }
 
@@ -262,8 +268,6 @@ 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) ); }
@@ -349,6 +353,7 @@ protected:
     uint32_t ts_last_packet = 0;
     uint32_t ts_last = 0;       // last timestamp (for PAWS)
     uint32_t fin_final_seq = 0;
+    uint32_t fin_i_seq = 0;
     uint32_t fin_seq_adjust = 0;
     uint16_t mss = 0;           // max segment size
     uint16_t wscale = 0;        // window scale setting
@@ -359,7 +364,6 @@ 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