Skip to content

Commit

Permalink
fix(ratelimit): Allow to bypass rate-limit from bruteforce allowlist
Browse files Browse the repository at this point in the history
Signed-off-by: Joas Schilling <[email protected]>
  • Loading branch information
nickvergessen committed Jan 17, 2025
1 parent 326120a commit 9bef113
Show file tree
Hide file tree
Showing 8 changed files with 240 additions and 279 deletions.
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1941,6 +1941,7 @@
'OC\\Security\\IdentityProof\\Manager' => $baseDir . '/lib/private/Security/IdentityProof/Manager.php',
'OC\\Security\\IdentityProof\\Signer' => $baseDir . '/lib/private/Security/IdentityProof/Signer.php',
'OC\\Security\\Ip\\Address' => $baseDir . '/lib/private/Security/Ip/Address.php',
'OC\\Security\\Ip\\BruteforceAllowList' => $baseDir . '/lib/private/Security/Ip/BruteforceAllowList.php',
'OC\\Security\\Ip\\Factory' => $baseDir . '/lib/private/Security/Ip/Factory.php',
'OC\\Security\\Ip\\Range' => $baseDir . '/lib/private/Security/Ip/Range.php',
'OC\\Security\\Ip\\RemoteAddress' => $baseDir . '/lib/private/Security/Ip/RemoteAddress.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1990,6 +1990,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Security\\IdentityProof\\Manager' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Manager.php',
'OC\\Security\\IdentityProof\\Signer' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Signer.php',
'OC\\Security\\Ip\\Address' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/Address.php',
'OC\\Security\\Ip\\BruteforceAllowList' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/BruteforceAllowList.php',
'OC\\Security\\Ip\\Factory' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/Factory.php',
'OC\\Security\\Ip\\Range' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/Range.php',
'OC\\Security\\Ip\\RemoteAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/RemoteAddress.php',
Expand Down
5 changes: 4 additions & 1 deletion lib/private/AppFramework/DependencyInjection/DIContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use OC\Core\Middleware\TwoFactorMiddleware;
use OC\Diagnostics\EventLogger;
use OC\Log\PsrLoggerAdapter;
use OC\Security\Ip\BruteforceAllowList;
use OC\ServerContainer;
use OC\Settings\AuthorizedGroupMapper;
use OC\User\Manager as UserManager;
Expand Down Expand Up @@ -280,7 +281,9 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta
$c->get(IUserSession::class),
$c->get(IControllerMethodReflector::class),
$c->get(OC\Security\RateLimiting\Limiter::class),
$c->get(ISession::class)
$c->get(ISession::class),
$c->get(\OCP\IAppConfig::class),
$c->get(BruteforceAllowList::class),
)
);
$dispatcher->registerMiddleware(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OC\AppFramework\Middleware\Security;

use OC\AppFramework\Utility\ControllerMethodReflector;
use OC\Security\Ip\BruteforceAllowList;
use OC\Security\RateLimiting\Exception\RateLimitExceededException;
use OC\Security\RateLimiting\Limiter;
use OC\User\Session;
Expand All @@ -20,6 +21,7 @@
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Middleware;
use OCP\IAppConfig;
use OCP\IRequest;
use OCP\ISession;
use OCP\IUserSession;
Expand Down Expand Up @@ -53,6 +55,8 @@ public function __construct(
protected ControllerMethodReflector $reflector,
protected Limiter $limiter,
protected ISession $session,
protected IAppConfig $appConfig,
protected BruteforceAllowList $allowList,
) {
}

Expand All @@ -73,6 +77,11 @@ public function beforeController(Controller $controller, string $methodName): vo
$rateLimit = $this->readLimitFromAnnotationOrAttribute($controller, $methodName, 'UserRateThrottle', UserRateLimit::class);

if ($rateLimit !== null) {
if ($this->appConfig->getValueBool('bruteforcesettings', 'allowlist_ratelimit')
&& $this->allowList->isBypassListed($this->request->getRemoteAddress())) {
return;
}

$this->limiter->registerUserRequest(
$rateLimitIdentifier,
$rateLimit->getLimit(),
Expand Down
69 changes: 3 additions & 66 deletions lib/private/Security/Bruteforce/Throttler.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OC\Security\Bruteforce;

use OC\Security\Bruteforce\Backend\IBackend;
use OC\Security\Ip\BruteforceAllowList;
use OC\Security\Normalizer\IpAddress;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
Expand All @@ -32,14 +33,13 @@
class Throttler implements IThrottler {
/** @var bool[] */
private array $hasAttemptsDeleted = [];
/** @var bool[] */
private array $ipIsWhitelisted = [];

public function __construct(
private ITimeFactory $timeFactory,
private LoggerInterface $logger,
private IConfig $config,
private IBackend $backend,
private BruteforceAllowList $allowList,
) {
}

Expand Down Expand Up @@ -83,70 +83,7 @@ public function registerAttempt(string $action,
* Check if the IP is whitelisted
*/
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;
}

$keys = $this->config->getAppKeys('bruteForce');
$keys = array_filter($keys, function ($key) {
return str_starts_with($key, 'whitelist_');
});

if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
$type = 4;
} elseif (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
$type = 6;
} else {
$this->ipIsWhitelisted[$ip] = false;
return false;
}

$ip = inet_pton($ip);

foreach ($keys as $key) {
$cidr = $this->config->getAppValue('bruteForce', $key, null);

$cx = explode('/', $cidr);
$addr = $cx[0];
$mask = (int)$cx[1];

// Do not compare ipv4 to ipv6
if (($type === 4 && !filter_var($addr, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) ||
($type === 6 && !filter_var($addr, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6))) {
continue;
}

$addr = inet_pton($addr);

$valid = true;
for ($i = 0; $i < $mask; $i++) {
$part = ord($addr[(int)($i / 8)]);
$orig = ord($ip[(int)($i / 8)]);

$bitmask = 1 << (7 - ($i % 8));

$part = $part & $bitmask;
$orig = $orig & $bitmask;

if ($part !== $orig) {
$valid = false;
break;
}
}

if ($valid === true) {
$this->ipIsWhitelisted[$ip] = true;
return true;
}
}

$this->ipIsWhitelisted[$ip] = false;
return false;
return $this->allowList->isBypassListed($ip);
}

/**
Expand Down
60 changes: 60 additions & 0 deletions lib/private/Security/Ip/BruteforceAllowList.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OC\Security\Ip;

use OCP\IAppConfig;
use OCP\Security\Ip\IFactory;

class BruteforceAllowList {
/** @var array<string, bool> */
protected array $ipIsAllowListed = [];

public function __construct(
private readonly IAppConfig $appConfig,
private readonly IFactory $factory,
) {
}

/**
* Check if the IP is allowed to bypass bruteforce protection
*/
public function isBypassListed(string $ip): bool {
if (isset($this->ipIsAllowListed[$ip])) {
return $this->ipIsAllowListed[$ip];
}

try {
$address = $this->factory->addressFromString($ip);
} catch (\InvalidArgumentException) {
$this->ipIsAllowListed[$ip] = false;
return false;
}

$keys = $this->appConfig->getKeys('bruteForce');
$keys = array_filter($keys, static fn ($key): bool => str_starts_with($key, 'whitelist_'));

foreach ($keys as $key) {
$rangeString = $this->appConfig->getValueString('bruteForce', $key);
try {
$range = $this->factory->rangeFromString($rangeString);
} catch (\InvalidArgumentException) {
continue;
}

$allowed = $range->contains($address);
if ($allowed) {
$this->ipIsAllowListed[$ip] = true;
return true;
}
}

$this->ipIsAllowListed[$ip] = false;
return false;
}
}
Loading

0 comments on commit 9bef113

Please sign in to comment.