BUG/MAJOR: h2: verify that :path starts with a '/' before concatenating it

Tim Dsterhus found that while the H2 path is checked for non-emptiness,
invalid chars and '*', a test is missing to verify that except for '*',
it always starts with exactly one '/'. During the reconstruction of the
full URI when passing to HTX, this missing test allows to affect the
apparent authority by appending a port number or a suffix name.

This only affects H2-to-H2 communications, as H2-to-H1 do not use the
full URI. Like for previous fix, the following rule inserted before
other ones in the frontend is sufficient to renormalize the internal
URI and let haproxy see the same authority as the target server:

    http-request set-uri %[url]

This needs to be backported to 2.2. Earlier versions do not rebuild a
full URI using the authority and will fail on the malformed path at the
HTTP layer, so they are safe.
This commit is contained in:
Willy Tarreau 2021-08-10 16:30:55 +02:00
parent a495e0d948
commit 4b8852c70d

View File

@ -275,6 +275,22 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr,
meth_sl = phdr[H2_PHDR_IDX_METH]; meth_sl = phdr[H2_PHDR_IDX_METH];
} }
if (fields & H2_PHDR_FND_PATH) {
/* 7540#8.1.2.3: :path must not be empty, and must be either
* '*' or an RFC3986 "path-absolute" starting with a "/" but
* not with "//".
*/
if (unlikely(!phdr[H2_PHDR_IDX_PATH].len))
goto fail;
else if (unlikely(phdr[H2_PHDR_IDX_PATH].ptr[0] != '/')) {
if (!isteq(phdr[H2_PHDR_IDX_PATH], ist("*")))
goto fail;
}
else if (phdr[H2_PHDR_IDX_PATH].len > 1 &&
phdr[H2_PHDR_IDX_PATH].ptr[1] == '/')
goto fail;
}
if (!(flags & HTX_SL_F_HAS_SCHM)) { if (!(flags & HTX_SL_F_HAS_SCHM)) {
/* no scheme, use authority only (CONNECT) */ /* no scheme, use authority only (CONNECT) */
uri = phdr[H2_PHDR_IDX_AUTH]; uri = phdr[H2_PHDR_IDX_AUTH];
@ -285,9 +301,6 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr,
* use the trash to concatenate them since all of them MUST fit * use the trash to concatenate them since all of them MUST fit
* in a bufsize since it's where they come from. * in a bufsize since it's where they come from.
*/ */
if (unlikely(!phdr[H2_PHDR_IDX_PATH].len))
goto fail; // 7540#8.1.2.3: :path must not be empty
uri = ist2bin(trash.area, phdr[H2_PHDR_IDX_SCHM]); uri = ist2bin(trash.area, phdr[H2_PHDR_IDX_SCHM]);
istcat(&uri, ist("://"), trash.size); istcat(&uri, ist("://"), trash.size);
istcat(&uri, phdr[H2_PHDR_IDX_AUTH], trash.size); istcat(&uri, phdr[H2_PHDR_IDX_AUTH], trash.size);