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]

Reply via email to