From: Christopher Faulet <cfaulet@haproxy.com>
Date: Wed, 7 Oct 2026 09:23:02 +0000 (+0200)
Subject: BUG/MEDIUM: h1: reject CONNECT request targets other than "host:port"
X-Git-Tag: v3.5-dev10~149
X-Git-Url: https://git.kaiwu.me/?a=commitdiff_plain;h=87112fdea2050e41902d8d1b2f9608e12f7c3a21;p=haproxy.git

BUG/MEDIUM: h1: reject CONNECT request targets other than "host:port"

For CONNECT requests, the H1 parser validates the authority extracted by
http_parse_authority() (with userinfo removed and stopping at the first '/',
'?' or '#') against the Host header and checks it for forbidden
characters. But the request target itself is kept and forwarded as-is.  So
the following targets were accepted with "Host: allowed.com:443":

    CONNECT internal:22@allowed.com:443 HTTP/1.1
    CONNECT allowed.com:443/x HTTP/1.1
    CONNECT allowed.com:443?x HTTP/1.1

and forwarded verbatim to the server (or as the :authority of an H2
request). RFC9112#3.2.3 says the authority-form only contains
"uri-host:port". The next hop (typically a forward proxy) may parse such
invalid targets differently than haproxy (e.g. "internal" port 22), and
connect to a destination that was not checked by the host-based rules.

Now, a CONNECT request is rejected (or the error is captured when unsafe
violations are accepted, like for other authority errors) if the
validated authority does not span the whole request target.

A reg-test was added to verify this.

This was introduced by commit 3f5fbe940 ("BUG/MEDIUM: h1: Improve
authority validation for CONNCET request") in 2.7 which validated the
stripped authority. It should be backported as far as 2.8.
---

diff --git a/reg-tests/http-messaging/h1_connect_target_validation.vtc b/reg-tests/http-messaging/h1_connect_target_validation.vtc
new file mode 100644
index 000000000..073684333
--- /dev/null
+++ b/reg-tests/http-messaging/h1_connect_target_validation.vtc
@@ -0,0 +1,50 @@
+varnishtest "H1 CONNECT request target validation"
+
+# RFC9112#3.2.3: the request target of a CONNECT request is in authority-form
+# ("uri-host:port"). Since it is forwarded as-is, it must not contain any
+# userinfo, path or query that would make haproxy and the next hop disagree
+# on the destination.
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    listen li1
+        bind "fd@${li1}"
+        http-request return status 200
+} -start
+
+client c1 -connect ${h1_li1_sock} {
+	txreq -req "CONNECT" -url "allowed.com:443" -hdr "Host: allowed.com:443"
+	rxresp
+	expect resp.status == 200
+} -run
+
+client c2 -connect ${h1_li1_sock} {
+	txreq -req "CONNECT" -url "internal:22@allowed.com:443" -hdr "Host: allowed.com:443"
+	rxresp
+	expect resp.status == 400
+} -run
+
+client c3 -connect ${h1_li1_sock} {
+	txreq -req "CONNECT" -url "allowed.com:443/x" -hdr "Host: allowed.com:443"
+	rxresp
+	expect resp.status == 400
+} -run
+
+client c4 -connect ${h1_li1_sock} {
+	txreq -req "CONNECT" -url "allowed.com:443?x" -hdr "Host: allowed.com:443"
+	rxresp
+	expect resp.status == 400
+} -run
+
+client c5 -connect ${h1_li1_sock} {
+	txreq -req "CONNECT" -url "internal:22@allowed.com:443"
+	rxresp
+	expect resp.status == 400
+} -run
diff --git a/src/h1.c b/src/h1.c
index 814a5ea4d..2a91407ab 100644
--- a/src/h1.c
+++ b/src/h1.c
@@ -1166,6 +1166,17 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
 					if (host_idx != -1)
 						host = &hdr[host_idx].v;
 					ret = h1_validate_connect_authority(scheme, authority, host);
+
+					/* RFC9112#3.2.3: the authority-form only contains
+					 * "uri-host:port". The authority was validated
+					 * without any userinfo, path or query, which must
+					 * then not be present since the request target is
+					 * forwarded as-is.
+					 */
+					if (!ret && (istptr(authority) != istptr(sl.rq.u) ||
+						     istlen(authority) != istlen(sl.rq.u)))
+						ret = -1;
+
 					if (ret < 0) {
 						if (h1m->err_pos < -1) {
 							state = H1_MSG_LAST_LF;