Implementation of script hashes for CSP.

Source/core/frame/ContentSecurityPolicy.cpp

Index: Source/core/frame/ContentSecurityPolicy.cpp
diff --git a/Source/core/frame/ContentSecurityPolicy.cpp b/Source/core/frame/ContentSecurityPolicy.cpp
index e141aa91cb904e121d2d03d7abf32e53fb9c6ee0..f3e9b67b7b82c459026edde3d0e03cb2d2f24fa0 100644
--- a/Source/core/frame/ContentSecurityPolicy.cpp
+++ b/Source/core/frame/ContentSecurityPolicy.cpp
@@ -44,11 +44,15 @@
 #include "core/platform/network/FormData.h"
 #include "core/platform/network/ResourceResponse.h"
 #include "platform/JSONValues.h"
+#include "platform/NotImplemented.h"
 #include "weborigin/KURL.h"
 #include "weborigin/KnownPorts.h"
 #include "weborigin/SchemeRegistry.h"
 #include "weborigin/SecurityOrigin.h"
 #include "wtf/HashSet.h"
+#include "wtf/SHA1.h"
+#include "wtf/text/Base64.h"
+#include "wtf/text/StringBuilder.h"
 #include "wtf/text/TextPosition.h"
 #include "wtf/text/WTFString.h"
 
@@ -69,7 +73,9 @@ bool isDirectiveValueCharacter(UChar c)
     return isASCIISpace(c) || (c >= 0x21 && c <= 0x7e); // Whitespace + VCHAR
 }
 
-bool isNonceCharacter(UChar c)
+// Only checks for general Base64 encoded chars, not '=' chars since '=' is
+// positional and may only appear at the end of a Base64 encoded string.
+bool isBase64EncodedCharacter(UChar c)
 {
     return isASCIIAlphanumeric(c) || c == '+' || c == '/';
 }
@@ -279,6 +285,7 @@ public:
     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); }
+    bool allowHash(const String& hash) const { return !hash.isNull() && m_hashes.contains(hash); }
 
 private:
     bool parseSource(const UChar* begin, const UChar* end, String& scheme, String& host, int& port, String& path, bool& hostHasWildcard, bool& portHasWildcard);
@@ -287,12 +294,14 @@ private:
     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);
+    bool parseHash(const UChar* begin, const UChar* end, String& hash, ContentSecurityPolicy::HashFunctions&);
 
     void addSourceSelf();
     void addSourceStar();
     void addSourceUnsafeInline();
     void addSourceUnsafeEval();
     void addSourceNonce(const String& nonce);
+    void addSourceScriptHash(const String& hash, const ContentSecurityPolicy::HashFunctions&);
 
     ContentSecurityPolicy* m_policy;
     Vector<CSPSource> m_list;
@@ -301,6 +310,7 @@ private:
     bool m_allowInline;
     bool m_allowEval;
     HashSet<String> m_nonces;
+    HashSet<String> m_hashes;

[Mike West, 2013/10/21 07:11:55]

I don't think we need two sets of hashes here (though we will need two calling
functions on ContentSecurityPolicy). This object represents the set of source
expressions associated with a given directive. Since we're explicitly scoped to
"script-src" or "style-src", I think we can safely just "addSourceHash" to
"m_hashes".

[jww, 2013/10/21 19:18:04]

My only objection is that, in theory, there are valid sets of scripts and styles
that might be the same hash, but you only want to authorize as a script *or* a
style (but not both). That having been said, I can't think of any attacks you
could make by using these scripts, but it seems to me, better safe than sorry
and we ought to keep them separate. What do you think?

[Mike West, 2013/10/21 19:28:48]

You'd have a policy like `script-src '<hash goes here>'`, right? That creates a
CSPSourceList for the 'script-src' directive, containing a hash value. It does
nothing for 'style-src'.

When you check 'allowScriptHash', you find the 'script-src' CSPDirective, and
ask it if the hash matches. Likewise for 'allowStyleHash'. I think that means
that we don't need to have multiple hashes for each CSPSourceList, because each
source list is tied directly to one directive, and not the other.

 };
 
 CSPSourceList::CSPSourceList(ContentSecurityPolicy* policy, const String& directiveName)
@@ -409,6 +419,16 @@ bool CSPSourceList::parseSource(const UChar* begin, const UChar* end,
             addSourceNonce(nonce);
             return true;
         }
+
+        String hash;
+        ContentSecurityPolicy::HashFunctions hashFunction = ContentSecurityPolicy::HashFunctionsNone;
+        if (!parseHash(begin, end, hash, hashFunction))
+            return false;
+
+        if (!hash.isNull()) {
+            addSourceScriptHash(hash, hashFunction);
+            return true;
+        }
     }
 
     const UChar* position = begin;
@@ -498,7 +518,7 @@ bool CSPSourceList::parseNonce(const UChar* begin, const UChar* end, String& non
     const UChar* position = begin + noncePrefix.length();
     const UChar* nonceBegin = position;
 
-    skipWhile<UChar, isNonceCharacter>(position, end);
+    skipWhile<UChar, isBase64EncodedCharacter>(position, end);
     ASSERT(nonceBegin <= position);
 
     if (((position + 1) != end  && *position != '\'') || !(position - nonceBegin))
@@ -508,6 +528,45 @@ bool CSPSourceList::parseNonce(const UChar* begin, const UChar* end, String& non
     return true;
 }
 
+// hash-source       = "'" hash-algorithm "-" hash-value "'"
+// hash-algorithm    = "sha1" / "sha256"
+// hash-value        = 1*( ALPHA / DIGIT / "+" / "/" / "=" )

[Mike West, 2013/10/21 07:11:55]

Spec: If we want to be explicit about this being base64 encoded, we'll need to
change the BNF to note that "=" is a) optional, and b) only valid at the end. I
have to believe someone has already defined that somewhere. Adam? :)

+//
+bool CSPSourceList::parseHash(const UChar* begin, const UChar* end, String& hash, ContentSecurityPolicy::HashFunctions& hashFunction)
+{
+    DEFINE_STATIC_LOCAL(const String, sha1Prefix, ("'sha1-"));
+    DEFINE_STATIC_LOCAL(const String, sha256Prefix, ("'sha256-"));
+
+    String prefix;
+    if (equalIgnoringCase(sha1Prefix.characters8(), begin, sha1Prefix.length())) {
+        prefix = sha1Prefix;
+        hashFunction = ContentSecurityPolicy::HashFunctionsSha1;
+    } else if (equalIgnoringCase(sha256Prefix.characters8(), begin, sha256Prefix.length())) {
+        notImplemented();
+    } else {
+        return true;
+    }
+
+    const UChar* position = begin + prefix.length();
+    const UChar* hashBegin = position;
+
+    skipWhile<UChar, isBase64EncodedCharacter>(position, end);
+    ASSERT(hashBegin <= position);
+
+    // Base64 encodings may end with exactly one or two '=' characters

[Mike West, 2013/10/21 07:11:55]

Might be clearer if you "skipExactly" twice (ignoring the result) rather than
looping.

[jww, 2013/10/21 19:18:04]

Done.

+    for (size_t i = 0; i < 2 && position != end; i++) {
+        if (*position != '=')
+            break;
+        position = position + 1;
+    }
+
+    if (((position + 1) != end  && *position != '\'') || !(position - hashBegin))
+        return false;
+
+    hash = String(begin + 1, position - begin - 1);
+    return true;
+}
+
 //                     ; <scheme> production from RFC 3986
 // scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
 //
@@ -650,6 +709,12 @@ void CSPSourceList::addSourceNonce(const String& nonce)
     m_nonces.add(nonce);
 }
 
+void CSPSourceList::addSourceScriptHash(const String& hash, const ContentSecurityPolicy::HashFunctions& hashFunction)
+{
+    m_hashes.add(hash);
+    m_policy->usesScriptHashFunction(hashFunction);
+}
+
 class CSPDirective {
 public:
     CSPDirective(const String& name, const String& value, ContentSecurityPolicy* policy)
@@ -766,6 +831,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()); }
+    bool allowHash(const String& hash) const { return m_sourceList.allowHash(hash); }
 
 private:
     CSPSourceList m_sourceList;
@@ -800,6 +866,7 @@ public:
     bool allowBaseURI(const KURL&, ContentSecurityPolicy::ReportingStatus) const;
     bool allowScriptNonce(const String&) const;
     bool allowStyleNonce(const String&) const;
+    bool allowScriptHash(const String&) const;
 
     void gatherReportURIs(DOMStringList&) const;
     const String& evalDisabledErrorMessage() const { return m_evalDisabledErrorMessage; }
@@ -828,6 +895,7 @@ private:
     bool checkEval(SourceListDirective*) const;
     bool checkInline(SourceListDirective*) const;
     bool checkNonce(SourceListDirective*, const String&) const;
+    bool checkHash(SourceListDirective*, const String&) const;
     bool checkSource(SourceListDirective*, const KURL&) const;
     bool checkMediaType(MediaListDirective*, const String& type, const String& typeAttribute) const;
 
@@ -930,6 +998,11 @@ bool CSPDirectiveList::checkNonce(SourceListDirective* directive, const String&
     return !directive || directive->allowNonce(nonce);
 }
 
+bool CSPDirectiveList::checkHash(SourceListDirective* directive, const String& hash) const
+{
+    return !directive || directive->allowHash(hash);
+}
+
 bool CSPDirectiveList::checkSource(SourceListDirective* directive, const KURL& url) const
 {
     return !directive || directive->allows(url);
@@ -1175,6 +1248,11 @@ bool CSPDirectiveList::allowStyleNonce(const String& nonce) const
     return checkNonce(operativeDirective(m_styleSrc.get()), nonce);
 }
 
+bool CSPDirectiveList::allowScriptHash(const String& hash) const
+{
+    return checkHash(operativeDirective(m_scriptSrc.get()), hash);
+}
+
 // policy            = directive-list
 // directive-list    = [ directive *( ";" [ directive ] ) ]
 //
@@ -1399,6 +1477,7 @@ void CSPDirectiveList::addDirective(const String& name, const String& value)
 ContentSecurityPolicy::ContentSecurityPolicy(ExecutionContext* executionContext)
     : m_executionContext(executionContext)
     , m_overrideInlineStyleAllowed(false)
+    , m_sourceHashFunctionsUsed(HashFunctionsNone)
 {
 }
 
@@ -1524,6 +1603,17 @@ bool isAllowedByAllWithNonce(const CSPDirectiveListVector& policies, const Strin
     }
     return true;
 }
+
+template<bool (CSPDirectiveList::*allowed)(const String&) const>
+bool isAllowedByAllWithHash(const CSPDirectiveListVector& policies, const String& hash)
+{
+    for (size_t i = 0; i < policies.size(); ++i) {
+        if (!(policies[i].get()->*allowed)(hash))
+            return false;
+    }
+    return true;
+}
+
 template<bool (CSPDirectiveList::*allowFromURL)(const KURL&, ContentSecurityPolicy::ReportingStatus) const>
 bool isAllowedByAllWithURL(const CSPDirectiveListVector& policies, const KURL& url, ContentSecurityPolicy::ReportingStatus reportingStatus)
 {
@@ -1597,6 +1687,34 @@ bool ContentSecurityPolicy::allowStyleNonce(const String& nonce) const
     return isAllowedByAllWithNonce<&CSPDirectiveList::allowStyleNonce>(m_policies, nonce);
 }
 
+bool ContentSecurityPolicy::allowScriptHash(const String& source) const
+{
+    DEFINE_STATIC_LOCAL(const String, sha1Prefix, ("sha1-"));
+
+    // TODO(jww) We don't currently have a WTF SHA256 implementation. Once we
+    // have that, we should implement a proper check for sha256 hash values here.
+    if (HashFunctionsSha1 & m_sourceHashFunctionsUsed) {
+        SHA1 sourceSha1;
+        sourceSha1.addBytes(UTF8Encoding().normalizeAndEncode(source, WTF::EntitiesForUnencodables));

[Mike West, 2013/10/21 07:11:55]

This all looks quite sane to me, but I'd like Adam to sign off on it, since he
actually knows things about encoding. :)

[jww, 2013/10/21 19:18:04]

sgtm

+        Vector<uint8_t, 20> digest;
+        sourceSha1.computeHash(digest);
+
+        StringBuilder hash;
+        hash.reserveCapacity(sha1Prefix.length() + digest.size());
+        hash.append(sha1Prefix);
+        hash.append(base64Encode(reinterpret_cast<char*>(digest.data()), digest.size()));
+        if (isAllowedByAllWithHash<&CSPDirectiveList::allowScriptHash>(m_policies, hash.toString()))

[Mike West, 2013/10/21 07:11:55]

Ok, this makes sense. You put the hash types on the policy object so that you
only need to calculate the value once, then you distribute it to all the
CSPDirectiveLists for validation. Can you add a comment to that effect in the .h
file?

[jww, 2013/10/21 19:18:04]

Done.

+            return true;
+    }
+
+    return false;
+}
+
+void ContentSecurityPolicy::usesScriptHashFunction(HashFunctions hashFunction)
+{
+    m_sourceHashFunctionsUsed |= hashFunction;
+}
+
 bool ContentSecurityPolicy::allowObjectFromSource(const KURL& url, ContentSecurityPolicy::ReportingStatus reportingStatus) const
 {
     return isAllowedByAllWithURL<&CSPDirectiveList::allowObjectFromSource>(m_policies, url, reportingStatus);
@@ -1851,12 +1969,6 @@ void ContentSecurityPolicy::reportInvalidPathCharacter(const String& directiveNa
     logToConsole(message);
 }
 
-void ContentSecurityPolicy::reportInvalidNonce(const String& nonce) const
-{
-    String message = "Ignoring invalid Content Security Policy script nonce: '" + nonce + "'.\n";
-    logToConsole(message);
-}
-
 void ContentSecurityPolicy::reportInvalidSourceExpression(const String& directiveName, const String& source) const
 {
     String message = "The source list for Content Security Policy directive '" + directiveName + "' contains an invalid source: '" + source + "'. It will be ignored.";