[master] 0e75d4635 http2_hpack: Enforce h2_max_header_list_size

[Dridi Boukelmoune]

commit 0e75d46357fc26ab59b9f660460d7c748f2c8be4
Author: Dridi Boukelmoune <dridi.boukelmoune at 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 124826805..1b50d4cda 100644
--- a/include/tbl/params.h
+++ b/include/tbl/params.h
@@ -1289,12 +1289,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