From: Amaury Denoyelle Date: Wed, 18 Jun 2025 13:12:31 +0000 (+0200) Subject: BUG/MINOR: mux-quic/h3: properly handle too low peer fctl initial stream X-Git-Tag: v3.3-dev2~35 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=805a070ab920d14b22a6b7beac3b0648e684b2d2;p=thirdparty%2Fhaproxy.git BUG/MINOR: mux-quic/h3: properly handle too low peer fctl initial stream Previously, no check on peer flow-control was implemented prior to open a local QUIC stream. This was a small problem for frontend implementation, as in this case haproxy as a server never opens bidirectional streams. On frontend, the only stream opened by haproxy in this case is for HTTP/3 control unidirectional data. If the peer uses an initial value for max uni streams set to 0, it would violate its flow control, and the peer will probably close the connection. Note however that RFC 9114 mandates that each peer defines minimal initial value so that at least the control stream can be created. This commit improves the situation of too low initial max uni streams value. Now, on HTTP/3 layer initialization, haproxy preemptively checks flow control limit on streams via a new function qcc_fctl_avail_streams(). If credit is already expired due to a too small initial value, haproxy preemptively closes the connection using H3_ERR_GENERAL_PROTOCOL_ERROR. This behavior is better as haproxy is now the initiator of the connection closure. This should be backported up to 2.8. --- diff --git a/include/haproxy/mux_quic-t.h b/include/haproxy/mux_quic-t.h index 4aaaf9b34..559111d4a 100644 --- a/include/haproxy/mux_quic-t.h +++ b/include/haproxy/mux_quic-t.h @@ -66,6 +66,8 @@ struct qcc { /* flow-control fields set by the peer which we must respect. */ struct { + uint64_t ms_uni; /* max sub-ID of uni stream allowed by the peer */ + uint64_t md; /* connection flow control limit updated on MAX_DATA frames reception */ uint64_t msd_bidi_l; /* initial max-stream-data from peer on local bidi streams */ uint64_t msd_bidi_r; /* initial max-stream-data from peer on remote bidi streams */ diff --git a/include/haproxy/mux_quic.h b/include/haproxy/mux_quic.h index ca71e940e..01391a6ad 100644 --- a/include/haproxy/mux_quic.h +++ b/include/haproxy/mux_quic.h @@ -19,6 +19,7 @@ void qcc_set_error(struct qcc *qcc, int err, int app); int _qcc_report_glitch(struct qcc *qcc, int inc); +int qcc_fctl_avail_streams(const struct qcc *qcc, int bidi); struct qcs *qcc_init_stream_local(struct qcc *qcc, int bidi); void qcs_send_metadata(struct qcs *qcs); int qcs_attach_sc(struct qcs *qcs, struct buffer *buf, char fin); diff --git a/src/h3.c b/src/h3.c index 9881424b7..b4278efda 100644 --- a/src/h3.c +++ b/src/h3.c @@ -3030,6 +3030,21 @@ static int h3_finalize(void *ctx) TRACE_ENTER(H3_EV_H3C_NEW, qcc->conn); if (!qcs) { + /* RFC 9114 6.2. Unidirectional Streams + * + * Each endpoint needs to create at least one unidirectional stream for + * the HTTP control stream. QPACK requires two additional unidirectional + * streams, and other extensions might require further streams. + * Therefore, the transport parameters sent by both clients and servers + * MUST allow the peer to create at least three unidirectional streams. + */ + if (qcc_fctl_avail_streams(qcc, 0) < 3) { + TRACE_ERROR("peer flow-control limit does not allow control stream creation", H3_EV_H3C_NEW, qcc->conn); + qcc_set_error(qcc, H3_ERR_GENERAL_PROTOCOL_ERROR, 1); + qcc_report_glitch(qcc, 1); + goto err; + } + qcs = qcc_init_stream_local(qcc, 0); if (!qcs) { /* Error must be set by qcc_init_stream_local(). */ diff --git a/src/mux_quic.c b/src/mux_quic.c index af8285818..936e73e34 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -785,6 +785,20 @@ int _qcc_report_glitch(struct qcc *qcc, int inc) return 0; } +/* Returns the number of streams which can still be opened until flow-control limit. */ +int qcc_fctl_avail_streams(const struct qcc *qcc, int bidi) +{ + if (bidi) { + /* TODO */ + return 0; + } + else { + const uint64_t next = qcc->next_uni_l / 4; + BUG_ON(qcc->rfctl.ms_uni < next); + return qcc->rfctl.ms_uni - next; + } +} + /* Open a locally initiated stream for the connection . Set for a * bidirectional stream, else an unidirectional stream is opened. The next * available ID on the connection will be used according to the stream type. @@ -804,14 +818,11 @@ struct qcs *qcc_init_stream_local(struct qcc *qcc, int bidi) type = conn_is_back(qcc->conn) ? QCS_CLT_BIDI : QCS_SRV_BIDI; } else { + BUG_ON(qcc->rfctl.ms_uni * 4 < qcc->next_uni_l); next = &qcc->next_uni_l; type = conn_is_back(qcc->conn) ? QCS_CLT_UNI : QCS_SRV_UNI; } - /* TODO ensure that we won't overflow remote peer flow control limit on - * streams. Else, we should emit a STREAMS_BLOCKED frame. - */ - qcs = qcs_new(qcc, *next, type); if (!qcs) { qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR, 0); @@ -820,6 +831,7 @@ struct qcs *qcc_init_stream_local(struct qcc *qcc, int bidi) } TRACE_PROTO("opening local stream", QMUX_EV_QCS_NEW, qcc->conn, qcs); + /* TODO emit STREAMS_BLOCKED if cannot create future streams. */ *next += 4; TRACE_LEAVE(QMUX_EV_QCS_NEW, qcc->conn, qcs); @@ -3439,6 +3451,7 @@ static int qmux_init(struct connection *conn, struct proxy *prx, rparams = &conn->handle.qc->tx.params; qfctl_init(&qcc->tx.fc, rparams->initial_max_data); + qcc->rfctl.ms_uni = rparams->initial_max_streams_uni; qcc->rfctl.msd_bidi_l = rparams->initial_max_stream_data_bidi_local; qcc->rfctl.msd_bidi_r = rparams->initial_max_stream_data_bidi_remote; qcc->rfctl.msd_uni_l = rparams->initial_max_stream_data_uni;