Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for ignored CSP_NONCE in ContentSecurity Header #43573

Merged
merged 4 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/templates/layout.base.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0">
<meta name="theme-color" content="<?php p($theme->getColorPrimary()); ?>">
<meta name="csp-nonce" nonce="<?php p($_['cspNonce']); /* Do not pass into "content" to prevent exfiltration */ ?>">
<link rel="icon" href="<?php print_unescaped(image_path('core', 'favicon.ico')); /* IE11+ supports png */ ?>">
<link rel="apple-touch-icon" href="<?php print_unescaped(image_path('core', 'favicon-touch.png')); ?>">
<link rel="mask-icon" sizes="any" href="<?php print_unescaped(image_path('core', 'favicon-mask.svg')); ?>" color="<?php p($theme->getColorPrimary()); ?>">
Expand Down
1 change: 1 addition & 0 deletions core/templates/layout.guest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
p($theme->getTitle());
?>
</title>
<meta name="csp-nonce" nonce="<?php p($_['cspNonce']); /* Do not pass into "content" to prevent exfiltration */ ?>">
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0">
<?php if ($theme->getiTunesAppId() !== '') { ?>
<meta name="apple-itunes-app" content="app-id=<?php p($theme->getiTunesAppId()); ?>">
Expand Down
6 changes: 3 additions & 3 deletions core/templates/layout.initial-state.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
?>
<div id="initial-state-container" style="display: none;">
<?php foreach ($_['initialStates'] as $app => $initialState) { ?>
<input type="hidden" id="initial-state-<?php p($app); ?>" value="<?php p(base64_encode($initialState)); ?>">
<?php }?>
<?php foreach ($_['initialStates'] as $app => $initialState) { ?>
<input type="hidden" id="initial-state-<?php p($app); ?>" value="<?php p(base64_encode($initialState)); ?>">
<?php }?>
</div>
1 change: 1 addition & 0 deletions core/templates/layout.public.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
p($theme->getTitle());
?>
</title>
<meta name="csp-nonce" nonce="<?php p($_['cspNonce']); /* Do not pass into "content" to prevent exfiltration */ ?>">
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0">
<?php if ($theme->getiTunesAppId() !== '') { ?>
<meta name="apple-itunes-app" content="app-id=<?php p($theme->getiTunesAppId()); ?>">
Expand Down
1 change: 1 addition & 0 deletions core/templates/layout.user.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
p($theme->getTitle());
?>
</title>
<meta name="csp-nonce" nonce="<?php p($_['cspNonce']); /* Do not pass into "content" to prevent exfiltration */ ?>">
<meta name="viewport" content="width=device-width, initial-scale=1.0" />

<?php if ($theme->getiTunesAppId() !== '') { ?>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function afterController($controller, $methodName, Response $response): R
$defaultPolicy = $this->contentSecurityPolicyManager->mergePolicies($defaultPolicy, $policy);

if ($this->cspNonceManager->browserSupportsCspV3()) {
$defaultPolicy->useJsNonce($this->csrfTokenManager->getToken()->getEncryptedValue());
$defaultPolicy->useJsNonce($this->cspNonceManager->getNonce());
susnux marked this conversation as resolved.
Show resolved Hide resolved
}

$response->setContentSecurityPolicy($defaultPolicy);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ public function __construct(
public function getNonce(): string {
if ($this->nonce === '') {
if (empty($this->request->server['CSP_NONCE'])) {
$this->nonce = base64_encode($this->csrfTokenManager->getToken()->getEncryptedValue());
// Get the token from the CSRF token, we only use the "shared secret" part
// as the first part does not add any security / entropy to the token
// so it can be ignored to keep the nonce short while keeping the same randomness
$csrfSecret = explode(':', ($this->csrfTokenManager->getToken()->getEncryptedValue()));
$this->nonce = end($csrfSecret);
} else {
$this->nonce = $this->request->server['CSP_NONCE'];
}
Expand Down
9 changes: 6 additions & 3 deletions lib/private/Template/Base.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@ class Base {
* @param string $template
* @param string $requestToken
* @param \OCP\IL10N $l10n
* @param string $cspNonce
* @param Defaults $theme
*/
public function __construct($template, $requestToken, $l10n, $theme) {
$this->vars = [];
$this->vars['requesttoken'] = $requestToken;
public function __construct($template, $requestToken, $l10n, $theme, $cspNonce) {
$this->vars = [
'cspNonce' => $cspNonce,
'requesttoken' => $requestToken,
];
$this->l10n = $l10n;
$this->template = $template;
$this->theme = $theme;
Expand Down
9 changes: 8 additions & 1 deletion lib/private/legacy/OC_Template.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public function __construct($app, $name, $renderAs = TemplateResponse::RENDER_AS
$theme = OC_Util::getTheme();

$requestToken = (OC::$server->getSession() && $registerCall) ? \OCP\Util::callRegister() : '';
$cspNonce = \OCP\Server::get(\OC\Security\CSP\ContentSecurityPolicyNonceManager::class)->getNonce();

$parts = explode('/', $app); // fix translation when app is something like core/lostpassword
$l10n = \OC::$server->getL10N($parts[0]);
Expand All @@ -56,7 +57,13 @@ public function __construct($app, $name, $renderAs = TemplateResponse::RENDER_AS
$this->path = $path;
$this->app = $app;

parent::__construct($template, $requestToken, $l10n, $themeDefaults);
parent::__construct(
$template,
$requestToken,
$l10n,
$themeDefaults,
$cspNonce,
);
}


Expand Down
4 changes: 2 additions & 2 deletions lib/public/AppFramework/Http/EmptyContentSecurityPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public function useStrictDynamicOnScripts(bool $state = false): self {
}

/**
* Use the according JS nonce
* The base64 encoded nonce to be used for script source.
* This method is only for CSPMiddleware, custom values are ignored in mergePolicies of ContentSecurityPolicyManager
*
* @param string $nonce
Expand Down Expand Up @@ -448,7 +448,7 @@ public function buildPolicy() {
if ($this->strictDynamicAllowed) {
$scriptSrc .= '\'strict-dynamic\' ';
}
$scriptSrc .= '\'nonce-'.base64_encode($this->jsNonce).'\'';
$scriptSrc .= '\'nonce-'.$this->jsNonce.'\'';
$allowedScriptDomains = array_flip($this->allowedScriptDomains);
unset($allowedScriptDomains['\'self\'']);
$this->allowedScriptDomains = array_flip($allowedScriptDomains);
Expand Down
Loading
Loading