| index 7ea68def7..14ead04e0 100644 |
| --- a/internal/edge/homemap.go |
| +++ b/internal/edge/homemap.go |
| @@ -37,14 +37,18 @@ package edge |
| // was, which is why verification gates exactly one thing: whether the |
| // inbound X-Forwarded-For is believed. |
| // |
| -// - XFF is trusted as an APPENDED element, never as a replacement. On a |
| -// verified hop the inbound chain is carried onto the outbound request and |
| -// the sending edge's own address is appended to it (setForwarded, below), |
| -// so the app sees "<client>, <sending edge>" — the same shape a |
| -// single-region app already sees, with one more hop in it. On anything |
| -// else the chain is discarded outright: an unverified sender is an |
| -// ordinary client, and an ordinary client has never been allowed to state |
| -// its own address here. |
| +// - XFF is trusted as an inbound chain to CARRY, never one to extend. On a |
| +// verified hop the chain the sending edge built is passed through |
| +// verbatim (setForwarded, below), so the app sees exactly what a |
| +// single-region app sees and the LAST element is still the client. The |
| +// backhaul leg is deliberately invisible in that header: this |
| +// deployment's own consumers take the last element as the caller |
| +// (internal/console/authcli.go's clientIP, and every tenant framework's |
| +// rightmost-public heuristic), so appending the sibling edge would make |
| +// every backhauled request look like it came from infrastructure. On |
| +// anything but a verified hop the chain is discarded outright: an |
| +// unverified sender is an ordinary client, and an ordinary client has |
| +// never been allowed to state its own address here. |
| // |
| // One hop, ever, is STRUCTURAL and not a policy check: a verified hop that |
| // names a host this box does not serve leaves through staleHopRefusal (503 + |
| @@ -488,6 +492,19 @@ func hopVerified(ctx context.Context) bool { |
| // signer and a verifier can never disagree (mesh.go). |
| func (e *Edge) acceptHop(r *http.Request, host string) (*http.Request, bool) { |
| if r.Header.Get(hopHeader) == "" { |
| + // A signature with no marker beside it is not a hop — but it is still |
| + // this edge's vocabulary, and the invariant above is "deleted in ALL |
| + // hop cases" without an exception for the half a client made up. |
| + // scrubInboundPurgeHeader deletes both purge headers unconditionally |
| + // for exactly this reason (purge_header.go): an app that reflects what |
| + // it was sent turns a client-supplied edge header into an |
| + // app-DECLARED one, and no tenant's reflection bug should be able to |
| + // put hop vocabulary back on the wire. The Get stays the fast path's |
| + // single lookup — the Del only happens on a request that actually |
| + // carries the stray header, which is never, in the ordinary case. |
| + if r.Header.Get(hopAuthHeader) != "" { |
| + r.Header.Del(hopAuthHeader) |
| + } |
| return r, false |
| } |
| auth := r.Header.Get(hopAuthHeader) |
| @@ -540,25 +557,35 @@ func staleHopRefusal(w http.ResponseWriter) { |
| // pr.SetXForwarded(), and it exists for one narrow case. |
| // |
| // ReverseProxy's Rewrite path deletes Forwarded and X-Forwarded-* from the |
| -// outbound request before Rewrite runs, so SetXForwarded's documented "append |
| -// to an existing value" behaviour never fires: an app behind this edge sees |
| -// exactly one address, the peer that dialled us. That is right for a client |
| -// and wrong for a verified hop, where the peer that dialled us is a sibling |
| -// edge and the address the app actually wants is the one that edge recorded. |
| +// outbound request before Rewrite runs, so SetXForwarded always writes a |
| +// single address: the peer that dialled us. That is right for a client and |
| +// wrong for a verified hop, where the peer that dialled us is a sibling edge |
| +// and the address the app wants is the one that edge already recorded. |
| // |
| -// So on a verified hop — and only there — the inbound chain is copied onto the |
| -// outbound request first, and SetXForwarded then appends the sending edge to |
| -// it. Multiple inbound X-Forwarded-For lines are joined, which is what |
| -// list-valued header semantics already say they mean (RFC 9110 §5.3). |
| +// So on a verified hop — and only there — SetXForwarded's answer is REPLACED |
| +// by the inbound chain, verbatim. Not appended to: the last element has to |
| +// stay the client. This deployment's own consumer says so in as many words |
| +// (internal/console/authcli.go's clientIP: "the LAST element is the one to |
| +// take"), and tenant frameworks' rightmost-public heuristics agree — a chain |
| +// of "<client>, <sending edge EIP>" would rate-limit every backhauled console |
| +// request as one bucket per region and show tenant apps infrastructure in |
| +// place of their caller. Passing the chain through unchanged means a |
| +// backhauled request is indistinguishable from a local one at the app, which |
| +// is the whole promise of the hop. Multiple inbound X-Forwarded-For lines are |
| +// joined, which is what list-valued header semantics already say they mean |
| +// (RFC 9110 §5.3). |
| // |
| -// X-Forwarded-Host and X-Forwarded-Proto need no such restore: the hop carries |
| -// the original Host header and is itself TLS, so SetXForwarded recomputes both |
| -// correctly at this edge. |
| +// SetXForwarded still runs first, and is still what sets X-Forwarded-Host and |
| +// X-Forwarded-Proto: the hop carries the original Host header and is itself |
| +// TLS, so both are correct as computed here and need no restore. It touches |
| +// only pr.Out, so reading the inbound chain after it is the same read as |
| +// before it. |
| func setForwarded(pr *httputil.ProxyRequest) { |
| - if hopVerified(pr.In.Context()) { |
| - if chain := pr.In.Header.Values("X-Forwarded-For"); len(chain) > 0 { |
| - pr.Out.Header.Set("X-Forwarded-For", strings.Join(chain, ", ")) |
| - } |
| - } |
| pr.SetXForwarded() |
| + if !hopVerified(pr.In.Context()) { |
| + return |
| + } |
| + if chain := pr.In.Header.Values("X-Forwarded-For"); len(chain) > 0 { |
| + pr.Out.Header.Set("X-Forwarded-For", strings.Join(chain, ", ")) |
| + } |
| } |
amadan