CSP: Enable whitelisting of external JavaScript via hashes

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

 // Copyright 2016 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 "core/frame/SubresourceIntegrity.h"
 #include "core/frame/csp/ContentSecurityPolicy.h"
 #include "core/frame/csp/SourceListDirective.h"
 #include "platform/loader/fetch/ResourceRequest.h"
 #include "platform/network/ContentSecurityPolicyParsers.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "wtf/text/StringOperators.h"
 #include "wtf/text/WTFString.h"
 
 namespace blink {
 
@@ skipping 141 matching lines @@
   for (const auto& test : cases) {
     SCOPED_TRACE(testing::Message() << "List: `" << test.list << "`, URL: `"
                                     << test.url << "`");
     KURL scriptSrc = KURL(KURL(), test.url);
 
     // Report-only
     Member<CSPDirectiveList> directiveList =
         createList(test.list, ContentSecurityPolicyHeaderTypeReport);
     EXPECT_EQ(test.expected,
               directiveList->allowScriptFromSource(
-                  scriptSrc, String(), ParserInserted,
+                  scriptSrc, String(), IntegrityMetadataSet(), ParserInserted,
                   ResourceRequest::RedirectStatus::NoRedirect,
                   SecurityViolationReportingPolicy::SuppressReporting));
 
     // Enforce
     directiveList =
         createList(test.list, ContentSecurityPolicyHeaderTypeEnforce);
     EXPECT_EQ(test.expected,
               directiveList->allowScriptFromSource(
-                  scriptSrc, String(), ParserInserted,
+                  scriptSrc, String(), IntegrityMetadataSet(), ParserInserted,
                   ResourceRequest::RedirectStatus::NoRedirect,
                   SecurityViolationReportingPolicy::SuppressReporting));
   }
 }
 
 TEST_F(CSPDirectiveListTest, AllowFromSourceWithNonce) {
   struct TestCase {
     const char* list;
     const char* url;
     const char* nonce;
@@ skipping 27 matching lines @@
     SCOPED_TRACE(testing::Message() << "List: `" << test.list << "`, URL: `"
                                     << test.url << "`");
     KURL resource = KURL(KURL(), test.url);
 
     // Report-only 'script-src'
     Member<CSPDirectiveList> directiveList =
         createList(String("script-src ") + test.list,
                    ContentSecurityPolicyHeaderTypeReport);
     EXPECT_EQ(test.expected,
               directiveList->allowScriptFromSource(
-                  resource, String(test.nonce), ParserInserted,
-                  ResourceRequest::RedirectStatus::NoRedirect,
+                  resource, String(test.nonce), IntegrityMetadataSet(),
+                  ParserInserted, ResourceRequest::RedirectStatus::NoRedirect,
                   SecurityViolationReportingPolicy::SuppressReporting));
 
     // Enforce 'script-src'
     directiveList = createList(String("script-src ") + test.list,
                                ContentSecurityPolicyHeaderTypeEnforce);
     EXPECT_EQ(test.expected,
               directiveList->allowScriptFromSource(
-                  resource, String(test.nonce), ParserInserted,
-                  ResourceRequest::RedirectStatus::NoRedirect,
+                  resource, String(test.nonce), IntegrityMetadataSet(),
+                  ParserInserted, ResourceRequest::RedirectStatus::NoRedirect,
                   SecurityViolationReportingPolicy::SuppressReporting));
 
     // Report-only 'style-src'
     directiveList = createList(String("style-src ") + test.list,
                                ContentSecurityPolicyHeaderTypeReport);
     EXPECT_EQ(test.expected,
               directiveList->allowStyleFromSource(
                   resource, String(test.nonce),
                   ResourceRequest::RedirectStatus::NoRedirect,
                   SecurityViolationReportingPolicy::SuppressReporting));
 
     // Enforce 'style-src'
     directiveList = createList(String("style-src ") + test.list,
                                ContentSecurityPolicyHeaderTypeEnforce);
     EXPECT_EQ(test.expected,
               directiveList->allowStyleFromSource(
                   resource, String(test.nonce),
                   ResourceRequest::RedirectStatus::NoRedirect,
                   SecurityViolationReportingPolicy::SuppressReporting));
 
     // Report-only 'style-src'
     directiveList = createList(String("default-src ") + test.list,
                                ContentSecurityPolicyHeaderTypeReport);
     EXPECT_EQ(test.expected,
               directiveList->allowScriptFromSource(
-                  resource, String(test.nonce), ParserInserted,
-                  ResourceRequest::RedirectStatus::NoRedirect,
+                  resource, String(test.nonce), IntegrityMetadataSet(),
+                  ParserInserted, ResourceRequest::RedirectStatus::NoRedirect,
                   SecurityViolationReportingPolicy::SuppressReporting));
     EXPECT_EQ(test.expected,
               directiveList->allowStyleFromSource(
                   resource, String(test.nonce),
                   ResourceRequest::RedirectStatus::NoRedirect,
                   SecurityViolationReportingPolicy::SuppressReporting));
 
     // Enforce 'style-src'
     directiveList = createList(String("default-src ") + test.list,
                                ContentSecurityPolicyHeaderTypeEnforce);
     EXPECT_EQ(test.expected,
               directiveList->allowScriptFromSource(
-                  resource, String(test.nonce), ParserInserted,
-                  ResourceRequest::RedirectStatus::NoRedirect,
+                  resource, String(test.nonce), IntegrityMetadataSet(),
+                  ParserInserted, ResourceRequest::RedirectStatus::NoRedirect,
                   SecurityViolationReportingPolicy::SuppressReporting));
     EXPECT_EQ(test.expected,
               directiveList->allowStyleFromSource(
                   resource, String(test.nonce),
                   ResourceRequest::RedirectStatus::NoRedirect,
                   SecurityViolationReportingPolicy::SuppressReporting));
   }
 }
 
