Skip to content

Commit da6dd95

Browse files
committed
feat(security): restrict admin actions to IP ranges
Signed-off-by: Benjamin Gaussorgues <[email protected]>
1 parent 4362ed5 commit da6dd95

File tree

17 files changed

+336
-101
lines changed

17 files changed

+336
-101
lines changed

apps/settings/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => $baseDir . '/../lib/Settings/Personal/Security/TwoFactor.php',
7979
'OCA\\Settings\\Settings\\Personal\\Security\\WebAuthn' => $baseDir . '/../lib/Settings/Personal/Security/WebAuthn.php',
8080
'OCA\\Settings\\Settings\\Personal\\ServerDevNotice' => $baseDir . '/../lib/Settings/Personal/ServerDevNotice.php',
81+
'OCA\\Settings\\SetupChecks\\AllowedAdminRanges' => $baseDir . '/../lib/SetupChecks/AllowedAdminRanges.php',
8182
'OCA\\Settings\\SetupChecks\\AppDirsWithDifferentOwner' => $baseDir . '/../lib/SetupChecks/AppDirsWithDifferentOwner.php',
8283
'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => $baseDir . '/../lib/SetupChecks/BruteForceThrottler.php',
8384
'OCA\\Settings\\SetupChecks\\CheckServerResponseTrait' => $baseDir . '/../lib/SetupChecks/CheckServerResponseTrait.php',

apps/settings/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ class ComposerStaticInitSettings
9393
'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/TwoFactor.php',
9494
'OCA\\Settings\\Settings\\Personal\\Security\\WebAuthn' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/WebAuthn.php',
9595
'OCA\\Settings\\Settings\\Personal\\ServerDevNotice' => __DIR__ . '/..' . '/../lib/Settings/Personal/ServerDevNotice.php',
96+
'OCA\\Settings\\SetupChecks\\AllowedAdminRanges' => __DIR__ . '/..' . '/../lib/SetupChecks/AllowedAdminRanges.php',
9697
'OCA\\Settings\\SetupChecks\\AppDirsWithDifferentOwner' => __DIR__ . '/..' . '/../lib/SetupChecks/AppDirsWithDifferentOwner.php',
9798
'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => __DIR__ . '/..' . '/../lib/SetupChecks/BruteForceThrottler.php',
9899
'OCA\\Settings\\SetupChecks\\CheckServerResponseTrait' => __DIR__ . '/..' . '/../lib/SetupChecks/CheckServerResponseTrait.php',

apps/settings/composer/composer/installed.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
'name' => '__root__',
44
'pretty_version' => 'dev-master',
55
'version' => 'dev-master',
6-
'reference' => '4ff660ca2e0baa02440ba07296ed7e75fa544c0e',
6+
'reference' => '071fe73d0a28f44c6e24cc87fbd00e54a3b92b57',
77
'type' => 'library',
88
'install_path' => __DIR__ . '/../',
99
'aliases' => array(),
@@ -13,7 +13,7 @@
1313
'__root__' => array(
1414
'pretty_version' => 'dev-master',
1515
'version' => 'dev-master',
16-
'reference' => '4ff660ca2e0baa02440ba07296ed7e75fa544c0e',
16+
'reference' => '071fe73d0a28f44c6e24cc87fbd00e54a3b92b57',
1717
'type' => 'library',
1818
'install_path' => __DIR__ . '/../',
1919
'aliases' => array(),

apps/settings/lib/AppInfo/Application.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use OCA\Settings\Search\AppSearch;
2323
use OCA\Settings\Search\SectionSearch;
2424
use OCA\Settings\Search\UserSearch;
25+
use OCA\Settings\SetupChecks\AllowedAdminRanges;
2526
use OCA\Settings\SetupChecks\AppDirsWithDifferentOwner;
2627
use OCA\Settings\SetupChecks\BruteForceThrottler;
2728
use OCA\Settings\SetupChecks\CheckUserCertificates;
@@ -154,6 +155,7 @@ public function register(IRegistrationContext $context): void {
154155
Util::getDefaultEmailAddress('no-reply')
155156
);
156157
});
158+
$context->registerSetupCheck(AllowedAdminRanges::class);
157159
$context->registerSetupCheck(AppDirsWithDifferentOwner::class);
158160
$context->registerSetupCheck(BruteForceThrottler::class);
159161
$context->registerSetupCheck(CheckUserCertificates::class);
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OCA\Settings\SetupChecks;
10+
11+
use IPLib\Factory;
12+
use OC\Security\RemoteIpAddress;
13+
use OCP\IConfig;
14+
use OCP\IL10N;
15+
use OCP\SetupCheck\ISetupCheck;
16+
use OCP\SetupCheck\SetupResult;
17+
18+
class AllowedAdminRanges implements ISetupCheck {
19+
public function __construct(
20+
private IConfig $config,
21+
private IL10N $l10n,
22+
) {
23+
}
24+
25+
public function getCategory(): string {
26+
return 'system';
27+
}
28+
29+
public function getName(): string {
30+
return $this->l10n->t('Allowed admin IP ranges');
31+
}
32+
33+
public function run(): SetupResult {
34+
$allowedAdminRanges = $this->config->getSystemValue(RemoteIpAddress::SETTING_NAME, false);
35+
if ($allowedAdminRanges === false) {
36+
return SetupResult::success($this->l10n->t('Admin IP filtering isn’t applied.'));
37+
}
38+
39+
if (!is_array($allowedAdminRanges)) {
40+
return SetupResult::error(
41+
$this->l10n->t(
42+
'Configuration key "%1$s" expects an array (%2$s found). Admin IP range validation will not be applied.',
43+
[RemoteIpAddress::SETTING_NAME, gettype($allowedAdminRanges)],
44+
)
45+
);
46+
}
47+
48+
$invalidRanges = array_reduce($allowedAdminRanges, static function (array $carry, mixed $range) {
49+
if (!is_string($range) || Factory::parseRangeString($range) === null) {
50+
$carry[] = $range;
51+
}
52+
53+
return $carry;
54+
}, []);
55+
if (count($invalidRanges) > 0) {
56+
return SetupResult::warning(
57+
$this->l10n->t(
58+
'Configuration key "%1$s" contains invalid IP range(s): "%2$s"',
59+
[RemoteIpAddress::SETTING_NAME, implode('", "', $invalidRanges)],
60+
),
61+
);
62+
}
63+
64+
return SetupResult::success($this->l10n->t('Admin IP filtering is correctly configured.'));
65+
}
66+
}

config/config.sample.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2207,6 +2207,16 @@
22072207
*/
22082208
'forwarded_for_headers' => ['HTTP_X_FORWARDED', 'HTTP_FORWARDED_FOR'],
22092209

