diff options
author | Julius Härtl <jus@bitgrid.net> | 2022-05-20 15:12:46 +0300 |
---|---|---|
committer | Julius Härtl <jus@bitgrid.net> | 2022-06-09 13:26:51 +0300 |
commit | 483ace0245be0ac704162c01439d754273b6bc55 (patch) | |
tree | 23e08afc930515e4a0aa944cd6670988e223a721 | |
parent | 2573108127d2d56a95112224b806e6302cf7bcec (diff) |
Move CSP handling to event listener
Signed-off-by: Julius Härtl <jus@bitgrid.net>
-rw-r--r-- | lib/AppConfig.php | 12 | ||||
-rw-r--r-- | lib/AppInfo/Application.php | 85 | ||||
-rw-r--r-- | lib/Listener/CSPListener.php | 126 | ||||
-rw-r--r-- | lib/Service/FederationService.php | 26 | ||||
-rw-r--r-- | tests/lib/Listener/CSPListenerTest.php | 211 |
5 files changed, 373 insertions, 87 deletions
diff --git a/lib/AppConfig.php b/lib/AppConfig.php index 9a9bec66..e38b0cda 100644 --- a/lib/AppConfig.php +++ b/lib/AppConfig.php @@ -15,6 +15,8 @@ use OCA\Richdocuments\AppInfo\Application; use \OCP\IConfig; class AppConfig { + public const WOPI_URL = 'wopi_url'; + public const WOPI_URL_PUBLIC = 'wopi_url_public'; public const FEDERATION_USE_TRUSTED_DOMAINS = 'federation_use_trusted_domains'; @@ -114,7 +116,7 @@ class AppConfig { /** * Returns a list of trusted domains from the gs.trustedHosts config */ - public function getTrustedDomains(): array { + public function getGlobalScaleTrustedHosts(): array { return $this->config->getSystemValue(self::SYSTEM_GS_TRUSTED_HOSTS, []); } @@ -125,4 +127,12 @@ class AppConfig { return $this->config->getAppValue(Application::APPNAME, self::FEDERATION_USE_TRUSTED_DOMAINS, 'no') === 'yes'; } + public function getCollaboraUrlPublic(): string { + return $this->config->getAppValue(Application::APPNAME, self::WOPI_URL_PUBLIC, $this->getCollaboraUrlInternal()); + } + + public function getCollaboraUrlInternal(): string { + return $this->config->getAppValue(Application::APPNAME, self::WOPI_URL, ''); + } + } diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 1df823fe..afc26c64 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -25,11 +25,9 @@ namespace OCA\Richdocuments\AppInfo; use OC\EventDispatcher\SymfonyAdapter; -use OC\Files\Type\Detection; -use OC\Security\CSP\ContentSecurityPolicy; -use OCA\Files_Sharing\Listener\LoadAdditionalListener; use OCA\Richdocuments\AppConfig; use OCA\Richdocuments\Capabilities; +use OCA\Richdocuments\Listener\CSPListener; use OCA\Richdocuments\Middleware\WOPIMiddleware; use OCA\Richdocuments\Listener\FileCreatedFromTemplateListener; use OCA\Richdocuments\PermissionManager; @@ -39,7 +37,6 @@ use OCA\Richdocuments\Preview\OOXML; use OCA\Richdocuments\Preview\OpenDocument; use OCA\Richdocuments\Preview\Pdf; use OCA\Richdocuments\Service\CapabilitiesService; -use OCA\Richdocuments\Service\FederationService; use OCA\Richdocuments\Service\InitialStateService; use OCA\Richdocuments\Template\CollaboraTemplateProvider; use OCA\Richdocuments\WOPI\DiscoveryManager; @@ -55,6 +52,7 @@ use OCP\Files\Template\TemplateFileCreator; use OCP\IConfig; use OCP\IL10N; use OCP\IPreview; +use OCP\Security\CSP\AddContentSecurityPolicyEvent; class Application extends App implements IBootstrap { @@ -62,7 +60,6 @@ class Application extends App implements IBootstrap { public function __construct(array $urlParams = array()) { parent::__construct(self::APPNAME, $urlParams); - $this->getContainer()->registerCapability(Capabilities::class); } @@ -71,6 +68,7 @@ class Application extends App implements IBootstrap { $context->registerCapability(Capabilities::class); $context->registerMiddleWare(WOPIMiddleware::class); $context->registerEventListener(FileCreatedFromTemplateEvent::class, FileCreatedFromTemplateListener::class); + $context->registerEventListener(AddContentSecurityPolicyEvent::class, CSPListener::class); } public function boot(IBootContext $context): void { @@ -163,7 +161,6 @@ class Application extends App implements IBootstrap { } $this->registerProvider(); - $this->updateCSP(); $this->checkAndEnableCODEServer(); }); } @@ -195,68 +192,6 @@ class Application extends App implements IBootstrap { }); } - public function updateCSP() { - $container = $this->getContainer(); - - // Do not apply CSP rules on WebDAV/OCS - // Ideally this could be a middleware running after the controller execution before rendering the result to only do it on page response - $scriptNameParts = explode('/', $container->getServer()->getRequest()->getScriptName()); - $scriptFile = end($scriptNameParts); - if ($scriptFile !== 'index.php') { - return; - } - - $publicWopiUrl = $container->getServer()->getConfig()->getAppValue('richdocuments', 'public_wopi_url', ''); - $publicWopiUrl = $publicWopiUrl === '' ? \OC::$server->getConfig()->getAppValue('richdocuments', 'wopi_url') : $publicWopiUrl; - $cspManager = $container->getServer()->getContentSecurityPolicyManager(); - $policy = new ContentSecurityPolicy(); - if ($publicWopiUrl !== '') { - $policy->addAllowedFrameDomain('\'self\''); - $policy->addAllowedFrameDomain($this->domainOnly($publicWopiUrl)); - $policy->addAllowedFormActionDomain($this->domainOnly($publicWopiUrl)); - } - - /** - * Dynamically add CSP for federated editing - */ - if ($container->getServer()->getAppManager()->isEnabledForUser('federation')) { - /** @var FederationService $federationService */ - $federationService = \OC::$server->query(FederationService::class); - - // Always add trusted servers on global scale - /** @var \OCP\GlobalScale\IConfig $globalScale */ - $globalScale = $container->query(\OCP\GlobalScale\IConfig::class); - if ($globalScale->isGlobalScaleEnabled()) { - $trustedList = \OC::$server->getConfig()->getSystemValue('gs.trustedHosts', []); - foreach ($trustedList as $server) { - $policy->addAllowedFrameDomain($server); - $this->addTrustedRemote($policy, $server); - $policy->addAllowedFormActionDomain($server); - } - } - $remoteAccess = $container->getServer()->getRequest()->getParam('richdocuments_remote_access'); - - if ($remoteAccess && $federationService->isTrustedRemote($remoteAccess)) { - $this->addTrustedRemote($policy, $remoteAccess); - } - } - - $cspManager->addDefaultPolicy($policy); - } - - private function addTrustedRemote($policy, $url) { - $federationService = \OC::$server->get(FederationService::class); - try { - $remoteCollabora = $federationService->getRemoteCollaboraURL($url); - $policy->addAllowedFrameDomain($url); - $policy->addAllowedFrameDomain($remoteCollabora); - } catch (\Exception $e) { - // We can ignore this exception for adding predefined domains to the CSP as it it would then just - // reload the page to set a proper allowed frame domain if we don't have a fixed list of trusted - // remotes in a global scale scenario - } - } - public function checkAndEnableCODEServer() { // Supported only on Linux OS, and x86_64 & ARM64 platforms $supportedArchs = array('x86_64', 'aarch64'); @@ -297,18 +232,4 @@ class Application extends App implements IBootstrap { $capabilitiesService->refetch(); } } - - /** - * Strips the path and query parameters from the URL. - * - * @param string $url - * @return string - */ - private function domainOnly($url) { - $parsed_url = parse_url($url); - $scheme = isset($parsed_url['scheme']) ? $parsed_url['scheme'] . '://' : ''; - $host = isset($parsed_url['host']) ? $parsed_url['host'] : ''; - $port = isset($parsed_url['port']) ? ':' . $parsed_url['port'] : ''; - return "$scheme$host$port"; - } } diff --git a/lib/Listener/CSPListener.php b/lib/Listener/CSPListener.php new file mode 100644 index 00000000..33098ef8 --- /dev/null +++ b/lib/Listener/CSPListener.php @@ -0,0 +1,126 @@ +<?php + +declare(strict_types=1); +/** + * @copyright Copyright (c) 2022 Julius Härtl <jus@bitgrid.net> + * + * @author Julius Härtl <jus@bitgrid.net> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ +namespace OCA\Richdocuments\Listener; + +use OCA\Richdocuments\AppConfig; +use OCA\Richdocuments\Service\FederationService; +use OCP\App\IAppManager; +use OCP\AppFramework\Http\EmptyContentSecurityPolicy; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\GlobalScale\IConfig as GlobalScaleConfig; +use OCP\IRequest; +use OCP\Security\CSP\AddContentSecurityPolicyEvent; + +class CSPListener implements IEventListener { + private IRequest $request; + private AppConfig $config; + private IAppManager $appManager; + private FederationService $federationService; + private GlobalScaleConfig $globalScaleConfig; + + public function __construct(IRequest $request, AppConfig $config, IAppManager $appManager, FederationService $federationService, GlobalScaleConfig $globalScaleConfig) { + $this->request = $request; + $this->config = $config; + $this->appManager = $appManager; + $this->federationService = $federationService; + $this->globalScaleConfig = $globalScaleConfig; + } + + public function handle(Event $event): void { + if (!$event instanceof AddContentSecurityPolicyEvent) { + return; + } + + if (!$this->isPageLoad()) { + return; + } + + $urls = array_merge( + [ $this->domainOnly($this->config->getCollaboraUrlPublic()) ], + $this->getFederationDomains(), + $this->getGSDomains() + ); + + $urls = array_filter($urls); + + $policy = new EmptyContentSecurityPolicy(); + foreach ($urls as $url) { + $policy->addAllowedFrameDomain($url); + $policy->addAllowedFormActionDomain($url); + $policy->addAllowedFrameAncestorDomain($url); + $policy->addAllowedImageDomain($url); + } + + $event->addPolicy($policy); + } + + private function isPageLoad(): bool { + $scriptNameParts = explode('/', $this->request->getScriptName()); + return end($scriptNameParts) === 'index.php'; + } + + private function getFederationDomains(): array { + if (!$this->appManager->isEnabledForUser('federation')) { + return []; + } + + $trustedNextcloudDomains = array_filter(array_map(function ($server) { + return $this->federationService->isTrustedRemote($server) ? $server : null; + }, $this->federationService->getTrustedServers())); + + $trustedCollaboraDomains = array_filter(array_map(function ($server) { + try { + return $this->federationService->getRemoteCollaboraURL($server); + } catch (\Exception $e) { + // If there is no remote collabora server we can just skip that + return null; + } + }, $trustedNextcloudDomains)); + + return array_map(function ($url) { + return $this->domainOnly($url); + }, array_merge($trustedNextcloudDomains, $trustedCollaboraDomains)); + } + + private function getGSDomains(): array { + if (!$this->globalScaleConfig->isGlobalScaleEnabled()) { + return []; + } + + return $this->config->getGlobalScaleTrustedHosts(); + } + + /** + * Strips the path and query parameters from the URL. + */ + private function domainOnly(string $url): string { + $parsedUrl = parse_url($url); + $scheme = isset($parsedUrl['scheme']) ? $parsedUrl['scheme'] . '://' : ''; + $host = $parsedUrl['host'] ?? ''; + $port = isset($parsedUrl['port']) ? ':' . $parsedUrl['port'] : ''; + return "$scheme$host$port"; + } +} diff --git a/lib/Service/FederationService.php b/lib/Service/FederationService.php index ac541f54..5f6cb464 100644 --- a/lib/Service/FederationService.php +++ b/lib/Service/FederationService.php @@ -1,4 +1,6 @@ <?php +declare(strict_types=1); + /** * @copyright Copyright (c) 2019 Julius Härtl <jus@bitgrid.net> * @@ -30,7 +32,7 @@ use OCA\Richdocuments\AppConfig; use OCA\Richdocuments\Db\Direct; use OCA\Richdocuments\Db\Wopi; use OCA\Richdocuments\TokenManager; -use OCP\AppFramework\QueryException; +use OCP\AutoloadNotAllowedException; use OCP\Files\File; use OCP\Files\InvalidPathException; use OCP\Files\NotFoundException; @@ -41,6 +43,8 @@ use OCP\ILogger; use OCP\IRequest; use OCP\IURLGenerator; use OCP\Share\IShare; +use Psr\Container\ContainerExceptionInterface; +use Psr\Container\NotFoundExceptionInterface; class FederationService { @@ -70,8 +74,22 @@ class FederationService { $this->request = $request; $this->urlGenerator = $urlGenerator; try { - $this->trustedServers = \OC::$server->query( \OCA\Federation\TrustedServers::class); - } catch (QueryException $e) {} + $this->trustedServers = \OC::$server->get(\OCA\Federation\TrustedServers::class); + } catch (NotFoundExceptionInterface $e) { + } catch (ContainerExceptionInterface $e) { + } catch (AutoloadNotAllowedException $e) { + } + + } + + public function getTrustedServers(): array { + if (!$this->trustedServers) { + return []; + } + + return array_map(function(array $server) { + return $server['url']; + }, $this->trustedServers->getServers()); } /** @@ -118,7 +136,7 @@ class FederationService { $domain = $this->getDomainWithoutPort($domainWithPort); - $trustedList = array_merge($this->appConfig->getTrustedDomains(), [$this->request->getServerHost()]); + $trustedList = array_merge($this->appConfig->getGlobalScaleTrustedHosts(), [$this->request->getServerHost()]); if (!is_array($trustedList)) { return false; } diff --git a/tests/lib/Listener/CSPListenerTest.php b/tests/lib/Listener/CSPListenerTest.php new file mode 100644 index 00000000..32d205e2 --- /dev/null +++ b/tests/lib/Listener/CSPListenerTest.php @@ -0,0 +1,211 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2022 Julius Härtl <jus@bitgrid.net> + * + * @author Julius Härtl <jus@bitgrid.net> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + * + */ + +use OC\Security\CSP\ContentSecurityPolicyManager; +use OCA\Richdocuments\AppConfig; +use OCA\Richdocuments\Listener\CSPListener; +use OCA\Richdocuments\Service\FederationService; +use OCP\App\IAppManager; +use OCP\AppFramework\Http\ContentSecurityPolicy; +use OCP\AppFramework\Http\EmptyContentSecurityPolicy; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\GlobalScale\IConfig as GlobalScaleConfig; +use OCP\IRequest; +use OCP\Security\CSP\AddContentSecurityPolicyEvent; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + +class CSPListenerTest extends TestCase { + + /** @var IRequest|MockObject */ + private $request; + /** @var AppConfig|MockObject */ + private $config; + /** @var IAppManager|MockObject */ + private $appManager; + /** @var GlobalScaleConfig|MockObject */ + private $gsConfig; + /** @var FederationService|MockObject */ + private $federationService; + private CSPListener $listener; + + public function setUp(): void { + parent::setUp(); + + $this->request = $this->createMock(IRequest::class); + $this->config = $this->createMock(AppConfig::class); + $this->appManager = $this->createMock(IAppManager::class); + $this->gsConfig = $this->createMock(GlobalScaleConfig::class); + $this->federationService = $this->createMock(FederationService::class); + + $this->listener = new CSPListener( + $this->request, + $this->config, + $this->appManager, + $this->federationService, + $this->gsConfig + ); + } + + private function getMergedPolicy(): ContentSecurityPolicy { + $eventDispatcher = $this->createMock(IEventDispatcher::class); + $eventDispatcher->expects(self::once()) + ->method('dispatchTyped') + ->willReturnCallback(function ($event) { + $this->listener->handle($event); + }); + $manager = new ContentSecurityPolicyManager($eventDispatcher); + + return $manager->getDefaultPolicy(); + } + + private function expectPageLoad(): void { + $this->request->expects(self::once()) + ->method('getScriptName') + ->willReturn('index.php'); + } + + public function testHandle() { + $this->expectPageLoad(); + $this->config->expects(self::any()) + ->method('getCollaboraUrlPublic') + ->willReturn('http://public'); + + $policy = $this->getMergedPolicy(); + + self::assertEquals(["http://public"], $policy->getAllowedFrameDomains()); + self::assertEquals(["'self'", "http://public"], $policy->getAllowedFormActionDomains()); + } + + public function testHandleRemote() { + $manager = $this->createMock(ContentSecurityPolicyManager::class); + $event = new AddContentSecurityPolicyEvent( + $manager + ); + + $this->request->expects(self::once()) + ->method('getScriptName') + ->willReturn('remote.php'); + + $manager->expects(self::never()) + ->method('addDefaultPolicy'); + $this->listener->handle($event); + } + + public function testNotSetup() { + $this->expectPageLoad(); + $this->config->expects(self::any()) + ->method('getCollaboraUrlPublic') + ->willReturn(''); + + $policy = $this->getMergedPolicy(); + + self::assertEquals([], $policy->getAllowedFrameDomains()); + self::assertEquals(["'self'"], $policy->getAllowedFormActionDomains()); + } + + public function testWopiUrlPublic() { + $this->expectPageLoad(); + $this->config->expects(self::any()) + ->method('getCollaboraUrlPublic') + ->willReturn('http://public'); + + $policy = $this->getMergedPolicy(); + + self::assertEquals(["http://public"], $policy->getAllowedFrameDomains()); + self::assertEquals(["'self'", "http://public"], $policy->getAllowedFormActionDomains()); + } + + public function testWopiUrl() { + $this->expectPageLoad(); + $this->config->expects(self::any()) + ->method('getCollaboraUrlPublic') + ->willReturn('https://public/collabora/'); + + $policy = $this->getMergedPolicy(); + + self::assertEquals(["https://public"], $policy->getAllowedFrameDomains()); + self::assertEquals(["'self'", "https://public"], $policy->getAllowedFormActionDomains()); + } + + public function testGS() { + $this->expectPageLoad(); + $this->config->expects(self::any()) + ->method('getCollaboraUrlPublic') + ->willReturn('https://public/collabora/'); + $this->gsConfig->expects(self::any()) + ->method('isGlobalScaleEnabled') + ->willReturn(true); + $this->config->expects(self::any()) + ->method('getGlobalScaleTrustedHosts') + ->willReturn(['*.example.com']); + + $policy = $this->getMergedPolicy(); + + self::assertEquals(["https://public", "*.example.com"], $policy->getAllowedFrameDomains()); + self::assertEquals(["'self'", "https://public", "*.example.com"], $policy->getAllowedFormActionDomains()); + } + + public function testNoGS() { + $this->expectPageLoad(); + $this->config->expects(self::any()) + ->method('getCollaboraUrlPublic') + ->willReturn('http://internal'); + $this->gsConfig->expects(self::any()) + ->method('isGlobalScaleEnabled') + ->willReturn(false); + + $policy = $this->getMergedPolicy(); + + self::assertEquals(["http://internal"], $policy->getAllowedFrameDomains()); + self::assertEquals(["'self'", "http://internal"], $policy->getAllowedFormActionDomains()); + } + + public function testHandleMerged() { + $this->expectPageLoad(); + $this->config->expects(self::any()) + ->method('getCollaboraUrlPublic') + ->willReturn('http://public'); + + $eventDispatcher = $this->createMock(IEventDispatcher::class); + $eventDispatcher->expects(self::once()) + ->method('dispatchTyped') + ->willReturnCallback(function (AddContentSecurityPolicyEvent $event) { + $otherPolicy = new EmptyContentSecurityPolicy(); + $otherPolicy->addAllowedFrameDomain('external.example.com'); + $otherPolicy->addAllowedFormActionDomain('external.example.com'); + $event->addPolicy($otherPolicy); + + $this->listener->handle($event); + }); + $manager = new ContentSecurityPolicyManager($eventDispatcher); + + $policy = $manager->getDefaultPolicy(); + + self::assertEquals(["external.example.com", "http://public"], $policy->getAllowedFrameDomains()); + self::assertEquals(["'self'", "external.example.com", "http://public"], $policy->getAllowedFormActionDomains()); + } +} |