Skip to content

Commit

Permalink
Merge pull request #37551 from nextcloud/backport/37542/stable26
Browse files Browse the repository at this point in the history
[stable26] feat(security): Allow to opt-out of ratelimit protection, e.g. for te…
  • Loading branch information
nickvergessen authored Apr 3, 2023
2 parents ca0a258 + 96204fe commit e9c1ad0
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 13 deletions.
11 changes: 10 additions & 1 deletion config/config.sample.php
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,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
Expand All @@ -320,6 +320,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
*/
Expand Down
20 changes: 14 additions & 6 deletions lib/private/Security/RateLimiting/Backend/DatabaseBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Expand Down Expand Up @@ -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();
}
}
21 changes: 15 additions & 6 deletions lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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
Expand All @@ -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;
}
Expand Down Expand Up @@ -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));
}
}
2 changes: 2 additions & 0 deletions lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -844,11 +844,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()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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);
Expand All @@ -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
);
Expand Down

0 comments on commit e9c1ad0

Please sign in to comment.