Pull request #4813: stream_tcp: separate logs and counters for left and right invalid...

Merge in SNORT/snort3 from ~JALIIMRA/snort3:left_right_invalid_seq2 to master

Squashed commit of the following:

commit a5a8276c436d1a0e07dcf680b7f549bc2c3c9dd9
Author: Juweria Ali Imran <jaliimra@cisco.com>
Date:   Tue Feb 4 06:24:29 2025 -0500

    stream_tcp: separate logs and counters for left and right invalid sequence numbers

diff --git a/src/stream/tcp/tcp_module.cc b/src/stream/tcp/tcp_module.cc
index db4ea19908947013c1abd6f1423912bd53bcaa34..ac2567d437da8bce2aae1d027ca5c31a9820de26 100644 (file)
--- a/src/stream/tcp/tcp_module.cc
+++ b/src/stream/tcp/tcp_module.cc
@@ -61,7 +61,10 @@ const PegInfo tcp_pegs[] =
     { CountType::SUM, "resyns", "SYN received on established session" },
     { CountType::SUM, "discards", "tcp packets discarded" },
     { CountType::SUM, "discards_skipped", "tcp packet discards skipped due to normalization disabled" },
-    { CountType::SUM, "invalid_seq_num", "tcp packets received with an invalid sequence number" },
+    { CountType::SUM, "invalid_seq_left", 
+        "tcp packets received that fall to the left of the current TCP window (spurious retransmits)" },
+    { CountType::SUM, "invalid_seq_right", 
+        "tcp packets received that fall to the right of the current TCP window" },
     { CountType::SUM, "invalid_ack", "tcp packets received with an invalid ack number" },
     { CountType::SUM, "no_flags_set", "tcp packets received with no TCP flags set" },
     { CountType::SUM, "events", "events generated" },

diff --git a/src/stream/tcp/tcp_module.h b/src/stream/tcp/tcp_module.h
index d6cf39921cac133894f917bbc08f2b700e24a4f9..7bcaccd2ea82d7003c2519547a3953c7a0ef46f2 100644 (file)
--- a/src/stream/tcp/tcp_module.h
+++ b/src/stream/tcp/tcp_module.h
@@ -65,7 +65,8 @@ struct TcpStats
     PegCount resyns;
     PegCount discards;
     PegCount discards_skipped;
-    PegCount invalid_seq_num;
+    PegCount invalid_seq_left;
+    PegCount invalid_seq_right;
     PegCount invalid_ack;
     PegCount no_flags_set;
     PegCount events;

