From 454281af03fb0e917f0135cd6ce51fec5024a0b4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 3 Apr 2023 07:23:34 +0200 Subject: [PATCH] feat(security): Allow to opt-out of ratelimit protection, e.g. for testing on CI Signed-off-by: Joas Schilling --- config/config.sample.php | 11 +++++++++- .../RateLimiting/Backend/DatabaseBackend.php | 20 ++++++++++++------ .../Backend/MemoryCacheBackend.php | 21 +++++++++++++------ lib/private/Server.php | 2 ++ .../Backend/MemoryCacheBackendTest.php | 9 ++++++++ 5 files changed, 50 insertions(+), 13 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index 19dcc66f279..e87dee087a3 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -302,7 +302,7 @@ $CONFIG = [ /** * The interval at which token activity should be updated. - * Increasing this value means that the last activty on the security page gets + * Increasing this value means that the last activity on the security page gets * more outdated. * * Tokens are still checked every 5 minutes for validity @@ -321,6 +321,15 @@ $CONFIG = [ */ 'auth.bruteforce.protection.enabled' => true, +/** + * Whether the rate limit protection shipped with Nextcloud should be enabled or not. + * + * Disabling this is discouraged for security reasons. + * + * Defaults to ``true`` + */ +'ratelimit.protection.enabled' => true, + /** * By default, WebAuthn is available, but it can be explicitly disabled by admins */ diff --git a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php index ea4bd87d6cd..d1631a8d0ae 100644 --- a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php +++ b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php @@ -3,8 +3,10 @@ declare(strict_types=1); /** + * @copyright Copyright (c) 2023 Joas Schilling * @copyright Copyright (c) 2021 Lukas Reschke * + * @author Joas Schilling * @author Lukas Reschke * * @license GNU AGPL version 3 or any later version @@ -27,24 +29,25 @@ namespace OC\Security\RateLimiting\Backend; use OCP\AppFramework\Utility\ITimeFactory; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IConfig; use OCP\IDBConnection; class DatabaseBackend implements IBackend { private const TABLE_NAME = 'ratelimit_entries'; + /** @var IConfig */ + private $config; /** @var IDBConnection */ private $dbConnection; /** @var ITimeFactory */ private $timeFactory; - /** - * @param IDBConnection $dbConnection - * @param ITimeFactory $timeFactory - */ public function __construct( + IConfig $config, IDBConnection $dbConnection, ITimeFactory $timeFactory ) { + $this->config = $config; $this->dbConnection = $dbConnection; $this->timeFactory = $timeFactory; } @@ -115,7 +118,12 @@ class DatabaseBackend implements IBackend { ->values([ 'hash' => $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR), 'delete_after' => $qb->createNamedParameter($deleteAfter, IQueryBuilder::PARAM_DATE), - ]) - ->executeStatement(); + ]); + + if (!$this->config->getSystemValueBool('ratelimit.protection.enabled', true)) { + return; + } + + $qb->executeStatement(); } } diff --git a/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php b/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php index f4880fb239c..4bcb459c64e 100644 --- a/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php +++ b/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php @@ -3,9 +3,11 @@ declare(strict_types=1); /** + * @copyright Copyright (c) 2023 Joas Schilling * @copyright Copyright (c) 2017 Lukas Reschke * * @author Christoph Wurst + * @author Joas Schilling * @author Lukas Reschke * @author Morris Jobke * @author Roeland Jago Douma @@ -31,6 +33,7 @@ namespace OC\Security\RateLimiting\Backend; use OCP\AppFramework\Utility\ITimeFactory; use OCP\ICache; use OCP\ICacheFactory; +use OCP\IConfig; /** * Class MemoryCacheBackend uses the configured distributed memory cache for storing @@ -39,17 +42,18 @@ use OCP\ICacheFactory; * @package OC\Security\RateLimiting\Backend */ class MemoryCacheBackend implements IBackend { + /** @var IConfig */ + private $config; /** @var ICache */ private $cache; /** @var ITimeFactory */ private $timeFactory; - /** - * @param ICacheFactory $cacheFactory - * @param ITimeFactory $timeFactory - */ - public function __construct(ICacheFactory $cacheFactory, - ITimeFactory $timeFactory) { + public function __construct( + IConfig $config, + ICacheFactory $cacheFactory, + ITimeFactory $timeFactory) { + $this->config = $config; $this->cache = $cacheFactory->createDistributed(__CLASS__); $this->timeFactory = $timeFactory; } @@ -121,6 +125,11 @@ class MemoryCacheBackend implements IBackend { // Store the new attempt $existingAttempts[] = (string)($currentTime + $period); + + if (!$this->config->getSystemValueBool('ratelimit.protection.enabled', true)) { + return; + } + $this->cache->set($identifier, json_encode($existingAttempts)); } } diff --git a/lib/private/Server.php b/lib/private/Server.php index f1e96170886..e71fd0a9c29 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -846,11 +846,13 @@ class Server extends ServerContainer implements IServerContainer { $cacheFactory = $c->get(ICacheFactory::class); if ($cacheFactory->isAvailable()) { $backend = new \OC\Security\RateLimiting\Backend\MemoryCacheBackend( + $c->get(AllConfig::class), $this->get(ICacheFactory::class), new \OC\AppFramework\Utility\TimeFactory() ); } else { $backend = new \OC\Security\RateLimiting\Backend\DatabaseBackend( + $c->get(AllConfig::class), $c->get(IDBConnection::class), new \OC\AppFramework\Utility\TimeFactory() ); diff --git a/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php b/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php index e7cb4a93a29..a48b1211587 100644 --- a/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php +++ b/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php @@ -28,9 +28,12 @@ use OC\Security\RateLimiting\Backend\MemoryCacheBackend; use OCP\AppFramework\Utility\ITimeFactory; use OCP\ICache; use OCP\ICacheFactory; +use OCP\IConfig; use Test\TestCase; class MemoryCacheBackendTest extends TestCase { + /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ + private $config; /** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */ private $cacheFactory; /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ @@ -43,6 +46,7 @@ class MemoryCacheBackendTest extends TestCase { protected function setUp(): void { parent::setUp(); + $this->config = $this->createMock(IConfig::class); $this->cacheFactory = $this->createMock(ICacheFactory::class); $this->timeFactory = $this->createMock(ITimeFactory::class); $this->cache = $this->createMock(ICache::class); @@ -53,7 +57,12 @@ class MemoryCacheBackendTest extends TestCase { ->with('OC\Security\RateLimiting\Backend\MemoryCacheBackend') ->willReturn($this->cache); + $this->config->method('getSystemValueBool') + ->with('ratelimit.protection.enabled') + ->willReturn(true); + $this->memoryCache = new MemoryCacheBackend( + $this->config, $this->cacheFactory, $this->timeFactory );