BUG/MINOR: mux-quic: fix transport VS app CONNECTION_CLOSE

A recent series of patch were introduced to streamline error generation
by QUIC MUX. However, a regression was introduced : every error
generated by the MUX was built as CONNECTION_CLOSE_APP frame, whereas it
should be only for H3/QPACK errors.

Fix this by adding an argument <app> in qcc_set_error. When false, a
standard CONNECTION_CLOSE is used as error.

This bug was detected by QUIC tracker with the following tests
"stop_sending" and "server_flow_control" which requires a
CONNECTION_CLOSE frame.

This must be backported up to 2.7.

diff --git a/include/haproxy/mux_quic.h b/include/haproxy/mux_quic.h
index e425e61d2f28da8589b193bb85e29596eff4e676..0e408bd1872a621dda3a5d85e31762ac9cba9fa5 100644 (file)
--- a/include/haproxy/mux_quic.h
+++ b/include/haproxy/mux_quic.h
@@ -12,7 +12,7 @@
 #include <haproxy/mux_quic-t.h>
 #include <haproxy/stream.h>
 
-void qcc_set_error(struct qcc *qcc, int err);
+void qcc_set_error(struct qcc *qcc, int err, int app);
 struct qcs *qcc_init_stream_local(struct qcc *qcc, int bidi);
 struct buffer *qc_get_buf(struct qcs *qcs, struct buffer *bptr);
 

diff --git a/src/h3.c b/src/h3.c
index a364212f504ea92f206274ba0dca0e747099800d..e686f0de0054caf7b641a172f58a5f42af3cadb6 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -191,7 +191,7 @@ static ssize_t h3_init_uni_stream(struct h3c *h3c, struct qcs *qcs,
        case H3_UNI_S_T_CTRL:
                if (h3c->flags & H3_CF_UNI_CTRL_SET) {
                        TRACE_ERROR("duplicated control stream", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
-                       qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR);
+                       qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR, 1);
                        goto err;
                }
                h3c->flags |= H3_CF_UNI_CTRL_SET;
@@ -206,7 +206,7 @@ static ssize_t h3_init_uni_stream(struct h3c *h3c, struct qcs *qcs,
        case H3_UNI_S_T_QPACK_DEC:
                if (h3c->flags & H3_CF_UNI_QPACK_DEC_SET) {
                        TRACE_ERROR("duplicated qpack decoder stream", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
-                       qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR);
+                       qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR, 1);
                        goto err;
                }
                h3c->flags |= H3_CF_UNI_QPACK_DEC_SET;
@@ -217,7 +217,7 @@ static ssize_t h3_init_uni_stream(struct h3c *h3c, struct qcs *qcs,
        case H3_UNI_S_T_QPACK_ENC:
                if (h3c->flags & H3_CF_UNI_QPACK_ENC_SET) {
                        TRACE_ERROR("duplicated qpack encoder stream", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
-                       qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR);
+                       qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR, 1);
                        goto err;
                }
                h3c->flags |= H3_CF_UNI_QPACK_ENC_SET;
@@ -1031,7 +1031,7 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
         */
        if (h3s->type == H3S_T_CTRL && fin) {
                TRACE_ERROR("control stream closed by remote peer", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
-               qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM);
+               qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
                goto err;
        }
 
@@ -1067,7 +1067,7 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
 
                        if (!h3_is_frame_valid(h3c, qcs, ftype)) {
                                TRACE_ERROR("received an invalid frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
-                               qcc_set_error(qcs->qcc, H3_FRAME_UNEXPECTED);
+                               qcc_set_error(qcs->qcc, H3_FRAME_UNEXPECTED, 1);
                                goto err;
                        }
 
@@ -1090,7 +1090,7 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
                         */
                        if (flen > QC_S_RX_BUF_SZ) {
                                TRACE_ERROR("received a too big frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
-                               qcc_set_error(qcs->qcc, H3_EXCESSIVE_LOAD);
+                               qcc_set_error(qcs->qcc, H3_EXCESSIVE_LOAD, 1);
                                goto err;
                        }
                        break;
@@ -1129,7 +1129,7 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
                        ret = h3_parse_settings_frm(qcs->qcc->ctx, b, flen);
                        if (ret < 0) {
                                TRACE_ERROR("error on SETTINGS parsing", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
-                               qcc_set_error(qcs->qcc, h3c->err);
+                               qcc_set_error(qcs->qcc, h3c->err, 1);
                                goto err;
                        }
                        h3c->flags |= H3_CF_SETTINGS_RECV;
@@ -1163,7 +1163,7 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
                return b_data(b);
        }
        else if (h3c->err) {
-               qcc_set_error(qcs->qcc, h3c->err);
+               qcc_set_error(qcs->qcc, h3c->err, 1);
                return b_data(b);
        }
 
@@ -1670,7 +1670,7 @@ static int h3_close(struct qcs *qcs, enum qcc_app_ops_close_side side)
         */
        if (qcs == h3c->ctrl_strm || h3s->type == H3S_T_CTRL) {
                TRACE_ERROR("closure detected on control stream", H3_EV_H3S_END, qcs->qcc->conn, qcs);
-               qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM);
+               qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
                return 1;
        }
 

diff --git a/src/mux_quic.c b/src/mux_quic.c
index e816613e8872802d1c23155a22feb43d7041e472..705008872fc7de12dd1f82f77689e3927e67c13b 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -497,10 +497,11 @@ void qcs_notify_send(struct qcs *qcs)
 }
 
 /* A fatal error is detected locally for <qcc> connection. It should be closed
- * with a CONNECTION_CLOSE using <err> code. This function must not be called
- * more than once by connection.
+ * with a CONNECTION_CLOSE using <err> code. Set <app> to true to indicate that
+ * the code must be considered as an application level error. This function
+ * must not be called more than once by connection.
  */
