fix(audit): trusted proxy hops is 2, measured rather than assumed #21

Merged
jlxq0 merged 1 commit from fix/trusted-proxy-hops-2 into main 2026-08-27 03:56:20 +00:00
Owner

The deployed v0.2.15 writes the edge gateway's address into the last_used provenance record as the client's, on every authenticated request. Refs #14.

DEFAULT_TRUSTED_PROXY_HOPS was 1, commented "(Traefik)". There is no Traefik on this cluster.

Two independent measurements, both 2

source result
observed pod logs, 7 authenticated requests at 2026-08-27T03:37:47Z xff_entries=2
derived oddie-apps/edge-config + Cilium config 2 appended entries

client -> Caddy edge -> Cilium gateway -> pod. The edge sets no trusted_proxies (0 occurrences in 80 site blocks), so it replaces XFF with its peer, giving entry 1, the client. Cilium runs gateway-api-xff-num-trusted-hops: 0, so Envoy appends the downstream address, giving entry 2, the edge.

One is read at the pod, the other from the edge repository. They share no instrument.

The route is singular as well: 203.24.209.5 times out on tcp/443 across three attempts while 203.24.209.8 answers 200 on all three, so every external client traverses the same chain.

parse_client_ip indexes from the right, so len=2, hops=1 -> parts[1], the edge. hops=2 -> parts[0], the client.

The claim I had wrong, found by Codex in review

I had said a higher hop count always fails safe, because len < hops -> None. It does not protect against a client padding a short chain.

Behind a front-most proxy that appends, a client sending X-Forwarded-For: 1.2.3.4 produces 1.2.3.4, <client>. len is 2, the guard never fires, and hops=2 selects the attacker's value. At hops=1 the same header yields what the proxy actually saw.

So raising this number is not a free safety margin: behind an appending proxy it turns a wrong address into a chosen one.

2 is correct here only because our edge replaces, which is a property of the edge rather than of the number. The doc comment and a_padded_chain_defeats_a_higher_hop_count now say so, so the next person raising it meets the reason rather than the tidy version.

Tests, all mutation-checked

mutation expected red observed
default back to 1 constant test red, on the message naming the regression
default to 3 constant test red, so it is not a tautology on the constant
parse_client_ip indexes from the left padding test red

Gates

Five on the pinned 1.93.0: fmt, clippy --all-targets --all-features -D warnings, test (170), audit, deny.

Also

Four Traefik references removed, zero remain. The comment names the systems that produce the number, and says a deployment not behind this edge must override via JMAP_MCP_TRUSTED_PROXY_HOPS — a backend behind the LAN-only home gateway sees one entry.

Cargo.toml bumped to 0.2.16; this releases rather than taking a temporary env override, since the override would be a line somebody has to remove later.

