-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[framework-bundle] Added support for the 0.0.0.0/0
trusted proxy
#15706
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
Conversation
0.0.0.0/0
trusted proxy0.0.0.0/0
trusted proxy
Guys, anyone? |
A traditional monthly reminder: guys, could someone please put some attention here? |
@@ -139,6 +139,10 @@ public function getConfigTreeBuilder() | |||
} | |||
|
|||
if (false !== strpos($v, '/')) { | |||
if ('0.0.0.0/0' === $v) { | |||
return false; | |||
} |
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.
I would move this above the earlier if
(no need to do the strpos()
check first.
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.
@xabbuh that's a fair point.
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.
@xabbuh from the other hand, the '0.0.0.0/0'
scenario is less likely to happen. So how about promoting the more-likely-to-happen scenario over the other?
If we move this check before the if (false !== strpos($v, '/')) {
then we would have 2 checks for almost every symfony user, instead of just 1 as it is now.
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.
That's probably true, but as the Configuration
class won't be checked at runtime it doesn't really matter.
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.
@xabbuh oh, right. Completely missed that point. It does not make much difference then. Let's see if there are more concerns here from someone else.
looks good to me |
Out of curiosity, why would you want to do that? Is it only for development purpose? |
@fabpot it's for production: I have the following configuration:
So far so good. But as you remember the application is served via HTTPS while symfony runs behind nginx http. Thankfully the x-forwarded-proto is there but for symfony to respect it the remote ip address must be trusted. What we have: all the requests to nginx come from a trusted proxy (everything is passed through haproxy) but the remote ip address is replaced with ngx_http_realip_module. Which means that for everything to work as we expected - we need to tell symfony to trust all ip's, hence to So in my implementation it's nginx that need to know who to trust to, and symfony just need to trust anyone, since everything was already constrained by haproxy and nginx. |
Thank you @zerkms. |
…ed proxy (zerkms) This PR was submitted for the 2.8 branch but it was merged into the 2.3 branch instead (closes #15706). Discussion ---------- [framework-bundle] Added support for the `0.0.0.0/0` trusted proxy | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT It is relevant to my other PR: #14690 The original intention was to start accepting `0.0.0.0/0` as a trusted proxy (which is a valid CIDR notation). The prupose is to allow all requests to be treated as trusted and to eliminate using dirty `['0.0.0.0/1', '128.0.0.0/1']` workaround. Commits ------- 3188e1b Added support for the `0.0.0.0/0` trusted proxy
Can this be backported to 2.8 please, @fabpot ? |
@dzuelke this fix was merged into 2.3, and is already present in the 2.8 branch. |
Thanks @jakzal |
It is relevant to my other PR: #14690
The original intention was to start accepting
0.0.0.0/0
as a trusted proxy (which is a valid CIDR notation).The prupose is to allow all requests to be treated as trusted and to eliminate using dirty
['0.0.0.0/1', '128.0.0.0/1']
workaround.