mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-08-06 23:27:04 +02:00
Code Issues Projects Releases Wiki Activity

BUG/MINOR: qpack: fix error code reported on QPACK decoding failure

Browse Source 
qpack_decode_fs() is used to decode QPACK field section on HTTP/3
headers parsing. Its return value is incoherent as it returns either
QPACK_DECOMPRESSION_FAILED defined in RFC 9204 or any other internal
values defined in qpack-dec.h. On failure, such return code is reused by
HTTP/3 layer to be reported via a CONNECTION_CLOSE frame. This is
incorrect if an internal error values was reported as it is not defined
by any specification.

Fir return values of qpack_decode_fs() in two ways. Firstly, fix invalid
usages of QPACK_DECOMPRESSION_FAILED when decoded content is too large
for the correct internal error QPACK_ERR_TOO_LARGE.

Secondly, adjust qpack_decode_fs() API to only returns internal code
values. A new internal enum QPACK_ERR_DECOMP is defined to replace
QPACK_DECOMPRESSION_FAILED. Caller is responsible to convert it to a
suitable error value. For other internal values, H3_INTERNAL_ERROR is
used. This is done through a set of convert functions.

This should be backported up to 2.6. Note that trailers are not
supported in 2.6 so chunk related to h3_trailers_to_htx() can be safely
skipped.
This commit is contained in:
Amaury Denoyelle 2024-05-13 16:01:08 +02:00
parent 4295dd21bd
commit 86aafd0236
3 changed files with 28 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
include/haproxy/qpack-dec.h
View File

