TCP: Duplicate ACKs hidden by Window Update and SACK presence

2 files changed, 54 insertions, 2 deletions

diff --git a/docbook/wsug_src/wsug_advanced.adoc b/docbook/wsug_src/wsug_advanced.adoc
index dde90aedf6..82a514c4a5 100644
--- a/docbook/wsug_src/wsug_advanced.adoc
+++ b/docbook/wsug_src/wsug_advanced.adoc
@@ -532,7 +532,7 @@ direction and it’s less than the current acknowledgment number.
 Set when all of the following are true:
 
 * The segment size is zero.
-* The window size is non-zero and hasn’t changed.
+* The window size is non-zero and hasn’t changed, or there is valid SACK data.
 * The next expected sequence number and last-seen acknowledgment number are non-zero (i.e., the connection has been established).
 * SYN, FIN, and RST are not set.
 
@@ -644,7 +644,7 @@ reverse direction.
 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 window size is non-zero and not equal to the last-seen window size, and there is no valid SACK data.
 * The sequence number is equal to the next expected sequence number.
 * The acknowledgment number is equal to the last-seen acknowledgment number,
 * or to the next expected sequence number when answering to a ZeroWindowProbe.
diff --git a/epan/dissectors/packet-tcp.c b/epan/dissectors/packet-tcp.c
index 83b5ce97b2..2e70592a04 100644
--- a/epan/dissectors/packet-tcp.c
+++ b/epan/dissectors/packet-tcp.c
@@ -2525,6 +2525,8 @@ finished_fwd:
             goto finished_checking_retransmission_type;
         }
 
+        nextseq = seq+seglen;
+
         gboolean precedence_count = tcp_fastrt_precedence;
         do {
             switch(precedence_count) {
@@ -2548,6 +2550,31 @@ finished_fwd:
                         tcpd->ta->flags|=TCP_A_FAST_RETRANSMISSION;
                         goto finished_checking_retransmission_type;
                     }
+
+                    /* Look for this segment in reported SACK ranges,
+                     * if not present this might very well be a FAST Retrans,
+                     * when the conditions above (timing, number of retrans) are still true */
+                    if( seq_not_advanced
+                    &&  t<20000000
+                    &&  tcpd->rev->tcp_analyze_seq_info->dupacknum>=2
+                    &&  tcpd->rev->tcp_analyze_seq_info->num_sack_ranges > 0) {
+
+                        gboolean is_sacked = FALSE;
+                        int i=0;
+                        while( !is_sacked && i<tcpd->rev->tcp_analyze_seq_info->num_sack_ranges ) {
+                            is_sacked = ((seq >= tcpd->rev->tcp_analyze_seq_info->sack_left_edge[i++])
+                                        && (nextseq <= tcpd->rev->tcp_analyze_seq_info->sack_right_edge[i]));
+                        }
+
+                        /* fine, it's probably a Fast Retrans triggered by the SACK sender algo */
+                        if(!is_sacked) {
+                            if(!tcpd->ta)
+                                tcp_analyze_get_acked_struct(pinfo->num, seq, ack, TRUE, tcpd);
+                            tcpd->ta->flags|=TCP_A_FAST_RETRANSMISSION;
+                            goto finished_checking_retransmission_type;
+                        }
+                    }
+
                     precedence_count=!precedence_count;
                     break;
 
@@ -5596,6 +5623,31 @@ dissect_tcpopt_sack(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* d
         }
     }
 
+    /* Late discovery of a 'false' Window Update in presence of SACK option,
+     * which means we are dealing with a Dup ACK rather than a Window Update.
+     * Classify accordingly by removing the UPDATE and adding the DUP flags.
+     * Mostly a copy/paste from tcp_analyze_sequence_number(), ensure consistency
+     * whenever the latter changes.
+     * see Issue #14937
+     */
+    if( tcp_analyze_seq && tcpd && tcpd->ta && tcpd->ta->flags&TCP_A_WINDOW_UPDATE ) {
+
+        /* MPTCP tolerates duplicate acks in some circumstances, see RFC 8684 4. */
+        if(tcpd->mptcp_analysis && (tcpd->mptcp_analysis->mp_operations!=tcpd->fwd->mp_operations)) {
+            /* just ignore this DUPLICATE ACK */
+        } else {
+            tcpd->fwd->tcp_analyze_seq_info->dupacknum++;
+
+            /* no initialization required of the tcpd->ta as this code would
+             * be unreachable otherwise
+             */
+            tcpd->ta->flags &= ~TCP_A_WINDOW_UPDATE;
+            tcpd->ta->flags |= TCP_A_DUPLICATE_ACK;
+            tcpd->ta->dupack_num=tcpd->fwd->tcp_analyze_seq_info->dupacknum;
+            tcpd->ta->dupack_frame=tcpd->fwd->tcp_analyze_seq_info->lastnondupack;
+       }
+    }
+
     ti = proto_tree_add_item(tree, proto_tcp_option_sack, tvb, offset, -1, ENC_NA);
     field_tree = proto_item_add_subtree(ti, ett_tcp_option_sack);