Merge pull request #37553 from nextcloud/backport/37542/stable24

[stable24] feat(security): Allow to opt-out of ratelimit protection, e.g. for te…

Author: nickvergessen

config/config.sample.php

@@ -284,7 +284,7 @@
 
 /**
  * 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
@@ -303,6 +303,15 @@
  */
 '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
  */

lib/private/Security/RateLimiting/Backend/DatabaseBackend.php

@@ -3,8 +3,10 @@
 declare(strict_types=1);
 
 /**
+ * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com>
  * @copyright Copyright (c) 2021 Lukas Reschke <lukas@statuscode.ch>
  *
+ * @author Joas Schilling <coding@schilljs.com>
  * @author Lukas Reschke <lukas@statuscode.ch>
  *
  * @license GNU AGPL version 3 or any later version
@@ -27,24 +29,25 @@
 
 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 @@ public function registerAttempt(string $methodIdentifier,
 			->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();
 	}
 }

lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php

@@ -3,9 +3,11 @@
 declare(strict_types=1);
 
 /**
+ * @copyright Copyright (c) 2023 Joas Schilling <coding@schilljs.com>
  * @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
  *
  * @author Christoph Wurst <christoph@winzerhof-wurst.at>
+ * @author Joas Schilling <coding@schilljs.com>
  * @author Lukas Reschke <lukas@statuscode.ch>
  * @author Morris Jobke <hey@morrisjobke.de>
  * @author Roeland Jago Douma <roeland@famdouma.nl>
@@ -31,6 +33,7 @@
 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 @@
  * @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 @@ public function registerAttempt(string $methodIdentifier,
 
 		// Store the new attempt
 		$existingAttempts[] = (string)($currentTime + $period);
+
+		if (!$this->config->getSystemValueBool('ratelimit.protection.enabled', true)) {
+			return;
+		}
+
 		$this->cache->set($identifier, json_encode($existingAttempts));
 	}
 }

lib/private/Server.php

@@ -832,11 +832,13 @@ public function __construct($webRoot, \OC\Config $config) {
 			$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()
 				);

tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php

@@ -28,9 +28,12 @@
 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 @@ protected function setUp(): void {
 			->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
 		);