security #cve-2021-21424 [Security][Guard] Prevent user enumeration (chalasr)
This PR was merged into the 3.4 branch.
This commit is contained in:
commit
2a581d22cc
@ -41,6 +41,7 @@
|
|||||||
<argument /> <!-- Provider-shared Key -->
|
<argument /> <!-- Provider-shared Key -->
|
||||||
<argument /> <!-- Authenticator -->
|
<argument /> <!-- Authenticator -->
|
||||||
<argument type="service" id="logger" on-invalid="null" />
|
<argument type="service" id="logger" on-invalid="null" />
|
||||||
|
<argument>%security.authentication.hide_user_not_found%</argument>
|
||||||
</service>
|
</service>
|
||||||
</services>
|
</services>
|
||||||
</container>
|
</container>
|
||||||
|
@ -13,6 +13,7 @@ namespace Symfony\Component\Security\Core\Authentication\Provider;
|
|||||||
|
|
||||||
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
|
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
|
||||||
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
|
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
|
||||||
|
use Symfony\Component\Security\Core\Exception\AccountStatusException;
|
||||||
use Symfony\Component\Security\Core\Exception\AuthenticationException;
|
use Symfony\Component\Security\Core\Exception\AuthenticationException;
|
||||||
use Symfony\Component\Security\Core\Exception\AuthenticationServiceException;
|
use Symfony\Component\Security\Core\Exception\AuthenticationServiceException;
|
||||||
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
|
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
|
||||||
@ -83,7 +84,7 @@ abstract class UserAuthenticationProvider implements AuthenticationProviderInter
|
|||||||
$this->userChecker->checkPreAuth($user);
|
$this->userChecker->checkPreAuth($user);
|
||||||
$this->checkAuthentication($user, $token);
|
$this->checkAuthentication($user, $token);
|
||||||
$this->userChecker->checkPostAuth($user);
|
$this->userChecker->checkPostAuth($user);
|
||||||
} catch (BadCredentialsException $e) {
|
} catch (AccountStatusException $e) {
|
||||||
if ($this->hideUserNotFoundExceptions) {
|
if ($this->hideUserNotFoundExceptions) {
|
||||||
throw new BadCredentialsException('Bad credentials.', 0, $e);
|
throw new BadCredentialsException('Bad credentials.', 0, $e);
|
||||||
}
|
}
|
||||||
|
@ -79,7 +79,7 @@ class UserAuthenticationProviderTest extends TestCase
|
|||||||
|
|
||||||
public function testAuthenticateWhenPreChecksFails()
|
public function testAuthenticateWhenPreChecksFails()
|
||||||
{
|
{
|
||||||
$this->expectException('Symfony\Component\Security\Core\Exception\CredentialsExpiredException');
|
$this->expectException(BadCredentialsException::class);
|
||||||
$userChecker = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserCheckerInterface')->getMock();
|
$userChecker = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserCheckerInterface')->getMock();
|
||||||
$userChecker->expects($this->once())
|
$userChecker->expects($this->once())
|
||||||
->method('checkPreAuth')
|
->method('checkPreAuth')
|
||||||
@ -97,7 +97,7 @@ class UserAuthenticationProviderTest extends TestCase
|
|||||||
|
|
||||||
public function testAuthenticateWhenPostChecksFails()
|
public function testAuthenticateWhenPostChecksFails()
|
||||||
{
|
{
|
||||||
$this->expectException('Symfony\Component\Security\Core\Exception\AccountExpiredException');
|
$this->expectException(BadCredentialsException::class);
|
||||||
$userChecker = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserCheckerInterface')->getMock();
|
$userChecker = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserCheckerInterface')->getMock();
|
||||||
$userChecker->expects($this->once())
|
$userChecker->expects($this->once())
|
||||||
->method('checkPostAuth')
|
->method('checkPostAuth')
|
||||||
@ -116,7 +116,7 @@ class UserAuthenticationProviderTest extends TestCase
|
|||||||
public function testAuthenticateWhenPostCheckAuthenticationFails()
|
public function testAuthenticateWhenPostCheckAuthenticationFails()
|
||||||
{
|
{
|
||||||
$this->expectException('Symfony\Component\Security\Core\Exception\BadCredentialsException');
|
$this->expectException('Symfony\Component\Security\Core\Exception\BadCredentialsException');
|
||||||
$this->expectExceptionMessage('Bad credentials');
|
$this->expectExceptionMessage('Bad credentials.');
|
||||||
$provider = $this->getProvider();
|
$provider = $this->getProvider();
|
||||||
$provider->expects($this->once())
|
$provider->expects($this->once())
|
||||||
->method('retrieveUser')
|
->method('retrieveUser')
|
||||||
@ -124,7 +124,7 @@ class UserAuthenticationProviderTest extends TestCase
|
|||||||
;
|
;
|
||||||
$provider->expects($this->once())
|
$provider->expects($this->once())
|
||||||
->method('checkAuthentication')
|
->method('checkAuthentication')
|
||||||
->willThrowException(new BadCredentialsException())
|
->willThrowException(new CredentialsExpiredException())
|
||||||
;
|
;
|
||||||
|
|
||||||
$provider->authenticate($this->getSupportedToken());
|
$provider->authenticate($this->getSupportedToken());
|
||||||
|
@ -17,7 +17,10 @@ use Symfony\Component\HttpFoundation\Response;
|
|||||||
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
|
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
|
||||||
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
|
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
|
||||||
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
|
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
|
||||||
|
use Symfony\Component\Security\Core\Exception\AccountStatusException;
|
||||||
use Symfony\Component\Security\Core\Exception\AuthenticationException;
|
use Symfony\Component\Security\Core\Exception\AuthenticationException;
|
||||||
|
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
|
||||||
|
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
|
||||||
use Symfony\Component\Security\Guard\AbstractGuardAuthenticator;
|
use Symfony\Component\Security\Guard\AbstractGuardAuthenticator;
|
||||||
use Symfony\Component\Security\Guard\AuthenticatorInterface;
|
use Symfony\Component\Security\Guard\AuthenticatorInterface;
|
||||||
use Symfony\Component\Security\Guard\GuardAuthenticatorHandler;
|
use Symfony\Component\Security\Guard\GuardAuthenticatorHandler;
|
||||||
@ -40,6 +43,7 @@ class GuardAuthenticationListener implements ListenerInterface
|
|||||||
private $guardAuthenticators;
|
private $guardAuthenticators;
|
||||||
private $logger;
|
private $logger;
|
||||||
private $rememberMeServices;
|
private $rememberMeServices;
|
||||||
|
private $hideUserNotFoundExceptions;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @param GuardAuthenticatorHandler $guardHandler The Guard handler
|
* @param GuardAuthenticatorHandler $guardHandler The Guard handler
|
||||||
@ -48,7 +52,7 @@ class GuardAuthenticationListener implements ListenerInterface
|
|||||||
* @param iterable|AuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider
|
* @param iterable|AuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider
|
||||||
* @param LoggerInterface $logger A LoggerInterface instance
|
* @param LoggerInterface $logger A LoggerInterface instance
|
||||||
*/
|
*/
|
||||||
public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, $guardAuthenticators, LoggerInterface $logger = null)
|
public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, $guardAuthenticators, LoggerInterface $logger = null, $hideUserNotFoundExceptions = true)
|
||||||
{
|
{
|
||||||
if (empty($providerKey)) {
|
if (empty($providerKey)) {
|
||||||
throw new \InvalidArgumentException('$providerKey must not be empty.');
|
throw new \InvalidArgumentException('$providerKey must not be empty.');
|
||||||
@ -59,6 +63,7 @@ class GuardAuthenticationListener implements ListenerInterface
|
|||||||
$this->providerKey = $providerKey;
|
$this->providerKey = $providerKey;
|
||||||
$this->guardAuthenticators = $guardAuthenticators;
|
$this->guardAuthenticators = $guardAuthenticators;
|
||||||
$this->logger = $logger;
|
$this->logger = $logger;
|
||||||
|
$this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -163,6 +168,12 @@ class GuardAuthenticationListener implements ListenerInterface
|
|||||||
$this->logger->info('Guard authentication failed.', ['exception' => $e, 'authenticator' => \get_class($guardAuthenticator)]);
|
$this->logger->info('Guard authentication failed.', ['exception' => $e, 'authenticator' => \get_class($guardAuthenticator)]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Avoid leaking error details in case of invalid user (e.g. user not found or invalid account status)
|
||||||
|
// to prevent user enumeration via response content
|
||||||
|
if ($this->hideUserNotFoundExceptions && ($e instanceof UsernameNotFoundException || $e instanceof AccountStatusException)) {
|
||||||
|
$e = new BadCredentialsException('Bad credentials.', 0, $e);
|
||||||
|
}
|
||||||
|
|
||||||
$response = $this->guardHandler->handleAuthenticationFailure($e, $request, $guardAuthenticator, $this->providerKey);
|
$response = $this->guardHandler->handleAuthenticationFailure($e, $request, $guardAuthenticator, $this->providerKey);
|
||||||
|
|
||||||
if ($response instanceof Response) {
|
if ($response instanceof Response) {
|
||||||
|
@ -16,6 +16,9 @@ use Symfony\Component\HttpFoundation\Request;
|
|||||||
use Symfony\Component\HttpFoundation\Response;
|
use Symfony\Component\HttpFoundation\Response;
|
||||||
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
|
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
|
||||||
use Symfony\Component\Security\Core\Exception\AuthenticationException;
|
use Symfony\Component\Security\Core\Exception\AuthenticationException;
|
||||||
|
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
|
||||||
|
use Symfony\Component\Security\Core\Exception\LockedException;
|
||||||
|
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
|
||||||
use Symfony\Component\Security\Guard\AbstractGuardAuthenticator;
|
use Symfony\Component\Security\Guard\AbstractGuardAuthenticator;
|
||||||
use Symfony\Component\Security\Guard\AuthenticatorInterface;
|
use Symfony\Component\Security\Guard\AuthenticatorInterface;
|
||||||
use Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener;
|
use Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener;
|
||||||
@ -208,6 +211,54 @@ class GuardAuthenticationListenerTest extends TestCase
|
|||||||
$listener->handle($this->event);
|
$listener->handle($this->event);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @dataProvider exceptionsToHide
|
||||||
|
*/
|
||||||
|
public function testHandleHidesInvalidUserExceptions(AuthenticationException $exceptionToHide)
|
||||||
|
{
|
||||||
|
$authenticator = $this->createMock(AuthenticatorInterface::class);
|
||||||
|
$providerKey = 'my_firewall2';
|
||||||
|
|
||||||
|
$authenticator
|
||||||
|
->expects($this->once())
|
||||||
|
->method('supports')
|
||||||
|
->willReturn(true);
|
||||||
|
$authenticator
|
||||||
|
->expects($this->once())
|
||||||
|
->method('getCredentials')
|
||||||
|
->willReturn(['username' => 'robin', 'password' => 'hood']);
|
||||||
|
|
||||||
|
$this->authenticationManager
|
||||||
|
->expects($this->once())
|
||||||
|
->method('authenticate')
|
||||||
|
->willThrowException($exceptionToHide);
|
||||||
|
|
||||||
|
$this->guardAuthenticatorHandler
|
||||||
|
->expects($this->once())
|
||||||
|
->method('handleAuthenticationFailure')
|
||||||
|
->with($this->callback(function ($e) use ($exceptionToHide) {
|
||||||
|
return $e instanceof BadCredentialsException && $exceptionToHide === $e->getPrevious();
|
||||||
|
}), $this->request, $authenticator, $providerKey);
|
||||||
|
|
||||||
|
$listener = new GuardAuthenticationListener(
|
||||||
|
$this->guardAuthenticatorHandler,
|
||||||
|
$this->authenticationManager,
|
||||||
|
$providerKey,
|
||||||
|
[$authenticator],
|
||||||
|
$this->logger
|
||||||
|
);
|
||||||
|
|
||||||
|
$listener->handle($this->event);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function exceptionsToHide()
|
||||||
|
{
|
||||||
|
return [
|
||||||
|
[new UsernameNotFoundException()],
|
||||||
|
[new LockedException()],
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @group legacy
|
* @group legacy
|
||||||
*/
|
*/
|
||||||
|
Reference in New Issue
Block a user