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 4aaaf9b3433b0a9052b80e3098b958ab13e72ce2..559111d4acabe80352a98c6e85f0085a7f3cc968 100644 (file)
--- 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 ca71e940e4f68a8ceff0313342f77ceccfe26769..01391a6ad34d245f1189ea3101a7aefe60d5819b 100644 (file)
--- 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 9881424b75c347f40976d68c3877b8259f905864..b4278efda50519e94d932a118a736f66926eaf44 100644 (file)
--- 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 af8285818a220eef591c43fe5109a24c6b00f450..936e73e348e31db47eef655ba7feac51e8702cdf 100644 (file)
--- 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 <qcc>. Set <bidi> 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;