8000 Added request rate limiters and improved login throttling · symfony/symfony@61f02fb · GitHub
[go: up one dir, main page]

Skip to content

Commit 61f02fb

Browse files
committed
Added request rate limiters and improved login throttling
This allows limiting on different elements of a request. This is usefull to e.g. prevent breadth-first attacks, by allowing to enforce a limit on both IP and IP+username.
1 parent d8d90b0 commit 61f02fb

File tree

6 files changed

+126
-37
lines changed

6 files changed

+126
-37
lines changed

src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginThrottlingFactory.php

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
use Symfony\Component\DependencyInjection\ChildDefinition;
1818
use Symfony\Component\DependencyInjection\ContainerBuilder;
1919
use Symfony\Component\DependencyInjection\Reference;
20+
use Symfony\Component\RateLimiter\AbstractRequestRateLimiter;
2021
use Symfony\Component\Security\Http\EventListener\LoginThrottlingListener;
22+
use Symfony\Component\Security\Http\RateLimiter\DefaultLoginRateLimiter;
2123

2224
/**
2325
* @author Wouter de Jong <wouter@wouterj.nl>
@@ -49,7 +51,7 @@ public function addConfiguration(NodeDefinition $builder)
4951
{
5052
$builder
5153
->children()
52-
->scalarNode('limiter')->info('The name of the limiter that you defined under "framework.rate_limiter".')->end()
54+
->scalarNode('limiter')->info(sprintf('A service id extending from "%s".', AbstractRequestRateLimiter::class))->end()
5355
->integerNode('max_attempts')->defaultValue(5)->end()
5456
->end();
5557
}
@@ -65,18 +67,27 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal
6567
throw new \LogicException('You must either configure a rate limiter for "security.firewalls.'.$firewallName.'.login_throttling" or install symfony/framework-bundle:^5.2');
6668
}
6769

68-
FrameworkExtension::registerRateLimiter($container, $config['limiter'] = '_login_'.$firewallName, [
70+
$limiterOptions = [
6971
'strategy' => 'fixed_window',
7072
'limit' => $config['max_attempts'],
7173
'interval' => '1 minute',
7274
'lock_factory' => 'lock.factory',
7375
'cache_pool' => 'cache.app',
74-
]);
76+
];
77+
FrameworkExtension::registerRateLimiter($container, $localId = '_login_local_'.$firewallName, $limiterOptions);
78+
79+
$limiterOptions['limit'] = 5 * $config['max_attempts'];
80+
FrameworkExtension::registerRateLimiter($container, $globalId = '_login_global_'.$firewallName, $limiterOptions);
81+
82+
$container->register($config['limiter'] = 'security.login_throttling.'.$firewallName.'.limiter', DefaultLoginRateLimiter::class)
83+
->addArgument(new Reference('limiter.'.$globalId))
84+
->addArgument(new Reference('limiter.'.$localId))
85+
;
7586
}
7687

7788
$container
7889
->setDefinition('security.listener.login_throttling.'.$firewallName, new ChildDefinition('security.listener.login_throttling'))
79-
->replaceArgument(1, new Reference('limiter.'.$config['li B41A miter']))
90+
->replaceArgument(1, new Reference($config['limiter']))
8091
->addTag('kernel.event_subscriber', ['dispatcher' => 'security.event_dispatcher.'.$firewallName]);
8192

8293
return [];

src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@
118118
->abstract()
119119
->args([
120120
service('request_stack'),
121-
abstract_arg('rate limiter'),
121+
abstract_arg('request rate limiter'),
122122
])
123123

124124
// Authenticators
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\RateLimiter;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
16+
/**
17+
* A special type of compound limiter that deals with requests.
18+
*
19+
* This allows to limit on different types of information
20+
* from the requests.
21+
*
22+
* @author Wouter de Jong <wouter@wouterj.nl>
23+
*
24+
* @experimental in Symfony 5.2
25+
*/
26+
abstract class AbstractRequestRateLimiter
27+
{
28+
public function consume(Request $request): bool
29+
{
30+
$allow = true;
31+
foreach ($this->getLimiters($request) as $limiter) {
32+
$allow = $limiter->consume(1) && $allow;
33+
}
34+
35+
return $allow;
36+
}
37+
38+
/**
39+
* @return LimiterInterface[]
40+
*/
41+
abstract protected function getLimiters(Request $request): array;
42+
}

src/Symfony/Component/Security/Http/EventListener/LoginThrottlingListener.php

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,12 @@
1212
namespace Symfony\Component\Security\Http\EventListener;
1313

1414
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
15-
use Symfony\Component\HttpFoundation\Request;
1615
use Symfony\Component\HttpFoundation\RequestStack;
17-
use Symfony\Component\RateLimiter\Limiter;
16+
use Symfony\Component\RateLimiter\AbstractRequestRateLimiter;
1817
use Symfony\Component\Security\Core\Exception\TooManyLoginAttemptsAuthenticationException;
18+
use Symfony\Component\Security\Core\Security;
1919
use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
2020
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
21-
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
2221

