diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index a55596358e5dc..29ef0343399c1 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -86,6 +86,7 @@ use OCP\IURLGenerator; use OCP\Lock\ILockingProvider; use OCP\Notification\IManager; +use OCP\Security\Bruteforce\IThrottler; use OCP\Security\ISecureRandom; use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -122,6 +123,8 @@ class CheckSetupController extends Controller { private $iniGetWrapper; /** @var IDBConnection */ private $connection; + /** @var IThrottler */ + private $throttler; /** @var ITempManager */ private $tempManager; /** @var IManager */ @@ -148,6 +151,7 @@ public function __construct($AppName, ISecureRandom $secureRandom, IniGetWrapper $iniGetWrapper, IDBConnection $connection, + IThrottler $throttler, ITempManager $tempManager, IManager $manager, IAppManager $appManager, @@ -163,6 +167,7 @@ public function __construct($AppName, $this->eventDispatcher = $eventDispatcher; $this->dispatcher = $dispatcher; $this->db = $db; + $this->throttler = $throttler; $this->lockingProvider = $lockingProvider; $this->dateTimeFormatter = $dateTimeFormatter; $this->memoryInfo = $memoryInfo; @@ -909,6 +914,8 @@ public function check() { 'cronInfo' => $this->getLastCronInfo(), 'cronErrors' => $this->getCronErrors(), 'isFairUseOfFreePushService' => $this->isFairUseOfFreePushService(), + 'isBruteforceThrottled' => $this->throttler->getAttempts($this->request->getRemoteAddress()) !== 0, + 'bruteforceRemoteAddress' => $this->request->getRemoteAddress(), 'serverHasInternetConnectionProblems' => $this->hasInternetConnectivityProblems(), 'isMemcacheConfigured' => $this->isMemcacheConfigured(), 'memcacheDocs' => $this->urlGenerator->linkToDocs('admin-performance'), diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 390166cb947ed..2d2d43434b17e 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -59,6 +59,7 @@ use OCP\IURLGenerator; use OCP\Lock\ILockingProvider; use OCP\Notification\IManager; +use OCP\Security\Bruteforce\IThrottler; use PHPUnit\Framework\MockObject\MockObject; use Psr\Http\Message\ResponseInterface; use Psr\Log\LoggerInterface; @@ -148,6 +149,7 @@ protected function setUp(): void { $this->logger = $this->getMockBuilder(LoggerInterface::class)->getMock(); $this->db = $this->getMockBuilder(Connection::class) ->disableOriginalConstructor()->getMock(); + $this->throttler = $this->createMock(IThrottler::class); $this->lockingProvider = $this->getMockBuilder(ILockingProvider::class)->getMock(); $this->dateTimeFormatter = $this->getMockBuilder(IDateTimeFormatter::class)->getMock(); $this->memoryInfo = $this->getMockBuilder(MemoryInfo::class) @@ -180,6 +182,7 @@ protected function setUp(): void { $this->secureRandom, $this->iniGetWrapper, $this->connection, + $this->throttler, $this->tempManager, $this->notificationManager, $this->appManager, @@ -665,6 +668,8 @@ public function testCheck() { 'isFairUseOfFreePushService' => false, 'temporaryDirectoryWritable' => false, \OCA\Settings\SetupChecks\LdapInvalidUuids::class => ['pass' => true, 'description' => 'Invalid UUIDs of LDAP users or groups have been found. Please review your "Override UUID detection" settings in the Expert part of the LDAP configuration and use "occ ldap:update-uuid" to update them.', 'severity' => 'warning'], + 'isBruteforceThrottled' => false, + 'bruteforceRemoteAddress' => '', ] ); $this->assertEquals($expected, $this->checkSetupController->check()); @@ -690,6 +695,7 @@ public function testGetCurlVersion() { $this->secureRandom, $this->iniGetWrapper, $this->connection, + $this->throttler, $this->tempManager, $this->notificationManager, $this->appManager, @@ -1455,6 +1461,7 @@ public function testIsMysqlUsedWithoutUTF8MB4(string $db, bool $useUTF8MB4, bool $this->secureRandom, $this->iniGetWrapper, $this->connection, + $this->throttler, $this->tempManager, $this->notificationManager, $this->appManager, @@ -1510,6 +1517,7 @@ public function testIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(string $m $this->secureRandom, $this->iniGetWrapper, $this->connection, + $this->throttler, $this->tempManager, $this->notificationManager, $this->appManager, diff --git a/config/config.sample.php b/config/config.sample.php index b0aac34c066f4..185473ea6c7c8 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -352,6 +352,19 @@ */ 'auth.bruteforce.protection.enabled' => true, +/** + * Whether the bruteforce protection shipped with Nextcloud should be set to testing mode. + * + * In testing mode bruteforce attempts are still recorded, but the requests do + * not sleep/wait for the specified time. They will still abort with + * "429 Too Many Requests" when the maximum delay is reached. + * Enabling this is discouraged for security reasons + * and should only be done for debugging and on CI when running tests. + * + * Defaults to ``false`` + */ +'auth.bruteforce.protection.testing' => false, + /** * Whether the rate limit protection shipped with Nextcloud should be enabled or not. * diff --git a/core/Command/Security/BruteforceAttempts.php b/core/Command/Security/BruteforceAttempts.php new file mode 100644 index 0000000000000..9237078339d71 --- /dev/null +++ b/core/Command/Security/BruteforceAttempts.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\Core\Command\Security; + +use OC\Core\Command\Base; +use OCP\Security\Bruteforce\IThrottler; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; + +class BruteforceAttempts extends Base { + public function __construct( + protected IThrottler $throttler, + ) { + parent::__construct(); + } + + protected function configure(): void { + parent::configure(); + $this + ->setName('security:bruteforce:attempts') + ->setDescription('resets bruteforce attempts for given IP address') + ->addArgument( + 'ipaddress', + InputArgument::REQUIRED, + 'IP address for which the attempts are to be reset', + ) + ->addArgument( + 'action', + InputArgument::OPTIONAL, + 'Only count attempts for the given action', + ) + ; + } + + protected function execute(InputInterface $input, OutputInterface $output): int { + $ip = $input->getArgument('ipaddress'); + + if (!filter_var($ip, FILTER_VALIDATE_IP)) { + $output->writeln('"' . $ip . '" is not a valid IP address'); + return 1; + } + + $data = [ + 'bypass-listed' => $this->throttler->isBypassListed($ip), + 'attempts' => $this->throttler->getAttempts( + $ip, + (string) $input->getArgument('action'), + ), + 'delay' => $this->throttler->getDelay( + $ip, + (string) $input->getArgument('action'), + ), + ]; + + $this->writeArrayInOutputFormat($input, $output, $data); + + return 0; + } +} diff --git a/core/Command/Security/ResetBruteforceAttempts.php b/core/Command/Security/BruteforceResetAttempts.php similarity index 84% rename from core/Command/Security/ResetBruteforceAttempts.php rename to core/Command/Security/BruteforceResetAttempts.php index 8def0873bdfca..3966bd7798bff 100644 --- a/core/Command/Security/ResetBruteforceAttempts.php +++ b/core/Command/Security/BruteforceResetAttempts.php @@ -1,4 +1,6 @@ throttler = $throttler; parent::__construct(); } - protected function configure() { + protected function configure(): void { $this ->setName('security:bruteforce:reset') - ->setDescription('resets bruteforce attemps for given IP address') + ->setDescription('resets bruteforce attempts for given IP address') ->addArgument( 'ipaddress', InputArgument::REQUIRED, diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 4fb020e44a311..c3e892de2945d 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -215,6 +215,14 @@ type: OC.SetupChecks.MESSAGE_TYPE_INFO }); } + if (data.isBruteforceThrottled) { + messages.push({ + msg: t('core', 'Your remote address was identified as "{remoteAddress}" and is bruteforce throttled at the moment slowing down the performance of various requests. If the remote address is not your address this can be an indication that a proxy is not configured correctly. Further information can be found in the {linkstart}documentation ↗{linkend}.', { remoteAddress: data.bruteforceRemoteAddress }) + .replace('{linkstart}', '') + .replace('{linkend}', ''), + type: OC.SetupChecks.MESSAGE_TYPE_ERROR + }); + } if(!data.hasWorkingFileLocking) { messages.push({ msg: t('core', 'Transactional file locking is disabled, this might lead to issues with race conditions. Enable "filelocking.enabled" in config.php to avoid these problems. See the {linkstart}documentation ↗{linkend} for more information.') diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 43f42d2610e66..163a21c46a786 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -814,6 +814,67 @@ describe('OC.SetupChecks tests', function() { }); }); + it('should return an error if the admin IP is bruteforce throttled', function(done) { + var async = OC.SetupChecks.checkSetup(); + + suite.server.requests[0].respond( + 200, + { + 'Content-Type': 'application/json', + }, + JSON.stringify({ + hasFileinfoInstalled: true, + isGetenvServerWorking: true, + isReadOnlyConfig: false, + wasEmailTestSuccessful: true, + hasWorkingFileLocking: true, + hasDBFileLocking: false, + hasValidTransactionIsolationLevel: true, + suggestedOverwriteCliURL: '', + isRandomnessSecure: true, + isFairUseOfFreePushService: true, + isBruteforceThrottled: true, + bruteforceRemoteAddress: '::1', + serverHasInternetConnectionProblems: false, + isMemcacheConfigured: true, + forwardedForHeadersWorking: true, + reverseProxyDocs: 'https://docs.nextcloud.com/foo/bar.html', + isCorrectMemcachedPHPModuleInstalled: true, + hasPassedCodeIntegrityCheck: true, + OpcacheSetupRecommendations: [], + isSettimelimitAvailable: true, + hasFreeTypeSupport: true, + missingIndexes: [], + missingPrimaryKeys: [], + missingColumns: [], + cronErrors: [], + cronInfo: { + diffInSeconds: 0 + }, + isMemoryLimitSufficient: true, + appDirsWithDifferentOwner: [], + isImagickEnabled: true, + areWebauthnExtensionsEnabled: true, + is64bit: true, + recommendedPHPModules: [], + pendingBigIntConversionColumns: [], + isMysqlUsedWithoutUTF8MB4: false, + isDefaultPhoneRegionSet: true, + isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true, + reverseProxyGeneratedURL: 'https://server', + temporaryDirectoryWritable: true, + }) + ); + + async.done(function( data, s, x ){ + expect(data).toEqual([{ + msg: 'Your remote address was identified as "::1" and is bruteforce throttled at the moment slowing down the performance of various requests. If the remote address is not your address this can be an indication that a proxy is not configured correctly. Further information can be found in the documentation ↗.', + type: OC.SetupChecks.MESSAGE_TYPE_ERROR + }]); + done(); + }); + }); + it('should return an error if set_time_limit is unavailable', function(done) { var async = OC.SetupChecks.checkSetup(); diff --git a/core/register_command.php b/core/register_command.php index 32cd4099618f1..0efeed7cd3343 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -209,7 +209,8 @@ $application->add(new OC\Core\Command\Security\ListCertificates(\OC::$server->getCertificateManager(), \OC::$server->getL10N('core'))); $application->add(new OC\Core\Command\Security\ImportCertificate(\OC::$server->getCertificateManager())); $application->add(new OC\Core\Command\Security\RemoveCertificate(\OC::$server->getCertificateManager())); - $application->add(new OC\Core\Command\Security\ResetBruteforceAttempts(\OC::$server->getBruteForceThrottler())); + $application->add(\OC::$server->get(\OC\Core\Command\Security\BruteforceAttempts::class)); + $application->add(\OC::$server->get(\OC\Core\Command\Security\BruteforceResetAttempts::class)); } else { $application->add(\OC::$server->get(\OC\Core\Command\Maintenance\Install::class)); } diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index b8c864abe4dc3..5b543ad55b939 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1002,10 +1002,11 @@ 'OC\\Core\\Command\\Preview\\Generate' => $baseDir . '/core/Command/Preview/Generate.php', 'OC\\Core\\Command\\Preview\\Repair' => $baseDir . '/core/Command/Preview/Repair.php', 'OC\\Core\\Command\\Preview\\ResetRenderedTexts' => $baseDir . '/core/Command/Preview/ResetRenderedTexts.php', + 'OC\\Core\\Command\\Security\\BruteforceAttempts' => $baseDir . '/core/Command/Security/BruteforceAttempts.php', + 'OC\\Core\\Command\\Security\\BruteforceResetAttempts' => $baseDir . '/core/Command/Security/BruteforceResetAttempts.php', 'OC\\Core\\Command\\Security\\ImportCertificate' => $baseDir . '/core/Command/Security/ImportCertificate.php', 'OC\\Core\\Command\\Security\\ListCertificates' => $baseDir . '/core/Command/Security/ListCertificates.php', 'OC\\Core\\Command\\Security\\RemoveCertificate' => $baseDir . '/core/Command/Security/RemoveCertificate.php', - 'OC\\Core\\Command\\Security\\ResetBruteforceAttempts' => $baseDir . '/core/Command/Security/ResetBruteforceAttempts.php', 'OC\\Core\\Command\\Status' => $baseDir . '/core/Command/Status.php', 'OC\\Core\\Command\\SystemTag\\Add' => $baseDir . '/core/Command/SystemTag/Add.php', 'OC\\Core\\Command\\SystemTag\\Delete' => $baseDir . '/core/Command/SystemTag/Delete.php', @@ -1565,6 +1566,9 @@ '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 e099accf68375..19e2f3393d6fe 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1035,10 +1035,11 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Command\\Preview\\Generate' => __DIR__ . '/../../..' . '/core/Command/Preview/Generate.php', 'OC\\Core\\Command\\Preview\\Repair' => __DIR__ . '/../../..' . '/core/Command/Preview/Repair.php', 'OC\\Core\\Command\\Preview\\ResetRenderedTexts' => __DIR__ . '/../../..' . '/core/Command/Preview/ResetRenderedTexts.php', + 'OC\\Core\\Command\\Security\\BruteforceAttempts' => __DIR__ . '/../../..' . '/core/Command/Security/BruteforceAttempts.php', + 'OC\\Core\\Command\\Security\\BruteforceResetAttempts' => __DIR__ . '/../../..' . '/core/Command/Security/BruteforceResetAttempts.php', 'OC\\Core\\Command\\Security\\ImportCertificate' => __DIR__ . '/../../..' . '/core/Command/Security/ImportCertificate.php', 'OC\\Core\\Command\\Security\\ListCertificates' => __DIR__ . '/../../..' . '/core/Command/Security/ListCertificates.php', 'OC\\Core\\Command\\Security\\RemoveCertificate' => __DIR__ . '/../../..' . '/core/Command/Security/RemoveCertificate.php', - 'OC\\Core\\Command\\Security\\ResetBruteforceAttempts' => __DIR__ . '/../../..' . '/core/Command/Security/ResetBruteforceAttempts.php', 'OC\\Core\\Command\\Status' => __DIR__ . '/../../..' . '/core/Command/Status.php', 'OC\\Core\\Command\\SystemTag\\Add' => __DIR__ . '/../../..' . '/core/Command/SystemTag/Add.php', 'OC\\Core\\Command\\SystemTag\\Delete' => __DIR__ . '/../../..' . '/core/Command/SystemTag/Delete.php', @@ -1598,6 +1599,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/AppFramework/Middleware/Security/BruteForceMiddleware.php b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php index 2ecd26a68e1ff..a0b915588ad16 100644 --- a/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php @@ -51,6 +51,8 @@ * @package OC\AppFramework\Middleware\Security */ class BruteForceMiddleware extends Middleware { + private int $delaySlept = 0; + public function __construct( protected ControllerMethodReflector $reflector, protected Throttler $throttler, @@ -67,7 +69,7 @@ public function beforeController($controller, $methodName) { if ($this->reflector->hasAnnotation('BruteForceProtection')) { $action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action'); - $this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), $action); + $this->delaySlept += $this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), $action); } else { $reflectionMethod = new ReflectionMethod($controller, $methodName); $attributes = $reflectionMethod->getAttributes(BruteForceProtection::class); @@ -79,7 +81,7 @@ public function beforeController($controller, $methodName) { /** @var BruteForceProtection $protection */ $protection = $attribute->newInstance(); $action = $protection->getAction(); - $this->throttler->sleepDelayOrThrowOnMax($remoteAddress, $action); + $this->delaySlept += $this->throttler->sleepDelayOrThrowOnMax($remoteAddress, $action); } } } @@ -95,7 +97,7 @@ public function afterController($controller, $methodName, Response $response) { $action = $this->reflector->getAnnotationParameter('BruteForceProtection', 'action'); $ip = $this->request->getRemoteAddress(); $this->throttler->registerAttempt($action, $ip, $response->getThrottleMetadata()); - $this->throttler->sleepDelayOrThrowOnMax($ip, $action); + $this->delaySlept += $this->throttler->sleepDelayOrThrowOnMax($ip, $action); } else { $reflectionMethod = new ReflectionMethod($controller, $methodName); $attributes = $reflectionMethod->getAttributes(BruteForceProtection::class); @@ -111,7 +113,7 @@ public function afterController($controller, $methodName, Response $response) { if (!isset($metaData['action']) || $metaData['action'] === $action) { $this->throttler->registerAttempt($action, $ip, $metaData); - $this->throttler->sleepDelayOrThrowOnMax($ip, $action); + $this->delaySlept += $this->throttler->sleepDelayOrThrowOnMax($ip, $action); } } } else { @@ -127,6 +129,10 @@ public function afterController($controller, $methodName, Response $response) { } } + if ($this->delaySlept) { + $response->addHeader('X-Nextcloud-Bruteforce-Throttled', $this->delaySlept . 'ms'); + } + return parent::afterController($controller, $methodName, $response); } diff --git a/lib/private/Security/Bruteforce/Backend/DatabaseBackend.php b/lib/private/Security/Bruteforce/Backend/DatabaseBackend.php new file mode 100644 index 0000000000000..04f2a7b639737 --- /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 0000000000000..4b40262e645dc --- /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 0000000000000..432e99700fe34 --- /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/Capabilities.php b/lib/private/Security/Bruteforce/Capabilities.php index 60cf3086f2db9..b50eea0b7af8e 100644 --- a/lib/private/Security/Bruteforce/Capabilities.php +++ b/lib/private/Security/Bruteforce/Capabilities.php @@ -3,9 +3,11 @@ declare(strict_types=1); /** + * @copyright Copyright (c) 2023 Joas Schilling * @copyright Copyright (c) 2017 Roeland Jago Douma * * @author J0WI + * @author Joas Schilling * @author Julius Härtl * @author Roeland Jago Douma * @@ -30,35 +32,24 @@ use OCP\Capabilities\IPublicCapability; use OCP\Capabilities\IInitialStateExcludedCapability; use OCP\IRequest; +use OCP\Security\Bruteforce\IThrottler; class Capabilities implements IPublicCapability, IInitialStateExcludedCapability { - /** @var IRequest */ - private $request; - - /** @var Throttler */ - private $throttler; + public function __construct( + private IRequest $request, + private IThrottler $throttler, + ) { + } /** - * Capabilities constructor. - * - * @param IRequest $request - * @param Throttler $throttler + * @return array{bruteforce: array{delay: int, allow-listed: bool}} */ - public function __construct(IRequest $request, - Throttler $throttler) { - $this->request = $request; - $this->throttler = $throttler; - } - public function getCapabilities(): array { - if (version_compare(\OC::$server->getConfig()->getSystemValueString('version', '0.0.0.0'), '12.0.0.0', '<')) { - return []; - } - return [ 'bruteforce' => [ - 'delay' => $this->throttler->getDelay($this->request->getRemoteAddress()) - ] + 'delay' => $this->throttler->getDelay($this->request->getRemoteAddress()), + 'allow-listed' => $this->throttler->isBypassListed($this->request->getRemoteAddress()), + ], ]; } } diff --git a/lib/private/Security/Bruteforce/Throttler.php b/lib/private/Security/Bruteforce/Throttler.php index d5fd0984baab6..a0cc5ae4ecb27 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 @@ */ 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 @@ * @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 @@ public function registerAttempt(string $action, } $ipAddress = new IpAddress($ip); - $values = [ - 'action' => $action, - 'occurred' => $this->timeFactory->getTime(), - 'ip' => (string)$ipAddress, - 'subnet' => $ipAddress->getSubnet(), - 'metadata' => json_encode($metadata), - ]; + if ($this->isBypassListed((string)$ipAddress)) { + return; + } $this->logger->notice( sprintf( @@ -136,12 +95,13 @@ public function registerAttempt(string $action, ] ); - $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 + ); } /** @@ -150,8 +110,13 @@ public function registerAttempt(string $action, * @param string $ip * @return bool */ - private function isIPWhitelisted(string $ip): bool { + public function isBypassListed(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 @@ private function isIPWhitelisted(string $ip): bool { } elseif (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { $type = 6; } else { + $this->ipIsWhitelisted[$ip] = false; return false; } @@ -202,20 +168,26 @@ private function isIPWhitelisted(string $ip): bool { } 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) { @@ -228,35 +200,21 @@ public function getAttempts(string $ip, string $action = '', float $maxAgeHours } $ipAddress = new IpAddress($ip); - if ($this->isIPWhitelisted((string)$ipAddress)) { + if ($this->isBypassListed((string)$ipAddress)) { 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,69 +236,58 @@ public function getDelay(string $ip, string $action = ''): int { } /** - * 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 { - $ipAddress = new IpAddress($ip); - if ($this->isIPWhitelisted((string)$ipAddress)) { + // No need to log if the bruteforce protection is disabled + if (!$this->config->getSystemValueBool('auth.bruteforce.protection.enabled', true)) { 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)))); + $ipAddress = new IpAddress($ip); + if ($this->isBypassListed((string)$ipAddress)) { + return; + } - $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->isBypassListed((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); - usleep($delay * 1000); + if (!$this->config->getSystemValueBool('auth.bruteforce.protection.testing')) { + usleep($delay * 1000); + } return $delay; } /** - * 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); @@ -359,7 +306,9 @@ public function sleepDelayOrThrowOnMax(string $ip, string $action = ''): int { 'delay' => $delay, ]); } - usleep($delay * 1000); + if (!$this->config->getSystemValueBool('auth.bruteforce.protection.testing')) { + usleep($delay * 1000); + } return $delay; } } diff --git a/lib/private/Server.php b/lib/private/Server.php index 516dc0b6e8b0c..bb073891d03ab 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1011,6 +1011,18 @@ public function __construct($webRoot, \OC\Config $config) { /** @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 6f492d6c59dce..620a53fd35479 100644 --- a/lib/public/Security/Bruteforce/IThrottler.php +++ b/lib/public/Security/Bruteforce/IThrottler.php @@ -40,16 +40,19 @@ 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,11 +61,21 @@ 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; + + /** + * Check if the IP is allowed to bypass the brute force protection + * + * @param string $ip + * @return bool + * @since 28.0.0 + */ + public function isBypassListed(string $ip): bool; + /** * Get the throttling delay (in milliseconds) * @@ -71,9 +84,20 @@ public function registerAttempt(string $action, string $ip, array $metadata = [] * @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 +105,7 @@ public function getAttempts(string $ip, string $action = '', float $maxAgeHours * @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 +124,7 @@ public function resetDelay(string $ip, string $action, array $metadata): void; * * @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 +135,7 @@ public function resetDelayForIP(string $ip): void; * @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 0000000000000..648d862742165 --- /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/CapabilitiesTest.php b/tests/lib/Security/Bruteforce/CapabilitiesTest.php index 1c2bbb6bc53b2..266acdcb285ad 100644 --- a/tests/lib/Security/Bruteforce/CapabilitiesTest.php +++ b/tests/lib/Security/Bruteforce/CapabilitiesTest.php @@ -25,8 +25,8 @@ namespace Test\Security\Bruteforce; use OC\Security\Bruteforce\Capabilities; -use OC\Security\Bruteforce\Throttler; use OCP\IRequest; +use OCP\Security\Bruteforce\IThrottler; use Test\TestCase; class CapabilitiesTest extends TestCase { @@ -36,7 +36,7 @@ class CapabilitiesTest extends TestCase { /** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */ private $request; - /** @var Throttler|\PHPUnit\Framework\MockObject\MockObject */ + /** @var IThrottler|\PHPUnit\Framework\MockObject\MockObject */ private $throttler; protected function setUp(): void { @@ -44,7 +44,7 @@ protected function setUp(): void { $this->request = $this->createMock(IRequest::class); - $this->throttler = $this->createMock(Throttler::class); + $this->throttler = $this->createMock(IThrottler::class); $this->capabilities = new Capabilities( $this->request, @@ -52,18 +52,24 @@ protected function setUp(): void { ); } - public function testGetCapabilities() { + public function testGetCapabilities(): void { $this->throttler->expects($this->atLeastOnce()) ->method('getDelay') ->with('10.10.10.10') ->willReturn(42); + $this->throttler->expects($this->atLeastOnce()) + ->method('isBypassListed') + ->with('10.10.10.10') + ->willReturn(true); + $this->request->method('getRemoteAddress') ->willReturn('10.10.10.10'); $expected = [ 'bruteforce' => [ - 'delay' => 42 + 'delay' => 42, + 'allow-listed' => true, ] ]; $result = $this->capabilities->getCapabilities(); @@ -71,7 +77,7 @@ public function testGetCapabilities() { $this->assertEquals($expected, $result); } - public function testGetCapabilitiesOnCli() { + public function testGetCapabilitiesOnCli(): void { $this->throttler->expects($this->atLeastOnce()) ->method('getDelay') ->with('') @@ -82,7 +88,8 @@ public function testGetCapabilitiesOnCli() { $expected = [ 'bruteforce' => [ - 'delay' => 0 + 'delay' => 0, + 'allow-listed' => false, ] ]; $result = $this->capabilities->getCapabilities(); diff --git a/tests/lib/Security/Bruteforce/ThrottlerTest.php b/tests/lib/Security/Bruteforce/ThrottlerTest.php index f23b15a06d70a..e368a0912b10a 100644 --- a/tests/lib/Security/Bruteforce/ThrottlerTest.php +++ b/tests/lib/Security/Bruteforce/ThrottlerTest.php @@ -24,8 +24,9 @@ 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 [ [ @@ -200,7 +185,7 @@ private function isIpWhiteListedHelper($ip, $this->assertSame( ($enabled === false) ? true : $isWhiteListed, - self::invokePrivate($this->throttler, 'isIPWhitelisted', [$ip]) + self::invokePrivate($this->throttler, 'isBypassListed', [$ip]) ); }