+TEST_F(CSPDirectiveListTest, AllowScriptFromSourceWithHash) {
+  struct TestCase {
+    const char* list;
+    const char* url;
+    const char* integrity;
+    bool expected;
+  } cases[] = {
+      // Doesn't affect lists without hashes.
+      {"https://example.com", "https://example.com/file", "sha256-yay", true},
+      {"https://example.com", "https://example.com/file", "sha256-boo", true},
+      {"https://example.com", "https://example.com/file", "", true},
+      {"https://example.com", "https://not.example.com/file", "sha256-yay",
+       false},
+      {"https://example.com", "https://not.example.com/file", "sha256-boo",
+       false},
+      {"https://example.com", "https://not.example.com/file", "", false},
+
+      // Doesn't affect URLs that match the whitelist.
+      {"https://example.com 'sha256-yay'", "https://example.com/file",
+       "sha256-yay", true},
+      {"https://example.com 'sha256-yay'", "https://example.com/file",
+       "sha256-boo", true},
+      {"https://example.com 'sha256-yay'", "https://example.com/file", "",
+       true},
+
+      // Does affect URLs that don't match the whitelist.
+      {"https://example.com 'sha256-yay'", "https://not.example.com/file",
+       "sha256-yay", true},
+      {"https://example.com 'sha256-yay'", "https://not.example.com/file",
+       "sha256-boo", false},
+      {"https://example.com 'sha256-yay'", "https://not.example.com/file", "",
+       false},
+
+      // Both algorithm and digest must match.
+      {"'sha256-yay'", "https://a.com/file", "sha384-yay", false},
+
+      // Sha-1 is not supported, but -384 and -512 are.
+      {"'sha1-yay'", "https://a.com/file", "sha1-yay", false},
+      {"'sha384-yay'", "https://a.com/file", "sha384-yay", true},
+      {"'sha512-yay'", "https://a.com/file", "sha512-yay", true},
+
+      // Unknown hash algorithms don't work.
+      {"'asdf256-yay'", "https://a.com/file", "asdf256-yay", false},
+
+      // Additional whitelisted hashes in the CSP don't interfere.
+      {"'sha256-yay' 'sha384-boo'", "https://a.com/file", "sha256-yay", true},
+      {"'sha256-yay' 'sha384-boo'", "https://a.com/file", "sha384-boo", true},
+
+      // All integrity hashes must appear in the CSP (and match).
+      {"'sha256-yay'", "https://a.com/file", "sha256-yay sha384-boo", false},
+      {"'sha384-boo'", "https://a.com/file", "sha256-yay sha384-boo", false},
+      {"'sha256-yay' 'sha384-boo'", "https://a.com/file",
+       "sha256-yay sha384-yay", false},
+      {"'sha256-yay' 'sha384-boo'", "https://a.com/file",
+       "sha256-boo sha384-boo", false},
+      {"'sha256-yay' 'sha384-boo'", "https://a.com/file",
+       "sha256-yay sha384-boo", true},
+
+      // At least one integrity hash must be present.
+      {"'sha256-yay'", "https://a.com/file", "", false},
+  };
+
+  for (const auto& test : cases) {
+    SCOPED_TRACE(testing::Message()
+                 << "List: `" << test.list << "`, URL: `" << test.url << "`");

[Mike West, 2017/04/04 13:59:46]

You'll probably want to add `test.integrity` in here as well.

[Marc Treib, 2017/04/04 14:17:21]

Good catch, done.

+    KURL resource = KURL(KURL(), test.url);
+
+    IntegrityMetadataSet integrityMetadata;
+    ASSERT_EQ(SubresourceIntegrity::IntegrityParseValidResult,
+              SubresourceIntegrity::parseIntegrityAttribute(test.integrity,
+                                                            integrityMetadata));
+
+    // Report-only 'script-src'
+    Member<CSPDirectiveList> directiveList =
+        createList(String("script-src ") + test.list,
+                   ContentSecurityPolicyHeaderTypeReport);
+    EXPECT_EQ(test.expected,

[Mike West, 2017/04/04 13:59:46]

Shouldn't this be `EXPECT_TRUE`, if we're only in reporting mode?

[Marc Treib, 2017/04/04 14:17:21]

Hm, good question. I just copied this from AllowFromSourceWithNonce above. In
fact, it looks like all the Allow* tests here treat enfore and report-only
exactly the same. I guess the report-only thing is just handled on another
level? Or something is seriously broken here...

[Marc Treib, 2017/04/04 14:29:49]

I'm confused now. There's ContentSecurityPolicyHeaderType (with values Report or
Enforce) which is a property of the CSPDirectiveList. This is what we're setting
here. But then there's also SecurityViolationReportingPolicy (with values
SuppressReporting or Report), which is a param to allowScriptFromSource, and
we're always passing SuppressReporting in the tests. That has the effect that
CSPDirectiveList::allowScriptFromSource calls CSPDirectiveList::checkSource
rather than CSPDirectiveList::checkSourceAndReportViolation, and checkSource
doesn't care about the HeaderType, only ..AndReportViolation does.

I'm not sure how all of that fits together to make sense. What are those two
enums?

+              directiveList->allowScriptFromSource(
+                  resource, String(), integrityMetadata, ParserInserted,
+                  ResourceRequest::RedirectStatus::NoRedirect,
+                  SecurityViolationReportingPolicy::SuppressReporting));
+
+    // Enforce 'script-src'
+    directiveList = createList(String("script-src ") + test.list,
+                               ContentSecurityPolicyHeaderTypeEnforce);
+    EXPECT_EQ(test.expected,
+              directiveList->allowScriptFromSource(
+                  resource, String(), integrityMetadata, ParserInserted,
+                  ResourceRequest::RedirectStatus::NoRedirect,
+                  SecurityViolationReportingPolicy::SuppressReporting));
+  }
+}
+
 TEST_F(CSPDirectiveListTest, allowRequestWithoutIntegrity) {
   struct TestCase {
     const char* list;
     const char* url;
     const WebURLRequest::RequestContext context;
     bool expected;
   } cases[] = {
 
       {"require-sri-for script", "https://example.com/file",
        WebURLRequest::RequestContextScript, false},
@@ skipping 726 matching lines @@
         CSPDirectiveList::getSourceVector(test.directive, policyVector).size(),
         udpatedTotal);
     EXPECT_EQ(CSPDirectiveList::getSourceVector(
                   ContentSecurityPolicy::DirectiveType::ChildSrc, policyVector)
                   .size(),
               expectedChildSrc);
   }
 }
 
 }  // namespace blink