Commit d6fffa90 authored by mkwst's avatar mkwst Committed by Commit bot

Experiment with hiding <script>'s 'nonce' content attribute.

Nonces are valuable, as they allow script execution. It would be lovely if
we could raise the bar on exfiltration to reduce the effectiveness of some
of the attacks noted at http://sebastian-lekies.de/csp/bypasses.php.

One mechanism that might be effective against some kinds of exfiltration is
to stop treating the 'nonce' content attribute as the source of truth, instead
pulling the nonce value into an internal slot on the HTMLScriptElement at
parse-time. That prevents exfiltration via attribute leakage, mitigating the
effect of vectors like `[nonce^=ab]` and `content: attr(nonce)`
(http://cspnonce-test.appspot.com/exploit?reset=1 and
http://sebastian-lekies.de/csp/social_engineering.php, respectively). We also
clear the nonce after use ("number used _once_", right?) which mitigates the
style of attack hinted at in https://sirdarckcat.github.io/csp/fakexss.html
(though that specific issue is also resolved by fixing the browser bug in
https://codereview.chromium.org/2618323002).

Here, we're replacing the nonce content attribute with '[Replaced]', as that
gives developers a hint at what's going on (e.g. in devtools), but we could
pretty easily drop that in the future and just make it a devtools feature
entirely. Not sure what the right thing to do is..

This prototype just effects `<script>`; once we decide on reasonable behavior,
we can extend it to `<link>` and `<style>`.

BUG=680419

Review-Url: https://codereview.chromium.org/2628733005
Cr-Commit-Position: refs/heads/master@{#443252}
parent c104b1c4
......@@ -1008,6 +1008,9 @@ crbug.com/492664 imported/csswg-test/css-writing-modes-3/text-combine-upright-va
crbug.com/492664 imported/csswg-test/css-writing-modes-3/text-combine-upright-value-all-002.html [ Failure ]
crbug.com/492664 imported/csswg-test/css-writing-modes-3/text-combine-upright-value-all-003.html [ Failure ]
# We're experimenting with changing the behavior of the 'nonce' attribute
crbug.com/680419 imported/wpt/html/dom/reflection-misc.html [ Failure ]
# These CSS Writing Modes Level 3 tests pass but causes 1px diff on images, notified to the submitter.
crbug.com/492664 [ Mac ] imported/csswg-test/css-writing-modes-3/block-flow-direction-htb-001.xht [ Failure ]
crbug.com/492664 [ Mac ] imported/csswg-test/css-writing-modes-3/block-flow-direction-vrl-002.xht [ Failure ]
......
......@@ -718,6 +718,9 @@ imported/wpt/html/semantics/embedded-content/the-iframe-element/iframe_sandbox_p
imported/wpt/html/semantics/forms/form-submission-0/url-encoded.html [ Skip ]
imported/wpt/svg/linking/scripted [ Skip ]
# crbug.com/680419: We're experimenting with changing the behavior of the 'nonce' attribute
imported/wpt/html/dom/reflection-misc.html [ Skip ]
# crbug.com/656171: w3c-test-importer fails to import file with non-valid utf8 (004.worker.js).
crbug.com/656171 imported/wpt/workers/semantics/encodings [ Skip ]
......
......@@ -223,7 +223,6 @@ TEST SUCCEEDED: The value was the string 'null'. [tested HTMLScriptElement.chars
TEST SUCCEEDED: The value was the string 'null' resolved as a URL. [tested HTMLScriptElement.src]
TEST SUCCEEDED: The value was the string 'null'. [tested HTMLScriptElement.type]
TEST SUCCEEDED: The value was null. [tested HTMLScriptElement.crossOrigin]
TEST SUCCEEDED: The value was the string 'null'. [tested HTMLScriptElement.nonce]
TEST SUCCEEDED: The value was the empty string. [tested HTMLSelectElement.value]
TEST SUCCEEDED: The value was the string 'null'. [tested HTMLSelectElement.name]
......
......@@ -521,8 +521,7 @@
{name: 'charset', expectedNull: 'null'},
{name: 'src', expectedNull: 'null', isUrl: true},
{name: 'type', expectedNull: 'null'},
{name: 'crossOrigin', expectedNull: null},
{name: 'nonce', expectedNull: 'null'}
{name: 'crossOrigin', expectedNull: null}
]
},
{
......
<?php
header("Content-Security-Policy: script-src 'self' 'nonce-abc'; img-src 'none'");
?>
<!doctype html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<body>
<!-- Basics -->
<script nonce="abc">
test(t => {
assert_equals(document.querySelector('[nonce=abc]'), null);
assert_equals(document.currentScript.getAttribute('nonce'), '[Replaced]');
assert_equals(document.currentScript.nonce, 'abc');
}, "Reading 'nonce' content attribute and IDL attribute.");
test(t => {
document.currentScript.setAttribute('nonce', 'xyz');
assert_equals(document.currentScript.getAttribute('nonce'), '[Replaced]');
assert_equals(document.currentScript.nonce, 'xyz');
}, "Writing 'nonce' content attribute.");
test(t => {
assert_equals(document.currentScript.nonce, 'xyz');
document.currentScript.nonce = 'foo';
assert_equals(document.currentScript.nonce, 'foo');
}, "Writing 'nonce' DOM attribute.");
async_test(t => {
var script = document.currentScript;
assert_equals(script.nonce, 'foo');
setTimeout(_ => {
assert_equals(script.nonce, "");
t.done();
}, 1);
}, "'nonce' DOM attribute cleared after current task.");
</script>
<!-- CSS Leakage -->
<style>
#test { display: block; }
#test[nonce=abc] { background: url(/security/resources/abe.png); }
</style>
<script nonce="abc">
var css_test = async_test(t => {
document.addEventListener('securitypolicyviolation', e => {
assert_unreached("No image should be requested via CSS.");
});
}, "Nonces don't leak via CSS side-channels.");
</script>
<script id="test" nonce="abc">
window.onload = e => {
css_test.done();
};
</script>
......@@ -347,10 +347,8 @@ bool ScriptLoader::fetchScript(const String& sourceUrl,
crossOrigin);
request.setCharset(scriptCharset());
if (ContentSecurityPolicy::isNonceableElement(m_element.get())) {
request.setContentSecurityPolicyNonce(
m_element->fastGetAttribute(HTMLNames::nonceAttr));
}
if (ContentSecurityPolicy::isNonceableElement(m_element.get()))
request.setContentSecurityPolicyNonce(client()->nonce());
request.setParserDisposition(isParserInserted() ? ParserInserted
: NotParserInserted);
......@@ -465,8 +463,8 @@ bool ScriptLoader::doExecuteScript(const ScriptSourceCode& sourceCode) {
AtomicString nonce =
ContentSecurityPolicy::isNonceableElement(m_element.get())
? m_element->fastGetAttribute(HTMLNames::nonceAttr)
: AtomicString();
? client()->nonce()
: nullAtom;
if (!m_isExternalScript &&
(!shouldBypassMainWorldCSP &&
!csp->allowInlineScript(m_element, elementDocument->url(), nonce,
......@@ -552,6 +550,10 @@ bool ScriptLoader::doExecuteScript(const ScriptSourceCode& sourceCode) {
contextDocument->popCurrentScript();
}
// "Number used _once_", so, clear it out after execution.
if (RuntimeEnabledFeatures::hideNonceContentAttributeEnabled())
client()->clearNonce();
return true;
}
......
......@@ -22,6 +22,7 @@
#define ScriptLoaderClient_h
#include "core/CoreExport.h"
#include "wtf/text/AtomicString.h"
#include "wtf/text/WTFString.h"
namespace blink {
......@@ -41,6 +42,10 @@ class CORE_EXPORT ScriptLoaderClient {
virtual bool asyncAttributeValue() const = 0;
virtual bool deferAttributeValue() const = 0;
virtual bool hasSourceAttribute() const = 0;
virtual AtomicString nonce() const = 0;
virtual void setNonce(const String&) = 0;
virtual void clearNonce() = 0;
};
} // namespace blink
......
......@@ -44,6 +44,7 @@
#include "core/frame/csp/CSPSource.h"
#include "core/frame/csp/MediaListDirective.h"
#include "core/frame/csp/SourceListDirective.h"
#include "core/html/HTMLScriptElement.h"
#include "core/inspector/ConsoleMessage.h"
#include "core/inspector/InspectorInstrumentation.h"
#include "core/loader/DocumentLoader.h"
......@@ -75,8 +76,13 @@
namespace blink {
bool ContentSecurityPolicy::isNonceableElement(const Element* element) {
if (!element->fastHasAttribute(HTMLNames::nonceAttr))
if (RuntimeEnabledFeatures::hideNonceContentAttributeEnabled() &&
isHTMLScriptElement(element)) {
if (toHTMLScriptElement(element)->nonce().isNull())
return false;
} else if (!element->fastHasAttribute(HTMLNames::nonceAttr)) {
return false;
}
bool nonceable = true;
......
......@@ -346,6 +346,7 @@ class CORE_EXPORT ContentSecurityPolicy
static bool shouldBypassMainWorld(const ExecutionContext*);
static bool isNonceableElement(const Element*);
static const char* getNonceReplacementString() { return "[Replaced]"; }
// This method checks whether the request should be allowed for an
// experimental EmbeddingCSP feature
......
......@@ -33,6 +33,7 @@
#include "core/dom/Text.h"
#include "core/events/Event.h"
#include "core/frame/UseCounter.h"
#include "core/frame/csp/ContentSecurityPolicy.h"
namespace blink {
......@@ -86,6 +87,14 @@ void HTMLScriptElement::parseAttribute(
logUpdateAttributeIfIsolatedWorldAndInDocument("script", params);
} else if (params.name == asyncAttr) {
m_loader->handleAsyncAttribute();
} else if (params.name == nonceAttr) {
if (params.newValue == ContentSecurityPolicy::getNonceReplacementString())
return;
m_nonce = params.newValue;
if (RuntimeEnabledFeatures::hideNonceContentAttributeEnabled()) {
setAttribute(nonceAttr,
ContentSecurityPolicy::getNonceReplacementString());
}
} else {
HTMLElement::parseAttribute(params);
}
......
......@@ -52,6 +52,11 @@ class CORE_EXPORT HTMLScriptElement final : public HTMLElement,
ScriptLoader* loader() const { return m_loader.get(); }
// ScriptLoaderClient
AtomicString nonce() const override { return m_nonce; }
void setNonce(const String& nonce) override { m_nonce = AtomicString(nonce); }
void clearNonce() override { m_nonce = emptyAtom; }
DECLARE_VIRTUAL_TRACE();
private:
......@@ -85,6 +90,7 @@ class CORE_EXPORT HTMLScriptElement final : public HTMLElement,
Element* cloneElementWithoutAttributesAndChildren() override;
Member<ScriptLoader> m_loader;
AtomicString m_nonce;
};
} // namespace blink
......
......@@ -37,7 +37,7 @@ interface HTMLScriptElement : HTMLElement {
// Content Security Policy
// https://w3c.github.io/webappsec/specs/content-security-policy/#script-src-the-nonce-attribute
[CEReactions, Reflect, RuntimeEnabled=ExperimentalContentSecurityPolicyFeatures] attribute DOMString nonce;
[CEReactions, RuntimeEnabled=ExperimentalContentSecurityPolicyFeatures] attribute DOMString nonce;
// Subresource Integrity
// https://w3c.github.io/webappsec-subresource-integrity/#HTMLScriptElement
......
......@@ -36,7 +36,13 @@ inline SVGScriptElement::SVGScriptElement(Document& document,
: SVGElement(SVGNames::scriptTag, document),
SVGURIReference(this),
m_loader(
ScriptLoader::create(this, wasInsertedByParser, alreadyStarted)) {}
ScriptLoader::create(this, wasInsertedByParser, alreadyStarted)) {
if (fastHasAttribute(HTMLNames::nonceAttr)) {
m_nonce = fastGetAttribute(HTMLNames::nonceAttr);
if (RuntimeEnabledFeatures::hideNonceContentAttributeEnabled())
removeAttribute(HTMLNames::nonceAttr);
}
}
SVGScriptElement* SVGScriptElement::create(Document& document,
bool insertedByParser) {
......
......@@ -46,6 +46,11 @@ class SVGScriptElement final : public SVGElement,
bool isAnimatableAttribute(const QualifiedName&) const override;
#endif
// ScriptLoaderClient
AtomicString nonce() const override { return m_nonce; }
void setNonce(const String& nonce) override { m_nonce = AtomicString(nonce); }
void clearNonce() override { m_nonce = nullAtom; }
DECLARE_VIRTUAL_TRACE();
private:
......@@ -80,6 +85,7 @@ class SVGScriptElement final : public SVGElement,
bool layoutObjectIsNeeded(const ComputedStyle&) override { return false; }
Member<ScriptLoader> m_loader;
AtomicString m_nonce;
};
} // namespace blink
......
......@@ -284,3 +284,4 @@ SendBeaconThrowForBlobWithNonSimpleType status=experimental
PerformanceNavigationTiming2 status=experimental
BackgroundVideoTrackOptimization status=stable
PerformancePaintTiming status=test
HideNonceContentAttribute status=experimental
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment