From 932cd10477a5fa507df4171ebc97323f8715c5a6 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 5 Jul 2011 19:38:29 +0200 Subject: [PATCH] made HTTP headers coming from proxies non-trusted by default --- UPDATE.md | 12 +++++++++++ .../DependencyInjection/Configuration.php | 1 + .../FrameworkExtension.php | 2 ++ .../FrameworkBundle/FrameworkBundle.php | 9 +++++++- .../Component/HttpFoundation/Request.php | 21 +++++++++++++++---- .../Component/HttpFoundation/RequestTest.php | 5 +++++ 6 files changed, 45 insertions(+), 5 deletions(-) diff --git a/UPDATE.md b/UPDATE.md index c4d68c830fda..e9054c67d411 100644 --- a/UPDATE.md +++ b/UPDATE.md @@ -6,8 +6,20 @@ one. It only discusses changes that need to be done when using the "public" API of the framework. If you "hack" the core, you should probably follow the timeline closely anyway. +RC4 to RC5 +---------- + +* To avoid security issues, HTTP headers coming from proxies are not trusted + anymore by default (like `HTTP_X_FORWARDED_FOR`, `X_FORWARDED_PROTO`, and + `X_FORWARDED_HOST`). If your application is behind a reverse proxy, add the + following configuration: + + framework: + proxy: true + RC3 to RC4 ---------- + * Annotation classes must be annotated with @Annotation (see the validator constraints for examples) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 46db74c90d82..6ff788920147 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -47,6 +47,7 @@ public function getConfigTreeBuilder() $rootNode ->children() ->scalarNode('charset')->end() + ->scalarNode('proxy')->defaultFalse()->end() ->scalarNode('secret')->isRequired()->end() ->scalarNode('exception_controller')->defaultValue('Symfony\\Bundle\\FrameworkBundle\\Controller\\ExceptionController::showAction')->end() ->scalarNode('ide')->defaultNull()->end() diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index d565a18df477..d9e3f4956c2b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -66,6 +66,8 @@ public function load(array $configs, ContainerBuilder $container) $container->setParameter('kernel.secret', $config['secret']); $container->setParameter('exception_listener.controller', $config['exception_controller']); + $container->setParameter('kernel.proxy', $config['proxy']); + if (!empty($config['test'])) { $loader->load('test.xml'); } diff --git a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php index fd0e53cda0ba..3800c0374586 100644 --- a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php +++ b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php @@ -24,7 +24,7 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\Scope; -use Symfony\Component\HttpFoundation\File\File; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Bundle\Bundle; /** @@ -34,6 +34,13 @@ */ class FrameworkBundle extends Bundle { + public function boot() + { + if ($this->container->getParameter('kernel.proxy')) { + Request::trustProxyData(); + } + } + public function build(ContainerBuilder $container) { parent::build($container); diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index 197e652cc44e..24bac40c65a8 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/src/Symfony/Component/HttpFoundation/Request.php @@ -20,6 +20,8 @@ */ class Request { + static protected $trustProxy = false; + /** * @var \Symfony\Component\HttpFoundation\ParameterBag */ @@ -322,6 +324,17 @@ public function overrideGlobals() $_REQUEST = array_merge($_GET, $_POST); } + /** + * Trusts $_SERVER entries coming from proxies. + * + * You should only call this method if your application + * is hosted behind a reverse proxy that you manage. + */ + static public function trustProxyData() + { + self::$trustProxy = true; + } + /** * Gets a "parameter" value. * @@ -397,7 +410,7 @@ public function getClientIp($proxy = false) if ($proxy) { if ($this->server->has('HTTP_CLIENT_IP')) { return $this->server->get('HTTP_CLIENT_IP'); - } elseif ($this->server->has('HTTP_X_FORWARDED_FOR')) { + } elseif (self::$trustProxy && $this->server->has('HTTP_X_FORWARDED_FOR')) { return $this->server->get('HTTP_X_FORWARDED_FOR'); } } @@ -600,9 +613,9 @@ public function isSecure() return ( (strtolower($this->server->get('HTTPS')) == 'on' || $this->server->get('HTTPS') == 1) || - (strtolower($this->headers->get('SSL_HTTPS')) == 'on' || $this->headers->get('SSL_HTTPS') == 1) + (self::$trustProxy && strtolower($this->headers->get('SSL_HTTPS')) == 'on' || $this->headers->get('SSL_HTTPS') == 1) || - (strtolower($this->headers->get('X_FORWARDED_PROTO')) == 'https') + (self::$trustProxy && strtolower($this->headers->get('X_FORWARDED_PROTO')) == 'https') ); } @@ -613,7 +626,7 @@ public function isSecure() */ public function getHost() { - if ($host = $this->headers->get('X_FORWARDED_HOST')) { + if (self::$trustProxy && $host = $this->headers->get('X_FORWARDED_HOST')) { $elements = explode(',', $host); $host = trim($elements[count($elements) - 1]); diff --git a/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php b/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php index f5e5ad27cc06..ad11a7d21dfb 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php @@ -21,6 +21,11 @@ class RequestTest extends \PHPUnit_Framework_TestCase { + public function setUp() + { + Request::trustProxyData(); + } + /** * @covers Symfony\Component\HttpFoundation\Request::__construct */