diff --git a/eZ/Bundle/EzPublishCoreBundle/DependencyInjection/Compiler/SecurityPass.php b/eZ/Bundle/EzPublishCoreBundle/DependencyInjection/Compiler/SecurityPass.php index e79a27bf81..cf8396083c 100644 --- a/eZ/Bundle/EzPublishCoreBundle/DependencyInjection/Compiler/SecurityPass.php +++ b/eZ/Bundle/EzPublishCoreBundle/DependencyInjection/Compiler/SecurityPass.php @@ -23,6 +23,10 @@ */ class SecurityPass implements CompilerPassInterface { + public const CONSTANT_AUTH_TIME_SETTING = 'ibexa.security.authentication.constant_auth_time'; + + public const CONSTANT_AUTH_TIME_DEFAULT = 1.0; + public function process(ContainerBuilder $container) { if (!($container->hasDefinition('security.authentication.provider.dao') && @@ -34,6 +38,7 @@ public function process(ContainerBuilder $container) $configResolverRef = new Reference('ezpublish.config.resolver'); $permissionResolverRef = new Reference(PermissionResolver::class); $userServiceRef = new Reference(UserService::class); + $loggerRef = new Reference('logger'); // Override and inject the Repository in the authentication provider. // We need it for checking user credentials @@ -47,6 +52,18 @@ public function process(ContainerBuilder $container) 'setUserService', [$userServiceRef] ); + $daoAuthenticationProviderDef->addMethodCall( + 'setConstantAuthTime', + [ + $container->hasParameter(self::CONSTANT_AUTH_TIME_SETTING) ? + (float)$container->getParameter(self::CONSTANT_AUTH_TIME_SETTING) : + self::CONSTANT_AUTH_TIME_DEFAULT, + ] + ); + $daoAuthenticationProviderDef->addMethodCall( + 'setLogger', + [$loggerRef] + ); $rememberMeAuthenticationProviderDef = $container->findDefinition('security.authentication.provider.rememberme'); $rememberMeAuthenticationProviderDef->setClass(RememberMeRepositoryAuthenticationProvider::class); diff --git a/eZ/Bundle/EzPublishCoreBundle/Resources/config/security.yml b/eZ/Bundle/EzPublishCoreBundle/Resources/config/security.yml index d3fd73ce0f..0f706baffe 100644 --- a/eZ/Bundle/EzPublishCoreBundle/Resources/config/security.yml +++ b/eZ/Bundle/EzPublishCoreBundle/Resources/config/security.yml @@ -1,3 +1,9 @@ +parameters: + # Constant authentication execution time in seconds (float). Blocks timing attacks. + # Must be larger than expected real execution time, with a good margin. + # If set to zero, constant time authentication is disabled. Do not do this on production environments. + ibexa.security.authentication.constant_auth_time: !php/const eZ\Bundle\EzPublishCoreBundle\DependencyInjection\Compiler\SecurityPass::CONSTANT_AUTH_TIME_DEFAULT + services: ezpublish.security.user_provider.username: class: eZ\Publish\Core\MVC\Symfony\Security\User\UsernameProvider diff --git a/eZ/Publish/Core/MVC/Symfony/Security/Authentication/RepositoryAuthenticationProvider.php b/eZ/Publish/Core/MVC/Symfony/Security/Authentication/RepositoryAuthenticationProvider.php index 6b3192fe46..a472a33eac 100644 --- a/eZ/Publish/Core/MVC/Symfony/Security/Authentication/RepositoryAuthenticationProvider.php +++ b/eZ/Publish/Core/MVC/Symfony/Security/Authentication/RepositoryAuthenticationProvider.php @@ -6,25 +6,38 @@ */ namespace eZ\Publish\Core\MVC\Symfony\Security\Authentication; +use eZ\Bundle\EzPublishCoreBundle\DependencyInjection\Compiler\SecurityPass; use eZ\Publish\API\Repository\Exceptions\PasswordInUnsupportedFormatException; use eZ\Publish\API\Repository\PermissionResolver; use eZ\Publish\API\Repository\UserService; use eZ\Publish\Core\MVC\Symfony\Security\UserInterface as EzUserInterface; use eZ\Publish\Core\Repository\User\Exception\UnsupportedPasswordHashType; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerAwareTrait; use Symfony\Component\Security\Core\Authentication\Provider\DaoAuthenticationProvider; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\User\UserInterface; -class RepositoryAuthenticationProvider extends DaoAuthenticationProvider +class RepositoryAuthenticationProvider extends DaoAuthenticationProvider implements LoggerAwareInterface { + use LoggerAwareTrait; + + /** @var float|null */ + private $constantAuthTime; + /** @var \eZ\Publish\API\Repository\PermissionResolver */ private $permissionResolver; /** @var \eZ\Publish\API\Repository\UserService */ private $userService; + public function setConstantAuthTime(float $constantAuthTime) + { + $this->constantAuthTime = $constantAuthTime; + } + public function setPermissionResolver(PermissionResolver $permissionResolver) { $this->permissionResolver = $permissionResolver; @@ -71,10 +84,45 @@ protected function checkAuthentication(UserInterface $user, UsernamePasswordToke */ public function authenticate(TokenInterface $token) { + $startTime = $this->startConstantTimer(); + try { - return parent::authenticate($token); + $result = parent::authenticate($token); } catch (UnsupportedPasswordHashType $exception) { + $this->sleepUsingConstantTimer($startTime); throw new PasswordInUnsupportedFormatException($exception); + } catch (\Exception $e) { + $this->sleepUsingConstantTimer($startTime); + throw $e; + } + + $this->sleepUsingConstantTimer($startTime); + + return $result; + } + + private function startConstantTimer() + { + return microtime(true); + } + + private function sleepUsingConstantTimer(float $startTime): void + { + if ($this->constantAuthTime <= 0.0) { + return; + } + + $remainingTime = $this->constantAuthTime - (microtime(true) - $startTime); + if ($remainingTime > 0) { + usleep($remainingTime * 1000000); + } elseif ($this->logger) { + $this->logger->warning( + sprintf( + 'Authentication took longer than the configured constant time. Consider increasing the value of %s', + SecurityPass::CONSTANT_AUTH_TIME_SETTING + ), + [static::class] + ); } } } diff --git a/eZ/Publish/Core/MVC/Symfony/Security/Tests/Authentication/RepositoryAuthenticationProviderTest.php b/eZ/Publish/Core/MVC/Symfony/Security/Tests/Authentication/RepositoryAuthenticationProviderTest.php index 51e72b8e89..13245f7347 100644 --- a/eZ/Publish/Core/MVC/Symfony/Security/Tests/Authentication/RepositoryAuthenticationProviderTest.php +++ b/eZ/Publish/Core/MVC/Symfony/Security/Tests/Authentication/RepositoryAuthenticationProviderTest.php @@ -6,6 +6,7 @@ */ namespace eZ\Publish\Core\MVC\Symfony\Security\Tests\Authentication; +use eZ\Bundle\EzPublishCoreBundle\DependencyInjection\Compiler\SecurityPass; use eZ\Publish\API\Repository\Exceptions\PasswordInUnsupportedFormatException; use eZ\Publish\API\Repository\PermissionResolver; use eZ\Publish\API\Repository\UserService; @@ -14,11 +15,14 @@ use eZ\Publish\Core\MVC\Symfony\Security\User; use eZ\Publish\Core\Repository\User\Exception\UnsupportedPasswordHashType; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface; +use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; +use Symfony\Component\Stopwatch\Stopwatch; class RepositoryAuthenticationProviderTest extends TestCase { @@ -41,6 +45,9 @@ class RepositoryAuthenticationProviderTest extends TestCase /** @var \eZ\Publish\API\Repository\UserService|\PHPUnit\Framework\MockObject\MockObject */ private $userService; + /** @var \Psr\Log\LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ + private $logger; + protected function setUp(): void { parent::setUp(); @@ -56,6 +63,9 @@ protected function setUp(): void $this->userService = $this->createMock(UserService::class); $this->authProvider->setPermissionResolver($this->permissionResolver); $this->authProvider->setUserService($this->userService); + + $this->logger = $this->createMock(LoggerInterface::class); + $this->authProvider->setLogger($this->logger); } public function testAuthenticationNotEzUser() @@ -209,4 +219,52 @@ public function testCheckAuthenticationFailedWhenPasswordInUnsupportedFormat() $this->authProvider->authenticate($token); } + + public function testAuthenticateInConstantTime(): void + { + $this->authProvider->setConstantAuthTime(SecurityPass::CONSTANT_AUTH_TIME_DEFAULT); // a reasonable value + + $token = new UsernamePasswordToken('my_username', 'my_password', 'bar'); + + $stopwatch = new Stopwatch(); + $stopwatch->start('authenticate_constant_time_test'); + + try { + $this->authProvider->authenticate($token); + } catch (\Exception $e) { + // We don't care, we just need test execution to continue + } + + $duration = $stopwatch->stop('authenticate_constant_time_test')->getDuration(); + self::assertGreaterThanOrEqual(SecurityPass::CONSTANT_AUTH_TIME_DEFAULT * 1000, $duration); + } + + public function testAuthenticateWarningOnConstantTimeExceeded(): void + { + $this->authProvider->setConstantAuthTime(0.0000001); // much too short, but not zero, which would disable the check + + $token = new UsernamePasswordToken('my_username', 'my_password', 'bar'); + + $this->logger + ->expects(self::atLeastOnce()) + ->method('warning') + ->with('Authentication took longer than the configured constant time. Consider increasing the value of ' . SecurityPass::CONSTANT_AUTH_TIME_SETTING); + + $this->expectException(AuthenticationException::class); + $this->authProvider->authenticate($token); + } + + public function testAuthenticateConstantTimeDisabled(): void + { + $this->authProvider->setConstantAuthTime(0.0); // zero disables the check + + $token = new UsernamePasswordToken('my_username', 'my_password', 'bar'); + + $this->logger + ->expects(self::never()) + ->method('warning'); + + $this->expectException(AuthenticationException::class); + $this->authProvider->authenticate($token); + } } diff --git a/eZ/Publish/Core/Repository/User/PasswordHashService.php b/eZ/Publish/Core/Repository/User/PasswordHashService.php index 10f1b32a4f..450d4d2921 100644 --- a/eZ/Publish/Core/Repository/User/PasswordHashService.php +++ b/eZ/Publish/Core/Repository/User/PasswordHashService.php @@ -65,9 +65,6 @@ public function isValidPassword(string $plainPassword, string $passwordHash, ?in return password_verify($plainPassword, $passwordHash); } - // Randomize login time to protect against timing attacks - usleep(random_int(0, 30000)); - return $passwordHash === $this->createPasswordHash($plainPassword, $hashType); } }