The raw Host header is forwarded without authority validation, where nginx returns 400 #63
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
From the v2.0.0 security audit.
authority_hostsplits on the first:, soHost: good.com:[email protected]routes asgood.com.original_hostthen falls back to the raw header value and it is forwarded asHost(defaultrewrite_host = false) and asX-Forwarded-Host.hyper's HTTP/1 server does no Host validation — confirmed by reading
hyper-1.8.1/src/proto/h1/role.rs— so any header-legal bytes get through. A backend parsing that value as an authority resolves the host as everything after the last@, which is whathttp'sAuthority, Go, and Python all do. So ER routes on the part before the colon while the backend reads the part after the@: one request, two different hosts.Consequence
The standard Host-header-injection class. A backend building an absolute URL from
Host— a password-reset link, an OAuth redirect, aLocationheader — emits the attacker's domain. Mechanism is confirmed; the impact depends on the backend, so it is PLAUSIBLE rather than demonstrated end to end.The framing that matters for this project: nginx, which ER is replacing fleet-wide, returns 400 for this. ER is looser than the thing it replaces, at precisely the boundary it exists to police.
Fix
Validate the Host header as an authority before routing on it, and reject rather than normalise — a request whose Host does not parse as a bare
host[:port]should be a 400. Userinfo in a Host header has no legitimate use.Related hygiene on the same line, no exploit path found:
Connection,Upgrade,TE,Keep-AliveandProxy-Authorizationare hop-by-hop and currently forwarded. h2c smuggling was chased specifically and does not work — ER never hands the client socket over on the HTTP path, so a 101 from a backend is returned as an ordinary response and no tunnel forms. Worth stripping them anyway.Taken out of v2.1, left open. Agreed with the Axianthe PM after I raised it as the one issue in the set I'd question spending on.
The reasoning, so it isn't re-litigated from scratch: this hardens against something that has not happened, and "nginx returns 400" is a description of another proxy's choice rather than an argument on its own. Edge Router already parses the Host with
proxy::authority_hosteverywhere it makes a decision, and the routing match is exact against configured hosts — an unroutable or malformed authority does not match a service and falls through, so the failure mode today is "no route", not "wrong route".What would move this up:
Host), rather than merely receiving something odd.Neither is demonstrated in the issue as written. Worth doing when Edge Router fronts something that trusts
Host, or if someone shows a divergence.Retracting my previous comment — it was wrong, and I wrote it without reading this issue's body. Back in v2.1.
I claimed no divergence between Edge Router's parse and a backend's was demonstrated. This issue demonstrates exactly that, and I have now verified every step in our own code:
authority_host(proxy.rs:95) splits on the first:, soHost: good.com:[email protected]yieldsgood.comand matches the real service.original_host(proxy.rs:856) keeps the raw authority, not the parsed host.rewrite_host = false, that raw value is set verbatim as the outboundHost(proxy.rs:891) and again asx-forwarded-host(proxy.rs:923).So Edge Router routes on
good.comand hands the backendgood.com:[email protected], whichhttp'sAuthority, Go and Python all resolve asevil.com. One request, two hosts, and the attacker's half is the one the backend builds password-reset links and OAuth redirects from. The drop list does not help here:hostis dropped from the client's headers precisely so Edge Router can re-set it authoritatively, and the value it re-sets is the unvalidated one.The point I dismissed as framing is also the substantive one: nginx returns 400 for this, and Edge Router is replacing nginx fleet-wide. Being looser than the thing we are replacing, at the boundary that exists to police exactly this, is not a tidiness concern.
My error was reading the title and reasoning from memory of the Host-parsing code instead of reading what was written here — the same mistake that produced a wrong claim about the admin gate and about devinfra's redirect this week. The fix this issue proposes stands: validate as a bare
host[:port]and reject with 400 rather than normalise, since userinfo in aHostheader has no legitimate use.I will also take the hop-by-hop header stripping noted at the end, since it is the same function.
Fixed in
f059ac8, both halves.proxy::valid_authorityaccepts a barehost[:port]and refuses anything two parsers could read differently;proxy_handlerchecks it before routing on the value or forwarding it, and returns400. Rejecting rather than normalising, as you proposed — userinfo in aHostheader has nothing worth salvaging.Still accepted, so this doesn't over-reach: rooted FQDNs (
example.com.), IPv6 literals with and without a port, and underscores in hostnames. Refused:@anywhere, a second:, a non-numeric or empty port, an empty host,/,?,#, backslash, whitespace and control bytes, an unterminated bracket, and trailing junk after].The guard is checked before the value is used rather than at the point of forwarding, because
original_hostfeeds both the backend'sHostandx-forwarded-host— guarding one site would have left the other.Hop-by-hop headers are no longer forwarded on the HTTP path (
Connection,Upgrade,TE,Keep-Alive,Proxy-Authorization). They are in their ownHOP_BY_HOP_HEADERSlist, not added toCLIENT_CONTROLLED_TRUST_HEADERS: the WebSocket path shares that list, and an upgrade needsconnectionandupgradeto survive. Merging them would have broken every upgrade silently, so a test asserts the lists are disjoint and that the WS path still forwards both.Two tests, both verified to discriminate:
Host: good.test:[email protected]→400. I disabled the guard and confirmed it returns404instead, so the test proves the guard is wired into the real router rather than merely defined.Release-notes line added, including that this is breaking for a client sending a genuinely malformed
Host.On my earlier comment: I argued for dropping this from v2.1 having read only the title, then reasoned from my recollection of the Host-parsing code. The body demonstrated the divergence I claimed was absent. Thank you for writing it out as explicitly as you did — it is what let me see the error when I opened the issue to de-milestone it.
380 tests pass, clippy
--all-targets -D warningsclean, fmt clean.