[!] fix handle Retire Prior To error when NEW_CONNECTION_ID arrives and retire all the previous CIDs (#427)

Kulsk · web-flow · commit 07b18386a7d7 · 2024-05-30T19:09:53.000+08:00
* [!] fix interop multiplexing testcase error when hq stream send content and fin separately.

* [!] fix handle Retire Prior To error when NEW_CONNECTION_ID arrives and retire all the previous CIDs

* [!] fix anti-amplification interop testcase error

* [!] fix annotation error

* [!] fix sending pkt with retired cid error after receiving NCI with Retire Prior To
diff --git a/demo/demo_server.c b/demo/demo_server.c
@@ -1506,6 +1506,7 @@ xqc_demo_svr_init_conn_settings(xqc_demo_svr_args_t *args)
         .is_interop_mode = args->quic_cfg.is_interop_mode,
         .max_pkt_out_size = args->quic_cfg.max_pkt_sz,
         .adaptive_ack_frequency = 1,
+        .anti_amplification_limit = 3,
     };
 
     xqc_server_set_conn_settings(&conn_settings);
diff --git a/include/xquic/xquic.h b/include/xquic/xquic.h
@@ -1145,7 +1145,7 @@ typedef struct xqc_conn_settings_s {
     xqc_msec_t                  init_idle_time_out; /* initial idle timeout interval, effective before handshake completion */
     xqc_msec_t                  idle_time_out;      /* idle timeout interval, effective after handshake completion */
     int32_t                     spurious_loss_detect_on;
-    uint32_t                    anti_amplification_limit;   /* limit of anti-amplification, default 3 */
+    uint32_t                    anti_amplification_limit;   /* limit of anti-amplification, default 5 */
     uint64_t                    keyupdate_pkt_threshold;    /* packet limit of a single 1-rtt key, 0 for unlimited */
     size_t                      max_pkt_out_size;
 
diff --git a/src/transport/xqc_cid.c b/src/transport/xqc_cid.c
@@ -167,6 +167,49 @@ xqc_destroy_cid_set(xqc_cid_set_t *cid_set)
     xqc_init_cid_set(cid_set);
 }
 
+uint64_t
+xqc_cid_set_cnt(xqc_cid_set_t *cid_set)
+{
+    return cid_set->unused_cnt + cid_set->used_cnt;
+}
+
+xqc_bool_t
+xqc_cid_set_validate_new_cid_limit(xqc_cid_set_t *cid_set,
+    uint64_t max_retire_prior_to, uint64_t *active_cid_limit)
+{
+    xqc_list_head_t *pos, *next;
+    xqc_cid_inner_t *cid = NULL;
+    uint64_t         retire_cnt = 0;
+
+    if (xqc_cid_set_cnt(cid_set) + 1 <= *active_cid_limit) {
+        return XQC_TRUE;
+    }
+
+    /* the new cid might exceed the active_cid_limit, but will not retire any
+       cid, shall not be inserted */
+    if (max_retire_prior_to == 0) {
+        return XQC_FALSE;
+    }
+
+    /* validate if cid cnt is legal after to be retired */
+    xqc_list_for_each_safe(pos, next, &cid_set->list_head) {
+        cid = xqc_list_entry(pos, xqc_cid_inner_t, list);
+        if (cid->cid.cid_seq_num < max_retire_prior_to) {
+            retire_cnt++;
+        }
+    }
+
+    if (xqc_cid_set_cnt(cid_set) + 1 - retire_cnt > *active_cid_limit) {
+        return XQC_FALSE;
+    }
+
+    /* allow the new connection id */
+    *active_cid_limit = xqc_cid_set_cnt(cid_set) + 1;
+
+    return XQC_TRUE;
+}
+
+
 xqc_int_t
 xqc_cid_set_insert_cid(xqc_cid_set_t *cid_set, xqc_cid_t *cid, xqc_cid_state_t state, uint64_t limit)
 {
diff --git a/src/transport/xqc_cid.h b/src/transport/xqc_cid.h
@@ -61,6 +61,10 @@ void xqc_init_scid_set(xqc_scid_set_t *scid_set);
 void xqc_init_dcid_set(xqc_dcid_set_t *dcid_set);
 void xqc_destroy_cid_set(xqc_cid_set_t *cid_set);
 
+uint64_t xqc_cid_set_cnt(xqc_cid_set_t *cid_set);
+xqc_bool_t xqc_cid_set_validate_new_cid_limit(xqc_cid_set_t *cid_set,
+    uint64_t max_retire_prior_to, uint64_t *active_cid_limit);
+
 xqc_int_t xqc_cid_set_insert_cid(xqc_cid_set_t *cid_set, xqc_cid_t *cid, xqc_cid_state_t state, uint64_t limit);
 xqc_int_t xqc_cid_set_delete_cid(xqc_cid_set_t *cid_set, xqc_cid_t *cid);
 
diff --git a/src/transport/xqc_conn.c b/src/transport/xqc_conn.c
@@ -3944,6 +3944,13 @@ xqc_conn_confirm_cid(xqc_connection_t *c, xqc_packet_t *pkt)
     if (!(c->conn_flag & XQC_CONN_FLAG_DCID_OK)) {
 
         if (xqc_cid_in_cid_set(&c->dcid_set.cid_set, &pkt->pkt_scid) == NULL) {
+            /* delete original cid which is not chosen by peer */
+            ret = xqc_cid_set_delete_cid(&c->dcid_set.cid_set, &c->original_dcid);
+            if (ret != XQC_OK) {
+                xqc_log(c->log, XQC_LOG_WARN, "|delete original dcid error");
+            }
+
+            /* insert peer's first dcid */
             ret = xqc_cid_set_insert_cid(&c->dcid_set.cid_set, &pkt->pkt_scid, XQC_CID_USED,
                                          c->local_settings.active_connection_id_limit);
             if (ret != XQC_OK) {
@@ -5943,3 +5950,68 @@ xqc_conn_handle_deprecated_stateless_reset(xqc_connection_t *conn,
 }
 
 #endif
+
+
+/* Retire DCID on initial path. this is called when NEW_CONNECTION_ID frame with
+   Retire Prior To field is received. */
+xqc_int_t
+xqc_conn_retire_dcid_prior_to(xqc_connection_t *conn, uint64_t retire_prior_to)
+{
+    xqc_int_t           ret;
+    uint64_t            seq_num;
+    xqc_cid_inner_t    *inner_cid;
+    xqc_list_head_t    *pos, *next;
+
+    xqc_log(conn->log, XQC_LOG_DEBUG, "|retire cid prior to:%ui|current "
+            "largest_retire_prior_to:%ui|", retire_prior_to,
+            conn->dcid_set.largest_retire_prior_to);
+
+    xqc_list_for_each_safe(pos, next, &conn->dcid_set.cid_set.list_head) {
+
+        inner_cid = xqc_list_entry(pos, xqc_cid_inner_t, list);
+        seq_num = inner_cid->cid.cid_seq_num;
+
+        if ((inner_cid->state == XQC_CID_UNUSED
+                || inner_cid->state == XQC_CID_USED)
+            && (seq_num >= conn->dcid_set.largest_retire_prior_to
+                && seq_num < retire_prior_to))
+        {
+            ret = xqc_write_retire_conn_id_frame_to_packet(conn, seq_num);
+            if (ret != XQC_OK) {
+                xqc_log(conn->log, XQC_LOG_ERROR,
+                        "|xqc_write_retire_conn_id_frame_to_packet error|");
+                return ret;
+            }
+
+            /* change state */
+            ret = xqc_cid_switch_to_next_state(&conn->dcid_set.cid_set,
+                                               inner_cid, XQC_CID_RETIRED);
+            if (ret != XQC_OK) {
+                xqc_log(conn->log, XQC_LOG_ERROR, "|switch cid state to "
+                        "RETIRED error|seq_num:%ui|cur_state:%d|", seq_num,
+                        inner_cid->state);
+            }
+
+            /* immediately delete cid */
+            xqc_list_del(pos);
+            xqc_free(inner_cid);
+
+            xqc_log(conn->log, XQC_LOG_INFO, "|cid[%ui] retired", seq_num);
+        }
+    }
+
+    conn->dcid_set.largest_retire_prior_to = retire_prior_to;
+
+    /* path dcid retired */
+    if (conn->conn_initial_path->path_dcid.cid_seq_num < retire_prior_to) {
+        xqc_log(conn->log, XQC_LOG_DEBUG, "");
+        xqc_cid_copy(&conn->conn_initial_path->path_dcid,
+                     &conn->dcid_set.current_dcid);
+    }
+
+    xqc_log(conn->log, XQC_LOG_DEBUG, "|retire_prior_to|%ui|increase to %ui"
+            "|cnt:%ui", conn->dcid_set.largest_retire_prior_to,
+            retire_prior_to, xqc_cid_set_cnt(&conn->dcid_set.cid_set));
+
+    return XQC_OK;
+}
diff --git a/src/transport/xqc_conn.h b/src/transport/xqc_conn.h
@@ -658,4 +658,6 @@ xqc_int_t xqc_conn_send_ping_internal(xqc_connection_t *conn, void *ping_user_da
 
 void xqc_conn_encode_mp_settings(xqc_connection_t *conn, char *buf, size_t buf_sz);
 
+xqc_int_t xqc_conn_retire_dcid_prior_to(xqc_connection_t *conn, uint64_t retire_prior_to);
+
 #endif /* _XQC_CONN_H_INCLUDED_ */
diff --git a/src/transport/xqc_frame.c b/src/transport/xqc_frame.c
@@ -738,12 +738,15 @@ xqc_process_ping_frame(xqc_connection_t *conn, xqc_packet_in_t *packet_in)
 xqc_int_t
 xqc_process_new_conn_id_frame(xqc_connection_t *conn, xqc_packet_in_t *packet_in)
 {
-    xqc_int_t ret = XQC_ERROR;
-    xqc_cid_t new_conn_cid;
-    uint64_t retire_prior_to;
+    xqc_int_t        ret = XQC_ERROR;
+    xqc_cid_t        new_conn_cid;
+    uint64_t         retire_prior_to;
+    uint64_t         largest_retire_prior_to;
+    uint64_t         cid_limit;
+
 
-    xqc_cid_inner_t *inner_cid;
-    xqc_list_head_t *pos, *next;
+    /* the initial cid limit */
+    cid_limit = conn->local_settings.active_connection_id_limit;
 
     ret = xqc_parse_new_conn_id_frame(packet_in, &new_conn_cid, &retire_prior_to, conn);
     if (ret != XQC_OK) {
@@ -769,6 +772,7 @@ xqc_process_new_conn_id_frame(xqc_connection_t *conn, xqc_packet_in_t *packet_in
 
     /* TODO: write_retire_conn_id_frame 可能涉及到 替换 path.dcid (当前无 retire_prior_to 因此不涉及) */
 
+    /* the current cid was retired and is needed no more */
     if (new_conn_cid.cid_seq_num < conn->dcid_set.largest_retire_prior_to) {
         /*
          * An endpoint that receives a NEW_CONNECTION_ID frame with a sequence number smaller
@@ -788,37 +792,26 @@ xqc_process_new_conn_id_frame(xqc_connection_t *conn, xqc_packet_in_t *packet_in
         return XQC_OK;
     }
 
-    if (retire_prior_to > conn->dcid_set.largest_retire_prior_to) {
-        /*
-         * Upon receipt of an increased Retire Prior To field, the peer MUST stop using the
-         * corresponding connection IDs and retire them with RETIRE_CONNECTION_ID frames before
-         * adding the newly provided connection ID to the set of active connection IDs.
-         */
-
-        xqc_list_for_each_safe(pos, next, &conn->dcid_set.cid_set.list_head) {
-            inner_cid = xqc_list_entry(pos, xqc_cid_inner_t, list);
-            uint64_t seq_num = inner_cid->cid.cid_seq_num;
-            if ((inner_cid->state == XQC_CID_UNUSED || inner_cid->state == XQC_CID_USED)
-                 && (seq_num >= conn->dcid_set.largest_retire_prior_to && seq_num < retire_prior_to))
-            {
-                ret = xqc_write_retire_conn_id_frame_to_packet(conn, seq_num);
-                if (ret != XQC_OK) {
-                    xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_write_retire_conn_id_frame_to_packet error|");
-                    return ret;
-                }
-            }
-        }
-
-        conn->dcid_set.largest_retire_prior_to = retire_prior_to;
-        xqc_log(conn->log, XQC_LOG_DEBUG, "|retire_prior_to|%ui|increase to|%ui|",
-                conn->dcid_set.largest_retire_prior_to, retire_prior_to);
-    }
-
     /* store dcid & add unused_dcid_count */
     if (xqc_cid_in_cid_set(&conn->dcid_set.cid_set, &new_conn_cid) != NULL) {
         return XQC_OK;
     }
 
+    /* check if insertion of the new conneciton id will violate the cid limit */
+    largest_retire_prior_to = xqc_max(retire_prior_to,
+                                      conn->dcid_set.largest_retire_prior_to);
+    if (!xqc_cid_set_validate_new_cid_limit(&conn->dcid_set.cid_set,
+            largest_retire_prior_to, &cid_limit))
+    {
+        xqc_log(conn->log, XQC_LOG_ERROR, "|retire_prior_to:%ui greater than seq_num:%ui|",
+                retire_prior_to, new_conn_cid.cid_seq_num);
+        XQC_CONN_ERR(conn, TRA_PROTOCOL_VIOLATION);
+        return -XQC_EPROTO;
+    }
+
+    xqc_log(conn->log, XQC_LOG_DEBUG, "|nci allow to be inserted|seq:%ui|dcid_cnt:%ui|cid_limit:%ui",
+            new_conn_cid.cid_seq_num, xqc_cid_set_cnt(&conn->dcid_set.cid_set), cid_limit);
+
     /* insert into dcid-connection hash, for processing the deprecated stateless
        reset packet */
     ret = xqc_insert_conns_hash(conn->engine->conns_hash_dcid, conn, 
@@ -840,13 +833,31 @@ xqc_process_new_conn_id_frame(xqc_connection_t *conn, xqc_packet_in_t *packet_in
         return ret;
     }
 
-    ret = xqc_cid_set_insert_cid(&conn->dcid_set.cid_set, &new_conn_cid, XQC_CID_UNUSED, conn->local_settings.active_connection_id_limit);
+    /* An endpoint MAY send connection IDs that temporarily exceed a peer's 
+     * limit if the NEW_CONNECTION_ID frame also requires the retirement of any
+     * excess, by including a sufficiently large value in the Retire Prior To
+     * field. Hence it is not reasonable to consider it as an error */
+    ret = xqc_cid_set_insert_cid(&conn->dcid_set.cid_set, &new_conn_cid,
+                                 XQC_CID_UNUSED, cid_limit);
     if (ret != XQC_OK) {
-        xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_cid_set_insert_cid error|limit:%ui|unused:%ui|used:%ui|",
-                conn->local_settings.active_connection_id_limit, conn->dcid_set.cid_set.unused_cnt, conn->dcid_set.cid_set.used_cnt);
+        xqc_log(conn->log, XQC_LOG_ERROR, "|xqc_cid_set_insert_cid error"
+                "|local_limit:%ui|largest_limit:%ui|retire_prior_to:%ui|"
+                "unused:%ui|used:%ui|",
+                conn->local_settings.active_connection_id_limit,
+                cid_limit, retire_prior_to, conn->dcid_set.cid_set.unused_cnt,
+                conn->dcid_set.cid_set.used_cnt);
         return ret;
     }
 
+    /*
+     * Upon receipt of an increased Retire Prior To field, the peer MUST stop
+     * using the corresponding connection IDs and retire them with
+     * RETIRE_CONNECTION_ID frames before adding the newly provided connection
+     * ID to the set of active connection IDs.
+     */
+    if (retire_prior_to > conn->dcid_set.largest_retire_prior_to) {
+        xqc_conn_retire_dcid_prior_to(conn, retire_prior_to);
+    }
 
     return XQC_OK;
 }
diff --git a/src/transport/xqc_packet_out.c b/src/transport/xqc_packet_out.c
@@ -1270,18 +1270,21 @@ xqc_write_retire_conn_id_frame_to_packet(xqc_connection_t *conn, uint64_t seq_nu
 {
     xqc_int_t ret = XQC_ERROR;
 
-	/* select new current_dcid to replace the cid to be retired */		
-    if (seq_num == conn->dcid_set.current_dcid.cid_seq_num) {	
-        // TODO: DCID changes	
-        ret = xqc_get_unused_cid(&conn->dcid_set.cid_set, &conn->dcid_set.current_dcid);		
-        if (ret != XQC_OK) {		
-            xqc_log(conn->log, XQC_LOG_ERROR, "|conn don't have available dcid|");		
-            return ret;		
+    /* select new current_dcid to replace the cid to be retired */
+    if (seq_num == conn->dcid_set.current_dcid.cid_seq_num) {
+        // TODO: DCID changes
+        ret = xqc_get_unused_cid(&conn->dcid_set.cid_set, &conn->dcid_set.current_dcid);
+        if (ret != XQC_OK) {
+            xqc_log(conn->log, XQC_LOG_ERROR, "|conn don't have available dcid"
+                    "|seq_num:%ui|cur_cid_seq_num:%ui", seq_num,
+                    conn->dcid_set.current_dcid.cid_seq_num);
+            return ret;
         }
-        xqc_datagram_record_mss(conn);		
-    }		
-    xqc_log(conn->log, XQC_LOG_DEBUG, "|get_new_dcid:%s|seq_num:%ui|",		
-            xqc_dcid_str(&conn->dcid_set.current_dcid), conn->dcid_set.current_dcid.cid_seq_num);
+        xqc_datagram_record_mss(conn);
+    }
+    xqc_log(conn->log, XQC_LOG_DEBUG, "|get_new_dcid:%s|seq_num:%ui|",
+            xqc_dcid_str(&conn->dcid_set.current_dcid),
+            conn->dcid_set.current_dcid.cid_seq_num);
 
     xqc_packet_out_t *packet_out = xqc_write_new_packet(conn, XQC_PTYPE_SHORT_HEADER);
     if (packet_out == NULL) {
