]> source.dussan.org Git - nextcloud-server.git/commitdiff
Add support for CSP nonces
authorLukas Reschke <lukas@statuscode.ch>
Mon, 24 Oct 2016 09:00:00 +0000 (10:00 +0100)
committerLukas Reschke <lukas@statuscode.ch>
Mon, 24 Oct 2016 10:27:50 +0000 (12:27 +0200)
CSP nonces are a feature available with CSP v2. Basically instead of saying "JS resources from the same domain are ok to be served" we now say "Ressources from everywhere are allowed as long as they add a `nonce` attribute to the script tag with the right nonce.

At the moment the nonce is basically just a `<?php p(base64_encode($_['requesttoken'])) ?>`, we have to decode the requesttoken since `:` is not an allowed value in the nonce. So if somebody does on their own include JS files (instead of using the `addScript` public API, they now must also include that attribute.)

IE does currently not implement CSP v2, thus there is a whitelist included that delivers the new CSP v2 policy to newer browsers. Check http://caniuse.com/#feat=contentsecuritypolicy2 for the current browser support list. An alternative approach would be to just add `'unsafe-inline'` as well as `'unsafe-inline'` is ignored by CSPv2 when a nonce is set. But this would make this security feature unusable at all in IE. Not worth it at the moment IMO.

Implementing this offers the following advantages:

1. **Security:** As we host resources from the same domain by design we don't have to worry about 'self' anymore being in the whitelist
2. **Performance:** We can move oc.js again to inline JS. This makes the loading way quicker as we don't have to load on every load of a new web page a blocking dynamically non-cached JavaScript file.

If you want to toy with CSP see also https://csp-evaluator.withgoogle.com/

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
14 files changed:
apps/theming/appinfo/app.php
core/templates/layout.base.php
core/templates/layout.guest.php
core/templates/layout.user.php
lib/private/AppFramework/DependencyInjection/DIContainer.php
lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
lib/private/Security/CSRF/CsrfToken.php
lib/private/Security/CSRF/CsrfTokenManager.php
lib/public/AppFramework/Http/ContentSecurityPolicy.php
lib/public/AppFramework/Http/EmptyContentSecurityPolicy.php
tests/lib/AppFramework/Http/EmptyContentSecurityPolicyTest.php
tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
tests/lib/Security/CSRF/CsrfTokenManagerTest.php
tests/lib/Security/CSRF/CsrfTokenTest.php

index e67092b642f2e4e80b3fe9d9f55e863a8d4babc3..03fdbc9d002fea8e54cbd3224e8fb8940dc631ad 100644 (file)
@@ -47,6 +47,7 @@ $linkToJs = \OC::$server->getURLGenerator()->linkToRoute(
        'script',
        [
                'src' => $linkToJs,
+               'nonce' => base64_encode(\OC::$server->getCsrfTokenManager()->getToken()->getEncryptedValue())
        ], ''
 );
 
index 7301ae690cc3a8f1758d67b099e8277c15565475..d6fda96dd68f95d77d77f90260f7499a3083772c 100644 (file)
@@ -19,7 +19,7 @@
                        <link rel="stylesheet" href="<?php print_unescaped($cssfile); ?>" media="print">
                <?php endforeach; ?>
                <?php foreach ($_['jsfiles'] as $jsfile): ?>
-                       <script src="<?php print_unescaped($jsfile); ?>"></script>
+                       <script src="<?php print_unescaped($jsfile); ?>" nonce="<?php p(base64_encode($_['requesttoken'])) ?>"></script>
                <?php endforeach; ?>
                <?php print_unescaped($_['headers']); ?>
        </head>
index 5850635315861e26ade0d698a55dc8356e7db8bf..a93224af5cca47272ac2979bd17123cf11c3bc4e 100644 (file)
@@ -20,7 +20,7 @@
                        <link rel="stylesheet" href="<?php print_unescaped($cssfile); ?>" media="print">
                <?php endforeach; ?>
                <?php foreach($_['jsfiles'] as $jsfile): ?>
-                       <script src="<?php print_unescaped($jsfile); ?>"></script>
+                       <script nonce="<?php p(base64_encode($_['requesttoken'])) ?>" src="<?php print_unescaped($jsfile); ?>"></script>
                <?php endforeach; ?>
                <?php print_unescaped($_['headers']); ?>
        </head>
index 285eb3ab5f33f31b6e9491c62dca88eaf1405984..d3dcd979d38601337c901acea7516b4555b8dde0 100644 (file)
@@ -27,7 +27,7 @@
                        <link rel="stylesheet" href="<?php print_unescaped($cssfile); ?>" media="print">
                <?php endforeach; ?>
                <?php foreach($_['jsfiles'] as $jsfile): ?>
-                       <script src="<?php print_unescaped($jsfile); ?>"></script>
+                       <script nonce="<?php p(base64_encode($_['requesttoken'])) ?>" src="<?php print_unescaped($jsfile); ?>"></script>
                <?php endforeach; ?>
                <?php print_unescaped($_['headers']); ?>
        </head>
index 21d5eaa9503d436fcdc3766e245a4b7413325e0f..97faa0edf49fe630999e786d72c1a6fe40ba6d72 100644 (file)
@@ -379,7 +379,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
                                $c['AppName'],
                                $app->isLoggedIn(),
                                $app->isAdminUser(),
-                               $app->getServer()->getContentSecurityPolicyManager()
+                               $app->getServer()->getContentSecurityPolicyManager(),
+                               $app->getServer()->getCsrfTokenManager()
                        );
                });
 
index 5e253d0954ac3b21a148b95224667ef3a41a59db..6c33c0023ea4f8ff062dc9bca15d1833a318b285 100644 (file)
@@ -36,6 +36,7 @@ use OC\AppFramework\Middleware\Security\Exceptions\NotLoggedInException;
 use OC\AppFramework\Middleware\Security\Exceptions\StrictCookieMissingException;
 use OC\AppFramework\Utility\ControllerMethodReflector;
 use OC\Security\CSP\ContentSecurityPolicyManager;
+use OC\Security\CSRF\CsrfTokenManager;
 use OCP\AppFramework\Http\ContentSecurityPolicy;
 use OCP\AppFramework\Http\EmptyContentSecurityPolicy;
 use OCP\AppFramework\Http\RedirectResponse;
@@ -77,6 +78,8 @@ class SecurityMiddleware extends Middleware {
        private $isAdminUser;
        /** @var ContentSecurityPolicyManager */
        private $contentSecurityPolicyManager;
+       /** @var CsrfTokenManager */
+       private $csrfTokenManager;
 
        /**
         * @param IRequest $request
@@ -88,6 +91,7 @@ class SecurityMiddleware extends Middleware {
         * @param bool $isLoggedIn
         * @param bool $isAdminUser
         * @param ContentSecurityPolicyManager $contentSecurityPolicyManager
+        * @param CSRFTokenManager $csrfTokenManager
         */
        public function __construct(IRequest $request,
                                                                ControllerMethodReflector $reflector,
@@ -97,7 +101,8 @@ class SecurityMiddleware extends Middleware {
                                                                $appName,
                                                                $isLoggedIn,
                                                                $isAdminUser,
-                                                               ContentSecurityPolicyManager $contentSecurityPolicyManager) {
+                                                               ContentSecurityPolicyManager $contentSecurityPolicyManager,
+                                                               CsrfTokenManager $csrfTokenManager) {
                $this->navigationManager = $navigationManager;
                $this->request = $request;
                $this->reflector = $reflector;
@@ -107,6 +112,7 @@ class SecurityMiddleware extends Middleware {
                $this->isLoggedIn = $isLoggedIn;
                $this->isAdminUser = $isAdminUser;
                $this->contentSecurityPolicyManager = $contentSecurityPolicyManager;
+               $this->csrfTokenManager = $csrfTokenManager;
        }
 
 
@@ -171,6 +177,23 @@ class SecurityMiddleware extends Middleware {
 
        }
 
+       private function browserSupportsCspV3() {
+               $browserWhitelist = [
+                       // Chrome 40+
+                       '/^Mozilla\/5\.0 \([^)]+\) AppleWebKit\/[0-9.]+ \(KHTML, like Gecko\) Chrome\/[4-9][0-9].[0-9.]+ (Mobile Safari|Safari)\/[0-9.]+$/',
+                       // Firefox 45+
+                       '/^Mozilla\/5\.0 \([^)]+\) Gecko\/[0-9.]+ Firefox\/(4[5-9]|[5-9][0-9])\.[0-9.]+$/',
+                       // Safari 10+
+                       '/^Mozilla\/5\.0 \([^)]+\) AppleWebKit\/[0-9.]+ \(KHTML, like Gecko\) Version\/1[0-9.]+ Safari\/[0-9.A-Z]+$/',
+               ];
+
+               if($this->request->isUserAgent($browserWhitelist)) {
+                       return true;
+               }
+
+               return false;
+       }
+
        /**
         * Performs the default CSP modifications that may be injected by other
         * applications
@@ -190,6 +213,10 @@ class SecurityMiddleware extends Middleware {
                $defaultPolicy = $this->contentSecurityPolicyManager->getDefaultPolicy();
                $defaultPolicy = $this->contentSecurityPolicyManager->mergePolicies($defaultPolicy, $policy);
 
+               if($this->browserSupportsCspV3()) {
+                       $defaultPolicy->useJsNonce($this->csrfTokenManager->getToken()->getEncryptedValue());
+               }
+
                $response->setContentSecurityPolicy($defaultPolicy);
 
                return $response;
index bf61e339f771786a1b7136ed2019849b85efba42..dce9a83b727316520a6a463592abfb0c1c88d4ac 100644 (file)
@@ -33,6 +33,8 @@ namespace OC\Security\CSRF;
 class CsrfToken {
        /** @var string */
        private $value;
+       /** @var string */
+       private $encryptedValue = '';
 
        /**
         * @param string $value Value of the token. Can be encrypted or not encrypted.
@@ -48,8 +50,12 @@ class CsrfToken {
         * @return string
         */
        public function getEncryptedValue() {
-               $sharedSecret = base64_encode(random_bytes(strlen($this->value)));
-               return base64_encode($this->value ^ $sharedSecret) .':'.$sharedSecret;
+               if($this->encryptedValue === '') {
+                       $sharedSecret = base64_encode(random_bytes(strlen($this->value)));
+                       $this->encryptedValue = base64_encode($this->value ^ $sharedSecret) . ':' . $sharedSecret;
+               }
+
+               return $this->encryptedValue;
        }
 
        /**
index d621cc2c29f66a3d33877c746cc896be02db140b..b43ca3d3679f61293fa2b76b265fa1c04d994a5b 100644 (file)
@@ -34,6 +34,8 @@ class CsrfTokenManager {
        private $tokenGenerator;
        /** @var SessionStorage */
        private $sessionStorage;
+       /** @var CsrfToken|null */
+       private $csrfToken = null;
 
        /**
         * @param CsrfTokenGenerator $tokenGenerator
@@ -51,6 +53,10 @@ class CsrfTokenManager {
         * @return CsrfToken
         */
        public function getToken() {
+               if(!is_null($this->csrfToken)) {
+                       return $this->csrfToken;
+               }
+
                if($this->sessionStorage->hasToken()) {
                        $value = $this->sessionStorage->getToken();
                } else {
@@ -58,7 +64,8 @@ class CsrfTokenManager {
                        $this->sessionStorage->setToken($value);
                }
 
-               return new CsrfToken($value);
+               $this->csrfToken = new CsrfToken($value);
+               return $this->csrfToken;
        }
 
        /**
@@ -69,13 +76,15 @@ class CsrfTokenManager {
        public function refreshToken() {
                $value = $this->tokenGenerator->generateToken();
                $this->sessionStorage->setToken($value);
-               return new CsrfToken($value);
+               $this->csrfToken = new CsrfToken($value);
+               return $this->csrfToken;
        }
 
        /**
         * Remove the current token from the storage.
         */
        public function removeToken() {
+               $this->csrfToken = null;
                $this->sessionStorage->removeToken();
        }
 
index 082aa0206c74392df0e26f4d0188361cdca372d8..17844497f94506d9e87cc27d6b89589222143971 100644 (file)
@@ -24,8 +24,6 @@
 
 namespace OCP\AppFramework\Http;
 
-use OCP\AppFramework\Http;
-
 /**
  * Class ContentSecurityPolicy is a simple helper which allows applications to
  * modify the Content-Security-Policy sent by ownCloud. Per default only JavaScript,
index 4fca1588e7fcecd3ab7c5582a81ee466eb85e890..ae4ceef192353b327cff5cf676b00e5e668c9303 100644 (file)
@@ -38,6 +38,8 @@ use OCP\AppFramework\Http;
 class EmptyContentSecurityPolicy {
        /** @var bool Whether inline JS snippets are allowed */
        protected $inlineScriptAllowed = null;
+       /** @var string Whether JS nonces should be used */
+       protected $useJsNonce = null;
        /**
         * @var bool Whether eval in JS scripts is allowed
         * TODO: Disallow per default
@@ -74,12 +76,25 @@ class EmptyContentSecurityPolicy {
         * @param bool $state
         * @return $this
         * @since 8.1.0
+        * @deprecated 10.0 CSP tokens are now used
         */
        public function allowInlineScript($state = false) {
                $this->inlineScriptAllowed = $state;
                return $this;
        }
 
+       /**
+        * Use the according JS nonce
+        *
+        * @param string $nonce
+        * @return $this
+        * @since 9.2.0
+        */
+       public function useJsNonce($nonce) {
+               $this->useJsNonce = $nonce;
+               return $this;
+       }
+
        /**
         * Whether eval in JavaScript is allowed or forbidden
         * @param bool $state
@@ -323,6 +338,15 @@ class EmptyContentSecurityPolicy {
 
                if(!empty($this->allowedScriptDomains) || $this->inlineScriptAllowed || $this->evalScriptAllowed) {
                        $policy .= 'script-src ';
+                       if(is_string($this->useJsNonce)) {
+                               $policy .= '\'nonce-'.base64_encode($this->useJsNonce).'\'';
+                               $allowedScriptDomains = array_flip($this->allowedScriptDomains);
+                               unset($allowedScriptDomains['\'self\'']);
+                               $this->allowedScriptDomains = array_flip($allowedScriptDomains);
+                               if(count($allowedScriptDomains) !== 0) {
+                                       $policy .= ' ';
+                               }
+                       }
                        if(is_array($this->allowedScriptDomains)) {
                                $policy .= implode(' ', $this->allowedScriptDomains);
                        }
index 248c3d808d214ac491b65ee049d89af930c6d481..33e2315ed8947977996fad4feff07fdf05e722a3 100644 (file)
@@ -427,4 +427,28 @@ class EmptyContentSecurityPolicyTest extends \Test\TestCase {
                $this->contentSecurityPolicy->disallowChildSrcDomain('www.owncloud.org')->disallowChildSrcDomain('www.owncloud.com');
                $this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
        }
+
+       public function testGetPolicyWithJsNonceAndScriptDomains() {
+               $expectedPolicy = "default-src 'none';script-src 'nonce-TXlKc05vbmNl' www.nextcloud.com www.nextcloud.org";
+
+               $this->contentSecurityPolicy->addAllowedScriptDomain('www.nextcloud.com');
+               $this->contentSecurityPolicy->useJsNonce('MyJsNonce');
+               $this->contentSecurityPolicy->addAllowedScriptDomain('www.nextcloud.org');
+               $this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
+       }
+
+       public function testGetPolicyWithJsNonceAndSelfScriptDomain() {
+               $expectedPolicy = "default-src 'none';script-src 'nonce-TXlKc05vbmNl'";
+
+               $this->contentSecurityPolicy->useJsNonce('MyJsNonce');
+               $this->contentSecurityPolicy->addAllowedScriptDomain("'self'");
+               $this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
+       }
+
+       public function testGetPolicyWithoutJsNonceAndSelfScriptDomain() {
+               $expectedPolicy = "default-src 'none';script-src 'self'";
+
+               $this->contentSecurityPolicy->addAllowedScriptDomain("'self'");
+               $this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
+       }
 }
index 55bf3e46e0710aceb901ea20c7bfe0d12f7fd90b..b597317fca4fd3ad88987c0856c75fc9886f4283 100644 (file)
@@ -36,6 +36,8 @@ use OC\AppFramework\Middleware\Security\SecurityMiddleware;
 use OC\AppFramework\Utility\ControllerMethodReflector;
 use OC\Security\CSP\ContentSecurityPolicy;
 use OC\Security\CSP\ContentSecurityPolicyManager;
+use OC\Security\CSRF\CsrfToken;
+use OC\Security\CSRF\CsrfTokenManager;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\EmptyContentSecurityPolicy;
 use OCP\AppFramework\Http\RedirectResponse;
@@ -72,6 +74,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
        private $urlGenerator;
        /** @var ContentSecurityPolicyManager|\PHPUnit_Framework_MockObject_MockObject */
        private $contentSecurityPolicyManager;
+       /** @var CsrfTokenManager|\PHPUnit_Framework_MockObject_MockObject */
+       private $csrfTokenManager;
 
        protected function setUp() {
                parent::setUp();
@@ -83,6 +87,7 @@ class SecurityMiddlewareTest extends \Test\TestCase {
                $this->urlGenerator = $this->createMock(IURLGenerator::class);
                $this->request = $this->createMock(IRequest::class);
                $this->contentSecurityPolicyManager = $this->createMock(ContentSecurityPolicyManager::class);
+               $this->csrfTokenManager = $this->createMock(CsrfTokenManager::class);
                $this->middleware = $this->getMiddleware(true, true);
                $this->secException = new SecurityException('hey', false);
                $this->secAjaxException = new SecurityException('hey', true);
@@ -103,7 +108,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
                        'files',
                        $isLoggedIn,
                        $isAdminUser,
-                       $this->contentSecurityPolicyManager
+                       $this->contentSecurityPolicyManager,
+                       $this->csrfTokenManager
                );
        }
 
@@ -553,6 +559,10 @@ class SecurityMiddlewareTest extends \Test\TestCase {
        }
 
        public function testAfterController() {
+               $this->request
+                       ->expects($this->once())
+                       ->method('isUserAgent')
+                       ->willReturn(false);
                $response = $this->createMock(Response::class);
                $defaultPolicy = new ContentSecurityPolicy();
                $defaultPolicy->addAllowedImageDomain('defaultpolicy');
@@ -591,4 +601,45 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 
                $this->middleware->afterController($this->controller, 'test', $response);
        }
+
+       public function testAfterControllerWithContentSecurityPolicy3Support() {
+               $this->request
+                       ->expects($this->once())
+                       ->method('isUserAgent')
+                       ->willReturn(true);
+               $token = $this->createMock(CsrfToken::class);
+               $token
+                       ->expects($this->once())
+                       ->method('getEncryptedValue')
+                       ->willReturn('MyEncryptedToken');
+               $this->csrfTokenManager
+                       ->expects($this->once())
+                       ->method('getToken')
+                       ->willReturn($token);
+               $response = $this->createMock(Response::class);
+               $defaultPolicy = new ContentSecurityPolicy();
+               $defaultPolicy->addAllowedImageDomain('defaultpolicy');
+               $currentPolicy = new ContentSecurityPolicy();
+               $currentPolicy->addAllowedConnectDomain('currentPolicy');
+               $mergedPolicy = new ContentSecurityPolicy();
+               $mergedPolicy->addAllowedMediaDomain('mergedPolicy');
+               $response
+                       ->expects($this->exactly(2))
+                       ->method('getContentSecurityPolicy')
+                       ->willReturn($currentPolicy);
+               $this->contentSecurityPolicyManager
+                       ->expects($this->once())
+                       ->method('getDefaultPolicy')
+                       ->willReturn($defaultPolicy);
+               $this->contentSecurityPolicyManager
+                       ->expects($this->once())
+                       ->method('mergePolicies')
+                       ->with($defaultPolicy, $currentPolicy)
+                       ->willReturn($mergedPolicy);
+               $response->expects($this->once())
+                       ->method('setContentSecurityPolicy')
+                       ->with($mergedPolicy);
+
+               $this->assertEquals($response, $this->middleware->afterController($this->controller, 'test', $response));
+       }
 }
index ab19a43e91eb902a08a65bdc0bdcb274b8cf0fd5..6f7842fdfd9b61952119b44204ee598f12f8abaa 100644 (file)
@@ -56,6 +56,22 @@ class CsrfTokenManagerTest extends \Test\TestCase {
                $this->assertEquals($expected, $this->csrfTokenManager->getToken());
        }
 
+       public function testGetTokenWithExistingTokenKeepsOnSecondRequest() {
+               $this->storageInterface
+                       ->expects($this->once())
+                       ->method('hasToken')
+                       ->willReturn(true);
+               $this->storageInterface
+                       ->expects($this->once())
+                       ->method('getToken')
+                       ->willReturn('MyExistingToken');
+
+               $expected = new \OC\Security\CSRF\CsrfToken('MyExistingToken');
+               $token = $this->csrfTokenManager->getToken();
+               $this->assertSame($token, $this->csrfTokenManager->getToken());
+               $this->assertSame($token, $this->csrfTokenManager->getToken());
+       }
+
        public function testGetTokenWithoutExistingToken() {
                $this->storageInterface
                        ->expects($this->once())
index da640ce5052a73ff86f441ff547c8ff48bdeb89c..d19d1de916c93804b875a0b304e0a048b9fc5653 100644 (file)
@@ -28,6 +28,13 @@ class CsrfTokenTest extends \Test\TestCase {
                $this->assertSame(':', $csrfToken->getEncryptedValue()[16]);
        }
 
+       public function testGetEncryptedValueStaysSameOnSecondRequest() {
+               $csrfToken = new \OC\Security\CSRF\CsrfToken('MyCsrfToken');
+               $tokenValue = $csrfToken->getEncryptedValue();
+               $this->assertSame($tokenValue, $csrfToken->getEncryptedValue());
+               $this->assertSame($tokenValue, $csrfToken->getEncryptedValue());
+       }
+
        public function testGetDecryptedValue() {
                $csrfToken = new \OC\Security\CSRF\CsrfToken('XlQhHjgWCgBXAEI0Khl+IQEiCXN2LUcDHAQTQAc1HQs=:qgkUlg8l3m8WnkOG4XM9Az33pAt1vSVMx4hcJFsxdqc=');
                $this->assertSame('/3JKTq2ldmzcDr1f5zDJ7Wt0lEgqqfKF', $csrfToken->getDecryptedValue());