Make the forwarded client-IP header an explicit per-listener choice #73

New issue

Open

opened 2026-10-27 14:53:23 +00:00 by james.oates · 0 comments

james.oates commented

2026-10-27 14:53:23 +00:00

Owner

Part 1 of #64, split out because it needs a design decision rather than a patch. Not started, no milestone.

access::effective_client_ip honours CF-Connecting-IP and falls back to X-Real-IP on a trust_forwarded_for listener. That priority order is only sound if the trusted intermediary overwrites or strips whichever header it does not set. Cloudflare and cloudflared set CF-Connecting-IP, which wins, so the documented deployment is correct — but the README presents nginx, HAProxy and NLB front-ends as supported, and nginx does not set X-Real-IP unless configured to. An operator with HAProxy in front, a trusting listener and allow_ips = ["100.64.0.0/10"] is then bypassed by a client sending X-Real-IP: 100.64.0.1.

#64's own conclusion is the right one and worth preserving: "which header carries the client IP" is a property of the intermediary, and guessing a priority order is what created this. So the fix is an explicit per-listener declaration rather than a documentation note — something like client_ip_header = "cf-connecting-ip" on the listener, with no fallback, and every other candidate name dropped from the request as CLIENT_CONTROLLED_TRUST_HEADERS already does.

Decisions it needs:

Is it required on a trusting listener, or does it default? Requiring it is a breaking change for every existing trust_forwarded_for = true listener, which is the shipped example config among others. Defaulting to today's order keeps the hazard for anyone who does not set it.
What about X-Forwarded-For? It is handled separately and chained, and its leftmost-vs-rightmost question is the same class of ambiguity.

Nothing in the fleet is exposed: no deployment puts nginx or HAProxy in front of Edge Router with a trusting listener. Worth doing before one does.

Also from #64, not done and why

A startup warning for trust_forwarded_for = true with no match_local_ip was the other suggestion. Not built, because it would fire on the recommended configuration: edge-router.example.toml's cloudflare listener is exactly that shape, and its own comment explains why it is sound — the parent binding is 0.0.0.0:8443 inside Docker and never published to the host. Edge Router cannot distinguish that from 0.0.0.0 on a host NIC, so the warning would cry wolf on the shipped example and be tuned out. If this issue lands, the explicit header declaration makes the dangerous shape detectable properly: a trusting listener that has not declared which header it trusts.

Part 1 of #64, split out because it needs a design decision rather than a patch. **Not started, no milestone.** `access::effective_client_ip` honours `CF-Connecting-IP` and falls back to `X-Real-IP` on a `trust_forwarded_for` listener. That priority order is only sound if the trusted intermediary overwrites or strips whichever header it does not set. Cloudflare and cloudflared set `CF-Connecting-IP`, which wins, so the documented deployment is correct — but the README presents nginx, HAProxy and NLB front-ends as supported, and nginx does not set `X-Real-IP` unless configured to. An operator with HAProxy in front, a trusting listener and `allow_ips = ["100.64.0.0/10"]` is then bypassed by a client sending `X-Real-IP: 100.64.0.1`. #64's own conclusion is the right one and worth preserving: **"which header carries the client IP" is a property of the intermediary, and guessing a priority order is what created this.** So the fix is an explicit per-listener declaration rather than a documentation note — something like `client_ip_header = "cf-connecting-ip"` on the listener, with no fallback, and every other candidate name dropped from the request as `CLIENT_CONTROLLED_TRUST_HEADERS` already does. Decisions it needs: - **Is it required on a trusting listener, or does it default?** Requiring it is a breaking change for every existing `trust_forwarded_for = true` listener, which is the shipped example config among others. Defaulting to today's order keeps the hazard for anyone who does not set it. - **What about `X-Forwarded-For`?** It is handled separately and chained, and its leftmost-vs-rightmost question is the same class of ambiguity. Nothing in the fleet is exposed: no deployment puts nginx or HAProxy in front of Edge Router with a trusting listener. Worth doing before one does. ## Also from #64, not done and why A **startup warning for `trust_forwarded_for = true` with no `match_local_ip`** was the other suggestion. Not built, because it would fire on the recommended configuration: `edge-router.example.toml`'s `cloudflare` listener is exactly that shape, and its own comment explains why it is sound — the parent binding is `0.0.0.0:8443` inside Docker and never published to the host. Edge Router cannot distinguish that from `0.0.0.0` on a host NIC, so the warning would cry wolf on the shipped example and be tuned out. If this issue lands, the explicit header declaration makes the dangerous shape detectable properly: a trusting listener that has not declared which header it trusts.

james.oates referenced this issue from a commit

2026-10-27 15:59:22 +00:00

Parse the admin Host with authority_host, not split(':') (#64)

james.oates referenced this issue

2026-10-27 15:59:38 +00:00

Forwarded-trust hardening: X-Real-IP fallback, and no warning for a trusting listener with no match_local_ip #64