Mailing List Archive

commit 1ab71cf18b448478ebe16bd7a7fa78fe6d5e0dae
Author: Asad Sajjad Ahmed <asadsa@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@varnish-software.com>

Conflicts:
bin/varnishtest/tests/t02025.vtc

diff --git a/bin/varnishd/http2/cache_http2.h b/bin/varnishd/http2/cache_http2.h
index ca2e65993..61d0a1d78 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -218,11 +218,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 b5451eb22..472b86294 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -127,7 +127,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;
@@ -188,9 +188,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, u = 0; u < len-namelen;
p++, u++) {
@@ -380,7 +389,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 edcc709d9..a474020b2 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -601,11 +601,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 (h2e != NULL) {
@@ -656,10 +658,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);

if (*req->http->hd[HTTP_HDR_URL].b == '*' &&
diff --git a/bin/varnishtest/tests/t02026.vtc b/bin/varnishtest/tests/t02026.vtc
new file mode 100644
index 000000000..b464f8cdc
--- /dev/null
+++ b/bin/varnishtest/tests/t02026.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
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit