Skip to content

Commit

Permalink
stream: improve 3whs completed by ACK with data
Browse files Browse the repository at this point in the history
If the ACK packet completing the 3whs is received, the stream engine will
transition to "established". However, the packet itself will not be tagged
as "established". This will only happen for the next packet after the 3whs,
so that `flow:established` only matches after the 3whs.

It is possible that the ACK packet completing the 3whs was lost. Since the
ACK packets themselves are not acknowledged, there will be no retransmission
of them. Instead, the next packet can have the expected ACK flag as well as
data.

This case was mishandled in a subtle way. The stream engine state transition
was done correctly, as well as the data handling and app-layer updates.
However, the packet itself was not tagged as "established", which meant
that `flow:established` would not yet match.

This patch detects this case and tags the packet as established if ACK
with data is received that completes the 3whs.

Bug: OISF#7264.
  • Loading branch information
victorjulien committed Sep 24, 2024
1 parent 521928e commit 45eb7e4
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/stream-tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -2520,6 +2520,14 @@ static int StreamTcpPacketStateSynRecv(
StreamTcpPacketSetState(p, ssn, TCP_ESTABLISHED);
SCLogDebug("ssn %p: =~ ssn state is now TCP_ESTABLISHED", ssn);

/* special case: normally the packet following the 3whs is
* considered flow established, but with data we need it to
* be established now. This can happen if the original ACK was
* lost. */
if (p->payload_len) {
p->flowflags |= FLOW_PKT_ESTABLISHED;
}

StreamTcpReassembleHandleSegment(tv, stt->ra_ctx, ssn, &ssn->client, p);

/* If asynchronous stream handling is allowed then set the session,
Expand Down

0 comments on commit 45eb7e4

Please sign in to comment.