feat(security): restrict admin actions to IP ranges

Signed-off-by: Benjamin Gaussorgues <benjamin.gaussorgues@nextcloud.com>

config/config.sample.php

@@ -2195,6 +2195,16 @@
  */
 'forwarded_for_headers' => ['HTTP_X_FORWARDED', 'HTTP_FORWARDED_FOR'],
 
+/**
+ * List of trusted IP ranges for admin actions
+ *
+ * If this list is non-empty, all admin actions must be triggered from
+ * IP addresses inside theses ranges.
+ *
+ * Defaults to an empty array.
+ */
+'allowed_admin_ranges' => ['192.0.2.42/32', '233.252.0.0/24', '2001:db8::13:37/64'],
+
 /**
  * max file size for animating gifs on public-sharing-site.
  * If the gif is bigger, it'll show a static preview

lib/composer/composer/autoload_classmap.php

@@ -893,6 +893,7 @@
     'OC\\AppFramework\\Middleware\\Security\\BruteForceMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php',
     'OC\\AppFramework\\Middleware\\Security\\CORSMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php',
     'OC\\AppFramework\\Middleware\\Security\\CSPMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php',
+    'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AdminIpNotAllowedException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/AdminIpNotAllowedException.php',
     'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php',
     'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php',
     'OC\\AppFramework\\Middleware\\Security\\Exceptions\\ExAppRequiredException' => $baseDir . '/lib/private/AppFramework/Middleware/Security/Exceptions/ExAppRequiredException.php',
@@ -1808,6 +1809,7 @@
     'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => $baseDir . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php',
     'OC\\Security\\RateLimiting\\Limiter' => $baseDir . '/lib/private/Security/RateLimiting/Limiter.php',
     'OC\\Security\\RemoteHostValidator' => $baseDir . '/lib/private/Security/RemoteHostValidator.php',
+    'OC\\Security\\RemoteIpAddress' => $baseDir . '/lib/private/Security/RemoteIpAddress.php',
     'OC\\Security\\SecureRandom' => $baseDir . '/lib/private/Security/SecureRandom.php',
     'OC\\Security\\TrustedDomainHelper' => $baseDir . '/lib/private/Security/TrustedDomainHelper.php',
     'OC\\Security\\VerificationToken\\CleanUpJob' => $baseDir . '/lib/private/Security/VerificationToken/CleanUpJob.php',

lib/composer/composer/autoload_static.php

@@ -926,6 +926,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\AppFramework\\Middleware\\Security\\BruteForceMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/BruteForceMiddleware.php',
         'OC\\AppFramework\\Middleware\\Security\\CORSMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php',
         'OC\\AppFramework\\Middleware\\Security\\CSPMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php',
+        'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AdminIpNotAllowedException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/AdminIpNotAllowedException.php',
         'OC\\AppFramework\\Middleware\\Security\\Exceptions\\AppNotEnabledException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/AppNotEnabledException.php',
         'OC\\AppFramework\\Middleware\\Security\\Exceptions\\CrossSiteRequestForgeryException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/CrossSiteRequestForgeryException.php',
         'OC\\AppFramework\\Middleware\\Security\\Exceptions\\ExAppRequiredException' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/Security/Exceptions/ExAppRequiredException.php',
@@ -1841,6 +1842,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Security\\RateLimiting\\Exception\\RateLimitExceededException' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Exception/RateLimitExceededException.php',
         'OC\\Security\\RateLimiting\\Limiter' => __DIR__ . '/../../..' . '/lib/private/Security/RateLimiting/Limiter.php',
         'OC\\Security\\RemoteHostValidator' => __DIR__ . '/../../..' . '/lib/private/Security/RemoteHostValidator.php',
+        'OC\\Security\\RemoteIpAddress' => __DIR__ . '/../../..' . '/lib/private/Security/RemoteIpAddress.php',
         'OC\\Security\\SecureRandom' => __DIR__ . '/../../..' . '/lib/private/Security/SecureRandom.php',
         'OC\\Security\\TrustedDomainHelper' => __DIR__ . '/../../..' . '/lib/private/Security/TrustedDomainHelper.php',
         'OC\\Security\\VerificationToken\\CleanUpJob' => __DIR__ . '/../../..' . '/lib/private/Security/VerificationToken/CleanUpJob.php',

lib/private/AppFramework/DependencyInjection/DIContainer.php

@@ -21,6 +21,7 @@
 use OC\Core\Middleware\TwoFactorMiddleware;
 use OC\Diagnostics\EventLogger;
 use OC\Log\PsrLoggerAdapter;
+use OC\Security\RemoteIpAddress;
 use OC\ServerContainer;
 use OC\Settings\AuthorizedGroupMapper;
 use OCA\WorkflowEngine\Manager;
@@ -231,7 +232,8 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta
 				$server->getAppManager(),
 				$server->getL10N('lib'),
 				$c->get(AuthorizedGroupMapper::class),
-				$server->get(IUserSession::class)
+				$server->get(IUserSession::class),
+				$c->get(RemoteIpAddress::class),
 			);
 			$dispatcher->registerMiddleware($securityMiddleware);
 			$dispatcher->registerMiddleware(

lib/private/AppFramework/Middleware/Security/Exceptions/AdminIpNotAllowedException.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+namespace OC\AppFramework\Middleware\Security\Exceptions;
+
+use OCP\AppFramework\Http;
+
+/**
+ * Class AdminIpNotAllowed is thrown when a resource has been requested by a
+ * an admin user connecting from an unauthorized IP address
+ * See configuration `allowed_admin_ranges`
+ *
+ * @package OC\AppFramework\Middleware\Security\Exceptions
+ */
+class AdminIpNotAllowedException extends SecurityException {
+	public function __construct(string $message) {
+		parent::__construct($message, Http::STATUS_FORBIDDEN);
+	}
+}

lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php

@@ -8,6 +8,7 @@
  */
 namespace OC\AppFramework\Middleware\Security;
 
+use OC\AppFramework\Middleware\Security\Exceptions\AdminIpNotAllowedException;
 use OC\AppFramework\Middleware\Security\Exceptions\AppNotEnabledException;
 use OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException;
 use OC\AppFramework\Middleware\Security\Exceptions\ExAppRequiredException;
@@ -16,6 +17,7 @@
 use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
 use OC\AppFramework\Middleware\Security\Exceptions\StrictCookieMissingException;
 use OC\AppFramework\Utility\ControllerMethodReflector;
+use OC\Security\RemoteIpAddress;
 use OC\Settings\AuthorizedGroupMapper;
 use OC\User\Session;
 use OCP\App\AppPathNotFoundException;
@@ -50,60 +52,22 @@
  * check fails
  */
 class SecurityMiddleware extends Middleware {
-	/** @var INavigationManager */
-	private $navigationManager;
-	/** @var IRequest */
-	private $request;
-	/** @var ControllerMethodReflector */
-	private $reflector;
-	/** @var string */
-	private $appName;
-	/** @var IURLGenerator */
-	private $urlGenerator;
-	/** @var LoggerInterface */
-	private $logger;
-	/** @var bool */
-	private $isLoggedIn;
-	/** @var bool */
-	private $isAdminUser;
-	/** @var bool */
-	private $isSubAdmin;
-	/** @var IAppManager */
-	private $appManager;
-	/** @var IL10N */
-	private $l10n;
-	/** @var AuthorizedGroupMapper */
-	private $groupAuthorizationMapper;
-	/** @var IUserSession */
-	private $userSession;
-
-	public function __construct(IRequest $request,
-		ControllerMethodReflector $reflector,
-		INavigationManager $navigationManager,
-		IURLGenerator $urlGenerator,
-		LoggerInterface $logger,
-		string $appName,
-		bool $isLoggedIn,
-		bool $isAdminUser,
-		bool $isSubAdmin,
-		IAppManager $appManager,
-		IL10N $l10n,
-		AuthorizedGroupMapper $mapper,
-		IUserSession $userSession
+	public function __construct(
+		private IRequest $request,
+		private ControllerMethodReflector $reflector,
+		private INavigationManager $navigationManager,
+		private IURLGenerator $urlGenerator,
+		private LoggerInterface $logger,
+		private string $appName,
+		private bool $isLoggedIn,
+		private bool $isAdminUser,
+		private bool $isSubAdmin,
+		private IAppManager $appManager,
+		private IL10N $l10n,
+		private AuthorizedGroupMapper $mapper,
+		private IUserSession $userSession,
+		private RemoteIpAddress $remoteIpAddress,
 	) {
-		$this->navigationManager = $navigationManager;
-		$this->request = $request;
-		$this->reflector = $reflector;
-		$this->appName = $appName;
-		$this->urlGenerator = $urlGenerator;
-		$this->logger = $logger;
-		$this->isLoggedIn = $isLoggedIn;
-		$this->isAdminUser = $isAdminUser;
-		$this->isSubAdmin = $isSubAdmin;
-		$this->appManager = $appManager;
-		$this->l10n = $l10n;
-		$this->groupAuthorizationMapper = $mapper;
-		$this->userSession = $userSession;
 	}
 
 	/**
@@ -161,6 +125,9 @@ public function beforeController($controller, $methodName) {
 				if (!$authorized) {
 					throw new NotAdminException($this->l10n->t('Logged in account must be an admin, a sub admin or gotten special right to access this setting'));
 				}
+				if (!$this->remoteIpAddress->allowsAdminActions()) {
+					throw new AdminIpNotAllowedException($this->l10n->t('Your current IP address doesn’t allow you to perform admin actions'));
+				}
 			}
 			if ($this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)
 				&& !$this->isSubAdmin
@@ -174,6 +141,16 @@ public function beforeController($controller, $methodName) {
 				&& !$authorized) {
 				throw new NotAdminException($this->l10n->t('Logged in account must be an admin'));
 			}
+			if ($this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)
+				&& !$this->remoteIpAddress->allowsAdminActions()) {
+				throw new AdminIpNotAllowedException($this->l10n->t('Your current IP address doesn’t allow you to perform admin actions'));
+			}
+			if (!$this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)
+				&& !$this->hasAnnotationOrAttribute($reflectionMethod, 'NoAdminRequired', NoAdminRequired::class)
+				&& !$this->remoteIpAddress->allowsAdminActions()) {
+				throw new AdminIpNotAllowedException($this->l10n->t('Your current IP address doesn’t allow you to perform admin actions'));
+			}
+
 		}
 
 		// Check for strict cookie requirement

lib/private/Group/Manager.php

@@ -8,6 +8,7 @@
 namespace OC\Group;
 
 use OC\Hooks\PublicEmitter;
+use OC\Security\RemoteIpAddress;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Group\Backend\IBatchMethodsBackend;
 use OCP\Group\Backend\ICreateNamedGroupBackend;
@@ -41,11 +42,6 @@ class Manager extends PublicEmitter implements IGroupManager {
 	/** @var GroupInterface[] */
 	private $backends = [];
 
-	/** @var \OC\User\Manager */
-	private $userManager;
-	private IEventDispatcher $dispatcher;
-	private LoggerInterface $logger;
-
 	/** @var array<string, IGroup> */
 	private $cachedGroups = [];
 
@@ -59,13 +55,13 @@ class Manager extends PublicEmitter implements IGroupManager {
 
 	private const MAX_GROUP_LENGTH = 255;
 
-	public function __construct(\OC\User\Manager $userManager,
-		IEventDispatcher $dispatcher,
-		LoggerInterface $logger,
-		ICacheFactory $cacheFactory) {
-		$this->userManager = $userManager;
-		$this->dispatcher = $dispatcher;
-		$this->logger = $logger;
+	public function __construct(
+		private \OC\User\Manager $userManager,
+		private IEventDispatcher $dispatcher,
+		private LoggerInterface $logger,
+		ICacheFactory $cacheFactory,
+		private RemoteIpAddress $remoteIpAddress,
+	) {
 		$this->displayNameCache = new DisplayNameCache($cacheFactory, $this);
 
 		$this->listen('\OC\Group', 'postDelete', function (IGroup $group): void {
@@ -325,6 +321,10 @@ public function getUserIdGroups(string $uid): array {
 	 * @return bool if admin
 	 */
 	public function isAdmin($userId) {
+		if (!$this->remoteIpAddress->allowsAdminActions()) {
+			return false;
+		}
+
 		foreach ($this->backends as $backend) {
 			if (is_string($userId) && $backend->implementsActions(Backend::IS_ADMIN) && $backend->isAdmin($userId)) {
 				return true;

lib/private/Security/RemoteIpAddress.php

@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\Security;
+
+use IPLib\Factory;
+use OCP\IConfig;
+use OCP\IRequest;
+use Psr\Log\LoggerInterface;
+
+/**
+ * @internal
+ */
+final class RemoteIpAddress {
+	public function __construct(
+		private IConfig $config,
+		private IRequest $request,
+		private LoggerInterface $logger,
+	) {
+	}
+
+	public function allowsAdminActions(): bool {
+		if (false === $allowedAdminRanges = $this->config->getSystemValue('allowed_admin_ranges', false)) {
+			return true;
+		}
+
+		if (!is_array($allowedAdminRanges)) {
+			$this->logger->warning('Invalid configuration for "allowed_admin_ranges"');
+			return true;
+		}
+
+		if (count($allowedAdminRanges) === 0) {
+			return true;
+		}
+
+		$ipAddress = Factory::parseAddressString($this->request->getRemoteAddress());
+		foreach ($allowedAdminRanges as $rangeString) {
+			$range = Factory::parseRangeString($rangeString);
+			if ($range->contains($ipAddress)) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+}

lib/private/Server.php

@@ -101,6 +101,7 @@
 use OC\Security\CSRF\TokenStorage\SessionStorage;
 use OC\Security\Hasher;
 use OC\Security\RateLimiting\Limiter;
+use OC\Security\RemoteIpAddress;
 use OC\Security\SecureRandom;
 use OC\Security\TrustedDomainHelper;
 use OC\Security\VerificationToken\VerificationToken;
@@ -464,7 +465,8 @@ public function __construct($webRoot, \OC\Config $config) {
 				$this->get(IUserManager::class),
 				$this->get(IEventDispatcher::class),
 				$this->get(LoggerInterface::class),
-				$this->get(ICacheFactory::class)
+				$this->get(ICacheFactory::class),
+				$this->get(RemoteIpAddress::class),
 			);
 			return $groupManager;
 		});