From befa2f6d51231c9f79f4c66457424870c120517a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 14 Aug 2023 18:46:59 +0200 Subject: [PATCH] feat(security): Add a bruteforce protection backend base on memcache Similar to the ratelimit backend Signed-off-by: Joas Schilling --- lib/composer/composer/autoload_classmap.php | 3 + lib/composer/composer/autoload_static.php | 3 + .../Bruteforce/Backend/DatabaseBackend.php | 116 +++++++++++ .../Security/Bruteforce/Backend/IBackend.php | 82 ++++++++ .../Bruteforce/Backend/MemoryCacheBackend.php | 161 ++++++++++++++ lib/private/Security/Bruteforce/Throttler.php | 197 +++++++----------- lib/private/Server.php | 12 ++ lib/public/Security/Bruteforce/IThrottler.php | 19 +- .../Backend/MemoryCacheBackendTest.php | 156 ++++++++++++++ .../lib/Security/Bruteforce/ThrottlerTest.php | 31 +-- 10 files changed, 630 insertions(+), 150 deletions(-) create mode 100644 lib/private/Security/Bruteforce/Backend/DatabaseBackend.php create mode 100644 lib/private/Security/Bruteforce/Backend/IBackend.php create mode 100644 lib/private/Security/Bruteforce/Backend/MemoryCacheBackend.php create mode 100644 tests/lib/Security/Bruteforce/Backend/MemoryCacheBackendTest.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index b8c864abe4d..443b013cedd 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1565,6 +1565,9 @@ return array( 'OC\\Search\\Result\\Image' => $baseDir . '/lib/private/Search/Result/Image.php', 'OC\\Search\\SearchComposer' => $baseDir . '/lib/private/Search/SearchComposer.php', 'OC\\Search\\SearchQuery' => $baseDir . '/lib/private/Search/SearchQuery.php', + 'OC\\Security\\Bruteforce\\Backend\\DatabaseBackend' => $baseDir . '/lib/private/Security/Bruteforce/Backend/DatabaseBackend.php', + 'OC\\Security\\Bruteforce\\Backend\\IBackend' => $baseDir . '/lib/private/Security/Bruteforce/Backend/IBackend.php', + 'OC\\Security\\Bruteforce\\Backend\\MemoryCacheBackend' => $baseDir . '/lib/private/Security/Bruteforce/Backend/MemoryCacheBackend.php', 'OC\\Security\\Bruteforce\\Capabilities' => $baseDir . '/lib/private/Security/Bruteforce/Capabilities.php', 'OC\\Security\\Bruteforce\\CleanupJob' => $baseDir . '/lib/private/Security/Bruteforce/CleanupJob.php', 'OC\\Security\\Bruteforce\\Throttler' => $baseDir . '/lib/private/Security/Bruteforce/Throttler.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index e099accf683..82d51e30edd 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1598,6 +1598,9 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Search\\Result\\Image' => __DIR__ . '/../../..' . '/lib/private/Search/Result/Image.php', 'OC\\Search\\SearchComposer' => __DIR__ . '/../../..' . '/lib/private/Search/SearchComposer.php', 'OC\\Search\\SearchQuery' => __DIR__ . '/../../..' . '/lib/private/Search/SearchQuery.php', + 'OC\\Security\\Bruteforce\\Backend\\DatabaseBackend' => __DIR__ . '/../../..' . '/lib/private/Security/Bruteforce/Backend/DatabaseBackend.php', + 'OC\\Security\\Bruteforce\\Backend\\IBackend' => __DIR__ . '/../../..' . '/lib/private/Security/Bruteforce/Backend/IBackend.php', + 'OC\\Security\\Bruteforce\\Backend\\MemoryCacheBackend' => __DIR__ . '/../../..' . '/lib/private/Security/Bruteforce/Backend/MemoryCacheBackend.php', 'OC\\Security\\Bruteforce\\Capabilities' => __DIR__ . '/../../..' . '/lib/private/Security/Bruteforce/Capabilities.php', 'OC\\Security\\Bruteforce\\CleanupJob' => __DIR__ . '/../../..' . '/lib/private/Security/Bruteforce/CleanupJob.php', 'OC\\Security\\Bruteforce\\Throttler' => __DIR__ . '/../../..' . '/lib/private/Security/Bruteforce/Throttler.php', diff --git a/lib/private/Security/Bruteforce/Backend/DatabaseBackend.php b/lib/private/Security/Bruteforce/Backend/DatabaseBackend.php new file mode 100644 index 00000000000..04f2a7b6397 --- /dev/null +++ b/lib/private/Security/Bruteforce/Backend/DatabaseBackend.php @@ -0,0 +1,116 @@ + + * + * @author Joas Schilling + * + * @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\Bruteforce\Backend; + +use OCP\IDBConnection; + +class DatabaseBackend implements IBackend { + private const TABLE_NAME = 'bruteforce_attempts'; + + public function __construct( + private IDBConnection $db, + ) { + } + + /** + * {@inheritDoc} + */ + public function getAttempts( + string $ipSubnet, + int $maxAgeTimestamp, + ?string $action = null, + ?array $metadata = null, + ): int { + $query = $this->db->getQueryBuilder(); + $query->select($query->func()->count('*', 'attempts')) + ->from(self::TABLE_NAME) + ->where($query->expr()->gt('occurred', $query->createNamedParameter($maxAgeTimestamp))) + ->andWhere($query->expr()->eq('subnet', $query->createNamedParameter($ipSubnet))); + + if ($action !== null) { + $query->andWhere($query->expr()->eq('action', $query->createNamedParameter($action))); + + if ($metadata !== null) { + $query->andWhere($query->expr()->eq('metadata', $query->createNamedParameter(json_encode($metadata)))); + } + } + + $result = $query->executeQuery(); + $row = $result->fetch(); + $result->closeCursor(); + + return (int) $row['attempts']; + } + + /** + * {@inheritDoc} + */ + public function resetAttempts( + string $ipSubnet, + ?string $action = null, + ?array $metadata = null, + ): void { + $query = $this->db->getQueryBuilder(); + $query->delete(self::TABLE_NAME) + ->where($query->expr()->eq('subnet', $query->createNamedParameter($ipSubnet))); + + if ($action !== null) { + $query->andWhere($query->expr()->eq('action', $query->createNamedParameter($action))); + + if ($metadata !== null) { + $query->andWhere($query->expr()->eq('metadata', $query->createNamedParameter(json_encode($metadata)))); + } + } + + $query->executeStatement(); + } + + /** + * {@inheritDoc} + */ + public function registerAttempt( + string $ip, + string $ipSubnet, + int $timestamp, + string $action, + array $metadata = [], + ): void { + $values = [ + 'ip' => $ip, + 'subnet' => $ipSubnet, + 'action' => $action, + 'metadata' => json_encode($metadata), + 'occurred' => $timestamp, + ]; + + $qb = $this->db->getQueryBuilder(); + $qb->insert(self::TABLE_NAME); + foreach ($values as $column => $value) { + $qb->setValue($column, $qb->createNamedParameter($value)); + } + $qb->executeStatement(); + } +} diff --git a/lib/private/Security/Bruteforce/Backend/IBackend.php b/lib/private/Security/Bruteforce/Backend/IBackend.php new file mode 100644 index 00000000000..4b40262e645 --- /dev/null +++ b/lib/private/Security/Bruteforce/Backend/IBackend.php @@ -0,0 +1,82 @@ + + * + * @author Joas Schilling + * + * @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\Bruteforce\Backend; + +/** + * Interface IBackend defines a storage backend for the bruteforce data. It + * should be noted that writing and reading brute force data is an expensive + * operation and one should thus make sure to only use sufficient fast backends. + */ +interface IBackend { + /** + * Gets the number of attempts for the specified subnet (and further filters) + * + * @param string $ipSubnet + * @param int $maxAgeTimestamp + * @param ?string $action Optional action to further limit attempts + * @param ?array $metadata Optional metadata stored to further limit attempts (Only considered when $action is set) + * @return int + * @since 28.0.0 + */ + public function getAttempts( + string $ipSubnet, + int $maxAgeTimestamp, + ?string $action = null, + ?array $metadata = null, + ): int; + + /** + * Reset the attempts for the specified subnet (and further filters) + * + * @param string $ipSubnet + * @param ?string $action Optional action to further limit attempts + * @param ?array $metadata Optional metadata stored to further limit attempts(Only considered when $action is set) + * @since 28.0.0 + */ + public function resetAttempts( + string $ipSubnet, + ?string $action = null, + ?array $metadata = null, + ): void; + + /** + * Register a failed attempt to bruteforce a security control + * + * @param string $ip + * @param string $ipSubnet + * @param int $timestamp + * @param string $action + * @param array $metadata Optional metadata stored to further limit attempts when getting + * @since 28.0.0 + */ + public function registerAttempt( + string $ip, + string $ipSubnet, + int $timestamp, + string $action, + array $metadata = [], + ): void; +} diff --git a/lib/private/Security/Bruteforce/Backend/MemoryCacheBackend.php b/lib/private/Security/Bruteforce/Backend/MemoryCacheBackend.php new file mode 100644 index 00000000000..432e99700fe --- /dev/null +++ b/lib/private/Security/Bruteforce/Backend/MemoryCacheBackend.php @@ -0,0 +1,161 @@ + + * + * @author Joas Schilling + * + * @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\Bruteforce\Backend; + +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\ICache; +use OCP\ICacheFactory; + +class MemoryCacheBackend implements IBackend { + private ICache $cache; + + public function __construct( + ICacheFactory $cacheFactory, + private ITimeFactory $timeFactory, + ) { + $this->cache = $cacheFactory->createDistributed(__CLASS__); + } + + private function hash( + null|string|array $data, + ): ?string { + if ($data === null) { + return null; + } + if (!is_string($data)) { + $data = json_encode($data); + } + return hash('sha1', $data); + } + + 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 $ipSubnet, + int $maxAgeTimestamp, + ?string $action = null, + ?array $metadata = null, + ): int { + $identifier = $this->hash($ipSubnet); + $actionHash = $this->hash($action); + $metadataHash = $this->hash($metadata); + $existingAttempts = $this->getExistingAttempts($identifier); + + $count = 0; + foreach ($existingAttempts as $info) { + [$occurredTime, $attemptAction, $attemptMetadata] = explode('#', $info, 3); + if ($action === null || $attemptAction === $actionHash) { + if ($metadata === null || $attemptMetadata === $metadataHash) { + if ($occurredTime > $maxAgeTimestamp) { + $count++; + } + } + } + } + + return $count; + } + + /** + * {@inheritDoc} + */ + public function resetAttempts( + string $ipSubnet, + ?string $action = null, + ?array $metadata = null, + ): void { + $identifier = $this->hash($ipSubnet); + if ($action === null) { + $this->cache->remove($identifier); + } else { + $actionHash = $this->hash($action); + $metadataHash = $this->hash($metadata); + $existingAttempts = $this->getExistingAttempts($identifier); + $maxAgeTimestamp = $this->timeFactory->getTime() - 12 * 3600; + + foreach ($existingAttempts as $key => $info) { + [$occurredTime, $attemptAction, $attemptMetadata] = explode('#', $info, 3); + if ($attemptAction === $actionHash) { + if ($metadata === null || $attemptMetadata === $metadataHash) { + unset($existingAttempts[$key]); + } elseif ($occurredTime < $maxAgeTimestamp) { + unset($existingAttempts[$key]); + } + } + } + + if (!empty($existingAttempts)) { + $this->cache->set($identifier, json_encode($existingAttempts), 12 * 3600); + } else { + $this->cache->remove($identifier); + } + } + } + + /** + * {@inheritDoc} + */ + public function registerAttempt( + string $ip, + string $ipSubnet, + int $timestamp, + string $action, + array $metadata = [], + ): void { + $identifier = $this->hash($ipSubnet); + $existingAttempts = $this->getExistingAttempts($identifier); + $maxAgeTimestamp = $this->timeFactory->getTime() - 12 * 3600; + + // Unset all attempts that are already expired + foreach ($existingAttempts as $key => $info) { + [$occurredTime,] = explode('#', $info, 3); + if ($occurredTime < $maxAgeTimestamp) { + unset($existingAttempts[$key]); + } + } + $existingAttempts = array_values($existingAttempts); + + // Store the new attempt + $existingAttempts[] = $timestamp . '#' . $this->hash($action) . '#' . $this->hash($metadata); + + $this->cache->set($identifier, json_encode($existingAttempts), 12 * 3600); + } +} diff --git a/lib/private/Security/Bruteforce/Throttler.php b/lib/private/Security/Bruteforce/Throttler.php index d5fd0984baa..a0a41a8b4c4 100644 --- a/lib/private/Security/Bruteforce/Throttler.php +++ b/lib/private/Security/Bruteforce/Throttler.php @@ -3,6 +3,7 @@ declare(strict_types=1); /** + * @copyright Copyright (c) 2023 Joas Schilling * @copyright Copyright (c) 2016 Lukas Reschke * * @author Bjoern Schiessle @@ -32,10 +33,10 @@ declare(strict_types=1); */ namespace OC\Security\Bruteforce; +use OC\Security\Bruteforce\Backend\IBackend; use OC\Security\Normalizer\IpAddress; use OCP\AppFramework\Utility\ITimeFactory; use OCP\IConfig; -use OCP\IDBConnection; use OCP\Security\Bruteforce\IThrottler; use OCP\Security\Bruteforce\MaxDelayReached; use Psr\Log\LoggerInterface; @@ -54,59 +55,21 @@ use Psr\Log\LoggerInterface; * @package OC\Security\Bruteforce */ class Throttler implements IThrottler { - public const LOGIN_ACTION = 'login'; - - /** @var IDBConnection */ - private $db; - /** @var ITimeFactory */ - private $timeFactory; - private LoggerInterface $logger; - /** @var IConfig */ - private $config; /** @var bool[] */ - private $hasAttemptsDeleted = []; - - public function __construct(IDBConnection $db, - ITimeFactory $timeFactory, - LoggerInterface $logger, - IConfig $config) { - $this->db = $db; - $this->timeFactory = $timeFactory; - $this->logger = $logger; - $this->config = $config; - } - - /** - * Convert a number of seconds into the appropriate DateInterval - * - * @param int $expire - * @return \DateInterval - */ - private function getCutoff(int $expire): \DateInterval { - $d1 = new \DateTime(); - $d2 = clone $d1; - $d2->sub(new \DateInterval('PT' . $expire . 'S')); - return $d2->diff($d1); - } - - /** - * Calculate the cut off timestamp - * - * @param float $maxAgeHours - * @return int - */ - private function getCutoffTimestamp(float $maxAgeHours = 12.0): int { - return (new \DateTime()) - ->sub($this->getCutoff((int) ($maxAgeHours * 3600))) - ->getTimestamp(); + private array $hasAttemptsDeleted = []; + /** @var bool[] */ + private array $ipIsWhitelisted = []; + + public function __construct( + private ITimeFactory $timeFactory, + private LoggerInterface $logger, + private IConfig $config, + private IBackend $backend, + ) { } /** - * Register a failed attempt to bruteforce a security control - * - * @param string $action - * @param string $ip - * @param array $metadata Optional metadata logged to the database + * {@inheritDoc} */ public function registerAttempt(string $action, string $ip, @@ -117,13 +80,9 @@ class Throttler implements IThrottler { } $ipAddress = new IpAddress($ip); - $values = [ - 'action' => $action, - 'occurred' => $this->timeFactory->getTime(), - 'ip' => (string)$ipAddress, - 'subnet' => $ipAddress->getSubnet(), - 'metadata' => json_encode($metadata), - ]; + if ($this->isIPWhitelisted((string)$ipAddress)) { + return; + } $this->logger->notice( sprintf( @@ -136,12 +95,13 @@ class Throttler implements IThrottler { ] ); - $qb = $this->db->getQueryBuilder(); - $qb->insert('bruteforce_attempts'); - foreach ($values as $column => $value) { - $qb->setValue($column, $qb->createNamedParameter($value)); - } - $qb->execute(); + $this->backend->registerAttempt( + (string)$ipAddress, + $ipAddress->getSubnet(), + $this->timeFactory->getTime(), + $action, + $metadata + ); } /** @@ -151,7 +111,12 @@ class Throttler implements IThrottler { * @return bool */ private function isIPWhitelisted(string $ip): bool { + if (isset($this->ipIsWhitelisted[$ip])) { + return $this->ipIsWhitelisted[$ip]; + } + if (!$this->config->getSystemValueBool('auth.bruteforce.protection.enabled', true)) { + $this->ipIsWhitelisted[$ip] = true; return true; } @@ -165,6 +130,7 @@ class Throttler implements IThrottler { } elseif (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { $type = 6; } else { + $this->ipIsWhitelisted[$ip] = false; return false; } @@ -202,20 +168,26 @@ class Throttler implements IThrottler { } if ($valid === true) { + $this->ipIsWhitelisted[$ip] = true; return true; } } + $this->ipIsWhitelisted[$ip] = false; return false; } /** - * Get the throttling delay (in milliseconds) - * - * @param string $ip - * @param string $action optionally filter by action - * @param float $maxAgeHours - * @return int + * {@inheritDoc} + */ + public function showBruteforceWarning(string $ip, string $action = ''): bool { + $attempts = $this->getAttempts($ip, $action); + // 4 failed attempts is the last delay below 5 seconds + return $attempts >= 4; + } + + /** + * {@inheritDoc} */ public function getAttempts(string $ip, string $action = '', float $maxAgeHours = 12): int { if ($maxAgeHours > 48) { @@ -232,31 +204,17 @@ class Throttler implements IThrottler { return 0; } - $cutoffTime = $this->getCutoffTimestamp($maxAgeHours); - - $qb = $this->db->getQueryBuilder(); - $qb->select($qb->func()->count('*', 'attempts')) - ->from('bruteforce_attempts') - ->where($qb->expr()->gt('occurred', $qb->createNamedParameter($cutoffTime))) - ->andWhere($qb->expr()->eq('subnet', $qb->createNamedParameter($ipAddress->getSubnet()))); - - if ($action !== '') { - $qb->andWhere($qb->expr()->eq('action', $qb->createNamedParameter($action))); - } - - $result = $qb->execute(); - $row = $result->fetch(); - $result->closeCursor(); + $maxAgeTimestamp = (int) ($this->timeFactory->getTime() - 3600 * $maxAgeHours); - return (int) $row['attempts']; + return $this->backend->getAttempts( + $ipAddress->getSubnet(), + $maxAgeTimestamp, + $action !== '' ? $action : null, + ); } /** - * Get the throttling delay (in milliseconds) - * - * @param string $ip - * @param string $action optionally filter by action - * @return int + * {@inheritDoc} */ public function getDelay(string $ip, string $action = ''): int { $attempts = $this->getAttempts($ip, $action); @@ -278,54 +236,47 @@ class Throttler implements IThrottler { } /** - * Reset the throttling delay for an IP address, action and metadata - * - * @param string $ip - * @param string $action - * @param array $metadata + * {@inheritDoc} */ public function resetDelay(string $ip, string $action, array $metadata): void { + // No need to log if the bruteforce protection is disabled + if (!$this->config->getSystemValueBool('auth.bruteforce.protection.enabled', true)) { + return; + } + $ipAddress = new IpAddress($ip); if ($this->isIPWhitelisted((string)$ipAddress)) { return; } - $cutoffTime = $this->getCutoffTimestamp(); - - $qb = $this->db->getQueryBuilder(); - $qb->delete('bruteforce_attempts') - ->where($qb->expr()->gt('occurred', $qb->createNamedParameter($cutoffTime))) - ->andWhere($qb->expr()->eq('subnet', $qb->createNamedParameter($ipAddress->getSubnet()))) - ->andWhere($qb->expr()->eq('action', $qb->createNamedParameter($action))) - ->andWhere($qb->expr()->eq('metadata', $qb->createNamedParameter(json_encode($metadata)))); - - $qb->executeStatement(); + $this->backend->resetAttempts( + $ipAddress->getSubnet(), + $action, + $metadata, + ); $this->hasAttemptsDeleted[$action] = true; } /** - * Reset the throttling delay for an IP address - * - * @param string $ip + * {@inheritDoc} */ public function resetDelayForIP(string $ip): void { - $cutoffTime = $this->getCutoffTimestamp(); + // No need to log if the bruteforce protection is disabled + if (!$this->config->getSystemValueBool('auth.bruteforce.protection.enabled', true)) { + return; + } - $qb = $this->db->getQueryBuilder(); - $qb->delete('bruteforce_attempts') - ->where($qb->expr()->gt('occurred', $qb->createNamedParameter($cutoffTime))) - ->andWhere($qb->expr()->eq('ip', $qb->createNamedParameter($ip))); + $ipAddress = new IpAddress($ip); + if ($this->isIPWhitelisted((string)$ipAddress)) { + return; + } - $qb->execute(); + $this->backend->resetAttempts($ipAddress->getSubnet()); } /** - * Will sleep for the defined amount of time - * - * @param string $ip - * @param string $action optionally filter by action - * @return int the time spent sleeping + * {@inheritDoc} */ public function sleepDelay(string $ip, string $action = ''): int { $delay = $this->getDelay($ip, $action); @@ -334,13 +285,7 @@ class Throttler implements IThrottler { } /** - * Will sleep for the defined amount of time unless maximum was reached in the last 30 minutes - * In this case a "429 Too Many Request" exception is thrown - * - * @param string $ip - * @param string $action optionally filter by action - * @return int the time spent sleeping - * @throws MaxDelayReached when reached the maximum + * {@inheritDoc} */ public function sleepDelayOrThrowOnMax(string $ip, string $action = ''): int { $delay = $this->getDelay($ip, $action); diff --git a/lib/private/Server.php b/lib/private/Server.php index 516dc0b6e8b..bb073891d03 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1011,6 +1011,18 @@ class Server extends ServerContainer implements IServerContainer { /** @deprecated 19.0.0 */ $this->registerDeprecatedAlias('Throttler', Throttler::class); $this->registerAlias(IThrottler::class, Throttler::class); + + $this->registerService(\OC\Security\Bruteforce\Backend\IBackend::class, function ($c) { + $config = $c->get(\OCP\IConfig::class); + if (ltrim($config->getSystemValueString('memcache.distributed', ''), '\\') === \OC\Memcache\Redis::class) { + $backend = $c->get(\OC\Security\Bruteforce\Backend\MemoryCacheBackend::class); + } else { + $backend = $c->get(\OC\Security\Bruteforce\Backend\DatabaseBackend::class); + } + + return $backend; + }); + $this->registerService('IntegrityCodeChecker', function (ContainerInterface $c) { // IConfig and IAppManager requires a working database. This code // might however be called when ownCloud is not yet setup. diff --git a/lib/public/Security/Bruteforce/IThrottler.php b/lib/public/Security/Bruteforce/IThrottler.php index 6f492d6c59d..03c8c56a23c 100644 --- a/lib/public/Security/Bruteforce/IThrottler.php +++ b/lib/public/Security/Bruteforce/IThrottler.php @@ -40,16 +40,19 @@ namespace OCP\Security\Bruteforce; interface IThrottler { /** * @since 25.0.0 + * @deprecated 28.0.0 */ public const MAX_DELAY = 25; /** * @since 25.0.0 + * @deprecated 28.0.0 */ public const MAX_DELAY_MS = 25000; // in milliseconds /** * @since 25.0.0 + * @deprecated 28.0.0 */ public const MAX_ATTEMPTS = 10; @@ -58,7 +61,7 @@ interface IThrottler { * * @param string $action * @param string $ip - * @param array $metadata Optional metadata logged to the database + * @param array $metadata Optional metadata logged with the attempt * @since 25.0.0 */ public function registerAttempt(string $action, string $ip, array $metadata = []): void; @@ -71,9 +74,20 @@ interface IThrottler { * @param float $maxAgeHours * @return int * @since 25.0.0 + * @deprecated 28.0.0 This method is considered internal as of Nextcloud 28. Use {@see showBruteforceWarning()} to decide whether a warning should be shown. */ public function getAttempts(string $ip, string $action = '', float $maxAgeHours = 12): int; + /** + * Whether a warning should be shown about the throttle + * + * @param string $ip + * @param string $action optionally filter by action + * @return bool + * @since 28.0.0 + */ + public function showBruteforceWarning(string $ip, string $action = ''): bool; + /** * Get the throttling delay (in milliseconds) * @@ -81,6 +95,7 @@ interface IThrottler { * @param string $action optionally filter by action * @return int * @since 25.0.0 + * @deprecated 28.0.0 This method is considered internal as of Nextcloud 28. Use {@see showBruteforceWarning()} to decide whether a warning should be shown. */ public function getDelay(string $ip, string $action = ''): int; @@ -99,6 +114,7 @@ interface IThrottler { * * @param string $ip * @since 25.0.0 + * @deprecated 28.0.0 This method is considered internal as of Nextcloud 28. Use {@see resetDelay()} and only reset the entries of your action and metadata */ public function resetDelayForIP(string $ip): void; @@ -109,6 +125,7 @@ interface IThrottler { * @param string $action optionally filter by action * @return int the time spent sleeping * @since 25.0.0 + * @deprecated 28.0.0 Use {@see sleepDelayOrThrowOnMax()} instead and abort handling the request when it throws */ public function sleepDelay(string $ip, string $action = ''): int; diff --git a/tests/lib/Security/Bruteforce/Backend/MemoryCacheBackendTest.php b/tests/lib/Security/Bruteforce/Backend/MemoryCacheBackendTest.php new file mode 100644 index 00000000000..648d8627421 --- /dev/null +++ b/tests/lib/Security/Bruteforce/Backend/MemoryCacheBackendTest.php @@ -0,0 +1,156 @@ + + * + * @author Joas Schilling + * + * @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 Test\Security\Bruteforce\Backend; + +use OC\Security\Bruteforce\Backend\IBackend; +use OC\Security\Bruteforce\Backend\MemoryCacheBackend; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\ICache; +use OCP\ICacheFactory; +use PHPUnit\Framework\MockObject\MockObject; +use Test\TestCase; + +class MemoryCacheBackendTest extends TestCase { + /** @var ICacheFactory|MockObject */ + private $cacheFactory; + /** @var ITimeFactory|MockObject */ + private $timeFactory; + /** @var ICache|MockObject */ + private $cache; + private IBackend $backend; + + protected function setUp(): void { + parent::setUp(); + + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); + $this->cache = $this->createMock(ICache::class); + + $this->cacheFactory + ->expects($this->once()) + ->method('createDistributed') + ->with('OC\Security\Bruteforce\Backend\MemoryCacheBackend') + ->willReturn($this->cache); + + $this->backend = new MemoryCacheBackend( + $this->cacheFactory, + $this->timeFactory + ); + } + + public function testGetAttemptsWithNoAttemptsBefore(): void { + $this->cache + ->expects($this->once()) + ->method('get') + ->with('8b9da631d1f7b022bb2c3c489e16092f82b42fd4') + ->willReturn(null); + + $this->assertSame(0, $this->backend->getAttempts('10.10.10.10/32', 0)); + } + + public function dataGetAttempts(): array { + return [ + [0, null, null, 4], + [100, null, null, 2], + [0, 'action1', null, 2], + [100, 'action1', null, 1], + [0, 'action1', ['metadata2'], 1], + [100, 'action1', ['metadata2'], 1], + [100, 'action1', ['metadata1'], 0], + ]; + } + + /** + * @dataProvider dataGetAttempts + */ + public function testGetAttempts(int $maxAge, ?string $action, ?array $metadata, int $expected): void { + $this->cache + ->expects($this->once()) + ->method('get') + ->with('8b9da631d1f7b022bb2c3c489e16092f82b42fd4') + ->willReturn(json_encode([ + '1' . '#' . hash('sha1', 'action1') . '#' . hash('sha1', json_encode(['metadata1'])), + '300' . '#' . hash('sha1', 'action1') . '#' . hash('sha1', json_encode(['metadata2'])), + '1' . '#' . hash('sha1', 'action2') . '#' . hash('sha1', json_encode(['metadata1'])), + '300' . '#' . hash('sha1', 'action2') . '#' . hash('sha1', json_encode(['metadata2'])), + ])); + + $this->assertSame($expected, $this->backend->getAttempts('10.10.10.10/32', $maxAge, $action, $metadata)); + } + + public function testRegisterAttemptWithNoAttemptsBefore(): void { + $this->cache + ->expects($this->once()) + ->method('get') + ->with('8b9da631d1f7b022bb2c3c489e16092f82b42fd4') + ->willReturn(null); + $this->cache + ->expects($this->once()) + ->method('set') + ->with( + '8b9da631d1f7b022bb2c3c489e16092f82b42fd4', + json_encode(['223#' . hash('sha1', 'action1') . '#' . hash('sha1', json_encode(['metadata1']))]) + ); + + $this->backend->registerAttempt('10.10.10.10', '10.10.10.10/32', 223, 'action1', ['metadata1']); + } + + public function testRegisterAttempt(): void { + $this->timeFactory + ->expects($this->once()) + ->method('getTime') + ->willReturn(12 * 3600 + 86); + + $this->cache + ->expects($this->once()) + ->method('get') + ->with('8b9da631d1f7b022bb2c3c489e16092f82b42fd4') + ->willReturn(json_encode([ + '1#' . hash('sha1', 'action1') . '#' . hash('sha1', json_encode(['metadata1'])), + '2#' . hash('sha1', 'action2') . '#' . hash('sha1', json_encode(['metadata1'])), + '87#' . hash('sha1', 'action3') . '#' . hash('sha1', json_encode(['metadata1'])), + '123#' . hash('sha1', 'action4') . '#' . hash('sha1', json_encode(['metadata1'])), + '123#' . hash('sha1', 'action5') . '#' . hash('sha1', json_encode(['metadata1'])), + '124#' . hash('sha1', 'action6') . '#' . hash('sha1', json_encode(['metadata1'])), + ])); + $this->cache + ->expects($this->once()) + ->method('set') + ->with( + '8b9da631d1f7b022bb2c3c489e16092f82b42fd4', + json_encode([ + '87#' . hash('sha1', 'action3') . '#' . hash('sha1', json_encode(['metadata1'])), + '123#' . hash('sha1', 'action4') . '#' . hash('sha1', json_encode(['metadata1'])), + '123#' . hash('sha1', 'action5') . '#' . hash('sha1', json_encode(['metadata1'])), + '124#' . hash('sha1', 'action6') . '#' . hash('sha1', json_encode(['metadata1'])), + '186#' . hash('sha1', 'action7') . '#' . hash('sha1', json_encode(['metadata2'])), + ]) + ); + + $this->backend->registerAttempt('10.10.10.10', '10.10.10.10/32', 186, 'action7', ['metadata2']); + } +} diff --git a/tests/lib/Security/Bruteforce/ThrottlerTest.php b/tests/lib/Security/Bruteforce/ThrottlerTest.php index f23b15a06d7..e7fd12645e1 100644 --- a/tests/lib/Security/Bruteforce/ThrottlerTest.php +++ b/tests/lib/Security/Bruteforce/ThrottlerTest.php @@ -24,8 +24,9 @@ declare(strict_types=1); namespace Test\Security\Bruteforce; -use OC\AppFramework\Utility\TimeFactory; +use OC\Security\Bruteforce\Backend\DatabaseBackend; use OC\Security\Bruteforce\Throttler; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IConfig; use OCP\IDBConnection; use Psr\Log\LoggerInterface; @@ -42,6 +43,8 @@ class ThrottlerTest extends TestCase { private $throttler; /** @var IDBConnection */ private $dbConnection; + /** @var ITimeFactory */ + private $timeFactory; /** @var LoggerInterface */ private $logger; /** @var IConfig|\PHPUnit\Framework\MockObject\MockObject */ @@ -49,37 +52,19 @@ class ThrottlerTest extends TestCase { protected function setUp(): void { $this->dbConnection = $this->createMock(IDBConnection::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); $this->logger = $this->createMock(LoggerInterface::class); $this->config = $this->createMock(IConfig::class); $this->throttler = new Throttler( - $this->dbConnection, - new TimeFactory(), + $this->timeFactory, $this->logger, - $this->config + $this->config, + new DatabaseBackend($this->dbConnection) ); parent::setUp(); } - public function testCutoff() { - // precisely 31 second shy of 12 hours - $cutoff = self::invokePrivate($this->throttler, 'getCutoff', [43169]); - $this->assertSame(0, $cutoff->y); - $this->assertSame(0, $cutoff->m); - $this->assertSame(0, $cutoff->d); - $this->assertSame(11, $cutoff->h); - $this->assertSame(59, $cutoff->i); - $this->assertSame(29, $cutoff->s); - $cutoff = self::invokePrivate($this->throttler, 'getCutoff', [86401]); - $this->assertSame(0, $cutoff->y); - $this->assertSame(0, $cutoff->m); - $this->assertSame(1, $cutoff->d); - $this->assertSame(0, $cutoff->h); - $this->assertSame(0, $cutoff->i); - // Leap second tolerance: - $this->assertLessThan(2, $cutoff->s); - } - public function dataIsIPWhitelisted() { return [ [ -- 2.39.5