From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Tue, 16 Aug 2022 15:42:47 +0000 (+0200)
Subject: MINOR: qpack: report error on enc/dec stream close
X-Git-Tag: v2.7-dev4~29
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=26aa399d6b245da3e82e768dd15931263842d7d2;p=thirdparty%2Fhaproxy.git

MINOR: qpack: report error on enc/dec stream close

As specified by RFC 9204, encoder and decoder streams must not be
closed. If the peer behaves incorrectly and closes one of them, emit a
H3_CLOSED_CRITICAL_STREAM connection error.

To implement this, QPACK stream decoding API has been slightly adjusted.
Firstly, fin parameter is passed to notify about FIN STREAM bit.
Secondly, qcs instance is passed via unused void* context. This allows
to use qcc_emit_cc_app() function to report a CONNECTION_CLOSE error.
---

diff --git a/include/haproxy/qpack-dec.h b/include/haproxy/qpack-dec.h
index 9239e21836..993f4509ea 100644
--- a/include/haproxy/qpack-dec.h
+++ b/include/haproxy/qpack-dec.h
@@ -45,7 +45,7 @@ struct qpack_dec {
 
 int qpack_decode_fs(const unsigned char *buf, uint64_t len, struct buffer *tmp,
                     struct http_hdr *list, int list_size);
-int qpack_decode_enc(struct buffer *buf, void *ctx);
-int qpack_decode_dec(struct buffer *buf, void *ctx);
+int qpack_decode_enc(struct buffer *buf, int fin, void *ctx);
+int qpack_decode_dec(struct buffer *buf, int fin, void *ctx);
 
 #endif /* _HAPROXY_QPACK_DEC_H */
diff --git a/src/h3.c b/src/h3.c
index 026f7d9e71..ea24fe8cd7 100644
--- a/src/h3.c
+++ b/src/h3.c
@@ -220,12 +220,13 @@ static ssize_t h3_init_uni_stream(struct h3c *h3c, struct qcs *qcs,
 	return len;
 }
 
-/* Parse an uni-stream <qcs> from <rxbuf> which does not contains H3 frames.
- * This may be used for QPACK encoder/decoder streams for example.
+/* Parse a buffer <b> for a <qcs> uni-stream which does not contains H3 frames.
+ * This may be used for QPACK encoder/decoder streams for example. <fin> is set
+ * if this is the last frame of the stream.
  *
  * Returns the number of consumed bytes or a negative error code.
  */
-static ssize_t h3_parse_uni_stream_no_h3(struct qcs *qcs, struct buffer *b)
+static ssize_t h3_parse_uni_stream_no_h3(struct qcs *qcs, struct buffer *b, int fin)
 {
 	struct h3s *h3s = qcs->ctx;
 
@@ -234,11 +235,11 @@ static ssize_t h3_parse_uni_stream_no_h3(struct qcs *qcs, struct buffer *b)
 
 	switch (h3s->type) {
 	case H3S_T_QPACK_DEC:
-		if (qpack_decode_dec(b, NULL))
+		if (qpack_decode_dec(b, fin, qcs))
 			return -1;
 		break;
 	case H3S_T_QPACK_ENC:
-		if (qpack_decode_enc(b, NULL))
+		if (qpack_decode_enc(b, fin, qcs))
 			return -1;
 		break;
 	case H3S_T_UNKNOWN:
@@ -618,7 +619,7 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
 
 	if (quic_stream_is_uni(qcs->id) && (h3s->flags & H3_SF_UNI_NO_H3)) {
 		/* For non-h3 STREAM, parse it and return immediately. */
-		if ((ret = h3_parse_uni_stream_no_h3(qcs, b)) < 0)
+		if ((ret = h3_parse_uni_stream_no_h3(qcs, b, fin)) < 0)
 			return -1;
 
 		total += ret;
diff --git a/src/qpack-dec.c b/src/qpack-dec.c
index aa30121e47..0da6cf89a7 100644
--- a/src/qpack-dec.c
+++ b/src/qpack-dec.c
@@ -27,6 +27,7 @@
 #include <haproxy/buf.h>
 #include <haproxy/chunk.h>
 #include <haproxy/h3.h>
+#include <haproxy/mux_quic.h>
 #include <haproxy/qpack-t.h>
 #include <haproxy/qpack-dec.h>
 #include <haproxy/qpack-tbl.h>
@@ -96,11 +97,24 @@ static uint64_t qpack_get_varint(const unsigned char **buf, uint64_t *len_in, in
  *
  * Returns 0 on success else non-zero.
  */
-int qpack_decode_enc(struct buffer *buf, void *ctx)
+int qpack_decode_enc(struct buffer *buf, int fin, void *ctx)
 {
+	struct qcs *qcs = ctx;
 	size_t len;
 	unsigned char inst;
 
+	/* RFC 9204 4.2. Encoder and Decoder Streams
+	 *
+	 * The sender MUST NOT close either of these streams, and the receiver
+	 * MUST NOT request that the sender close either of these streams.
+	 * Closure of either unidirectional stream type MUST be treated as a
+	 * connection error of type H3_CLOSED_CRITICAL_STREAM.
+	 */
+	if (fin) {
+		qcc_emit_cc_app(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
+		return -1;
+	}
+
 	len = b_data(buf);
 	qpack_debug_hexdump(stderr, "[QPACK-DEC-ENC] ", b_head(buf), 0, len);
 
@@ -130,11 +144,24 @@ int qpack_decode_enc(struct buffer *buf, void *ctx)
  *
  * Returns 0 on success else non-zero.
  */
-int qpack_decode_dec(struct buffer *buf, void *ctx)
+int qpack_decode_dec(struct buffer *buf, int fin, void *ctx)
 {
+	struct qcs *qcs = ctx;
 	size_t len;
 	unsigned char inst;
 
+	/* RFC 9204 4.2. Encoder and Decoder Streams
+	 *
+	 * The sender MUST NOT close either of these streams, and the receiver
+	 * MUST NOT request that the sender close either of these streams.
+	 * Closure of either unidirectional stream type MUST be treated as a
+	 * connection error of type H3_CLOSED_CRITICAL_STREAM.
+	 */
+	if (fin) {
+		qcc_emit_cc_app(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
+		return -1;
+	}
+
 	len = b_data(buf);
 	qpack_debug_hexdump(stderr, "[QPACK-DEC-DEC] ", b_head(buf), 0, len);