[6.0] 6fbb93775 h2: the :scheme pseudo header is not optional

Martin Blix Grydeland martin at varnish-software.com
Tue Nov 8 10:03:08 UTC 2022


commit 6fbb937758dfa7ad131d5a40d2c9b7b5929dc7dd
Author: Asad Sajjad Ahmed <asadsa at varnish-software.com>
Date:   Wed Sep 28 14:58:38 2022 +0200

    h2: the :scheme pseudo header is not optional
    
    The :scheme pseudo header is not optional in H/2 except when doing CONNECT.
    There is also a strict requirement for it appear only once.
    
    Signed-off-by: Asad Sajjad Ahmed <asadsa at varnish-software.com>

diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index 205b96ccb..bba2b7e7d 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -199,11 +199,14 @@ vtr_deliver_f h2_deliver;
 vtr_minimal_response_f h2_minimal_response;
 #endif /* TRANSPORT_MAGIC */
 
+#define H2H_DECODE_FLAG_SCHEME_SEEN	0x1
+
 /* http2/cache_http2_hpack.c */
 struct h2h_decode {
 	unsigned			magic;
 #define H2H_DECODE_MAGIC		0xd092bde4
 
+	int				flags;
 	h2_error			error;
 	enum vhd_ret_e			vhd_ret;
 	char				*out;
diff --git a/bin/varnishd/http2/cache_http2_hpack.c b/bin/varnishd/http2/cache_http2_hpack.c
index e247e8f8c..45cddb34b 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -90,7 +90,7 @@ h2h_checkhdr(const struct http *hp, const char *b, size_t namelen, size_t len)
 }
 
 static h2_error
-h2h_addhdr(struct http *hp, char *b, size_t namelen, size_t len)
+h2h_addhdr(struct http *hp, char *b, size_t namelen, size_t len, int *flags)
 {
 	/* XXX: This might belong in cache/cache_http.c */
 	const char *b0;
@@ -142,9 +142,18 @@ h2h_addhdr(struct http *hp, char *b, size_t namelen, size_t len)
 			/* XXX: What to do about this one? (typically
 			   "http" or "https"). For now set it as a normal
 			   header, stripping the first ':'. */
+			if (*flags & H2H_DECODE_FLAG_SCHEME_SEEN) {
+				VSLb(hp->vsl, SLT_BogoHeader,
+				    "Duplicate pseudo-header %.*s%.*s",
+				    (int)namelen, b0,
+				    (int)(len > 20 ? 20 : len), b);
+				return (H2SE_PROTOCOL_ERROR);
+			}
+
 			b++;
 			len-=1;
 			n = hp->nhd;
+			*flags |= H2H_DECODE_FLAG_SCHEME_SEEN;
 
 			for (p = b + namelen, i = 0; i < len-namelen;
 			    p++, i++) {
@@ -330,7 +339,8 @@ h2h_decode_bytes(struct h2_sess *h2, const uint8_t *in, size_t in_l)
 			    d->out_u);
 			if (d->error)
 				break;
-			d->error = h2h_addhdr(hp, d->out, d->namelen, d->out_u);
+			d->error = h2h_addhdr(hp, d->out, d->namelen, d->out_u,
+			    &d->flags);
 			if (d->error)
 				break;
 			d->out[d->out_u++] = '\0'; /* Zero guard */
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 98f5dc4f3..03a8cc5f0 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -545,11 +545,13 @@ static h2_error
 h2_end_headers(struct worker *wrk, struct h2_sess *h2,
     struct req *req, struct h2_req *r2)
 {
+	int scheme_seen;
 	h2_error h2e;
 	ssize_t cl;
 
 	ASSERT_RXTHR(h2);
 	assert(r2->state == H2_S_OPEN);
+	scheme_seen = h2->decode->flags & H2H_DECODE_FLAG_SCHEME_SEEN;
 	h2e = h2h_decode_fini(h2);
 	h2->new_req = NULL;
 	if (r2->req->req_body_status == REQ_BODY_NONE) {
@@ -597,10 +599,17 @@ h2_end_headers(struct worker *wrk, struct h2_sess *h2,
 		VSLb(h2->vsl, SLT_Debug, "Missing :method");
 		return (H2SE_PROTOCOL_ERROR); //rfc7540,l,3087,3090
 	}
+
 	if (req->http->hd[HTTP_HDR_URL].b == NULL) {
 		VSLb(h2->vsl, SLT_Debug, "Missing :path");
 		return (H2SE_PROTOCOL_ERROR); //rfc7540,l,3087,3090
 	}
+
+	if (!(scheme_seen)) {
+		VSLb(h2->vsl, SLT_Debug, "Missing :scheme");
+		return (H2SE_PROTOCOL_ERROR); //rfc7540,l,3087,3090
+	}
+
 	AN(req->http->hd[HTTP_HDR_PROTO].b);
 
 	req->req_step = R_STP_TRANSPORT;
diff --git a/bin/varnishtest/tests/t02025.vtc b/bin/varnishtest/tests/t02025.vtc
new file mode 100644
index 000000000..b464f8cdc
--- /dev/null
+++ b/bin/varnishtest/tests/t02025.vtc
@@ -0,0 +1,48 @@
+varnishtest "Dublicate pseudo-headers"
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -arg "-p feature=+http2" -vcl+backend {
+} -start
+
+#client c1 {
+#	txreq -url "/some/path" -url "/some/other/path"
+#	rxresp
+#	expect resp.status == 400
+#} -run
+
+#client c1 {
+#	txreq -req "GET" -req "POST"
+#	rxresp
+#	expect resp.status == 400
+#} -run
+
+#client c1 {
+#	txreq -proto "HTTP/1.1" -proto "HTTP/2.0"
+#	rxresp
+#	expect resp.status == 400
+#} -run
+
+client c1 {
+	stream 1 {
+		txreq -url "/some/path" -url "/some/other/path"
+		rxrst
+	} -run
+} -run
+
+client c1 {
+	stream 1 {
+		txreq -scheme "http" -scheme "https"
+		rxrst
+	} -run
+} -run
+
+client c1 {
+	stream 1 {
+		txreq -req "GET" -req "POST"
+		rxrst
+	} -run
+} -run


More information about the varnish-commit mailing list