Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Http][DI] Replace REMOTE_ADDR in trusted proxies with the current REMOTE_ADDR#33574
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedSep 18, 2019
I don't think you need this: use |
stof commentedSep 18, 2019
@nicolas-grekas this is not equivalent. Proxies can be chained. Trusting |
stof commentedSep 18, 2019
@mcfedr can you check whether your ELB proxies appears as an remote address belonging to the private IP range ? If yes, maybe you could trust the private IP ranges for your use case (though I don't know how isolated your AWS instances are when not using an AWS VPC). |
nicolas-grekas commentedSep 18, 2019
you may be right, but I'm missing the why: This only reads the direct REMOTE_ADDR. When does the difference you describe apply? |
stof commentedSep 18, 2019
@nicolas-grekas |
nicolas-grekas commentedSep 18, 2019
|
stof commentedSep 18, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@nicolas-grekas italso uses |
mcfedr commentedSep 18, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The chain is the problem with using |
nicolas-grekas commentedSep 18, 2019
Oh, OK! Now I get the issue, it's related to |
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
(with a minor CS suggestion)
mcfedr commentedSep 18, 2019
All done. |
fabpot commentedSep 27, 2019
Thank you@mcfedr. |
… the current REMOTE_ADDR (mcfedr)This PR was merged into the 4.4 branch.Discussion----------[Http][DI] Replace REMOTE_ADDR in trusted proxies with the current REMOTE_ADDR| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | yes| Deprecations? | no| License | MIT| Doc PR |Currently handling trusted ips when deploying behind some CDNs/Load balancers such as ELB is difficult because they dont have a constant IP address, its possible to overcome this as is suggested by the docs -https://symfony.com/doc/current/deployment/proxies.html#but-what-if-the-ip-of-my-reverse-proxy-changes-constantly - by settings trusted proxies to `$request->server->get('REMOTE_ADDR')` - but this has to be done in code, and so becomes dangerous if you code is deployed in different environments.This change would allow the developer to stick to providing the envvar `TRUSTED_PROXIES`, and in the environment behind a ELB set the value to the literal string `REMOTE_ADDR`, and have it replaced at run time. This way in environments that are not using ELB his app is kept safe.I think doing this replacement in `Request:: setTrustedProxies` is the best place because it means this feature isn't exposed to other parts of the code that might call `Request::getTrustedProxies`.Commits-------643c9ff Replace REMOTE_ADDR in trusted proxies with the current REMOTE_ADDR
…or private IP address ranges to `Request::setTrustedProxies()` (nicolas-grekas)This PR was merged into the 7.2 branch.Discussion----------[HttpFoundation] Add `PRIVATE_SUBNETS` as a shortcut for private IP address ranges to `Request::setTrustedProxies()`| Q | A| ------------- | ---| Branch? | 7.2| Bug fix? | no| New feature? | no| Deprecations? | no| Issues | -| License | MITLet's save some memory allocations and callbacks when we can.Tweaks#33574 and#52924Commits-------6bd4b4a [HttpFoundation] Add `PRIVATE_SUBNETS` as a shortcut for private IP address ranges to `Request::setTrustedProxies()`
Currently handling trusted ips when deploying behind some CDNs/Load balancers such as ELB is difficult because they dont have a constant IP address, its possible to overcome this as is suggested by the docs -https://symfony.com/doc/current/deployment/proxies.html#but-what-if-the-ip-of-my-reverse-proxy-changes-constantly - by settings trusted proxies to
$request->server->get('REMOTE_ADDR')- but this has to be done in code, and so becomes dangerous if you code is deployed in different environments.This change would allow the developer to stick to providing the envvar
TRUSTED_PROXIES, and in the environment behind a ELB set the value to the literal stringREMOTE_ADDR, and have it replaced at run time. This way in environments that are not using ELB his app is kept safe.I think doing this replacement in
Request:: setTrustedProxiesis the best place because it means this feature isn't exposed to other parts of the code that might callRequest::getTrustedProxies.