Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ee8842f

Browse filesBrowse files
magnusnordlanderfabpot
magnusnordlander
authored and
fabpot
committed
[HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For
1 parent fb92d7a commit ee8842f
Copy full SHA for ee8842f

File tree

7 files changed

+275
-27
lines changed
Filter options

7 files changed

+275
-27
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,9 @@
4646
<argument type="service" id="request_stack" />
4747
<tag name="kernel.event_subscriber" />
4848
</service>
49+
50+
<service id="validate_request_listener" class="Symfony\Component\HttpKernel\EventListener\ValidateRequestListener">
51+
<tag name="kernel.event_subscriber" />
52+
</service>
4953
</services>
5054
</container>

‎src/Symfony/Component/HttpFoundation/Exception/ConflictingHeadersException.php

Copy file name to clipboard
+23Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpFoundation\Exception;
13+
14+
/**
15+
* The HTTP request contains headers with conflicting information.
16+
*
17+
* This exception should trigger an HTTP 400 response in your application code.
18+
*
19+
* @author Magnus Nordlander <magnus@fervo.se>
20+
*/
21+
class ConflictingHeadersException extends \RuntimeException
22+
{
23+
}

‎src/Symfony/Component/HttpFoundation/Request.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpFoundation/Request.php
+51-26Lines changed: 51 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpFoundation;
1313

14+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
1415
use Symfony\Component\HttpFoundation\Session\SessionInterface;
1516

1617
/**
@@ -805,41 +806,34 @@ public function getClientIps()
805806
return array($ip);
806807
}
807808

808-
if (self::$trustedHeaders[self::HEADER_FORWARDED] && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED])) {
809+
$hasTrustedForwardedHeader = self::$trustedHeaders[self::HEADER_FORWARDED] && $this->headers->has(self::$trustedHeaders[self::HEADER_FORWARDED]);
810+
$hasTrustedClientIpHeader = self::$trustedHeaders[self::HEADER_CLIENT_IP] && $this->headers->has(self::$trustedHeaders[self::HEADER_CLIENT_IP]);
811+
812+
if ($hasTrustedForwardedHeader) {
809813
$forwardedHeader = $this->headers->get(self::$trustedHeaders[self::HEADER_FORWARDED]);
810814
preg_match_all('{(for)=("?\[?)([a-z0-9\.:_\-/]*)}', $forwardedHeader, $matches);
811-
$clientIps = $matches[3];
812-
} elseif (self::$trustedHeaders[self::HEADER_CLIENT_IP] && $this->headers->has(self::$trustedHeaders[self::HEADER_CLIENT_IP])) {
813-
$clientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
814-
}
815+
$forwardedClientIps = $matches[3];
815816

816-
$clientIps[] = $ip; // Complete the IP chain with the IP the request actually came from
817-
$firstTrustedIp = null;
818-
819-
foreach ($clientIps as $key => $clientIp) {
820-
// Remove port (unfortunately, it does happen)
821-
if (preg_match('{((?:\d+\.){3}\d+)\:\d+}', $clientIp, $match)) {
822-
$clientIps[$key] = $clientIp = $match[1];
823-
}
817+
$forwardedClientIps = $this->normalizeAndFilterClientIps($forwardedClientIps, $ip);
818+
$clientIps = $forwardedClientIps;
819+
}
824820

825-
if (!filter_var($clientIp, FILTER_VALIDATE_IP)) {
826-
unset($clientIps[$key]);
821+
if ($hasTrustedClientIpHeader) {
822+
$xForwardedForClientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
827823

828-
continue;
829-
}
824+
$xForwardedForClientIps = $this->normalizeAndFilterClientIps($xForwardedForClientIps, $ip);
825+
$clientIps = $xForwardedForClientIps;
826+
}
830827

831-
if (IpUtils::checkIp($clientIp, self::$trustedProxies)) {
832-
unset($clientIps[$key]);
828+
if ($hasTrustedForwardedHeader && $hasTrustedClientIpHeader && $forwardedClientIps !== $xForwardedForClientIps) {
829+
throw new ConflictingHeadersException('The request has both a trusted Forwarded header and a trusted Client IP header, conflicting with each other with regards to the originating IP addresses of the request. This is the result of a misconfiguration. You should either configure your proxy only to send one of these headers, or configure Symfony to distrust one of them.');
830+
}
833831

834-
// Fallback to this when the client IP falls into the range of trusted proxies
835-
if (null === $firstTrustedIp) {
836-
$firstTrustedIp = $clientIp;
837-
}
838-
}
832+
if (!$hasTrustedForwardedHeader && !$hasTrustedClientIpHeader) {
833+
return $this->normalizeAndFilterClientIps(array(), $ip);
839834
}
840835

841-
// Now the IP chain contains only untrusted proxies and the client IP
842-
return $clientIps ? array_reverse($clientIps) : array($firstTrustedIp);
836+
return $clientIps;
843837
}
844838

845839
/**
@@ -1930,4 +1924,35 @@ private function isFromTrustedProxy()
19301924
{
19311925
return self::$trustedProxies && IpUtils::checkIp($this->server->get('REMOTE_ADDR'), self::$trustedProxies);
19321926
}
1927+
1928+
private function normalizeAndFilterClientIps(array $clientIps, $ip)
1929+
{
1930+
$clientIps[] = $ip; // Complete the IP chain with the IP the request actually came from
1931+
$firstTrustedIp = null;
1932+
1933+
foreach ($clientIps as $key => $clientIp) {
1934+
// Remove port (unfortunately, it does happen)
1935+
if (preg_match('{((?:\d+\.){3}\d+)\:\d+}', $clientIp, $match)) {
1936+
$clientIps[$key] = $clientIp = $match[1];
1937+
}
1938+
1939+
if (!filter_var($clientIp, FILTER_VALIDATE_IP)) {
1940+
unset($clientIps[$key]);
1941+
1942+
continue;
1943+
}
1944+
1945+
if (IpUtils::checkIp($clientIp, self::$trustedProxies)) {
1946+
unset($clientIps[$key]);
1947+
1948+
// Fallback to this when the client IP falls into the range of trusted proxies
1949+
if (null === $firstTrustedIp) {
1950+
$firstTrustedIp = $clientIp;
1951+
}
1952+
}
1953+
}
1954+
1955+
// Now the IP chain contains only untrusted proxies and the client IP
1956+
return $clientIps ? array_reverse($clientIps) : array($firstTrustedIp);
1957+
}
19331958
}

‎src/Symfony/Component/HttpFoundation/Tests/RequestTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
+68Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,6 +923,74 @@ public function testGetClientIpsProvider()
923923
);
924924
}
925925

926+
/**
927+
* @expectedException \Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException
928+
* @dataProvider testGetClientIpsWithConflictingHeadersProvider
929+
*/
930+
public function testGetClientIpsWithConflictingHeaders($httpForwarded, $httpXForwardedFor)
931+
{
932+
$request = new Request();
933+
934+
$server = array(
935+
'REMOTE_ADDR' => '88.88.88.88',
936+
'HTTP_FORWARDED' => $httpForwarded,
937+
'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor,
938+
);
939+
940+
Request::setTrustedProxies(array('88.88.88.88'));
941+
942+
$request->initialize(array(), array(), array(), array(), array(), $server);
943+
944+
$request->getClientIps();
945+
}
946+
947+
public function testGetClientIpsWithConflictingHeadersProvider()
948+
{
949+
// $httpForwarded $httpXForwardedFor
950+
return array(
951+
array('for=87.65.43.21', '192.0.2.60'),
952+
array('for=87.65.43.21, for=192.0.2.60', '192.0.2.60'),
953+
array('for=192.0.2.60', '192.0.2.60,87.65.43.21'),
954+
array('for="::face", for=192.0.2.60', '192.0.2.60,192.0.2.43'),
955+
array('for=87.65.43.21, for=192.0.2.60', '192.0.2.60,87.65.43.21'),
956+
);
957+
}
958+
959+
/**
960+
* @dataProvider testGetClientIpsWithAgreeingHeadersProvider
961+
*/
962+
public function testGetClientIpsWithAgreeingHeaders($httpForwarded, $httpXForwardedFor)
963+
{
964+
$request = new Request();
965+
966+
$server = array(
967+
'REMOTE_ADDR' => '88.88.88.88',
968+
'HTTP_FORWARDED' => $httpForwarded,
969+
'HTTP_X_FORWARDED_FOR' => $httpXForwardedFor,
970+
);
971+
972+
Request::setTrustedProxies(array('88.88.88.88'));
973+
974+
$request->initialize(array(), array(), array(), array(), array(), $server);
975+
976+
$request->getClientIps();
977+
978+
Request::setTrustedProxies(array());
979+
}
980+
981+
public function testGetClientIpsWithAgreeingHeadersProvider()
982+
{
983+
// $httpForwarded $httpXForwardedFor
984+
return array(
985+
array('for="192.0.2.60"', '192.0.2.60'),
986+
array('for=192.0.2.60, for=87.65.43.21', '192.0.2.60,87.65.43.21'),
987+
array('for="[::face]", for=192.0.2.60', '::face,192.0.2.60'),
988+
array('for="192.0.2.60:80"', '192.0.2.60'),
989+
array('for=192.0.2.60;proto=http;by=203.0.113.43', '192.0.2.60'),
990+
array('for="[2001:db8:cafe::17]:4711"', '2001:db8:cafe::17'),
991+
);
992+
}
993+
926994
public function testGetContentWorksTwiceInDefaultMode()
927995
{
928996
$req = new Request();

‎src/Symfony/Component/HttpKernel/EventListener/ValidateRequestListener.php

Copy file name to clipboard
+56Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpKernel\EventListener;
13+
14+
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
15+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
16+
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
17+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
18+
use Symfony\Component\HttpKernel\KernelEvents;
19+
20+
/**
21+
* Validates that the headers and other information indicating the
22+
* client IP address of a request are consistent.
23+
*
24+
* @author Magnus Nordlander <magnus@fervo.se>
25+
*/
26+
class ValidateRequestListener implements EventSubscriberInterface
27+
{
28+
/**
29+
* Performs the validation.
30+
*
31+
* @param GetResponseEvent $event
32+
*/
33+
public function onKernelRequest(GetResponseEvent $event)
34+
{
35+
if ($event->isMasterRequest()) {
36+
try {
37+
// This will throw an exception if the headers are inconsistent.
38+
$event->getRequest()->getClientIps();
39+
} catch (ConflictingHeadersException $e) {
40+
throw new BadRequestHttpException('The request headers contain conflicting information regarding the origin of this request.', $e);
41+
}
42+
}
43+
}
44+
45+
/**
46+
* {@inheritdoc}
47+
*/
48+
public static function getSubscribedEvents()
49+
{
50+
return array(
51+
KernelEvents::REQUEST => array(
52+
array('onKernelRequest', 256),
53+
),
54+
);
55+
}
56+
}

‎src/Symfony/Component/HttpKernel/Profiler/Profiler.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/Profiler/Profiler.php
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpKernel\Profiler;
1313

14+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
1415
use Symfony\Component\HttpFoundation\Request;
1516
use Symfony\Component\HttpFoundation\Response;
1617
use Symfony\Component\HttpKernel\DataCollector\DataCollectorInterface;
@@ -200,9 +201,13 @@ public function collect(Request $request, Response $response, \Exception $except
200201
$profile = new Profile(substr(hash('sha256', uniqid(mt_rand(), true)), 0, 6));
201202
$profile->setTime(time());
202203
$profile->setUrl($request->getUri());
203-
$profile->setIp($request->getClientIp());
204204
$profile->setMethod($request->getMethod());
205205
$profile->setStatusCode($response->getStatusCode());
206+
try {
207+
$profile->setIp($request->getClientIp());
208+
} catch (ConflictingHeadersException $e) {
209+
$profile->setIp('Unknown');
210+
}
206211

207212
$response->headers->set('X-Debug-Token', $profile->getToken());
208213

‎src/Symfony/Component/HttpKernel/Tests/EventListener/ValidateRequestListenerTest.php

Copy file name to clipboard
+67Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpKernel\Tests\EventListener;
13+
14+
use Symfony\Component\EventDispatcher\EventDispatcher;
15+
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
16+
use Symfony\Component\HttpFoundation\Request;
17+
use Symfony\Component\HttpKernel\EventListener\ValidateRequestListener;
18+
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
19+
use Symfony\Component\HttpKernel\HttpKernelInterface;
20+
use Symfony\Component\HttpKernel\KernelEvents;
21+
22+
class ValidateRequestListenerTest extends \PHPUnit_Framework_TestCase
23+
{
24+
public function testListenerThrowsWhenMasterRequestHasInconsistentClientIps()
25+
{
26+
$dispatcher = new EventDispatcher();
27+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
28+
$listener = new ValidateRequestListener();
29+
$request = $this->getMock('Symfony\Component\HttpFoundation\Request');
30+
$request->method('getClientIps')
31+
->will($this->throwException(new ConflictingHeadersException()));
32+
33+
$dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
34+
$event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
35+
36+
$this->setExpectedException('Symfony\Component\HttpKernel\Exception\BadRequestHttpException');
37+
$dispatcher->dispatch(KernelEvents::REQUEST, $event);
38+
}
39+
40+
public function testListenerDoesNothingOnValidRequests()
41+
{
42+
$dispatcher = new EventDispatcher();
43+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
44+
$listener = new ValidateRequestListener();
45+
$request = $this->getMock('Symfony\Component\HttpFoundation\Request');
46+
$request->method('getClientIps')
47+
->willReturn(array('127.0.0.1'));
48+
49+
$dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
50+
$event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
51+
$dispatcher->dispatch(KernelEvents::REQUEST, $event);
52+
}
53+
54+
public function testListenerDoesNothingOnSubrequests()
55+
{
56+
$dispatcher = new EventDispatcher();
57+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
58+
$listener = new ValidateRequestListener();
59+
$request = $this->getMock('Symfony\Component\HttpFoundation\Request');
60+
$request->method('getClientIps')
61+
->will($this->throwException(new ConflictingHeadersException()));
62+
63+
$dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
64+
$event = new GetResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST);
65+
$dispatcher->dispatch(KernelEvents::REQUEST, $event);
66+
}
67+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.