[6.0] 62c525e81 http2_hpack: Enforce h2_max_header_list_size

[Dridi Boukelmoune]

commit 62c525e81cb82ee391eeda1a8ec639abdf8af72d
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.
    
    Conflicts:
            include/tbl/params.h

diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index 00fbe7c6b..a157e9b29 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -235,6 +235,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 1b476c9aa..e32da14ed 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -276,6 +276,14 @@ h2h_decode_init(const struct h2_sess *h2)
 	XXXAN(d->out_l);
 	d->out = h2->new_req->http->ws->f;
 	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:
@@ -346,7 +354,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,
@@ -364,6 +372,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;
@@ -397,6 +406,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;
@@ -413,15 +423,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 = hp->ws->r - d->out;
+			d->limit -= d->out_u;
 			d->out_u = 0;
 			assert(d->out_u < d->out_l);
 		} 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 7c3945d33..2cb61260a 100644
--- a/include/tbl/params.h
+++ b/include/tbl/params.h
@@ -1897,13 +1897,21 @@ PARAM(
 	/* typ */       bytes_u,
 	/* min */       "0b",
 	/* max */	"4294967295b",
-	/* default */	"4294967295b",
+	/* default */	"0b",
 	/* units */     "bytes",
 	/* flags */     0,
 	/* s-text */
 	"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."
+	"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.",
 	/* l-text */    "",
 	/* func */      NULL
 )