Added a listener to validate requests

symfony · magnusnordlander · May 2, 2016 · May 3, 2016 · May 5, 2016 · May 5, 2016
commit c0172900e5a069d6b26b4f498fbdeb3055853bb1
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml
@@ -46,5 +46,9 @@
             <argument type="service" id="request_stack" />
             <tag name="kernel.event_subscriber" />
         </service>
+
+        <service id="validate_request_client_ip_listener" class="Symfony\Component\HttpKernel\EventListener\ValidateRequestClientIpListener">
+            <tag name="kernel.event_subscriber" />
+        </service>
     </services>
 </container>
diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php
@@ -827,7 +827,9 @@ public function getClientIps()
 
         if ($hasTrustedForwardedHeader && $hasTrustedClientIpHeader && $forwardedClientIps !== $xForwardedForClientIps) {
             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.');
-        } elseif (!$hasTrustedForwardedHeader && !$hasTrustedClientIpHeader) {
+        }
+
+        if (!$hasTrustedForwardedHeader && !$hasTrustedClientIpHeader) {
             return $this->normalizeAndFilterClientIps(array(), $ip);
         }
 
@@ -1923,7 +1925,7 @@ private function isFromTrustedProxy()
         return self::$trustedProxies && IpUtils::checkIp($this->server->get('REMOTE_ADDR'), self::$trustedProxies);
     }
 
-    private function normalizeAndFilterClientIps($clientIps, $ip)
+    private function normalizeAndFilterClientIps(array $clientIps, $ip)
     {
         $clientIps[] = $ip; // Complete the IP chain with the IP the request actually came from
         $firstTrustedIp = null;
@@ -1944,7 +1946,7 @@ private function normalizeAndFilterClientIps($clientIps, $ip)
                 unset($clientIps[$key]);
 
                 // Fallback to this when the client IP falls into the range of trusted proxies
-                if (null ===  $firstTrustedIp) {
+                if (null === $firstTrustedIp) {
                     $firstTrustedIp = $clientIp;
                 }
             }

diff --git a/src/Symfony/Component/HttpKernel/EventListener/ValidateRequestClientIpListener.php b/src/Symfony/Component/HttpKernel/EventListener/ValidateRequestClientIpListener.php
@@ -0,0 +1,54 @@
+<?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\HttpException;
+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 ValidateRequestClientIpListener implements EventSubscriberInterface
+{
+    /**
+     * Performs the validation
+     *
+     * @param GetResponseEvent $event
+     */
+    public function onKernelRequest(GetResponseEvent $event)
+    {
+        try {
+            // This will throw an exception if the headers are inconsistent.
+            $event->getRequest()->getClientIps();
+        } catch (ConflictingHeadersException $e) {
+            throw new HttpException(400, "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),
+            ),
+        );
+    }
+}
diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/ValidateRequestClientIpTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/ValidateRequestClientIpTest.php
@@ -0,0 +1,53 @@
+<?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\ValidateRequestClientIpListener;
+use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\HttpKernel\KernelEvents;
+
+class ValidateRequestClientIpTest extends \PHPUnit_Framework_TestCase
+{
+    public function testListenerThrowsOnInconsistentRequests()
+    {
+        $dispatcher = new EventDispatcher();
+        $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
+        $listener = new ValidateRequestClientIpListener();
+        $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);
+
+        $this->setExpectedException('Symfony\Component\HttpKernel\Exception\HttpException');
+        $dispatcher->dispatch(KernelEvents::REQUEST, $event);
+    }
+
+    public function testListenerDoesNothingOnConsistenRequests()
+    {
+        $dispatcher = new EventDispatcher();
+        $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
+        $listener = new ValidateRequestClientIpListener();
+        $request = $this->getMock('Symfony\Component\HttpFoundation\Request');
+        $request->method('getClientIps')
+            ->willReturn(['127.0.0.1']);
+
+        $dispatcher->addListener(KernelEvents::REQUEST, array($listener, 'onKernelRequest'));
+        $event = new GetResponseEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST);
+        $dispatcher->dispatch(KernelEvents::REQUEST, $event);
+    }
+}