RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2025-02-16 10:36:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
Amaury Denoyelle 2023-05-09 18:01:09 +02:00
parent 89fb210214
commit 58721f2192
4 changed files with 31 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/haproxy/mux_quic.h
View File

@ -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);

18
src/h3.c
View File

@ -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;
}

37
src/mux_quic.c
View File

@ -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;
}

4
src/qpack-dec.c
View File

@ -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;
}