The raw Host header is forwarded without authority validation, where nginx returns 400 #63

Closed
opened 2026-10-26 15:38:57 +00:00 by james.oates · 3 comments
Owner

From the v2.0.0 security audit.

authority_host splits on the first :, so Host: good.com:[email protected] routes as good.com. original_host then falls back to the raw header value and it is forwarded as Host (default rewrite_host = false) and as X-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 what http's Authority, 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, a Location header — 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-Alive and Proxy-Authorization are 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.

From the v2.0.0 security audit. `authority_host` splits on the first `:`, so `Host: good.com:[email protected]` routes as `good.com`. `original_host` then falls back to the raw header value and it is forwarded as `Host` (default `rewrite_host = false`) and as `X-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 what `http`'s `Authority`, 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, a `Location` header — 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-Alive` and `Proxy-Authorization` are 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.
james.oates added this to the v2.1 milestone 2026-10-26 17:53:26 +00:00
Author
Owner

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_host everywhere 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:

  • A concrete case where the unvalidated value reaches a backend that then makes a security decision on it (a backend doing virtual-host dispatch or generating absolute URLs from Host), rather than merely receiving something odd.
  • Request smuggling: if a specific malformed authority can be made to mean different things to Edge Router and to a backend, that is a real finding and this becomes urgent rather than tidy.

Neither is demonstrated in the issue as written. Worth doing when Edge Router fronts something that trusts Host, or if someone shows a divergence.

**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_host` everywhere 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: - A concrete case where the unvalidated value reaches a backend that then makes a **security** decision on it (a backend doing virtual-host dispatch or generating absolute URLs from `Host`), rather than merely receiving something odd. - Request smuggling: if a specific malformed authority can be made to mean different things to Edge Router and to a backend, that is a real finding and this becomes urgent rather than tidy. Neither is demonstrated in the issue as written. Worth doing when Edge Router fronts something that trusts `Host`, or if someone shows a divergence.
james.oates removed this from the v2.1 milestone 2026-10-27 10:28:41 +00:00
Author
Owner

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 :, so Host: good.com:[email protected] yields good.com and matches the real service.
  • original_host (proxy.rs:856) keeps the raw authority, not the parsed host.
  • With the default rewrite_host = false, that raw value is set verbatim as the outbound Host (proxy.rs:891) and again as x-forwarded-host (proxy.rs:923).

So Edge Router routes on good.com and hands the backend good.com:[email protected], which http's Authority, Go and Python all resolve as evil.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: host is 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 a Host header has no legitimate use.

I will also take the hop-by-hop header stripping noted at the end, since it is the same function.

**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** `:`, so `Host: good.com:[email protected]` yields `good.com` and matches the real service. - `original_host` (`proxy.rs:856`) keeps the **raw** authority, not the parsed host. - With the default `rewrite_host = false`, that raw value is set verbatim as the outbound `Host` (`proxy.rs:891`) and again as `x-forwarded-host` (`proxy.rs:923`). So Edge Router routes on `good.com` and hands the backend `good.com:[email protected]`, which `http`'s `Authority`, Go and Python all resolve as `evil.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: `host` is 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 a `Host` header has no legitimate use. I will also take the hop-by-hop header stripping noted at the end, since it is the same function.
james.oates added this to the v2.1 milestone 2026-10-27 10:29:44 +00:00
Author
Owner

Fixed in f059ac8, both halves.

proxy::valid_authority accepts a bare host[:port] and refuses anything two parsers could read differently; proxy_handler checks it before routing on the value or forwarding it, and returns 400. Rejecting rather than normalising, as you proposed — userinfo in a Host header 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_host feeds both the backend's Host and x-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 own HOP_BY_HOP_HEADERS list, not added to CLIENT_CONTROLLED_TRUST_HEADERS: the WebSocket path shares that list, and an upgrade needs connection and upgrade to 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:

  • The validator matrix, covering every form above.
  • An app-level request with Host: good.test:[email protected] → 400. I disabled the guard and confirmed it returns 404 instead, 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 warnings clean, fmt clean.

Fixed in `f059ac8`, both halves. `proxy::valid_authority` accepts a bare `host[:port]` and refuses anything two parsers could read differently; `proxy_handler` checks it before routing on the value or forwarding it, and returns `400`. Rejecting rather than normalising, as you proposed — userinfo in a `Host` header 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_host` feeds both the backend's `Host` and `x-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 own `HOP_BY_HOP_HEADERS` list, **not** added to `CLIENT_CONTROLLED_TRUST_HEADERS`: the WebSocket path shares that list, and an upgrade needs `connection` and `upgrade` to 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: - The validator matrix, covering every form above. - An app-level request with `Host: good.test:[email protected]` → `400`. I disabled the guard and confirmed it returns `404` instead, 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 warnings` clean, fmt clean.
Sign in to join this conversation.
No labels
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
IsoHex/edge-router#63
No description provided.