stream: RST no longer acks all data

Since forever (1578ef1e3e8a24d0cc615430c4e6bec1fefdad28) a valid RST
would update the internal `last_ack` representation to include all
unack'd data. This was originally done to make sure the unACK'd data was
inspected/processed at flow timeout.

It was observed however, that if GAPs existed in this unACK'd data, a
GAP could be reported in the stats and a GAP event would be raised. This
doesn't make sense, as missing segments in the unACK'd part of the
stream are completely normal. Segments simply do not all arrive in
order.

It turns out that the original behavior of updating `last_ack` to
include all unACK'd data is no longer needed.

For raw stream inspection, the detection engine will already include the
unACK'd data on flow end.

For app-layer updates the unACK'd data is often harmful, as the data
often has GAPs. Parser like the http parser would report these GAPs and
could also get confused about the post-GAP data being a new transaction
including a file. This lead to many reported errors and fantom txs and
files.

Since the GAP detection uses `last_ack` to determine GAPs, not moving
`last_ack` addresses the GAP false positives.

Ticket: #7422.
(cherry picked from commit bd1b9f62299422767056651f8f66d2bbd4e0bb14)

diff --git a/src/stream-tcp.c b/src/stream-tcp.c
index b847fb6f8a001380b97982ae7ce7acc2bc39de13..f179b41bfcb450159fc0012ee28422ae620ed81e 100644 (file)
--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@@ -2900,32 +2900,6 @@ static int HandleEstablishedPacketToClient(
     return 0;
 }
 
