stream: fix bad last_ack update leading to gaps

A bad last_ack update where it would be set beyond next_seq could
lead to rejection of valid segments and thus stream gaps.

Update tests to reflect new last_ack/next_seq behaviour.

diff --git a/src/stream-tcp.c b/src/stream-tcp.c
index d414186bf87d24acf8ec3f11552d90ee77b8bd02..5cc5513ab54ab2eeac0f7e3b10e0556ed84ea820 100644 (file)
--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@@ -753,18 +753,25 @@ uint32_t StreamTcpGetStreamSize(TcpStream *stream)
 }
 
 /**
- *  \brief macro to update last_ack only if the new value is higher
+ *  \brief macro to update last_ack only if the new value is higher and
+ *         the ack value isn't beyond the next_seq
  *
  *  \param ssn session
  *  \param stream stream to update
  *  \param ack ACK value to test and set
  */
 #define StreamTcpUpdateLastAck(ssn, stream, ack) { \
-    if (SEQ_GT((ack), (stream)->last_ack)) { \
+    if (SEQ_GT((ack), (stream)->last_ack) && \
+            (SEQ_LEQ((ack),(stream)->next_seq) || \
+            ((ssn)->state >= TCP_FIN_WAIT1 && SEQ_EQ((ack),((stream)->next_seq + 1))))) \
+    { \
         (stream)->last_ack = (ack); \
         SCLogDebug("ssn %p: last_ack set to %"PRIu32, (ssn), (stream)->last_ack); \
         StreamTcpSackPruneList((stream)); \
-    } \
+    } else { \
+        SCLogDebug("ssn %p: no update: ack %u, last_ack %"PRIu32", next_seq %u (state %u)", \
+                    (ssn), (ack), (stream)->last_ack, (stream)->next_seq, (ssn)->state); \
+    }\
 }
 
 /**
@@ -2036,6 +2043,7 @@ static int HandleEstablishedPacketToServer(ThreadVars *tv, TcpSession *ssn, Pack
 
         /* Check if the ACK value is sane and inside the window limit */
         StreamTcpUpdateLastAck(ssn, &ssn->server, TCP_GET_ACK(p));
+        SCLogDebug("ack %u last_ack %u next_seq %u", TCP_GET_ACK(p), ssn->server.last_ack, ssn->server.next_seq);
 
         if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
             StreamTcpHandleTimestamp(ssn, p);
@@ -9910,12 +9918,11 @@ static int StreamTcpTest38 (void)
     Flow f;
     ThreadVars tv;
     StreamTcpThread stt;
-    uint8_t payload[4];
+    uint8_t payload[128];
     TCPHdr tcph;
-    TcpReassemblyThreadCtx ra_ctx;
     PacketQueue pq;
 
-    memset(&ra_ctx, 0, sizeof(TcpReassemblyThreadCtx));
+    TcpReassemblyThreadCtx *ra_ctx = StreamTcpReassembleInitThreadCtx(NULL);
     memset (&f, 0, sizeof(Flow));
     memset(&tv, 0, sizeof (ThreadVars));
     memset(&stt, 0, sizeof (StreamTcpThread));
@@ -9933,7 +9940,7 @@ static int StreamTcpTest38 (void)
     tcph.th_flags = TH_SYN;
     p->tcph = &tcph;
     p->flowflags = FLOW_PKT_TOSERVER;
-    stt.ra_ctx = &ra_ctx;
+    stt.ra_ctx = ra_ctx;
 
     StreamTcpInitConfig(TRUE);
     SCMutexLock(&f.m);
@@ -9983,7 +9990,27 @@ static int StreamTcpTest38 (void)
         goto end;
     }
 
-    p->tcph->th_ack = htonl(2984);
+    p->tcph->th_ack = htonl(1);
+    p->tcph->th_seq = htonl(1);
+    p->tcph->th_flags = TH_PUSH | TH_ACK;
+    p->flowflags = FLOW_PKT_TOCLIENT;
+
+    StreamTcpCreateTestPacket(payload, 0x41, 127, 128); /*AAA*/
+    p->payload = payload;
+    p->payload_len = 127;
+
+    if (StreamTcpPacket(&tv, p, &stt, &pq) == -1) {
+        printf("failed in processing packet in StreamTcpPacket\n");
+        goto end;
+    }
+
+    if (((TcpSession *)(p->flow->protoctx))->server.next_seq != 128) {
+        printf("the server.next_seq should be 128, but it is %"PRIu32"\n",
+                ((TcpSession *)(p->flow->protoctx))->server.next_seq);
+        goto end;
+    }
+
+    p->tcph->th_ack = htonl(256); // in window, but beyond next_seq
     p->tcph->th_seq = htonl(5);
     p->tcph->th_flags = TH_PUSH | TH_ACK;
     p->flowflags = FLOW_PKT_TOSERVER;
@@ -9997,10 +10024,32 @@ static int StreamTcpTest38 (void)
         goto end;
     }
 
+    /* last_ack value should be 1, not 256, as the previous sent ACK value
+       is inside window, but beyond next_seq */
+    if (((TcpSession *)(p->flow->protoctx))->server.last_ack != 1) {
+        printf("the server.last_ack should be 1, but it is %"PRIu32"\n",
+                ((TcpSession *)(p->flow->protoctx))->server.last_ack);
+        goto end;
+    }
+
+    p->tcph->th_ack = htonl(128);
+    p->tcph->th_seq = htonl(8);
+    p->tcph->th_flags = TH_PUSH | TH_ACK;
+    p->flowflags = FLOW_PKT_TOSERVER;
+
+    StreamTcpCreateTestPacket(payload, 0x41, 3, 4); /*AAA*/
+    p->payload = payload;
+    p->payload_len = 3;
+
+    if (StreamTcpPacket(&tv, p, &stt, &pq) == -1) {
+        printf("failed in processing packet in StreamTcpPacket\n");
+        goto end;
+    }
+
     /* last_ack value should be 2984 as the previous sent ACK value is inside
        window */
