Copilot commented on code in PR #13611:
URL: https://github.com/apache/apisix/pull/13611#discussion_r3480068979
##########
conf/config.yaml.example:
##########
@@ -154,8 +154,11 @@ apisix:
# When set to true, it overrides all
upstream healthcheck configurations and globally disabling healthchecks.
# trusted_addresses: # When configured, APISIX will trust the
`X-Forwarded-*` Headers
# - 127.0.0.1 # passed in requests from the IP/CIDR in the
list.
-# - 172.18.0.0/16 # CAUTION: When not configured or the
request from an untrusted address,
- # APISIX will override `X-Forwarded-*`
headers with trusted values.
+# - 172.18.0.0/16 # CAUTION: When not configured or the
request is from an untrusted
+ # address, APISIX overrides
`X-Forwarded-Proto/Host/Port` with trusted
+ # 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.
Review Comment:
The comment says the upstream receives the “real client IP” as the last hop,
but for an untrusted multi-proxy chain the only guaranteed trustworthy value is
the downstream connection/peer IP (i.e., the IP directly connected to APISIX).
Rewording avoids implying APISIX can recover the original end-user IP when the
proxy chain is untrusted.
##########
apisix/init.lua:
##########
@@ -647,8 +647,9 @@ local function handle_x_forwarded_headers(api_ctx)
core.request.set_header(api_ctx, "X-Forwarded-Proto", proto)
core.request.set_header(api_ctx, "X-Forwarded-Host", host)
core.request.set_header(api_ctx, "X-Forwarded-Port", port)
- -- later processed in ngx_tpl by `$proxy_add_x_forwarded_for`.
- core.request.set_header(api_ctx, "X-Forwarded-For", nil)
+ -- 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.
Review Comment:
This comment says the upstream “always sees the real client IP” as the last
hop, but when the downstream connection is a proxy the only non-forgeable value
here is the downstream connection/peer IP. Consider rewording to avoid
suggesting the original end-user IP is reliably available in the untrusted case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]