Skip to content

Defining a CSP through AddContentSecurityPolicyEvent is overridden for some policies #25226

New issue
New issue
Closed
Closed
Defining a CSP through AddContentSecurityPolicyEvent is overridden for some policies#25226
Labels
0. Needs triagePending check for reproducibility or if it fits our roadmapbugneeds infostaleTicket or PR with no recent activity
@tcitworld

Description

@tcitworld
tcitworld
opened on Jan 20, 2021

I noticed when developing nextcloud/app-certificate-requests#361 that the allowInlineScript, allowEvalScript and allowInlineStyle policies weren't being applied when defined through AddContentSecurityPolicyEvent.

The policy added through AddContentSecurityPolicyEvent gets merged here:

server/lib/private/Security/CSP/ContentSecurityPolicyManager.php

Lines 59 to 68 in a7a60de

public function getDefaultPolicy(): ContentSecurityPolicy {
$event = new AddContentSecurityPolicyEvent($this);
$this->dispatcher->dispatchTyped($event);
$defaultPolicy = new \OC\Security\CSP\ContentSecurityPolicy();
foreach ($this->policies as $policy) {
$defaultPolicy = $this->mergePolicies($defaultPolicy, $policy);
}
return $defaultPolicy;
}

And that is fine, and works properly but then it gets merged a second time there:

server/lib/private/AppFramework/Middleware/Security/CSPMiddleware.php

Lines 65 to 82 in a7a60de

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;
}

Policies which are arrays (allowedFontDomains, allowedImageDomains, ...) get merged properly, but policies which are booleans (for instance allowEvalScript, allowInlineScript) get overridden by the response policy defined by the app (either custom, either default one when using TemplateResponse, ...), or by the default fallback to OCP\AppFramework\Http\ContentSecurityPolicy (line 66).

I'm not sure whether this is an acceptable behaviour or not, and I don't care much about defining these ones, and on top of that the allowInlineScript and allowEvalScript methods are deprecated, but this behaviour isn't much expected or documented.

cc @rullzer because of CSP stuff.

Metadata

Metadata

Assignees

No one assigned

    Labels

    0. Needs triagePending check for reproducibility or if it fits our roadmapbugneeds infostaleTicket or PR with no recent activity

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions