Mailing List Archive

commit 0c3086e458af8df0f8fb5068275aa5e3177fbddf
Author: Dridi Boukelmoune <dridi.boukelmoune@gmail.com>
Date: Thu Mar 28 16:50:39 2024 +0100

http2_hpack: Enforce h2_max_header_list_size

This parameter has a new role that consists in interrupting connections
when decoding an HPACK block leads to a header list so large that the
client must be stopped.

By default, too large is 150% of http_req_size.

diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index 9fbb58443..89e32309c 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -236,6 +236,7 @@ struct h2h_decode {
enum vhd_ret_e vhd_ret;
char *out;
char *reset;
+ int64_t limit;
size_t out_l;
size_t out_u;
size_t namelen;
diff --git a/bin/varnishd/http2/cache_http2_hpack.c b/bin/varnishd/http2/cache_http2_hpack.c
index 4d155e3d1..c4274a656 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -278,6 +278,14 @@ h2h_decode_init(const struct h2_sess *h2)
XXXAN(d->out_l);
d->out = WS_Reservation(h2->new_req->http->ws);
d->reset = d->out;
+
+ if (cache_param->h2_max_header_list_size == 0)
+ d->limit = h2->local_settings.max_header_list_size * 1.5;
+ else
+ d->limit = cache_param->h2_max_header_list_size;
+
+ if (d->limit < h2->local_settings.max_header_list_size)
+ d->limit = INT64_MAX;
}

/* Possible error returns:
@@ -351,7 +359,7 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
if (d->error != NULL)
assert(H2_ERROR_MATCH(d->error, H2SE_ENHANCE_YOUR_CALM));

- while (1) {
+ while (d->limit >= 0) {
AN(d->out);
assert(d->out_u <= d->out_l);
d->vhd_ret = VHD_Decode(d->vhd, h2->dectbl, in, in_l, &in_u,
@@ -369,6 +377,7 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
}

if (H2_ERROR_MATCH(d->error, H2SE_ENHANCE_YOUR_CALM)) {
+ d->limit -= d->out_u;
d->out_u = 0;
assert(d->out_u < d->out_l);
continue;
@@ -402,6 +411,7 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
d->out[d->out_u++] = '\0'; /* Zero guard */
d->out += d->out_u;
d->out_l -= d->out_u;
+ d->limit -= d->out_u;
d->out_u = 0;
d->namelen = 0;
break;
@@ -418,15 +428,25 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
if (H2_ERROR_MATCH(d->error, H2SE_ENHANCE_YOUR_CALM)) {
d->out = d->reset;
d->out_l = e - d->out;
+ d->limit -= d->out_u;
d->out_u = 0;
assert(d->out_l > 0);
} else if (d->error)
break;
}

- if (H2_ERROR_MATCH(d->error, H2SE_ENHANCE_YOUR_CALM))
- return (0); /* Stream error, delay reporting until
- h2h_decode_fini so that we can process the
- complete header block */
+ if (d->limit < 0) {
+ /* Fatal error, the client exceeded both http_req_size
+ * and h2_max_header_list_size. */
+ VSLb(h2->vsl, SLT_SessError, "Header list too large");
+ return (H2CE_ENHANCE_YOUR_CALM);
+ }
+
+ if (H2_ERROR_MATCH(d->error, H2SE_ENHANCE_YOUR_CALM)) {
+ /* Stream error, delay reporting until h2h_decode_fini so
+ * that we can process the complete header block. */
+ return (NULL);
+ }
+
return (d->error);
}
diff --git a/bin/varnishtest/tests/f00015.vtc b/bin/varnishtest/tests/f00015.vtc
new file mode 100644
index 000000000..de5df8ed2
--- /dev/null
+++ b/bin/varnishtest/tests/f00015.vtc
@@ -0,0 +1,19 @@
+varnishtest "h2 CONTINUATION flood"
+
+varnish v1 -cliok "param.set feature +http2"
+varnish v1 -cliok "param.set vsl_mask -H2RxHdr,-H2RxBody"
+varnish v1 -vcl { backend be none; } -start
+
+client c1 {
+ non_fatal
+ stream next {
+ txreq -nohdrend
+ loop 1000 {
+ txcont -nohdrend -hdr noise ${string,repeat,4096,x}
+ }
+ txcont -hdr last header
+ } -run
+} -run
+
+varnish v1 -expect MAIN.s_req_hdrbytes < 65536
+varnish v1 -expect MAIN.sc_overload == 1
diff --git a/include/tbl/params.h b/include/tbl/params.h
index 687a7c2f2..4ffdddae1 100644
--- a/include/tbl/params.h
+++ b/include/tbl/params.h
@@ -1277,12 +1277,20 @@ PARAM_SIMPLE(
/* type */ bytes_u,
/* min */ "0b",
/* max */ "4294967295b",
- /* def */ "4294967295b",
+ /* def */ "0b",
/* units */ "bytes",
/* descr */
"HTTP2 maximum size of an uncompressed header list. This parameter "
"is not mapped to " H2_SETTING_NAME(MAX_HEADER_LIST_SIZE) " in the "
- "initial SETTINGS frame, the http_req_size parameter is instead."
+ "initial SETTINGS frame, the http_req_size parameter is instead.\n\n"
+ "The http_req_size advises HTTP2 clients of the maximum size for "
+ "the header list. Exceeding http_req_size results in a reset stream "
+ "after processing the HPACK block to perserve the connection, but "
+ "exceeding h2_max_header_list_size results in the HTTP2 connection "
+ "going away immediately.\n\n"
+ "If h2_max_header_list_size is lower than http_req_size, it has no "
+ "effect, except for the special value zero interpreted as 150% of "
+ "http_req_size."
)

#undef H2_SETTING_DESCR
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit