[!] fix: randomly skipping packet numbers to align with RFC9000 21.4 (#483)

Author: yangfurong

Diff for: include/xquic/xquic.h

@@ -1488,6 +1488,8 @@ typedef struct xqc_conn_settings_s {
     uint64_t                    extended_ack_features;
     uint64_t                    max_receive_timestamps_per_ack;
     uint64_t                    receive_timestamps_exponent;
+
+    uint8_t                     disable_pn_skipping;
 } xqc_conn_settings_t;
 
 

Diff for: src/transport/xqc_conn.c

@@ -114,7 +114,8 @@ xqc_conn_settings_t internal_default_conn_settings = {
 
     .extended_ack_features     = 0,
     .max_receive_timestamps_per_ack    = 0,
-    .receive_timestamps_exponent       = 0
+    .receive_timestamps_exponent       = 0,
+    .disable_pn_skipping               = 0
 };
 
 
@@ -2456,7 +2457,8 @@ xqc_conn_enc_packet(xqc_connection_t *conn,
 
     /* generate packet number and update packet length, might do packet number encoding here */
     xqc_pn_ctl_t *pn_ctl = xqc_get_pn_ctl(conn, path);
-    packet_out->po_pkt.pkt_num = pn_ctl->ctl_packet_number[packet_out->po_pkt.pkt_pns]++;
+    xqc_send_ctl_set_next_pn_for_packet(conn, pn_ctl, packet_out, current_time);
+
     xqc_write_packet_number(packet_out->po_ppktno, packet_out->po_pkt.pkt_num, XQC_PKTNO_BITS);
     xqc_long_packet_update_length(packet_out);
     xqc_short_packet_update_key_phase(packet_out, conn->key_update_ctx.cur_out_key_phase);
@@ -2590,7 +2592,6 @@ xqc_send_packet_with_pn(xqc_connection_t *conn, xqc_path_ctx_t *path, xqc_packet
 
     /* deliver packet to send control */
     xqc_pn_ctl_t *pn_ctl = xqc_get_pn_ctl(conn, path);
-    pn_ctl->ctl_packet_number[packet_out->po_pkt.pkt_pns]++;
 
     xqc_conn_log_sent_packet(conn, packet_out, now);
     xqc_send_ctl_on_packet_sent(path->path_send_ctl, pn_ctl, packet_out, now);
@@ -2620,7 +2621,9 @@ xqc_enc_packet_with_pn(xqc_connection_t *conn, xqc_path_ctx_t *path, xqc_packet_
 
     /* generate packet number */
     xqc_pn_ctl_t *pn_ctl = xqc_get_pn_ctl(conn, path);
-    packet_out->po_pkt.pkt_num = pn_ctl->ctl_packet_number[packet_out->po_pkt.pkt_pns];
+    xqc_usec_t current_time = xqc_monotonic_timestamp();
+    
+    xqc_send_ctl_set_next_pn_for_packet(conn, pn_ctl, packet_out, current_time);
     xqc_write_packet_number(packet_out->po_ppktno, packet_out->po_pkt.pkt_num, XQC_PKTNO_BITS);
     xqc_long_packet_update_length(packet_out);
     xqc_short_packet_update_key_phase(packet_out, conn->key_update_ctx.cur_out_key_phase);
@@ -6368,7 +6371,6 @@ xqc_conn_send_path_challenge(xqc_connection_t *conn, xqc_path_ctx_t *path)
     xqc_packet_out_t   *packet_out;
     xqc_usec_t          now;
     ssize_t             sent;
-    xqc_pn_ctl_t       *pn_ctl;
 
 
     /* send data */
@@ -6434,9 +6436,6 @@ xqc_conn_send_path_challenge(xqc_connection_t *conn, xqc_path_ctx_t *path)
                 xqc_frame_type_2_str(conn->engine, packet_out->po_frame_types), path->path_send_ctl->ctl_bytes_in_flight, now);
     }
 
-    pn_ctl = xqc_get_pn_ctl(conn, path);
-    pn_ctl->ctl_packet_number[packet_out->po_pkt.pkt_pns]++;
-
 end:
     xqc_send_queue_remove_send(&packet_out->po_list);
     xqc_send_queue_insert_free(packet_out, &conn->conn_send_queue->sndq_free_packets, conn->conn_send_queue);

Diff for: src/transport/xqc_packet_parser.c

@@ -138,11 +138,14 @@ xqc_packet_decode_packet_number(xqc_packet_number_t largest_pn, xqc_packet_numbe
 
     candidate_pn = (expected_pn & ~pn_mask) | truncated_pn;
 
-    if (candidate_pn + pn_hwin <= expected_pn) {
+    // To fully align with RFC9000
+    if ((candidate_pn + pn_hwin <= expected_pn)
+        && (candidate_pn < ((1ULL << 62) - pn_win))) 
+    {
         return candidate_pn + pn_win;
     }
 
-    if (candidate_pn > expected_pn + pn_hwin && candidate_pn > pn_win) {
+    if (candidate_pn > expected_pn + pn_hwin && candidate_pn >= pn_win) {
         return candidate_pn - pn_win;
     }
 

Diff for: src/transport/xqc_send_ctl.c

@@ -778,15 +778,43 @@ xqc_send_ctl_on_packet_sent(xqc_send_ctl_t *send_ctl, xqc_pn_ctl_t *pn_ctl, xqc_
     }
 }
 
+
+xqc_int_t 
+xqc_send_ctl_detect_optimistic_ack_attack(xqc_send_ctl_t *send_ctl, 
+    xqc_pn_ctl_t *pn_ctl, xqc_ack_info_t *const ack_info, xqc_usec_t ack_recv_time)
+{
+    xqc_connection_t *conn = send_ctl->ctl_conn;
+    xqc_pktno_range_t *range = NULL;
+    xqc_pkt_num_space_t pns = ack_info->pns;
+    int i;
+    
+    if (!conn->conn_settings.disable_pn_skipping
+        /* only check application data */
+        && pns == XQC_PNS_APP_DATA 
+        /* if ack is received after the next skip chance, 
+           it is unlikely to be optimistic ack */
+        && ack_recv_time < pn_ctl->ctl_next_skip_chance) 
+    {
+        for (i = ack_info->n_ranges - 1; i >= 0; i--) {
+            range = &ack_info->ranges[i];
+            /* check if skipped packet numbers are acked */
+            if (xqc_max(range->low, pn_ctl->ctl_skipped_pn_low) <= xqc_min(range->high, pn_ctl->ctl_skipped_pn_high)) {
+                xqc_log(conn->log, XQC_LOG_ERROR, "|optimistic ack attack detected|ack_range:%ui-%ui|skipped_range:%ui-%ui|", range->low, range->high, pn_ctl->ctl_skipped_pn_low, pn_ctl->ctl_skipped_pn_high);
+                return -XQC_EPROTO;
+            }
+        }
+    }
+
+    return XQC_OK;
+}
+
+
 /**
  * OnAckReceived
  */
 int
 xqc_send_ctl_on_ack_received(xqc_send_ctl_t *send_ctl, xqc_pn_ctl_t *pn_ctl, xqc_send_queue_t *send_queue, xqc_ack_info_t *const ack_info, xqc_usec_t ack_recv_time, xqc_bool_t ack_on_same_path)
 {
-    /* ack info里包含的packet不一定会是send_ctl这条路径发出的 */
-    /* info里的largest ack 不一定是send_ctl这条路径的largest ack */
-
     xqc_connection_t *conn = send_ctl->ctl_conn;
 
     xqc_packet_out_t *packet_out;
@@ -803,6 +831,11 @@ xqc_send_ctl_on_ack_received(xqc_send_ctl_t *send_ctl, xqc_pn_ctl_t *pn_ctl, xqc
     unsigned char need_del_record = 0;
     int stream_frame_acked = 0;
 
+    if (xqc_send_ctl_detect_optimistic_ack_attack(send_ctl, pn_ctl, ack_info, ack_recv_time) < 0) {
+        XQC_CONN_ERR(conn, TRA_PROTOCOL_VIOLATION);
+        return -XQC_EPROTO;
+    }
+
     xqc_packet_number_t largest_acked_ack = xqc_ack_sent_record_on_ack(&pn_ctl->ack_sent_record[pns], ack_info);
     if (largest_acked_ack > pn_ctl->ctl_largest_acked_ack[pns]) {
         pn_ctl->ctl_largest_acked_ack[pns] = largest_acked_ack;
@@ -1864,3 +1897,37 @@ uint64_t
 xqc_send_ctl_get_pacing_rate(xqc_send_ctl_t *send_ctl) {
     return xqc_pacing_rate_calc(&send_ctl->ctl_pacing);
 }
+
+
+void 
+xqc_send_ctl_set_next_pn_for_packet(xqc_connection_t *conn, xqc_pn_ctl_t *pn_ctl,  
+    xqc_packet_out_t *packet_out, xqc_usec_t current_time)
+{
+    if (conn->conn_settings.disable_pn_skipping) {
+        packet_out->po_pkt.pkt_num = pn_ctl->ctl_packet_number[packet_out->po_pkt.pkt_pns]++;
+
+    } else {
+        packet_out->po_pkt.pkt_num = pn_ctl->ctl_packet_number[packet_out->po_pkt.pkt_pns];
+        if (packet_out->po_pkt.pkt_num > 0 
+            && packet_out->po_pkt.pkt_pns == XQC_PNS_APP_DATA) 
+        {
+            /* randomly skip some packet numbers to avoid optimistic ack attacks */
+            if (current_time > pn_ctl->ctl_next_skip_chance) {
+                /* we have 20% chance to skip some packet numbers */
+                /* we must skip some packet numbers if pn has been increased up to 2^10 since the last skip */
+                if (current_time % 10 < 2
+                    || packet_out->po_pkt.pkt_num > (pn_ctl->ctl_skipped_pn_high + 1024)) 
+                {
+                    /* the number of skipped packet numbers is also random */
+                    pn_ctl->ctl_skipped_pn_low = packet_out->po_pkt.pkt_num;
+                    pn_ctl->ctl_skipped_pn_high = pn_ctl->ctl_skipped_pn_low + current_time % 7;
+                    packet_out->po_pkt.pkt_num = pn_ctl->ctl_skipped_pn_high + 1;
+                    /* we should keep this range for 3xPTO to detect malicious ACKs */
+                    pn_ctl->ctl_next_skip_chance = current_time + 3 * xqc_conn_get_max_pto(conn);
+                    xqc_log(conn->log, XQC_LOG_DEBUG, "|optimistic ack detection|skipped_range:%ui-%ui|next_skip_chance:%ui|current_time:%ui|", pn_ctl->ctl_skipped_pn_low, pn_ctl->ctl_skipped_pn_high, pn_ctl->ctl_next_skip_chance, current_time);
+                }
+            }
+        }
+        pn_ctl->ctl_packet_number[packet_out->po_pkt.pkt_pns] = packet_out->po_pkt.pkt_num + 1;
+    }
+}

Diff for: src/transport/xqc_send_ctl.h

@@ -65,6 +65,13 @@ typedef struct xqc_pn_ctl_s {
     /* record ack sent */
     xqc_ack_sent_record_t       ack_sent_record[XQC_PNS_N];
 
+
+    /* fields are used for detecting optimistic ack attacks */
+    /* we skip pn in [ctl_skipped_pn_low, ctl_skipped_pn_high] */
+    xqc_packet_number_t         ctl_skipped_pn_low;
+    xqc_packet_number_t         ctl_skipped_pn_high;
+    xqc_usec_t                  ctl_next_skip_chance;
+
 } xqc_pn_ctl_t;
 
 typedef struct xqc_send_ctl_s {
@@ -273,4 +280,10 @@ xqc_packet_number_t xqc_send_ctl_get_pkt_num_gap(xqc_send_ctl_t *send_ctl, xqc_p
 uint64_t xqc_send_ctl_get_est_bw(xqc_send_ctl_t *send_ctl);
 uint64_t xqc_send_ctl_get_pacing_rate(xqc_send_ctl_t *send_ctl);
 
+xqc_int_t xqc_send_ctl_detect_optimistic_ack_attack(xqc_send_ctl_t *send_ctl, 
+    xqc_pn_ctl_t *pn_ctl, xqc_ack_info_t *const ack_info, xqc_usec_t ack_recv_time);
+
+void xqc_send_ctl_set_next_pn_for_packet(xqc_connection_t *conn, xqc_pn_ctl_t *pn_ctl,  
+    xqc_packet_out_t *packet_out, xqc_usec_t current_time);
+
 #endif /* _XQC_SEND_CTL_H_INCLUDED_ */