@ -31,6 +31,7 @@ struct http_hdr;
*/ */
enum { enum {
QPACK_ERR_NONE = 0, /* no error */ QPACK_ERR_NONE = 0, /* no error */
QPACK_ERR_DECOMP, /* corresponds to RFC 9204 decompression error */
QPACK_ERR_RIC, /* cannot decode Required Insert Count prefix field */ QPACK_ERR_RIC, /* cannot decode Required Insert Count prefix field */
QPACK_ERR_DB, /* cannot decode Delta Base prefix field */ QPACK_ERR_DB, /* cannot decode Delta Base prefix field */
QPACK_ERR_TRUNCATED, /* truncated stream */ QPACK_ERR_TRUNCATED, /* truncated stream */
@ -50,4 +51,6 @@ int qpack_decode_fs(const unsigned char *buf, uint64_t len, struct buffer *tmp,
int qpack_decode_enc(struct buffer *buf, int fin, void *ctx); int qpack_decode_enc(struct buffer *buf, int fin, void *ctx);
int qpack_decode_dec(struct buffer *buf, int fin, void *ctx); int qpack_decode_dec(struct buffer *buf, int fin, void *ctx);
int qpack_err_decode(const int value);
#endif /* _HAPROXY_QPACK_DEC_H */ #endif /* _HAPROXY_QPACK_DEC_H */

12
src/h3.c
View File

@ -131,7 +131,7 @@ static uint64_t h3_settings_max_field_section_size = QUIC_VARINT_8_BYTE_MAX; /*
struct h3c { struct h3c {
struct qcc *qcc; struct qcc *qcc;
struct qcs *ctrl_strm; /* Control stream */ struct qcs *ctrl_strm; /* Control stream */
enum h3_err err; int err;
uint32_t flags; uint32_t flags;
/* Settings */ /* Settings */
@ -504,6 +504,12 @@ static int h3_set_authority(struct qcs *qcs, struct ist *auth, const struct ist
return 0; return 0;
} }
/* Return <value> as is or H3_INTERNAL_ERROR if negative. Useful to prepare a standard error code. */
static int h3_err(const int value)
{
return value >= 0 ? value : H3_INTERNAL_ERROR;
}
/* Parse from buffer <buf> a H3 HEADERS frame of length <len>. Data are copied /* Parse from buffer <buf> a H3 HEADERS frame of length <len>. Data are copied
* in a local HTX buffer and transfer to the stream connector layer. <fin> must be * in a local HTX buffer and transfer to the stream connector layer. <fin> must be
* set if this is the last data to transfer from this stream. * set if this is the last data to transfer from this stream.
@ -560,7 +566,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
list, sizeof(list) / sizeof(list[0])); list, sizeof(list) / sizeof(list[0]));
if (ret < 0) { if (ret < 0) {
TRACE_ERROR("QPACK decoding error", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); TRACE_ERROR("QPACK decoding error", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
h3c->err = -ret; h3c->err = h3_err(qpack_err_decode(ret));
len = -1; len = -1;
goto out; goto out;
} }
@ -939,7 +945,7 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
list, sizeof(list) / sizeof(list[0])); list, sizeof(list) / sizeof(list[0]));
if (ret < 0) { if (ret < 0) {
TRACE_ERROR("QPACK decoding error", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); TRACE_ERROR("QPACK decoding error", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
h3c->err = -ret; h3c->err = h3_err(qpack_err_decode(ret));
len = -1; len = -1;
goto out; goto out;
} }

24
src/qpack-dec.c
View File

@ -316,7 +316,7 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
* Count (Section 4.5.1), it MUST treat this as a connection error of * Count (Section 4.5.1), it MUST treat this as a connection error of
* type QPACK_DECOMPRESSION_FAILED. * type QPACK_DECOMPRESSION_FAILED.
*/ */
return -QPACK_DECOMPRESSION_FAILED; return -QPACK_ERR_DECOMP;
} }
else if (efl_type == QPACK_IFL_WPBI) { else if (efl_type == QPACK_IFL_WPBI) {
/* Indexed field line with post-base index /* Indexed field line with post-base index
@ -344,7 +344,7 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
* Count (Section 4.5.1), it MUST treat this as a connection error of * Count (Section 4.5.1), it MUST treat this as a connection error of
* type QPACK_DECOMPRESSION_FAILED. * type QPACK_DECOMPRESSION_FAILED.
*/ */
return -QPACK_DECOMPRESSION_FAILED; return -QPACK_ERR_DECOMP;
} }
else if (efl_type & QPACK_IFL_BIT) { else if (efl_type & QPACK_IFL_BIT) {
/* Indexed field line */ /* Indexed field line */
@ -375,7 +375,7 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
* *
* TODO adjust this when dynamic table support is implemented. * TODO adjust this when dynamic table support is implemented.
*/ */
return -QPACK_DECOMPRESSION_FAILED; return -QPACK_ERR_DECOMP;
} }
qpack_debug_printf(stderr, " t=%d index=%llu", !!static_tbl, (unsigned long long)index); qpack_debug_printf(stderr, " t=%d index=%llu", !!static_tbl, (unsigned long long)index);
@ -409,7 +409,7 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
* *
* TODO adjust this when dynamic table support is implemented. * TODO adjust this when dynamic table support is implemented.
*/ */
return -QPACK_DECOMPRESSION_FAILED; return -QPACK_ERR_DECOMP;
} }
qpack_debug_printf(stderr, " n=%d t=%d index=%llu", !!n, !!static_tbl, (unsigned long long)index); qpack_debug_printf(stderr, " n=%d t=%d index=%llu", !!n, !!static_tbl, (unsigned long long)index);
@ -429,7 +429,7 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
trash = chunk_newstr(tmp); trash = chunk_newstr(tmp);
if (!trash) { if (!trash) {
qpack_debug_printf(stderr, "##ERR@%d\n", __LINE__); qpack_debug_printf(stderr, "##ERR@%d\n", __LINE__);
ret = -QPACK_DECOMPRESSION_FAILED; ret = -QPACK_ERR_TOO_LARGE;
goto out; goto out;
} }
nlen = huff_dec(raw, length, trash, tmp->size - tmp->data); nlen = huff_dec(raw, length, trash, tmp->size - tmp->data);
@ -488,7 +488,7 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
trash = chunk_newstr(tmp); trash = chunk_newstr(tmp);
if (!trash) { if (!trash) {
qpack_debug_printf(stderr, "##ERR@%d\n", __LINE__); qpack_debug_printf(stderr, "##ERR@%d\n", __LINE__);
ret = -QPACK_DECOMPRESSION_FAILED; ret = -QPACK_ERR_TOO_LARGE;
goto out; goto out;
} }
nlen = huff_dec(raw, name_len, trash, tmp->size - tmp->data); nlen = huff_dec(raw, name_len, trash, tmp->size - tmp->data);
@ -533,7 +533,7 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
trash = chunk_newstr(tmp); trash = chunk_newstr(tmp);
if (!trash) { if (!trash) {
qpack_debug_printf(stderr, "##ERR@%d\n", __LINE__); qpack_debug_printf(stderr, "##ERR@%d\n", __LINE__);
ret = -QPACK_DECOMPRESSION_FAILED; ret = -QPACK_ERR_TOO_LARGE;
goto out; goto out;
} }
nlen = huff_dec(raw, value_len, trash, tmp->size - tmp->data); nlen = huff_dec(raw, value_len, trash, tmp->size - tmp->data);
@ -561,7 +561,7 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
*/ */
if (!name.len) { if (!name.len) {
qpack_debug_printf(stderr, "##ERR@%d\n", __LINE__); qpack_debug_printf(stderr, "##ERR@%d\n", __LINE__);
ret = -QPACK_DECOMPRESSION_FAILED; ret = -QPACK_ERR_DECOMP;
goto out; goto out;
} }
@ -586,3 +586,11 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
qpack_debug_printf(stderr, "-- done: ret=%d\n", ret); qpack_debug_printf(stderr, "-- done: ret=%d\n", ret);
return ret; return ret;
} }
/* Convert return value from qpack_decode_fs() to a standard error code usable
* in CONNECTION_CLOSE or -1 for an internal error.
*/
int qpack_err_decode(const int value)
{
return (value == -QPACK_ERR_DECOMP) ? QPACK_DECOMPRESSION_FAILED : -1;
}