From: Russ Combs (rucombs) <rucombs@cisco.com>
Date: Tue, 22 Oct 2019 13:54:56 +0000 (-0400)
Subject: Merge pull request #1806 in SNORT/snort3 from ~STECHEW/snort3:handle_invalid_acks_v2... 
X-Git-Tag: 3.0.0-263~14
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=06c13d0580bed213856d3cee95e4a3e80808b264;p=thirdparty%2Fsnort3.git

Merge pull request #1806 in SNORT/snort3 from ~STECHEW/snort3:handle_invalid_acks_v2 to master

Squashed commit of the following:

commit a8ff46342ba2547b7bef27e529013a047aff6f22
Author: Steve Chew <stechew@cisco.com>
Date:   Thu Oct 17 14:47:10 2019 -0400

    stream_tcp: If no-ack is on, rewrite ACK value to be the expected ACK.
---

diff --git a/src/stream/libtcp/tcp_segment_descriptor.h b/src/stream/libtcp/tcp_segment_descriptor.h
index 7e2fe2035..33cd334a5 100644
--- a/src/stream/libtcp/tcp_segment_descriptor.h
+++ b/src/stream/libtcp/tcp_segment_descriptor.h
@@ -72,6 +72,11 @@ public:
         return seg_ack;
     }
 
+    void set_seg_ack(uint32_t ack)
+    {
+        this->seg_ack = ack;
+    }
+
     void set_end_seq(uint32_t end_seq)
     {
         this->end_seq = end_seq;
@@ -155,7 +160,7 @@ private:
     const uint16_t src_port;
     const uint16_t dst_port;
     uint32_t seg_seq;
-    const uint32_t seg_ack;
+    uint32_t seg_ack;
     uint32_t seg_wnd;
     uint32_t end_seq;
     uint32_t ts = 0;
diff --git a/src/stream/libtcp/tcp_stream_tracker.cc b/src/stream/libtcp/tcp_stream_tracker.cc
index 13c1f05f7..7de202e1b 100644
--- a/src/stream/libtcp/tcp_stream_tracker.cc
+++ b/src/stream/libtcp/tcp_stream_tracker.cc
@@ -477,27 +477,11 @@ void TcpStreamTracker::update_tracker_ack_recv(TcpSegmentDescriptor& tsd)
 // In no-ack policy, data is implicitly acked immediately.
 void TcpStreamTracker::update_tracker_no_ack_recv(TcpSegmentDescriptor& tsd)
 {
-    // No_ack mode requires that segments be provided in order. If we see
-    // a gap, don't advance the seq and turn off no-ack mode.
-    if(tsd.get_seg_len() != (tsd.get_end_seq() - snd_una))
-    {
-        Stream::set_no_ack_mode(tsd.get_flow(), false);
-        return;
-    }
-
     snd_una = snd_nxt = tsd.get_end_seq();
 }
 
 void TcpStreamTracker::update_tracker_no_ack_sent(TcpSegmentDescriptor& tsd)
 {
-    // No_ack mode requires that segments be provided in order. If we see
-    // a gap, don't advance the seq and turn off no-ack mode.
-    if(tsd.get_seg_len() != (tsd.get_end_seq() - r_win_base))
-    {
-        Stream::set_no_ack_mode(tsd.get_flow(), false);
-        return;
-    }
-
     r_win_base = tsd.get_end_seq();
     reassembler.flush_on_ack_policy(tsd.get_pkt());
 }
diff --git a/src/stream/tcp/tcp_session.cc b/src/stream/tcp/tcp_session.cc
index b232a07ba..e2679aab7 100644
--- a/src/stream/tcp/tcp_session.cc
+++ b/src/stream/tcp/tcp_session.cc
@@ -993,13 +993,24 @@ bool TcpSession::do_packet_analysis_pre_checks(Packet* p, TcpSegmentDescriptor&
     pkt_action_mask = ACTION_NOTHING;
     tel.clear_tcp_events();
     // process thru state machine...talker first
-    if (p->is_from_client())
+    // When in no-ack mode, don't trust ACK numbers. Set the ACK value
+    // as if the last packet in the other direction was ACK'd.
+    // FIXIT-M: The snd_nxt and snd_una checks are only needed because
+    // the snd_nxt value isn't valid for SYN/ACK packet. Can remove those
+    // checks if that is fixed.
+    if ( p->is_from_client() )
     {
         update_session_on_client_packet(tsd);
+
+        if ( no_ack_mode_enabled() and (server.get_snd_nxt() or server.get_snd_una()) )
+            tsd.set_seg_ack(server.get_snd_nxt());
     }
     else
     {
         update_session_on_server_packet(tsd);
+
+        if ( no_ack_mode_enabled() and (client.get_snd_nxt() or client.get_snd_una()) )
+            tsd.set_seg_ack(client.get_snd_nxt());
     }
 
     update_ignored_session(tsd);