1berry/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

BUG/MEDIUM: h2/h3: reject some forbidden chars in :authority before reassembly

Browse Source 
As discussed here:
   https://github.com/httpwg/http2-spec/pull/936
   https://github.com/haproxy/haproxy/issues/2941

It's important to take care of some special characters in the :authority
pseudo header before reassembling a complete URI, because after assembly
it's too late (e.g. the '/'). This patch does this, both for h2 and h3.

The impact on H2 was measured in the worst case at 0.3% of the request
rate, while the impact on H3 is around 1%, but H3 was about 1% faster
than H2 before and is now on par.

It may be backported after a period of observation, and in this case it
relies on this previous commit:

   MINOR: http: add a function to validate characters of :authority

Thanks to @DemiMarie for reviving this topic in issue #2941 and bringing
new potential interesting cases.
This commit is contained in:
Willy Tarreau 2025-05-12 17:45:33 +02:00
parent ebab479cdf
commit 9a05c1f574
2 changed files with 26 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/h2.c
View File

@ -203,6 +203,17 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr,
}
}
/* We're going to concatenate :authority with :path to form a URI. Some
* characters must absolutely be avoided in :authority to make sure not
* to result in a broken concatenation. See the following links for a
* discussion on this topic:
* https://github.com/httpwg/http2-spec/pull/936
* https://github.com/haproxy/haproxy/issues/2941
*/
if ((fields & H2_PHDR_FND_AUTH) &&
http_authority_has_forbidden_char(phdr[H2_PHDR_IDX_AUTH]))
goto fail;
if (!(flags & HTX_SL_F_HAS_SCHM)) {
/* no scheme, use authority only (CONNECT) */
uri = phdr[H2_PHDR_IDX_AUTH];

15
src/h3.c
View File

@ -753,6 +753,21 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
goto out;
}
/* We're going to concatenate :authority with :path to form a URI. Some
* characters must absolutely be avoided in :authority to make sure not
* to result in a broken concatenation. See the following links for a
* discussion on this topic:
* https://github.com/httpwg/http2-spec/pull/936
* https://github.com/haproxy/haproxy/issues/2941
*/
if (http_authority_has_forbidden_char(authority)) {
TRACE_ERROR("invalid character in authority", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
h3s->err = H3_ERR_MESSAGE_ERROR;
qcc_report_glitch(h3c->qcc, 1);
len = -1;
goto out;
}
if (!istlen(scheme)) {
/* No scheme (CONNECT), use :authority only. */
uri = authority;