From e351ba56f13f82a9d5a8f95ee42f5343a167d5f4 Mon Sep 17 00:00:00 2001
From: Roeland Jago Douma <roeland@famdouma.nl>
Date: Tue, 25 Oct 2016 21:36:17 +0200
Subject: [PATCH] Move browserSupportsCspV3 to CSPNonceManager
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
---
.../DependencyInjection/DIContainer.php | 3 +-
.../Security/SecurityMiddleware.php | 27 +++++------------
.../CSP/ContentSecurityPolicyNonceManager.php | 29 ++++++++++++++++++-
lib/private/Server.php | 3 +-
.../Security/SecurityMiddlewareTest.php | 15 ++++++----
.../ContentSecurityPolicyNonceManagerTest.php | 4 ++-
6 files changed, 53 insertions(+), 28 deletions(-)
diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php
index 97faa0edf49..8fe9b4dca03 100644
--- a/lib/private/AppFramework/DependencyInjection/DIContainer.php
+++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php
@@ -380,7 +380,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
$app->isLoggedIn(),
$app->isAdminUser(),
$app->getServer()->getContentSecurityPolicyManager(),
- $app->getServer()->getCsrfTokenManager()
+ $app->getServer()->getCsrfTokenManager(),
+ $app->getServer()->getContentSecurityPolicyNonceManager()
);
});
diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
index 6c33c0023ea..183e55740ea 100644
--- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
@@ -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\CSP\ContentSecurityPolicyNonceManager;
use OC\Security\CSRF\CsrfTokenManager;
use OCP\AppFramework\Http\ContentSecurityPolicy;
use OCP\AppFramework\Http\EmptyContentSecurityPolicy;
@@ -80,6 +81,8 @@ class SecurityMiddleware extends Middleware {
private $contentSecurityPolicyManager;
/** @var CsrfTokenManager */
private $csrfTokenManager;
+ /** @var ContentSecurityPolicyNonceManager */
+ private $cspNonceManager;
/**
* @param IRequest $request
@@ -92,6 +95,7 @@ class SecurityMiddleware extends Middleware {
* @param bool $isAdminUser
* @param ContentSecurityPolicyManager $contentSecurityPolicyManager
* @param CSRFTokenManager $csrfTokenManager
+ * @param ContentSecurityPolicyNonceManager $cspNonceManager
*/
public function __construct(IRequest $request,
ControllerMethodReflector $reflector,
@@ -102,7 +106,8 @@ class SecurityMiddleware extends Middleware {
$isLoggedIn,
$isAdminUser,
ContentSecurityPolicyManager $contentSecurityPolicyManager,
- CsrfTokenManager $csrfTokenManager) {
+ CsrfTokenManager $csrfTokenManager,
+ ContentSecurityPolicyNonceManager $cspNonceManager) {
$this->navigationManager = $navigationManager;
$this->request = $request;
$this->reflector = $reflector;
@@ -113,6 +118,7 @@ class SecurityMiddleware extends Middleware {
$this->isAdminUser = $isAdminUser;
$this->contentSecurityPolicyManager = $contentSecurityPolicyManager;
$this->csrfTokenManager = $csrfTokenManager;
+ $this->cspNonceManager = $cspNonceManager;
}
@@ -177,23 +183,6 @@ 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
@@ -213,7 +202,7 @@ class SecurityMiddleware extends Middleware {
$defaultPolicy = $this->contentSecurityPolicyManager->getDefaultPolicy();
$defaultPolicy = $this->contentSecurityPolicyManager->mergePolicies($defaultPolicy, $policy);
- if($this->browserSupportsCspV3()) {
+ if($this->cspNonceManager->browserSupportsCspV3()) {
$defaultPolicy->useJsNonce($this->csrfTokenManager->getToken()->getEncryptedValue());
}
diff --git a/lib/private/Security/CSP/ContentSecurityPolicyNonceManager.php b/lib/private/Security/CSP/ContentSecurityPolicyNonceManager.php
index 0482ea49e5c..fe1c2e4404b 100644
--- a/lib/private/Security/CSP/ContentSecurityPolicyNonceManager.php
+++ b/lib/private/Security/CSP/ContentSecurityPolicyNonceManager.php
@@ -22,6 +22,7 @@
namespace OC\Security\CSP;
use OC\Security\CSRF\CsrfTokenManager;
+use OCP\IRequest;
/**
* @package OC\Security\CSP
@@ -29,14 +30,19 @@ use OC\Security\CSRF\CsrfTokenManager;
class ContentSecurityPolicyNonceManager {
/** @var CsrfTokenManager */
private $csrfTokenManager;
+ /** @var IRequest */
+ private $request;
/** @var string */
private $nonce = '';
/**
* @param CsrfTokenManager $csrfTokenManager
+ * @param IRequest $request
*/
- public function __construct(CsrfTokenManager $csrfTokenManager) {
+ public function __construct(CsrfTokenManager $csrfTokenManager,
+ IRequest $request) {
$this->csrfTokenManager = $csrfTokenManager;
+ $this->request = $request;
}
/**
@@ -51,4 +57,25 @@ class ContentSecurityPolicyNonceManager {
return $this->nonce;
}
+
+ /**
+ * Check if the browser supports CSP v3
+ * @return bool
+ */
+ public 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;
+ }
}
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 1ccc27802d2..21ec311401d 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -711,7 +711,8 @@ class Server extends ServerContainer implements IServerContainer {
});
$this->registerService('ContentSecurityPolicyNonceManager', function(Server $c) {
return new ContentSecurityPolicyNonceManager(
- $c->getCsrfTokenManager()
+ $c->getCsrfTokenManager(),
+ $c->getRequest()
);
});
$this->registerService('ShareManager', function(Server $c) {
diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
index b597317fca4..1fdcf485c28 100644
--- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
@@ -36,6 +36,7 @@ use OC\AppFramework\Middleware\Security\SecurityMiddleware;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OC\Security\CSP\ContentSecurityPolicy;
use OC\Security\CSP\ContentSecurityPolicyManager;
+use OC\Security\CSP\ContentSecurityPolicyNonceManager;
use OC\Security\CSRF\CsrfToken;
use OC\Security\CSRF\CsrfTokenManager;
use OCP\AppFramework\Controller;
@@ -76,6 +77,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
private $contentSecurityPolicyManager;
/** @var CsrfTokenManager|\PHPUnit_Framework_MockObject_MockObject */
private $csrfTokenManager;
+ /** @var ContentSecurityPolicyNonceManager|\PHPUnit_Framework_MockObject_MockObject */
+ private $cspNonceManager;
protected function setUp() {
parent::setUp();
@@ -88,6 +91,7 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->request = $this->createMock(IRequest::class);
$this->contentSecurityPolicyManager = $this->createMock(ContentSecurityPolicyManager::class);
$this->csrfTokenManager = $this->createMock(CsrfTokenManager::class);
+ $this->cspNonceManager = $this->createMock(ContentSecurityPolicyNonceManager::class);
$this->middleware = $this->getMiddleware(true, true);
$this->secException = new SecurityException('hey', false);
$this->secAjaxException = new SecurityException('hey', true);
@@ -109,7 +113,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$isLoggedIn,
$isAdminUser,
$this->contentSecurityPolicyManager,
- $this->csrfTokenManager
+ $this->csrfTokenManager,
+ $this->cspNonceManager
);
}
@@ -559,9 +564,9 @@ class SecurityMiddlewareTest extends \Test\TestCase {
}
public function testAfterController() {
- $this->request
+ $this->cspNonceManager
->expects($this->once())
- ->method('isUserAgent')
+ ->method('browserSupportsCspV3')
->willReturn(false);
$response = $this->createMock(Response::class);
$defaultPolicy = new ContentSecurityPolicy();
@@ -603,9 +608,9 @@ class SecurityMiddlewareTest extends \Test\TestCase {
}
public function testAfterControllerWithContentSecurityPolicy3Support() {
- $this->request
+ $this->cspNonceManager
->expects($this->once())
- ->method('isUserAgent')
+ ->method('browserSupportsCspV3')
->willReturn(true);
$token = $this->createMock(CsrfToken::class);
$token
diff --git a/tests/lib/Security/CSP/ContentSecurityPolicyNonceManagerTest.php b/tests/lib/Security/CSP/ContentSecurityPolicyNonceManagerTest.php
index 39d24807d5b..3211a5284f8 100644
--- a/tests/lib/Security/CSP/ContentSecurityPolicyNonceManagerTest.php
+++ b/tests/lib/Security/CSP/ContentSecurityPolicyNonceManagerTest.php
@@ -24,6 +24,7 @@ namespace Test\Security\CSP;
use OC\Security\CSP\ContentSecurityPolicyNonceManager;
use OC\Security\CSRF\CsrfToken;
use OC\Security\CSRF\CsrfTokenManager;
+use OCP\IRequest;
use Test\TestCase;
class ContentSecurityPolicyNonceManagerTest extends TestCase {
@@ -35,7 +36,8 @@ class ContentSecurityPolicyNonceManagerTest extends TestCase {
public function setUp() {
$this->csrfTokenManager = $this->createMock(CsrfTokenManager::class);
$this->nonceManager = new ContentSecurityPolicyNonceManager(
- $this->csrfTokenManager
+ $this->csrfTokenManager,
+ $this->createMock(IRequest::class)
);
}
--
GitLab