From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Mon, 30 Jan 2023 11:12:43 +0000 (+0100)
Subject: BUG/MEDIUM: h3: handle STOP_SENDING on control stream
X-Git-Tag: v2.8-dev3~52
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=87f8766d3fbd10f9e8bf4902d37712612db64df5;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: h3: handle STOP_SENDING on control stream

Before this patch, STOP_SENDING reception was considered valid even on
H3 control stream. This causes the emission in return of RESET_STREAM
and eventually the closure and freeing of the QCS instance. This then
causes a crash during connection closure as a GOAWAY frame is emitted on
the control stream which is now released.

To fix this crash, STOP_SENDING on the control stream is now properly
rejected as specified by RFC 9114. The new app_ops close callback is
used which in turn will generate a CONNECTION_CLOSE with error
H3_CLOSED_CRITICAL_STREAM.

This bug was detected in github issue #2006. Note that however it is
triggered by an incorrect client behavior. It may be useful to determine
which client behaves like this. If this case is too frequent,
STOP_SENDING should probably be silently ignored.

To reproduce this issue, quiche was patched to emit a STOP_SENDING on
its send() function in quiche/src/lib.rs:
     pub fn send(&mut self, out: &mut [u8]) -> Result<(usize, SendInfo)> {
-        self.send_on_path(out, None, None)
+        let ret = self.send_on_path(out, None, None);
+        self.streams.mark_stopped(3, true, 0);
+        ret
     }

This must be backported up to 2.6 along with the preceeding commit :
  MINOR: mux-quic/h3: define close callback
---

diff --git a/src/h3.c b/src/h3.c
index e802d299cc..ea512a4043 100644
--- a/src/h3.c
+++ b/src/h3.c
@@ -1615,7 +1615,23 @@ static size_t h3_snd_buf(struct qcs *qcs, struct htx *htx, size_t count)
  */
 static int h3_close(struct qcs *qcs, enum qcc_app_ops_close_side side)
 {
-	/* TODO */
+	struct h3s *h3s = qcs->ctx;
+	struct h3c *h3c = h3s->h3c;;
+
+	/* RFC 9114 6.2.1. Control Streams
+	 *
+	 * The sender
+	 * MUST NOT close the control stream, and the receiver MUST NOT
+	 * request that the sender close the control stream.  If either
+	 * control stream is closed at any point, this MUST be treated
+	 * as a connection error of type H3_CLOSED_CRITICAL_STREAM.
+	 */
+	if (qcs == h3c->ctrl_strm) {
+		TRACE_ERROR("closure detected on control stream", H3_EV_H3S_END, qcs->qcc, qcs);
+		qcc_emit_cc_app(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
+		return 1;
+	}
+
 	return 0;
 }
 
diff --git a/src/mux_quic.c b/src/mux_quic.c
index 401ccc0c10..cae22df1ff 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -1232,6 +1232,13 @@ int qcc_recv_stop_sending(struct qcc *qcc, uint64_t id, uint64_t err)
 
 	qcs_idle_open(qcs);
 
+	if (qcc->app_ops->close) {
+		if (qcc->app_ops->close(qcs, QCC_APP_OPS_CLOSE_SIDE_WR)) {
+			TRACE_ERROR("closure rejected by app layer", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
+			goto out;
+		}
+	}
+
 	/* RFC 9000 3.5. Solicited State Transitions
 	 *
 	 * An endpoint that receives a STOP_SENDING frame