Skip to content

Commit 1bb90af

Browse files
nickvergessennickvergessen
committed
fix(ratelimit): Allow to bypass rate-limit from bruteforce allowlist
Signed-off-by: Joas Schilling <[email protected]>
1 parent 56c4565 commit 1bb90af

File tree

9 files changed

+244
-288
lines changed

9 files changed

+244
-288
lines changed

‎lib/composer/composer/autoload_classmap.php

+1
Original file line numberDiff line numberDiff line change
@@ -1942,6 +1942,7 @@
19421942
'OC\\Security\\IdentityProof\\Manager' => $baseDir . '/lib/private/Security/IdentityProof/Manager.php',
19431943
'OC\\Security\\IdentityProof\\Signer' => $baseDir . '/lib/private/Security/IdentityProof/Signer.php',
19441944
'OC\\Security\\Ip\\Address' => $baseDir . '/lib/private/Security/Ip/Address.php',
1945+
'OC\\Security\\Ip\\BruteforceAllowList' => $baseDir . '/lib/private/Security/Ip/BruteforceAllowList.php',
19451946
'OC\\Security\\Ip\\Factory' => $baseDir . '/lib/private/Security/Ip/Factory.php',
19461947
'OC\\Security\\Ip\\Range' => $baseDir . '/lib/private/Security/Ip/Range.php',
19471948
'OC\\Security\\Ip\\RemoteAddress' => $baseDir . '/lib/private/Security/Ip/RemoteAddress.php',

‎lib/composer/composer/autoload_static.php

+1
Original file line numberDiff line numberDiff line change
@@ -1991,6 +1991,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
19911991
'OC\\Security\\IdentityProof\\Manager' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Manager.php',
19921992
'OC\\Security\\IdentityProof\\Signer' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Signer.php',
19931993
'OC\\Security\\Ip\\Address' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/Address.php',
1994+
'OC\\Security\\Ip\\BruteforceAllowList' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/BruteforceAllowList.php',
19941995
'OC\\Security\\Ip\\Factory' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/Factory.php',
19951996
'OC\\Security\\Ip\\Range' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/Range.php',
19961997
'OC\\Security\\Ip\\RemoteAddress' => __DIR__ . '/../../..' . '/lib/private/Security/Ip/RemoteAddress.php',

‎lib/private/AppFramework/DependencyInjection/DIContainer.php

