BUG/MINOR: quic: reject invalid stream ID after MUX closure

QUIC transport layer may remain for some time after MUX closure, until
all sent frames have been acknowledged. In the meantime, if the client
opens a new stream, an application layer callback is used to reject it.
With HTTP/3, this produces a STOP_SENDING + RESET_STREAM.

Previously, there was no limit on the queued frames in this case. This
could cause unnecessary memory consumption until the connection is
finally closed.

Address this issue by ensuring the client won't try to open new streams
with invalid ID. First, if the ID corresponds to a server initiated
stream, the connection is closed with error STREAM_STATE.

Another check is performed to ensure that the stream ID is not beyond
the limit advertised last by MUX layer. This requires two new fields for
uni and bidi types in quic_conn, updated during qcc_release() similarly
to the already existing max fields. In case of a violation, the
connection is immediately closed with transport error STREAM_LIMIT.

This must be backported up to 2.6 after a period of observation.

diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h
index 81dd8db423cfc7853cd67121716f59b26a367797..9f7d955dfdb81f030acd881e20e07964a37ecac0 100644 (file)
--- a/include/haproxy/quic_conn-t.h
+++ b/include/haproxy/quic_conn-t.h
@@ -391,6 +391,9 @@ struct quic_conn {
                /* first unhandled streams ID, set by MUX after release */
                uint64_t stream_max_uni;
                uint64_t stream_max_bidi;
+               /* first stream IDs, which cannot be opened by the peer due to flow-control, set by MUX after release */
+               uint64_t stream_lim_uni;
+               uint64_t stream_lim_bidi;
 
                /* Number of received packets which failed authentication. */
                uint64_t auth_fails;

diff --git a/src/mux_quic.c b/src/mux_quic.c
index 484c6af7804905ac7476d656035654816240bcc5..a4e7f6a5193f8bd0e79d409ab51f750ce5f2c768 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -3792,6 +3792,8 @@ static void qcc_release(struct qcc *qcc)
                if (!conn_is_back(conn)) {
                        qc->rx.stream_max_uni = qcc->largest_uni_r;
                        qc->rx.stream_max_bidi = qcc->largest_bidi_r;
+                       qc->rx.stream_lim_uni = qcc->lfctl.ms_uni * 4;
+                       qc->rx.stream_lim_bidi = qcc->lfctl.ms_bidi * 4;
                }
        }
 

diff --git a/src/quic_conn.c b/src/quic_conn.c
index 6414af935d6b964bfdbfbde6c2efa8558c2c2663..12d6060637d0fcffb11dbe69ec2379a4a1724c78 100644 (file)
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -1308,6 +1308,7 @@ struct quic_conn *qc_new_conn(void *target,
        memset(&qc->rx.params, 0, sizeof(qc->rx.params));
        qc->rx.buf = b_make(qc->rx.buf.area, QUIC_CONN_RX_BUFSZ, 0, 0);
        qc->rx.stream_max_uni = qc->rx.stream_max_bidi = 0;
+       qc->rx.stream_lim_uni = qc->rx.stream_lim_bidi = UINT64_MAX;
        qc->rx.auth_fails = 0;
 
        qc->max_udp_payload = l ?

diff --git a/src/quic_rx.c b/src/quic_rx.c
index a8a5e998e51b9f6aa68b7aa8b6b2c9bccef67162..cd8244dd06dae27ec0783514020a1e1867cca32b 100644 (file)
--- a/src/quic_rx.c
+++ b/src/quic_rx.c
@@ -1029,7 +1029,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                {
                        struct qf_stream *strm_frm = &frm->stream;
                        const char fin = frm->type & QUIC_STREAM_FRAME_TYPE_FIN_BIT;
-                       uint64_t max;
+                       uint64_t max, lim;
 
                        /* The upper layer may not be allocated. */
                        if (!qc_is_conn_ready(qc)) {
@@ -1042,12 +1042,30 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                                        break;
                                }
 
-                               max = quic_stream_is_uni(strm_frm->id) ?
-                                 qc->rx.stream_max_uni : qc->rx.stream_max_bidi;
+                               if (quic_stream_is_uni(strm_frm->id)) {
+                                       lim = qc->rx.stream_lim_uni;
+                                       max = qc->rx.stream_max_uni;
+                               }
+                               else {
+                                       lim = qc->rx.stream_lim_bidi;
+                                       max = qc->rx.stream_max_bidi;
+                               }
+
                                if (strm_frm->id < max) {
                                        TRACE_DATA("Already closed stream", QUIC_EV_CONN_PRSHPKT, qc);
                                }
                                else {
+                                       if (strm_frm->id & QCS_ID_SRV_INTIATOR_BIT) {
+                                               TRACE_ERROR("Unknown local stream ID after MUX closure", QUIC_EV_CONN_PRSHPKT, qc);
+                                               quic_set_connection_close(qc, quic_err_transport(QC_ERR_STREAM_STATE_ERROR));
+                                               goto err;
+                                       }
+                                       else if (strm_frm->id >= lim) {
+                                               TRACE_ERROR("Stream ID limit violation after MUX closure", QUIC_EV_CONN_PRSHPKT, qc);
+                                               quic_set_connection_close(qc, quic_err_transport(QC_ERR_STREAM_LIMIT_ERROR));
+                                               goto err;
+                                       }
+
                                        TRACE_DEVEL("No mux for new stream", QUIC_EV_CONN_PRSHPKT, qc);
                                        if (qc->strm_reject) {
                                                if (!qc->strm_reject(&qc->ael->pktns->tx.frms, strm_frm->id)) {