2322
/**
2423
* @author Wouter de Jong <wouter@wouterj.nl>
@@ -30,7 +29,7 @@ final class LoginThrottlingListener implements EventSubscriberInterface
3029
private $requestStack;
3130
private $limiter;
3231

33-
public function __construct(RequestStack $requestStack, Limiter $limiter)
32+
public function __construct(RequestStack $requestStack, AbstractRequestRateLimiter $limiter)
3433
{
3534
$this->requestStack = $requestStack;
3635
$this->limiter = $limiter;
@@ -44,33 +43,17 @@ public function checkPassport(CheckPassportEvent $event): void
4443
}
4544

4645
$request = $this->requestStack->getMasterRequest();
47-
$username = $passport->getBadge(UserBadge::class)->getUserIdentifier();
48-
$limiterKey = $this->createLimiterKey($username, $request);
46+
$request->attributes->set(Security::LAST_USERNAME, $passport->getBadge(UserBadge::class)->getUserIdentifier());
4947

50-
$limiter = $this->limiter->create($limiterKey);
51-
if (!$limiter->consume()) {
48+
if (!$this->limiter->consume($request)) {
5249
throw new TooManyLoginAttemptsAuthenticationException();
5350
}
5451
}
5552

56-
public function onSuccessfulLogin(LoginSuccessEvent $event): void
57-
{
58-
$limiterKey = $this->createLimiterKey($event->getAuthenticatedToken()->getUsername(), $event->getRequest());
59-
$limiter = $this->limiter->create($limiterKey);
60-
61-
$limiter->reset();
62-
}
63-
6453
public static function getSubscribedEvents(): array
6554
{
6655
return [
6756
CheckPassportEvent::class => ['checkPassport', 64],
68-
LoginSuccessEvent::class => 'onSuccessfulLogin',
6957
];
7058
}
71-
72-
private function createLimiterKey($username, Request $request): string
73-
{
74-
return $username.$request->getClientIp();
75-
}
7659
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Http\RateLimiter;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\RateLimiter\AbstractRequestRateLimiter;
16+
use Symfony\Component\RateLimiter\Limiter;
17+
use Symfony\Component\Security\Core\Security;
18+
19+
/**
20+
* A default login throttling limiter.
21+
*
22+
* This limiter prevents breadth-first attacks by enforcing
23+
* a limit on username+IP and a (higher) limit on IP.
24+
*
25+
* @author Wouter de Jong <wouter@wouterj.nl>
26+
*
27+
* @experimental in Symfony 5.2
28+
*/
29+
final class DefaultLoginRateLimiter extends AbstractRequestRateLimiter
30+
{
31+
private $globalLimiter;
32+
private $localLimiter;
33+
34+
public function __construct(Limiter $globalLimiter, Limiter $localLimiter)
35+
{
36+
$this->globalLimiter = $globalLimiter;
37+
$this->localLimiter = $localLimiter;
38+
}
39+
40+
protected function getLimiters(Request $request): array
41+
{
42+
return [
43+
$this->globalLimiter->create($request->getClientIp()),
44+
$this->localLimiter->create($request->attributes->get(Security::LAST_USERNAME).$request->getClientIp()),
45+
];
46+
}
47+
}

src/Symfony/Component/Security/Http/Tests/EventListener/LoginThrottlingListenerTest.php

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
2525
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
2626
use Symfony\Component\Security\Http\EventListener\LoginThrottlingListener;
27+
use Symfony\Component\Security\Http\RateLimiter\DefaultLoginRateLimiter;
2728

2829
class LoginThrottlingListenerTest extends TestCase
2930
{
@@ -34,17 +35,24 @@ protected function setUp(): void
3435
{
3536
$this->requestStack = new RequestStack();
3637

37-
$limiter = new Limiter([
38+
$localLimiter = new Limiter([
3839
'id' => 'login',
3940
'strategy' => 'fixed_window',
4041
'limit' => 3,
4142
'interval' => '1 minute',
4243
], new InMemoryStorage());
44+
$globalLimiter = new Limiter([
45+
'id' => 'login',
46+
'strategy' => 'fixed_window',
47+
'limit' => 6,
48+
'interval' => '1 minute',
49+
], new InMemoryStorage());
50+
$limiter = new DefaultLoginRateLimiter($globalLimiter, $localLimiter);
4351

4452
$this->listener = new LoginThrottlingListener($this->requestStack, $limiter);
4553
}
4654

47-
public function testPreventsLoginWhenOverThreshold()
55+
public function testPreventsLoginWhenOverLocalThreshold()
4856
{
4957
$request = $this->createRequest();
5058
$passport = $this->createPassport('wouter');
@@ -59,21 +67,19 @@ public function testPreventsLoginWhenOverThreshold()
5967
$this->listener->checkPassport($this->createCheckPassportEvent($passport));
6068
}
6169

62-
public function testSuccessfulLoginResetsCount()
70+
public function testPreventsLoginWhenOverGlobalThreshold()
6371
{
64-
$this->expectNotToPerformAssertions();
65-
6672
$request = $this->createRequest();
67-
$passport = $this->createPassport('wouter');
73+
$passports = [$this->createPassport('wouter'), $this->createPassport('ryan')];
6874

6975
$this->requestStack->push($request);
7076

71-
for ($i = 0; $i < 3; ++$i) {
72-
$this->listener->checkPassport($this->createCheckPassportEvent($passport));
77+
for ($i = 0; $i < 6; ++$i) {
78+
$this->listener->checkPassport($this->createCheckPassportEvent($passports[$i % 2]));
7379
}
7480

75-
$this->listener->onSuccessfulLogin($this->createLoginSuccessfulEvent($passport));
76-
$this->listener->checkPassport($this->createCheckPassportEvent($passport));
81+
$this->expectException(TooManyLoginAttemptsAuthenticationException::class);
82+
$this->listener->checkPassport($this->createCheckPassportEvent($passports[0]));
7783
}
7884

7985
private function createPassport($username)

0 commit comments

Comments
 (0)
0