-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Always trust 127.0.0.1 as a proxy #9292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I support this. IMHO It also feels a little bit inconsistent with Symfony's reverse proxy as it's In the documentation, this would affect: symfony/symfony-docs#3926 (when I initially wrote that doc PR I wasn't aware of |
it is a trusted proxy only if there is a proxy running there. If you don't have a proxy on 127.0.0.1, registering it all the time open a security hole for the check on the forwarded host, forwarded port and forwarded proto |
@stof I think you're right. But still it's inconvenient having to add 127.0.0.1 to the list only if you need to get the client IP from within a sub-request. What about It just feels more "Symfony-like" to keep the developer out of such cumbersome tasks. |
the HttpCache could indeed register itself as a trusted proxy |
Ok. I'll work out a small PR for that. |
This does indeed seem to be a confusing issue, so I'm happy there's some attention on it :). Thanks @ureimers |
Sorry, I didn't have the time yet because of a very high project load (crunch, crunch, crunch). |
…lkybarkid) This PR was merged into the 2.3 branch. Discussion ---------- [HttpKernel] Make sure HttpCache is a trusted proxy | Q | A | ------------- | --- | Bug fix? | yes (of sorts) | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #9292 | License | MIT | Doc PR | symfony/symfony-docs#4239 Fixes #9292 by adding `127.0.0.1` as a trusted proxy when using `HttpCache` (assuming it hasn't been already). Commits ------- ca65362 Make sure HttpCache is a trusted proxy
Hi @ureimers I've got the same issue here and I'm using Symfony 5.2.14. The way I reproduce it is by using Is it recommended way or like @stof said, this will make a security hole in my website? |
Well, always trusting When you use ngrok, you are deploying your website behind a trusted proxy, and you need to configure it accordingly (based on the headers than ngrok sets) |
The remote address in a fragment sub-request is always set to
127.0.0.1
and the original remote address is pushed into theX-FORWARDED_FOR
header. This is the expected behavior implemented by @fabpot in 2f3b33a and explained for theHttpCache
component here.But when retrieving the client IP via
Request::getClientIp()
, one will now always get127.0.0.1
when it's not added as a trusted proxy. This seems strange, as it will most likely be incorrect and might confuse users which now have to mangle with configuring a trusted proxy, even though they don't actually have a real proxy in place.Also in the commit message of 2f3b33a it says:
But no according change is part of that commit,
getClientIp
does actually consider 127.0.0.1 as the client IP.The text was updated successfully, but these errors were encountered: