BUG/MINOR: quic: reject RETIRE_CONNECTION_ID for a CID not sent to the peer

A RETIRE_CONNECTION_ID frame for a sequence number below
<next_cid_seq_num> is accepted even if the corresponding
NEW_CONNECTION_ID frame has not been sent yet: the counter counts
allocated CIDs, not sent ones. A client can thus retire a CID it
cannot know about, for example the one allocated while processing a
previous RETIRE_CONNECTION_ID frame of the same packet. This is
forbidden by RFC 9000 19.16.

This adds <max_sent_cid_seq_num>, updated when a packet containing
NEW_CONNECTION_ID frame(s) is sent, and rejects such
RETIRE_CONNECTION_ID frames with a connection error of type
PROTOCOL_VIOLATION, as required by RFC 9000 19.16.

Must be backported as far as 2.8.

diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h
index ab28093ee938fa1e0cab631b09d306c728f4454c..81dd8db423cfc7853cd67121716f59b26a367797 100644 (file)
--- a/include/haproxy/quic_conn-t.h
+++ b/include/haproxy/quic_conn-t.h
@@ -335,6 +335,11 @@ struct quic_conn {
 #endif
 
        uint64_t next_cid_seq_num;
+       /* Highest CID sequence number sent to the peer. <next_cid_seq_num>
+        * counts allocated CIDs: a NEW_CONNECTION_ID frame may still be queued
+        * or in flight for a recently allocated one.
+        */
+       uint64_t max_sent_cid_seq_num;
        /* Initial hash computed from first ID (derived from ODCID).
         * it could be reused to derive extra CIDs from the same hash
         */

diff --git a/include/haproxy/quic_tx-t.h b/include/haproxy/quic_tx-t.h
index 606f652212b9b8744b6fbf45bef9a9e6b9e47c54..d6570ddb5912488a316e4bea80abb0b5a37e42aa 100644 (file)
--- a/include/haproxy/quic_tx-t.h
+++ b/include/haproxy/quic_tx-t.h
@@ -36,6 +36,8 @@ extern struct pool_head *pool_head_quic_cc_buf;
 #define QUIC_FL_TX_PACKET_COALESCED     (1UL << 4)
 /* Flag a sent packet as being probing with old data */
 #define QUIC_FL_TX_PACKET_PROBE_WITH_OLD_DATA (1UL << 5)
+/* Flag a sent packet as containing NEW_CONNECTION_ID frame(s). */
+#define QUIC_FL_TX_PACKET_NCIDS         (1UL << 6)
 
 /* Structure to store enough information about TX QUIC packets. */
 struct quic_tx_packet {

diff --git a/src/quic_conn.c b/src/quic_conn.c
index e992cf91cf4cb1d375f13822c57b56293968cbfe..6414af935d6b964bfdbfbde6c2efa8558c2c2663 100644 (file)
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -1198,6 +1198,7 @@ struct quic_conn *qc_new_conn(void *target,
        }
        *qc->cids = EB_ROOT;
        qc->next_cid_seq_num = 0;
+       qc->max_sent_cid_seq_num = 0;
        qc->scid = conn_id->cid;
 
        /* QUIC Server (or listener). */

diff --git a/src/quic_frame.c b/src/quic_frame.c
index 0233869bd69d5bd1a0bfe1de5ee6e6ad32c1a582..35c8c23eb532ea1088ea47b7f5b7427d15ba0b66 100644 (file)
--- a/src/quic_frame.c
+++ b/src/quic_frame.c
@@ -1123,7 +1123,7 @@ const struct quic_frame_builder quic_frame_builders[] = {
        [QUIC_FT_STREAM_DATA_BLOCKED]  = { .func = quic_build_stream_data_blocked_frame,  .flags = QUIC_FL_TX_PACKET_ACK_ELICITING, .mask = QUIC_FT_PKT_TYPE___01_BITMASK, },
        [QUIC_FT_STREAMS_BLOCKED_BIDI] = { .func = quic_build_streams_blocked_bidi_frame, .flags = QUIC_FL_TX_PACKET_ACK_ELICITING, .mask = QUIC_FT_PKT_TYPE___01_BITMASK, },
        [QUIC_FT_STREAMS_BLOCKED_UNI]  = { .func = quic_build_streams_blocked_uni_frame,  .flags = QUIC_FL_TX_PACKET_ACK_ELICITING, .mask = QUIC_FT_PKT_TYPE___01_BITMASK, },
-       [QUIC_FT_NEW_CONNECTION_ID]    = { .func = quic_build_new_connection_id_frame,    .flags = QUIC_FL_TX_PACKET_ACK_ELICITING, .mask = QUIC_FT_PKT_TYPE___01_BITMASK, },
+       [QUIC_FT_NEW_CONNECTION_ID]    = { .func = quic_build_new_connection_id_frame,    .flags = QUIC_FL_TX_PACKET_ACK_ELICITING | QUIC_FL_TX_PACKET_NCIDS, .mask = QUIC_FT_PKT_TYPE___01_BITMASK, },
        [QUIC_FT_RETIRE_CONNECTION_ID] = { .func = quic_build_retire_connection_id_frame, .flags = QUIC_FL_TX_PACKET_ACK_ELICITING, .mask = QUIC_FT_PKT_TYPE___01_BITMASK, },
        [QUIC_FT_PATH_CHALLENGE]       = { .func = quic_build_path_challenge_frame,       .flags = QUIC_FL_TX_PACKET_ACK_ELICITING, .mask = QUIC_FT_PKT_TYPE___01_BITMASK, },
        [QUIC_FT_PATH_RESPONSE]        = { .func = quic_build_path_response_frame,        .flags = QUIC_FL_TX_PACKET_ACK_ELICITING, .mask = QUIC_FT_PKT_TYPE___01_BITMASK, },

diff --git a/src/quic_rx.c b/src/quic_rx.c
index 74e3f79a58603528a1c82265b7114200d0deb431..a8a5e998e51b9f6aa68b7aa8b6b2c9bccef67162 100644 (file)
--- a/src/quic_rx.c
+++ b/src/quic_rx.c
@@ -802,6 +802,17 @@ static int qc_handle_retire_connection_id_frm(struct quic_conn *qc,
                goto protocol_violation;
        }
 
+       /* RFC 9000 19.16. RETIRE_CONNECTION_ID Frames:
+        * Receipt of a RETIRE_CONNECTION_ID frame containing a sequence number
+        * greater than any previously sent to the peer MUST be treated as a
+        * connection error of type PROTOCOL_VIOLATION.
+        */
+       /* Note that <next_cid_seq_num> counts CIDs at allocation time. */
+       if (rcid_frm->seq_num > qc->max_sent_cid_seq_num) {
+               TRACE_PROTO("CID seq. number not sent to peer", QUIC_EV_CONN_PSTRM, qc, frm);
+               goto protocol_violation;
+       }
+
        /* RFC 9000 19.16. RETIRE_CONNECTION_ID Frames:
         * The sequence number specified in a RETIRE_CONNECTION_ID frame MUST NOT refer to
         * the Destination Connection ID field of the packet in which the frame is contained.

diff --git a/src/quic_tx.c b/src/quic_tx.c
index aa62f84cfdfb43d7a7af5529f97d40fb10401246..dc7d3438efc2c1d6fe7e76fb9da93367d8668688 100644 (file)
--- a/src/quic_tx.c
+++ b/src/quic_tx.c
@@ -447,6 +447,19 @@ static int qc_send_ppkts(struct buffer *buf, struct quic_conn *qc)
 
                        pkt->time_sent_ns = time_sent_ns;
                        pkt->time_sent_ms = time_sent_ms;
+                       /* Record the NEW_CONNECTION_ID sequence numbers as sent */
+                       if (pkt->flags & QUIC_FL_TX_PACKET_NCIDS) {
+                               struct quic_frame *f;
+
+                               list_for_each_entry(f, &pkt->frms, list) {
+                                       if (f->type != QUIC_FT_NEW_CONNECTION_ID)
+                                               continue;
+
+                                       qc->max_sent_cid_seq_num =
+                                               QUIC_MAX(qc->max_sent_cid_seq_num,
+                                                        f->new_connection_id.seq_num);
+                               }
+                       }
                        if (pkt->flags & QUIC_FL_TX_PACKET_ACK_ELICITING) {
                                pkt->pktns->tx.time_of_last_eliciting = time_sent_ms;
                                qc->path->ifae_pkts++;