[6.0] 2069001c4 http2_hpack: Refuse multiple :authority pseudo headers

[Dridi Boukelmoune]

commit 2069001c4a003489530f14746f4fbdd91a0d585e
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Wed Mar 27 16:17:08 2024 +0100

    http2_hpack: Refuse multiple :authority pseudo headers
    
    It became explicit in rfc9113:
    
    > The same pseudo-header field name MUST NOT appear more than once in a
    > field block.
    
    While at it, the duplicate pseudo-header error can be consolidated in a
    single location instead of adding one more branch.

diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index 53339dbe5..26cba9055 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -226,6 +226,7 @@ struct h2h_decode {
 	unsigned			magic;
 #define H2H_DECODE_MAGIC		0xd092bde4
 
+	unsigned			has_authority:1;
 	unsigned			has_scheme:1;
 	h2_error			error;
 	enum vhd_ret_e			vhd_ret;
diff --git a/bin/varnishd/http2/cache_http2_hpack.c b/bin/varnishd/http2/cache_http2_hpack.c
index 739cb0e80..14009502e 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -143,7 +143,7 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
 	txt hdr, nm, val;
 	int disallow_empty;
 	const char *p;
-	unsigned n;
+	unsigned n, has_dup;
 
 	CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
 	h2h_assert_ready(d);
@@ -160,6 +160,7 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
 	val.e = hdr.e;
 
 	disallow_empty = 0;
+	has_dup = 0;
 
 	if (Tlen(hdr) > UINT_MAX) {	/* XXX: cache_param max header size */
 		VSLb(hp->vsl, SLT_BogoHeader, "Header too large: %.20s", hdr.b);
@@ -203,14 +204,8 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
 			/* XXX: What to do about this one? (typically
 			   "http" or "https"). For now set it as a normal
 			   header, stripping the first ':'. */
-			if (d->has_scheme) {
-				VSLb(hp->vsl, SLT_BogoHeader,
-				    "Duplicate pseudo-header :scheme: %.*s",
-				    vmin_t(int, Tlen(val), 20), val.b);
-				return (H2SE_PROTOCOL_ERROR);
-			}
-
 			hdr.b++;
+			has_dup = d->has_scheme;
 			d->has_scheme = 1;
 			disallow_empty = 1;
 
@@ -226,6 +221,8 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
 			memcpy(d->out + 6, "host", 4);
 			hdr.b += 6;
 			nm = Tstr(":authority"); /* preserve original */
+			has_dup = d->has_authority;
+			d->has_authority = 1;
 		} else {
 			/* Unknown pseudo-header */
 			VSLb(hp->vsl, SLT_BogoHeader,
@@ -242,14 +239,7 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
 		return (H2SE_PROTOCOL_ERROR);
 	}
 
-	if (n < HTTP_HDR_FIRST) {
-		if (hp->hd[n].b != NULL) {
-			VSLb(hp->vsl, SLT_BogoHeader,
-			    "Duplicate pseudo-header %.*s",
-			    (int)Tlen(nm), nm.b);
-			return (H2SE_PROTOCOL_ERROR);	// rfc7540,l,3158,3162
-		}
-	} else {
+	if (n >= HTTP_HDR_FIRST) {
 		/* Check for space in struct http */
 		if (n >= hp->shd) {
 			VSLb(hp->vsl, SLT_LostHeader,
@@ -258,6 +248,15 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
 			return (H2SE_ENHANCE_YOUR_CALM);
 		}
 		hp->nhd++;
+		AZ(hp->hd[n].b);
+	}
+
+	if (has_dup || hp->hd[n].b != NULL) {
+		assert(nm.b[0] == ':');
+		VSLb(hp->vsl, SLT_BogoHeader,
+		    "Duplicate pseudo-header %.*s",
+		    (int)Tlen(nm), nm.b);
+		return (H2SE_PROTOCOL_ERROR);	// rfc7540,l,3158,3162
 	}
 
 	hp->hd[n] = hdr;
diff --git a/bin/varnishtest/tests/r02351.vtc b/bin/varnishtest/tests/r02351.vtc
index d2ee19af8..6fdee96d5 100644
--- a/bin/varnishtest/tests/r02351.vtc
+++ b/bin/varnishtest/tests/r02351.vtc
@@ -1,4 +1,4 @@
-varnishtest "#2351: :path/:method error handling"
+varnishtest "#2351: h2 pseudo-headers error handling"
 
 server s1 {
 	rxreq
@@ -39,6 +39,16 @@ client c1 {
 	} -run
 } -run
 
+client c2 {
+	# Duplicate :authority
+	stream next {
+		txreq -noadd -hdr :path / -hdr :method GET -hdr :scheme http \
+		    -hdr :authority example.com -hdr :authority example.org
+		rxrst
+		expect rst.err == PROTOCOL_ERROR
+	} -run
+} -run
+
 varnish v1 -expect MEMPOOL.req0.live == 0
 varnish v1 -expect MEMPOOL.req1.live == 0
 varnish v1 -expect MEMPOOL.sess0.live == 0