The deployed `v0.2.15` writes **the edge gateway's address** into the `last_used` provenance record as the client's, on every authenticated request. Refs #14. `DEFAULT_TRUSTED_PROXY_HOPS` was `1`, commented `"(Traefik)"`. There is no Traefik on this cluster. ## Two independent measurements, both 2 | | source | result | |---|---|---| | observed | pod logs, 7 authenticated requests at 2026-08-27T03:37:47Z | `xff_entries=2` | | derived | `oddie-apps/edge-config` + Cilium config | 2 appended entries | `client -> Caddy edge -> Cilium gateway -> pod`. The edge sets no `trusted_proxies` (0 occurrences in 80 site blocks), so it **replaces** XFF with its peer, giving entry 1, the client. Cilium runs `gateway-api-xff-num-trusted-hops: 0`, so Envoy **appends** the downstream address, giving entry 2, the edge. One is read at the pod, the other from the edge repository. They share no instrument. The route is singular as well: `203.24.209.5` times out on tcp/443 across three attempts while `203.24.209.8` answers 200 on all three, so every external client traverses the same chain. `parse_client_ip` indexes from the right, so `len=2, hops=1 -> parts[1]`, the edge. `hops=2 -> parts[0]`, the client. ## The claim I had wrong, found by Codex in review I had said a higher hop count always fails safe, because `len < hops -> None`. **It does not protect against a client padding a short chain.** Behind a front-most proxy that **appends**, a client sending `X-Forwarded-For: 1.2.3.4` produces `1.2.3.4, <client>`. `len` is 2, the guard never fires, and `hops=2` selects **the attacker's value**. At `hops=1` the same header yields what the proxy actually saw. So raising this number is not a free safety margin: behind an appending proxy it turns a wrong address into a chosen one. **2 is correct here only because our edge replaces**, which is a property of the edge rather than of the number. The doc comment and `a_padded_chain_defeats_a_higher_hop_count` now say so, so the next person raising it meets the reason rather than the tidy version. ## Tests, all mutation-checked | mutation | expected red | observed | |---|---|---| | default back to `1` | constant test | red, on the message naming the regression | | default to `3` | constant test | red, so it is not a tautology on the constant | | `parse_client_ip` indexes from the left | padding test | red | ## Gates Five on the pinned 1.93.0: `fmt`, `clippy --all-targets --all-features -D warnings`, `test` (170), `audit`, `deny`. ## Also Four `Traefik` references removed, zero remain. The comment names the systems that produce the number, and says a deployment not behind this edge must override via `JMAP_MCP_TRUSTED_PROXY_HOPS` — a backend behind the LAN-only `home` gateway sees one entry. `Cargo.toml` bumped to 0.2.16; this releases rather than taking a temporary env override, since the override would be a line somebody has to remove later.
fix(audit): trusted proxy hops is 2, measured rather than assumed
All checks were successful
CI / tag-ancestry (pull_request) Successful in 3s
CI / cargo (pull_request) Successful in 58s
CI / docker (pull_request) Successful in 1m8s
63de6a3702
DEFAULT_TRUSTED_PROXY_HOPS was 1, commented "(Traefik)". There is no Traefik
on this cluster, and parse_client_ip counts in from the right, so 1 against a
two-entry header selected the edge's own address and wrote it into the
last_used provenance record as the client's. Not blank, not obviously wrong: a
well-formed address identifying the wrong party in the field read during an
incident.

Two independent measurements agree on 2:

  observed  the pod logged xff_entries=2 on 7 authenticated requests,
            2026-08-27T03:37:47Z, the only such requests it has served
  derived   client -> Caddy edge -> Cilium gateway -> pod. The edge sets no
            trusted_proxies so it REPLACES XFF with its peer (entry 1, the
            client); Cilium runs gateway-api-xff-num-trusted-hops: 0 so Envoy
            APPENDS the downstream address (entry 2, the edge)

One is read at the pod and the other from oddie-apps/edge-config, sharing no
instrument. The route is singular too: 203.24.209.5 times out on 443 across
three attempts while 203.24.209.8 answers 200 on all three.

2 is safe here ONLY because the edge replaces rather than appends, and Codex
found that in review: "len < hops -> None" does not protect against a client
PADDING a short chain. Behind a front-most proxy that appends, a client sending
X-Forwarded-For: 1.2.3.4 produces "1.2.3.4, <client>", len is 2, the guard
never fires, and hops=2 selects the attacker's value. So a higher hop count is
not a free safety margin: behind an appending proxy it turns a wrong address
into a chosen one. I had told Alan the opposite; the comment and a test now
carry the correction rather than the tidy version.

The doc comment names the systems that produce the number instead of a product
that is not deployed, and says a deployment not behind that edge must override
via JMAP_MCP_TRUSTED_PROXY_HOPS. A backend behind the LAN-only home gateway
sees one entry.

Tests pin the constant, the pairing that makes it matter, and the padding case.
Mutation-checked: reverting the default to 1 goes red on the message naming the
regression, 3 goes red too so it is not a tautology, and reversing the index
direction turns the padding test red.

Five gates green on 1.93.0: fmt, clippy -D warnings, test (170), audit, deny.

Version bumped to 0.2.16 for the release.

Refs #14

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jlxq0 merged commit 7653ee8c22 into main 2026-08-27 03:56:20 +00:00
jlxq0 deleted branch fix/trusted-proxy-hops-2 2026-08-27 03:56:21 +00:00
Sign in to join this conversation.
No reviewers
No labels
waiting-on-julian
No milestone
No project
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
jlxq0/jmap-mcp!21
No description provided.