From 378cc922c429524b872e83c7b3842eb86bc4b770 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 6 Sep 2021 17:31:36 +0200 Subject: Adjust logic to store period instead of current timestamp Signed-off-by: Lukas Reschke --- .../RateLimiting/Backend/DatabaseBackend.php | 37 +++--- .../Security/RateLimiting/Backend/IBackend.php | 6 +- .../Security/RateLimiting/Backend/MemoryCache.php | 128 --------------------- .../RateLimiting/Backend/MemoryCacheBackend.php | 126 ++++++++++++++++++++ lib/private/Security/RateLimiting/Limiter.php | 11 +- 5 files changed, 147 insertions(+), 161 deletions(-) delete mode 100644 lib/private/Security/RateLimiting/Backend/MemoryCache.php create mode 100644 lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php (limited to 'lib/private/Security') diff --git a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php index a7f2a7b4b62..3b7382d747e 100644 --- a/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php +++ b/lib/private/Security/RateLimiting/Backend/DatabaseBackend.php @@ -71,21 +71,28 @@ class DatabaseBackend implements IBackend { * @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 @@ class DatabaseBackend implements IBackend { * {@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 @@ class DatabaseBackend implements IBackend { 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 d87f53311b2..835a97993da 100644 --- a/lib/private/Security/RateLimiting/Backend/IBackend.php +++ b/lib/private/Security/RateLimiting/Backend/IBackend.php @@ -35,16 +35,14 @@ namespace OC\Security\RateLimiting\Backend; */ 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/MemoryCache.php deleted file mode 100644 index 0dab25e4048..00000000000 --- a/lib/private/Security/RateLimiting/Backend/MemoryCache.php +++ /dev/null @@ -1,128 +0,0 @@ - - * - * @author Christoph Wurst - * @author Lukas Reschke - * @author Morris Jobke - * @author Roeland Jago Douma - * - * @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\ICache; -use OCP\ICacheFactory; - -/** - * Class MemoryCache uses the configured distributed memory cache for storing - * rate limiting data. - * - * @package OC\Security\RateLimiting\Backend - */ -class MemoryCache implements IBackend { - /** @var ICache */ - private $cache; - /** @var ITimeFactory */ - private $timeFactory; - - /** - * @param ICacheFactory $cacheFactory - * @param ITimeFactory $timeFactory - */ - public function __construct(ICacheFactory $cacheFactory, - ITimeFactory $timeFactory) { - $this->cache = $cacheFactory->createDistributed(__CLASS__); - $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 - * @return array - */ - private function getExistingAttempts(string $identifier): array { - $cachedAttempts = $this->cache->get($identifier); - if ($cachedAttempts === null) { - return []; - } - - $cachedAttempts = json_decode($cachedAttempts, true); - if (\is_array($cachedAttempts)) { - return $cachedAttempts; - } - - return []; - } - - /** - * {@inheritDoc} - */ - public function getAttempts(string $methodIdentifier, - string $userIdentifier, - int $seconds): 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) { - $count++; - } - } - - return $count; - } - - /** - * {@inheritDoc} - */ - public function registerAttempt(string $methodIdentifier, - string $userIdentifier, - int $period) { - $identifier = $this->hash($methodIdentifier, $userIdentifier); - $existingAttempts = $this->getExistingAttempts($identifier); - $currentTime = $this->timeFactory->getTime(); - - // Unset all attempts older than $period - foreach ($existingAttempts as $key => $attempt) { - if (($attempt + $period) < $currentTime) { - unset($existingAttempts[$key]); - } - } - $existingAttempts = array_values($existingAttempts); - - // Store the new attempt - $existingAttempts[] = (string)$currentTime; - $this->cache->set($identifier, json_encode($existingAttempts)); - } -} diff --git a/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php b/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php new file mode 100644 index 00000000000..f4880fb239c --- /dev/null +++ b/lib/private/Security/RateLimiting/Backend/MemoryCacheBackend.php @@ -0,0 +1,126 @@ + + * + * @author Christoph Wurst + * @author Lukas Reschke + * @author Morris Jobke + * @author Roeland Jago Douma + * + * @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\ICache; +use OCP\ICacheFactory; + +/** + * Class MemoryCacheBackend uses the configured distributed memory cache for storing + * rate limiting data. + * + * @package OC\Security\RateLimiting\Backend + */ +class MemoryCacheBackend implements IBackend { + /** @var ICache */ + private $cache; + /** @var ITimeFactory */ + private $timeFactory; + + /** + * @param ICacheFactory $cacheFactory + * @param ITimeFactory $timeFactory + */ + public function __construct(ICacheFactory $cacheFactory, + ITimeFactory $timeFactory) { + $this->cache = $cacheFactory->createDistributed(__CLASS__); + $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 + * @return array + */ + private function getExistingAttempts(string $identifier): array { + $cachedAttempts = $this->cache->get($identifier); + if ($cachedAttempts === null) { + return []; + } + + $cachedAttempts = json_decode($cachedAttempts, true); + if (\is_array($cachedAttempts)) { + return $cachedAttempts; + } + + return []; + } + + /** + * {@inheritDoc} + */ + public function getAttempts(string $methodIdentifier, + string $userIdentifier): int { + $identifier = $this->hash($methodIdentifier, $userIdentifier); + $existingAttempts = $this->getExistingAttempts($identifier); + + $count = 0; + $currentTime = $this->timeFactory->getTime(); + foreach ($existingAttempts as $expirationTime) { + if ($expirationTime > $currentTime) { + $count++; + } + } + + return $count; + } + + /** + * {@inheritDoc} + */ + public function registerAttempt(string $methodIdentifier, + string $userIdentifier, + int $period) { + $identifier = $this->hash($methodIdentifier, $userIdentifier); + $existingAttempts = $this->getExistingAttempts($identifier); + $currentTime = $this->timeFactory->getTime(); + + // 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 + $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 ede72e887fc..1c14fce2a55 100644 --- a/lib/private/Security/RateLimiting/Limiter.php +++ b/lib/private/Security/RateLimiting/Limiter.php @@ -35,17 +35,12 @@ use OCP\IUser; 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 @@ class Limiter { 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); } /** -- cgit v1.2.3