From d001eb3a3abc477eb1f558e6f26fe977ff437c08 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Tue, 20 Oct 2026 19:03:08 -0400 Subject: [PATCH] feat(security): let apps refuse requests early with request guards Apps can register an IRequestGuard. Guards are asked for every web request after app registration and before the session is started, so a refusal covers every entry point (index.php, remote.php, ocs, public.php, status.php). A refused request gets a 403 with Retry-After and a short reference the user can give to an administrator. A guard that throws is logged and ignored. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Frank Karlitschek --- lib/OC.php | 25 ++++ lib/composer/composer/autoload_classmap.php | 3 + lib/composer/composer/autoload_static.php | 3 + .../Bootstrap/RegistrationContext.php | 26 ++++ .../Security/Guard/RequestGuardManager.php | 53 ++++++++ .../Bootstrap/IRegistrationContext.php | 9 ++ lib/public/Security/Guard/GuardDecision.php | 46 +++++++ lib/public/Security/Guard/IRequestGuard.php | 30 +++++ .../Guard/RequestGuardManagerTest.php | 120 ++++++++++++++++++ 9 files changed, 315 insertions(+) create mode 100644 lib/private/Security/Guard/RequestGuardManager.php create mode 100644 lib/public/Security/Guard/GuardDecision.php create mode 100644 lib/public/Security/Guard/IRequestGuard.php create mode 100644 tests/lib/Security/Guard/RequestGuardManagerTest.php diff --git a/lib/OC.php b/lib/OC.php index 725e945219d47..b7234f68996cb 100644 --- a/lib/OC.php +++ b/lib/OC.php @@ -12,6 +12,7 @@ use OC\NavigationManager; use OC\Profiler\BuiltInProfiler; use OC\Security\CSP\ContentSecurityPolicyNonceManager; +use OC\Security\Guard\RequestGuardManager; use OC\Share20\GroupDeletedListener; use OC\Share20\Hooks; use OC\Share20\UserDeletedListener; @@ -420,6 +421,26 @@ private static function getProductName(): string { return $productName; } + /** + * Lets apps refuse a request before the session is started, + * e.g. one from a banned address. + */ + private static function checkRequestGuards(): void { + $decision = Server::get(RequestGuardManager::class)->check(Server::get(IRequest::class)); + if ($decision === null) { + return; + } + + http_response_code(403); + header('Retry-After: ' . $decision->getRetryAfter()); + header('Content-Type: text/plain; charset=utf-8'); + echo Server::get(\OCP\L10N\IFactory::class)->get('lib')->t( + 'Access from your network is temporarily blocked. If you think this is a mistake, contact your administrator and give them this reference: %s', + [$decision->getReference()] + ); + exit(); + } + public static function initSession(): void { $request = Server::get(IRequest::class); @@ -833,6 +854,10 @@ public static function initForRequest(): void { $bootstrapCoordinator = Server::get(\OC\AppFramework\Bootstrap\Coordinator::class); $bootstrapCoordinator->runInitialRegistration(); + if (!self::$CLI) { + self::checkRequestGuards(); + } + $eventLogger->start('init_session', 'Initialize session'); $systemConfig = Server::get(\OC\SystemConfig::class); diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 4467e4e6179c2..8298ea9f24e21 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -885,6 +885,8 @@ 'OCP\\Security\\Events\\GenerateSecurePasswordEvent' => $baseDir . '/lib/public/Security/Events/GenerateSecurePasswordEvent.php', 'OCP\\Security\\Events\\ValidatePasswordPolicyEvent' => $baseDir . '/lib/public/Security/Events/ValidatePasswordPolicyEvent.php', 'OCP\\Security\\FeaturePolicy\\AddFeaturePolicyEvent' => $baseDir . '/lib/public/Security/FeaturePolicy/AddFeaturePolicyEvent.php', + 'OCP\\Security\\Guard\\GuardDecision' => $baseDir . '/lib/public/Security/Guard/GuardDecision.php', + 'OCP\\Security\\Guard\\IRequestGuard' => $baseDir . '/lib/public/Security/Guard/IRequestGuard.php', 'OCP\\Security\\IContentSecurityPolicyManager' => $baseDir . '/lib/public/Security/IContentSecurityPolicyManager.php', 'OCP\\Security\\ICredentialsManager' => $baseDir . '/lib/public/Security/ICredentialsManager.php', 'OCP\\Security\\ICrypto' => $baseDir . '/lib/public/Security/ICrypto.php', @@ -2319,6 +2321,7 @@ 'OC\\Security\\Crypto' => $baseDir . '/lib/private/Security/Crypto.php', 'OC\\Security\\FeaturePolicy\\FeaturePolicy' => $baseDir . '/lib/private/Security/FeaturePolicy/FeaturePolicy.php', 'OC\\Security\\FeaturePolicy\\FeaturePolicyManager' => $baseDir . '/lib/private/Security/FeaturePolicy/FeaturePolicyManager.php', + 'OC\\Security\\Guard\\RequestGuardManager' => $baseDir . '/lib/private/Security/Guard/RequestGuardManager.php', 'OC\\Security\\Hasher' => $baseDir . '/lib/private/Security/Hasher.php', 'OC\\Security\\IdentityProof\\Key' => $baseDir . '/lib/private/Security/IdentityProof/Key.php', 'OC\\Security\\IdentityProof\\Manager' => $baseDir . '/lib/private/Security/IdentityProof/Manager.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index dac59af04087a..7f93516122471 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -926,6 +926,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\Security\\Events\\GenerateSecurePasswordEvent' => __DIR__ . '/../../..' . '/lib/public/Security/Events/GenerateSecurePasswordEvent.php', 'OCP\\Security\\Events\\ValidatePasswordPolicyEvent' => __DIR__ . '/../../..' . '/lib/public/Security/Events/ValidatePasswordPolicyEvent.php', 'OCP\\Security\\FeaturePolicy\\AddFeaturePolicyEvent' => __DIR__ . '/../../..' . '/lib/public/Security/FeaturePolicy/AddFeaturePolicyEvent.php', + 'OCP\\Security\\Guard\\GuardDecision' => __DIR__ . '/../../..' . '/lib/public/Security/Guard/GuardDecision.php', + 'OCP\\Security\\Guard\\IRequestGuard' => __DIR__ . '/../../..' . '/lib/public/Security/Guard/IRequestGuard.php', 'OCP\\Security\\IContentSecurityPolicyManager' => __DIR__ . '/../../..' . '/lib/public/Security/IContentSecurityPolicyManager.php', 'OCP\\Security\\ICredentialsManager' => __DIR__ . '/../../..' . '/lib/public/Security/ICredentialsManager.php', 'OCP\\Security\\ICrypto' => __DIR__ . '/../../..' . '/lib/public/Security/ICrypto.php', @@ -2360,6 +2362,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Security\\Crypto' => __DIR__ . '/../../..' . '/lib/private/Security/Crypto.php', 'OC\\Security\\FeaturePolicy\\FeaturePolicy' => __DIR__ . '/../../..' . '/lib/private/Security/FeaturePolicy/FeaturePolicy.php', 'OC\\Security\\FeaturePolicy\\FeaturePolicyManager' => __DIR__ . '/../../..' . '/lib/private/Security/FeaturePolicy/FeaturePolicyManager.php', + 'OC\\Security\\Guard\\RequestGuardManager' => __DIR__ . '/../../..' . '/lib/private/Security/Guard/RequestGuardManager.php', 'OC\\Security\\Hasher' => __DIR__ . '/../../..' . '/lib/private/Security/Hasher.php', 'OC\\Security\\IdentityProof\\Key' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Key.php', 'OC\\Security\\IdentityProof\\Manager' => __DIR__ . '/../../..' . '/lib/private/Security/IdentityProof/Manager.php', diff --git a/lib/private/AppFramework/Bootstrap/RegistrationContext.php b/lib/private/AppFramework/Bootstrap/RegistrationContext.php index f86890441243b..f827df4cb8da8 100644 --- a/lib/private/AppFramework/Bootstrap/RegistrationContext.php +++ b/lib/private/AppFramework/Bootstrap/RegistrationContext.php @@ -36,6 +36,7 @@ use OCP\Notification\INotifier; use OCP\Profile\ILinkAction; use OCP\Search\IProvider; +use OCP\Security\Guard\IRequestGuard; use OCP\Server; use OCP\Settings\IDeclarativeSettingsForm; use OCP\SetupCheck\ISetupCheck; @@ -151,6 +152,9 @@ class RegistrationContext { /** @var ServiceRegistration[] */ private array $systemReportSections = []; + /** @var ServiceRegistration[] */ + private array $requestGuards = []; + /** @var PreviewProviderRegistration[] */ private array $previewProviders = []; @@ -471,6 +475,14 @@ public function registerSystemReportSection(string $sectionClass): void { ); } + #[\Override] + public function registerRequestGuard(string $guardClass): void { + $this->context->registerRequestGuard( + $this->appId, + $guardClass + ); + } + #[\Override] public function registerDeclarativeSettings(string $declarativeSettingsClass): void { $this->context->registerDeclarativeSettings( @@ -712,6 +724,13 @@ public function registerSystemReportSection(string $appId, string $sectionClass) $this->systemReportSections[] = new ServiceRegistration($appId, $sectionClass); } + /** + * @psalm-param class-string $guardClass + */ + public function registerRequestGuard(string $appId, string $guardClass): void { + $this->requestGuards[] = new ServiceRegistration($appId, $guardClass); + } + /** * @psalm-param class-string $declarativeSettingsClass */ @@ -1110,6 +1129,13 @@ public function getSystemReportSections(): array { return $this->systemReportSections; } + /** + * @return ServiceRegistration[] + */ + public function getRequestGuards(): array { + return $this->requestGuards; + } + /** * @return ServiceRegistration[] */ diff --git a/lib/private/Security/Guard/RequestGuardManager.php b/lib/private/Security/Guard/RequestGuardManager.php new file mode 100644 index 0000000000000..13f465b6ae8d7 --- /dev/null +++ b/lib/private/Security/Guard/RequestGuardManager.php @@ -0,0 +1,53 @@ +coordinator->getRegistrationContext()?->getRequestGuards() ?? []; + foreach ($registrations as $registration) { + $class = $registration->getService(); + try { + /** @var IRequestGuard $guard */ + $guard = $this->container->get($class); + $decision = $guard->check($request); + } catch (\Throwable $t) { + $this->logger->error('Exception in request guard ' . $class . ': ' . $t->getMessage(), [ + 'app' => $registration->getAppId(), + 'exception' => $t, + ]); + continue; + } + + if ($decision !== null) { + return $decision; + } + } + + return null; + } +} diff --git a/lib/public/AppFramework/Bootstrap/IRegistrationContext.php b/lib/public/AppFramework/Bootstrap/IRegistrationContext.php index 598bbb5fdae73..2384b22bc810a 100644 --- a/lib/public/AppFramework/Bootstrap/IRegistrationContext.php +++ b/lib/public/AppFramework/Bootstrap/IRegistrationContext.php @@ -413,6 +413,15 @@ public function registerSetupCheck(string $setupCheckClass): void; */ public function registerSystemReportSection(string $sectionClass): void; + /** + * Register an implementation of \OCP\Security\Guard\IRequestGuard that + * may refuse web requests before the session is started + * + * @param class-string<\OCP\Security\Guard\IRequestGuard> $guardClass + * @since 36.0.0 + */ + public function registerRequestGuard(string $guardClass): void; + /** * Register an implementation of \OCP\Settings\IDeclarativeSettings that * will handle the implementation of declarative settings diff --git a/lib/public/Security/Guard/GuardDecision.php b/lib/public/Security/Guard/GuardDecision.php new file mode 100644 index 0000000000000..a0cf251c97d4b --- /dev/null +++ b/lib/public/Security/Guard/GuardDecision.php @@ -0,0 +1,46 @@ +retryAfter; + } + + /** + * @since 36.0.0 + */ + public function getReference(): string { + return $this->reference; + } +} diff --git a/lib/public/Security/Guard/IRequestGuard.php b/lib/public/Security/Guard/IRequestGuard.php new file mode 100644 index 0000000000000..c4e92966529fe --- /dev/null +++ b/lib/public/Security/Guard/IRequestGuard.php @@ -0,0 +1,30 @@ +manager = $this->createInstanceWithMocks(RequestGuardManager::class); + $this->request = $this->createMock(IRequest::class); + } + + /** + * @param array $guards class name => guard + */ + private function withRegisteredGuards(array $guards): void { + $registrations = []; + foreach (array_keys($guards) as $class) { + $registrations[] = new ServiceRegistration('testing', $class); + } + + $context = $this->createMock(RegistrationContext::class); + $context->method('getRequestGuards') + ->willReturn($registrations); + $this->getAutoMock(Coordinator::class)->method('getRegistrationContext') + ->willReturn($context); + $this->getAutoMock(ContainerInterface::class)->method('get') + ->willReturnCallback(static fn (string $class): IRequestGuard => $guards[$class]); + } + + private function guard(?GuardDecision $decision): IRequestGuard { + $guard = $this->createMock(IRequestGuard::class); + $guard->method('check') + ->with($this->request) + ->willReturn($decision); + return $guard; + } + + public function testNoGuardsLetTheRequestPass(): void { + $this->withRegisteredGuards([]); + + $this->assertNull($this->manager->check($this->request)); + } + + public function testNoRegistrationContextLetsTheRequestPass(): void { + $this->getAutoMock(Coordinator::class)->method('getRegistrationContext') + ->willReturn(null); + + $this->assertNull($this->manager->check($this->request)); + } + + public function testGuardsThatAllowLetTheRequestPass(): void { + $this->withRegisteredGuards([ + 'GuardA' => $this->guard(null), + 'GuardB' => $this->guard(null), + ]); + + $this->assertNull($this->manager->check($this->request)); + } + + public function testFirstRefusalWins(): void { + $first = GuardDecision::deny(3600, 'FIRST'); + $last = $this->createMock(IRequestGuard::class); + $last->expects(self::never())->method('check'); + $this->withRegisteredGuards([ + 'GuardA' => $this->guard(null), + 'GuardB' => $this->guard($first), + 'GuardC' => $last, + ]); + + $this->assertSame($first, $this->manager->check($this->request)); + } + + public function testThrowingGuardIsLoggedAndSkipped(): void { + $throwing = $this->createMock(IRequestGuard::class); + $throwing->method('check') + ->willThrowException(new \RuntimeException('boom')); + $decision = GuardDecision::deny(60, 'REF'); + $this->withRegisteredGuards([ + 'Throwing' => $throwing, + 'Refusing' => $this->guard($decision), + ]); + $this->getAutoMock(LoggerInterface::class)->expects(self::once()) + ->method('error'); + + $this->assertSame($decision, $this->manager->check($this->request)); + } + + public function testDecisionKeepsItsValues(): void { + $decision = GuardDecision::deny(14400, 'K7Q2MPXA'); + + $this->assertSame(14400, $decision->getRetryAfter()); + $this->assertSame('K7Q2MPXA', $decision->getReference()); + } + + public function testNegativeRetryAfterIsClamped(): void { + $this->assertSame(0, GuardDecision::deny(-5, 'REF')->getRetryAfter()); + } +}