+1-9
Original file line numberDiff line numberDiff line change
@@ -274,15 +274,7 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta
274274
$c->get(LoggerInterface::class)
275275
)
276276
);
277-
$dispatcher->registerMiddleware(
278-
new RateLimitingMiddleware(
279-
$c->get(IRequest::class),
280-
$c->get(IUserSession::class),
281-
$c->get(IControllerMethodReflector::class),
282-
$c->get(OC\Security\RateLimiting\Limiter::class),
283-
$c->get(ISession::class)
284-
)
285-
);
277+
$dispatcher->registerMiddleware($c->get(RateLimitingMiddleware::class));
286278
$dispatcher->registerMiddleware(
287279
new OC\AppFramework\Middleware\PublicShare\PublicShareMiddleware(
288280
$c->get(IRequest::class),

‎lib/private/AppFramework/Middleware/Security/RateLimitingMiddleware.php

+9
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace OC\AppFramework\Middleware\Security;
1010

1111
use OC\AppFramework\Utility\ControllerMethodReflector;
12+
use OC\Security\Ip\BruteforceAllowList;
1213
use OC\Security\RateLimiting\Exception\RateLimitExceededException;
1314
use OC\Security\RateLimiting\Limiter;
1415
use OC\User\Session;
@@ -20,6 +21,7 @@
2021
use OCP\AppFramework\Http\Response;
2122
use OCP\AppFramework\Http\TemplateResponse;
2223
use OCP\AppFramework\Middleware;
24+
use OCP\IAppConfig;
2325
use OCP\IRequest;
2426
use OCP\ISession;
2527
use OCP\IUserSession;
@@ -53,6 +55,8 @@ public function __construct(
5355
protected ControllerMethodReflector $reflector,
5456
protected Limiter $limiter,
5557
protected ISession $session,
58+
protected IAppConfig $appConfig,
59+
protected BruteforceAllowList $allowList,
5660
) {
5761
}
5862

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

7579
if ($rateLimit !== null) {
80+
if ($this->appConfig->getValueBool('bruteforcesettings', 'allowlist_ratelimit')
81+
&& $this->allowList->isBypassListed($this->request->getRemoteAddress())) {
82+
return;
83+
}
84+
7685
$this->limiter->registerUserRequest(
7786
$rateLimitIdentifier,
7887
$rateLimit->getLimit(),

‎lib/private/Security/Bruteforce/Throttler.php

+3-66
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace OC\Security\Bruteforce;
1010

1111
use OC\Security\Bruteforce\Backend\IBackend;
12+
use OC\Security\Ip\BruteforceAllowList;
1213
use OC\Security\Normalizer\IpAddress;
1314
use OCP\AppFramework\Utility\ITimeFactory;
1415
use OCP\IConfig;
@@ -32,14 +33,13 @@
3233
class Throttler implements IThrottler {
3334
/** @var bool[] */
3435
private array $hasAttemptsDeleted = [];
35-
/** @var bool[] */
36-
private array $ipIsWhitelisted = [];
3736

3837
public function __construct(
3938
private ITimeFactory $timeFactory,
4039
private LoggerInterface $logger,
4140
private IConfig $config,
4241
private IBackend $backend,
42+
private BruteforceAllowList $allowList,
4343
) {
4444
}
4545

@@ -83,70 +83,7 @@ public function registerAttempt(string $action,
8383
* Check if the IP is whitelisted
8484
*/
8585
public function isBypassListed(string $ip): bool {
86-
if (isset($this->ipIsWhitelisted[$ip])) {
87-
return $this->ipIsWhitelisted[$ip];
88-
}
89-
90-
if (!$this->config->getSystemValueBool('auth.bruteforce.protection.enabled', true)) {
91-
$this->ipIsWhitelisted[$ip] = true;
92-
return true;
93-
}
94-
95-
$keys = $this->config->getAppKeys('bruteForce');
96-
$keys = array_filter($keys, function ($key) {
97-
return str_starts_with($key, 'whitelist_');
98-
});
99-
100-
if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
101-
$type = 4;
102-
} elseif (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
103-
$type = 6;
104-
} else {
105-
$this->ipIsWhitelisted[$ip] = false;
106-
return false;
107-
}
108-
109-
$ip = inet_pton($ip);
110-
111-
foreach ($keys as $key) {
112-
$cidr = $this->config->getAppValue('bruteForce', $key, null);
113-
114-
$cx = explode('/', $cidr);
115-
$addr = $cx[0];
116-
$mask = (int)$cx[1];
117-
118-
// Do not compare ipv4 to ipv6
119-
if (($type === 4 && !filter_var($addr, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) ||
120-
($type === 6 && !filter_var($addr, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6))) {
121-
continue;
122-
}
123-
124-
$addr = inet_pton($addr);
125-
126-
$valid = true;
127-
for ($i = 0; $i < $mask; $i++) {
128-
$part = ord($addr[(int)($i / 8)]);
129-
$orig = ord($ip[(int)($i / 8)]);
130-
131-
$bitmask = 1 << (7 - ($i % 8));
132-
133-
$part = $part & $bitmask;
134-
$orig = $orig & $bitmask;
135-
136-
if ($part !== $orig) {
137-
$valid = false;
138-
break;
139-
}
140-
}
141-
142-
if ($valid === true) {
143-
$this->ipIsWhitelisted[$ip] = true;
144-
return true;
145-
}
146-
}
147-
148-
$this->ipIsWhitelisted[$ip] = false;
149-
return false;
86+
return $this->allowList->isBypassListed($ip);
15087
}
15188

15289
/**

‎lib/private/Security/Ip/BruteforceAllowList.php

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OC\Security\Ip;
10+
11+
use OCP\IAppConfig;
12+
use OCP\Security\Ip\IFactory;
13+
14+
class BruteforceAllowList {
15+
/** @var array<string, bool> */
16+
protected array $ipIsAllowListed = [];
17+
18+
public function __construct(
19+
private readonly IAppConfig $appConfig,
20+
private readonly IFactory $factory,
21+
) {
22+
}
23+
24+
/**
25+
* Check if the IP is allowed to bypass bruteforce protection
26+
*/
27+
public function isBypassListed(string $ip): bool {
28+
if (isset($this->ipIsAllowListed[$ip])) {
29+
return $this->ipIsAllowListed[$ip];
30+
}
31+
32+
try {
33+
$address = $this->factory->addressFromString($ip);
34+
} catch (\InvalidArgumentException) {
35+
$this->ipIsAllowListed[$ip] = false;
36+
return false;
37+
}
38+
39+
$keys = $this->appConfig->getKeys('bruteForce');
40+
$keys = array_filter($keys, static fn ($key): bool => str_starts_with($key, 'whitelist_'));
41+
42+
foreach ($keys as $key) {
43+
$rangeString = $this->appConfig->getValueString('bruteForce', $key);
44+
try {
45+
$range = $this->factory->rangeFromString($rangeString);
46+
} catch (\InvalidArgumentException) {
47+
continue;
48+
}
49+
50+
$allowed = $range->contains($address);
51+
if ($allowed) {
52+
$this->ipIsAllowListed[$ip] = true;
53+
return true;
54+
}
55+
}
56+
57+
$this->ipIsAllowListed[$ip] = false;
58+
return false;
59+
}
60+
}

‎tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php

+7-1
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@
1111

1212
use OC\AppFramework\Middleware\Security\RateLimitingMiddleware;
1313
use OC\AppFramework\Utility\ControllerMethodReflector;
14+
use OC\Security\Ip\BruteforceAllowList;
1415
use OC\Security\RateLimiting\Exception\RateLimitExceededException;
1516
use OC\Security\RateLimiting\Limiter;
1617
use OCP\AppFramework\Controller;
1718
use OCP\AppFramework\Http\Attribute\AnonRateLimit;
1819
use OCP\AppFramework\Http\Attribute\UserRateLimit;
1920
use OCP\AppFramework\Http\DataResponse;
2021
use OCP\AppFramework\Http\TemplateResponse;
22+
use OCP\IAppConfig;
2123
use OCP\IRequest;
2224
use OCP\ISession;
2325
use OCP\IUser;
@@ -71,13 +73,17 @@ protected function setUp(): void {
7173
$this->reflector = new ControllerMethodReflector();
7274
$this->limiter = $this->createMock(Limiter::class);
7375
$this->session = $this->createMock(ISession::class);
76+
$this->appConfig = $this->createMock(IAppConfig::class);
77+
$this->allowList = $this->createMock(BruteforceAllowList::class);
7478

7579
$this->rateLimitingMiddleware = new RateLimitingMiddleware(
7680
$this->request,
7781
$this->userSession,
7882
$this->reflector,
7983
$this->limiter,
80-
$this->session
84+
$this->session,
85+
$this->appConfig,
86+
$this->allowList,
8187
);
8288
}
8389

0 commit comments

Comments
 (0)