You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
So with trial and error I've worked out what Caddy currently does for x-forwarded-for, and I found it surprising, and I'd like to suggest a change.
Firstly - I used the server level trusted_proxies setting, along with trusted_proxies_strict
I was able to see in the caddy logs the expected client_ip":"1.1.1.1" value, picked from the list that came in x-forwared-for from upstream, and correctly ignoring IPs further to the left.
What surprised me was that the x-forwarded-for header set upstream using reverse_proxy from caddy included the full x-forwarded-for header, including untrusted IPs further to the left
The docs state
For these X-Forwarded-* headers, by default, the proxy will ignore their values from incoming requests, to prevent spoofing.
If Caddy is not the first server being connected to by your clients (for example when a CDN is in front of Caddy),
you may configure trusted_proxies with a list of IP ranges (CIDRs) from which incoming requests are trusted
to have sent good values for these headers.
Which I read to mean that x-forwarded-for going upstream would only include trusted values. But it doesnt - it includes the full value, if at least one of them is trusted.
Example
trusted_proxies static 64.252.66.102 10.0.0.0/8
request to caddy has
GET /
X-Forwarded-For: 73.99.36.212, 34.138.34.110, 64.252.66.102, 10.3.124.6
so client_ip is 34.138.34.110 - this is shown in the logs - 73.99.36.212 is correctly ignored
Request to upstream now has - with remote_addr added to the right
GET /
X-Forwarded-For: 73.99.36.212, 34.138.34.110, 64.252.66.102, 10.3.124.6, 127.0.0.1
This means I need to repeat the logic of finding the trusted client ip in my application. I thought I should be able to take the left most value and its going to be correct
So I want that value to be
GET /
X-Forwarded-For: 34.138.34.110, 64.252.66.102, 10.3.124.6, 127.0.0.1
then my app just reads the left most value, and its a good trusted value
Instead, as what i see is a workaround, I've added header_up x-client-ip {client_ip} and read that value in the application.
Would a PR changing the code to only forward the trusted part of x-forwarded-for be welcomed? maybe adding trusted_proxies_strict directive to the reverse_proxy as a way to enable this, as it would be a breaking change otherwise
The text was updated successfully, but these errors were encountered:
So with trial and error I've worked out what Caddy currently does for x-forwarded-for, and I found it surprising, and I'd like to suggest a change.
Firstly - I used the server level
trusted_proxies
setting, along withtrusted_proxies_strict
I was able to see in the caddy logs the expected
client_ip":"1.1.1.1"
value, picked from the list that came inx-forwared-for
from upstream, and correctly ignoring IPs further to the left.What surprised me was that the
x-forwarded-for
header set upstream usingreverse_proxy
from caddy included the fullx-forwarded-for
header, including untrusted IPs further to the leftThe docs state
Which I read to mean that x-forwarded-for going upstream would only include trusted values. But it doesnt - it includes the full value, if at least one of them is trusted.
Example
request to caddy has
so
client_ip
is34.138.34.110
- this is shown in the logs -73.99.36.212
is correctly ignoredRequest to upstream now has - with remote_addr added to the right
This means I need to repeat the logic of finding the trusted client ip in my application. I thought I should be able to take the left most value and its going to be correct
So I want that value to be
then my app just reads the left most value, and its a good trusted value
Instead, as what i see is a workaround, I've added
header_up x-client-ip {client_ip}
and read that value in the application.Would a PR changing the code to only forward the trusted part of x-forwarded-for be welcomed? maybe adding
trusted_proxies_strict
directive to thereverse_proxy
as a way to enable this, as it would be a breaking change otherwiseThe text was updated successfully, but these errors were encountered: