From 2a98ad3866533caff45d6f7d472e01f3a5b2c8c6 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Tue, 10 Aug 2021 15:37:34 +0200 Subject: BUG/MAJOR: h2: verify early that non-http/https schemes match the valid syntax MIME-Version: 1.0 Content-Type: text/plain; charset=latin1 Content-Transfer-Encoding: 8bit While we do explicitly check for strict character sets in the scheme, this is only done when extracting URL components from an assembled one, and we have special handling for "http" and "https" schemes directly in the H2-to-HTX conversion. Sadly, this lets all other ones pass through if they start exactly with "http://" or "https://", allowing the reconstructed URI to start with a different looking authority if it was part of the scheme. It's interesting to note that in this case the valid authority is in the Host hedaer and that the request will only be wrong if emitted over H2 on the backend side, since H1 will not emit an absolute URI by default and will drop the scheme. So in essence, this is a variant of the scheme-based attack described below in that it only affects H2-H2 and not H2-H1 forwarding: https://portswigger.net/research/http2 As such, a simple workaround consists in just adding the following rule in the frontend or backend, which will have for effect to renormalize the authority in the request line according to the concatenated version: http-request set-uri %[url] This patch simply adds the missing syntax checks for non-http/https schemes before the concatenation in the H2 code. An improvement may consist in the future in splitting these ones apart in the start line so that only the "url" sample fetch function requires to access them together and that all other places continue to access them separately. This will then allow the core code to perform such checks itself. The patch needs to be backported as far as 2.2. Before 2.2 the full URI was not being reconstructed so the scheme and authority part were always dropped from H2 requests to leave only origin requests. Note for backporters: this depends on this previous patch: MINOR: http: add a new function http_validate_scheme() to validate a scheme Many thanks to Tim Düsterhus for figuring that one and providing a reproducer. (cherry picked from commit d2b179db54846aee11356f033dfc490978147593) Signed-off-by: Willy Tarreau (cherry picked from commit 2ac4f553cc1ea7a8a4ff28db18fa01f04b9d84ce) [wt: no rfc8441 in 2.3] Signed-off-by: Willy Tarreau (cherry picked from commit e618d9bf5f6b48bb45aceb8e7a886c43d62b3ed5) Signed-off-by: Willy Tarreau --- src/h2.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/h2.c b/src/h2.c index 668461086..bf44611a2 100644 --- a/src/h2.c +++ b/src/h2.c @@ -230,6 +230,8 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr, flags |= HTX_SL_F_SCHM_HTTP; else if (isteqi(phdr[H2_PHDR_IDX_SCHM], ist("https"))) flags |= HTX_SL_F_SCHM_HTTPS; + else if (!http_validate_scheme(phdr[H2_PHDR_IDX_SCHM])) + htx->flags |= HTX_FL_PARSING_ERROR; } if (!(flags & HTX_SL_F_HAS_SCHM)) { -- 2.28.0