Skip to content

fix(proxy): bind the MITM'd inner request to the CONNECT authority - #248

Open
surki wants to merge 1 commit into
paradigmxyz:mainfrom
surki:fix/bind-inner-host-to-connect-authority
Open

surki wants to merge 1 commit into
paradigmxyz:mainfrom
surki:fix/bind-inner-host-to-connect-authority

Conversation

@surki

@surki surki commented Sep 10, 2026

Copy link
Copy Markdown

What

In MITM mode, refuse an inner request whose Host does not name the same origin as the tunnel's CONNECT target. Comparison is by host (case-insensitive) and effective port, so Host: example.com matches CONNECT example.com:443, and [2001:db8::1] matches [2001:db8::1]:443.

Why

The CONNECT stage authorizes a destination, but handleHTTP only checks SNI == Host and then dials r.Host. So a client can:

  1. CONNECT allowed.example.com:443 — passes the allowlist / external policy at the CONNECT stage;
  2. handshake with SNI blocked.example.com, send Host: blocked.example.com.

SNI and Host agree with each other, so the existing check passes and the proxy mints a leaf for, and dials, blocked.example.com — a host the CONNECT stage never saw. When allowed and blocked hosts share a front end that routes on Host (CDN, virtual hosting) this is a clean bypass of any CONNECT-stage policy; RFC 9110 §7.4 names this case. Together with the SNI check, the new check gives CONNECT target == SNI == Host, so the proxy only ever MITMs and dials the host the CONNECT stage authorized.

Behavior notes

  • Only tunnelled requests are affected (tunnelInfo != nil); direct HTTP proxying is unchanged.
  • Repeated requests to the same authority over one tunnel keep working (HTTP/1.1 keep-alive, HTTP/2 multiplexing). Reusing a tunnel for a different origin (HTTP/2 connection coalescing, RFC 9113 §9.1.1) is refused; supporting that would need per-request authorization of the new authority, which is out of scope here.
  • A client that CONNECTs to an IP literal and then sends a hostname in Host is refused — that is the virtual-host case above. SOCKS5 clients should resolve remotely (socks5h://, --socks5-hostname), which the README already shows.
  • Rejection is 400 with a one-line body, matching the adjacent SNI/Host mismatch path. Happy to switch to 421 Misdirected Request if you'd prefer that semantics.

Testing

  • TestSameAuthority — table over default/explicit/non-default ports, case, and IPv6 literals.
  • TestTunnel_CONNECT_RejectsHostMismatch — end-to-end through the tunnel listener: CONNECT to allowed.example.com:443, TLS with SNI/Host other.example.com → 400, with a transport that fails the test if any upstream dial is attempted. The matching case (CONNECT host:443, inner Host without port) is already exercised by TestTunnel_CONNECT_HTTPS_MITM.
  • go test -race ./internal/proxy/, go vet, golangci-lint run clean.

The CONNECT stage authorizes a destination — the CONNECT target — but the
MITM path only checks that the inner request's SNI matches its Host, and
then dials that Host. A client can CONNECT to an allowed authority, complete
the tunnel, and send an inner request whose SNI and Host both name a
different one. The SNI==Host check passes, and the proxy MITMs and dials a
host the CONNECT stage never saw. Where allowed and blocked hosts sit behind
the same front end that routes on Host (shared CDN, virtual hosting) this
bypasses a CONNECT-stage allowlist; RFC 9110 §7.4 calls out exactly this.

Require the inner Host to name the same origin as the CONNECT target,
compared by host (case-insensitive) and effective port, so "example.com"
over https equals "example.com:443" and a bracketed IPv6 literal without a
port equals its with-port form. Combined with the existing SNI check this
gives CONNECT target == SNI == Host. Repeated requests to the same authority
over one tunnel (HTTP/1.1 keep-alive, HTTP/2 multiplexing) are unaffected;
reusing a tunnel for a different origin (HTTP/2 coalescing, RFC 9113
§9.1.1) is refused with 400, like the SNI/Host mismatch next to it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant