bug #18688 [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For (magnusnordlander)

This PR was squashed before being merged into the 2.7 branch (closes #18688).

Discussion
----------

[HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        | symfony/symfony-docs#6526

Emit a warning when a request has both a trusted Forwarded header and a trusted X-Forwarded-For header, as this is most likely a misconfiguration which causes security issues.

Commits
-------

ee8842f [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For

Author: fabpot

EventListener/ValidateRequestListener.php

@@ -0,0 +1,56 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpKernel\EventListener;
+
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
+use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
+use Symfony\Component\HttpKernel\KernelEvents;
+
+/**
+ * Validates that the headers and other information indicating the
+ * client IP address of a request are consistent.
+ *
+ * @author Magnus Nordlander <magnus@fervo.se>
+ */
+class ValidateRequestListener implements EventSubscriberInterface
+{
+    /**
+     * Performs the validation.
+     *
+     * @param GetResponseEvent $event
+     */
+    public function onKernelRequest(GetResponseEvent $event)
+    {
+        if ($event->isMasterRequest()) {
+            try {
+                // This will throw an exception if the headers are inconsistent.
+                $event->getRequest()->getClientIps();
+            } catch (ConflictingHeadersException $e) {
+                throw new BadRequestHttpException('The request headers contain conflicting information regarding the origin of this request.', $e);
+            }
+        }
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public static function getSubscribedEvents()
+    {
+        return array(
+            KernelEvents::REQUEST => array(
+                array('onKernelRequest', 256),
+            ),
+        );
+    }
+}

Profiler/Profiler.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\HttpKernel\Profiler;
 
+use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\DataCollector\DataCollectorInterface;
@@ -200,9 +201,13 @@ public function collect(Request $request, Response $response, \Exception $except
         $profile = new Profile(substr(hash('sha256', uniqid(mt_rand(), true)), 0, 6));
         $profile->setTime(time());
         $profile->setUrl($request->getUri());
-        $profile->setIp($request->getClientIp());
         $profile->setMethod($request->getMethod());
         $profile->setStatusCode($response->getStatusCode());
+        try {
+            $profile->setIp($request->getClientIp());
+        } catch (ConflictingHeadersException $e) {
+            $profile->setIp('Unknown');
+        }
 
         $response->headers->set('X-Debug-Token', $profile->getToken());
 

Tests/EventListener/ValidateRequestListenerTest.php

@@ -0,0 +1,67 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpKernel\Tests\EventListener;
+
+use Symfony\Component\EventDispatcher\EventDispatcher;
+use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\EventListener\ValidateRequestListener;
+use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\HttpKernel\KernelEvents;
+
+class ValidateRequestListenerTest extends \PHPUnit_Framework_TestCase
+{
+    public function testListenerThrowsWhenMasterRequestHasInconsistentClientIps()
+    {
+        $dispatcher = new EventDispatcher();
+        $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
+        $listener = new ValidateRequestListener();
+        $request = $this->getMock('Symfony\Component\HttpFoundation\Request');
+        $request->method('getClientIps')
+            ->will($this->throwException(new ConflictingHeadersException()));
+
+        $dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
+        $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
+
+        $this->setExpectedException('Symfony\Component\HttpKernel\Exception\BadRequestHttpException');
+        $dispatcher->dispatch(KernelEvents::REQUEST, $event);
+    }
+
+    public function testListenerDoesNothingOnValidRequests()
+    {
+        $dispatcher = new EventDispatcher();
+        $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
+        $listener = new ValidateRequestListener();
+        $request = $this->getMock('Symfony\Component\HttpFoundation\Request');
+        $request->method('getClientIps')
+            ->willReturn(array('127.0.0.1'));
+
+        $dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
+        $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST);
+        $dispatcher->dispatch(KernelEvents::REQUEST, $event);
+    }
+
+    public function testListenerDoesNothingOnSubrequests()
+    {
+        $dispatcher = new EventDispatcher();
+        $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
+        $listener = new ValidateRequestListener();
+        $request = $this->getMock('Symfony\Component\HttpFoundation\Request');
+        $request->method('getClientIps')
+            ->will($this->throwException(new ConflictingHeadersException()));
+
+        $dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
+        $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST);
+        $dispatcher->dispatch(KernelEvents::REQUEST, $event);
+    }
+}