diff --git a/apps/dav/appinfo/v1/publicwebdav.php b/apps/dav/appinfo/v1/publicwebdav.php index 556dd35de582b..32451e02af6b5 100644 --- a/apps/dav/appinfo/v1/publicwebdav.php +++ b/apps/dav/appinfo/v1/publicwebdav.php @@ -42,7 +42,8 @@ $authBackend = new OCA\DAV\Connector\PublicAuth( \OC::$server->getRequest(), \OC::$server->getShareManager(), - \OC::$server->getSession() + \OC::$server->getSession(), + \OC::$server->getBruteForceThrottler() ); $authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend); diff --git a/apps/dav/lib/Connector/PublicAuth.php b/apps/dav/lib/Connector/PublicAuth.php index 450217722937a..9d4ef8ba9b251 100644 --- a/apps/dav/lib/Connector/PublicAuth.php +++ b/apps/dav/lib/Connector/PublicAuth.php @@ -31,6 +31,7 @@ namespace OCA\DAV\Connector; +use OC\Security\Bruteforce\Throttler; use OCP\IRequest; use OCP\ISession; use OCP\Share\Exceptions\ShareNotFound; @@ -44,6 +45,7 @@ * @package OCA\DAV\Connector */ class PublicAuth extends AbstractBasic { + private const BRUTEFORCE_ACTION = 'public_webdav_auth'; /** @var \OCP\Share\IShare */ private $share; @@ -57,17 +59,23 @@ class PublicAuth extends AbstractBasic { /** @var IRequest */ private $request; + /** @var Throttler */ + private $throttler; + /** * @param IRequest $request * @param IManager $shareManager * @param ISession $session + * @param Throttler $throttler */ public function __construct(IRequest $request, IManager $shareManager, - ISession $session) { + ISession $session, + Throttler $throttler) { $this->request = $request; $this->shareManager = $shareManager; $this->session = $session; + $this->throttler = $throttler; // setup realm $defaults = new \OCP\Defaults(); @@ -87,9 +95,12 @@ public function __construct(IRequest $request, * @throws \Sabre\DAV\Exception\NotAuthenticated */ protected function validateUserPass($username, $password) { + $this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), self::BRUTEFORCE_ACTION); + try { $share = $this->shareManager->getShareByToken($username); } catch (ShareNotFound $e) { + $this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress()); return false; } @@ -114,11 +125,14 @@ protected function validateUserPass($username, $password) { header('WWW-Authenticate: DummyBasic realm="' . $this->realm . '"'); throw new \Sabre\DAV\Exception\NotAuthenticated('Cannot authenticate over ajax calls'); } + + $this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress()); return false; } } elseif ($share->getShareType() === IShare::TYPE_REMOTE) { return true; } else { + $this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress()); return false; } } else { diff --git a/apps/dav/tests/unit/Connector/PublicAuthTest.php b/apps/dav/tests/unit/Connector/PublicAuthTest.php index bcec84d414c43..0ac41295cb033 100644 --- a/apps/dav/tests/unit/Connector/PublicAuthTest.php +++ b/apps/dav/tests/unit/Connector/PublicAuthTest.php @@ -27,6 +27,7 @@ namespace OCA\DAV\Tests\unit\Connector; +use OC\Security\Bruteforce\Throttler; use OCP\IRequest; use OCP\ISession; use OCP\Share\Exceptions\ShareNotFound; @@ -50,6 +51,8 @@ class PublicAuthTest extends \Test\TestCase { private $shareManager; /** @var \OCA\DAV\Connector\PublicAuth */ private $auth; + /** @var Throttler|\PHPUnit\Framework\MockObject\MockObject */ + private $throttler; /** @var string */ private $oldUser; @@ -66,11 +69,15 @@ protected function setUp(): void { $this->shareManager = $this->getMockBuilder(IManager::class) ->disableOriginalConstructor() ->getMock(); + $this->throttler = $this->getMockBuilder(Throttler::class) + ->disableOriginalConstructor() + ->getMock(); $this->auth = new \OCA\DAV\Connector\PublicAuth( $this->request, $this->shareManager, - $this->session + $this->session, + $this->throttler ); // Store current user diff --git a/core/templates/429.php b/core/templates/429.php new file mode 100644 index 0000000000000..caf8a3675e297 --- /dev/null +++ b/core/templates/429.php @@ -0,0 +1,4 @@ +
+

t('Too many requests')); ?>

+

t('There were too many requests from your network. Retry later or contact your administrator if this is an error.')); ?>

+
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 37c6bc3fdf503..8998038eb1775 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -59,6 +59,7 @@ 'OCP\\AppFramework\\Http\\Template\\LinkMenuAction' => $baseDir . '/lib/public/AppFramework/Http/Template/LinkMenuAction.php', 'OCP\\AppFramework\\Http\\Template\\PublicTemplateResponse' => $baseDir . '/lib/public/AppFramework/Http/Template/PublicTemplateResponse.php', 'OCP\\AppFramework\\Http\\Template\\SimpleMenuAction' => $baseDir . '/lib/public/AppFramework/Http/Template/SimpleMenuAction.php', + 'OCP\\AppFramework\\Http\\TooManyRequestsResponse' => $baseDir . '/lib/public/AppFramework/Http/TooManyRequestsResponse.php', 'OCP\\AppFramework\\Http\\ZipResponse' => $baseDir . '/lib/public/AppFramework/Http/ZipResponse.php', 'OCP\\AppFramework\\IAppContainer' => $baseDir . '/lib/public/AppFramework/IAppContainer.php', 'OCP\\AppFramework\\Middleware' => $baseDir . '/lib/public/AppFramework/Middleware.php', @@ -415,6 +416,7 @@ 'OCP\\Search\\PagedProvider' => $baseDir . '/lib/public/Search/PagedProvider.php', 'OCP\\Search\\Provider' => $baseDir . '/lib/public/Search/Provider.php', 'OCP\\Search\\Result' => $baseDir . '/lib/public/Search/Result.php', + 'OCP\\Security\\Bruteforce\\MaxDelayReached' => $baseDir . '/lib/public/Security/Bruteforce/MaxDelayReached.php', 'OCP\\Security\\CSP\\AddContentSecurityPolicyEvent' => $baseDir . '/lib/public/Security/CSP/AddContentSecurityPolicyEvent.php', 'OCP\\Security\\Events\\GenerateSecurePasswordEvent' => $baseDir . '/lib/public/Security/Events/GenerateSecurePasswordEvent.php', 'OCP\\Security\\Events\\ValidatePasswordPolicyEvent' => $baseDir . '/lib/public/Security/Events/ValidatePasswordPolicyEvent.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index abb132ab88584..51daaefeee6f3 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -88,6 +88,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OCP\\AppFramework\\Http\\Template\\LinkMenuAction' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Template/LinkMenuAction.php', 'OCP\\AppFramework\\Http\\Template\\PublicTemplateResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Template/PublicTemplateResponse.php', 'OCP\\AppFramework\\Http\\Template\\SimpleMenuAction' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Template/SimpleMenuAction.php', + 'OCP\\AppFramework\\Http\\TooManyRequestsResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/TooManyRequestsResponse.php', 'OCP\\AppFramework\\Http\\ZipResponse' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/ZipResponse.php', 'OCP\\AppFramework\\IAppContainer' => __DIR__ . '/../../..' . '/lib/public/AppFramework/IAppContainer.php', 'OCP\\AppFramework\\Middleware' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Middleware.php', @@ -444,6 +445,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OCP\\Search\\PagedProvider' => __DIR__ . '/../../..' . '/lib/public/Search/PagedProvider.php', 'OCP\\Search\\Provider' => __DIR__ . '/../../..' . '/lib/public/Search/Provider.php', 'OCP\\Search\\Result' => __DIR__ . '/../../..' . '/lib/public/Search/Result.php', + 'OCP\\Security\\Bruteforce\\MaxDelayReached' => __DIR__ . '/../../..' . '/lib/public/Security/Bruteforce/MaxDelayReached.php', 'OCP\\Security\\CSP\\AddContentSecurityPolicyEvent' => __DIR__ . '/../../..' . '/lib/public/Security/CSP/AddContentSecurityPolicyEvent.php', 'OCP\\Security\\Events\\GenerateSecurePasswordEvent' => __DIR__ . '/../../..' . '/lib/public/Security/Events/GenerateSecurePasswordEvent.php', 'OCP\\Security\\Events\\ValidatePasswordPolicyEvent' => __DIR__ . '/../../..' . '/lib/public/Security/Events/ValidatePasswordPolicyEvent.php', diff --git a/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php index 398c2f3f3a447..e6c511537a055 100644 --- a/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php @@ -1,4 +1,6 @@ * @@ -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; + } } diff --git a/lib/private/Security/Bruteforce/Throttler.php b/lib/private/Security/Bruteforce/Throttler.php index d1a5f6d44889a..4db80cd426690 100644 --- a/lib/private/Security/Bruteforce/Throttler.php +++ b/lib/private/Security/Bruteforce/Throttler.php @@ -34,6 +34,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 +51,7 @@ */ class Throttler { public const LOGIN_ACTION = 'login'; + public const MAX_DELAY = 25; /** @var IDBConnection */ private $db; @@ -236,7 +238,7 @@ public function getDelay($ip, $action = '') { return 0; } - $maxDelay = 25; + $maxDelay = self::MAX_DELAY; $firstDelay = 0.1; if ($attempts > (8 * PHP_INT_SIZE - 1)) { // Don't ever overflow. Just assume the maxDelay time:s @@ -289,4 +291,22 @@ public function sleepDelay($ip, $action = '') { usleep($delay * 1000); return $delay; } + + /** + * Will sleep for the defined amount of time unless maximum is reached + * In case of maximum a "429 Too Many Request" response 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($ip, $action = '') { + $delay = $this->getDelay($ip, $action); + if ($delay === self::MAX_DELAY * 1000) { + throw new MaxDelayReached(); + } + usleep($delay * 1000); + return $delay; + } } diff --git a/lib/public/AppFramework/Http/TooManyRequestsResponse.php b/lib/public/AppFramework/Http/TooManyRequestsResponse.php new file mode 100644 index 0000000000000..b15df303bfee2 --- /dev/null +++ b/lib/public/AppFramework/Http/TooManyRequestsResponse.php @@ -0,0 +1,52 @@ + + * + * @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 . + * + */ + +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(); + } +} diff --git a/lib/public/Security/Bruteforce/MaxDelayReached.php b/lib/public/Security/Bruteforce/MaxDelayReached.php new file mode 100644 index 0000000000000..3aaa7d0515932 --- /dev/null +++ b/lib/public/Security/Bruteforce/MaxDelayReached.php @@ -0,0 +1,31 @@ + + * + * @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 . + * + */ + +namespace OCP\Security\Bruteforce; + +/** + * Class MaxDelayReached + * @since 19.0 + */ +class MaxDelayReached extends \RuntimeException { +} diff --git a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php index 55c874ccdb52b..84cbddff46c07 100644 --- a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php @@ -70,7 +70,7 @@ public function testBeforeControllerWithAnnotation() { ->willReturn('127.0.0.1'); $this->throttler ->expects($this->once()) - ->method('sleepDelay') + ->method('sleepDelayOrThrowOnMax') ->with('127.0.0.1', 'login'); /** @var Controller|\PHPUnit_Framework_MockObject_MockObject $controller */ @@ -92,7 +92,7 @@ public function testBeforeControllerWithoutAnnotation() { ->method('getRemoteAddress'); $this->throttler ->expects($this->never()) - ->method('sleepDelay'); + ->method('sleepDelayOrThrowOnMax'); /** @var Controller|\PHPUnit_Framework_MockObject_MockObject $controller */ $controller = $this->createMock(Controller::class);