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 9239e218360a7c9f6204e529374f19c2175658f7..993f4509ea10612f43649153b965a878e7ab91dc 100644 (file)
--- 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 026f7d9e71d93785cf45ec04aba2a5a0726050f4..ea24fe8cd75b5955d6a14a2cd08533ee089d2740 100644 (file)
--- 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 aa30121e47b3b81de0cfaf2eaaf14aa7531599b2..0da6cf89a72762891654b10a346a7d57d7da90b7 100644 (file)
--- 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);