From: Masud Hasan (mashasan) Date: Tue, 8 Mar 2022 23:05:45 +0000 (+0000) Subject: Pull request #3257: stream_tcp: call flush_queued_segments() from flush_on_ack_policy() X-Git-Tag: 3.1.25.0~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9eb45494182f051b0905576099d4af3bc534f9c4;p=thirdparty%2Fsnort3.git 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 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 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 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 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 5e463debf..92723ee50 100644 --- 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 8a12f21b7..83ce6b17b 100644 --- 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 13b138147..d819fb6fa 100644 --- 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 027f556bf..3e952704a 100644 --- 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 aaba9c7f3..a26b0391e 100644 --- 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 702543ce1..4c0bb06f7 100644 --- 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 7af0a2864..499882f04 100644 --- 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 e92feed95..e2eacf9af 100644 --- 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 c3d2cc2b7..bdaf5ea33 100644 --- 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 19c0fe499..2188082f6 100644 --- 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 6cdcd00b1..309fdef2c 100644 --- 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