-void qcc_set_error(struct qcc *qcc, int err)
+void qcc_set_error(struct qcc *qcc, int err, int app)
 {
        /* This must not be called multiple times per connection. */
        BUG_ON(qcc->flags & QC_CF_ERRL);
@@ -508,7 +509,7 @@ void qcc_set_error(struct qcc *qcc, int err)
        TRACE_STATE("connection on error", QMUX_EV_QCC_ERR, qcc->conn);
 
        qcc->flags |= QC_CF_ERRL;
-       qcc->err = quic_err_app(err);
+       qcc->err = app ? quic_err_app(err) : quic_err_transport(err);
 }
 
 /* Open a locally initiated stream for the connection <qcc>. Set <bidi> for a
@@ -541,7 +542,7 @@ struct qcs *qcc_init_stream_local(struct qcc *qcc, int bidi)
        qcs = qcs_new(qcc, *next, type);
        if (!qcs) {
                TRACE_LEAVE(QMUX_EV_QCS_NEW, qcc->conn);
-               qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR);
+               qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR, 0);
                return NULL;
        }
 
@@ -588,7 +589,7 @@ static struct qcs *qcc_init_stream_remote(struct qcc *qcc, uint64_t id)
                                           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_set_error(qcc, QC_ERR_STREAM_LIMIT_ERROR);
+               qcc_set_error(qcc, QC_ERR_STREAM_LIMIT_ERROR, 0);
                goto err;
        }
 
@@ -602,7 +603,7 @@ static struct qcs *qcc_init_stream_remote(struct qcc *qcc, uint64_t id)
                qcs = qcs_new(qcc, *largest, type);
                if (!qcs) {
                        TRACE_ERROR("stream fallocation failure", QMUX_EV_QCS_NEW, qcc->conn);
-                       qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR);
+                       qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR, 0);
                        goto err;
                }
 
@@ -661,13 +662,13 @@ int qcc_get_qcs(struct qcc *qcc, uint64_t id, int receive_only, int send_only,
 
        if (!receive_only && quic_stream_is_uni(id) && quic_stream_is_remote(qcc, id)) {
                TRACE_ERROR("receive-only stream not allowed", QMUX_EV_QCC_RECV|QMUX_EV_QCC_NQCS|QMUX_EV_PROTO_ERR, qcc->conn, NULL, &id);
-               qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR);
+               qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR, 0);
                goto err;
        }
 
        if (!send_only && quic_stream_is_uni(id) && quic_stream_is_local(qcc, id)) {
                TRACE_ERROR("send-only stream not allowed", QMUX_EV_QCC_RECV|QMUX_EV_QCC_NQCS|QMUX_EV_PROTO_ERR, qcc->conn, NULL, &id);
-               qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR);
+               qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR, 0);
                goto err;
        }
 
@@ -699,7 +700,7 @@ int qcc_get_qcs(struct qcc *qcc, uint64_t id, int receive_only, int send_only,
                 * stream.
                 */
                TRACE_ERROR("locally initiated stream not yet created", QMUX_EV_QCC_RECV|QMUX_EV_QCC_NQCS|QMUX_EV_PROTO_ERR, qcc->conn, NULL, &id);
-               qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR);
+               qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR, 0);
                goto err;
        }
        else {
@@ -755,7 +756,7 @@ static void qcs_consume(struct qcs *qcs, uint64_t bytes)
                TRACE_DATA("increase stream credit via MAX_STREAM_DATA", QMUX_EV_QCS_RECV, qcc->conn, qcs);
                frm = qc_frm_alloc(QUIC_FT_MAX_STREAM_DATA);
                if (!frm) {
-                       qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR);
+                       qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR, 0);
                        return;
                }
 
@@ -774,7 +775,7 @@ static void qcs_consume(struct qcs *qcs, uint64_t bytes)
                TRACE_DATA("increase conn credit via MAX_DATA", QMUX_EV_QCS_RECV, qcc->conn, qcs);
                frm = qc_frm_alloc(QUIC_FT_MAX_DATA);
                if (!frm) {
-                       qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR);
+                       qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR, 0);
                        return;
                }
 
@@ -989,7 +990,7 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
        if (qcs->flags & QC_SF_SIZE_KNOWN &&
            (offset + len > qcs->rx.offset_max || (fin && offset + len < qcs->rx.offset_max))) {
                TRACE_ERROR("final size error", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV|QMUX_EV_PROTO_ERR, qcc->conn, qcs);
-               qcc_set_error(qcc, QC_ERR_FINAL_SIZE_ERROR);
+               qcc_set_error(qcc, QC_ERR_FINAL_SIZE_ERROR, 0);
                goto err;
        }
 
@@ -1022,7 +1023,7 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
                         */
                        TRACE_ERROR("flow control error", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV|QMUX_EV_PROTO_ERR,
                                    qcc->conn, qcs);
-                       qcc_set_error(qcc, QC_ERR_FLOW_CONTROL_ERROR);
+                       qcc_set_error(qcc, QC_ERR_FLOW_CONTROL_ERROR, 0);
                        goto err;
                }
        }
@@ -1061,7 +1062,7 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
                         */
                        TRACE_ERROR("overlapping data rejected", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV|QMUX_EV_PROTO_ERR,
                                    qcc->conn, qcs);
-                       qcc_set_error(qcc, QC_ERR_PROTOCOL_VIOLATION);
+                       qcc_set_error(qcc, QC_ERR_PROTOCOL_VIOLATION, 0);
                        return 1;
 
                case NCB_RET_GAP_SIZE:
@@ -1194,7 +1195,7 @@ int qcc_recv_reset_stream(struct qcc *qcc, uint64_t id, uint64_t err, uint64_t f
         */
        if (qcc_get_qcs(qcc, id, 1, 0, &qcs)) {
                TRACE_ERROR("RESET_STREAM for send-only stream received", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
-               qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR);
+               qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR, 0);
                goto err;
        }
 
@@ -1214,7 +1215,7 @@ int qcc_recv_reset_stream(struct qcc *qcc, uint64_t id, uint64_t err, uint64_t f
        if (qcs->rx.offset_max > final_size ||
            ((qcs->flags & QC_SF_SIZE_KNOWN) && qcs->rx.offset_max != final_size)) {
                TRACE_ERROR("final size error on RESET_STREAM", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
-               qcc_set_error(qcc, QC_ERR_FINAL_SIZE_ERROR);
+               qcc_set_error(qcc, QC_ERR_FINAL_SIZE_ERROR, 0);
                goto err;
        }
 
@@ -1341,7 +1342,7 @@ static int qcc_release_remote_stream(struct qcc *qcc, uint64_t id)
                        TRACE_DATA("increase max stream limit with MAX_STREAMS_BIDI", QMUX_EV_QCC_SEND, qcc->conn);
                        frm = qc_frm_alloc(QUIC_FT_MAX_STREAMS_BIDI);
                        if (!frm) {
-                               qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR);
+                               qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR, 0);
                                goto err;
                        }
 

diff --git a/src/qpack-dec.c b/src/qpack-dec.c
index 1ae2ef35d51fa6fe384f49b2353bd496881bdb48..97392bbc3e80b3bdb44dc68c21989ff6985e7914 100644 (file)
--- a/src/qpack-dec.c
+++ b/src/qpack-dec.c
@@ -111,7 +111,7 @@ int qpack_decode_enc(struct buffer *buf, int fin, void *ctx)
         * connection error of type H3_CLOSED_CRITICAL_STREAM.
         */
        if (fin) {
-               qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM);
+               qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
                return -1;
        }
 
@@ -158,7 +158,7 @@ int qpack_decode_dec(struct buffer *buf, int fin, void *ctx)
         * connection error of type H3_CLOSED_CRITICAL_STREAM.
         */
        if (fin) {
-               qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM);
+               qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
                return -1;
        }