Pull request #4330: stream_tcp: don't attempt to verify or process keep-alive probes...

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

Squashed commit of the following:

commit 4282d9a3268fb8facfdb5cd20842980ee2fc6dd4
Author: Juweria Ali Imran <jaliimra@cisco.com>
Date:   Wed May 22 09:23:57 2024 -0400

    stream_tcp: don't attempt to verify or process keep-alive probes with data

diff --git a/src/stream/tcp/tcp_defs.h b/src/stream/tcp/tcp_defs.h
index 5cf65a71cc1e73834892b5e7a3115e6c656a1695..4bb27b4298f966be596bbb041b8ccf0a398a3956 100644 (file)
--- a/src/stream/tcp/tcp_defs.h
+++ b/src/stream/tcp/tcp_defs.h
@@ -58,6 +58,7 @@ struct Packet;
 #define SLAM_MAX 4
 
 #define MAX_ZERO_WIN_PROBE_LEN 1
+#define MAX_KEEP_ALIVE_PROBE_LEN 1
 
 // target-based policy types - changes to this enum require changes to stream.h::TCP_POLICIES
 enum StreamPolicy : uint8_t

diff --git a/src/stream/tcp/tcp_module.cc b/src/stream/tcp/tcp_module.cc
index f99d60399158f92abe6325aa84e36d7b80c866ae..1d80f624f484ecfdf1e54e0f534ec9cf31b433fd 100644 (file)
--- a/src/stream/tcp/tcp_module.cc
+++ b/src/stream/tcp/tcp_module.cc
@@ -117,6 +117,7 @@ const PegInfo tcp_pegs[] =
     { CountType::MAX, "max_bytes", "maximum number of bytes queued in any flow" },
     { CountType::SUM, "zero_len_tcp_opt", "number of zero length tcp options" },
     { CountType::SUM, "zero_win_probes", "number of tcp zero window probes" },
+    { CountType::SUM, "keep_alive_probes", "number of tcp keep-alive probes" },
     { CountType::SUM, "proxy_mode_flows", "number of flows set to proxy normalization policy" },
     { CountType::SUM, "full_retransmits", "number of fully retransmitted segments" },
     { CountType::SUM, "flush_on_asymmetric_flow", "number of flushes on asymmetric flows" },

diff --git a/src/stream/tcp/tcp_module.h b/src/stream/tcp/tcp_module.h
index 4b6f00999b050eb6810b7d3dbe4bb466d6c57de9..06b196b3d3ff963126e56746d123fec42693c481 100644 (file)
--- a/src/stream/tcp/tcp_module.h
+++ b/src/stream/tcp/tcp_module.h
@@ -116,6 +116,7 @@ struct TcpStats
     PegCount max_bytes;
     PegCount zero_len_tcp_opt;
     PegCount zero_win_probes;
+    PegCount keep_alive_probes;
     PegCount proxy_mode_flows;
     PegCount full_retransmits;
     PegCount flush_on_asymmetric_flow;

