From 3fe31add5c0b04544f56a74a62c13d09f87f3a38 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 29 Nov 2024 14:37:08 +0100 Subject: [PATCH] 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. Both raw stream inspection and app-layer updates will include the unACK'd data on stream timeout. Since the GAP detection uses `last_ack` to determine GAPs, not moving `last_ack` addresses the GAP false positives. Ticket: #7422. --- src/stream-tcp.c | 96 ++++++++++++++---------------------------------- 1 file changed, 28 insertions(+), 68 deletions(-) diff --git a/src/stream-tcp.c b/src/stream-tcp.c index eafedc187c39..16d1f42ab31e 100644 --- a/src/stream-tcp.c +++ b/src/stream-tcp.c @@ -2995,32 +2995,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) { const TCPHdr *tcph = PacketGetTCP(p); @@ -3242,10 +3216,9 @@ static int StreamTcpPacketStateEstablished( ssn->client.window = window << ssn->client.wscale; if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, window)); + StreamTcpUpdateLastAck(ssn, &ssn->server, ack); - StreamTcpUpdateLastAck(ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->client, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -3270,10 +3243,9 @@ static int StreamTcpPacketStateEstablished( ssn->server.window = window << ssn->server.wscale; if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->client, ack); - StreamTcpUpdateLastAck(ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->server, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -3574,10 +3546,9 @@ static int StreamTcpPacketStateFinWait1( if (PKT_IS_TOSERVER(p)) { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->server, ack); - StreamTcpUpdateLastAck(ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->client, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -3586,10 +3557,9 @@ static int StreamTcpPacketStateFinWait1( StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p); } else { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->client, ack); - StreamTcpUpdateLastAck(ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->server, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -4018,10 +3988,9 @@ static int StreamTcpPacketStateFinWait2( if (PKT_IS_TOSERVER(p)) { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->server, ack); - StreamTcpUpdateLastAck(ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->client, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -4030,10 +3999,9 @@ static int StreamTcpPacketStateFinWait2( StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p); } else { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->client, ack); - StreamTcpUpdateLastAck(ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->server, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -4318,10 +4286,9 @@ static int StreamTcpPacketStateClosing( if (PKT_IS_TOSERVER(p)) { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->server, ack); - StreamTcpUpdateLastAck(ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->client, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -4330,10 +4297,9 @@ static int StreamTcpPacketStateClosing( StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p); } else { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->client, ack); - StreamTcpUpdateLastAck(ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->server, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -4493,10 +4459,9 @@ static int StreamTcpPacketStateCloseWait( if (PKT_IS_TOSERVER(p)) { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->server, ack); - StreamTcpUpdateLastAck(ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->client, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -4505,10 +4470,9 @@ static int StreamTcpPacketStateCloseWait( StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p); } else { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->client, ack); - StreamTcpUpdateLastAck(ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->server, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -4778,10 +4742,9 @@ static int StreamTcpPacketStateLastAck( if (PKT_IS_TOSERVER(p)) { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->server, ack); - StreamTcpUpdateLastAck(ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->client, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -4790,10 +4753,9 @@ static int StreamTcpPacketStateLastAck( StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p); } else { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->client, ack); - StreamTcpUpdateLastAck(ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->server, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -4901,10 +4863,9 @@ static int StreamTcpPacketStateTimeWait( if (PKT_IS_TOSERVER(p)) { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->server, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->server, ack); - StreamTcpUpdateLastAck(ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->client, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p); @@ -4913,10 +4874,9 @@ static int StreamTcpPacketStateTimeWait( StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p); } else { if ((tcph->th_flags & TH_ACK) && StreamTcpValidateAck(ssn, &ssn->client, p) == 0) - StreamTcpUpdateLastAck( - ssn, &ssn->client, StreamTcpResetGetMaxAck(&ssn->client, ack)); + StreamTcpUpdateLastAck(ssn, &ssn->client, ack); - StreamTcpUpdateLastAck(ssn, &ssn->server, StreamTcpResetGetMaxAck(&ssn->server, seq)); + StreamTcpUpdateLastAck(ssn, &ssn->server, seq); if (ssn->flags & STREAMTCP_FLAG_TIMESTAMP) { StreamTcpHandleTimestamp(ssn, p);