Mailing List Archive

commit c0b452b301a69f85ad5494eb2de99f5fc0b06e7c
Author: Dridi Boukelmoune <dridi.boukelmoune@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 126a70d3a..5962ddb51 100644
--- a/bin/varnishd/http2/cache_http2.h
+++ b/bin/varnishd/http2/cache_http2.h
@@ -227,6 +227,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 48e6e5c5c..a134c379a 100644
--- a/bin/varnishd/http2/cache_http2_hpack.c
+++ b/bin/varnishd/http2/cache_http2_hpack.c
@@ -145,7 +145,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);
@@ -162,6 +162,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);
@@ -205,14 +206,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;

@@ -228,6 +223,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,
@@ -244,14 +241,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,
@@ -260,6 +250,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 09d12e541..936f522b9 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
@@ -43,6 +43,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
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit