From: Victor Julien <vjulien@oisf.net>
Date: Sat, 18 Feb 2023 14:36:55 +0000 (+0100)
Subject: stream: fix next_seq updates after temporary gap
X-Git-Tag: suricata-6.0.11~58
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dcefc00b177b3e073729917c0385b5e0db2757e8;p=thirdparty%2Fsuricata.git

stream: fix next_seq updates after temporary gap

On every accepted packet in established state, update next_seq if
packet seq+len is larger than existing next_seq. This allows it to
catch up after large gaps that are filled again a bit later.

Bug: #5877.
(cherry picked from commit 76225bf9ac87ed4312d44b83d9499794cc760207)
---

diff --git a/src/stream-tcp.c b/src/stream-tcp.c
index 2755aec6f5..9c83a12ae4 100644
--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@@ -2297,13 +2297,11 @@ static int HandleEstablishedPacketToServer(ThreadVars *tv, TcpSession *ssn, Pack
         } else if (SEQ_GT(ssn->client.last_ack, ssn->client.next_seq) &&
                    SEQ_GT((TCP_GET_SEQ(p)+p->payload_len),ssn->client.next_seq))
         {
-            SCLogDebug("ssn %p: PKT SEQ %"PRIu32" payload_len %"PRIu16
-                    " before last_ack %"PRIu32", after next_seq %"PRIu32":"
-                    " acked data that we haven't seen before",
-                    ssn, TCP_GET_SEQ(p), p->payload_len, ssn->client.last_ack, ssn->client.next_seq);
-            if (SEQ_EQ(TCP_GET_SEQ(p),ssn->client.next_seq)) {
-                StreamTcpUpdateNextSeq(ssn, &ssn->client, (ssn->client.next_seq + p->payload_len));
-            }
+            SCLogDebug("ssn %p: PKT SEQ %" PRIu32 " payload_len %" PRIu16
+                       " before last_ack %" PRIu32 ", after next_seq %" PRIu32 ":"
+                       " acked data that we haven't seen before",
+                    ssn, TCP_GET_SEQ(p), p->payload_len, ssn->client.last_ack,
+                    ssn->client.next_seq);
         } else {
             SCLogDebug("ssn %p: server => SEQ before last_ack, packet SEQ"
                     " %" PRIu32 ", payload size %" PRIu32 " (%" PRIu32 "), "
@@ -2325,31 +2323,8 @@ static int HandleEstablishedPacketToServer(ThreadVars *tv, TcpSession *ssn, Pack
         SCLogDebug("ssn %p: zero window probe", ssn);
         zerowindowprobe = 1;
 
-    /* expected packet */
-    } else if (SEQ_EQ(ssn->client.next_seq, TCP_GET_SEQ(p))) {
-        StreamTcpUpdateNextSeq(ssn, &ssn->client, (ssn->client.next_seq + p->payload_len));
-
-    /* not completely as expected, but valid */
-    } else if (SEQ_LT(TCP_GET_SEQ(p),ssn->client.next_seq) &&
-               SEQ_GT((TCP_GET_SEQ(p)+p->payload_len), ssn->client.next_seq))
-    {
+    } else if (SEQ_GEQ(TCP_GET_SEQ(p) + p->payload_len, ssn->client.next_seq)) {
         StreamTcpUpdateNextSeq(ssn, &ssn->client, (TCP_GET_SEQ(p) + p->payload_len));
-        SCLogDebug("ssn %p: ssn->client.next_seq %"PRIu32
-                   " (started before next_seq, ended after)",
-                   ssn, ssn->client.next_seq);
-
-    /* if next_seq has fallen behind last_ack, we got some catching up to do */
-    } else if (SEQ_LT(ssn->client.next_seq, ssn->client.last_ack)) {
-        StreamTcpUpdateNextSeq(ssn, &ssn->client, (TCP_GET_SEQ(p) + p->payload_len));
-        SCLogDebug("ssn %p: ssn->client.next_seq %"PRIu32
-                   " (next_seq had fallen behind last_ack)",
-                   ssn, ssn->client.next_seq);
-
-    } else {
-        SCLogDebug("ssn %p: no update to ssn->client.next_seq %"PRIu32
-                   " SEQ %u SEQ+ %u last_ack %u",
-                   ssn, ssn->client.next_seq,
-                   TCP_GET_SEQ(p), TCP_GET_SEQ(p)+p->payload_len, ssn->client.last_ack);
     }
 
     /* in window check */
@@ -2461,13 +2436,11 @@ static int HandleEstablishedPacketToClient(ThreadVars *tv, TcpSession *ssn, Pack
         } else if (SEQ_GT(ssn->server.last_ack, ssn->server.next_seq) &&
                    SEQ_GT((TCP_GET_SEQ(p)+p->payload_len),ssn->server.next_seq))
         {
-            SCLogDebug("ssn %p: PKT SEQ %"PRIu32" payload_len %"PRIu16
-                    " before last_ack %"PRIu32", after next_seq %"PRIu32":"
-                    " acked data that we haven't seen before",
-                    ssn, TCP_GET_SEQ(p), p->payload_len, ssn->server.last_ack, ssn->server.next_seq);
-            if (SEQ_EQ(TCP_GET_SEQ(p),ssn->server.next_seq)) {
-                StreamTcpUpdateNextSeq(ssn, &ssn->server, (ssn->server.next_seq + p->payload_len));
-            }
+            SCLogDebug("ssn %p: PKT SEQ %" PRIu32 " payload_len %" PRIu16
+                       " before last_ack %" PRIu32 ", after next_seq %" PRIu32 ":"
+                       " acked data that we haven't seen before",
+                    ssn, TCP_GET_SEQ(p), p->payload_len, ssn->server.last_ack,
+                    ssn->server.next_seq);
         } else {
             SCLogDebug("ssn %p: PKT SEQ %"PRIu32" payload_len %"PRIu16
                     " before last_ack %"PRIu32". next_seq %"PRIu32,
@@ -2483,31 +2456,8 @@ static int HandleEstablishedPacketToClient(ThreadVars *tv, TcpSession *ssn, Pack
         SCLogDebug("ssn %p: zero window probe", ssn);
         zerowindowprobe = 1;
 
-    /* expected packet */
-    } else if (SEQ_EQ(ssn->server.next_seq, TCP_GET_SEQ(p))) {
-        StreamTcpUpdateNextSeq(ssn, &ssn->server, (ssn->server.next_seq + p->payload_len));
-
-    /* not completely as expected, but valid */
-    } else if (SEQ_LT(TCP_GET_SEQ(p),ssn->server.next_seq) &&
-               SEQ_GT((TCP_GET_SEQ(p)+p->payload_len), ssn->server.next_seq))
-    {
-        StreamTcpUpdateNextSeq(ssn, &ssn->server, (TCP_GET_SEQ(p) + p->payload_len));
-        SCLogDebug("ssn %p: ssn->server.next_seq %" PRIu32
-                   " (started before next_seq, ended after)",
-                   ssn, ssn->server.next_seq);
-
-    /* if next_seq has fallen behind last_ack, we got some catching up to do */
-    } else if (SEQ_LT(ssn->server.next_seq, ssn->server.last_ack)) {
+    } else if (SEQ_GEQ(TCP_GET_SEQ(p) + p->payload_len, ssn->server.next_seq)) {
         StreamTcpUpdateNextSeq(ssn, &ssn->server, (TCP_GET_SEQ(p) + p->payload_len));
-        SCLogDebug("ssn %p: ssn->server.next_seq %"PRIu32
-                   " (next_seq had fallen behind last_ack)",
-                   ssn, ssn->server.next_seq);
-
-    } else {
-        SCLogDebug("ssn %p: no update to ssn->server.next_seq %"PRIu32
-                   " SEQ %u SEQ+ %u last_ack %u",
-                   ssn, ssn->server.next_seq,
-                   TCP_GET_SEQ(p), TCP_GET_SEQ(p)+p->payload_len, ssn->server.last_ack);
     }
 
     if (zerowindowprobe) {
@@ -7235,12 +7185,12 @@ static int StreamTcpTest11 (void)
 
     FAIL_IF(StreamTcpPacket(&tv, p, &stt, &pq) == -1);
 
-    FAIL_IF(! (((TcpSession *)(p->flow->protoctx))->flags & STREAMTCP_FLAG_ASYNC));
-
-    FAIL_IF(((TcpSession *)(p->flow->protoctx))->state != TCP_ESTABLISHED);
+    TcpSession *ssn = p->flow->protoctx;
+    FAIL_IF((ssn->flags & STREAMTCP_FLAG_ASYNC) == 0);
+    FAIL_IF(ssn->state != TCP_ESTABLISHED);
 
-    FAIL_IF(((TcpSession *)(p->flow->protoctx))->server.last_ack != 2 &&
-            ((TcpSession *)(p->flow->protoctx))->client.next_seq != 1);
+    FAIL_IF(ssn->server.last_ack != 11);
+    FAIL_IF(ssn->client.next_seq != 14);
 
     StreamTcpSessionClear(p->flow->protoctx);
     SCFree(p);