From d4f97affc1a0ecfaacfbdc26aab820cad1650a06 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 6 Sep 2021 16:31:01 +0200 Subject: [PATCH 1/8] Add database ratelimiting backend In case no distributed memory cache is specified this adds a database backend for ratelimit purposes. Signed-off-by: Lukas Reschke --- .../Version23000Date20210906132259.php | 47 ++++++ lib/composer/composer/autoload_classmap.php | 6 + lib/composer/composer/autoload_psr4.php | 1 + lib/composer/composer/autoload_static.php | 14 ++ lib/composer/composer/installed.json | 62 +++++++- lib/composer/composer/installed.php | 15 +- .../RateLimiting/Backend/DatabaseBackend.php | 136 ++++++++++++++++++ lib/private/Server.php | 18 ++- version.php | 2 +- 9 files changed, 290 insertions(+), 11 deletions(-) create mode 100644 core/Migrations/Version23000Date20210906132259.php create mode 100644 lib/private/Security/RateLimiting/Backend/DatabaseBackend.php diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php new file mode 100644 index 0000000000000..b0eb7bc4dcce3 --- /dev/null +++ b/core/Migrations/Version23000Date20210906132259.php @@ -0,0 +1,47 @@ +hasTable(self::TABLE_NAME); + + if(!$hasTable) + { + $table = $schema->createTable(self::TABLE_NAME); + $table->addColumn('hash', Types::STRING, [ + 'notnull' => true, + 'length' => 128, + ]); + $table->addColumn('timestamp', 'datetime', [ + 'notnull' => true, + ]); + $table->addIndex(['hash'], 'ratelimit_hash_idx'); + $table->addIndex(['timestamp'], 'ratelimit_timestamp_idx'); + } + + return $schema; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index ea1473f27dc34..ca0607a5d3584 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -6,6 +6,10 @@ $baseDir = dirname(dirname($vendorDir)); return array( + 'Bamarni\\Composer\\Bin\\BinCommand' => $vendorDir . '/bamarni/composer-bin-plugin/src/BinCommand.php', + 'Bamarni\\Composer\\Bin\\CommandProvider' => $vendorDir . '/bamarni/composer-bin-plugin/src/CommandProvider.php', + 'Bamarni\\Composer\\Bin\\Config' => $vendorDir . '/bamarni/composer-bin-plugin/src/Config.php', + 'Bamarni\\Composer\\Bin\\Plugin' => $vendorDir . '/bamarni/composer-bin-plugin/src/Plugin.php', 'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php', 'OCP\\Accounts\\IAccount' => $baseDir . '/lib/public/Accounts/IAccount.php', 'OCP\\Accounts\\IAccountManager' => $baseDir . '/lib/public/Accounts/IAccountManager.php', @@ -969,6 +973,7 @@ 'OC\\Core\\Migrations\\Version21000Date20210309185126' => $baseDir . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version21000Date20210309185127' => $baseDir . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => $baseDir . '/core/Migrations/Version22000Date20210216080825.php', + 'OC\\Core\\Migrations\\Version23000Date20210906132259' => $baseDir . '/core/Migrations/Version23000Date20210906132259.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', @@ -1365,6 +1370,7 @@ 'OC\\Security\\IdentityProof\\Manager' => $baseDir . '/lib/private/Security/IdentityProof/Manager.php', 'OC\\Security\\IdentityProof\\Signer' => $baseDir . '/lib/private/Security/IdentityProof/Signer.php', 'OC\\Security\\Normalizer\\IpAddress' => $baseDir . '/lib/private/Security/Normalizer/IpAddress.php', + 'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php', 'OC\\Security\\RateLimiting\\Backend\\IBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/IBackend.php', 'OC\\Security\\RateLimiting\\Backend\\MemoryCache' => $baseDir . '/lib/private/Security/RateLimiting/Backend/MemoryCache.php', 'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => $baseDir . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php', diff --git a/lib/composer/composer/autoload_psr4.php b/lib/composer/composer/autoload_psr4.php index b641d9c6a0319..fe0f6a5fed280 100644 --- a/lib/composer/composer/autoload_psr4.php +++ b/lib/composer/composer/autoload_psr4.php @@ -9,5 +9,6 @@ 'OC\\Core\\' => array($baseDir . '/core'), 'OC\\' => array($baseDir . '/lib/private'), 'OCP\\' => array($baseDir . '/lib/public'), + 'Bamarni\\Composer\\Bin\\' => array($vendorDir . '/bamarni/composer-bin-plugin/src'), '' => array($baseDir . '/lib/private/legacy'), ); diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 93208f6ff1502..98633252c402e 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -13,6 +13,10 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\' => 3, 'OCP\\' => 4, ), + 'B' => + array ( + 'Bamarni\\Composer\\Bin\\' => 21, + ), ); public static $prefixDirsPsr4 = array ( @@ -28,6 +32,10 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c array ( 0 => __DIR__ . '/../../..' . '/lib/public', ), + 'Bamarni\\Composer\\Bin\\' => + array ( + 0 => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src', + ), ); public static $fallbackDirsPsr4 = array ( @@ -35,6 +43,10 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c ); public static $classMap = array ( + 'Bamarni\\Composer\\Bin\\BinCommand' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/BinCommand.php', + 'Bamarni\\Composer\\Bin\\CommandProvider' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/CommandProvider.php', + 'Bamarni\\Composer\\Bin\\Config' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/Config.php', + 'Bamarni\\Composer\\Bin\\Plugin' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/Plugin.php', 'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php', 'OCP\\Accounts\\IAccount' => __DIR__ . '/../../..' . '/lib/public/Accounts/IAccount.php', 'OCP\\Accounts\\IAccountManager' => __DIR__ . '/../../..' . '/lib/public/Accounts/IAccountManager.php', @@ -998,6 +1010,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Migrations\\Version21000Date20210309185126' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version21000Date20210309185127' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => __DIR__ . '/../../..' . '/core/Migrations/Version22000Date20210216080825.php', + 'OC\\Core\\Migrations\\Version23000Date20210906132259' => __DIR__ . '/../../..' . '/core/Migrations/Version23000Date20210906132259.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', @@ -1394,6 +1407,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Security\\IdentityProof\\Manager' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Manager.php', 'OC\\Security\\IdentityProof\\Signer' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Signer.php', 'OC\\Security\\Normalizer\\IpAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Normalizer/IpAddress.php', + 'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php', 'OC\\Security\\RateLimiting\\Backend\\IBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/IBackend.php', 'OC\\Security\\RateLimiting\\Backend\\MemoryCache' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/MemoryCache.php', 'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php', diff --git a/lib/composer/composer/installed.json b/lib/composer/composer/installed.json index f20a6c47c6d4f..9c4acdfa4832e 100644 --- a/lib/composer/composer/installed.json +++ b/lib/composer/composer/installed.json @@ -1,5 +1,61 @@ { - "packages": [], - "dev": false, - "dev-package-names": [] + "packages": [ + { + "name": "bamarni/composer-bin-plugin", + "version": "1.4.1", + "version_normalized": "1.4.1.0", + "source": { + "type": "git", + "url": "https://github.com/bamarni/composer-bin-plugin.git", + "reference": "9329fb0fbe29e0e1b2db8f4639a193e4f5406225" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/bamarni/composer-bin-plugin/zipball/9329fb0fbe29e0e1b2db8f4639a193e4f5406225", + "reference": "9329fb0fbe29e0e1b2db8f4639a193e4f5406225", + "shasum": "" + }, + "require": { + "composer-plugin-api": "^1.0 || ^2.0", + "php": "^5.5.9 || ^7.0 || ^8.0" + }, + "require-dev": { + "composer/composer": "^1.0 || ^2.0", + "symfony/console": "^2.5 || ^3.0 || ^4.0" + }, + "time": "2020-05-03T08:27:20+00:00", + "type": "composer-plugin", + "extra": { + "class": "Bamarni\\Composer\\Bin\\Plugin" + }, + "installation-source": "dist", + "autoload": { + "psr-4": { + "Bamarni\\Composer\\Bin\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "No conflicts for your bin dependencies", + "keywords": [ + "composer", + "conflict", + "dependency", + "executable", + "isolation", + "tool" + ], + "support": { + "issues": "https://github.com/bamarni/composer-bin-plugin/issues", + "source": "https://github.com/bamarni/composer-bin-plugin/tree/master" + }, + "install-path": "../bamarni/composer-bin-plugin" + } + ], + "dev": true, + "dev-package-names": [ + "bamarni/composer-bin-plugin" + ] } diff --git a/lib/composer/composer/installed.php b/lib/composer/composer/installed.php index b811738fbd0f0..583c0c32593d6 100644 --- a/lib/composer/composer/installed.php +++ b/lib/composer/composer/installed.php @@ -5,9 +5,9 @@ 'type' => 'library', 'install_path' => __DIR__ . '/../../../', 'aliases' => array(), - 'reference' => '66144c300395458ff38b86e50cd92174443cd85e', + 'reference' => '33a0b75c83a1c56fa84b98d3a07a26b5c4932b65', 'name' => '__root__', - 'dev' => false, + 'dev' => true, ), 'versions' => array( '__root__' => array( @@ -16,8 +16,17 @@ 'type' => 'library', 'install_path' => __DIR__ . '/../../../', 'aliases' => array(), - 'reference' => '66144c300395458ff38b86e50cd92174443cd85e', + 'reference' => '33a0b75c83a1c56fa84b98d3a07a26b5c4932b65', 'dev_requirement' => false, ), + 'bamarni/composer-bin-plugin' => array( + 'pretty_version' => '1.4.1', + 'version' => '1.4.1.0', + 'type' => 'composer-plugin', + 'install_path' => __DIR__ . '/../bamarni/composer-bin-plugin', + 'aliases' => array(), + 'reference' => '9329fb0fbe29e0e1b2db8f4639a193e4f5406225', + 'dev_requirement' => true, + ), ), ); diff --git a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php new file mode 100644 index 0000000000000..a7f2a7b4b6292 --- /dev/null +++ b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php @@ -0,0 +1,136 @@ + + * + * @author Lukas Reschke + * + * @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 OC\Security\RateLimiting\Backend; + +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; + +/** + * Class DatabaseBackend uses the database for storing rate limiting data. + * + * @package OC\Security\RateLimiting\Backend + */ +class DatabaseBackend implements IBackend { + private const TABLE_NAME = 'ratelimit_entries'; + + /** @var IDBConnection */ + private $dbConnection; + /** @var ITimeFactory */ + private $timeFactory; + + /** + * @param IDBConnection $dbConnection + * @param ITimeFactory $timeFactory + */ + public function __construct( + IDBConnection $dbConnection, + ITimeFactory $timeFactory + ) { + $this->dbConnection = $dbConnection; + $this->timeFactory = $timeFactory; + } + + /** + * @param string $methodIdentifier + * @param string $userIdentifier + * @return string + */ + private function hash(string $methodIdentifier, + string $userIdentifier): string { + return hash('sha512', $methodIdentifier . $userIdentifier); + } + + /** + * @param string $identifier + * @param int $seconds + * @return int + * @throws \OCP\DB\Exception + */ + private function getExistingAttemptCount( + string $identifier, + int $seconds + ): int { + $qb = $this->dbConnection->getQueryBuilder(); + $notOlderThan = $this->timeFactory->getDateTime()->sub(new \DateInterval("PT{$seconds}S")); + + $qb->selectAlias($qb->createFunction('COUNT(*)'), 'count') + ->from(self::TABLE_NAME) + ->where( + $qb->expr()->eq('hash', $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->gte('timestamp', $qb->createParameter('notOlderThan')) + ) + ->setParameter('notOlderThan', $notOlderThan, 'datetime'); + + $cursor = $qb->executeQuery(); + $row = $cursor->fetch(); + $cursor->closeCursor(); + + return (int)$row['count']; + } + + /** + * {@inheritDoc} + */ + public function getAttempts(string $methodIdentifier, + string $userIdentifier, + int $seconds): int { + $identifier = $this->hash($methodIdentifier, $userIdentifier); + return $this->getExistingAttemptCount($identifier, $seconds); + } + + /** + * {@inheritDoc} + */ + public function registerAttempt(string $methodIdentifier, + string $userIdentifier, + int $period) { + $identifier = $this->hash($methodIdentifier, $userIdentifier); + $currentTime = $this->timeFactory->getDateTime(); + $notOlderThan = $this->timeFactory->getDateTime('@' . $period); + + $qb = $this->dbConnection->getQueryBuilder(); + + $qb->delete(self::TABLE_NAME) + ->where( + $qb->expr()->eq('hash', $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR)) + ) + ->andWhere( + $qb->expr()->lt('timestamp', $qb->createParameter('notOlderThan')) + ) + ->setParameter('notOlderThan', $notOlderThan, 'datetime') + ->executeStatement(); + + $qb->insert(self::TABLE_NAME) + ->values([ + 'hash' => $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR), + 'timestamp' => $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATE), + ]) + ->executeStatement(); + } +} diff --git a/lib/private/Server.php b/lib/private/Server.php index 0320eda2b91c7..83a3c905f9486 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -785,10 +785,20 @@ public function __construct($webRoot, \OC\Config $config) { $this->registerDeprecatedAlias('Search', ISearch::class); $this->registerService(\OC\Security\RateLimiting\Backend\IBackend::class, function ($c) { - return new \OC\Security\RateLimiting\Backend\MemoryCache( - $this->get(ICacheFactory::class), - new \OC\AppFramework\Utility\TimeFactory() - ); + $cacheFactory = $c->get(ICacheFactory::class); + if ($cacheFactory->isAvailable()) { + $backend = new \OC\Security\RateLimiting\Backend\MemoryCache( + $this->get(ICacheFactory::class), + new \OC\AppFramework\Utility\TimeFactory() + ); + } else { + $backend = new \OC\Security\RateLimiting\Backend\DatabaseBackend( + $c->get(IDBConnection::class), + new \OC\AppFramework\Utility\TimeFactory() + ); + } + + return $backend; }); $this->registerAlias(\OCP\Security\ISecureRandom::class, SecureRandom::class); diff --git a/version.php b/version.php index 0f7f176f04c14..045fa2866b693 100644 --- a/version.php +++ b/version.php @@ -30,7 +30,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel // when updating major/minor version number. -$OC_Version = [23, 0, 0, 0]; +$OC_Version = [23, 0, 0, 1]; // The human readable string $OC_VersionString = '23.0.0 alpha'; From 378cc922c429524b872e83c7b3842eb86bc4b770 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 6 Sep 2021 17:31:36 +0200 Subject: [PATCH 2/8] Adjust logic to store period instead of current timestamp Signed-off-by: Lukas Reschke --- .../Version23000Date20210906132259.php | 4 +- lib/composer/composer/autoload_classmap.php | 6 -- lib/composer/composer/autoload_psr4.php | 1 - lib/composer/composer/autoload_static.php | 14 ----- lib/composer/composer/installed.json | 62 +------------------ lib/composer/composer/installed.php | 15 +---- .../RateLimiting/Backend/DatabaseBackend.php | 37 +++++------ .../RateLimiting/Backend/IBackend.php | 6 +- ...MemoryCache.php => MemoryCacheBackend.php} | 20 +++--- lib/private/Security/RateLimiting/Limiter.php | 11 +--- lib/private/Server.php | 2 +- ...cheTest.php => MemoryCacheBackendTest.php} | 14 ++--- .../lib/Security/RateLimiting/LimiterTest.php | 13 ---- 13 files changed, 46 insertions(+), 159 deletions(-) rename lib/private/Security/RateLimiting/Backend/{MemoryCache.php => MemoryCacheBackend.php} (86%) rename tests/lib/Security/RateLimiting/Backend/{MemoryCacheTest.php => MemoryCacheBackendTest.php} (93%) diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php index b0eb7bc4dcce3..c4e0c42075c6d 100644 --- a/core/Migrations/Version23000Date20210906132259.php +++ b/core/Migrations/Version23000Date20210906132259.php @@ -35,11 +35,11 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt 'notnull' => true, 'length' => 128, ]); - $table->addColumn('timestamp', 'datetime', [ + $table->addColumn('delete_after', 'datetime', [ 'notnull' => true, ]); $table->addIndex(['hash'], 'ratelimit_hash_idx'); - $table->addIndex(['timestamp'], 'ratelimit_timestamp_idx'); + $table->addIndex(['delete_after'], 'ratelimit_delete_after_idx'); } return $schema; diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index ca0607a5d3584..ea1473f27dc34 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -6,10 +6,6 @@ $baseDir = dirname(dirname($vendorDir)); return array( - 'Bamarni\\Composer\\Bin\\BinCommand' => $vendorDir . '/bamarni/composer-bin-plugin/src/BinCommand.php', - 'Bamarni\\Composer\\Bin\\CommandProvider' => $vendorDir . '/bamarni/composer-bin-plugin/src/CommandProvider.php', - 'Bamarni\\Composer\\Bin\\Config' => $vendorDir . '/bamarni/composer-bin-plugin/src/Config.php', - 'Bamarni\\Composer\\Bin\\Plugin' => $vendorDir . '/bamarni/composer-bin-plugin/src/Plugin.php', 'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php', 'OCP\\Accounts\\IAccount' => $baseDir . '/lib/public/Accounts/IAccount.php', 'OCP\\Accounts\\IAccountManager' => $baseDir . '/lib/public/Accounts/IAccountManager.php', @@ -973,7 +969,6 @@ 'OC\\Core\\Migrations\\Version21000Date20210309185126' => $baseDir . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version21000Date20210309185127' => $baseDir . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => $baseDir . '/core/Migrations/Version22000Date20210216080825.php', - 'OC\\Core\\Migrations\\Version23000Date20210906132259' => $baseDir . '/core/Migrations/Version23000Date20210906132259.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', @@ -1370,7 +1365,6 @@ 'OC\\Security\\IdentityProof\\Manager' => $baseDir . '/lib/private/Security/IdentityProof/Manager.php', 'OC\\Security\\IdentityProof\\Signer' => $baseDir . '/lib/private/Security/IdentityProof/Signer.php', 'OC\\Security\\Normalizer\\IpAddress' => $baseDir . '/lib/private/Security/Normalizer/IpAddress.php', - 'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php', 'OC\\Security\\RateLimiting\\Backend\\IBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/IBackend.php', 'OC\\Security\\RateLimiting\\Backend\\MemoryCache' => $baseDir . '/lib/private/Security/RateLimiting/Backend/MemoryCache.php', 'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => $baseDir . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php', diff --git a/lib/composer/composer/autoload_psr4.php b/lib/composer/composer/autoload_psr4.php index fe0f6a5fed280..b641d9c6a0319 100644 --- a/lib/composer/composer/autoload_psr4.php +++ b/lib/composer/composer/autoload_psr4.php @@ -9,6 +9,5 @@ 'OC\\Core\\' => array($baseDir . '/core'), 'OC\\' => array($baseDir . '/lib/private'), 'OCP\\' => array($baseDir . '/lib/public'), - 'Bamarni\\Composer\\Bin\\' => array($vendorDir . '/bamarni/composer-bin-plugin/src'), '' => array($baseDir . '/lib/private/legacy'), ); diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 98633252c402e..93208f6ff1502 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -13,10 +13,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\' => 3, 'OCP\\' => 4, ), - 'B' => - array ( - 'Bamarni\\Composer\\Bin\\' => 21, - ), ); public static $prefixDirsPsr4 = array ( @@ -32,10 +28,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c array ( 0 => __DIR__ . '/../../..' . '/lib/public', ), - 'Bamarni\\Composer\\Bin\\' => - array ( - 0 => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src', - ), ); public static $fallbackDirsPsr4 = array ( @@ -43,10 +35,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c ); public static $classMap = array ( - 'Bamarni\\Composer\\Bin\\BinCommand' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/BinCommand.php', - 'Bamarni\\Composer\\Bin\\CommandProvider' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/CommandProvider.php', - 'Bamarni\\Composer\\Bin\\Config' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/Config.php', - 'Bamarni\\Composer\\Bin\\Plugin' => __DIR__ . '/..' . '/bamarni/composer-bin-plugin/src/Plugin.php', 'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php', 'OCP\\Accounts\\IAccount' => __DIR__ . '/../../..' . '/lib/public/Accounts/IAccount.php', 'OCP\\Accounts\\IAccountManager' => __DIR__ . '/../../..' . '/lib/public/Accounts/IAccountManager.php', @@ -1010,7 +998,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Migrations\\Version21000Date20210309185126' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version21000Date20210309185127' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => __DIR__ . '/../../..' . '/core/Migrations/Version22000Date20210216080825.php', - 'OC\\Core\\Migrations\\Version23000Date20210906132259' => __DIR__ . '/../../..' . '/core/Migrations/Version23000Date20210906132259.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', @@ -1407,7 +1394,6 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Security\\IdentityProof\\Manager' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Manager.php', 'OC\\Security\\IdentityProof\\Signer' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Signer.php', 'OC\\Security\\Normalizer\\IpAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Normalizer/IpAddress.php', - 'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php', 'OC\\Security\\RateLimiting\\Backend\\IBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/IBackend.php', 'OC\\Security\\RateLimiting\\Backend\\MemoryCache' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/MemoryCache.php', 'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php', diff --git a/lib/composer/composer/installed.json b/lib/composer/composer/installed.json index 9c4acdfa4832e..f20a6c47c6d4f 100644 --- a/lib/composer/composer/installed.json +++ b/lib/composer/composer/installed.json @@ -1,61 +1,5 @@ { - "packages": [ - { - "name": "bamarni/composer-bin-plugin", - "version": "1.4.1", - "version_normalized": "1.4.1.0", - "source": { - "type": "git", - "url": "https://github.com/bamarni/composer-bin-plugin.git", - "reference": "9329fb0fbe29e0e1b2db8f4639a193e4f5406225" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/bamarni/composer-bin-plugin/zipball/9329fb0fbe29e0e1b2db8f4639a193e4f5406225", - "reference": "9329fb0fbe29e0e1b2db8f4639a193e4f5406225", - "shasum": "" - }, - "require": { - "composer-plugin-api": "^1.0 || ^2.0", - "php": "^5.5.9 || ^7.0 || ^8.0" - }, - "require-dev": { - "composer/composer": "^1.0 || ^2.0", - "symfony/console": "^2.5 || ^3.0 || ^4.0" - }, - "time": "2020-05-03T08:27:20+00:00", - "type": "composer-plugin", - "extra": { - "class": "Bamarni\\Composer\\Bin\\Plugin" - }, - "installation-source": "dist", - "autoload": { - "psr-4": { - "Bamarni\\Composer\\Bin\\": "src" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "description": "No conflicts for your bin dependencies", - "keywords": [ - "composer", - "conflict", - "dependency", - "executable", - "isolation", - "tool" - ], - "support": { - "issues": "https://github.com/bamarni/composer-bin-plugin/issues", - "source": "https://github.com/bamarni/composer-bin-plugin/tree/master" - }, - "install-path": "../bamarni/composer-bin-plugin" - } - ], - "dev": true, - "dev-package-names": [ - "bamarni/composer-bin-plugin" - ] + "packages": [], + "dev": false, + "dev-package-names": [] } diff --git a/lib/composer/composer/installed.php b/lib/composer/composer/installed.php index 583c0c32593d6..b811738fbd0f0 100644 --- a/lib/composer/composer/installed.php +++ b/lib/composer/composer/installed.php @@ -5,9 +5,9 @@ 'type' => 'library', 'install_path' => __DIR__ . '/../../../', 'aliases' => array(), - 'reference' => '33a0b75c83a1c56fa84b98d3a07a26b5c4932b65', + 'reference' => '66144c300395458ff38b86e50cd92174443cd85e', 'name' => '__root__', - 'dev' => true, + 'dev' => false, ), 'versions' => array( '__root__' => array( @@ -16,17 +16,8 @@ 'type' => 'library', 'install_path' => __DIR__ . '/../../../', 'aliases' => array(), - 'reference' => '33a0b75c83a1c56fa84b98d3a07a26b5c4932b65', + 'reference' => '66144c300395458ff38b86e50cd92174443cd85e', 'dev_requirement' => false, ), - 'bamarni/composer-bin-plugin' => array( - 'pretty_version' => '1.4.1', - 'version' => '1.4.1.0', - 'type' => 'composer-plugin', - 'install_path' => __DIR__ . '/../bamarni/composer-bin-plugin', - 'aliases' => array(), - 'reference' => '9329fb0fbe29e0e1b2db8f4639a193e4f5406225', - 'dev_requirement' => true, - ), ), ); diff --git a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php index a7f2a7b4b6292..3b7382d747e32 100644 --- a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php +++ b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php @@ -71,21 +71,28 @@ private function hash(string $methodIdentifier, * @throws \OCP\DB\Exception */ private function getExistingAttemptCount( - string $identifier, - int $seconds + string $identifier ): int { + $currentTime = $this->timeFactory->getDateTime(); + $qb = $this->dbConnection->getQueryBuilder(); - $notOlderThan = $this->timeFactory->getDateTime()->sub(new \DateInterval("PT{$seconds}S")); + $qb->delete(self::TABLE_NAME) + ->where( + $qb->expr()->lte('delete_after', $qb->createParameter('currentTime')) + ) + ->setParameter('currentTime', $currentTime, 'datetime') + ->executeStatement(); + $qb = $this->dbConnection->getQueryBuilder(); $qb->selectAlias($qb->createFunction('COUNT(*)'), 'count') ->from(self::TABLE_NAME) ->where( $qb->expr()->eq('hash', $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR)) ) ->andWhere( - $qb->expr()->gte('timestamp', $qb->createParameter('notOlderThan')) + $qb->expr()->gte('delete_after', $qb->createParameter('currentTime')) ) - ->setParameter('notOlderThan', $notOlderThan, 'datetime'); + ->setParameter('currentTime', $currentTime, 'datetime'); $cursor = $qb->executeQuery(); $row = $cursor->fetch(); @@ -98,10 +105,9 @@ private function getExistingAttemptCount( * {@inheritDoc} */ public function getAttempts(string $methodIdentifier, - string $userIdentifier, - int $seconds): int { + string $userIdentifier): int { $identifier = $this->hash($methodIdentifier, $userIdentifier); - return $this->getExistingAttemptCount($identifier, $seconds); + return $this->getExistingAttemptCount($identifier); } /** @@ -111,25 +117,14 @@ public function registerAttempt(string $methodIdentifier, string $userIdentifier, int $period) { $identifier = $this->hash($methodIdentifier, $userIdentifier); - $currentTime = $this->timeFactory->getDateTime(); - $notOlderThan = $this->timeFactory->getDateTime('@' . $period); + $deleteAfter = $this->timeFactory->getDateTime()->add(new \DateInterval("PT{$period}S")); $qb = $this->dbConnection->getQueryBuilder(); - $qb->delete(self::TABLE_NAME) - ->where( - $qb->expr()->eq('hash', $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR)) - ) - ->andWhere( - $qb->expr()->lt('timestamp', $qb->createParameter('notOlderThan')) - ) - ->setParameter('notOlderThan', $notOlderThan, 'datetime') - ->executeStatement(); - $qb->insert(self::TABLE_NAME) ->values([ 'hash' => $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR), - 'timestamp' => $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATE), + 'delete_after' => $qb->createNamedParameter($deleteAfter, IQueryBuilder::PARAM_DATE), ]) ->executeStatement(); } diff --git a/lib/private/Security/RateLimiting/Backend/IBackend.php b/lib/private/Security/RateLimiting/Backend/IBackend.php index d87f53311b2e3..835a97993da6f 100644 --- a/lib/private/Security/RateLimiting/Backend/IBackend.php +++ b/lib/private/Security/RateLimiting/Backend/IBackend.php @@ -35,16 +35,14 @@ */ interface IBackend { /** - * Gets the amount of attempts within the last specified seconds + * Gets the amount of attempts for the specified method * * @param string $methodIdentifier Identifier for the method * @param string $userIdentifier Identifier for the user - * @param int $seconds Seconds to look back at * @return int */ public function getAttempts(string $methodIdentifier, - string $userIdentifier, - int $seconds): int; + string $userIdentifier): int; /** * Registers an attempt diff --git a/lib/private/Security/RateLimiting/Backend/MemoryCache.php b/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php similarity index 86% rename from lib/private/Security/RateLimiting/Backend/MemoryCache.php rename to lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php index 0dab25e4048bc..f4880fb239c16 100644 --- a/lib/private/Security/RateLimiting/Backend/MemoryCache.php +++ b/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php @@ -33,12 +33,12 @@ use OCP\ICacheFactory; /** - * Class MemoryCache uses the configured distributed memory cache for storing + * Class MemoryCacheBackend uses the configured distributed memory cache for storing * rate limiting data. * * @package OC\Security\RateLimiting\Backend */ -class MemoryCache implements IBackend { +class MemoryCacheBackend implements IBackend { /** @var ICache */ private $cache; /** @var ITimeFactory */ @@ -86,16 +86,14 @@ private function getExistingAttempts(string $identifier): array { * {@inheritDoc} */ public function getAttempts(string $methodIdentifier, - string $userIdentifier, - int $seconds): int { + string $userIdentifier): int { $identifier = $this->hash($methodIdentifier, $userIdentifier); $existingAttempts = $this->getExistingAttempts($identifier); $count = 0; $currentTime = $this->timeFactory->getTime(); - /** @var array $existingAttempts */ - foreach ($existingAttempts as $attempt) { - if (($attempt + $seconds) > $currentTime) { + foreach ($existingAttempts as $expirationTime) { + if ($expirationTime > $currentTime) { $count++; } } @@ -113,16 +111,16 @@ public function registerAttempt(string $methodIdentifier, $existingAttempts = $this->getExistingAttempts($identifier); $currentTime = $this->timeFactory->getTime(); - // Unset all attempts older than $period - foreach ($existingAttempts as $key => $attempt) { - if (($attempt + $period) < $currentTime) { + // Unset all attempts that are already expired + foreach ($existingAttempts as $key => $expirationTime) { + if ($expirationTime < $currentTime) { unset($existingAttempts[$key]); } } $existingAttempts = array_values($existingAttempts); // Store the new attempt - $existingAttempts[] = (string)$currentTime; + $existingAttempts[] = (string)($currentTime + $period); $this->cache->set($identifier, json_encode($existingAttempts)); } } diff --git a/lib/private/Security/RateLimiting/Limiter.php b/lib/private/Security/RateLimiting/Limiter.php index ede72e887fcb1..1c14fce2a55e8 100644 --- a/lib/private/Security/RateLimiting/Limiter.php +++ b/lib/private/Security/RateLimiting/Limiter.php @@ -35,17 +35,12 @@ class Limiter { /** @var IBackend */ private $backend; - /** @var ITimeFactory */ - private $timeFactory; /** - * @param ITimeFactory $timeFactory * @param IBackend $backend */ - public function __construct(ITimeFactory $timeFactory, - IBackend $backend) { + public function __construct(IBackend $backend) { $this->backend = $backend; - $this->timeFactory = $timeFactory; } /** @@ -59,12 +54,12 @@ private function register(string $methodIdentifier, string $userIdentifier, int $period, int $limit): void { - $existingAttempts = $this->backend->getAttempts($methodIdentifier, $userIdentifier, $period); + $existingAttempts = $this->backend->getAttempts($methodIdentifier, $userIdentifier); if ($existingAttempts >= $limit) { throw new RateLimitExceededException(); } - $this->backend->registerAttempt($methodIdentifier, $userIdentifier, $this->timeFactory->getTime()); + $this->backend->registerAttempt($methodIdentifier, $userIdentifier, $period); } /** diff --git a/lib/private/Server.php b/lib/private/Server.php index 83a3c905f9486..a70b25d90af62 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -787,7 +787,7 @@ public function __construct($webRoot, \OC\Config $config) { $this->registerService(\OC\Security\RateLimiting\Backend\IBackend::class, function ($c) { $cacheFactory = $c->get(ICacheFactory::class); if ($cacheFactory->isAvailable()) { - $backend = new \OC\Security\RateLimiting\Backend\MemoryCache( + $backend = new \OC\Security\RateLimiting\Backend\MemoryCacheBackend( $this->get(ICacheFactory::class), new \OC\AppFramework\Utility\TimeFactory() ); diff --git a/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php b/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php similarity index 93% rename from tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php rename to tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php index ff58bd5c09efa..8bbf88e8a8e08 100644 --- a/tests/lib/Security/RateLimiting/Backend/MemoryCacheTest.php +++ b/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php @@ -24,20 +24,20 @@ namespace Test\Security\RateLimiting\Backend; -use OC\Security\RateLimiting\Backend\MemoryCache; +use OC\Security\RateLimiting\Backend\MemoryCacheBackend; use OCP\AppFramework\Utility\ITimeFactory; use OCP\ICache; use OCP\ICacheFactory; use Test\TestCase; -class MemoryCacheTest extends TestCase { +class MemoryCacheBackendTest extends TestCase { /** @var ICacheFactory|\PHPUnit\Framework\MockObject\MockObject */ private $cacheFactory; /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ private $timeFactory; /** @var ICache|\PHPUnit\Framework\MockObject\MockObject */ private $cache; - /** @var MemoryCache */ + /** @var MemoryCacheBackend */ private $memoryCache; protected function setUp(): void { @@ -50,10 +50,10 @@ protected function setUp(): void { $this->cacheFactory ->expects($this->once()) ->method('createDistributed') - ->with('OC\Security\RateLimiting\Backend\MemoryCache') + ->with('OC\Security\RateLimiting\Backend\MemoryCacheBackend') ->willReturn($this->cache); - $this->memoryCache = new MemoryCache( + $this->memoryCache = new MemoryCacheBackend( $this->cacheFactory, $this->timeFactory ); @@ -66,7 +66,7 @@ public function testGetAttemptsWithNoAttemptsBefore() { ->with('eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b') ->willReturn(null); - $this->assertSame(0, $this->memoryCache->getAttempts('Method', 'User', 123)); + $this->assertSame(0, $this->memoryCache->getAttempts('Method', 'User')); } public function testGetAttempts() { @@ -87,7 +87,7 @@ public function testGetAttempts() { '124', ])); - $this->assertSame(3, $this->memoryCache->getAttempts('Method', 'User', 123)); + $this->assertSame(3, $this->memoryCache->getAttempts('Method', 'User')); } public function testRegisterAttemptWithNoAttemptsBefore() { diff --git a/tests/lib/Security/RateLimiting/LimiterTest.php b/tests/lib/Security/RateLimiting/LimiterTest.php index 8b3509a4790bf..31c877cbda392 100644 --- a/tests/lib/Security/RateLimiting/LimiterTest.php +++ b/tests/lib/Security/RateLimiting/LimiterTest.php @@ -26,13 +26,10 @@ use OC\Security\RateLimiting\Backend\IBackend; use OC\Security\RateLimiting\Limiter; -use OCP\AppFramework\Utility\ITimeFactory; use OCP\IUser; use Test\TestCase; class LimiterTest extends TestCase { - /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ - private $timeFactory; /** @var IBackend|\PHPUnit\Framework\MockObject\MockObject */ private $backend; /** @var Limiter */ @@ -41,11 +38,9 @@ class LimiterTest extends TestCase { protected function setUp(): void { parent::setUp(); - $this->timeFactory = $this->createMock(ITimeFactory::class); $this->backend = $this->createMock(IBackend::class); $this->limiter = new Limiter( - $this->timeFactory, $this->backend ); } @@ -69,10 +64,6 @@ public function testRegisterAnonRequestExceeded() { } public function testRegisterAnonRequestSuccess() { - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(2000); $this->backend ->expects($this->once()) ->method('getAttempts') @@ -126,10 +117,6 @@ public function testRegisterUserRequestSuccess() { ->method('getUID') ->willReturn('MyUid'); - $this->timeFactory - ->expects($this->once()) - ->method('getTime') - ->willReturn(2000); $this->backend ->expects($this->once()) ->method('getAttempts') From 5600c7b43fba3a71f0426932ade6737758665311 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 6 Sep 2021 17:32:25 +0200 Subject: [PATCH 3/8] Bump autoloader Signed-off-by: Lukas Reschke --- lib/composer/composer/autoload_classmap.php | 4 +++- lib/composer/composer/autoload_static.php | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index ea1473f27dc34..50ea9bb375157 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -969,6 +969,7 @@ 'OC\\Core\\Migrations\\Version21000Date20210309185126' => $baseDir . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version21000Date20210309185127' => $baseDir . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => $baseDir . '/core/Migrations/Version22000Date20210216080825.php', + 'OC\\Core\\Migrations\\Version23000Date20210906132259' => $baseDir . '/core/Migrations/Version23000Date20210906132259.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php', @@ -1365,8 +1366,9 @@ 'OC\\Security\\IdentityProof\\Manager' => $baseDir . '/lib/private/Security/IdentityProof/Manager.php', 'OC\\Security\\IdentityProof\\Signer' => $baseDir . '/lib/private/Security/IdentityProof/Signer.php', 'OC\\Security\\Normalizer\\IpAddress' => $baseDir . '/lib/private/Security/Normalizer/IpAddress.php', + 'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php', 'OC\\Security\\RateLimiting\\Backend\\IBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/IBackend.php', - 'OC\\Security\\RateLimiting\\Backend\\MemoryCache' => $baseDir . '/lib/private/Security/RateLimiting/Backend/MemoryCache.php', + 'OC\\Security\\RateLimiting\\Backend\\MemoryCacheBackend' => $baseDir . '/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php', 'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => $baseDir . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php', 'OC\\Security\\RateLimiting\\Limiter' => $baseDir . '/lib/private/Security/RateLimiting/Limiter.php', 'OC\\Security\\SecureRandom' => $baseDir . '/lib/private/Security/SecureRandom.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 93208f6ff1502..2bb6ff2293674 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -998,6 +998,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Migrations\\Version21000Date20210309185126' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185126.php', 'OC\\Core\\Migrations\\Version21000Date20210309185127' => __DIR__ . '/../../..' . '/core/Migrations/Version21000Date20210309185127.php', 'OC\\Core\\Migrations\\Version22000Date20210216080825' => __DIR__ . '/../../..' . '/core/Migrations/Version22000Date20210216080825.php', + 'OC\\Core\\Migrations\\Version23000Date20210906132259' => __DIR__ . '/../../..' . '/core/Migrations/Version23000Date20210906132259.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php', 'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php', @@ -1394,8 +1395,9 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Security\\IdentityProof\\Manager' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Manager.php', 'OC\\Security\\IdentityProof\\Signer' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Signer.php', 'OC\\Security\\Normalizer\\IpAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Normalizer/IpAddress.php', + 'OC\\Security\\RateLimiting\\Backend\\DatabaseBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php', 'OC\\Security\\RateLimiting\\Backend\\IBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/IBackend.php', - 'OC\\Security\\RateLimiting\\Backend\\MemoryCache' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/MemoryCache.php', + 'OC\\Security\\RateLimiting\\Backend\\MemoryCacheBackend' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php', 'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php', 'OC\\Security\\RateLimiting\\Limiter' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Limiter.php', 'OC\\Security\\SecureRandom' => __DIR__ . '/../../..' . '/lib/private/Security/SecureRandom.php', From 6337bb3f59e7e47e20d634bd9b92e6e4b14cfe1c Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 6 Sep 2021 17:46:02 +0200 Subject: [PATCH 4/8] Adjust tests Signed-off-by: Lukas Reschke --- .../Backend/MemoryCacheBackendTest.php | 12 ++++++------ tests/lib/Security/RateLimiting/LimiterTest.php | 16 ++++++---------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php b/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php index 8bbf88e8a8e08..e7cb4a93a299e 100644 --- a/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php +++ b/tests/lib/Security/RateLimiting/Backend/MemoryCacheBackendTest.php @@ -82,9 +82,9 @@ public function testGetAttempts() { '1', '2', '87', - '123', - '123', - '124', + '223', + '223', + '224', ])); $this->assertSame(3, $this->memoryCache->getAttempts('Method', 'User')); @@ -106,7 +106,7 @@ public function testRegisterAttemptWithNoAttemptsBefore() { ->method('set') ->with( 'eea460b8d756885099c7f0a4c083bf6a745069ee4a301984e726df58fd4510bffa2dac4b7fd5d835726a6753ffa8343ba31c7e902bbef78fc68c2e743667cb4b', - json_encode(['123']) + json_encode(['223']) ); $this->memoryCache->registerAttempt('Method', 'User', 100); @@ -116,7 +116,7 @@ public function testRegisterAttempt() { $this->timeFactory ->expects($this->once()) ->method('getTime') - ->willReturn(129); + ->willReturn(86); $this->cache ->expects($this->once()) @@ -140,7 +140,7 @@ public function testRegisterAttempt() { '123', '123', '124', - '129', + '186', ]) ); diff --git a/tests/lib/Security/RateLimiting/LimiterTest.php b/tests/lib/Security/RateLimiting/LimiterTest.php index 31c877cbda392..a5b187fb54607 100644 --- a/tests/lib/Security/RateLimiting/LimiterTest.php +++ b/tests/lib/Security/RateLimiting/LimiterTest.php @@ -55,8 +55,7 @@ public function testRegisterAnonRequestExceeded() { ->method('getAttempts') ->with( 'MyIdentifier', - '4664f0d9c88dcb7552be47b37bb52ce35977b2e60e1ac13757cf625f31f87050a41f3da064887fa87d49fd042e4c8eb20de8f10464877d3959677ab011b73a47', - 100 + '4664f0d9c88dcb7552be47b37bb52ce35977b2e60e1ac13757cf625f31f87050a41f3da064887fa87d49fd042e4c8eb20de8f10464877d3959677ab011b73a47' ) ->willReturn(101); @@ -69,8 +68,7 @@ public function testRegisterAnonRequestSuccess() { ->method('getAttempts') ->with( 'MyIdentifier', - '4664f0d9c88dcb7552be47b37bb52ce35977b2e60e1ac13757cf625f31f87050a41f3da064887fa87d49fd042e4c8eb20de8f10464877d3959677ab011b73a47', - 100 + '4664f0d9c88dcb7552be47b37bb52ce35977b2e60e1ac13757cf625f31f87050a41f3da064887fa87d49fd042e4c8eb20de8f10464877d3959677ab011b73a47' ) ->willReturn(99); $this->backend @@ -79,7 +77,7 @@ public function testRegisterAnonRequestSuccess() { ->with( 'MyIdentifier', '4664f0d9c88dcb7552be47b37bb52ce35977b2e60e1ac13757cf625f31f87050a41f3da064887fa87d49fd042e4c8eb20de8f10464877d3959677ab011b73a47', - 2000 + 100 ); $this->limiter->registerAnonRequest('MyIdentifier', 100, 100, '127.0.0.1'); @@ -101,8 +99,7 @@ public function testRegisterUserRequestExceeded() { ->method('getAttempts') ->with( 'MyIdentifier', - 'ddb2ec50fa973fd49ecf3d816f677c8095143e944ad10485f30fb3dac85c13a346dace4dae2d0a15af91867320957bfd38a43d9eefbb74fe6919e15119b6d805', - 100 + 'ddb2ec50fa973fd49ecf3d816f677c8095143e944ad10485f30fb3dac85c13a346dace4dae2d0a15af91867320957bfd38a43d9eefbb74fe6919e15119b6d805' ) ->willReturn(101); @@ -122,8 +119,7 @@ public function testRegisterUserRequestSuccess() { ->method('getAttempts') ->with( 'MyIdentifier', - 'ddb2ec50fa973fd49ecf3d816f677c8095143e944ad10485f30fb3dac85c13a346dace4dae2d0a15af91867320957bfd38a43d9eefbb74fe6919e15119b6d805', - 100 + 'ddb2ec50fa973fd49ecf3d816f677c8095143e944ad10485f30fb3dac85c13a346dace4dae2d0a15af91867320957bfd38a43d9eefbb74fe6919e15119b6d805' ) ->willReturn(99); $this->backend @@ -132,7 +128,7 @@ public function testRegisterUserRequestSuccess() { ->with( 'MyIdentifier', 'ddb2ec50fa973fd49ecf3d816f677c8095143e944ad10485f30fb3dac85c13a346dace4dae2d0a15af91867320957bfd38a43d9eefbb74fe6919e15119b6d805', - 2000 + 100 ); $this->limiter->registerUserRequest('MyIdentifier', 100, 100, $user); From a915372c56724815e3e0a63c8c77e60e7170b325 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 6 Sep 2021 17:50:23 +0200 Subject: [PATCH 5/8] phpcs Signed-off-by: Lukas Reschke --- core/Migrations/Version23000Date20210906132259.php | 3 +-- lib/private/Security/RateLimiting/Limiter.php | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php index c4e0c42075c6d..37935201918b0 100644 --- a/core/Migrations/Version23000Date20210906132259.php +++ b/core/Migrations/Version23000Date20210906132259.php @@ -28,8 +28,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $hasTable = $schema->hasTable(self::TABLE_NAME); - if(!$hasTable) - { + if (!$hasTable) { $table = $schema->createTable(self::TABLE_NAME); $table->addColumn('hash', Types::STRING, [ 'notnull' => true, diff --git a/lib/private/Security/RateLimiting/Limiter.php b/lib/private/Security/RateLimiting/Limiter.php index 1c14fce2a55e8..91657452d9916 100644 --- a/lib/private/Security/RateLimiting/Limiter.php +++ b/lib/private/Security/RateLimiting/Limiter.php @@ -29,7 +29,6 @@ use OC\Security\Normalizer\IpAddress; use OC\Security\RateLimiting\Backend\IBackend; use OC\Security\RateLimiting\Exception\RateLimitExceededException; -use OCP\AppFramework\Utility\ITimeFactory; use OCP\IUser; class Limiter { From 471167019c7c9071fa815b2c92691f65370a43b9 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 7 Sep 2021 18:03:34 +0200 Subject: [PATCH 6/8] Implement PR review feedback Signed-off-by: Lukas Reschke --- core/Migrations/Version23000Date20210906132259.php | 3 --- .../Security/RateLimiting/Backend/DatabaseBackend.php | 5 ----- lib/private/Security/RateLimiting/Backend/IBackend.php | 2 +- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php index 37935201918b0..b4568ab069cb3 100644 --- a/core/Migrations/Version23000Date20210906132259.php +++ b/core/Migrations/Version23000Date20210906132259.php @@ -10,9 +10,6 @@ use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; -/** - * Auto-generated migration step: Please modify to your needs! - */ class Version23000Date20210906132259 extends SimpleMigrationStep { private const TABLE_NAME = 'ratelimit_entries'; diff --git a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php index 3b7382d747e32..2ba6dc73c470e 100644 --- a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php +++ b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php @@ -29,11 +29,6 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; -/** - * Class DatabaseBackend uses the database for storing rate limiting data. - * - * @package OC\Security\RateLimiting\Backend - */ class DatabaseBackend implements IBackend { private const TABLE_NAME = 'ratelimit_entries'; diff --git a/lib/private/Security/RateLimiting/Backend/IBackend.php b/lib/private/Security/RateLimiting/Backend/IBackend.php index 835a97993da6f..960bfd2d15901 100644 --- a/lib/private/Security/RateLimiting/Backend/IBackend.php +++ b/lib/private/Security/RateLimiting/Backend/IBackend.php @@ -35,7 +35,7 @@ */ interface IBackend { /** - * Gets the amount of attempts for the specified method + * Gets the number of attempts for the specified method * * @param string $methodIdentifier Identifier for the method * @param string $userIdentifier Identifier for the user From 358eaba7dd45e5c5dbce44011bd7eadd88fe8534 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 13 Sep 2021 10:43:01 +0200 Subject: [PATCH 7/8] Apply suggestions from code review Signed-off-by: Lukas Reschke Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com> --- core/Migrations/Version23000Date20210906132259.php | 5 +++-- .../RateLimiting/Backend/DatabaseBackend.php | 12 +++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php index b4568ab069cb3..e65846f40b907 100644 --- a/core/Migrations/Version23000Date20210906132259.php +++ b/core/Migrations/Version23000Date20210906132259.php @@ -31,13 +31,14 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt 'notnull' => true, 'length' => 128, ]); - $table->addColumn('delete_after', 'datetime', [ + $table->addColumn('delete_after', Types::DATETIME, [ 'notnull' => true, ]); $table->addIndex(['hash'], 'ratelimit_hash_idx'); $table->addIndex(['delete_after'], 'ratelimit_delete_after_idx'); + return $schema; } - return $schema; + return null; } } diff --git a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php index 2ba6dc73c470e..5f579bfef85ab 100644 --- a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php +++ b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php @@ -73,21 +73,19 @@ private function getExistingAttemptCount( $qb = $this->dbConnection->getQueryBuilder(); $qb->delete(self::TABLE_NAME) ->where( - $qb->expr()->lte('delete_after', $qb->createParameter('currentTime')) - ) - ->setParameter('currentTime', $currentTime, 'datetime') + $qb->expr()->lte('delete_after', $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATE)) + ); ->executeStatement(); $qb = $this->dbConnection->getQueryBuilder(); - $qb->selectAlias($qb->createFunction('COUNT(*)'), 'count') + $qb->select($qb->func()->count()) ->from(self::TABLE_NAME) ->where( $qb->expr()->eq('hash', $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR)) ) ->andWhere( - $qb->expr()->gte('delete_after', $qb->createParameter('currentTime')) - ) - ->setParameter('currentTime', $currentTime, 'datetime'); + $qb->expr()->gte('delete_after', $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATE)) + ); $cursor = $qb->executeQuery(); $row = $cursor->fetch(); From 474a5b55d39e621c12947b50ea6c8febfceed7d7 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 13 Sep 2021 11:01:35 +0200 Subject: [PATCH 8/8] Implement review feedback Signed-off-by: Lukas Reschke --- core/Migrations/Version23000Date20210906132259.php | 4 ++-- .../Security/RateLimiting/Backend/DatabaseBackend.php | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/Migrations/Version23000Date20210906132259.php b/core/Migrations/Version23000Date20210906132259.php index e65846f40b907..26d18edc8f10a 100644 --- a/core/Migrations/Version23000Date20210906132259.php +++ b/core/Migrations/Version23000Date20210906132259.php @@ -34,8 +34,8 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $table->addColumn('delete_after', Types::DATETIME, [ 'notnull' => true, ]); - $table->addIndex(['hash'], 'ratelimit_hash_idx'); - $table->addIndex(['delete_after'], 'ratelimit_delete_after_idx'); + $table->addIndex(['hash'], 'ratelimit_hash'); + $table->addIndex(['delete_after'], 'ratelimit_delete_after'); return $schema; } diff --git a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php index 5f579bfef85ab..1415b5c4131c3 100644 --- a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php +++ b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php @@ -74,7 +74,7 @@ private function getExistingAttemptCount( $qb->delete(self::TABLE_NAME) ->where( $qb->expr()->lte('delete_after', $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATE)) - ); + ) ->executeStatement(); $qb = $this->dbConnection->getQueryBuilder(); @@ -88,10 +88,10 @@ private function getExistingAttemptCount( ); $cursor = $qb->executeQuery(); - $row = $cursor->fetch(); + $row = $cursor->fetchOne(); $cursor->closeCursor(); - return (int)$row['count']; + return (int)$row; } /**