[framework-bundle] Added support for the 0.0.0.0/0 trusted proxy
#15706
Conversation
zerkms
changed the title
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.
I would move this above the earlier if (no need to do the strpos() check first.
There was a problem hiding this comment.
@xabbuh that's a fair point.
There was a problem hiding this comment.
@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.
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.
@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/0as 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.