Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1603)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: Source/core/page/ContentSecurityPolicy.cpp

Issue 14949017: Implementation of W3C compliant CSP script-src nonce. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Fixed broken nonce behavior on script redirects. Added test for redirects as well. Created 7 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« Source/core/loader/cache/CachedResourceLoader.cpp ('K') | « Source/core/page/ContentSecurityPolicy.h ('k') | Source/core/page/DOMSecurityPolicy.cpp » ('j') | Source/core/page/DOMSecurityPolicy.cpp » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/page/ContentSecurityPolicy.cpp
diff --git a/Source/core/page/ContentSecurityPolicy.cpp b/Source/core/page/ContentSecurityPolicy.cpp
index 7aea7903bb3f7f7e2889577aa5ee2624d6ceeace..8c0eb3d34b1b3104f352410c657ac7e896dfb4a1 100644
--- a/Source/core/page/ContentSecurityPolicy.cpp
+++ b/Source/core/page/ContentSecurityPolicy.cpp
@@ -69,7 +69,7 @@ bool isDirectiveValueCharacter(UChar c)
bool isNonceCharacter(UChar c)
{
- return (c >= 0x21 && c <= 0x7e) && c != ',' && c != ';'; // VCHAR - ',' - ';'
+ return isASCIIAlphanumeric(c);
}
bool isSourceCharacter(UChar c)
@@ -124,7 +124,6 @@ static const char styleSrc[] = "style-src";
static const char baseURI[] = "base-uri";
static const char formAction[] = "form-action";
static const char pluginTypes[] = "plugin-types";
-static const char scriptNonce[] = "script-nonce";
static const char reflectedXSS[] = "reflected-xss";
bool isDirectiveName(const String& name)
@@ -143,7 +142,6 @@ bool isDirectiveName(const String& name)
|| equalIgnoringCase(name, baseURI)
|| equalIgnoringCase(name, formAction)
|| equalIgnoringCase(name, pluginTypes)
- || equalIgnoringCase(name, scriptNonce)
|| equalIgnoringCase(name, reflectedXSS)
);
}
@@ -320,6 +318,7 @@ public:
bool matches(const KURL&);
bool allowInline() const { return m_allowInline; }
bool allowEval() const { return m_allowEval; }
+ bool allowNonce(const String& nonce) const { return !nonce.isNull() && m_nonces.contains(nonce); }
private:
void parse(const UChar* begin, const UChar* end);
@@ -329,11 +328,13 @@ private:
bool parseHost(const UChar* begin, const UChar* end, String& host, bool& hostHasWildcard);
bool parsePort(const UChar* begin, const UChar* end, int& port, bool& portHasWildcard);
bool parsePath(const UChar* begin, const UChar* end, String& path);
+ bool parseNonce(const UChar* begin, const UChar* end, String& nonce);
void addSourceSelf();
void addSourceStar();
void addSourceUnsafeInline();
void addSourceUnsafeEval();
+ void addSourceNonce(const String& nonce);
ContentSecurityPolicy* m_policy;
Vector<CSPSource> m_list;
@@ -341,6 +342,7 @@ private:
bool m_allowStar;
bool m_allowInline;
bool m_allowEval;
+ HashSet<String> m_nonces;
};
CSPSourceList::CSPSourceList(ContentSecurityPolicy* policy, const String& directiveName)
@@ -445,6 +447,15 @@ bool CSPSourceList::parseSource(const UChar* begin, const UChar* end,
return true;
}
+ String nonce;
+ if (!parseNonce(begin, end, nonce))
+ return false;
+
+ if (!nonce.isNull()) {
+ addSourceNonce(nonce);
+ return true;
+ }
+
const UChar* position = begin;
const UChar* beginHost = begin;
const UChar* beginPath = end;
@@ -524,6 +535,30 @@ bool CSPSourceList::parseSource(const UChar* begin, const UChar* end,
return true;
}
+// nonce-source = "'nonce-" nonce-value "'"
+// nonce-value = *( ALPHA / DIGIT )
+//
+bool CSPSourceList::parseNonce(const UChar* begin, const UChar* end, String& nonce)
+{
+ DEFINE_STATIC_LOCAL(const String, noncePrefix, (ASCIILiteral("'nonce-")));
+
+ if (!equalIgnoringCase(noncePrefix.characters(), begin, noncePrefix.length()))
+ return true;
+
+ const UChar* position = begin + noncePrefix.length();
+ const UChar* nonceBegin = position;
+
+ skipWhile<isNonceCharacter>(position, end);
+ ASSERT(nonceBegin <= position);
+ nonce = String(nonceBegin, position - nonceBegin);
+
+ if ((position + 1) != end && *position != '\'') {
+ return false;
+ }
+
+ return true;
+}
+
// ; <scheme> production from RFC 3986
// scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
//
@@ -661,6 +696,11 @@ void CSPSourceList::addSourceUnsafeEval()
m_allowEval = true;
}
+void CSPSourceList::addSourceNonce(const String& nonce)
+{
+ m_nonces.add(nonce);
+}
+
class CSPDirective {
public:
CSPDirective(const String& name, const String& value, ContentSecurityPolicy* policy)
@@ -681,50 +721,6 @@ private:
ContentSecurityPolicy* m_policy;
};
-class NonceDirective : public CSPDirective {
-public:
- NonceDirective(const String& name, const String& value, ContentSecurityPolicy* policy)
- : CSPDirective(name, value, policy)
- {
- parse(value);
- }
-
- bool allows(const String& nonce) const
- {
- return (!m_scriptNonce.isEmpty() && nonce.stripWhiteSpace() == m_scriptNonce);
- }
-
-private:
- void parse(const String& value)
- {
- String nonce;
- const UChar* position = value.characters();
- const UChar* end = position + value.length();
-
- skipWhile<isASCIISpace>(position, end);
- const UChar* nonceBegin = position;
- if (position == end) {
- policy()->reportInvalidNonce(String());
- m_scriptNonce = "";
- return;
- }
- skipWhile<isNonceCharacter>(position, end);
- if (nonceBegin < position)
- nonce = String(nonceBegin, position - nonceBegin);
-
- // Trim off trailing whitespace: If we're not at the end of the string, log
- // an error.
- skipWhile<isASCIISpace>(position, end);
- if (position < end) {
- policy()->reportInvalidNonce(value);
- m_scriptNonce = "";
- } else
- m_scriptNonce = nonce;
- }
-
- String m_scriptNonce;
-};
-
class MediaListDirective : public CSPDirective {
public:
MediaListDirective(const String& name, const String& value, ContentSecurityPolicy* policy)
@@ -817,6 +813,7 @@ public:
bool allowInline() const { return m_sourceList.allowInline(); }
bool allowEval() const { return m_sourceList.allowEval(); }
+ bool allowNonce(const String& nonce) const { return m_sourceList.allowNonce(nonce.stripWhiteSpace()); }
private:
CSPSourceList m_sourceList;
@@ -835,7 +832,6 @@ public:
bool allowInlineScript(const String& contextURL, const WTF::OrdinalNumber& contextLine, ContentSecurityPolicy::ReportingStatus) const;
bool allowInlineStyle(const String& contextURL, const WTF::OrdinalNumber& contextLine, ContentSecurityPolicy::ReportingStatus) const;
bool allowEval(ScriptState*, ContentSecurityPolicy::ReportingStatus) const;
- bool allowScriptNonce(const String& nonce, const String& contextURL, const WTF::OrdinalNumber& contextLine, const KURL&) const;
bool allowPluginType(const String& type, const String& typeAttribute, const KURL&, ContentSecurityPolicy::ReportingStatus) const;
bool allowScriptFromSource(const KURL&, ContentSecurityPolicy::ReportingStatus) const;
@@ -848,6 +844,7 @@ public:
bool allowConnectToSource(const KURL&, ContentSecurityPolicy::ReportingStatus) const;
bool allowFormAction(const KURL&, ContentSecurityPolicy::ReportingStatus) const;
bool allowBaseURI(const KURL&, ContentSecurityPolicy::ReportingStatus) const;
+ bool allowNonce(const String&) const;
void gatherReportURIs(DOMStringList&) const;
const String& evalDisabledErrorMessage() { return m_evalDisabledErrorMessage; }
@@ -862,7 +859,6 @@ private:
bool parseDirective(const UChar* begin, const UChar* end, String& name, String& value);
void parseReportURI(const String& name, const String& value);
- void parseScriptNonce(const String& name, const String& value);
void parsePluginTypes(const String& name, const String& value);
void parseReflectedXSS(const String& name, const String& value);
void addDirective(const String& name, const String& value);
@@ -876,7 +872,7 @@ private:
bool checkEval(SourceListDirective*) const;
bool checkInline(SourceListDirective*) const;
- bool checkNonce(NonceDirective*, const String&) const;
+ bool checkNonce(SourceListDirective*, const String&) const;
bool checkSource(SourceListDirective*, const KURL&) const;
bool checkMediaType(MediaListDirective*, const String& type, const String& typeAttribute) const;
@@ -884,7 +880,6 @@ private:
bool checkEvalAndReportViolation(SourceListDirective*, const String& consoleMessage, const String& contextURL = String(), const WTF::OrdinalNumber& contextLine = WTF::OrdinalNumber::beforeFirst(), ScriptState* = 0) const;
bool checkInlineAndReportViolation(SourceListDirective*, const String& consoleMessage, const String& contextURL, const WTF::OrdinalNumber& contextLine, bool isScript) const;
- bool checkNonceAndReportViolation(NonceDirective*, const String& nonce, const String& consoleMessage, const String& contextURL, const WTF::OrdinalNumber& contextLine) const;
bool checkSourceAndReportViolation(SourceListDirective*, const KURL&, const String& effectiveDirective) const;
bool checkMediaTypeAndReportViolation(MediaListDirective*, const String& type, const String& typeAttribute, const String& consoleMessage) const;
@@ -901,7 +896,6 @@ private:
ContentSecurityPolicy::ReflectedXSSDisposition m_reflectedXSSDisposition;
OwnPtr<MediaListDirective> m_pluginTypes;
- OwnPtr<NonceDirective> m_scriptNonce;
OwnPtr<SourceListDirective> m_baseURI;
OwnPtr<SourceListDirective> m_connectSrc;
OwnPtr<SourceListDirective> m_defaultSrc;
@@ -961,9 +955,9 @@ bool CSPDirectiveList::checkInline(SourceListDirective* directive) const
return !directive || directive->allowInline();
}
-bool CSPDirectiveList::checkNonce(NonceDirective* directive, const String& nonce) const
+bool CSPDirectiveList::checkNonce(SourceListDirective* directive, const String& nonce) const
{
- return !directive || directive->allows(nonce);
+ return !directive || directive->allowNonce(nonce);
}
bool CSPDirectiveList::checkSource(SourceListDirective* directive, const KURL& url) const
@@ -1002,14 +996,6 @@ bool CSPDirectiveList::checkEvalAndReportViolation(SourceListDirective* directiv
return true;
}
-bool CSPDirectiveList::checkNonceAndReportViolation(NonceDirective* directive, const String& nonce, const String& consoleMessage, const String& contextURL, const WTF::OrdinalNumber& contextLine) const
-{
- if (checkNonce(directive, nonce))
- return true;
- reportViolation(directive->text(), scriptNonce, consoleMessage + "\"" + directive->text() + "\".\n", KURL(), contextURL, contextLine);
- return denyIfEnforcingPolicy();
-}
-
bool CSPDirectiveList::checkMediaTypeAndReportViolation(MediaListDirective* directive, const String& type, const String& typeAttribute, const String& consoleMessage) const
{
if (checkMediaType(directive, type, typeAttribute))
@@ -1081,11 +1067,9 @@ bool CSPDirectiveList::allowJavaScriptURLs(const String& contextURL, const WTF::
{
DEFINE_STATIC_LOCAL(String, consoleMessage, (ASCIILiteral("Refused to execute JavaScript URL because it violates the following Content Security Policy directive: ")));
if (reportingStatus == ContentSecurityPolicy::SendReport) {
- return (checkInlineAndReportViolation(operativeDirective(m_scriptSrc.get()), consoleMessage, contextURL, contextLine, true)
- && checkNonceAndReportViolation(m_scriptNonce.get(), String(), consoleMessage, contextURL, contextLine));
+ return checkInlineAndReportViolation(operativeDirective(m_scriptSrc.get()), consoleMessage, contextURL, contextLine, true);
} else {
- return (checkInline(operativeDirective(m_scriptSrc.get()))
- && checkNonce(m_scriptNonce.get(), String()));
+ return checkInline(operativeDirective(m_scriptSrc.get()));
}
}
@@ -1093,11 +1077,9 @@ bool CSPDirectiveList::allowInlineEventHandlers(const String& contextURL, const
{
DEFINE_STATIC_LOCAL(String, consoleMessage, (ASCIILiteral("Refused to execute inline event handler because it violates the following Content Security Policy directive: ")));
if (reportingStatus == ContentSecurityPolicy::SendReport) {
- return (checkInlineAndReportViolation(operativeDirective(m_scriptSrc.get()), consoleMessage, contextURL, contextLine, true)
- && checkNonceAndReportViolation(m_scriptNonce.get(), String(), consoleMessage, contextURL, contextLine));
+ return checkInlineAndReportViolation(operativeDirective(m_scriptSrc.get()), consoleMessage, contextURL, contextLine, true);
} else {
- return (checkInline(operativeDirective(m_scriptSrc.get()))
- && checkNonce(m_scriptNonce.get(), String()));
+ return checkInline(operativeDirective(m_scriptSrc.get()));
}
}
@@ -1125,14 +1107,6 @@ bool CSPDirectiveList::allowEval(ScriptState* state, ContentSecurityPolicy::Repo
checkEval(operativeDirective(m_scriptSrc.get()));
}
-bool CSPDirectiveList::allowScriptNonce(const String& nonce, const String& contextURL, const WTF::OrdinalNumber& contextLine, const KURL& url) const
-{
- DEFINE_STATIC_LOCAL(String, consoleMessage, (ASCIILiteral("Refused to execute script because it violates the following Content Security Policy directive: ")));
- if (url.isEmpty())
- return checkNonceAndReportViolation(m_scriptNonce.get(), nonce, consoleMessage, contextURL, contextLine);
- return checkNonceAndReportViolation(m_scriptNonce.get(), nonce, "Refused to load '" + url.elidedString() + "' because it violates the following Content Security Policy directive: ", contextURL, contextLine);
-}
-
bool CSPDirectiveList::allowPluginType(const String& type, const String& typeAttribute, const KURL& url, ContentSecurityPolicy::ReportingStatus reportingStatus) const
{
return reportingStatus == ContentSecurityPolicy::SendReport ?
@@ -1220,6 +1194,11 @@ bool CSPDirectiveList::allowBaseURI(const KURL& url, ContentSecurityPolicy::Repo
checkSource(m_baseURI.get(), url);
}
+bool CSPDirectiveList::allowNonce(const String& nonce) const
+{
+ return checkNonce(operativeDirective(m_scriptSrc.get()), nonce);
abarth-chromium 2013/05/16 00:59:27 Yeah, allowNonce is secretly script-src specific.
jww 2013/05/16 20:59:00 I've renamed it to "allowScriptNonce", which shoul
+}
+
// policy = directive-list
// directive-list = [ directive *( ";" [ directive ] ) ]
//
@@ -1426,8 +1405,6 @@ void CSPDirectiveList::addDirective(const String& name, const String& value)
setCSPDirective<SourceListDirective>(name, value, m_formAction);
else if (equalIgnoringCase(name, pluginTypes))
setCSPDirective<MediaListDirective>(name, value, m_pluginTypes);
- else if (equalIgnoringCase(name, scriptNonce))
- setCSPDirective<NonceDirective>(name, value, m_scriptNonce);
else if (equalIgnoringCase(name, reflectedXSS))
parseReflectedXSS(name, value);
else
@@ -1533,16 +1510,15 @@ bool isAllowedByAllWithContext(const CSPDirectiveListVector& policies, const Str
return true;
}
-template<bool (CSPDirectiveList::*allowed)(const String&, const String&, const WTF::OrdinalNumber&, const KURL&) const>
-bool isAllowedByAllWithNonce(const CSPDirectiveListVector& policies, const String& nonce, const String& contextURL, const WTF::OrdinalNumber& contextLine, const KURL& url)
+template<bool (CSPDirectiveList::*allowed)(const String&) const>
+bool isAllowedByAllWithNonce(const CSPDirectiveListVector& policies, const String& nonce)
{
for (size_t i = 0; i < policies.size(); ++i) {
- if (!(policies[i].get()->*allowed)(nonce, contextURL, contextLine, url))
+ if (!(policies[i].get()->*allowed)(nonce))
return false;
}
return true;
}
-
template<bool (CSPDirectiveList::*allowFromURL)(const KURL&, ContentSecurityPolicy::ReportingStatus) const>
bool isAllowedByAllWithURL(const CSPDirectiveListVector& policies, const KURL& url, ContentSecurityPolicy::ReportingStatus reportingStatus)
{
@@ -1566,9 +1542,9 @@ bool ContentSecurityPolicy::allowInlineEventHandlers(const String& contextURL, c
return isAllowedByAllWithContext<&CSPDirectiveList::allowInlineEventHandlers>(m_policies, contextURL, contextLine, reportingStatus);
}
-bool ContentSecurityPolicy::allowInlineScript(const String& contextURL, const WTF::OrdinalNumber& contextLine, ContentSecurityPolicy::ReportingStatus reportingStatus) const
+bool ContentSecurityPolicy::allowInlineScript(bool validNonce, const String& contextURL, const WTF::OrdinalNumber& contextLine, ContentSecurityPolicy::ReportingStatus reportingStatus) const
{
- return isAllowedByAllWithContext<&CSPDirectiveList::allowInlineScript>(m_policies, contextURL, contextLine, reportingStatus);
+ return validNonce || isAllowedByAllWithContext<&CSPDirectiveList::allowInlineScript>(m_policies, contextURL, contextLine, reportingStatus);
abarth-chromium 2013/05/16 00:59:27 Rather than pass this boolean value into this func
jww 2013/05/16 20:59:00 Done.
}
bool ContentSecurityPolicy::allowInlineStyle(const String& contextURL, const WTF::OrdinalNumber& contextLine, ContentSecurityPolicy::ReportingStatus reportingStatus) const
@@ -1592,11 +1568,6 @@ String ContentSecurityPolicy::evalDisabledErrorMessage() const
return String();
}
-bool ContentSecurityPolicy::allowScriptNonce(const String& nonce, const String& contextURL, const WTF::OrdinalNumber& contextLine, const KURL& url) const
-{
- return isAllowedByAllWithNonce<&CSPDirectiveList::allowScriptNonce>(m_policies, nonce, contextURL, contextLine, url);
-}
-
bool ContentSecurityPolicy::allowPluginType(const String& type, const String& typeAttribute, const KURL& url, ContentSecurityPolicy::ReportingStatus reportingStatus) const
{
for (size_t i = 0; i < m_policies.size(); ++i) {
@@ -1606,9 +1577,14 @@ bool ContentSecurityPolicy::allowPluginType(const String& type, const String& ty
return true;
}
-bool ContentSecurityPolicy::allowScriptFromSource(const KURL& url, ContentSecurityPolicy::ReportingStatus reportingStatus) const
+bool ContentSecurityPolicy::allowScriptFromSource(const KURL& url, bool validNonce, ContentSecurityPolicy::ReportingStatus reportingStatus) const
+{
+ return validNonce || isAllowedByAllWithURL<&CSPDirectiveList::allowScriptFromSource>(m_policies, url, reportingStatus);
+}
+
+bool ContentSecurityPolicy::allowNonce(const String& nonce) const
{
- return isAllowedByAllWithURL<&CSPDirectiveList::allowScriptFromSource>(m_policies, url, reportingStatus);
+ return isAllowedByAllWithNonce<&CSPDirectiveList::allowNonce>(m_policies, nonce);
}
bool ContentSecurityPolicy::allowObjectFromSource(const KURL& url, ContentSecurityPolicy::ReportingStatus reportingStatus) const
« Source/core/loader/cache/CachedResourceLoader.cpp ('K') | « Source/core/page/ContentSecurityPolicy.h ('k') | Source/core/page/DOMSecurityPolicy.cpp » ('j') | Source/core/page/DOMSecurityPolicy.cpp » ('J')

Powered by Google App Engine
This is Rietveld 408576698