From 76225bf9ac87ed4312d44b83d9499794cc760207 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Sat, 18 Feb 2023 15:36:55 +0100 Subject: [PATCH] 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. --- src/stream-tcp.c | 73 +++++++----------------------------------- src/tests/stream-tcp.c | 10 +++--- 2 files changed, 17 insertions(+), 66 deletions(-) diff --git a/src/stream-tcp.c b/src/stream-tcp.c index 686381d2c7..464ef75292 100644 --- a/src/stream-tcp.c +++ b/src/stream-tcp.c @@ -2573,13 +2573,11 @@ static int HandleEstablishedPacketToServer( * beyond next seq */ } 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 "), " @@ -2601,31 +2599,8 @@ static int HandleEstablishedPacketToServer( 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)) - { - 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)) { + } 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 - " (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 */ @@ -2737,13 +2712,11 @@ static int HandleEstablishedPacketToClient( } 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, @@ -2759,30 +2732,8 @@ static int HandleEstablishedPacketToClient( 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)) - { + } 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 - " (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)) { - 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) { diff --git a/src/tests/stream-tcp.c b/src/tests/stream-tcp.c index d5c9fecfb7..c9ab17d71b 100644 --- a/src/tests/stream-tcp.c +++ b/src/tests/stream-tcp.c @@ -781,12 +781,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);