Commit c0b452b3 authored by Dridi Boukelmoune's avatar Dridi Boukelmoune

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.
parent b7bc7f56
...@@ -227,6 +227,7 @@ struct h2h_decode { ...@@ -227,6 +227,7 @@ struct h2h_decode {
unsigned magic; unsigned magic;
#define H2H_DECODE_MAGIC 0xd092bde4 #define H2H_DECODE_MAGIC 0xd092bde4
unsigned has_authority:1;
unsigned has_scheme:1; unsigned has_scheme:1;
h2_error error; h2_error error;
enum vhd_ret_e vhd_ret; enum vhd_ret_e vhd_ret;
......
...@@ -145,7 +145,7 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d) ...@@ -145,7 +145,7 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
txt hdr, nm, val; txt hdr, nm, val;
int disallow_empty; int disallow_empty;
const char *p; const char *p;
unsigned n; unsigned n, has_dup;
CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC); CHECK_OBJ_NOTNULL(hp, HTTP_MAGIC);
h2h_assert_ready(d); h2h_assert_ready(d);
...@@ -162,6 +162,7 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d) ...@@ -162,6 +162,7 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
val.e = hdr.e; val.e = hdr.e;
disallow_empty = 0; disallow_empty = 0;
has_dup = 0;
if (Tlen(hdr) > UINT_MAX) { /* XXX: cache_param max header size */ if (Tlen(hdr) > UINT_MAX) { /* XXX: cache_param max header size */
VSLb(hp->vsl, SLT_BogoHeader, "Header too large: %.20s", hdr.b); VSLb(hp->vsl, SLT_BogoHeader, "Header too large: %.20s", hdr.b);
...@@ -205,14 +206,8 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d) ...@@ -205,14 +206,8 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
/* XXX: What to do about this one? (typically /* XXX: What to do about this one? (typically
"http" or "https"). For now set it as a normal "http" or "https"). For now set it as a normal
header, stripping the first ':'. */ 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++; hdr.b++;
has_dup = d->has_scheme;
d->has_scheme = 1; d->has_scheme = 1;
disallow_empty = 1; disallow_empty = 1;
...@@ -228,6 +223,8 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d) ...@@ -228,6 +223,8 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
memcpy(d->out + 6, "host", 4); memcpy(d->out + 6, "host", 4);
hdr.b += 6; hdr.b += 6;
nm = Tstr(":authority"); /* preserve original */ nm = Tstr(":authority"); /* preserve original */
has_dup = d->has_authority;
d->has_authority = 1;
} else { } else {
/* Unknown pseudo-header */ /* Unknown pseudo-header */
VSLb(hp->vsl, SLT_BogoHeader, VSLb(hp->vsl, SLT_BogoHeader,
...@@ -244,14 +241,7 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d) ...@@ -244,14 +241,7 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
return (H2SE_PROTOCOL_ERROR); return (H2SE_PROTOCOL_ERROR);
} }
if (n < HTTP_HDR_FIRST) { 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 {
/* Check for space in struct http */ /* Check for space in struct http */
if (n >= hp->shd) { if (n >= hp->shd) {
VSLb(hp->vsl, SLT_LostHeader, VSLb(hp->vsl, SLT_LostHeader,
...@@ -260,6 +250,15 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d) ...@@ -260,6 +250,15 @@ h2h_addhdr(struct http *hp, struct h2h_decode *d)
return (H2SE_ENHANCE_YOUR_CALM); return (H2SE_ENHANCE_YOUR_CALM);
} }
hp->nhd++; 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; hp->hd[n] = hdr;
......
varnishtest "#2351: :path/:method error handling" varnishtest "#2351: h2 pseudo-headers error handling"
server s1 { server s1 {
rxreq rxreq
...@@ -43,6 +43,16 @@ client c1 { ...@@ -43,6 +43,16 @@ client c1 {
} -run } -run
} -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.req0.live == 0
varnish v1 -expect MEMPOOL.req1.live == 0 varnish v1 -expect MEMPOOL.req1.live == 0
varnish v1 -expect MEMPOOL.sess0.live == 0 varnish v1 -expect MEMPOOL.sess0.live == 0
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment