From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Tue, 7 Feb 2023 13:24:54 +0000 (+0100)
Subject: MINOR: quic: adjust request reject when MUX is already freed
X-Git-Tag: v2.8-dev5~169
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=38836b6b3da227ee9be2f32632215578bcd61b55;p=thirdparty%2Fhaproxy.git

MINOR: quic: adjust request reject when MUX is already freed

When the MUX is freed, the quic-conn layer may stay active until all
streams acknowledgment are processed. In this interval, if a new stream
is opened by the client, the quic-conn is thus now responsible to handle
it. This is done by the emission of a STOP_SENDING.

This process is closely related to HTTP/3 protocol despite being handled
by the quic-conn layer. This highlights a flaw in our QUIC architecture
which should be adjusted. To reflect this situation, the function
qc_stop_sending_frm_enqueue() is renamed qc_h3_request_reject(). Also,
internal H3 treatment such as uni-directional bypass has been moved
inside the function.

This commit is only a refactor. However, bug fix on next patches will
rely on it so it should be backported up to 2.6.
---

diff --git a/src/quic_conn.c b/src/quic_conn.c
index 4d22a49b41..4b1d56ccec 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -2697,24 +2697,33 @@ static void qc_cc_err_count_inc(struct quic_conn *qc, struct quic_frame *frm)
 	TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
 }
 
-/* Enqueue a STOP_SENDING frame to send into 1RTT packet number space
- * frame list to send.
- * Return 1 if succeeded, 0 if not.
+/* Cancel a request on connection <qc> for stream id <id>. This is useful when
+ * the client opens a new stream but the MUX has already been released. A
+ * STOP_SENDING frame is prepared for emission.
+ *
+ * TODO this function is closely related to H3. Its place should be in H3 layer
+ * instead of quic-conn but this requires an architecture adjustment.
+ *
+ * Returns 1 on sucess else 0.
  */
-static int qc_stop_sending_frm_enqueue(struct quic_conn *qc, uint64_t id)
+static int qc_h3_request_reject(struct quic_conn *qc, uint64_t id)
 {
 	int ret = 0;
 	struct quic_frame *frm;
 	struct quic_enc_level *qel = &qc->els[QUIC_TLS_ENC_LEVEL_APP];
-	uint64_t app_error_code;
+	const uint64_t app_error_code = H3_REQUEST_REJECTED;
 
 	TRACE_ENTER(QUIC_EV_CONN_PRSHPKT, qc);
 
-	/* TODO: the mux may be released, we cannot have more
-	 * information about the application error code to send
-	 * at this time.
+	/* Do not emit rejection for unknown unidirectional stream as it is
+	 * forbidden to close some of them (H3 control stream and QPACK
+	 * encoder/decoder streams).
 	 */
-	app_error_code = H3_REQUEST_REJECTED;
+	if (quic_stream_is_uni(id)) {
+		ret = 1;
+		goto out;
+	}
+
 	frm = qc_frm_alloc(QUIC_FT_STOP_SENDING);
 	if (!frm) {
 		TRACE_ERROR("failed to allocate quic_frame", QUIC_EV_CONN_PRSHPKT, qc);
@@ -2935,19 +2944,10 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
 				}
 				else {
 					TRACE_DEVEL("No mux for new stream", QUIC_EV_CONN_PRSHPKT, qc);
-					if (qc->app_ops == &h3_ops && quic_stream_is_uni(stream->id)) {
-						/* Do not send STOP_SENDING frames for h3 unidirectional streams.
-						 * TODO: this test should be removed when the connection closure
-						 * will be more clean.
-						 * At quic_conn level there is no mean to know that an application
-						 * want to forbid stream closure requests to receivers. This is the
-						 * case for the Control and QPACK h3 unidirectional streams.
-						 */
-						goto leave;
+					if (qc->app_ops == &h3_ops) {
+						if (!qc_h3_request_reject(qc, stream->id))
+							TRACE_ERROR("could not enqueue STOP_SENDING frame", QUIC_EV_CONN_PRSHPKT, qc);
 					}
-
-					if (!qc_stop_sending_frm_enqueue(qc, stream->id))
-						TRACE_ERROR("could not enqueue STOP_SENDING frame", QUIC_EV_CONN_PRSHPKT, qc);
 					/* This packet will not be acknowledged */
 					goto leave;
 				}