feature #33574 [Http][DI] Replace REMOTE_ADDR in trusted proxies with the current REMOTE_ADDR (mcfedr)

fabpot · fabpot · commit 66f8f68e13d5 · 2019-09-27T07:42:32.000+02:00
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
diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php
@@ -567,14 +567,22 @@ public function overrideGlobals()
      *
      * You should only list the reverse proxies that you manage directly.
      *
-     * @param array $proxies          A list of trusted proxies
+     * @param array $proxies          A list of trusted proxies, the string 'REMOTE_ADDR' will be replaced with $_SERVER['REMOTE_ADDR']
      * @param int   $trustedHeaderSet A bit field of Request::HEADER_*, to set which headers to trust from your proxies
      *
      * @throws \InvalidArgumentException When $trustedHeaderSet is invalid
      */
     public static function setTrustedProxies(array $proxies, int $trustedHeaderSet)
     {
-        self::$trustedProxies = $proxies;
+        self::$trustedProxies = array_reduce($proxies, function ($proxies, $proxy) {
+            if ('REMOTE_ADDR' !== $proxy) {
+                $proxies[] = $proxy;
+            } elseif (isset($_SERVER['REMOTE_ADDR'])) {
+                $proxies[] = $_SERVER['REMOTE_ADDR'];
+            }
+
+            return $proxies;
+        }, []);
         self::$trustedHeaderSet = $trustedHeaderSet;
     }
 
diff --git a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
@@ -2324,6 +2324,26 @@ public function testTrustedPortDoesNotDefaultToZero()
 
         $this->assertSame(80, $request->getPort());
     }
+
+    /**
+     * @dataProvider trustedProxiesRemoteAddr
+     */
+    public function testTrustedProxiesRemoteAddr($serverRemoteAddr, $trustedProxies, $result)
+    {
+        $_SERVER['REMOTE_ADDR'] = $serverRemoteAddr;
+        Request::setTrustedProxies($trustedProxies, Request::HEADER_X_FORWARDED_ALL);
+        $this->assertSame($result, Request::getTrustedProxies());
+    }
+
+    public function trustedProxiesRemoteAddr()
+    {
+        return [
+            ['1.1.1.1', ['REMOTE_ADDR'], ['1.1.1.1']],
+            ['1.1.1.1', ['REMOTE_ADDR', '2.2.2.2'], ['1.1.1.1', '2.2.2.2']],
+            [null, ['REMOTE_ADDR'], []],
+            [null, ['REMOTE_ADDR', '2.2.2.2'], ['2.2.2.2']],
+        ];
+    }
 }
 
 class RequestContentProxy extends Request

Original file line number	Diff line number	Diff line change
`@@ -567,14 +567,22 @@ public function overrideGlobals()`
`567`	`567`	`*`
`568`	`568`	`* You should only list the reverse proxies that you manage directly.`
`569`	`569`	`*`
`570`		`- * @param array $proxies A list of trusted proxies`
	`570`	`+ * @param array $proxies A list of trusted proxies, the string 'REMOTE_ADDR' will be replaced with $_SERVER['REMOTE_ADDR']`
`571`	`571`	`* @param int $trustedHeaderSet A bit field of Request::HEADER_*, to set which headers to trust from your proxies`
`572`	`572`	`*`
`573`	`573`	`* @throws \InvalidArgumentException When $trustedHeaderSet is invalid`
`574`	`574`	`*/`
`575`	`575`	`public static function setTrustedProxies(array $proxies, int $trustedHeaderSet)`
`576`	`576`	`{`
`577`		`- self::$trustedProxies = $proxies;`
	`577`	`+ self::$trustedProxies = array_reduce($proxies, function ($proxies, $proxy) {`
	`578`	`+ if ('REMOTE_ADDR' !== $proxy) {`
	`579`	`+ $proxies[] = $proxy;`
	`580`	`+ } elseif (isset($_SERVER['REMOTE_ADDR'])) {`
	`581`	`+ $proxies[] = $_SERVER['REMOTE_ADDR'];`
	`582`	`+ }`
	`583`	`+`
	`584`	`+ return $proxies;`
	`585`	`+ }, []);`
`578`	`586`	`self::$trustedHeaderSet = $trustedHeaderSet;`
`579`	`587`	`}`
`580`	`588`