2210+
/**
2211+
* List of trusted IP ranges for admin actions
2212+
*
2213+
* If this list is non-empty, all admin actions must be triggered from
2214+
* IP addresses inside theses ranges.
2215+
*
2216+
* Defaults to an empty array.
2217+
*/
2218+
'allowed_admin_ranges' => ['192.0.2.42/32', '233.252.0.0/24', '2001:db8::13:37/64'],
2219+
22102220
/**
22112221
* max file size for animating gifs on public-sharing-site.
22122222
* If the gif is bigger, it'll show a static preview

lib/composer/composer/autoload_classmap.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -894,6 +894,7 @@
894894
'OC\\AppFramework\\Middleware\\Security\\BruteForceMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php',
895895
'OC\\AppFramework\\Middleware\\Security\\CORSMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php',
896896
'OC\\AppFramework\\Middleware\\Security\\CSPMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php',
897+
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AdminIpNotAllowedException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/AdminIpNotAllowedException.php',
897898
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php',
898899
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php',
899900
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\ExAppRequiredException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/ExAppRequiredException.php',
@@ -1811,6 +1812,7 @@
18111812
'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => $baseDir . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php',
18121813
'OC\\Security\\RateLimiting\\Limiter' => $baseDir . '/lib/private/Security/RateLimiting/Limiter.php',
18131814
'OC\\Security\\RemoteHostValidator' => $baseDir . '/lib/private/Security/RemoteHostValidator.php',
1815+
'OC\\Security\\RemoteIpAddress' => $baseDir . '/lib/private/Security/RemoteIpAddress.php',
18141816
'OC\\Security\\SecureRandom' => $baseDir . '/lib/private/Security/SecureRandom.php',
18151817
'OC\\Security\\TrustedDomainHelper' => $baseDir . '/lib/private/Security/TrustedDomainHelper.php',
18161818
'OC\\Security\\VerificationToken\\CleanUpJob' => $baseDir . '/lib/private/Security/VerificationToken/CleanUpJob.php',

lib/composer/composer/autoload_static.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,6 +927,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
927927
'OC\\AppFramework\\Middleware\\Security\\BruteForceMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php',
928928
'OC\\AppFramework\\Middleware\\Security\\CORSMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php',
929929
'OC\\AppFramework\\Middleware\\Security\\CSPMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php',
930+
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AdminIpNotAllowedException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/AdminIpNotAllowedException.php',
930931
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php',
931932
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php',
932933
'OC\\AppFramework\\Middleware\\Security\\Exceptions\\ExAppRequiredException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/ExAppRequiredException.php',
@@ -1844,6 +1845,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
18441845
'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php',
18451846
'OC\\Security\\RateLimiting\\Limiter' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Limiter.php',
18461847
'OC\\Security\\RemoteHostValidator' => __DIR__ . '/../../..' . '/lib/private/Security/RemoteHostValidator.php',
1848+
'OC\\Security\\RemoteIpAddress' => __DIR__ . '/../../..' . '/lib/private/Security/RemoteIpAddress.php',
18471849
'OC\\Security\\SecureRandom' => __DIR__ . '/../../..' . '/lib/private/Security/SecureRandom.php',
18481850
'OC\\Security\\TrustedDomainHelper' => __DIR__ . '/../../..' . '/lib/private/Security/TrustedDomainHelper.php',
18491851
'OC\\Security\\VerificationToken\\CleanUpJob' => __DIR__ . '/../../..' . '/lib/private/Security/VerificationToken/CleanUpJob.php',

lib/private/AppFramework/DependencyInjection/DIContainer.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use OC\Core\Middleware\TwoFactorMiddleware;
2222
use OC\Diagnostics\EventLogger;
2323
use OC\Log\PsrLoggerAdapter;
24+
use OC\Security\RemoteIpAddress;
2425
use OC\ServerContainer;
2526
use OC\Settings\AuthorizedGroupMapper;
2627
use OCA\WorkflowEngine\Manager;
@@ -231,7 +232,8 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta
231232
$server->getAppManager(),
232233
$server->getL10N('lib'),
233234
$c->get(AuthorizedGroupMapper::class),
234-
$server->get(IUserSession::class)
235+
$server->get(IUserSession::class),
236+
$c->get(RemoteIpAddress::class),
235237
);
236238
$dispatcher->registerMiddleware($securityMiddleware);
237239
$dispatcher->registerMiddleware(
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-only
7+
*/
8+
namespace OC\AppFramework\Middleware\Security\Exceptions;
9+
10+
use OCP\AppFramework\Http;
11+
12+
/**
13+
* Class AdminIpNotAllowed is thrown when a resource has been requested by a
14+
* an admin user connecting from an unauthorized IP address
15+
* See configuration `allowed_admin_ranges`
16+
*
17+
* @package OC\AppFramework\Middleware\Security\Exceptions
18+
*/
19+
class AdminIpNotAllowedException extends SecurityException {
20+
public function __construct(string $message) {
21+
parent::__construct($message, Http::STATUS_FORBIDDEN);
22+
}
23+
}

0 commit comments

Comments
 (0)