CSP: Replace blink::SourceListDirective with mojo type

In the Blink Content Security Policy code, we switch from using the
internal blink type blink::SourceListDirective to using the mojo type
network::mojom::blink::CSPSourceList for handling lists of CSP source
expressions.

This is part of a project to harmonize the CSP code in Blink and in
services/network, and will make it easier to synchronize Content
Security Policies between the two.

Bug: 1021462,1149272
Change-Id: Ibb860148689399c9b169bb9e604bfed5d5df429f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2560100
Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836104}

services/network/public/cpp/content_security_policy/content_security_policy.cc

@@ -6,8 +6,10 @@
 
 #include <sstream>
 #include <string>
+#include "base/base64url.h"
 #include "base/containers/flat_set.h"
 #include "base/ranges/algorithm.h"
+#include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_split.h"
@@ -563,7 +565,18 @@ bool ParseHash(base::StringPiece expression, mojom::CSPHashSource* hash) {
         return false;
 
       hash->algorithm = item.type;
-      hash->value = subexpression.as_string();
+
+      // We lazily accept both base64url and base64-encoded data.
+      std::string normalized_value;
+      base::ReplaceChars(subexpression, "+", "-", &normalized_value);
+      base::ReplaceChars(normalized_value, "/", "_", &normalized_value);
+
+      std::string out;
+      if (!base::Base64UrlDecode(normalized_value,
+                                 base::Base64UrlDecodePolicy::IGNORE_PADDING,
+                                 &out))
+        return false;
+      hash->value = std::vector<uint8_t>(out.begin(), out.end());
       return true;
     }
   }

services/network/public/cpp/content_security_policy/content_security_policy_unittest.cc

@@ -1273,25 +1273,16 @@ TEST(ContentSecurityPolicy, ParseSerializedSourceList) {
           "",
       },
       {
-          "'sha256-abc' 'sha256-ABC' 'sha256 'sha256-' 'sha384-abc' "
-          "'sha512-abc' 'sha-abc' 'sha256-*' 'sha-256-cde' 'sha-384-cde' "
-          "'sha-512-cde'",
+          "'sha256-YWJj' 'nonce-cde' 'sha256-QUJD'",
           base::Bind([] {
             auto csp = mojom::CSPSourceList::New();
-            csp->hashes.push_back(mojom::CSPHashSource::New(
-                mojom::CSPHashAlgorithm::SHA256, "abc"));
-            csp->hashes.push_back(mojom::CSPHashSource::New(
-                mojom::CSPHashAlgorithm::SHA256, "ABC"));
-            csp->hashes.push_back(mojom::CSPHashSource::New(
-                mojom::CSPHashAlgorithm::SHA384, "abc"));
-            csp->hashes.push_back(mojom::CSPHashSource::New(
-                mojom::CSPHashAlgorithm::SHA512, "abc"));
-            csp->hashes.push_back(mojom::CSPHashSource::New(
-                mojom::CSPHashAlgorithm::SHA256, "cde"));
-            csp->hashes.push_back(mojom::CSPHashSource::New(
-                mojom::CSPHashAlgorithm::SHA384, "cde"));
-            csp->hashes.push_back(mojom::CSPHashSource::New(
-                mojom::CSPHashAlgorithm::SHA512, "cde"));
+            csp->hashes.push_back(
+                mojom::CSPHashSource::New(mojom::CSPHashAlgorithm::SHA256,
+                                          std::vector<uint8_t>{'a', 'b', 'c'}));
+            csp->hashes.push_back(
+                mojom::CSPHashSource::New(mojom::CSPHashAlgorithm::SHA256,
+                                          std::vector<uint8_t>{'A', 'B', 'C'}));
+            csp->nonces.push_back("cde");
             return csp;
           }),
           "",
@@ -1426,6 +1417,58 @@ TEST(ContentSecurityPolicy, ParseSerializedSourceList) {
   }
 }
 
+TEST(ContentSecurityPolicy, ParseHash) {
+  using Algo = mojom::CSPHashAlgorithm;
+  struct TestCase {
+    std::string hash;
+    Algo expected_algorithm;
+    std::vector<uint8_t> expected_hash;
+  } cases[] = {
+      // For this test, we have the following base64 encoding:
+      // abc => YWJj    ABC => QUJD    cd => Y2Q=    abcd => YWJjZA==
+      // We also test base64 without padding.
+      {"'sha256-YWJj'", Algo::SHA256, {'a', 'b', 'c'}},
+      {"'sha256-QUJD'", Algo::SHA256, {'A', 'B', 'C'}},
+      {"'sha256", Algo::None, {}},
+      {"'sha256-'", Algo::None, {}},
+      {"'sha384-YWJj'", Algo::SHA384, {'a', 'b', 'c'}},
+      {"'sha512-YWJjZA'", Algo::SHA512, {'a', 'b', 'c', 'd'}},
+      {"'sha-YWJj'", Algo::None, {}},
+      {"'sha256-*'", Algo::None, {}},
+      {"'sha-256-Y2Q'", Algo::SHA256, {'c', 'd'}},
+      {"'sha-384-Y2Q='", Algo::SHA384, {'c', 'd'}},
+      {"'sha-512-Y2Q='", Algo::SHA512, {'c', 'd'}},
+      // "ABCDE" is not valid base64 and should be ignored.
+      {"'sha256-ABCDE'", Algo::None, {}},
+      {"'sha256--__'", Algo::SHA256, {0xfb, 0xff}},
+      {"'sha256-++/'", Algo::SHA256, {0xfb, 0xef}},
+      // Other invalid hashes should be ignored.
+      {"'sha256-YWJj", Algo::None, {}},
+      {"'sha111-YWJj'", Algo::None, {}},
+      {"'sha256-ABC('", Algo::None, {}},
+  };
+
+  for (auto& test : cases) {
+    scoped_refptr<net::HttpResponseHeaders> headers(
+        new net::HttpResponseHeaders("HTTP/1.1 200 OK"));
+    headers->SetHeader("Content-Security-Policy", "script-src " + test.hash);
+    std::vector<mojom::ContentSecurityPolicyPtr> policies;
+    AddContentSecurityPolicyFromHeaders(*headers, GURL("https://example.com/"),
+                                        &policies);
+    const std::vector<mojom::CSPHashSourcePtr>& hashes =
+        policies[0]->directives[mojom::CSPDirectiveName::ScriptSrc]->hashes;
+    if (test.expected_algorithm != Algo::None) {
+      EXPECT_EQ(1u, hashes.size()) << test.hash << " should parse to one hash";
+      EXPECT_EQ(test.expected_algorithm, hashes[0]->algorithm)
+          << test.hash << " should have algorithm " << test.expected_algorithm;
+      EXPECT_EQ(test.expected_hash, hashes[0]->value)
+          << test.hash << " has not been base64decoded correctly";
+    } else {
+      EXPECT_TRUE(hashes.empty()) << test.hash << " should be an invalid hash";
+    }
+  }
+}
+
 TEST(ContentSecurityPolicy, IsValidRequiredCSPAttr) {
   struct TestCase {
     const char* csp;

services/network/public/mojom/content_security_policy.mojom

@@ -64,14 +64,15 @@ struct CSPSource {
 };
 
 enum CSPHashAlgorithm {
-  SHA256,
-  SHA384,
-  SHA512,
+  None = 0,
+  SHA256 = 1,
+  SHA384 = 2,
+  SHA512 = 4,
 };
 
 struct CSPHashSource {
   CSPHashAlgorithm algorithm;
-  string value;
+  array<uint8> value;
 };
 
 struct CSPSourceList {

third_party/blink/renderer/core/frame/csp/content_security_policy.cc

@@ -452,17 +452,17 @@ Vector<CSPHeaderAndType> ContentSecurityPolicy::Headers() const {
 void ContentSecurityPolicy::FillInCSPHashValues(
     const String& source,
     uint8_t hash_algorithms_used,
-    Vector<CSPHashValue>* csp_hash_values) {
+    Vector<network::mojom::blink::CSPHashSourcePtr>& csp_hash_values) {
   // Any additions or subtractions from this struct should also modify the
   // respective entries in the kSupportedPrefixes array in
   // SourceListDirective::parseHash().
   static const struct {
-    ContentSecurityPolicyHashAlgorithm csp_hash_algorithm;
+    network::mojom::blink::CSPHashAlgorithm csp_hash_algorithm;
     HashAlgorithm algorithm;
   } kAlgorithmMap[] = {
-      {kContentSecurityPolicyHashAlgorithmSha256, kHashAlgorithmSha256},
-      {kContentSecurityPolicyHashAlgorithmSha384, kHashAlgorithmSha384},
-      {kContentSecurityPolicyHashAlgorithmSha512, kHashAlgorithmSha512}};
+      {network::mojom::blink::CSPHashAlgorithm::SHA256, kHashAlgorithmSha256},
+      {network::mojom::blink::CSPHashAlgorithm::SHA384, kHashAlgorithmSha384},
+      {network::mojom::blink::CSPHashAlgorithm::SHA512, kHashAlgorithmSha512}};
 
   // Only bother normalizing the source/computing digests if there are any
   // checks to be done.
@@ -474,13 +474,14 @@ void ContentSecurityPolicy::FillInCSPHashValues(
 
   for (const auto& algorithm_map : kAlgorithmMap) {
     DigestValue digest;
-    if (algorithm_map.csp_hash_algorithm & hash_algorithms_used) {
+    if (static_cast<int32_t>(algorithm_map.csp_hash_algorithm) &
+        hash_algorithms_used) {
       bool digest_success =
           ComputeDigest(algorithm_map.algorithm, utf8_source.data(),
                         utf8_source.size(), digest);
       if (digest_success) {
-        csp_hash_values->push_back(
-            CSPHashValue(algorithm_map.csp_hash_algorithm, digest));
+        csp_hash_values.push_back(network::mojom::blink::CSPHashSource::New(
+            algorithm_map.csp_hash_algorithm, Vector<uint8_t>(digest)));
       }
     }
   }
@@ -488,11 +489,11 @@ void ContentSecurityPolicy::FillInCSPHashValues(
 
 // static
 bool ContentSecurityPolicy::CheckHashAgainstPolicy(
-    Vector<CSPHashValue>& csp_hash_values,
+    Vector<network::mojom::blink::CSPHashSourcePtr>& csp_hash_values,
     const Member<CSPDirectiveList>& policy,
     InlineType inline_type) {
   for (const auto& csp_hash_value : csp_hash_values) {
-    if (policy->AllowHash(csp_hash_value, inline_type))
+    if (policy->AllowHash(*csp_hash_value, inline_type))
       return true;
   }
   return false;
@@ -515,11 +516,11 @@ bool ContentSecurityPolicy::AllowInline(
     return true;
   }
 
-  Vector<CSPHashValue> csp_hash_values;
+  Vector<network::mojom::blink::CSPHashSourcePtr> csp_hash_values;
   FillInCSPHashValues(
       content,
       is_script ? script_hash_algorithms_used_ : style_hash_algorithms_used_,
-      &csp_hash_values);
+      csp_hash_values);
 
   // Step 2. Let result be "Allowed". [spec text]
   bool is_allowed = true;

third_party/blink/renderer/core/frame/csp/content_security_policy.h

@@ -551,13 +551,15 @@ class CORE_EXPORT ContentSecurityPolicy final
                        const IntegrityMetadataSet& = IntegrityMetadataSet(),
                        ParserDisposition = kParserInserted) const;
 
-  static void FillInCSPHashValues(const String& source,
+  static void FillInCSPHashValues(
+      const String& source,
       uint8_t hash_algorithms_used,
-                                  Vector<CSPHashValue>* csp_hash_values);
+      Vector<network::mojom::blink::CSPHashSourcePtr>& csp_hash_values);
 
   // checks a vector of csp hashes against policy, probably a good idea
   // to use in tandem with FillInCSPHashValues.
-  static bool CheckHashAgainstPolicy(Vector<CSPHashValue>&,
+  static bool CheckHashAgainstPolicy(
+      Vector<network::mojom::blink::CSPHashSourcePtr>&,
       const Member<CSPDirectiveList>&,
       InlineType);
 

third_party/blink/renderer/core/frame/csp/csp_directive_list_test.cc

@@ -137,8 +137,8 @@ TEST_F(CSPDirectiveListTest, IsMatchingNoncePresent) {
     // Report-only
     Member<CSPDirectiveList> directive_list =
         CreateList(test.list, ContentSecurityPolicyType::kReport);
-    Member<SourceListDirective> directive =
-        directive_list->OperativeDirective(test.type);
+    const network::mojom::blink::CSPSourceList* directive =
+        directive_list->OperativeDirective(test.type).source_list;
     EXPECT_EQ(test.expected,
               directive_list->IsMatchingNoncePresent(directive, test.nonce));
     // Empty/null strings are always not present, regardless of the policy.
@@ -147,7 +147,7 @@ TEST_F(CSPDirectiveListTest, IsMatchingNoncePresent) {
 
     // Enforce
     directive_list = CreateList(test.list, ContentSecurityPolicyType::kEnforce);
-    directive = directive_list->OperativeDirective(test.type);
+    directive = directive_list->OperativeDirective(test.type).source_list;
     EXPECT_EQ(test.expected,
               directive_list->IsMatchingNoncePresent(directive, test.nonce));
     // Empty/null strings are always not present, regardless of the policy.
@@ -599,7 +599,7 @@ TEST_F(CSPDirectiveListTest, OperativeDirectiveGivenType) {
   for (auto& test : cases) {
     // With an empty directive list the returned directive should always be
     // null.
-    EXPECT_FALSE(empty->OperativeDirective(test.directive));
+    EXPECT_FALSE(empty->OperativeDirective(test.directive).source_list);
 
     // Add the directive itself as it should be the first one to be returned.
     test.fallback_list.push_front(test.directive);
@@ -611,16 +611,14 @@ TEST_F(CSPDirectiveListTest, OperativeDirectiveGivenType) {
       directive_list = CreateList(directive_string.c_str(),
                                   ContentSecurityPolicyType::kEnforce);
 
-      CSPDirective* operative_directive =
+      CSPOperativeDirective operative_directive =
           directive_list->OperativeDirective(test.directive);
 
       // We should have an actual directive returned here.
-      EXPECT_TRUE(operative_directive);
+      EXPECT_TRUE(operative_directive.source_list);
 
       // The OperativeDirective should be first one in the fallback chain.
-      EXPECT_EQ(test.fallback_list.front(),
-                ContentSecurityPolicy::GetDirectiveType(
-                    operative_directive->GetName()));
+      EXPECT_EQ(test.fallback_list.front(), operative_directive.type);
 
       // Remove the first directive in the fallback chain from the directive
       // list and continue by testing that the next one is returned until we
@@ -643,7 +641,8 @@ TEST_F(CSPDirectiveListTest, OperativeDirectiveGivenType) {
     // the fallback chain that is returned.
     directive_list = CreateList(directive_string.c_str(),
                                 ContentSecurityPolicyType::kEnforce);
-    EXPECT_FALSE(directive_list->OperativeDirective(test.directive));
+    EXPECT_FALSE(
+        directive_list->OperativeDirective(test.directive).source_list);
   }
 }
 

third_party/blink/renderer/core/frame/csp/source_list_directive.h

@@ -7,121 +7,58 @@
 
 #include "base/macros.h"
 #include "third_party/blink/renderer/core/core_export.h"
-#include "third_party/blink/renderer/core/frame/csp/csp_directive.h"
 #include "third_party/blink/renderer/core/frame/csp/csp_source.h"
-#include "third_party/blink/renderer/platform/crypto.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_request.h"
-#include "third_party/blink/renderer/platform/network/content_security_policy_parsers.h"
-#include "third_party/blink/renderer/platform/wtf/hash_set.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
 
-class ContentSecurityPolicy;
 class KURL;
 
-class CORE_EXPORT SourceListDirective final : public CSPDirective {
- public:
-  SourceListDirective(const String& name,
+CORE_EXPORT
+network::mojom::blink::CSPSourceListPtr CSPSourceListParse(
+    const String& name,
     const String& value,
-                      ContentSecurityPolicy*);
-  void Trace(Visitor*) const override;
+    ContentSecurityPolicy* policy);
 
-  void Parse(const UChar* begin, const UChar* end);
-
-  bool Matches(const KURL&,
-               ResourceRequest::RedirectStatus =
-                   ResourceRequest::RedirectStatus::kNoRedirect) const;
-
-  bool Allows(const KURL&,
+CORE_EXPORT
+bool CSPSourceListAllows(
+    const network::mojom::blink::CSPSourceList& source_list,
+    const network::mojom::blink::CSPSource& self_source,
+    const KURL&,
     ResourceRequest::RedirectStatus =
-                  ResourceRequest::RedirectStatus::kNoRedirect) const;
-  bool AllowInline() const;
-  bool AllowEval() const;
-  bool AllowWasmEval() const;
-  bool AllowDynamic() const;
-  bool AllowNonce(const String& nonce) const;
-  bool AllowHash(const CSPHashValue&) const;
-  bool AllowUnsafeHashes() const;
-  bool AllowReportSample() const;
-  bool IsNone() const;
-  bool IsSelf() const;
-  bool IsHashOrNoncePresent() const;
-  uint8_t HashAlgorithmsUsed() const;
-  bool AllowAllInline() const;
-  bool AllowsURLBasedMatching() const;
-
-  // The algorithm is described more extensively here:
-  // https://w3c.github.io/webappsec-csp/embedded/#subsume-source-list
-  bool Subsumes(const HeapVector<Member<SourceListDirective>>&) const;
-
-  // Export a subset of the source list that affect navigation.
-  // It contains every source-expressions, '*', 'none' and 'self'.
-  // It doesn't contain 'unsafe-inline' or 'unsafe-eval' for instance.
-  network::mojom::blink::CSPSourceListPtr ExposeForNavigationalChecks() const;
-  String DirectiveName() const { return directive_name_; }
-
- private:
-  FRIEND_TEST_ALL_PREFIXES(SourceListDirectiveTest, ParseHost);
-  FRIEND_TEST_ALL_PREFIXES(CSPDirectiveListTest, OperativeDirectiveGivenType);
-
-  bool ParseSource(const UChar* begin,
-                   const UChar* end,
-                   String* scheme,
-                   String* host,
-                   int* port,
-                   String* path,
-                   bool* is_host_wildcard,
-                   bool* is_port_wildcard);
-  bool ParseScheme(const UChar* begin, const UChar* end, String* scheme);
-  static bool ParseHost(const UChar* begin,
-                        const UChar* end,
-                        String* host,
-                        bool* is_host_wildcard);
-  bool ParsePort(const UChar* begin,
-                 const UChar* end,
-                 int* port,
-                 bool* is_port_wildcard);
-  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,
-                 DigestValue* hash,
-                 ContentSecurityPolicyHashAlgorithm*);
-
-  void AddSourceSelf();
-  void AddSourceStar();
-  void AddSourceUnsafeAllowRedirects();
-  void AddSourceUnsafeInline();
-  void AddSourceUnsafeEval();
-  void AddSourceWasmEval();
-  void AddSourceStrictDynamic();
-  void AddSourceUnsafeHashes();
-  void AddReportSample();
-  void AddSourceNonce(const String& nonce);
-  void AddSourceHash(const ContentSecurityPolicyHashAlgorithm&,
-                     const DigestValue& hash);
-
-  bool HasSourceMatchInList(const KURL&, ResourceRequest::RedirectStatus) const;
-
-  Member<ContentSecurityPolicy> policy_;
-  WTF::Vector<network::mojom::blink::CSPSourcePtr> list_;
-  String directive_name_;
-  bool allow_self_;
-  bool allow_star_;
-  bool allow_inline_;
-  bool allow_eval_;
-  bool allow_wasm_eval_;
-  bool allow_dynamic_;
-  bool allow_unsafe_hashes_;
-  bool allow_redirects_;
-  bool report_sample_;
-  HashSet<String> nonces_;
-  HashSet<CSPHashValue> hashes_;
-  uint8_t hash_algorithms_used_;
-
-  DISALLOW_COPY_AND_ASSIGN(SourceListDirective);
-};
+        ResourceRequest::RedirectStatus::kNoRedirect);
+
+CORE_EXPORT
+bool CSPSourceListAllowNonce(
+    const network::mojom::blink::CSPSourceList& source_list,
+    const String& nonce);
+
+CORE_EXPORT
+bool CSPSourceListAllowHash(
+    const network::mojom::blink::CSPSourceList& source_list,
+    const network::mojom::blink::CSPHashSource& hash);
+
+CORE_EXPORT
+bool CSPSourceListIsNone(
+    const network::mojom::blink::CSPSourceList& source_list);
+
+CORE_EXPORT
+bool CSPSourceListIsSelf(
+    const network::mojom::blink::CSPSourceList& source_list);
+
+CORE_EXPORT
+bool CSPSourceListIsHashOrNoncePresent(
+    const network::mojom::blink::CSPSourceList& source_list);
+
+CORE_EXPORT
+bool CSPSourceListAllowAllInline(
+    ContentSecurityPolicy::DirectiveType directive_type,
+    const network::mojom::blink::CSPSourceList& source_list);
+
+CORE_EXPORT
+bool CSPSourceListAllowsURLBasedMatching(
+    const network::mojom::blink::CSPSourceList& source_list);
 
 }  // namespace blink
 

third_party/blink/renderer/platform/network/http_parsers.cc

@@ -66,8 +66,11 @@ blink::CSPSourcePtr ConvertToBlink(CSPSourcePtr source) {
 }
 
 blink::CSPHashSourcePtr ConvertToBlink(CSPHashSourcePtr hash) {
-  return blink::CSPHashSource::New(hash->algorithm,
-                                   String::FromUTF8(hash->value));
+  WTF::Vector<uint8_t> hash_value;
+  for (uint8_t el : hash->value)
+    hash_value.push_back(el);
+
+  return blink::CSPHashSource::New(hash->algorithm, hash_value);
 }
 
 blink::CSPSourceListPtr ConvertToBlink(CSPSourceListPtr source_list) {