fix: preserve X-Forwarded-For from untrusted sources#13611
Open
shreemaan-abhishek wants to merge 1 commit into
Open
fix: preserve X-Forwarded-For from untrusted sources#13611shreemaan-abhishek wants to merge 1 commit into
shreemaan-abhishek wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts APISIX’s handling of forwarded headers for downstream connections that are not in apisix.trusted_addresses, preserving the incoming X-Forwarded-For header so upstreams can still receive the original chain (with the downstream connection IP appended by Nginx).
Changes:
- Stop clearing
X-Forwarded-Forfor untrusted sources while continuing to overrideX-Forwarded-Proto/Host/Portand clearForwarded. - Update core tests to assert
X-Forwarded-Forpreservation + append behavior in untrusted scenarios. - Update the example configuration comments to document the revised behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
apisix/init.lua |
Keeps X-Forwarded-For intact for untrusted sources while still sanitizing other forwarded headers. |
t/core/trusted-addresses.t |
Updates expectations to verify preserved incoming XFF plus appended downstream connection IP. |
t/plugin/real-ip.t |
Adjusts behavior/expectations so the real-ip plugin can still honor XFF when global trusted addresses don’t trust the downstream source. |
conf/config.yaml.example |
Documents the updated trusted/untrusted forwarded-header behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+159
to
+161
| # values and clears the `Forwarded` header. `X-Forwarded-For` is kept | ||
| # and the trusted connection IP is appended, so the upstream still | ||
| # receives the real client IP as the last hop of the chain. |
Comment on lines
+650
to
+652
| -- X-Forwarded-For is kept as-is: ngx_tpl appends the trusted | ||
| -- connection IP via `$proxy_add_x_forwarded_for`, so the upstream | ||
| -- always sees the real client IP as the last hop of the chain. |
When the connection is not in `apisix.trusted_addresses`, APISIX cleared the incoming X-Forwarded-For entirely, so upstreams behind a multi-tier proxy chain (client -> proxy A -> APISIX -> upstream) lost the real client IP after upgrading. X-Forwarded-For has a safe append model: ngx_tpl appends the trusted connection IP via $proxy_add_x_forwarded_for, so even a forged chain always carries the real client IP as its last hop. Keep it intact and only overwrite the headers that have no append mechanism (X-Forwarded-Proto / Host / Port) and clear the RFC 7239 Forwarded header. This matches the default behavior of Kong, Tyk and Envoy edge mode.
98c0647 to
9193ec5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When the downstream connection is not listed in
apisix.trusted_addresses,handle_x_forwarded_headers()cleared the incomingX-Forwarded-Forheader outright. For deployments with a multi-tier proxy chain (e.g.client -> front proxy -> APISIX -> upstream) where the front proxy is not added totrusted_addresses, the upstream loses the original client IP and only sees the connection IP rebuilt by$proxy_add_x_forwarded_for. This is a behavior change from older releases and breaks upstreams that read the client IP fromX-Forwarded-For.Unlike
X-Forwarded-Proto/Host/Portand the RFC 7239Forwardedheader (which are set wholesale and have no append mechanism, so a forged value would pass straight through),X-Forwarded-Forhas a safe append model:ngx_tplappends the trusted connection IP via$proxy_add_x_forwarded_for, so even a client-forged chain always carries the real connection IP as its last hop. Consumers that want the real client IP should anchor on the trusted rightmost hop (this is what thereal-ipplugin does).This PR keeps
X-Forwarded-Forintact for untrusted sources and continues to overwriteX-Forwarded-Proto/Host/Portand clearForwarded. This aligns with the default behavior of Kong, Tyk and Envoy edge mode, which preserve the incoming XFF and append the connection IP.Behavior
For an untrusted source sending
X-Forwarded-For: 1.2.3.4:X-Forwarded-For: <connection-ip>X-Forwarded-For: 1.2.3.4, <connection-ip>X-Forwarded-Proto/Host/PortandForwardedhandling is unchanged.Checklist