]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
stream: fix next_seq updates after temporary gap
authorVictor Julien <vjulien@oisf.net>
Sat, 18 Feb 2023 14:36:55 +0000 (15:36 +0100)
committerVictor Julien <vjulien@oisf.net>
Thu, 2 Mar 2023 08:01:28 +0000 (09:01 +0100)
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)

src/stream-tcp.c

index 2755aec6f57006a5b95c104b5218909fc8bf16fd..9c83a12ae4c92f2244bba8685dab851b4fda59ee 100644 (file)
@@ -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);