diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feature #38954 [HttpFundation][FrameworkBundle] Deprecate the HEADER_X_FORWARDED_ALL constant (jderusse)

Browse Source 
This PR was merged into the 5.2-dev branch.

Discussion
----------

[HttpFundation][FrameworkBundle] Deprecate the HEADER_X_FORWARDED_ALL constant

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       | -
| License       | MIT
| Doc PR        | TODO

The `HEADER_X_FORWARDED_ALL` implicitly trust the `x-forwarded-host` header, leading to possible host header attack (as warned in the [documentation](https://symfony.com/doc/current/reference/configuration/framework.html#trusted-hosts).)

Moreover, this `HEADER_X_FORWARDED_ALL` does not really fowards **all** headers, as ti does not supports `X-Forwarded-Prefix` headers.

This PR deprecate the constant and the new framework bundle configuration. It will be removed in 6.0. People have to use: either:
- `Request::setTrustedProxies(['1.2.3.4'], Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO);`
- `Request::setTrustedProxies(['1.2.3.4'], Request::HEADER_X_FORWARDED_TRAEFIK);`
- `framework.trusted_headers: [x-forwarded-for, x-forwarded-host, x-forwarded-port, x-forwarded-proto]`

Commits
-------

7cf4dd6917 Deprecate HEADER_X_FORWARDED_ALL constant
This commit is contained in:
Fabien Potencier 2020-11-04 08:16:55 +01:00
commit 6251c4ee6e
10 changed files with 48 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
UPGRADE-5.2.md
View File

@ -43,6 +43,7 @@ HttpFoundation
--------------
* Deprecated not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()`; wrap your filter in a closure instead.
* Deprecated the `Request::HEADER_X_FORWARDED_ALL` constant, use either `Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO` or `Request::HEADER_X_FORWARDED_AWS_ELB` or `Request::HEADER_X_FORWARDED_TRAEFIK`constants instead.
Lock
----

1
UPGRADE-6.0.md
View File

@ -67,6 +67,7 @@ HttpFoundation
`RedirectResponse::create()`, and `StreamedResponse::create()` methods (use
`__construct()` instead)
* Not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()` throws an `InvalidArgumentException`; wrap your filter in a closure instead.
* Removed the `Request::HEADER_X_FORWARDED_ALL` constant, use either `Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO` or `Request::HEADER_X_FORWARDED_AWS_ELB` or `Request::HEADER_X_FORWARDED_TRAEFIK`constants instead.
HttpKernel
----------

2
src/Symfony/Bridge/Monolog/Tests/Processor/WebProcessorTest.php
View File

@ -38,7 +38,7 @@ class WebProcessorTest extends TestCase
public function testUseRequestClientIp()
{
Request::setTrustedProxies(['192.168.0.1'], Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies(['192.168.0.1'], Request::HEADER_X_FORWARDED_FOR);
[$event, $server] = $this->createRequestEvent(['X_FORWARDED_FOR' => '192.168.0.2']);
$processor = new WebProcessor();

3
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
View File

@ -92,13 +92,12 @@ class Configuration implements ConfigurationInterface
->arrayNode('trusted_headers')
->fixXmlConfig('trusted_header')
->performNoDeepMerging()
->defaultValue(['x-forwarded-all', '!x-forwarded-host', '!x-forwarded-prefix'])
->defaultValue(['x-forwarded-for', 'x-forwarded-port', 'x-forwarded-proto'])
->beforeNormalization()->ifString()->then(function ($v) { return $v ? array_map('trim', explode(',', $v)) : []; })->end()
->enumPrototype()
->values([
'forwarded',
'x-forwarded-for', 'x-forwarded-host', 'x-forwarded-proto', 'x-forwarded-port',
'x-forwarded-all', '!x-forwarded-host', '!x-forwarded-prefix',
])
->end()
->end()

7
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View File

@ -2294,13 +2294,6 @@ class FrameworkExtension extends Extension
case 'x-forwarded-host': $trustedHeaders |= Request::HEADER_X_FORWARDED_HOST; break;
case 'x-forwarded-proto': $trustedHeaders |= Request::HEADER_X_FORWARDED_PROTO; break;
case 'x-forwarded-port': $trustedHeaders |= Request::HEADER_X_FORWARDED_PORT; break;
case '!x-forwarded-host': $trustedHeaders &= ~Request::HEADER_X_FORWARDED_HOST; break;
case 'x-forwarded-all':
if (!\in_array('!x-forwarded-prefix', $headers)) {
throw new LogicException('When using "x-forwarded-all" in "framework.trusted_headers", "!x-forwarded-prefix" must be explicitly listed until support for X-Forwarded-Prefix is implemented.');
}
$trustedHeaders |= Request::HEADER_X_FORWARDED_ALL;
break;
}
}

6
src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
View File

@ -341,9 +341,9 @@ class ConfigurationTest extends TestCase
'secret' => 's3cr3t',
'trusted_hosts' => [],
'trusted_headers' => [
'x-forwarded-all',
'!x-forwarded-host',
'!x-forwarded-prefix',
'x-forwarded-for',
'x-forwarded-port',
'x-forwarded-proto',
],
'csrf_protection' => [
'enabled' => false,

2
src/Symfony/Component/HttpFoundation/CHANGELOG.md
View File

@ -11,6 +11,8 @@ CHANGELOG
* added `Request::toArray()` to parse a JSON request body to an array
* added `RateLimiter\RequestRateLimiterInterface` and `RateLimiter\AbstractRequestRateLimiter`
* deprecated not passing a `Closure` together with `FILTER_CALLBACK` to `ParameterBag::filter()`; wrap your filter in a closure instead.
* Deprecated the `Request::HEADER_X_FORWARDED_ALL` constant, use either `HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO` or `HEADER_X_FORWARDED_AWS_ELB` or `HEADER_X_FORWARDED_TRAEFIK` constants instead.
5.1.0
-----

10
src/Symfony/Component/HttpFoundation/Request.php
View File

@ -47,9 +47,10 @@ class Request
const HEADER_X_FORWARDED_PORT = 0b010000;
const HEADER_X_FORWARDED_PREFIX = 0b100000;
const HEADER_X_FORWARDED_ALL = 0b011110; // All "X-Forwarded-*" headers sent by "usual" reverse proxy
const HEADER_X_FORWARDED_AWS_ELB = 0b011010; // AWS ELB doesn't send X-Forwarded-Host
const HEADER_X_FORWARDED_TRAEFIK = 0b111110; // All "X-Forwarded-*" headers sent by Traefik reverse proxy
/** @deprecated since Symfony 5.2, use either "HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO" or "HEADER_X_FORWARDED_AWS_ELB" or "HEADER_X_FORWARDED_TRAEFIK" constants instead. */
const HEADER_X_FORWARDED_ALL = 0b1011110; // All "X-Forwarded-*" headers sent by "usual" reverse proxy
const HEADER_X_FORWARDED_AWS_ELB = 0b0011010; // AWS ELB doesn't send X-Forwarded-Host
const HEADER_X_FORWARDED_TRAEFIK = 0b0111110; // All "X-Forwarded-*" headers sent by Traefik reverse proxy
const METHOD_HEAD = 'HEAD';
const METHOD_GET = 'GET';
@ -593,6 +594,9 @@ class Request
*/
public static function setTrustedProxies(array $proxies, int $trustedHeaderSet)
{
if (self::HEADER_X_FORWARDED_ALL === $trustedHeaderSet) {
trigger_deprecation('symfony/http-fundation', '5.2', 'The "HEADER_X_FORWARDED_ALL" constant is deprecated, use either "HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO" or "HEADER_X_FORWARDED_AWS_ELB" or "HEADER_X_FORWARDED_TRAEFIK" constants instead.');
}
self::$trustedProxies = array_reduce($proxies, function ($proxies, $proxy) {
if ('REMOTE_ADDR' !== $proxy) {
$proxies[] = $proxy;

42
src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
View File

@ -12,6 +12,7 @@
namespace Symfony\Component\HttpFoundation\Tests;
use PHPUnit\Framework\TestCase;
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
use Symfony\Component\HttpFoundation\Exception\JsonException;
use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
use Symfony\Component\HttpFoundation\InputBag;
@ -22,6 +23,8 @@ use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
class RequestTest extends TestCase
{
use ExpectDeprecationTrait;
protected function tearDown(): void
{
Request::setTrustedProxies([], -1);
@ -867,7 +870,7 @@ class RequestTest extends TestCase
$this->assertEquals(80, $port, 'Without trusted proxies FORWARDED_PROTO and FORWARDED_PORT are ignored.');
Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_PROTO | Request::HEADER_X_FORWARDED_PORT);
$request = Request::create('http://example.com', 'GET', [], [], [], [
'HTTP_X_FORWARDED_PROTO' => 'https',
'HTTP_X_FORWARDED_PORT' => '8443',
@ -1091,7 +1094,7 @@ class RequestTest extends TestCase
'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor,
];
Request::setTrustedProxies(['88.88.88.88'], Request::HEADER_X_FORWARDED_ALL | Request::HEADER_FORWARDED);
Request::setTrustedProxies(['88.88.88.88'], Request::HEADER_X_FORWARDED_FOR | Request::HEADER_FORWARDED);
$request->initialize([], [], [], [], [], $server);
@ -1349,7 +1352,7 @@ class RequestTest extends TestCase
$request->headers->set('X_FORWARDED_PROTO', 'https');
Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_PROTO);
$this->assertFalse($request->isSecure());
$request->server->set('REMOTE_ADDR', '1.1.1.1');
$this->assertTrue($request->isSecure());
@ -1830,7 +1833,7 @@ class RequestTest extends TestCase
}
if ($trustedProxies) {
Request::setTrustedProxies($trustedProxies, Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies($trustedProxies, Request::HEADER_X_FORWARDED_FOR);
}
$request->initialize([], [], [], [], [], $server);
@ -1873,35 +1876,35 @@ class RequestTest extends TestCase
$this->assertFalse($request->isSecure());
// disabling proxy trusting
Request::setTrustedProxies([], Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies([], Request::HEADER_X_FORWARDED_FOR);
$this->assertEquals('3.3.3.3', $request->getClientIp());
$this->assertEquals('example.com', $request->getHost());
$this->assertEquals(80, $request->getPort());
$this->assertFalse($request->isSecure());
// request is forwarded by a non-trusted proxy
Request::setTrustedProxies(['2.2.2.2'], Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies(['2.2.2.2'], Request::HEADER_X_FORWARDED_FOR);
$this->assertEquals('3.3.3.3', $request->getClientIp());
$this->assertEquals('example.com', $request->getHost());
$this->assertEquals(80, $request->getPort());
$this->assertFalse($request->isSecure());
// trusted proxy via setTrustedProxies()
Request::setTrustedProxies(['3.3.3.3', '2.2.2.2'], Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies(['3.3.3.3', '2.2.2.2'], Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO);
$this->assertEquals('1.1.1.1', $request->getClientIp());
$this->assertEquals('foo.example.com', $request->getHost());
$this->assertEquals(443, $request->getPort());
$this->assertTrue($request->isSecure());
// trusted proxy via setTrustedProxies()
Request::setTrustedProxies(['3.3.3.4', '2.2.2.2'], Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies(['3.3.3.4', '2.2.2.2'], Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO);
$this->assertEquals('3.3.3.3', $request->getClientIp());
$this->assertEquals('example.com', $request->getHost());
$this->assertEquals(80, $request->getPort());
$this->assertFalse($request->isSecure());
// check various X_FORWARDED_PROTO header values
Request::setTrustedProxies(['3.3.3.3', '2.2.2.2'], Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies(['3.3.3.3', '2.2.2.2'], Request::HEADER_X_FORWARDED_PROTO);
$request->headers->set('X_FORWARDED_PROTO', 'ssl');
$this->assertTrue($request->isSecure());
@ -2377,7 +2380,7 @@ class RequestTest extends TestCase
public function testTrustedPortDoesNotDefaultToZero()
{
Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies(['1.1.1.1'], Request::HEADER_X_FORWARDED_FOR);
$request = Request::create('/');
$request->server->set('REMOTE_ADDR', '1.1.1.1');
@ -2393,7 +2396,7 @@ class RequestTest extends TestCase
public function testTrustedProxiesRemoteAddr($serverRemoteAddr, $trustedProxies, $result)
{
$_SERVER['REMOTE_ADDR'] = $serverRemoteAddr;
Request::setTrustedProxies($trustedProxies, Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies($trustedProxies, Request::HEADER_X_FORWARDED_FOR);
$this->assertSame($result, Request::getTrustedProxies());
}
@ -2464,6 +2467,23 @@ class RequestTest extends TestCase
],
];
}
/**
* @group legacy
*/
public function testXForwarededAllConstantDeprecated()
{
$this->expectDeprecation('Since symfony/http-fundation 5.2: The "HEADER_X_FORWARDED_ALL" constant is deprecated, use either "HEADER_X_FORWARDED_FOR | HEADER_X_FORWARDED_HOST | HEADER_X_FORWARDED_PORT | HEADER_X_FORWARDED_PROTO" or "HEADER_X_FORWARDED_AWS_ELB" or "HEADER_X_FORWARDED_TRAEFIK" constants instead.');
Request::setTrustedProxies([], Request::HEADER_X_FORWARDED_ALL);
}
public function testReservedFlags()
{
foreach ((new \ReflectionClass(Request::class))->getConstants() as $constant => $value) {
$this->assertNotSame(0b10000000, $value, sprintf('The constant "%s" should not use the reserved value "0b10000000".', $constant));
}
}
}
class RequestContentProxy extends Request

2
src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php
View File

@ -1361,7 +1361,7 @@ class HttpCacheTest extends HttpCacheTestCase
*/
public function testHttpCacheIsSetAsATrustedProxy(array $existing)
{
Request::setTrustedProxies($existing, Request::HEADER_X_FORWARDED_ALL);
Request::setTrustedProxies($existing, Request::HEADER_X_FORWARDED_FOR);
$this->setNextResponse();
$this->request('GET', '/', ['REMOTE_ADDR' => '10.0.0.1']);