diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php new file mode 100644 index 0000000000..0adc143b90 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSessionDomainConstraintPass.php @@ -0,0 +1,40 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler; + +use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; + +/** + * Uses the session domain to restrict allowed redirection targets. + * + * @author Nicolas Grekas
+ */ +class AddSessionDomainConstraintPass implements CompilerPassInterface +{ + /** + * {@inheritdoc} + */ + public function process(ContainerBuilder $container) + { + if (!$container->hasParameter('session.storage.options') || !$container->has('security.http_utils')) { + return; + } + + $sessionOptions = $container->getParameter('session.storage.options'); + $domainRegexp = empty($sessionOptions['cookie_domain']) ? '%s' : sprintf('(?:%%s|(?:.+\.)?%s)', preg_quote(trim($sessionOptions['cookie_domain'], '.'))); + $domainRegexp = (empty($sessionOptions['cookie_secure']) ? 'https?://' : 'https://').$domainRegexp; + + $container->findDefinition('security.http_utils')->addArgument(sprintf('{^%s$}i', $domainRegexp)); + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php index 72f7b68de9..9bcd504a9c 100644 --- a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php +++ b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php @@ -12,8 +12,10 @@ namespace Symfony\Bundle\SecurityBundle; use Symfony\Component\HttpKernel\Bundle\Bundle; +use Symfony\Component\DependencyInjection\Compiler\PassConfig; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass; +use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpBasicFactory; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\HttpDigestFactory; @@ -47,5 +49,6 @@ class SecurityBundle extends Bundle $extension->addUserProviderFactory(new InMemoryFactory()); $container->addCompilerPass(new AddSecurityVotersPass()); + $container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_AFTER_REMOVING); } } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php new file mode 100644 index 0000000000..f476b5ef73 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSessionDomainConstraintPassTest.php @@ -0,0 +1,131 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Compiler; + +use PHPUnit\Framework\TestCase; +use Symfony\Bundle\FrameworkBundle\DependencyInjection\FrameworkExtension; +use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass; +use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\HttpFoundation\Request; + +class AddSessionDomainConstraintPassTest extends TestCase +{ + public function testSessionCookie() + { + $container = $this->createContainer(array('cookie_domain' => '.symfony.com.', 'cookie_secure' => true)); + + $utils = $container->get('security.http_utils'); + $request = Request::create('/', 'get'); + + $this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/')); + $this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/')); + $this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/')); + } + + public function testSessionNoDomain() + { + $container = $this->createContainer(array('cookie_secure' => true)); + + $utils = $container->get('security.http_utils'); + $request = Request::create('/', 'get'); + + $this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('http://localhost/')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('http://localhost/')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/')); + $this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/')); + $this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/')); + } + + public function testSessionNoSecure() + { + $container = $this->createContainer(array('cookie_domain' => '.symfony.com.')); + + $utils = $container->get('security.http_utils'); + $request = Request::create('/', 'get'); + + $this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/')); + $this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://symfony.com/blog')); + $this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/')); + } + + public function testSessionNoSecureAndNoDomain() + { + $container = $this->createContainer(array()); + + $utils = $container->get('security.http_utils'); + $request = Request::create('/', 'get'); + + $this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('http://localhost/')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('http://localhost/')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo')); + $this->assertTrue($utils->createRedirectResponse($request, 'http://localhost/foo')->isRedirect('http://localhost/foo')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('http://localhost/')); + $this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://localhost/')); + $this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://localhost/')); + } + + public function testNoSession() + { + $container = $this->createContainer(null); + + $utils = $container->get('security.http_utils'); + $request = Request::create('/', 'get'); + + $this->assertTrue($utils->createRedirectResponse($request, 'https://symfony.com/blog')->isRedirect('https://symfony.com/blog')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://www.symfony.com/blog')->isRedirect('https://www.symfony.com/blog')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://localhost/foo')->isRedirect('https://localhost/foo')); + $this->assertTrue($utils->createRedirectResponse($request, 'https://www.localhost/foo')->isRedirect('https://www.localhost/foo')); + $this->assertTrue($utils->createRedirectResponse($request, 'http://symfony.com/blog')->isRedirect('http://symfony.com/blog')); + $this->assertTrue($utils->createRedirectResponse($request, 'http://pirate.com/foo')->isRedirect('http://pirate.com/foo')); + } + + private function createContainer($sessionStorageOptions) + { + $container = new ContainerBuilder(); + $container->setParameter('kernel.cache_dir', __DIR__); + $container->setParameter('kernel.charset', 'UTF-8'); + $container->setParameter('kernel.container_class', 'cc'); + $container->setParameter('kernel.debug', true); + $container->setParameter('kernel.root_dir', __DIR__); + $container->setParameter('kernel.secret', __DIR__); + if (null !== $sessionStorageOptions) { + $container->setParameter('session.storage.options', $sessionStorageOptions); + } + $container->setParameter('request_listener.http_port', 80); + $container->setParameter('request_listener.https_port', 443); + + $config = array( + 'security' => array( + 'providers' => array('some_provider' => array('id' => 'foo')), + 'firewalls' => array('some_firewall' => array('security' => false)), + ), + ); + + $ext = new FrameworkExtension(); + $ext->load(array(), $container); + + $ext = new SecurityExtension(); + $ext->load($config, $container); + + (new AddSessionDomainConstraintPass())->process($container); + + return $container; + } +} diff --git a/src/Symfony/Component/Security/Http/HttpUtils.php b/src/Symfony/Component/Security/Http/HttpUtils.php index f46af415b7..30071b880d 100644 --- a/src/Symfony/Component/Security/Http/HttpUtils.php +++ b/src/Symfony/Component/Security/Http/HttpUtils.php @@ -29,20 +29,23 @@ class HttpUtils { private $urlGenerator; private $urlMatcher; + private $domainRegexp; /** * @param UrlGeneratorInterface $urlGenerator A UrlGeneratorInterface instance * @param UrlMatcherInterface|RequestMatcherInterface $urlMatcher The URL or Request matcher + * @param string|null $domainRegexp A regexp that the target of HTTP redirections must match, scheme included * * @throws \InvalidArgumentException */ - public function __construct(UrlGeneratorInterface $urlGenerator = null, $urlMatcher = null) + public function __construct(UrlGeneratorInterface $urlGenerator = null, $urlMatcher = null, $domainRegexp = null) { $this->urlGenerator = $urlGenerator; if (null !== $urlMatcher && !$urlMatcher instanceof UrlMatcherInterface && !$urlMatcher instanceof RequestMatcherInterface) { throw new \InvalidArgumentException('Matcher must either implement UrlMatcherInterface or RequestMatcherInterface.'); } $this->urlMatcher = $urlMatcher; + $this->domainRegexp = $domainRegexp; } /** @@ -56,6 +59,10 @@ class HttpUtils */ public function createRedirectResponse(Request $request, $path, $status = 302) { + if (null !== $this->domainRegexp && preg_match('#^https?://[^/]++#i', $path, $host) && !preg_match(sprintf($this->domainRegexp, preg_quote($request->getHttpHost())), $host[0])) { + $path = '/'; + } + return new RedirectResponse($this->generateUri($request, $path), $status); } diff --git a/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php b/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php index b508012665..352f8977e7 100644 --- a/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php +++ b/src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php @@ -38,6 +38,38 @@ class HttpUtilsTest extends TestCase $this->assertTrue($response->isRedirect('http://symfony.com/')); } + public function testCreateRedirectResponseWithDomainRegexp() + { + $utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://symfony\.com$#i'); + $response = $utils->createRedirectResponse($this->getRequest(), 'http://symfony.com/blog'); + + $this->assertTrue($response->isRedirect('http://symfony.com/blog')); + } + + public function testCreateRedirectResponseWithRequestsDomain() + { + $utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://%s$#i'); + $response = $utils->createRedirectResponse($this->getRequest(), 'http://localhost/blog'); + + $this->assertTrue($response->isRedirect('http://localhost/blog')); + } + + public function testCreateRedirectResponseWithBadRequestsDomain() + { + $utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://%s$#i'); + $response = $utils->createRedirectResponse($this->getRequest(), 'http://pirate.net/foo'); + + $this->assertTrue($response->isRedirect('http://localhost/')); + } + + public function testCreateRedirectResponseWithProtocolRelativeTarget() + { + $utils = new HttpUtils($this->getUrlGenerator(), null, '#^https?://%s$#i'); + $response = $utils->createRedirectResponse($this->getRequest(), '//evil.com/do-bad-things'); + + $this->assertTrue($response->isRedirect('http://localhost//evil.com/do-bad-things'), 'Protocol-relative redirection should not be supported for security reasons'); + } + public function testCreateRedirectResponseWithRouteName() { $utils = new HttpUtils($urlGenerator = $this->getMockBuilder('Symfony\Component\Routing\Generator\UrlGeneratorInterface')->getMock());