-    if (((TcpSession *)(p->flow->protoctx))->server.last_ack != 2984) {
-        printf("the server.last_ack should be 2984, but it is %"PRIu32"\n",
+    if (((TcpSession *)(p->flow->protoctx))->server.last_ack != 128) {
+        printf("the server.last_ack should be 128, but it is %"PRIu32"\n",
                 ((TcpSession *)(p->flow->protoctx))->server.last_ack);
         goto end;
     }
@@ -10013,6 +10062,8 @@ end:
     SCMutexUnlock(&f.m);
     SCFree(p);
     FLOW_DESTROY(&f);
+    if (stt.ra_ctx != NULL)
+        StreamTcpReassembleFreeThreadCtx(stt.ra_ctx);
     return ret;
 }
 
@@ -10030,10 +10081,9 @@ static int StreamTcpTest39 (void)
     StreamTcpThread stt;
     uint8_t payload[4];
     TCPHdr tcph;
-    TcpReassemblyThreadCtx ra_ctx;
     PacketQueue pq;
 
-    memset(&ra_ctx, 0, sizeof(TcpReassemblyThreadCtx));
+    TcpReassemblyThreadCtx *ra_ctx = StreamTcpReassembleInitThreadCtx(NULL);
     memset (&f, 0, sizeof(Flow));
     memset(&tv, 0, sizeof (ThreadVars));
     memset(&stt, 0, sizeof (StreamTcpThread));
@@ -10052,7 +10102,7 @@ static int StreamTcpTest39 (void)
     p->tcph = &tcph;
     p->flowflags = FLOW_PKT_TOSERVER;
     int ret = 0;
-    stt.ra_ctx = &ra_ctx;
+    stt.ra_ctx = ra_ctx;
 
     StreamTcpInitConfig(TRUE);
 
@@ -10081,7 +10131,27 @@ static int StreamTcpTest39 (void)
         goto end;
     }
 
-    p->tcph->th_ack = htonl(2984);
+    p->tcph->th_ack = htonl(1);
+    p->tcph->th_seq = htonl(1);
+    p->tcph->th_flags = TH_PUSH | TH_ACK;
+    p->flowflags = FLOW_PKT_TOCLIENT;
+
+    StreamTcpCreateTestPacket(payload, 0x41, 3, 4); /*AAA*/
+    p->payload = payload;
+    p->payload_len = 3;
+
+    if (StreamTcpPacket(&tv, p, &stt, &pq) == -1) {
+        printf("failed in processing packet in StreamTcpPacket\n");
+        goto end;
+    }
+
+    if (((TcpSession *)(p->flow->protoctx))->server.next_seq != 4) {
+        printf("the server.next_seq should be 4, but it is %"PRIu32"\n",
+                ((TcpSession *)(p->flow->protoctx))->server.next_seq);
+        goto end;
+    }
+
+    p->tcph->th_ack = htonl(4);
     p->tcph->th_seq = htonl(2);
     p->tcph->th_flags = TH_PUSH | TH_ACK;
     p->flowflags = FLOW_PKT_TOSERVER;
@@ -10095,15 +10165,15 @@ static int StreamTcpTest39 (void)
         goto end;
     }
 
-    /* last_ack value should be 2984 as the previous sent ACK value is inside
+    /* last_ack value should be 4 as the previous sent ACK value is inside
        window */
-    if (((TcpSession *)(p->flow->protoctx))->server.last_ack != 2984) {
-        printf("the server.last_ack should be 2984, but it is %"PRIu32"\n",
+    if (((TcpSession *)(p->flow->protoctx))->server.last_ack != 4) {
+        printf("the server.last_ack should be 4, but it is %"PRIu32"\n",
                 ((TcpSession *)(p->flow->protoctx))->server.last_ack);
         goto end;
     }
 
-    p->tcph->th_seq = htonl(2984);
+    p->tcph->th_seq = htonl(4);
     p->tcph->th_ack = htonl(5);
     p->tcph->th_flags = TH_PUSH | TH_ACK;
     p->flowflags = FLOW_PKT_TOCLIENT;
@@ -10119,8 +10189,8 @@ static int StreamTcpTest39 (void)
 
     /* next_seq value should be 2987 as the previous sent ACK value is inside
        window */
-    if (((TcpSession *)(p->flow->protoctx))->server.next_seq != 2987) {
-        printf("the server.next_seq should be 2987, but it is %"PRIu32"\n",
+    if (((TcpSession *)(p->flow->protoctx))->server.next_seq != 7) {
+        printf("the server.next_seq should be 7, but it is %"PRIu32"\n",
                 ((TcpSession *)(p->flow->protoctx))->server.next_seq);
         goto end;
     }
@@ -10133,6 +10203,8 @@ end:
     SCMutexUnlock(&f.m);
     SCFree(p);
     FLOW_DESTROY(&f);
+    if (stt.ra_ctx != NULL)
+        StreamTcpReassembleFreeThreadCtx(stt.ra_ctx);
     return ret;
 }