From 843c4b20da3e1389d2b1be942b536c0c3c77a954 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Mon, 4 Jan 2021 22:10:08 +0100 Subject: [PATCH] stream: check if ACK packet is outdated Outdated packets are ACK packets w/o data that have an ACK value lower than our last_ack and also don't have an SACK records that are new. This can happen when some packets come in later than others (possibly due to different paths taken). --- src/stream-tcp-sack.c | 79 +++++++++++++++++++++++++++++++++++++++++++ src/stream-tcp-sack.h | 1 + src/stream-tcp.c | 68 +++++++++++++++++++++++++++++++++++-- 3 files changed, 146 insertions(+), 2 deletions(-) diff --git a/src/stream-tcp-sack.c b/src/stream-tcp-sack.c index f26c5ebab0..9a36a94958 100644 --- a/src/stream-tcp-sack.c +++ b/src/stream-tcp-sack.c @@ -296,6 +296,85 @@ int StreamTcpSackUpdatePacket(TcpStream *stream, Packet *p) SCReturnInt(0); } +static inline int CompareOverlap( + struct StreamTcpSackRecord *lookup, struct StreamTcpSackRecord *intree) +{ + if (lookup->re <= intree->le) // entirely before + return -1; + else if (lookup->re >= intree->le && lookup->le < intree->re) // (some) overlap + return 0; + else + return 1; // entirely after +} + +static struct StreamTcpSackRecord *FindOverlap( + struct TCPSACK *head, struct StreamTcpSackRecord *elm) +{ + SCLogDebug("looking up le:%u re:%u", elm->le, elm->re); + + struct StreamTcpSackRecord *tmp = RB_ROOT(head); + struct StreamTcpSackRecord *res = NULL; + while (tmp) { + SCLogDebug("compare with le:%u re:%u", tmp->le, tmp->re); + const int comp = CompareOverlap(elm, tmp); + SCLogDebug("compare result: %d", comp); + if (comp < 0) { + res = tmp; + tmp = RB_LEFT(tmp, rb); + } else if (comp > 0) { + tmp = RB_RIGHT(tmp, rb); + } else { + return tmp; + } + } + return res; +} + +bool StreamTcpSackPacketIsOutdated(TcpStream *stream, Packet *p) +{ + const int records = TCP_GET_SACK_CNT(p); + const uint8_t *data = TCP_GET_SACK_PTR(p); + if (records > 0 && data != NULL) { + int sack_outdated = 0; + TCPOptSackRecord rec[records], *sack_rec = rec; + memcpy(&rec, data, sizeof(TCPOptSackRecord) * records); + for (int record = 0; record < records; record++) { + const uint32_t le = SCNtohl(sack_rec->le); + const uint32_t re = SCNtohl(sack_rec->re); + SCLogDebug("%p last_ack %u, left edge %u, right edge %u", sack_rec, stream->last_ack, + le, re); + + struct StreamTcpSackRecord lookup = { .le = le, .re = re }; + struct StreamTcpSackRecord *res = FindOverlap(&stream->sack_tree, &lookup); + SCLogDebug("res %p", res); + if (res) { + if (le >= res->le && re <= res->re) { + SCLogDebug("SACK rec le:%u re:%u eclipsed by in tree le:%u re:%u", le, re, + res->le, res->re); + sack_outdated++; + } else { + SCLogDebug("SACK rec le:%u re:%u SACKs new DATA vs in tree le:%u re:%u", le, re, + res->le, res->re); + } + } else { + SCLogDebug("SACK rec le:%u re:%u SACKs new DATA. No match in tree", le, re); + } + sack_rec++; + } +#ifdef DEBUG + StreamTcpSackPrintList(stream); +#endif + if (records != sack_outdated) { + // SACK tree needs updating + return false; + } else { + // SACK list is packet is completely outdated + return true; + } + } + return false; +} + void StreamTcpSackPruneList(TcpStream *stream) { SCEnter(); diff --git a/src/stream-tcp-sack.h b/src/stream-tcp-sack.h index d1ba627243..84e7cb1bdf 100644 --- a/src/stream-tcp-sack.h +++ b/src/stream-tcp-sack.h @@ -39,6 +39,7 @@ static inline uint32_t StreamTcpSackedSize(TcpStream *stream) } int StreamTcpSackUpdatePacket(TcpStream *, Packet *); +bool StreamTcpSackPacketIsOutdated(TcpStream *stream, Packet *p); void StreamTcpSackPruneList(TcpStream *); void StreamTcpSackFreeList(TcpStream *); void StreamTcpSackRegisterTests (void); diff --git a/src/stream-tcp.c b/src/stream-tcp.c index 277d9834b4..29ed3c9f69 100644 --- a/src/stream-tcp.c +++ b/src/stream-tcp.c @@ -2510,6 +2510,66 @@ static inline uint32_t StreamTcpResetGetMaxAck(TcpStream *stream, uint32_t seq) SCReturnUInt(ack); } +/** \internal + * \brief check if a ACK packet is outdated so processing can be fast tracked + * + * Consider a packet outdated ack if: + * - state is >= ESTABLISHED + * - ACK < last_ACK + * - SACK acks nothing new + * - packet has no data + * - SEQ == next_SEQ + * - flags has ACK set but don't contain SYN/FIN/RST + * + * \todo the most likely explanation for this packet is that we already + * accepted a "newer" ACK. We will not consider an outdated timestamp + * option an issue for this packet, but we should probably still + * check if the ts isn't too far off. + */ +static bool StreamTcpPacketIsOutdatedAck(TcpSession *ssn, Packet *p) +{ + if (ssn->state < TCP_ESTABLISHED) + return false; + if (p->payload_len != 0) + return false; + if ((p->tcph->th_flags & (TH_ACK | TH_SYN | TH_FIN | TH_RST)) != TH_ACK) + return false; + + /* lets see if this is a packet that is entirely eclipsed by earlier ACKs */ + if (PKT_IS_TOSERVER(p)) { + if (SEQ_EQ(TCP_GET_SEQ(p), ssn->client.next_seq) && + SEQ_LT(TCP_GET_ACK(p), ssn->server.last_ack)) { + if (!TCP_HAS_SACK(p)) { + SCLogDebug("outdated ACK (no SACK, SEQ %u vs next_seq %u)", TCP_GET_SEQ(p), + ssn->client.next_seq); + return true; + } + + if (StreamTcpSackPacketIsOutdated(&ssn->server, p)) { + SCLogDebug("outdated ACK (have SACK, SEQ %u vs next_seq %u)", TCP_GET_SEQ(p), + ssn->client.next_seq); + return true; + } + } + } else { + if (SEQ_EQ(TCP_GET_SEQ(p), ssn->server.next_seq) && + SEQ_LT(TCP_GET_ACK(p), ssn->client.last_ack)) { + if (!TCP_HAS_SACK(p)) { + SCLogDebug("outdated ACK (no SACK, SEQ %u vs next_seq %u)", TCP_GET_SEQ(p), + ssn->client.next_seq); + return true; + } + + if (StreamTcpSackPacketIsOutdated(&ssn->client, p)) { + SCLogDebug("outdated ACK (have SACK, SEQ %u vs next_seq %u)", TCP_GET_SEQ(p), + ssn->client.next_seq); + return true; + } + } + } + return false; +} + /** * \brief Function to handle the TCP_ESTABLISHED state. The function handles * ACK, FIN, RST packets and correspondingly changes the connection @@ -4893,10 +4953,14 @@ int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt, /* if packet is not a valid window update, check if it is perhaps * a bad window update that we should ignore (and alert on) */ - if (StreamTcpPacketIsFinShutdownAck(ssn, p) == 0) - if (StreamTcpPacketIsWindowUpdate(ssn, p) == 0) + if (StreamTcpPacketIsFinShutdownAck(ssn, p) == 0) { + if (StreamTcpPacketIsWindowUpdate(ssn, p) == 0) { if (StreamTcpPacketIsBadWindowUpdate(ssn,p)) goto skip; + if (StreamTcpPacketIsOutdatedAck(ssn, p)) + goto skip; + } + } /* handle the per 'state' logic */ if (StreamTcpStateDispatch(tv, p, stt, ssn, &stt->pseudo_queue, ssn->state) < 0) -- 2.47.2