IBX-1755: Implemented constant time authentication

eZ/Bundle/EzPublishCoreBundle/DependencyInjection/Compiler/SecurityPass.php

@@ -21,6 +21,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') &&
@@ -31,6 +35,7 @@ public function process(ContainerBuilder $container)
 
         $configResolverRef = new Reference('ezpublish.config.resolver');
         $repositoryReference = new Reference('ezpublish.api.repository');
+        $loggerReference = new Reference('logger');
 
         // Override and inject the Repository in the authentication provider.
         // We need it for checking user credentials
@@ -40,6 +45,18 @@ public function process(ContainerBuilder $container)
             'setRepository',
             [$repositoryReference]
         );
+        $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',
+            [$loggerReference]
+        );
 
         $rememberMeAuthenticationProviderDef = $container->findDefinition('security.authentication.provider.rememberme');
         $rememberMeAuthenticationProviderDef->setClass(RememberMeRepositoryAuthenticationProvider::class);

eZ/Bundle/EzPublishCoreBundle/Resources/config/security.yml

@@ -5,6 +5,11 @@ parameters:
     ezpublish.security.controller.class: eZ\Publish\Core\MVC\Symfony\Controller\SecurityController
     ezpublish.security.login_listener.class: eZ\Publish\Core\MVC\Symfony\Security\EventListener\SecurityListener
 
+    # 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:
         class: "%ezpublish.security.user_provider.class%"

eZ/Bundle/EzPublishRestBundle/Resources/config/default_settings.yml

@@ -84,7 +84,3 @@ parameters:
         refreshSession:
             mediaType: 'UserSession'
             href: 'templateRouter.generate("ezpublish_rest_refreshSession", {sessionId: "{sessionId}"})'
-
-    # Boundary times in microseconds which the authentication check will be delayed by.
-    ezpublish_rest.authentication_min_delay_time: 30000
-    ezpublish_rest.authentication_max_delay_time: 500000

eZ/Bundle/EzPublishRestBundle/Resources/config/security.yml

@@ -15,8 +15,6 @@ services:
             - "@ezpublish.config.resolver"
             - "@session.storage"
             - "@?logger"
-            - "%ezpublish_rest.authentication_min_delay_time%"
-            - "%ezpublish_rest.authentication_max_delay_time%"
         abstract: true
 
     ezpublish_rest.security.authentication.logout_handler:

eZ/Publish/Core/MVC/Symfony/Security/Authentication/RepositoryAuthenticationProvider.php

@@ -6,18 +6,32 @@
  */
 namespace eZ\Publish\Core\MVC\Symfony\Security\Authentication;
 
+use eZ\Bundle\EzPublishCoreBundle\DependencyInjection\Compiler\SecurityPass;
 use eZ\Publish\API\Repository\Repository;
 use eZ\Publish\Core\MVC\Symfony\Security\UserInterface as EzUserInterface;
+use JMS\TranslationBundle\Logger\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\Repository */
     private $repository;
 
+    public function setConstantAuthTime(float $constantAuthTime)
+    {
+        $this->constantAuthTime = $constantAuthTime;
+    }
+
     public function setRepository(Repository $repository)
     {
         $this->repository = $repository;
@@ -51,4 +65,45 @@ protected function checkAuthentication(UserInterface $user, UsernamePasswordToke
         // Finally inject current user in the Repository
         $this->repository->setCurrentUser($apiUser);
     }
+
+    public function authenticate(TokenInterface $token)
+    {
+        $startTime = $this->startConstantTimer();
+
+        try {
+            $result = parent::authenticate($token);
+        } 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
+                ),
+                [get_class($this)]
+            );
+        }
+    }
 }

eZ/Publish/Core/MVC/Symfony/Security/Tests/Authentication/RepositoryAuthenticationProviderTest.php

@@ -6,17 +6,21 @@
  */
 namespace eZ\Publish\Core\MVC\Symfony\Security\Tests\Authentication;
 
+use eZ\Bundle\EzPublishCoreBundle\DependencyInjection\Compiler\SecurityPass;
 use eZ\Publish\API\Repository\Repository;
 use eZ\Publish\API\Repository\UserService;
 use eZ\Publish\API\Repository\Values\User\User as APIUser;
 use eZ\Publish\Core\MVC\Symfony\Security\Authentication\RepositoryAuthenticationProvider;
 use PHPUnit\Framework\TestCase;
 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\UserProviderInterface;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Core\User\UserCheckerInterface;
+use Symfony\Component\Stopwatch\Stopwatch;
 use eZ\Publish\Core\MVC\Symfony\Security\User;
+use Psr\Log\LoggerInterface;
 
 class RepositoryAuthenticationProviderTest extends TestCase
 {
@@ -29,6 +33,9 @@ class RepositoryAuthenticationProviderTest extends TestCase
     /** @var \PHPUnit\Framework\MockObject\MockObject|\eZ\Publish\API\Repository\Repository */
     private $repository;
 
+    /** @var \Psr\Log\LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
+    private $logger;
+
     protected function setUp()
     {
         parent::setUp();
@@ -41,6 +48,9 @@ protected function setUp()
             $this->encoderFactory
         );
         $this->authProvider->setRepository($repository);
+
+        $this->logger = $this->createMock(LoggerInterface::class);
+        $this->authProvider->setLogger($this->logger);
     }
 
     public function testAuthenticationNotEzUser()
@@ -181,4 +191,52 @@ public function testCheckAuthentication()
         $method->setAccessible(true);
         $method->invoke($this->authProvider, $user, $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();
+        $this->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($this->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($this->never())
+            ->method('warning');
+
+        $this->expectException(AuthenticationException::class);
+        $this->authProvider->authenticate($token);
+    }
 }

eZ/Publish/Core/REST/Server/Security/RestAuthenticator.php

@@ -36,10 +36,6 @@
  */
 class RestAuthenticator implements ListenerInterface, AuthenticatorInterface
 {
-    const DEFAULT_MIN_SLEEP_VALUE = 30000;
-
-    const DEFAULT_MAX_SLEEP_VALUE = 500000;
-
     /** @var \Psr\Log\LoggerInterface */
     private $logger;
 
@@ -63,26 +59,14 @@ class RestAuthenticator implements ListenerInterface, AuthenticatorInterface
     /** @var \Symfony\Component\Security\Http\Logout\LogoutHandlerInterface[] */
     private $logoutHandlers = [];
 
-    /**
-     * @var int|null
-     */
-    private $minSleepTime;
-
-    /**
-     * @var int|null
-     */
-    private $maxSleepTime;
-
     public function __construct(
         TokenStorageInterface $tokenStorage,
         AuthenticationManagerInterface $authenticationManager,
         $providerKey,
         EventDispatcherInterface $dispatcher,
         ConfigResolverInterface $configResolver,
         SessionStorageInterface $sessionStorage,
-        LoggerInterface $logger = null,
-        $minSleepTime = self::DEFAULT_MIN_SLEEP_VALUE,
-        $maxSleepTime = self::DEFAULT_MAX_SLEEP_VALUE
+        ?LoggerInterface $logger = null
     ) {
         $this->tokenStorage = $tokenStorage;
         $this->authenticationManager = $authenticationManager;
@@ -91,8 +75,6 @@ public function __construct(
         $this->configResolver = $configResolver;
         $this->sessionStorage = $sessionStorage;
         $this->logger = $logger;
-        $this->minSleepTime = !is_int($minSleepTime) ? self::DEFAULT_MIN_SLEEP_VALUE : $minSleepTime;
-        $this->maxSleepTime = !is_int($maxSleepTime) ? self::DEFAULT_MAX_SLEEP_VALUE : $maxSleepTime;
     }
 
     /**
@@ -107,8 +89,6 @@ public function handle(GetResponseEvent $event)
 
     public function authenticate(Request $request)
     {
-        usleep(random_int($this->minSleepTime, $this->maxSleepTime));
-
         // If a token already exists and username is the same as the one we request authentication for,
         // then return it and mark it as coming from session.
         $previousToken = $this->tokenStorage->getToken();

eZ/Publish/Core/Repository/UserService.php

@@ -1507,9 +1507,6 @@ private function comparePasswordHashes(
             return password_verify($plainPassword, $passwordHash);
         }
 
-        // Randomize login time to protect against timing attacks
-        usleep(random_int(0, 30000));
-
         return $passwordHash === $this->createPasswordHash(
             $login,
             $plainPassword,