diff --git a/src/stream/tcp/tcp_normalizer.cc b/src/stream/tcp/tcp_normalizer.cc
index 70deb56a3ad685dc0873508f5393fc83ab6a969e..c6acc6f899464054cf8e22390f93e491d4cfedca 100644 (file)
--- a/src/stream/tcp/tcp_normalizer.cc
+++ b/src/stream/tcp/tcp_normalizer.cc
@@ -40,14 +40,32 @@ TcpNormalizer::NormStatus TcpNormalizer::apply_normalizations(
     TcpNormalizerState& tns, TcpSegmentDescriptor& tsd, uint32_t seq, bool stream_is_inorder)
 {
     // drop packet if sequence num is invalid
-    if ( !tns.tracker->is_segment_seq_valid(tsd) )
+    ValidSeqStatus invalid_seq = tns.tracker->is_segment_seq_valid(tsd);
+    if (invalid_seq)
     {
-        if ( is_keep_alive_probe(tns, tsd) )
-            return NORM_BAD_SEQ;
-
         bool inline_mode = tsd.is_nap_policy_inline();
-        tcpStats.invalid_seq_num++;
-        log_drop_reason(tns, tsd, inline_mode, "stream", "Normalizer: Sequence number is invalid\n");
+        switch (invalid_seq)
+        {
+        case ValidSeqStatus::LEFT_INVALID:
+            if (is_keep_alive_probe(tns, tsd))
+                return NORM_BAD_SEQ;
+
+            tcpStats.invalid_seq_left++;
+            log_drop_reason(tns, tsd, inline_mode, "stream", "Normalizer: Received a Spurious Retransmission\n");
+            break;
+
+        case ValidSeqStatus::RIGHT_INVALID:
+            tcpStats.invalid_seq_right++;
+            log_drop_reason(tns, tsd, inline_mode, "stream", "Normalizer: Received a packet to the right of the allowed TCP window \n");
+            break;
+        
+        default:
+            break;
+        }
+        if (PacketTracer::is_active())
+            PacketTracer::log("Current TCP window : [ %u - %u ]\n"
+            , tns.tracker->r_win_base, tns.tracker->r_win_base + tns.tracker->get_snd_wnd());
+        tns.tracker->seglist.print_stream_state(tsd.get_talker());
         trim_win_payload(tns, tsd, 0, inline_mode);
         return NORM_BAD_SEQ;
     }
@@ -71,6 +89,10 @@ TcpNormalizer::NormStatus TcpNormalizer::apply_normalizations(
             if ( !data_inside_window(tns, tsd) )
             {
                 log_drop_reason(tns, tsd, inline_mode, "stream", "Normalizer: Data is outside the TCP Window\n");
+                if (PacketTracer::is_active())
+                    PacketTracer::log("Current TCP window : [ %u - %u ]\n"
+                    , tns.tracker->r_win_base, tns.tracker->r_win_base + tns.tracker->get_snd_wnd());
+                tns.tracker->seglist.print_stream_state(tsd.get_talker());
                 trim_win_payload(tns, tsd, 0, inline_mode);
                 return NORM_TRIMMED;
             }
@@ -99,7 +121,9 @@ TcpNormalizer::NormStatus TcpNormalizer::apply_normalizations(
         }
 
         log_drop_reason(tns, tsd, inline_mode, "stream",
-            "Normalizer: Received data during a Zero Window that is not a Zero Window Probe\n");
+            "Normalizer: Received seq during a Zero Window that is not a Zero Window Probe ("
+                + std::to_string(tns.tracker->rcv_nxt) + ")\n");
+        tns.tracker->seglist.print_stream_state(tsd.get_talker());
         trim_win_payload(tns, tsd, 0, inline_mode);
         return NORM_TRIMMED;
     }

