From d00468742fe63a27bd37a32a4cdc05d126d7daf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eug=C3=A8ne=20Adell?= Date: Sun, 11 Jun 2023 17:11:26 +0000 Subject: [PATCH] TCP: Zero Window Probe ACK detection for improper clients --- docbook/wsug_src/wsug_advanced.adoc | 3 +- epan/dissectors/packet-tcp.c | 81 ++++++++++++++++++++--------- 2 files changed, 57 insertions(+), 27 deletions(-) diff --git a/docbook/wsug_src/wsug_advanced.adoc b/docbook/wsug_src/wsug_advanced.adoc index 3334240108..e28a416775 100644 --- a/docbook/wsug_src/wsug_advanced.adoc +++ b/docbook/wsug_src/wsug_advanced.adoc @@ -643,7 +643,8 @@ Set when the all of the following are true: * The segment size is zero. * The window size is non-zero and not equal to the last-seen window size. * The sequence number is equal to the next expected sequence number. -* The acknowledgment number is equal to the last-seen acknowledgment number. +* The acknowledgment number is equal to the last-seen acknowledgment number, +* or to the next expected sequence number when answering to a ZeroWindowProbe. * None of SYN, FIN, or RST are set. // TCP_A_ZERO_WINDOW diff --git a/epan/dissectors/packet-tcp.c b/epan/dissectors/packet-tcp.c index 43cce55feb..c65d2fad67 100644 --- a/epan/dissectors/packet-tcp.c +++ b/epan/dissectors/packet-tcp.c @@ -2274,13 +2274,23 @@ tcp_analyze_sequence_number(packet_info *pinfo, guint32 seq, guint32 ack, guint3 && window==0 && window==tcpd->fwd->window && seq==tcpd->fwd->tcp_analyze_seq_info->nextseq - && ack==tcpd->fwd->tcp_analyze_seq_info->lastack + && (ack==tcpd->fwd->tcp_analyze_seq_info->lastack || EQ_SEQ(ack,tcpd->fwd->tcp_analyze_seq_info->lastack+1)) && (tcpd->rev->lastsegmentflags&TCP_A_ZERO_WINDOW_PROBE) && (flags&(TH_SYN|TH_FIN|TH_RST))==0 ) { if(!tcpd->ta) { tcp_analyze_get_acked_struct(pinfo->num, seq, ack, TRUE, tcpd); } tcpd->ta->flags|=TCP_A_ZERO_WINDOW_PROBE_ACK; + + /* Some receivers consume that extra byte brought in the PROBE, + * but it was too early to know that during the WINDOW PROBE analysis. + * Do it now by moving the rev nextseq & maxseqtobeacked. + * See issue 10745. + */ + if(EQ_SEQ(ack,tcpd->fwd->tcp_analyze_seq_info->lastack+1)) { + tcpd->rev->tcp_analyze_seq_info->nextseq=ack; + tcpd->rev->tcp_analyze_seq_info->maxseqtobeacked=ack; + } goto finished_fwd; } @@ -2324,7 +2334,10 @@ finished_fwd: /* ACKED LOST PACKET * If this segment acks beyond the 'max seq to be acked' in the other direction * then that means we have missed packets going in the - * other direction + * other direction. + * It might also indicate we are resuming from a Zero Window, + * where a Probe is just followed by an ACK opening again the window. + * See issue 8404. * * We only check this if we have actually seen some seq numbers * in the other direction. @@ -2336,33 +2349,46 @@ finished_fwd: tcp_analyze_get_acked_struct(pinfo->num, seq, ack, TRUE, tcpd); } - /* We ensure there is no matching packet waiting in the unacked list, - * and take this opportunity to push the tail further than this single packet + /* resuming from a Zero Window Probe which re-opens the window, + * mark it as a Window Update */ - gboolean is_seq_in_unacked = FALSE; - guint32 maxseqtail = ack; - ual = tcpd->rev->tcp_analyze_seq_info->segments; - while(ual) { - /* prevent false positives */ - if(GT_SEQ(ack,ual->seq) && LE_SEQ(ack,ual->nextseq)) { - is_seq_in_unacked = TRUE; - } - /* look for a possible tail pushing the maxseqtobeacked further */ - if(maxseqtail==ual->seq) { - maxseqtail = ual->nextseq; - } - ual=ual->next; - } - - /* update 'max seq to be acked' in the other direction so we don't get - * this indication again. - */ - if(is_seq_in_unacked) { - tcpd->rev->tcp_analyze_seq_info->maxseqtobeacked=(GT_SEQ(maxseqtail, ack)) ? ack : maxseqtail; + if(EQ_SEQ(ack,tcpd->fwd->tcp_analyze_seq_info->lastack+1) + && (seq==tcpd->fwd->tcp_analyze_seq_info->nextseq) + && (tcpd->rev->lastsegmentflags&TCP_A_ZERO_WINDOW_PROBE) ) { + tcpd->rev->tcp_analyze_seq_info->nextseq=ack; + tcpd->rev->tcp_analyze_seq_info->maxseqtobeacked=ack; + tcpd->ta->flags|=TCP_A_WINDOW_UPDATE; } + /* real ACKED LOST PACKET */ else { - tcpd->rev->tcp_analyze_seq_info->maxseqtobeacked=maxseqtail; - tcpd->ta->flags|=TCP_A_ACK_LOST_PACKET; + /* We ensure there is no matching packet waiting in the unacked list, + * and take this opportunity to push the tail further than this single packet + */ + gboolean is_seq_in_unacked = FALSE; + guint32 maxseqtail = ack; + ual = tcpd->rev->tcp_analyze_seq_info->segments; + while(ual) { + /* prevent false positives */ + if(GT_SEQ(ack,ual->seq) && LE_SEQ(ack,ual->nextseq)) { + is_seq_in_unacked = TRUE; + } + /* look for a possible tail pushing the maxseqtobeacked further */ + if(maxseqtail==ual->seq) { + maxseqtail = ual->nextseq; + } + ual=ual->next; + } + + /* update 'max seq to be acked' in the other direction so we don't get + * this indication again. + */ + if(is_seq_in_unacked) { + tcpd->rev->tcp_analyze_seq_info->maxseqtobeacked=(GT_SEQ(maxseqtail, ack)) ? ack : maxseqtail; + } + else { + tcpd->rev->tcp_analyze_seq_info->maxseqtobeacked=maxseqtail; + tcpd->ta->flags|=TCP_A_ACK_LOST_PACKET; + } } } @@ -2626,6 +2652,9 @@ finished_checking_retransmission_type: * If this ever happens, this boundary value can "jump" further in order to * avoid duplicating multiple messages for the very same lost packet. See later * how ACKED LOST PACKET are handled. + * Zero Window Probes are logically left out at this moment, but if their data + * really were to be ack'ed, then it will be done later when analyzing their + * Probe ACK (be it a real Probe ACK, or an ordinary ACK moving the RCV Window). */ if(EQ_SEQ(seq, tcpd->fwd->tcp_analyze_seq_info->maxseqtobeacked) || !tcpd->fwd->tcp_analyze_seq_info->maxseqtobeacked) { if( !tcpd->ta || !(tcpd->ta->flags&TCP_A_ZERO_WINDOW_PROBE) ) {