-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Send "429 Too Many Requests" in case of brute force protection #22280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
e66bc4a
Send "429 Too Many Requests" in case of brute force protection
nickvergessen cdb36c8
Let the database count the entries
nickvergessen c8fea66
Split delay calculation from getting the attempts
nickvergessen 64539a6
Make Throttler strict
nickvergessen 6f751d0
Make the throttling O(2^n) instead of O(n^n)
nickvergessen 8376c48
Only throw when also the last 30 mins were attacking
nickvergessen dfeee3b
Fix wrong doc + type hint
nickvergessen d9c4c9e
Simplify array filter
nickvergessen 931aca2
Add missing default
nickvergessen 770381c
Correctly return ms delay when at max
nickvergessen 35a8519
Fix CS
nickvergessen 41d8a7e
Fix unit tests
nickvergessen 6f5f71d
Update autoloader
nickvergessen a9f22ac
More test fixing
nickvergessen e93bf71
Fix the return type of OC_Template->fetchPage() to be string only
MorrisJobke File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| <div class="body-login-container update"> | ||
| <h2><?php p($l->t('Too many requests')); ?></h2> | ||
| <p class="infogroup"><?php p($l->t('There were too many requests from your network. Retry later or contact your administrator if this is an error.')); ?></p> | ||
| </div> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,6 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
| /** | ||
| * @copyright Copyright (c) 2017 Lukas Reschke <[email protected]> | ||
| * | ||
|
|
@@ -26,9 +28,15 @@ | |
|
|
||
| use OC\AppFramework\Utility\ControllerMethodReflector; | ||
| use OC\Security\Bruteforce\Throttler; | ||
| use OCP\AppFramework\Controller; | ||
| use OCP\AppFramework\Http; | ||
| use OCP\AppFramework\Http\Response; | ||
| use OCP\AppFramework\Http\TooManyRequestsResponse; | ||
| use OCP\AppFramework\Middleware; | ||
| use OCP\AppFramework\OCS\OCSException; | ||
| use OCP\AppFramework\OCSController; | ||
| use OCP\IRequest; | ||
| use OCP\Security\Bruteforce\MaxDelayReached; | ||
|
|
||
| /** | ||
| * Class BruteForceMiddleware performs the bruteforce protection for controllers | ||
|
|
@@ -66,7 +74,7 @@ public function beforeController($controller, $methodName) { | |
|
|
||
| if ($this->reflector->hasAnnotation('BruteForceProtection')) { | ||
| $action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action'); | ||
| $this->throttler->sleepDelay($this->request->getRemoteAddress(), $action); | ||
| $this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), $action); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -83,4 +91,23 @@ public function afterController($controller, $methodName, Response $response) { | |
|
|
||
| return parent::afterController($controller, $methodName, $response); | ||
| } | ||
|
|
||
| /** | ||
| * @param Controller $controller | ||
| * @param string $methodName | ||
| * @param \Exception $exception | ||
| * @throws \Exception | ||
| * @return Response | ||
| */ | ||
| public function afterException($controller, $methodName, \Exception $exception): Response { | ||
| if ($exception instanceof MaxDelayReached) { | ||
| if ($controller instanceof OCSController) { | ||
| throw new OCSException($exception->getMessage(), Http::STATUS_TOO_MANY_REQUESTS); | ||
| } | ||
|
|
||
| return new TooManyRequestsResponse(); | ||
| } | ||
|
|
||
| throw $exception; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,6 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
| /** | ||
| * @copyright Copyright (c) 2016 Lukas Reschke <[email protected]> | ||
| * | ||
|
|
@@ -34,6 +36,7 @@ | |
| use OCP\IConfig; | ||
| use OCP\IDBConnection; | ||
| use OCP\ILogger; | ||
| use OCP\Security\Bruteforce\MaxDelayReached; | ||
|
|
||
| /** | ||
| * Class Throttler implements the bruteforce protection for security actions in | ||
|
|
@@ -50,6 +53,9 @@ | |
| */ | ||
| class Throttler { | ||
| public const LOGIN_ACTION = 'login'; | ||
| public const MAX_DELAY = 25; | ||
| public const MAX_DELAY_MS = 25000; // in milliseconds | ||
| public const MAX_ATTEMPTS = 10; | ||
|
|
||
| /** @var IDBConnection */ | ||
| private $db; | ||
|
|
@@ -82,7 +88,7 @@ public function __construct(IDBConnection $db, | |
| * @param int $expire | ||
| * @return \DateInterval | ||
| */ | ||
| private function getCutoff($expire) { | ||
| private function getCutoff(int $expire): \DateInterval { | ||
| $d1 = new \DateTime(); | ||
| $d2 = clone $d1; | ||
| $d2->sub(new \DateInterval('PT' . $expire . 'S')); | ||
|
|
@@ -92,11 +98,12 @@ private function getCutoff($expire) { | |
| /** | ||
| * Calculate the cut off timestamp | ||
| * | ||
| * @param float $maxAgeHours | ||
| * @return int | ||
| */ | ||
| private function getCutoffTimestamp(): int { | ||
| private function getCutoffTimestamp(float $maxAgeHours = 12.0): int { | ||
| return (new \DateTime()) | ||
| ->sub($this->getCutoff(43200)) | ||
| ->sub($this->getCutoff((int) ($maxAgeHours * 3600))) | ||
| ->getTimestamp(); | ||
| } | ||
|
|
||
|
|
@@ -108,9 +115,9 @@ private function getCutoffTimestamp(): int { | |
| * @param array $metadata Optional metadata logged to the database | ||
| * @suppress SqlInjectionChecker | ||
| */ | ||
| public function registerAttempt($action, | ||
| $ip, | ||
| array $metadata = []) { | ||
| public function registerAttempt(string $action, | ||
| string $ip, | ||
| array $metadata = []): void { | ||
| // No need to log if the bruteforce protection is disabled | ||
| if ($this->config->getSystemValue('auth.bruteforce.protection.enabled', true) === false) { | ||
| return; | ||
|
|
@@ -150,15 +157,14 @@ public function registerAttempt($action, | |
| * @param string $ip | ||
| * @return bool | ||
| */ | ||
| private function isIPWhitelisted($ip) { | ||
| private function isIPWhitelisted(string $ip): bool { | ||
| if ($this->config->getSystemValue('auth.bruteforce.protection.enabled', true) === false) { | ||
| return true; | ||
| } | ||
|
|
||
| $keys = $this->config->getAppKeys('bruteForce'); | ||
| $keys = array_filter($keys, function ($key) { | ||
| $regex = '/^whitelist_/S'; | ||
| return preg_match($regex, $key) === 1; | ||
| return 0 === strpos($key, 'whitelist_'); | ||
| }); | ||
|
|
||
| if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) { | ||
|
|
@@ -215,18 +221,19 @@ private function isIPWhitelisted($ip) { | |
| * | ||
| * @param string $ip | ||
| * @param string $action optionally filter by action | ||
| * @param float $maxAgeHours | ||
| * @return int | ||
| */ | ||
| public function getDelay($ip, $action = '') { | ||
| public function getAttempts(string $ip, string $action = '', float $maxAgeHours = 12): int { | ||
| $ipAddress = new IpAddress($ip); | ||
| if ($this->isIPWhitelisted((string)$ipAddress)) { | ||
| return 0; | ||
| } | ||
|
|
||
| $cutoffTime = $this->getCutoffTimestamp(); | ||
| $cutoffTime = $this->getCutoffTimestamp($maxAgeHours); | ||
|
|
||
| $qb = $this->db->getQueryBuilder(); | ||
| $qb->select('*') | ||
| $qb->select($qb->func()->count('*', 'attempts')) | ||
| ->from('bruteforce_attempts') | ||
| ->where($qb->expr()->gt('occurred', $qb->createNamedParameter($cutoffTime))) | ||
| ->andWhere($qb->expr()->eq('subnet', $qb->createNamedParameter($ipAddress->getSubnet()))); | ||
|
|
@@ -235,34 +242,47 @@ public function getDelay($ip, $action = '') { | |
| $qb->andWhere($qb->expr()->eq('action', $qb->createNamedParameter($action))); | ||
| } | ||
|
|
||
| $attempts = count($qb->execute()->fetchAll()); | ||
| $result = $qb->execute(); | ||
| $row = $result->fetch(); | ||
| $result->closeCursor(); | ||
|
|
||
| return (int) $row['attempts']; | ||
| } | ||
|
|
||
| /** | ||
| * Get the throttling delay (in milliseconds) | ||
| * | ||
| * @param string $ip | ||
| * @param string $action optionally filter by action | ||
| * @return int | ||
| */ | ||
| public function getDelay(string $ip, string $action = ''): int { | ||
| $attempts = $this->getAttempts($ip, $action); | ||
| if ($attempts === 0) { | ||
| return 0; | ||
| } | ||
|
|
||
| $maxDelay = 25; | ||
| $firstDelay = 0.1; | ||
| if ($attempts > (8 * PHP_INT_SIZE - 1)) { | ||
| if ($attempts > self::MAX_ATTEMPTS) { | ||
| // Don't ever overflow. Just assume the maxDelay time:s | ||
| $firstDelay = $maxDelay; | ||
| } else { | ||
| $firstDelay *= pow(2, $attempts); | ||
| if ($firstDelay > $maxDelay) { | ||
| $firstDelay = $maxDelay; | ||
| } | ||
| return self::MAX_DELAY_MS; | ||
| } | ||
| return (int) \ceil($firstDelay * 1000); | ||
|
|
||
| $delay = $firstDelay * 2**$attempts; | ||
| if ($delay > self::MAX_DELAY) { | ||
| return self::MAX_DELAY_MS; | ||
| } | ||
| return (int) \ceil($delay * 1000); | ||
| } | ||
|
|
||
| /** | ||
| * Reset the throttling delay for an IP address, action and metadata | ||
| * | ||
| * @param string $ip | ||
| * @param string $action | ||
| * @param string $metadata | ||
| * @param array $metadata | ||
| */ | ||
| public function resetDelay($ip, $action, $metadata) { | ||
| public function resetDelay(string $ip, string $action, array $metadata): void { | ||
| $ipAddress = new IpAddress($ip); | ||
| if ($this->isIPWhitelisted((string)$ipAddress)) { | ||
| return; | ||
|
|
@@ -303,9 +323,28 @@ public function resetDelayForIP($ip) { | |
| * @param string $action optionally filter by action | ||
| * @return int the time spent sleeping | ||
| */ | ||
| public function sleepDelay($ip, $action = '') { | ||
| public function sleepDelay(string $ip, string $action = ''): int { | ||
| $delay = $this->getDelay($ip, $action); | ||
| usleep($delay * 1000); | ||
| return $delay; | ||
| } | ||
|
|
||
| /** | ||
| * Will sleep for the defined amount of time unless maximum was reached in the last 30 minutes | ||
| * In this case a "429 Too Many Request" exception is thrown | ||
| * | ||
| * @param string $ip | ||
| * @param string $action optionally filter by action | ||
| * @return int the time spent sleeping | ||
| * @throws MaxDelayReached when reached the maximum | ||
| */ | ||
| public function sleepDelayOrThrowOnMax(string $ip, string $action = ''): int { | ||
| $delay = $this->getDelay($ip, $action); | ||
| if (($delay === self::MAX_DELAY_MS) && $this->getAttempts($ip, $action, 0.5) > self::MAX_ATTEMPTS) { | ||
| // If the ip made too many attempts within the last 30 mins we don't execute anymore | ||
| throw new MaxDelayReached('Reached maximum delay'); | ||
| } | ||
| usleep($delay * 1000); | ||
| return $delay; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
| /** | ||
| * @copyright Copyright (c) 2020 Joas Schilling <[email protected]> | ||
| * | ||
| * @license GNU AGPL version 3 or any later version | ||
| * | ||
| * This program is free software: you can redistribute it and/or modify | ||
| * it under the terms of the GNU Affero General Public License as | ||
| * published by the Free Software Foundation, either version 3 of the | ||
| * License, or (at your option) any later version. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, | ||
| * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| * GNU Affero General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Affero General Public License | ||
| * along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
| * | ||
| */ | ||
|
|
||
| namespace OCP\AppFramework\Http; | ||
|
|
||
| use OCP\Template; | ||
|
|
||
| /** | ||
| * A generic 429 response showing an 404 error page as well to the end-user | ||
| * @since 19.0.0 | ||
| */ | ||
| class TooManyRequestsResponse extends Response { | ||
|
|
||
| /** | ||
| * @since 19.0.0 | ||
| */ | ||
| public function __construct() { | ||
| parent::__construct(); | ||
|
|
||
| $this->setContentSecurityPolicy(new ContentSecurityPolicy()); | ||
| $this->setStatus(429); | ||
| } | ||
|
|
||
| /** | ||
| * @return string | ||
| * @since 19.0.0 | ||
| */ | ||
| public function render() { | ||
| $template = new Template('core', '429', 'blank'); | ||
| return $template->fetchPage(); | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.