diff options
author | Roeland Jago Douma <roeland@famdouma.nl> | 2019-07-27 16:04:51 +0200 |
---|---|---|
committer | Roeland Jago Douma <roeland@famdouma.nl> | 2019-07-27 16:11:45 +0200 |
commit | 37a4282c7ae27c518ce7143be491a00a651e4f4a (patch) | |
tree | 47ac0e32a0e7725134a7ce57ea1babef84b24961 /lib/private/AppFramework | |
parent | 1ec98af3e028f5aa8591bda26d5dac96dfd66f43 (diff) | |
download | nextcloud-server-37a4282c7ae27c518ce7143be491a00a651e4f4a.tar.gz nextcloud-server-37a4282c7ae27c518ce7143be491a00a651e4f4a.zip |
Split up security middleware
With upcoming work for the feature policy header. Splitting this in
smaller classes that just do 1 thing makes sense.
I rather have a few small classes that are tiny and do 1 thing right
(and we all understand what is going on) than have big ones.
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Diffstat (limited to 'lib/private/AppFramework')
3 files changed, 87 insertions, 43 deletions
diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 6d337bb9327..f47af340b38 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -220,14 +220,18 @@ class DIContainer extends SimpleContainer implements IAppContainer { $server->getUserSession()->isLoggedIn(), $server->getGroupManager()->isAdmin($this->getUserId()), $server->getUserSession()->getUser() !== null && $server->query(ISubAdmin::class)->isSubAdmin($server->getUserSession()->getUser()), - $server->getContentSecurityPolicyManager(), - $server->getCsrfTokenManager(), - $server->getContentSecurityPolicyNonceManager(), $server->getAppManager(), $server->getL10N('lib') ); $dispatcher->registerMiddleware($securityMiddleware); $dispatcher->registerMiddleware( + new OC\AppFramework\Middleware\Security\CSPMiddleware( + $server->query(OC\Security\CSP\ContentSecurityPolicyManager::class), + $server->query(OC\Security\CSP\ContentSecurityPolicyNonceManager::class), + $server->query(OC\Security\CSRF\CsrfTokenManager::class) + ) + ); + $dispatcher->registerMiddleware( new OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware( $c->query(IControllerMethodReflector::class), $c->query(ISession::class), diff --git a/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php b/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php new file mode 100644 index 00000000000..0eaeda2b1bd --- /dev/null +++ b/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php @@ -0,0 +1,80 @@ +<?php +declare(strict_types=1); +/** + * @copyright Copyright (c) 2019, Roeland Jago Douma <roeland@famdouma.nl> + * + * @author Roeland Jago Douma <roeland@famdouma.nl> + * + * @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 OC\AppFramework\Middleware\Security; + +use OC\Security\CSP\ContentSecurityPolicyManager; +use OC\Security\CSP\ContentSecurityPolicyNonceManager; +use OC\Security\CSRF\CsrfTokenManager; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\ContentSecurityPolicy; +use OCP\AppFramework\Http\EmptyContentSecurityPolicy; +use OCP\AppFramework\Http\Response; +use OCP\AppFramework\Middleware; + +class CSPMiddleware extends Middleware { + + /** @var ContentSecurityPolicyManager */ + private $contentSecurityPolicyManager; + /** @var ContentSecurityPolicyNonceManager */ + private $cspNonceManager; + /** @var CsrfTokenManager */ + private $csrfTokenManager; + + public function __construct(ContentSecurityPolicyManager $policyManager, + ContentSecurityPolicyNonceManager $cspNonceManager, + CsrfTokenManager $csrfTokenManager) { + $this->contentSecurityPolicyManager = $policyManager; + $this->cspNonceManager = $cspNonceManager; + $this->csrfTokenManager = $csrfTokenManager; + } + + /** + * Performs the default CSP modifications that may be injected by other + * applications + * + * @param Controller $controller + * @param string $methodName + * @param Response $response + * @return Response + */ + public function afterController($controller, $methodName, Response $response): Response { + $policy = !is_null($response->getContentSecurityPolicy()) ? $response->getContentSecurityPolicy() : new ContentSecurityPolicy(); + + if (get_class($policy) === EmptyContentSecurityPolicy::class) { + return $response; + } + + $defaultPolicy = $this->contentSecurityPolicyManager->getDefaultPolicy(); + $defaultPolicy = $this->contentSecurityPolicyManager->mergePolicies($defaultPolicy, $policy); + + if($this->cspNonceManager->browserSupportsCspV3()) { + $defaultPolicy->useJsNonce($this->csrfTokenManager->getToken()->getEncryptedValue()); + } + + $response->setContentSecurityPolicy($defaultPolicy); + + return $response; + } +} diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index fef3f226e15..4f380f07d91 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -84,12 +84,6 @@ class SecurityMiddleware extends Middleware { private $isAdminUser; /** @var bool */ private $isSubAdmin; - /** @var ContentSecurityPolicyManager */ - private $contentSecurityPolicyManager; - /** @var CsrfTokenManager */ - private $csrfTokenManager; - /** @var ContentSecurityPolicyNonceManager */ - private $cspNonceManager; /** @var IAppManager */ private $appManager; /** @var IL10N */ @@ -104,9 +98,6 @@ class SecurityMiddleware extends Middleware { bool $isLoggedIn, bool $isAdminUser, bool $isSubAdmin, - ContentSecurityPolicyManager $contentSecurityPolicyManager, - CsrfTokenManager $csrfTokenManager, - ContentSecurityPolicyNonceManager $cspNonceManager, IAppManager $appManager, IL10N $l10n ) { @@ -119,9 +110,6 @@ class SecurityMiddleware extends Middleware { $this->isLoggedIn = $isLoggedIn; $this->isAdminUser = $isAdminUser; $this->isSubAdmin = $isSubAdmin; - $this->contentSecurityPolicyManager = $contentSecurityPolicyManager; - $this->csrfTokenManager = $csrfTokenManager; - $this->cspNonceManager = $cspNonceManager; $this->appManager = $appManager; $this->l10n = $l10n; } @@ -204,34 +192,6 @@ class SecurityMiddleware extends Middleware { } /** - * Performs the default CSP modifications that may be injected by other - * applications - * - * @param Controller $controller - * @param string $methodName - * @param Response $response - * @return Response - */ - public function afterController($controller, $methodName, Response $response): Response { - $policy = !is_null($response->getContentSecurityPolicy()) ? $response->getContentSecurityPolicy() : new ContentSecurityPolicy(); - - if (get_class($policy) === EmptyContentSecurityPolicy::class) { - return $response; - } - - $defaultPolicy = $this->contentSecurityPolicyManager->getDefaultPolicy(); - $defaultPolicy = $this->contentSecurityPolicyManager->mergePolicies($defaultPolicy, $policy); - - if($this->cspNonceManager->browserSupportsCspV3()) { - $defaultPolicy->useJsNonce($this->csrfTokenManager->getToken()->getEncryptedValue()); - } - - $response->setContentSecurityPolicy($defaultPolicy); - - return $response; - } - - /** * If an SecurityException is being caught, ajax requests return a JSON error * response and non ajax requests redirect to the index * @param Controller $controller the controller that is being called |