diff --git a/src/stream/tcp/tcp_reassembly_segments.cc b/src/stream/tcp/tcp_reassembly_segments.cc
index 04e03cff116216613b0ad3a94fc9f9e29f7fce69..0b5ad0402a936b79e52ab5dbfc3c761d32b5d036 100644 (file)
--- a/src/stream/tcp/tcp_reassembly_segments.cc
+++ b/src/stream/tcp/tcp_reassembly_segments.cc
@@ -102,11 +102,9 @@ uint32_t TcpReassemblySegments::get_pending_segment_count(const unsigned max) co
 
 bool TcpReassemblySegments::segment_within_seglist_window(TcpSegmentDescriptor& tsd)
 {
-    if ( !head )
-        return true;
-
-    // Left side
+    assert (head);
     uint32_t start;
+
     if ( SEQ_LT(seglist_base_seq, head->start_seq()) )
         start = seglist_base_seq;
     else

diff --git a/src/stream/tcp/tcp_session.cc b/src/stream/tcp/tcp_session.cc
index 5c5915b85a04f059a5a0b417fa7a8d52ca9c5f6c..387f4b906937d588828ae3d8cbb7d43195e397ad 100644 (file)
--- a/src/stream/tcp/tcp_session.cc
+++ b/src/stream/tcp/tcp_session.cc
@@ -599,8 +599,7 @@ void TcpSession::mark_packet_for_drop(TcpSegmentDescriptor& tsd)
 
 bool TcpSession::check_reassembly_queue_thresholds(TcpSegmentDescriptor& tsd, TcpStreamTracker* listener)
 {
-    // if this packet fits within the current queue limit window then it's good
-    if ( listener->seglist.segment_within_seglist_window(tsd) )
+    if ( !listener->seglist.head )
         return false;
 
     bool inline_mode = tsd.is_nap_policy_inline();
@@ -612,13 +611,15 @@ bool TcpSession::check_reassembly_queue_thresholds(TcpSegmentDescriptor& tsd, Tc
 
         if ( space_left < (int32_t)tsd.get_len() )
         {
+            // if this packet fits within the current queue limit window then it's good
+            if ( listener->seglist.segment_within_seglist_window(tsd) )
+                return false;
+
             tcpStats.exceeded_max_bytes++;
 
             if ( is_hole_present(listener, tsd.get_pkt()) )
                 tcpStats.max_bytes_exceeded_hole++;
 
-            bool ret_val = true;
-
             // if this is an asymmetric flow then skip over any seglist holes
             // and flush to free up seglist space
             if ( !tsd.get_pkt()->flow->two_way_traffic() )
@@ -628,6 +629,8 @@ bool TcpSession::check_reassembly_queue_thresholds(TcpSegmentDescriptor& tsd, Tc
                     return false;
             }
 
+            bool ret_val = true;
+
             if ( space_left > 0 )
                 ret_val = !inline_mode; // For partial trim, reassemble only if we can force an inject
             else
@@ -653,6 +656,10 @@ bool TcpSession::check_reassembly_queue_thresholds(TcpSegmentDescriptor& tsd, Tc
     {
         if ( listener->seglist.get_seg_count() + 1 > tcp_config->max_queued_segs )
         {
+            // if this packet fits within the current queue limit window then it's good
+            if ( listener->seglist.segment_within_seglist_window(tsd) )
+                return false;
+
             tcpStats.exceeded_max_segs++;
 
             if ( is_hole_present(listener, tsd.get_pkt()) )

diff --git a/src/stream/tcp/tcp_stream_tracker.cc b/src/stream/tcp/tcp_stream_tracker.cc
index 38c66f7889b92bf24cf9e1d7771f66a23fec9840..aecce6a7732c540ba0046ad1d3872997dbd3ed38 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.cc
+++ b/src/stream/tcp/tcp_stream_tracker.cc
@@ -784,9 +784,9 @@ bool TcpStreamTracker::update_on_fin_sent(TcpSegmentDescriptor& tsd)
     return true;
 }
 
-bool TcpStreamTracker::is_segment_seq_valid(TcpSegmentDescriptor& tsd)
+ValidSeqStatus TcpStreamTracker::is_segment_seq_valid(TcpSegmentDescriptor& tsd)
 {
-    bool valid_seq = true;
+    ValidSeqStatus valid_seq = ValidSeqStatus::VALID;
 
     int right_ok;
     uint32_t left_seq;
@@ -806,12 +806,12 @@ bool TcpStreamTracker::is_segment_seq_valid(TcpSegmentDescriptor& tsd)
         uint32_t win = normalizer.get_stream_window(tsd);
 
         if ( SEQ_LEQ(tsd.get_seq(), r_win_base + win) )
-            return true;
+            return ValidSeqStatus::VALID;
         else
-            valid_seq = false;
+            valid_seq = ValidSeqStatus::RIGHT_INVALID;
     }
     else
-        valid_seq = false;
+        valid_seq = ValidSeqStatus::LEFT_INVALID;
 
     return valid_seq;
 }

diff --git a/src/stream/tcp/tcp_stream_tracker.h b/src/stream/tcp/tcp_stream_tracker.h
index 5f7d23bbc8f692fdd006a6e5ac6069eb499f52da..97c255bddacd62460732d5efad7618eea5c3385e 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.h
+++ b/src/stream/tcp/tcp_stream_tracker.h
@@ -46,6 +46,7 @@ class HeldPacket;
 class TcpSession;
 
 enum FinSeqNumStatus : uint8_t { FIN_NOT_SEEN, FIN_WITH_SEQ_SEEN, FIN_WITH_SEQ_ACKED };
+enum ValidSeqStatus : uint8_t { VALID, LEFT_INVALID, RIGHT_INVALID };
 
 class TcpStreamTracker
 {
@@ -296,7 +297,7 @@ public:
     void update_on_rst_sent();
     bool update_on_fin_recv(TcpSegmentDescriptor&);
     bool update_on_fin_sent(TcpSegmentDescriptor&);
-    bool is_segment_seq_valid(TcpSegmentDescriptor&);
+    ValidSeqStatus is_segment_seq_valid(TcpSegmentDescriptor&);
     bool set_held_packet(snort::Packet*);
     bool is_retransmit_of_held_packet(snort::Packet*);
     void finalize_held_packet(snort::Packet*);