BUG/MEDIUM: mux-quic: reject uni stream ID exceeding flow control
Emit STREAM_LIMIT_ERROR if a client tries to open an unidirectional
stream with an ID greater than the value specified by our flow-control
limit. The code is similar to the bidirectional stream opening.
MAX_STREAMS_UNI emission is not implement for the moment and is left as
a TODO. This should not be too urgent for the moment : in HTTP/3, a
client has only a limited use for unidirectional streams (H3 control
stream + 2 QPACK streams). This is covered by the value provided by
haproxy in transport parameters.
This patch has been tagged with BUG as it should have prevented last
crash reported on github issue #1808 when opening a new unidirectional
streams with an invalid ID. However, it is probably not the main cause
of the bug contrary to the patch
commit 11a6f4007b908b49ecd3abd5cd10fba177f07c11
BUG/MINOR: quic: Wrong status returned by qc_pkt_decrypt()
This must be backported up to 2.6.
diff --git a/include/haproxy/mux_quic-t.h b/include/haproxy/mux_quic-t.h
index c555ace..aa29c00 100644
--- a/include/haproxy/mux_quic-t.h
+++ b/include/haproxy/mux_quic-t.h
@@ -52,10 +52,13 @@
/* flow-control fields set by us enforced on our side. */
struct {
struct list frms; /* prepared frames related to flow-control */
+
uint64_t ms_bidi_init; /* max initial sub-ID of bidi stream allowed for the peer */
uint64_t ms_bidi; /* max sub-ID of bidi stream allowed for the peer */
uint64_t cl_bidi_r; /* total count of closed remote bidi stream since last MAX_STREAMS emission */
+ uint64_t ms_uni; /* max sub-ID of uni stream allowed for the peer */
+
uint64_t msd_bidi_l; /* initial max-stream-data on local streams */
uint64_t msd_bidi_r; /* initial max-stream-data on remote streams */
diff --git a/src/mux_quic.c b/src/mux_quic.c
index 57d7fdb..7b71ddb 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -582,7 +582,7 @@
{
struct qcs *qcs = NULL;
enum qcs_type type;
- uint64_t *largest;
+ uint64_t *largest, max_id;
TRACE_ENTER(QMUX_EV_QCS_NEW, qcc->conn);
@@ -597,20 +597,18 @@
type = conn_is_back(qcc->conn) ? QCS_SRV_UNI : QCS_CLT_UNI;
}
- /* TODO also checks max-streams for uni streams */
- if (quic_stream_is_bidi(id)) {
- if (id >= qcc->lfctl.ms_bidi * 4) {
- /* RFC 9000 4.6. Controlling Concurrency
- *
- * An endpoint that receives a frame with a
- * stream ID exceeding the limit it has sent
- * MUST treat this as a connection error of
- * type STREAM_LIMIT_ERROR
- */
- TRACE_ERROR("flow control error", QMUX_EV_QCS_NEW|QMUX_EV_PROTO_ERR, qcc->conn);
- qcc_emit_cc(qcc, QC_ERR_STREAM_LIMIT_ERROR);
- goto err;
- }
+ /* RFC 9000 4.6. Controlling Concurrency
+ *
+ * An endpoint that receives a frame with a stream ID exceeding the
+ * limit it has sent MUST treat this as a connection error of type
+ * STREAM_LIMIT_ERROR
+ */
+ max_id = quic_stream_is_bidi(id) ? qcc->lfctl.ms_bidi * 4 :
+ qcc->lfctl.ms_uni * 4;
+ if (id >= max_id) {
+ TRACE_ERROR("flow control error", QMUX_EV_QCS_NEW|QMUX_EV_PROTO_ERR, qcc->conn);
+ qcc_emit_cc(qcc, QC_ERR_STREAM_LIMIT_ERROR);
+ goto err;
}
/* Only stream ID not already opened can be used. */
@@ -1204,7 +1202,12 @@
}
}
else {
- /* TODO */
+ /* TODO in HTTP/3 unidirectional streams cannot be closed or a
+ * H3_CLOSED_CRITICAL_STREAM will be triggered before
+ * entering here. If a new application protocol is supported it
+ * might be necessary to implement MAX_STREAMS_UNI emission.
+ */
+ ABORT_NOW();
}
TRACE_LEAVE(QMUX_EV_QCS_END, qcc->conn);
@@ -1980,6 +1983,7 @@
LIST_INIT(&qcc->lfctl.frms);
qcc->lfctl.ms_bidi = qcc->lfctl.ms_bidi_init = lparams->initial_max_streams_bidi;
+ qcc->lfctl.ms_uni = lparams->initial_max_streams_uni;
qcc->lfctl.msd_bidi_l = lparams->initial_max_stream_data_bidi_local;
qcc->lfctl.msd_bidi_r = lparams->initial_max_stream_data_bidi_remote;
qcc->lfctl.cl_bidi_r = 0;