Implement the `require-sri-for` CSP directive

third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp

Index: third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp
diff --git a/third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp b/third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp
index d90f9266bb2925e3255599e4c592c0aeadb5c845..6532029981e205b6771ec2c20d206c4ea95a1355 100644
--- a/third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp
+++ b/third_party/WebKit/Source/core/frame/csp/CSPDirectiveList.cpp
@@ -173,6 +173,51 @@ bool CSPDirectiveList::checkAncestors(SourceListDirective* directive, LocalFrame
     return true;
 }
 
+bool CSPDirectiveList::checkIntegrityPresence(WebURLRequest::RequestContext context, const IntegrityMetadataSet& integrityMetadata) const
+{
+    if (!integrityMetadata.isEmpty())
+        return true;
+
+    if (m_requireSRIFor.isEmpty())
+        return true;
+
+    for (const auto& token : m_requireSRIFor) {
+        if (equalIgnoringCase(token, "script")) {
+            if (context == WebURLRequest::RequestContextScript || context == WebURLRequest::RequestContextImport) {
+                if (integrityMetadata.isEmpty())
+                    return false;
+            }
+        } else if (equalIgnoringCase(token, "style")) {
+            if (context == WebURLRequest::RequestContextStyle) {
+                if (integrityMetadata.isEmpty())
+                    return false;
+            }
+        }
+    }
+    return true;
+}
+
+bool CSPDirectiveList::checkIntegrityPresenceAndReportViolation(WebURLRequest::RequestContext context, const KURL& url, const IntegrityMetadataSet& integrityMetadata) const
+{
+    if (checkIntegrityPresence(context, integrityMetadata))
+        return true;
+    String resourceType;
+    if (context == WebURLRequest::RequestContextScript || context == WebURLRequest::RequestContextImport)

[jww, 2016/06/11 22:45:12]

Treating RequestContextImport as guaranteed to be a script resource needs an
explanatory comment, I think. Is the reasoning just that since integrity is only
enforced for scripts and stylesheets that any import leading to this code must
be a script import, and not an HTML import or something?

[Sergey Shekyan, 2016/06/20 07:12:00]

I inherited the logic that RequestContextImport is enforced for scripts from
ContentSecurityPolicy::allowRequest, which is a little inaccurate.
RequestContext is the closest property of the request that can be aligned with
spec'ed algorithm, which says to use request's destination
(https://fetch.spec.whatwg.org/#concept-request-destination).

[Mike West, 2016/06/20 08:11:37]

Right. Contexts are from an earlier version of Fetch, before it was refactored
into the initiator/type/destination tuple. Using the context here is the right
thing to do until such time as we refactor WebURLRequest. You'll also need to
add in the Worker types.

+        resourceType = "script";
+    else if (context == WebURLRequest::RequestContextStyle)
+        resourceType = "stylesheet";
+    reportViolation(ContentSecurityPolicy::RequireSRIFor, ContentSecurityPolicy::RequireSRIFor, "Refused to load the " + resourceType + " '" + url.elidedString() + "' because 'require-sri-for' directive requires integrity attribute be present for all " + resourceType + "s.", url, ResourceRequest::RedirectStatus::NoRedirect);

[Mike West, 2016/06/10 09:25:15]

`NoRedirect` probably isn't correct here, as I think the current code is going
to trigger this on every step through a redirect in `report-only` mode. You'll
either need to refactor the code so that we only do this check on the initial
step of a request, or pass in the redirect status here so that we properly strip
the report data cross-origin.

[Sergey Shekyan, 2016/06/20 07:11:59]

Acknowledged.

+    return denyIfEnforcingPolicy();
+}
+
+bool CSPDirectiveList::allowRequestWithoutMetadata(WebURLRequest::RequestContext context, const KURL& url, const IntegrityMetadataSet& integrityMetadata, ContentSecurityPolicy::ReportingStatus reportingStatus) const
+{
+    if (reportingStatus == ContentSecurityPolicy::SendReport) {
+        return checkIntegrityPresenceAndReportViolation(context, url, integrityMetadata);
+    }
+    return checkIntegrityPresence(context, integrityMetadata);
+}
+
 bool CSPDirectiveList::checkMediaType(MediaListDirective* directive, const String& type, const String& typeAttribute) const
 {
     if (!directive)
@@ -551,6 +596,62 @@ bool CSPDirectiveList::parseDirective(const UChar* begin, const UChar* end, Stri
     return true;
 }
 
+void CSPDirectiveList::parseRequireSRIFor(const String& name, const String& value)
+{
+    if (!m_requireSRIFor.isEmpty()) {
+        m_policy->reportDuplicateDirective(name);
+        return;
+    }
+    StringBuilder tokenErrors;
+    StringBuilder validTokens;
+    unsigned numberOfTokenErrors = 0;
+    Vector<UChar> characters;
+    value.appendTo(characters);
+
+    const UChar* position = characters.data();
+    const UChar* end = position + characters.size();
+
+    while (position < end) {
+        skipWhile<UChar, isASCIISpace>(position, end);
+
+        const UChar* tokenBegin = position;
+        skipWhile<UChar, isNotASCIISpace>(position, end);
+
+        if (tokenBegin < position) {
+            String token = String(tokenBegin, position - tokenBegin);
+            if (equalIgnoringCase(token, "script") || equalIgnoringCase(token, "style")) {
+                m_requireSRIFor.append(token);
+                if (!validTokens.isEmpty())
+                    validTokens.append(' ');
+                validTokens.append(token);
+            } else {
+                if (numberOfTokenErrors)
+                    tokenErrors.append(", \'");
+                else
+                    tokenErrors.append('\'');
+                tokenErrors.append(token);
+                tokenErrors.append('\'');
+                numberOfTokenErrors++;
+            }
+        }
+    }
+
+    if (numberOfTokenErrors == 0)
+        return;
+
+    String invalidTokensErrorMessage;
+    if (numberOfTokenErrors > 1)
+        tokenErrors.append(" are invalid 'require-sri-for' tokens.");
+    else
+        tokenErrors.append(" is an invalid 'require-sri-for' token.");
+
+    invalidTokensErrorMessage = tokenErrors.toString();
+
+    if (!invalidTokensErrorMessage.isEmpty()) {

[jww, 2016/06/11 22:45:12]

I think inavildTokensErrorMessage cannot be empty here. I'm happy for this to be
a DCHECK that it's not empty, but I don't think it should be a conditional.

[Sergey Shekyan, 2016/06/20 07:12:00]

Acknowledged.

+        m_policy->reportInvalidRequireSRIForTokens(invalidTokensErrorMessage);
+    }
+}
+
 void CSPDirectiveList::parseReportURI(const String& name, const String& value)
 {
     if (!m_reportEndpoints.isEmpty()) {
@@ -824,6 +925,8 @@ void CSPDirectiveList::addDirective(const String& name, const String& value)
         setCSPDirective<SourceListDirective>(name, value, m_manifestSrc);
     } else if (equalIgnoringCase(name, ContentSecurityPolicy::TreatAsPublicAddress)) {
         treatAsPublicAddress(name, value);
+    } else if (equalIgnoringCase(name, ContentSecurityPolicy::RequireSRIFor)) {
+        parseRequireSRIFor(name, value);

[Mike West, 2016/06/10 09:25:15]

This functionality should be hidden behind the experimental features flag. You
could call `ContentSecurityPolicy::experimentalFeaturesEnabled()` before
parsing, for example.

[Sergey Shekyan, 2016/06/20 07:12:00]

Acknowledged.

     } else {
         m_policy->reportUnsupportedDirective(name);
     }