Pull request #4277: stream_tcp: Include the overlap offset when calculating index...

Merge in SNORT/snort3 from ~DAVMCPHE/snort3:tcp_overlap_offset_patch to master

Squashed commit of the following:

commit cbd20f0882f754005e7e5c096a65ec7ee7d02bad
Author: davis mcpherson <davmcphe@cisco.com>
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 d6377a5af038ef25b7f1d29aec495c31fd5e7f67..c6ddcd784443b81cf0a4f10a73b2dfb61314ea68 100644 (file)
--- 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 726cf6cd040bfbca257ebaf0d0557a1f47d1742e..a340fa643bd411f67f0f3f14bfed6644c89b6d67 100644 (file)
--- 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 9e7b328e10c914e225b74e0901faac4122d4016d..a1302cea2d100d511a3e97e2227c259712ef94b0 100644 (file)
--- 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 3415def479ad269a9a5856729d6b63e31b238640..42ea2671e0253fd12ad6e808aa8429f436da14ec 100644 (file)
--- 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 3e58f0eacc94d0dd6fd386750e2fd27dbafb921d..25488502ffe44bd9b7264a65a098f74c2c104b1d 100644 (file)
--- 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 0d9bc074a8f45f1f3596a25fe783a8d4b441f1df..11ea9781b45e970d1657a695249481feacdd3e2b 100644 (file)
--- 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: