Skip to content

Commit

Permalink
feat(ContentSecurityPolicy): Allow to set strict-dynamic on `script…
Browse files Browse the repository at this point in the history 
…-src-elem` only

This adds the possibility to set `strict-dynamic` on `script-src-elem` only while keep the default rules for `script-src`.
The idea is to allow loading module js which imports other files and thus does not allow nonces on import but on the initial script tag.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
  • Loading branch information
@susnux
susnux committed Nov 17, 2023
1 parent 158aedb commit 9ea0159
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 6 deletions.
8 changes: 8 additions & 0 deletions lib/private/Security/CSP/ContentSecurityPolicy.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,12 @@ public function isStrictDynamicAllowed(): bool {
public function setStrictDynamicAllowed(bool $strictDynamicAllowed): void {
$this->strictDynamicAllowed = $strictDynamicAllowed;
}

public function isStrictDynamicAllowedOnScripts(): bool {
return $this->strictDynamicAllowedOnScripts;
}

public function setStrictDynamicAllowedOnScripts(bool $strictDynamicAllowedOnScripts): void {
$this->strictDynamicAllowedOnScripts = $strictDynamicAllowedOnScripts;
}
}
2 changes: 2 additions & 0 deletions lib/public/AppFramework/Http/ContentSecurityPolicy.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class ContentSecurityPolicy extends EmptyContentSecurityPolicy {
protected ?bool $evalWasmAllowed = false;
/** @var bool Whether strict-dynamic should be set */
protected $strictDynamicAllowed = false;
/** @var bool Whether strict-dynamic should be set for 'script-src-elem' */
protected $strictDynamicAllowedOnScripts = false;
/** @var array Domains from which scripts can get loaded */
protected $allowedScriptDomains = [
'\'self\'',
Expand Down
34 changes: 28 additions & 6 deletions lib/public/AppFramework/Http/EmptyContentSecurityPolicy.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ class EmptyContentSecurityPolicy {
protected $useJsNonce = null;
/** @var bool Whether strict-dynamic should be used */
protected $strictDynamicAllowed = null;
/** @var bool Whether strict-dynamic should be used on script-src-elem */
protected $strictDynamicAllowedOnScripts = null;
/**
* @var bool Whether eval in JS scripts is allowed
* TODO: Disallow per default
Expand Down Expand Up @@ -93,6 +95,18 @@ public function useStrictDynamic(bool $state = false): self {
return $this;
}

/**
* In contrast to `useStrictDynamic` this only sets strict-dynamic on script-src-elem
* Meaning only grants trust to all imports of scripts that were loaded in `<script>` tags, and thus weakens less the CSP.
* @param bool $state
* @return EmptyContentSecurityPolicy
* @since 28.0.0
*/
public function useStrictDynamicOnScripts(bool $state = false): self {
$this->strictDynamicAllowedOnScripts = $state;
return $this;
}

/**
* Use the according JS nonce
* This method is only for CSPMiddleware, custom values are ignored in mergePolicies of ContentSecurityPolicyManager
Expand Down Expand Up @@ -448,27 +462,35 @@ public function buildPolicy() {

if (!empty($this->allowedScriptDomains) || $this->evalScriptAllowed || $this->evalWasmAllowed) {
$policy .= 'script-src ';
$scriptSrc = '';
if (is_string($this->useJsNonce)) {
if ($this->strictDynamicAllowed) {
$policy .= '\'strict-dynamic\' ';
$scriptSrc .= '\'strict-dynamic\' ';
}
$policy .= '\'nonce-'.base64_encode($this->useJsNonce).'\'';
$scriptSrc .= '\'nonce-'.base64_encode($this->useJsNonce).'\'';
$allowedScriptDomains = array_flip($this->allowedScriptDomains);
unset($allowedScriptDomains['\'self\'']);
$this->allowedScriptDomains = array_flip($allowedScriptDomains);
if (count($allowedScriptDomains) !== 0) {
$policy .= ' ';
$scriptSrc .= ' ';
}
}
if (is_array($this->allowedScriptDomains)) {
$policy .= implode(' ', $this->allowedScriptDomains);
$scriptSrc .= implode(' ', $this->allowedScriptDomains);
}
if ($this->evalScriptAllowed) {
$policy .= ' \'unsafe-eval\'';
$scriptSrc .= ' \'unsafe-eval\'';
}
if ($this->evalWasmAllowed) {
$policy .= ' \'wasm-unsafe-eval\'';
$scriptSrc .= ' \'wasm-unsafe-eval\'';
}
$policy .= $scriptSrc . ';';
}

// We only need to set this if 'strictDynamicAllowed' is not set because otherwise we can simply fall back to script-src
if ($this->strictDynamicAllowedOnScripts && !(is_string($this->useJsNonce) && $this->strictDynamicAllowed)) {
$policy .= 'script-src-elem \'strict-dynamic\' ';
$policy .= $scriptSrc ?? '';
$policy .= ';';
}

Expand Down
37 changes: 37 additions & 0 deletions tests/lib/AppFramework/Http/ContentSecurityPolicyTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,4 +479,41 @@ public function testGetPolicyNonceStrictDynamic() {
$this->contentSecurityPolicy->useStrictDynamic(true);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyNonceStrictDynamicOnScripts() {
$nonce = 'my-nonce';
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'nonce-".base64_encode($nonce) . "';script-src-elem 'strict-dynamic' 'nonce-".base64_encode($nonce) . "';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self' data:;connect-src 'self';media-src 'self';frame-ancestors 'self';form-action 'self'";

$this->contentSecurityPolicy->useJsNonce($nonce);
$this->contentSecurityPolicy->useStrictDynamicOnScripts(true);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyStrictDynamicOnScripts() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self';script-src-elem 'strict-dynamic' 'self';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self' data:;connect-src 'self';media-src 'self';frame-ancestors 'self';form-action 'self'";

$this->contentSecurityPolicy->useStrictDynamicOnScripts(true);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyStrictDynamicAndStrictDynamicOnScripts() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'self';script-src-elem 'strict-dynamic' 'self';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self' data:;connect-src 'self';media-src 'self';frame-ancestors 'self';form-action 'self'";

$this->contentSecurityPolicy->useStrictDynamic(true);
$this->contentSecurityPolicy->useStrictDynamicOnScripts(true);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

/**
* No duplication as we can fallback
*/
public function testGetPolicyNonceStrictDynamicAndStrictDynamicOnScripts() {
$nonce = 'my-nonce';
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'strict-dynamic' 'nonce-".base64_encode($nonce) . "';style-src 'self' 'unsafe-inline';img-src 'self' data: blob:;font-src 'self' data:;connect-src 'self';media-src 'self';frame-ancestors 'self';form-action 'self'";

$this->contentSecurityPolicy->useJsNonce($nonce);
$this->contentSecurityPolicy->useStrictDynamic(true);
$this->contentSecurityPolicy->useStrictDynamicOnScripts(true);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}
}
36 changes: 36 additions & 0 deletions tests/lib/AppFramework/Http/EmptyContentSecurityPolicyTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,42 @@ public function testGetPolicyWithJsNonceAndScriptDomains() {
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyWithJsNonceAndStrictDynamic() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'strict-dynamic' 'nonce-TXlKc05vbmNl' www.nextcloud.com;frame-ancestors 'none'";

$this->contentSecurityPolicy->addAllowedScriptDomain('www.nextcloud.com');
$this->contentSecurityPolicy->useStrictDynamic(true);
$this->contentSecurityPolicy->useJsNonce('MyJsNonce');
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyWithJsNonceAndStrictDynamicAndStrictDynamicOnScripts() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'strict-dynamic' 'nonce-TXlKc05vbmNl' www.nextcloud.com;frame-ancestors 'none'";

$this->contentSecurityPolicy->addAllowedScriptDomain('www.nextcloud.com');
$this->contentSecurityPolicy->useStrictDynamic(true);
$this->contentSecurityPolicy->useStrictDynamicOnScripts(true);
$this->contentSecurityPolicy->useJsNonce('MyJsNonce');
// Should be same as `testGetPolicyWithJsNonceAndStrictDynamic` because of fallback
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyWithJsNonceAndStrictDynamicOnScripts() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'nonce-TXlKc05vbmNl' www.nextcloud.com;script-src-elem 'strict-dynamic' 'nonce-TXlKc05vbmNl' www.nextcloud.com;frame-ancestors 'none'";

$this->contentSecurityPolicy->addAllowedScriptDomain('www.nextcloud.com');
$this->contentSecurityPolicy->useStrictDynamicOnScripts(true);
$this->contentSecurityPolicy->useJsNonce('MyJsNonce');
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyWithStrictDynamicOnScripts() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src-elem 'strict-dynamic' ;frame-ancestors 'none'";

$this->contentSecurityPolicy->useStrictDynamicOnScripts(true);
$this->assertSame($expectedPolicy, $this->contentSecurityPolicy->buildPolicy());
}

public function testGetPolicyWithJsNonceAndSelfScriptDomain() {
$expectedPolicy = "default-src 'none';base-uri 'none';manifest-src 'self';script-src 'nonce-TXlKc05vbmNl';frame-ancestors 'none'";

Expand Down

0 comments on commit 9ea0159

Please sign in to comment.