-/**
- *  \internal
- *
- *  \brief Find the highest sequence number needed to consider all segments as ACK'd
- *
- *  Used to treat all segments as ACK'd upon receiving a valid RST.
- *
- *  \param stream stream to inspect the segments from
- *  \param seq sequence number to check against
- *
- *  \retval ack highest ack we need to set
- */
-static inline uint32_t StreamTcpResetGetMaxAck(TcpStream *stream, uint32_t seq)
-{
-    uint32_t ack = seq;
-
-    if (STREAM_HAS_SEEN_DATA(stream)) {
-        const uint32_t tail_seq = STREAM_SEQ_RIGHT_EDGE(stream);
-        if (SEQ_GT(tail_seq, ack)) {
-            ack = tail_seq;
-        }
-    }
-
-    SCReturnUInt(ack);
-}
-
 static bool StreamTcpPacketIsZeroWindowProbeAck(const TcpSession *ssn, const Packet *p)
 {
     if (ssn->state < TCP_ESTABLISHED)
@@ -3124,6 +3098,9 @@ static int StreamTcpPacketStateEstablished(
 {
     DEBUG_VALIDATE_BUG_ON(ssn == NULL);
 
+    const uint32_t seq = TCP_GET_SEQ(p);
+    const uint32_t ack = TCP_GET_ACK(p);
+
     if (p->tcph->th_flags & TH_RST) {
         if (!StreamTcpValidateRst(ssn, p))
             return -1;
@@ -3138,11 +3115,9 @@ static int StreamTcpPacketStateEstablished(
             ssn->client.window = TCP_GET_WINDOW(p) << ssn->client.wscale;
 
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->server,
-                        StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->server, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->client,
-                    StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->client, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -3167,11 +3142,9 @@ static int StreamTcpPacketStateEstablished(
             ssn->server.window = TCP_GET_WINDOW(p) << ssn->server.wscale;
 
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->client,
-                        StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->client, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->server,
-                    StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->server, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -3456,6 +3429,9 @@ static int StreamTcpPacketStateFinWait1(
 {
     DEBUG_VALIDATE_BUG_ON(ssn == NULL);
 
+    const uint32_t seq = TCP_GET_SEQ(p);
+    const uint32_t ack = TCP_GET_ACK(p);
+
     if (p->tcph->th_flags & TH_RST) {
         if (!StreamTcpValidateRst(ssn, p))
             return -1;
@@ -3464,11 +3440,9 @@ static int StreamTcpPacketStateFinWait1(
 
         if (PKT_IS_TOSERVER(p)) {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->server,
-                        StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->server, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->client,
-                    StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->client, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -3477,11 +3451,9 @@ static int StreamTcpPacketStateFinWait1(
             StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p);
         } else {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->client,
-                        StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->client, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->server,
-                    StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->server, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -3900,6 +3872,9 @@ static int StreamTcpPacketStateFinWait2(
 {
     DEBUG_VALIDATE_BUG_ON(ssn == NULL);
 
+    const uint32_t seq = TCP_GET_SEQ(p);
+    const uint32_t ack = TCP_GET_ACK(p);
+
     if (p->tcph->th_flags & TH_RST) {
         if (!StreamTcpValidateRst(ssn, p))
             return -1;
@@ -3908,11 +3883,9 @@ static int StreamTcpPacketStateFinWait2(
 
         if (PKT_IS_TOSERVER(p)) {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->server,
-                        StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->server, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->client,
-                    StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->client, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -3921,11 +3894,9 @@ static int StreamTcpPacketStateFinWait2(
             StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p);
         } else {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->client,
-                        StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->client, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->server,
-                    StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->server, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -4202,6 +4173,9 @@ static int StreamTcpPacketStateClosing(
 {
     DEBUG_VALIDATE_BUG_ON(ssn == NULL);
 
+    const uint32_t seq = TCP_GET_SEQ(p);
+    const uint32_t ack = TCP_GET_ACK(p);
+
     if (p->tcph->th_flags & TH_RST) {
         if (!StreamTcpValidateRst(ssn, p))
             return -1;
@@ -4210,11 +4184,9 @@ static int StreamTcpPacketStateClosing(
 
         if (PKT_IS_TOSERVER(p)) {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->server,
-                        StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->server, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->client,
-                    StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->client, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -4223,11 +4195,9 @@ static int StreamTcpPacketStateClosing(
             StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p);
         } else {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->client,
-                        StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->client, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->server,
-                    StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->server, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -4365,6 +4335,9 @@ static int StreamTcpPacketStateCloseWait(
 
     DEBUG_VALIDATE_BUG_ON(ssn == NULL);
 
+    const uint32_t seq = TCP_GET_SEQ(p);
+    const uint32_t ack = TCP_GET_ACK(p);
+
     if (PKT_IS_TOCLIENT(p)) {
         SCLogDebug("ssn %p: pkt (%" PRIu32 ") is to client: SEQ "
                 "%" PRIu32 ", ACK %" PRIu32 "", ssn, p->payload_len,
@@ -4383,11 +4356,9 @@ static int StreamTcpPacketStateCloseWait(
 
         if (PKT_IS_TOSERVER(p)) {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->server,
-                        StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->server, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->client,
-                    StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->client, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -4396,11 +4367,9 @@ static int StreamTcpPacketStateCloseWait(
             StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p);
         } else {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->client,
-                        StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->client, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->server,
-                    StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->server, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -4662,6 +4631,9 @@ static int StreamTcpPacketStateLastAck(
 {
     DEBUG_VALIDATE_BUG_ON(ssn == NULL);
 
+    const uint32_t seq = TCP_GET_SEQ(p);
+    const uint32_t ack = TCP_GET_ACK(p);
+
     if (p->tcph->th_flags & TH_RST) {
         if (!StreamTcpValidateRst(ssn, p))
             return -1;
@@ -4670,11 +4642,9 @@ static int StreamTcpPacketStateLastAck(
 
         if (PKT_IS_TOSERVER(p)) {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->server,
-                        StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->server, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->client,
-                    StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->client, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -4683,11 +4653,9 @@ static int StreamTcpPacketStateLastAck(
             StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p);
         } else {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->client,
-                        StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->client, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->server,
-                    StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->server, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -4784,6 +4752,9 @@ static int StreamTcpPacketStateTimeWait(
 {
     DEBUG_VALIDATE_BUG_ON(ssn == NULL);
 
+    const uint32_t seq = TCP_GET_SEQ(p);
+    const uint32_t ack = TCP_GET_ACK(p);
+
     if (p->tcph->th_flags & TH_RST) {
         if (!StreamTcpValidateRst(ssn, p))
             return -1;
@@ -4792,11 +4763,9 @@ static int StreamTcpPacketStateTimeWait(
 
         if (PKT_IS_TOSERVER(p)) {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->server,
-                        StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->server, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->client,
-                    StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->client, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);
@@ -4805,11 +4774,9 @@ static int StreamTcpPacketStateTimeWait(
             StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p);
         } else {
             if ((p->tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0)
-                StreamTcpUpdateLastAck(ssn, &ssn->client,
-                        StreamTcpResetGetMaxAck(&ssn->client, TCP_GET_ACK(p)));
+                StreamTcpUpdateLastAck(ssn, &ssn->client, ack);
 
-            StreamTcpUpdateLastAck(ssn, &ssn->server,
-                    StreamTcpResetGetMaxAck(&ssn->server, TCP_GET_SEQ(p)));
+            StreamTcpUpdateLastAck(ssn, &ssn->server, seq);
 
             if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) {
                 StreamTcpHandleTimestamp(ssn, p);