From: Davis McPherson -X (davmcphe - XORIANT CORPORATION at Cisco) Date: Wed, 17 Apr 2024 21:18:47 +0000 (+0000) Subject: Pull request #4277: stream_tcp: Include the overlap offset when calculating index... X-Git-Tag: 3.2.1.0~20 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b1361d06c4f4ef6630908d95043aef352b74f49b;p=thirdparty%2Fsnort3.git Pull request #4277: stream_tcp: Include the overlap offset when calculating index into the data buffer of TcpSegmentNodes for payload rewrites Merge in SNORT/snort3 from ~DAVMCPHE/snort3:tcp_overlap_offset_patch to master Squashed commit of the following: commit cbd20f0882f754005e7e5c096a65ec7ee7d02bad Author: davis mcpherson Date: Thu Apr 4 22:23:46 2024 -0400 stream_tcp: The offset into the data buffer of TcpSegmentNodes due to overlaps was not being used with calculating the to/from address for payload rewrites. This patch updates the overlap rewrite code to properly use this offset. stream_tcp: track offset into data buffer due to overlaps with state variable on the TCP segment node use length of data segment of new packet to adjust seglist logical bytes on lastpolicy left overlap stream_tcp: fix bugs in handling certain OS specific overlay resolutions fix off by 1 bug with handling payload length for SYN packets with data --- diff --git a/src/stream/tcp/segment_overlap_editor.cc b/src/stream/tcp/segment_overlap_editor.cc index d6377a5af..c6ddcd784 100644 --- a/src/stream/tcp/segment_overlap_editor.cc +++ b/src/stream/tcp/segment_overlap_editor.cc @@ -155,7 +155,6 @@ void SegmentOverlapEditor::eval_right(TcpReassemblerState& trs) snort::DetectionEngine::disable_content(trs.sos.tsd->get_pkt()); trs.sos.keep_segment = false; tcpStats.full_retransmits++; - } else { @@ -206,7 +205,6 @@ void SegmentOverlapEditor::left_overlap_keep_first(TcpReassemblerState& trs) // seq is always greater than left->seq assert(SEQ_GT(trs.sos.seq, trs.sos.left->i_seq)); - trs.sos.len = trs.sos.tsd->get_len(); trs.sos.overlap = trs.sos.left->i_seq + trs.sos.left->i_len - trs.sos.seq; if ( trs.sos.len < trs.sos.overlap ) @@ -221,7 +219,7 @@ void SegmentOverlapEditor::left_overlap_keep_first(TcpReassemblerState& trs) { if (trs.sos.tcp_ips_data == NORM_MODE_ON) { - unsigned offset = trs.sos.tsd->get_seq() - trs.sos.left->i_seq; + unsigned offset = trs.sos.tsd->get_seq() - (trs.sos.left->i_seq - trs.sos.left->o_offset); trs.sos.tsd->rewrite_payload(0, trs.sos.left->data + offset); } norm_stats[PC_TCP_IPS_DATA][trs.sos.tcp_ips_data]++; @@ -230,9 +228,8 @@ void SegmentOverlapEditor::left_overlap_keep_first(TcpReassemblerState& trs) { if ( trs.sos.tcp_ips_data == NORM_MODE_ON ) { - unsigned offset = trs.sos.tsd->get_seq() - trs.sos.left->i_seq; - unsigned length = - trs.sos.left->i_seq + trs.sos.left->i_len - trs.sos.tsd->get_seq(); + unsigned offset = trs.sos.tsd->get_seq() - (trs.sos.left->i_seq - trs.sos.left->o_offset); + unsigned length = trs.sos.left->i_seq + trs.sos.left->i_len - trs.sos.tsd->get_seq(); trs.sos.tsd->rewrite_payload(0, trs.sos.left->data + offset, length); } @@ -247,7 +244,6 @@ void SegmentOverlapEditor::left_overlap_trim_first(TcpReassemblerState& trs) { assert(SEQ_GT(trs.sos.seq, trs.sos.left->i_seq)); - trs.sos.len = trs.sos.tsd->get_len(); trs.sos.overlap = trs.sos.left->i_seq + trs.sos.left->i_len - trs.sos.seq; if ( trs.sos.overlap > 0 ) @@ -274,7 +270,6 @@ void SegmentOverlapEditor::left_overlap_keep_last(TcpReassemblerState& trs) { assert(SEQ_GT(trs.sos.seq, trs.sos.left->i_seq)); - trs.sos.len = trs.sos.tsd->get_len(); trs.sos.overlap = trs.sos.left->i_seq + trs.sos.left->i_len - trs.sos.seq; if ( trs.sos.overlap > 0 ) @@ -301,8 +296,9 @@ void SegmentOverlapEditor::left_overlap_keep_last(TcpReassemblerState& trs) trs.sos.right->c_len -= delta; trs.sos.right->i_len -= delta; trs.sos.right->offset += delta; + trs.sos.right->o_offset += delta; - trs.sos.seg_bytes_logical -= delta; + trs.sos.seg_bytes_logical -= trs.sos.len; } else { @@ -318,8 +314,7 @@ void SegmentOverlapEditor::right_overlap_truncate_existing(TcpReassemblerState& if ( SEQ_EQ(trs.sos.right->i_seq, trs.sos.seq) && ( trs.sos.reassembly_policy != StreamPolicy::OS_LAST ) ) { - trs.sos.slide = ( trs.sos.right->i_seq + trs.sos.right->i_len - trs.sos.seq ); - trs.sos.seq += trs.sos.slide; + trs.sos.seq += trs.sos.overlap; } else { @@ -327,6 +322,7 @@ void SegmentOverlapEditor::right_overlap_truncate_existing(TcpReassemblerState& trs.sos.right->i_seq += trs.sos.overlap; trs.sos.right->c_seq = trs.sos.right->i_seq; trs.sos.right->offset += trs.sos.overlap; + trs.sos.right->o_offset += trs.sos.overlap; trs.sos.right->c_len -= (int16_t)trs.sos.overlap; trs.sos.right->i_len -= ( int16_t )trs.sos.overlap; trs.sos.seg_bytes_logical -= trs.sos.overlap; @@ -339,9 +335,8 @@ void SegmentOverlapEditor::right_overlap_truncate_new(TcpReassemblerState& trs) if (trs.sos.tcp_ips_data == NORM_MODE_ON) { unsigned offset = trs.sos.right->i_seq - trs.sos.tsd->get_seq(); - unsigned length = - trs.sos.tsd->get_seq() + trs.sos.tsd->get_len() - trs.sos.right->i_seq; - trs.sos.tsd->rewrite_payload(offset, trs.sos.right->data, length); + unsigned length = trs.sos.tsd->get_seq() + trs.sos.tsd->get_len() - trs.sos.right->i_seq; + trs.sos.tsd->rewrite_payload(offset, trs.sos.right->data + trs.sos.right->o_offset, length); } norm_stats[PC_TCP_IPS_DATA][trs.sos.tcp_ips_data]++; @@ -364,7 +359,7 @@ void SegmentOverlapEditor::full_right_overlap_truncate_new(TcpReassemblerState& return; } - trs.sos.tsd->rewrite_payload(offset, trs.sos.right->data, trs.sos.right->i_len); + trs.sos.tsd->rewrite_payload(offset, trs.sos.right->data + trs.sos.right->o_offset, trs.sos.right->i_len); } norm_stats[PC_TCP_IPS_DATA][trs.sos.tcp_ips_data]++; @@ -376,18 +371,12 @@ void SegmentOverlapEditor::full_right_overlap_truncate_new(TcpReassemblerState& trs.sos.seq += trs.sos.right->i_len; trs.sos.left = trs.sos.right; trs.sos.right = trs.sos.right->next; - - /* Adjusted seq is fully overlapped */ - if ( SEQ_EQ(trs.sos.seq, trs.sos.seq_end) ) - return; } else { - /* seq is less than right->i_seq, trunc length is reset to 0 at beginning of loop */ + // seq is less than right->i_seq, set trunc length and slide + // and insert chunk before current right segment... trs.sos.trunc_len = trs.sos.overlap; - - /* insert this one, and see if we need to chunk it up - Adjust slide so that is correct relative to orig seq */ trs.sos.slide = trs.sos.seq - trs.sos.tsd->get_seq(); add_reassembly_segment(trs, *trs.sos.tsd, trs.sos.len, trs.sos.slide, trs.sos.trunc_len, trs.sos.seq, trs.sos.left); @@ -445,8 +434,7 @@ void SegmentOverlapEditor::full_right_overlap_os3(TcpReassemblerState& trs) if ( SEQ_EQ(trs.sos.right->i_seq, trs.sos.seq) && (trs.sos.right->i_len == trs.sos.len) && (trs.sos.left && !SEQ_EQ(trs.sos.left->i_seq + trs.sos.left->i_len, trs.sos.seq)) ) { - right_overlap_truncate_new(trs); - + trs.sos.trunc_len = trs.sos.right->i_len; trs.sos.rdata += trs.sos.right->i_len; trs.sos.rsize -= trs.sos.right->i_len; trs.sos.rseq += trs.sos.right->i_len; diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc index 726cf6cd0..a340fa643 100644 --- a/src/stream/tcp/tcp_reassembler.cc +++ b/src/stream/tcp/tcp_reassembler.cc @@ -233,6 +233,7 @@ void TcpReassembler::add_reassembly_segment( TcpSegmentNode* const tsn = TcpSegmentNode::init(tsd); tsn->offset = slide; + tsn->o_offset = slide; tsn->c_len = (uint16_t)new_size; tsn->i_len = (uint16_t)new_size; tsn->i_seq = tsn->c_seq = seq; @@ -1336,15 +1337,9 @@ void TcpReassembler::purge_segment_list(TcpReassemblerState& trs) void TcpReassembler::insert_segment_in_empty_seglist( TcpReassemblerState& trs, TcpSegmentDescriptor& tsd) { - const tcp::TCPHdr* tcph = tsd.get_tcph(); - uint32_t overlap = 0; - uint32_t seq = tsd.get_seq(); - - if ( tcph->is_syn() ) - seq++; - if ( SEQ_GT(trs.sos.seglist_base_seq, seq) ) + if ( SEQ_GT(trs.sos.seglist_base_seq, tsd.get_seq()) ) { overlap = trs.sos.seglist_base_seq - tsd.get_seq(); if ( overlap >= tsd.get_len() ) @@ -1352,7 +1347,7 @@ void TcpReassembler::insert_segment_in_empty_seglist( } add_reassembly_segment( - trs, tsd, tsd.get_len(), overlap, 0, seq + overlap, nullptr); + trs, tsd, tsd.get_len(), overlap, 0, tsd.get_seq() + overlap, nullptr); } void TcpReassembler::init_overlap_editor( @@ -1379,7 +1374,6 @@ void TcpReassembler::init_overlap_editor( for ( tsn = trs.sos.seglist.head; tsn; tsn = tsn->next ) { right = tsn; - if ( SEQ_GEQ(right->i_seq, tsd.get_seq() ) ) break; @@ -1394,7 +1388,6 @@ void TcpReassembler::init_overlap_editor( for ( tsn = trs.sos.seglist.tail; tsn; tsn = tsn->prev ) { left = tsn; - if ( SEQ_LT(left->i_seq, tsd.get_seq() ) ) break; @@ -1432,11 +1425,12 @@ void TcpReassembler::insert_segment_in_seglist( return; } - /* Adjust slide so that is correct relative to orig seq */ - trs.sos.slide = trs.sos.seq - tsd.get_seq(); - // FIXIT-L for some reason length - slide - trunc_len is sometimes negative - if (trs.sos.len - trs.sos.slide - trs.sos.trunc_len < 0) - return; + // slide is current seq number - initial seq number unless all data + // truncated from right, then slide is 0 + if ( trs.sos.len - trs.sos.trunc_len > 0 ) + trs.sos.slide = trs.sos.seq - tsd.get_seq(); + else + trs.sos.slide = 0; add_reassembly_segment( trs, tsd, trs.sos.len, trs.sos.slide, trs.sos.trunc_len, trs.sos.seq, trs.sos.left); diff --git a/src/stream/tcp/tcp_reassemblers.cc b/src/stream/tcp/tcp_reassemblers.cc index 9e7b328e1..a1302cea2 100644 --- a/src/stream/tcp/tcp_reassemblers.cc +++ b/src/stream/tcp/tcp_reassemblers.cc @@ -308,9 +308,6 @@ void TcpReassemblerFactory::term() TcpReassembler* TcpReassemblerFactory::get_instance(StreamPolicy os_policy) { - NormMode tcp_ips_data = Normalize_GetMode(NORM_TCP_IPS); - StreamPolicy sp = (tcp_ips_data == NORM_MODE_ON) ? StreamPolicy::OS_FIRST : os_policy; - - assert( sp <= StreamPolicy::OS_PROXY ); - return reassemblers[sp]; + assert( os_policy <= StreamPolicy::OS_PROXY ); + return reassemblers[os_policy]; } diff --git a/src/stream/tcp/tcp_segment_node.cc b/src/stream/tcp/tcp_segment_node.cc index 3415def47..42ea2671e 100644 --- a/src/stream/tcp/tcp_segment_node.cc +++ b/src/stream/tcp/tcp_segment_node.cc @@ -93,7 +93,7 @@ TcpSegmentNode* TcpSegmentNode::create( tsn->prev = tsn->next = nullptr; tsn->i_seq = tsn->c_seq = 0; - tsn->offset = 0; + tsn->offset = tsn->o_offset = 0; tsn->ts = 0; return tsn; diff --git a/src/stream/tcp/tcp_segment_node.h b/src/stream/tcp/tcp_segment_node.h index 3e58f0eac..25488502f 100644 --- a/src/stream/tcp/tcp_segment_node.h +++ b/src/stream/tcp/tcp_segment_node.h @@ -78,7 +78,8 @@ public: uint32_t c_seq; // current seq # of data for reassembly uint16_t i_len; // initial length of the data segment uint16_t c_len; // length of data remaining for reassembly - uint16_t offset; + uint16_t offset; // current offset into the data buffer for reassembly + uint16_t o_offset; // offset into the data buffer due to overlaps uint16_t size; // actual allocated size (overlaps cause i_len to differ) uint8_t data[1]; }; diff --git a/src/stream/tcp/tcp_session.cc b/src/stream/tcp/tcp_session.cc index 0d9bc074a..11ea9781b 100644 --- a/src/stream/tcp/tcp_session.cc +++ b/src/stream/tcp/tcp_session.cc @@ -473,6 +473,12 @@ void TcpSession::set_os_policy() client.normalizer.init(client_os_policy, this, &client, &server); server.normalizer.init(server_os_policy, this, &server, &client); + if (Normalize_GetMode(NORM_TCP_IPS) == NORM_MODE_ON) + { + client_os_policy = StreamPolicy::OS_FIRST; + server_os_policy = StreamPolicy::OS_FIRST; + } + client.reassembler.init(this, &client, client_os_policy, false); server.reassembler.init(this, &server, server_os_policy, true); } @@ -640,6 +646,8 @@ void TcpSession::handle_data_on_syn(TcpSegmentDescriptor& tsd) if ( !listener->normalizer.trim_syn_payload(tsd) ) { + // skip the byte in sequence space for SYN...data starts at the next byte + tsd.update_seq(1); handle_data_segment(tsd); tel.set_tcp_event(EVENT_DATA_ON_SYN); } @@ -770,10 +778,9 @@ void TcpSession::handle_data_segment(TcpSegmentDescriptor& tsd, bool flush) else server.set_tcp_options_len(tcp_options_len); - uint32_t seq = tsd.get_tcph()->is_syn() ? tsd.get_seq() + 1 : tsd.get_seq(); - bool stream_is_inorder = ( seq == listener->rcv_nxt ); + bool stream_is_inorder = ( tsd.get_seq() == listener->rcv_nxt ); - int rc = listener->normalizer.apply_normalizations(tsd, seq, stream_is_inorder); + int rc = listener->normalizer.apply_normalizations(tsd, tsd.get_seq(), stream_is_inorder); switch ( rc ) { case TcpNormalizer::NORM_OK: