Implement the `require-sri-for` CSP directive

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/frame/csp/CSPDirectiveList.h"
 
 #include "bindings/core/v8/SourceLocation.h"
 #include "core/dom/Document.h"
 #include "core/dom/SecurityContext.h"
 #include "core/dom/SpaceSplitString.h"
@@ skipping 155 matching lines @@
         //
         // TODO(mkwst): Move this check up into the browser process.  See
         // https://crbug.com/555418.
         KURL url(KURL(), current->securityContext()->getSecurityOrigin()->toString());
         if (!directive->allows(url, ResourceRequest::RedirectStatus::NoRedirect))
             return false;
     }
     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)
         return true;
     if (typeAttribute.isEmpty() || typeAttribute.stripWhiteSpace() != type)
         return false;
     return directive->allows(type);
 }
 
 SourceListDirective* CSPDirectiveList::operativeDirective(SourceListDirective* directive) const
@@ skipping 358 matching lines @@
     }
 
     // The directive-value may be empty.
     if (valueBegin == position)
         return true;
 
     value = String(valueBegin, position - valueBegin);
     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()) {
         m_policy->reportDuplicateDirective(name);
         return;
     }
 
     // Remove report-uri in meta policies, per https://www.w3.org/TR/CSP2/#delivery-html-meta-element.
     if (m_headerSource == ContentSecurityPolicyHeaderSourceMeta) {
         UseCounter::count(m_policy->document(), UseCounter::InvalidReportUriDirectiveInMetaCSP);
@@ skipping 253 matching lines @@
     } else if (equalIgnoringCase(name, ContentSecurityPolicy::Referrer)) {
         parseReferrer(name, value);
     } else if (equalIgnoringCase(name, ContentSecurityPolicy::UpgradeInsecureRequests)) {
         enableInsecureRequestsUpgrade(name, value);
     } else if (equalIgnoringCase(name, ContentSecurityPolicy::BlockAllMixedContent)) {
         enforceStrictMixedContentChecking(name, value);
     } else if (equalIgnoringCase(name, ContentSecurityPolicy::ManifestSrc)) {
         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);
     }
 }
 
 DEFINE_TRACE(CSPDirectiveList)
 {
     visitor->trace(m_policy);
     visitor->trace(m_pluginTypes);
     visitor->trace(m_baseURI);
     visitor->trace(m_childSrc);
     visitor->trace(m_connectSrc);
     visitor->trace(m_defaultSrc);
     visitor->trace(m_fontSrc);
     visitor->trace(m_formAction);
     visitor->trace(m_frameAncestors);
     visitor->trace(m_frameSrc);
     visitor->trace(m_imgSrc);
     visitor->trace(m_mediaSrc);
     visitor->trace(m_manifestSrc);
     visitor->trace(m_objectSrc);
     visitor->trace(m_scriptSrc);
     visitor->trace(m_styleSrc);
 }
 
 
 } // namespace blink