From 4a3f3beb0babf312d83b2dc6feb7b036cd106529 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Wed, 9 Nov 2022 18:46:54 +0100 Subject: [PATCH] use bruteforce protection on all methods wrapped by PublicShareMiddleware if an invalid token is provided or when share password is wrong Signed-off-by: Julien Veyssier --- .../DependencyInjection/DIContainer.php | 3 ++- .../PublicShare/PublicShareMiddleware.php | 22 ++++++++++++++++++- .../PublicShare/PublicShareMiddlewareTest.php | 7 +++++- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 298b9f000e3..4c413a8ae87 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -304,7 +304,8 @@ class DIContainer extends SimpleContainer implements IAppContainer { new OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware( $c->get(IRequest::class), $c->get(ISession::class), - $c->get(\OCP\IConfig::class) + $c->get(\OCP\IConfig::class), + $c->get(OC\Security\Bruteforce\Throttler::class) ) ); $dispatcher->registerMiddleware( diff --git a/lib/private/AppFramework/Middleware/PublicShare/PublicShareMiddleware.php b/lib/private/AppFramework/Middleware/PublicShare/PublicShareMiddleware.php index d3beb4fd3a8..d956ce58912 100644 --- a/lib/private/AppFramework/Middleware/PublicShare/PublicShareMiddleware.php +++ b/lib/private/AppFramework/Middleware/PublicShare/PublicShareMiddleware.php @@ -24,6 +24,7 @@ namespace OC\AppFramework\Middleware\PublicShare; use OC\AppFramework\Middleware\PublicShare\Exceptions\NeedAuthenticationException; +use OC\Security\Bruteforce\Throttler; use OCP\AppFramework\AuthPublicShareController; use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\Middleware; @@ -34,6 +35,7 @@ use OCP\IRequest; use OCP\ISession; class PublicShareMiddleware extends Middleware { + /** @var IRequest */ private $request; @@ -43,10 +45,14 @@ class PublicShareMiddleware extends Middleware { /** @var IConfig */ private $config; - public function __construct(IRequest $request, ISession $session, IConfig $config) { + /** @var Throttler */ + private $throttler; + + public function __construct(IRequest $request, ISession $session, IConfig $config, Throttler $throttler) { $this->request = $request; $this->session = $session; $this->config = $config; + $this->throttler = $throttler; } public function beforeController($controller, $methodName) { @@ -54,6 +60,11 @@ class PublicShareMiddleware extends Middleware { return; } + $controllerClassPath = explode('\\', get_class($controller)); + $controllerShortClass = end($controllerClassPath); + $bruteforceProtectionAction = $controllerShortClass . '::' . $methodName; + $this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), $bruteforceProtectionAction); + if (!$this->isLinkSharingEnabled()) { throw new NotFoundException('Link sharing is disabled'); } @@ -68,6 +79,8 @@ class PublicShareMiddleware extends Middleware { $controller->setToken($token); if (!$controller->isValidToken()) { + $this->throttle($bruteforceProtectionAction, $token); + $controller->shareNotFound(); throw new NotFoundException(); } @@ -88,6 +101,7 @@ class PublicShareMiddleware extends Middleware { throw new NeedAuthenticationException(); } + $this->throttle($bruteforceProtectionAction, $token); throw new NotFoundException(); } @@ -128,4 +142,10 @@ class PublicShareMiddleware extends Middleware { return true; } + + private function throttle($bruteforceProtectionAction, $token): void { + $ip = $this->request->getRemoteAddress(); + $this->throttler->sleepDelay($ip, $bruteforceProtectionAction); + $this->throttler->registerAttempt($bruteforceProtectionAction, $ip, ['token' => $token]); + } } diff --git a/tests/lib/AppFramework/Middleware/PublicShare/PublicShareMiddlewareTest.php b/tests/lib/AppFramework/Middleware/PublicShare/PublicShareMiddlewareTest.php index 7e7140971e4..7d07bffc46b 100644 --- a/tests/lib/AppFramework/Middleware/PublicShare/PublicShareMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/PublicShare/PublicShareMiddlewareTest.php @@ -25,6 +25,7 @@ namespace Test\AppFramework\Middleware\PublicShare; use OC\AppFramework\Middleware\PublicShare\Exceptions\NeedAuthenticationException; use OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware; +use OC\Security\Bruteforce\Throttler; use OCP\AppFramework\AuthPublicShareController; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\NotFoundResponse; @@ -44,6 +45,8 @@ class PublicShareMiddlewareTest extends \Test\TestCase { private $session; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ private $config; + /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ + private $throttler; /** @var PublicShareMiddleware */ private $middleware; @@ -55,11 +58,13 @@ class PublicShareMiddlewareTest extends \Test\TestCase { $this->request = $this->createMock(IRequest::class); $this->session = $this->createMock(ISession::class); $this->config = $this->createMock(IConfig::class); + $this->throttler = $this->createMock(Throttler::class); $this->middleware = new PublicShareMiddleware( $this->request, $this->session, - $this->config + $this->config, + $this->throttler ); }