diff --git a/src/stream/tcp/tcp_normalizer.cc b/src/stream/tcp/tcp_normalizer.cc
index 79f5326d76e35d1c011bac0e9a14b6fb2d1a08a4..84c080f4037cce36c3bfb8452a302035fc999d14 100644 (file)
--- a/src/stream/tcp/tcp_normalizer.cc
+++ b/src/stream/tcp/tcp_normalizer.cc
@@ -40,6 +40,9 @@ TcpNormalizer::NormStatus TcpNormalizer::apply_normalizations(
     // drop packet if sequence num is invalid
     if ( !tns.tracker->is_segment_seq_valid(tsd) )
     {
+        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");
@@ -517,6 +520,19 @@ void TcpNormalizer::log_drop_reason(TcpNormalizerState& tns, const TcpSegmentDes
     }
 }
 
+bool TcpNormalizer::is_keep_alive_probe(TcpNormalizerState& tns, const TcpSegmentDescriptor& tsd)
+{
+    if ( (tns.tracker->r_win_base - tsd.get_seq()) == MAX_KEEP_ALIVE_PROBE_LEN
+        and tsd.get_len() <= MAX_KEEP_ALIVE_PROBE_LEN and 
+        !(tsd.get_tcph()->th_flags & (TH_SYN|TH_FIN|TH_RST)) )
+    {
+        tcpStats.keep_alive_probes++;
+        return true;
+    }
+
+    return false;
+}
+
 uint16_t TcpNormalizer::set_urg_offset(
     TcpNormalizerState&, const tcp::TCPHdr* tcph, uint16_t dsize)
 {

diff --git a/src/stream/tcp/tcp_normalizer.h b/src/stream/tcp/tcp_normalizer.h
index 17c5c8d629debe754f14938f87daeff5a4254594..1e800ad3c7c11b5ddfc721139a3748d7b392ed2a 100644 (file)
--- a/src/stream/tcp/tcp_normalizer.h
+++ b/src/stream/tcp/tcp_normalizer.h
@@ -90,6 +90,7 @@ public:
     virtual uint16_t set_urg_offset(State&, const snort::tcp::TCPHdr* tcph, uint16_t dsize);
     virtual void set_zwp_seq(State&, uint32_t seq);
     virtual void log_drop_reason(State&, const TcpSegmentDescriptor&, bool inline_mode, const char *issuer, const std::string& log);
+    virtual bool is_keep_alive_probe(State&, const TcpSegmentDescriptor&);
 
     static void reset_stats();
 

diff --git a/src/stream/tcp/tcp_normalizers.h b/src/stream/tcp/tcp_normalizers.h
index eaa3190de53d2cd7823058f1d796f861be6ee6fc..46f1443708f7916d0c074df4a21cba144a83a2d4 100644 (file)
--- a/src/stream/tcp/tcp_normalizers.h
+++ b/src/stream/tcp/tcp_normalizers.h
@@ -102,7 +102,10 @@ public:
     { return norm->set_zwp_seq(tns, seq); }
 
     void log_drop_reason(const TcpSegmentDescriptor& tsd, bool inline_mode, const char *issuer, const std::string& log)
-    { return norm->log_drop_reason(tns, tsd , inline_mode, issuer, log); }
+    { return norm->log_drop_reason(tns, tsd, inline_mode, issuer, log); }
+
+    bool is_keep_alive_probe(const TcpSegmentDescriptor& tsd)
+    { return norm->is_keep_alive_probe(tns, tsd); }
 
     uint16_t set_urg_offset(const snort::tcp::TCPHdr* tcph, uint16_t dsize)
     { return norm->set_urg_offset(tns, tcph, dsize); }

diff --git a/src/stream/tcp/tcp_stream_tracker.cc b/src/stream/tcp/tcp_stream_tracker.cc
index e982a2c2990bbb4a8a853350e0b8538c09365f2f..4068a4b27b659689482d5e5f5cfdda9f2ae72fe6 100644 (file)
--- a/src/stream/tcp/tcp_stream_tracker.cc
+++ b/src/stream/tcp/tcp_stream_tracker.cc
@@ -487,9 +487,14 @@ void TcpStreamTracker::update_tracker_ack_recv(TcpSegmentDescriptor& tsd)
         if ( SEQ_LT(snd_nxt, snd_una) )
             snd_nxt = snd_una;
     }
-    if ( !tsd.get_len() and snd_wnd == 0
-        and SEQ_LT(tsd.get_seq(), r_win_base) )
-        tcpStats.zero_win_probes++;
+    if ( !tsd.get_len() and SEQ_LT(tsd.get_seq(), r_win_base) )
+    {
+        if ( snd_wnd == 0 )
+            tcpStats.zero_win_probes++;
+        else if ( (r_win_base - tsd.get_seq()) == MAX_KEEP_ALIVE_PROBE_LEN
+            and !(tsd.get_tcph()->th_flags & (TH_SYN|TH_FIN|TH_RST)) )
+            tcpStats.keep_alive_probes++;
+    }
 }
 
 // In no-ack policy, data is implicitly acked immediately.