CSP: Separate parsing and applying in Blink CSP code

This is a small refactoring of the code in blink::CSPDirectiveList
which is parsing new Content Security Policies. This change separates
the raw parsing of the policies from the side effects (applying the
policies and storing state).

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: Iace800247ef6165a47adef2601d336a09e930897
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2601368
Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843562}

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

@@ -155,8 +155,7 @@ using network::mojom::ContentSecurityPolicyType;
 
 CSPDirectiveList::CSPDirectiveList(ContentSecurityPolicy* policy)
     : policy_(policy),
-      has_sandbox_policy_(false),
-      strict_mixed_content_checking_enforced_(false),
+      block_all_mixed_content_(false),
       upgrade_insecure_requests_(false),
       use_reporting_api_(false) {}
 
@@ -173,25 +172,67 @@ CSPDirectiveList* CSPDirectiveList::Create(ContentSecurityPolicy* policy,
 
   directives->Parse(begin, end, should_parse_wasm_eval);
 
+  directives->ApplyParsedDirectives();
+
+  return directives;
+}
+
+void CSPDirectiveList::ApplyParsedDirectives() {
+  if (block_all_mixed_content_ && !IsReportOnly())
+    policy_->EnforceStrictMixedContentChecking();
+
+  if (RequiresTrustedTypes())
+    policy_->RequireTrustedTypes();
+
+  policy_->EnforceSandboxFlags(sandbox_flags_);
+
+  if (upgrade_insecure_requests_)
+    policy_->UpgradeInsecureRequests();
+
+  for (const auto& directive : directives_) {
+    switch (directive.key) {
+      case CSPDirectiveName::DefaultSrc:
+        // TODO(mkwst) It seems unlikely that developers would use different
+        // algorithms for scripts and styles. We may want to combine the
+        // usesScriptHashAlgorithms() and usesStyleHashAlgorithms.
+        policy_->UsesScriptHashAlgorithms(
+            HashAlgorithmsUsed(directive.value.get()));
+        policy_->UsesStyleHashAlgorithms(
+            HashAlgorithmsUsed(directive.value.get()));
+        break;
+      case CSPDirectiveName::ScriptSrc:
+      case CSPDirectiveName::ScriptSrcAttr:
+      case CSPDirectiveName::ScriptSrcElem:
+        policy_->UsesScriptHashAlgorithms(
+            HashAlgorithmsUsed(directive.value.get()));
+        break;
+      case CSPDirectiveName::StyleSrc:
+      case CSPDirectiveName::StyleSrcAttr:
+      case CSPDirectiveName::StyleSrcElem:
+        policy_->UsesStyleHashAlgorithms(
+            HashAlgorithmsUsed(directive.value.get()));
+        break;
+      default:
+        break;
+    }
+  }
+
   CSPOperativeDirective directive =
-      directives->OperativeDirective(CSPDirectiveName::ScriptSrc);
+      OperativeDirective(CSPDirectiveName::ScriptSrc);
 
-  if (!directives->CheckEval(directive.source_list)) {
+  if (!CheckEval(directive.source_list)) {
     String message =
         "Refused to evaluate a string as JavaScript because 'unsafe-eval' is "
         "not an allowed source of script in the following Content Security "
         "Policy directive: \"" +
-        GetRawDirectiveForMessage(directives->raw_directives_, directive.type) +
-        "\".\n";
-    directives->SetEvalDisabledErrorMessage(message);
-  } else if (directives->RequiresTrustedTypes()) {
+        GetRawDirectiveForMessage(raw_directives_, directive.type) + "\".\n";
+    SetEvalDisabledErrorMessage(message);
+  } else if (RequiresTrustedTypes()) {
     String message =
         "Refused to evaluate a string as JavaScript because this document "
         "requires 'Trusted Type' assignment.";
-    directives->SetEvalDisabledErrorMessage(message);
+    SetEvalDisabledErrorMessage(message);
   }
-
-  return directives;
 }
 
 void CSPDirectiveList::ReportViolation(
@@ -967,6 +1008,9 @@ bool CSPDirectiveList::ParseDirective(const UChar* begin,
 }
 
 void CSPDirectiveList::ParseReportTo(const String& name, const String& value) {
+  if (!base::FeatureList::IsEnabled(network::features::kReporting))
+    return;
+
   if (!use_reporting_api_) {
     use_reporting_api_ = true;
     report_endpoints_.clear();
@@ -1081,7 +1125,7 @@ void CSPDirectiveList::ParseAndAppendReportEndpoints(const String& value) {
                      : WebFeature::kReportUriSingleEndpoint);
 }
 
-void CSPDirectiveList::ApplySandboxPolicy(const String& name,
+void CSPDirectiveList::ParseSandboxPolicy(const String& name,
                                           const String& sandbox_policy) {
   // Remove sandbox directives in meta policies, per
   // https://www.w3.org/TR/CSP2/#delivery-html-meta-element.
@@ -1093,10 +1137,6 @@ void CSPDirectiveList::ApplySandboxPolicy(const String& name,
     policy_->ReportInvalidInReportOnly(name);
     return;
   }
-  if (has_sandbox_policy_) {
-    policy_->ReportDuplicateDirective(name);
-    return;
-  }
 
   using network::mojom::blink::WebSandboxFlags;
   WebSandboxFlags ignored_flags =
@@ -1104,17 +1144,16 @@ void CSPDirectiveList::ApplySandboxPolicy(const String& name,
           ? WebSandboxFlags::kStorageAccessByUserActivation
           : WebSandboxFlags::kNone;
 
-  has_sandbox_policy_ = true;
   network::WebSandboxFlagsParsingResult parsed =
       network::ParseWebSandboxPolicy(sandbox_policy.Utf8(), ignored_flags);
-  policy_->EnforceSandboxFlags(parsed.flags);
+  sandbox_flags_ = parsed.flags;
   if (!parsed.error_message.empty()) {
     policy_->ReportInvalidSandboxFlags(
         WebString::FromUTF8(parsed.error_message));
   }
 }
 
-void CSPDirectiveList::ApplyTreatAsPublicAddress() {
+void CSPDirectiveList::ParseTreatAsPublicAddress() {
   // Remove treat-as-public-address directives in meta policies, per
   // https://wicg.github.io/cors-rfc1918/#csp
   if (header_->source == ContentSecurityPolicySource::kMeta) {
@@ -1133,34 +1172,22 @@ void CSPDirectiveList::ApplyTreatAsPublicAddress() {
   // browser process.
 }
 
-void CSPDirectiveList::EnforceStrictMixedContentChecking(const String& name,
+void CSPDirectiveList::ParseBlockAllMixedContent(const String& name,
                                                  const String& value) {
-  if (strict_mixed_content_checking_enforced_) {
-    policy_->ReportDuplicateDirective(name);
-    return;
-  }
   if (!value.IsEmpty())
     policy_->ReportValueForEmptyDirective(name, value);
 
-  strict_mixed_content_checking_enforced_ = true;
-
-  if (!IsReportOnly())
-    policy_->EnforceStrictMixedContentChecking();
+  block_all_mixed_content_ = true;
 }
 
-void CSPDirectiveList::EnableInsecureRequestsUpgrade(const String& name,
+void CSPDirectiveList::ParseUpgradeInsecureRequests(const String& name,
                                                     const String& value) {
   if (IsReportOnly()) {
     policy_->ReportInvalidInReportOnly(name);
     return;
   }
-  if (upgrade_insecure_requests_) {
-    policy_->ReportDuplicateDirective(name);
-    return;
-  }
   upgrade_insecure_requests_ = true;
 
-  policy_->UpgradeInsecureRequests();
   if (!value.IsEmpty())
     policy_->ReportValueForEmptyDirective(name, value);
 }
@@ -1184,26 +1211,24 @@ void CSPDirectiveList::AddDirective(const String& name, const String& value) {
 
   switch (type) {
     case CSPDirectiveName::BaseURI:
-      directives_.insert(type, CSPSourceListParse(name, value, policy_));
-      return;
-    case CSPDirectiveName::BlockAllMixedContent:
-      EnforceStrictMixedContentChecking(name, value);
-      return;
     case CSPDirectiveName::ChildSrc:
     case CSPDirectiveName::ConnectSrc:
-      directives_.insert(type, CSPSourceListParse(name, value, policy_));
-      return;
     case CSPDirectiveName::DefaultSrc:
-      source_list = CSPSourceListParse(name, value, policy_);
-      // TODO(mkwst) It seems unlikely that developers would use different
-      // algorithms for scripts and styles. We may want to combine the
-      // usesScriptHashAlgorithms() and usesStyleHashAlgorithms.
-      policy_->UsesScriptHashAlgorithms(HashAlgorithmsUsed(source_list.get()));
-      policy_->UsesStyleHashAlgorithms(HashAlgorithmsUsed(source_list.get()));
-      directives_.insert(type, std::move(source_list));
-      return;
     case CSPDirectiveName::FontSrc:
     case CSPDirectiveName::FormAction:
+    case CSPDirectiveName::FrameSrc:
+    case CSPDirectiveName::ImgSrc:
+    case CSPDirectiveName::ManifestSrc:
+    case CSPDirectiveName::MediaSrc:
+    case CSPDirectiveName::NavigateTo:
+    case CSPDirectiveName::ObjectSrc:
+    case CSPDirectiveName::ScriptSrc:
+    case CSPDirectiveName::ScriptSrcAttr:
+    case CSPDirectiveName::ScriptSrcElem:
+    case CSPDirectiveName::StyleSrc:
+    case CSPDirectiveName::StyleSrcAttr:
+    case CSPDirectiveName::StyleSrcElem:
+    case CSPDirectiveName::WorkerSrc:
       directives_.insert(type, CSPSourceListParse(name, value, policy_));
       return;
     case CSPDirectiveName::FrameAncestors:
@@ -1215,25 +1240,19 @@ void CSPDirectiveList::AddDirective(const String& name, const String& value) {
         directives_.insert(type, CSPSourceListParse(name, value, policy_));
       }
       return;
-    case CSPDirectiveName::FrameSrc:
-    case CSPDirectiveName::ImgSrc:
-    case CSPDirectiveName::ManifestSrc:
-    case CSPDirectiveName::MediaSrc:
-    case CSPDirectiveName::NavigateTo:
-    case CSPDirectiveName::ObjectSrc:
-      directives_.insert(type, CSPSourceListParse(name, value, policy_));
-      return;
-    case CSPDirectiveName::PluginTypes:
-      plugin_types_ = CSPPluginTypesParse(value, policy_);
-      return;
     case CSPDirectiveName::PrefetchSrc:
       if (!policy_->ExperimentalFeaturesEnabled())
         policy_->ReportUnsupportedDirective(name);
       else
         directives_.insert(type, CSPSourceListParse(name, value, policy_));
       return;
+    case CSPDirectiveName::BlockAllMixedContent:
+      ParseBlockAllMixedContent(name, value);
+      return;
+    case CSPDirectiveName::PluginTypes:
+      plugin_types_ = CSPPluginTypesParse(value, policy_);
+      return;
     case CSPDirectiveName::ReportTo:
-      if (base::FeatureList::IsEnabled(network::features::kReporting))
       ParseReportTo(name, value);
       return;
     case CSPDirectiveName::ReportURI:
@@ -1242,41 +1261,22 @@ void CSPDirectiveList::AddDirective(const String& name, const String& value) {
     case CSPDirectiveName::RequireTrustedTypesFor:
       require_trusted_types_for_ =
           CSPRequireTrustedTypesForParse(value, policy_);
-      if (RequiresTrustedTypes())
-        policy_->RequireTrustedTypes();
       return;
     case CSPDirectiveName::Sandbox:
-      ApplySandboxPolicy(name, value);
-      return;
-    case CSPDirectiveName::ScriptSrc:
-    case CSPDirectiveName::ScriptSrcAttr:
-    case CSPDirectiveName::ScriptSrcElem:
-      source_list = CSPSourceListParse(name, value, policy_);
-      policy_->UsesScriptHashAlgorithms(HashAlgorithmsUsed(source_list.get()));
-      directives_.insert(type, std::move(source_list));
-      return;
-    case CSPDirectiveName::StyleSrc:
-    case CSPDirectiveName::StyleSrcAttr:
-    case CSPDirectiveName::StyleSrcElem:
-      source_list = CSPSourceListParse(name, value, policy_);
-      policy_->UsesStyleHashAlgorithms(HashAlgorithmsUsed(source_list.get()));
-      directives_.insert(type, std::move(source_list));
+      ParseSandboxPolicy(name, value);
       return;
     case CSPDirectiveName::TreatAsPublicAddress:
-      ApplyTreatAsPublicAddress();
+      ParseTreatAsPublicAddress();
       return;
     case CSPDirectiveName::TrustedTypes:
       trusted_types_ = CSPTrustedTypesParse(value, policy_);
       return;
     case CSPDirectiveName::UpgradeInsecureRequests:
-      EnableInsecureRequestsUpgrade(name, value);
+      ParseUpgradeInsecureRequests(name, value);
       return;
     case CSPDirectiveName::Unknown:
       NOTREACHED();
       return;
-    case CSPDirectiveName::WorkerSrc:
-      directives_.insert(type, CSPSourceListParse(name, value, policy_));
-      return;
   }
 }
 

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

@@ -100,9 +100,7 @@ class CORE_EXPORT CSPDirectiveList final
                                          const String& sample,
                                          const String& sample_prefix) const;
 
-  bool StrictMixedContentChecking() const {
-    return strict_mixed_content_checking_enforced_;
-  }
+  bool StrictMixedContentChecking() const { return block_all_mixed_content_; }
   void ReportMixedContent(const KURL& blocked_url,
                           ResourceRequest::RedirectStatus) const;
 
@@ -166,6 +164,7 @@ class CORE_EXPORT CSPDirectiveList final
   FRIEND_TEST_ALL_PREFIXES(CSPDirectiveListTest, IsMatchingNoncePresent);
   FRIEND_TEST_ALL_PREFIXES(CSPDirectiveListTest, OperativeDirectiveGivenType);
 
+  void ApplyParsedDirectives();
   bool ParseDirective(const UChar* begin,
                       const UChar* end,
                       String* name,
@@ -175,11 +174,10 @@ class CORE_EXPORT CSPDirectiveList final
   void ParseAndAppendReportEndpoints(const String& value);
   void ParsePluginTypes(const String& name, const String& value);
   void AddDirective(const String& name, const String& value);
-  void ApplySandboxPolicy(const String& name, const String& sandbox_policy);
-  void ApplyTreatAsPublicAddress();
-  void EnforceStrictMixedContentChecking(const String& name,
-                                         const String& value);
-  void EnableInsecureRequestsUpgrade(const String& name, const String& value);
+  void ParseSandboxPolicy(const String& name, const String& sandbox_policy);
+  void ParseTreatAsPublicAddress();
+  void ParseBlockAllMixedContent(const String& name, const String& value);
+  void ParseUpgradeInsecureRequests(const String& name, const String& value);
 
   CSPDirectiveName FallbackDirective(CSPDirectiveName current_directive,
                                      CSPDirectiveName original_directive) const;
@@ -279,9 +277,9 @@ class CORE_EXPORT CSPDirectiveList final
 
   HashMap<CSPDirectiveName, String> raw_directives_;
 
-  bool has_sandbox_policy_;
+  network::mojom::blink::WebSandboxFlags sandbox_flags_;
 
-  bool strict_mixed_content_checking_enforced_;
+  bool block_all_mixed_content_;
 
   bool upgrade_insecure_requests_;