CSP: Create multiple CSP policies when parsing multiple headers.

Bug: 759184
Change-Id: I59e05579e1a4dda4dcbf3f2dff7b314b8a812c33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1960545
Commit-Queue: Lucas Gadani <lfg@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#725148}

content/browser/cache_storage/cache_storage_cache_unittest.cc

@@ -498,7 +498,7 @@ class CacheStorageCacheTest : public testing::Test {
         std::vector<std::string>() /* cors_exposed_header_names */,
         nullptr /* side_data_blob */,
         nullptr /* side_data_blob_for_cache_put */,
-        nullptr /* content_security_policy */,
+        std::vector<network::mojom::ContentSecurityPolicyPtr>(),
         false /* loaded_with_credentials */);
   }
 

content/browser/cache_storage/cache_storage_manager_unittest.cc

@@ -664,7 +664,7 @@ class CacheStorageManagerTest : public testing::Test {
         std::vector<std::string>() /* cors_exposed_header_names */,
         nullptr /* side_data_blob */,
         nullptr /* side_data_blob_for_cache_put */,
-        nullptr /* content_security_policy */,
+        std::vector<network::mojom::ContentSecurityPolicyPtr>(),
         false /* loaded_with_credentials */);
 
     blink::mojom::BatchOperationPtr operation =

content/browser/cache_storage/legacy/legacy_cache_storage_cache.cc

@@ -357,7 +357,7 @@ blink::mojom::FetchAPIResponsePtr CreateResponse(
           metadata.response().cors_exposed_header_names().begin(),
           metadata.response().cors_exposed_header_names().end()),
       nullptr /* side_data_blob */, nullptr /* side_data_blob_for_cache_put */,
-      nullptr /* content_security_policy */,
+      std::vector<network::mojom::ContentSecurityPolicyPtr>(),
       metadata.response().loaded_with_credentials());
 }
 

content/browser/frame_host/ancestor_throttle.cc

@@ -193,14 +193,14 @@ NavigationThrottle::ThrottleCheckResult AncestorThrottle::ProcessResponseImpl(
   // existing content-security-policy on the response.
   if (is_response_check && base::FeatureList::IsEnabled(
                                network::features::kOutOfBlinkFrameAncestors)) {
-    if (auto& policy = request->response()->content_security_policy) {
+    if (!request->response()->content_security_policy.empty()) {
       // TODO(arthursonzogni): Remove content::ContentSecurityPolicy in favor of
       // network::mojom::ContentSecurityPolicy, this will avoid conversion
       // between type here.
-      // TODO(lfg): Pass every ContentSecurityPolicy here instead of one.
-      std::vector<ContentSecurityPolicy> policies = {
-          ContentSecurityPolicy(policy.Clone()),
-      };
+      std::vector<ContentSecurityPolicy> policies;
+      policies.reserve(request->response()->content_security_policy.size());
+      for (auto& policy : request->response()->content_security_policy)
+        policies.push_back(ContentSecurityPolicy(policy.Clone()));
       // TODO(lfg): If the initiating document is known and correspond to the
       // navigating frame's current document, consider using:
       // navigation_request().common_params().source_location here instead.

content/common/background_fetch/background_fetch_types.cc

@@ -37,7 +37,7 @@ blink::mojom::FetchAPIResponsePtr BackgroundFetchSettledFetch::CloneResponse(
       response->cors_exposed_header_names,
       CloneSerializedBlob(response->side_data_blob),
       CloneSerializedBlob(response->side_data_blob_for_cache_put),
-      response->content_security_policy.Clone(),
+      mojo::Clone(response->content_security_policy),
       response->loaded_with_credentials);
 }
 

content/common/service_worker/service_worker_loader_helpers.cc

@@ -109,7 +109,8 @@ void ServiceWorkerLoaderHelpers::SaveResponseInfo(
     out_head->cache_storage_cache_name.clear();
   out_head->cors_exposed_header_names = response.cors_exposed_header_names;
   out_head->did_service_worker_navigation_preload = false;
-  out_head->content_security_policy = response.content_security_policy.Clone();
+  out_head->content_security_policy =
+      mojo::Clone(response.content_security_policy);
 }
 
 // static

services/network/public/cpp/content_security_policy.cc

@@ -301,29 +301,58 @@ bool ParseReportDirective(const GURL& request_url,
   return true;
 }
 
-}  // namespace
+// Parses the frame-ancestor directive of a Content-Security-Policy header.
+bool ParseFrameAncestors(
+    const mojom::ContentSecurityPolicyPtr& content_security_policy_ptr,
+    base::StringPiece frame_ancestors_value) {
+  // A frame-ancestors directive has already been parsed. Skip further
+  // frame-ancestors directives per
+  // https://www.w3.org/TR/CSP3/#parse-serialized-policy.
+  if (FindDirective(mojom::CSPDirective::Name::FrameAncestors,
+                    &(content_security_policy_ptr->directives))) {
+    // TODO(arthursonzogni, lfg): Should a warning be fired to the user here?
+    return true;
+  }
 
-ContentSecurityPolicy::ContentSecurityPolicy() = default;
-ContentSecurityPolicy::~ContentSecurityPolicy() = default;
+  auto source_list = ParseFrameAncestorsSourceList(frame_ancestors_value);
 
-ContentSecurityPolicy::ContentSecurityPolicy(
-    mojom::ContentSecurityPolicyPtr content_security_policy_ptr)
-    : content_security_policy_ptr_(std::move(content_security_policy_ptr)) {}
+  // TODO(lfg): Emit a warning to the user when parsing an invalid
+  // expression.
+  if (!source_list)
+    return false;
 
-ContentSecurityPolicy::ContentSecurityPolicy(const ContentSecurityPolicy& other)
-    : content_security_policy_ptr_(other.content_security_policy_ptr_.Clone()) {
+  content_security_policy_ptr->directives.push_back(mojom::CSPDirective::New(
+      mojom::CSPDirective::Name::FrameAncestors, std::move(source_list)));
+
+  return true;
 }
 
-ContentSecurityPolicy::ContentSecurityPolicy(ContentSecurityPolicy&& other) =
-    default;
+// Parses the report-uri directive of a Content-Security-Policy header.
+bool ParseReportEndpoint(
+    const mojom::ContentSecurityPolicyPtr& content_security_policy_ptr,
+    const GURL& base_url,
+    base::StringPiece header_value,
+    bool using_reporting_api) {
+  // A report-uri directive has already been parsed. Skip further directives per
+  // https://www.w3.org/TR/CSP3/#parse-serialized-policy.
+  if (!content_security_policy_ptr->report_endpoints.empty())
+    return true;
 
-ContentSecurityPolicy& ContentSecurityPolicy::operator=(
-    const ContentSecurityPolicy& other) {
-  content_security_policy_ptr_ = other.content_security_policy_ptr_.Clone();
+  if (!ParseReportDirective(base_url, header_value, using_reporting_api,
+                            &(content_security_policy_ptr->report_endpoints))) {
+    // TODO(lfg): Emit a warning to the user when parsing an invalid
+    // expression.
+    return false;
+  }
 
-  return *this;
+  return true;
 }
 
+}  // namespace
+
+ContentSecurityPolicy::ContentSecurityPolicy() = default;
+ContentSecurityPolicy::~ContentSecurityPolicy() = default;
+
 bool ContentSecurityPolicy::Parse(const GURL& base_url,
                                   const net::HttpResponseHeaders& headers) {
   size_t iter = 0;
@@ -338,90 +367,45 @@ bool ContentSecurityPolicy::Parse(const GURL& base_url,
 
 bool ContentSecurityPolicy::Parse(const GURL& base_url,
                                   base::StringPiece header_value) {
-  if (!content_security_policy_ptr_) {
-    content_security_policy_ptr_ = mojom::ContentSecurityPolicy::New();
-  }
-
   // RFC7230, section 3.2.2 specifies that headers appearing multiple times can
   // be combined with a comma. Walk the header string, and parse each comma
   // separated chunk as a separate header.
-  //
-  // TODO(arthursonzogni, lfg): The ContentSecurityPolicy policy shouldn't be
-  // combined. Several ContentSecurityPolicies should be produced, not one.
   for (const auto& header :
        base::SplitStringPiece(header_value, ",", base::TRIM_WHITESPACE,
                               base::SPLIT_WANT_NONEMPTY)) {
     DirectivesMap directives = ParseHeaderValue(header);
+    auto content_security_policy_ptr = mojom::ContentSecurityPolicy::New();
 
     auto frame_ancestors = directives.find("frame-ancestors");
     if (frame_ancestors != directives.end()) {
-      if (!ParseFrameAncestors(frame_ancestors->second)) {
-        content_security_policy_ptr_.reset();
+      if (!ParseFrameAncestors(content_security_policy_ptr,
+                               frame_ancestors->second)) {
+        content_security_policy_ptr.reset();
         return false;
       }
     }
 
     auto report_endpoints = directives.find("report-to");
     if (report_endpoints != directives.end()) {
-      if (!content_security_policy_ptr_->use_reporting_api) {
-        content_security_policy_ptr_->use_reporting_api = true;
-        content_security_policy_ptr_->report_endpoints.clear();
+      if (!content_security_policy_ptr->use_reporting_api) {
+        content_security_policy_ptr->use_reporting_api = true;
+        content_security_policy_ptr->report_endpoints.clear();
       }
     } else {
       report_endpoints = directives.find("report-uri");
     }
 
     if (report_endpoints != directives.end()) {
       if (!ParseReportEndpoint(
-              base_url, report_endpoints->second,
-              content_security_policy_ptr_->use_reporting_api)) {
-        content_security_policy_ptr_.reset();
+              content_security_policy_ptr, base_url, report_endpoints->second,
+              content_security_policy_ptr->use_reporting_api)) {
+        content_security_policy_ptr.reset();
         return false;
       }
     }
-  }
-
-  return true;
-}
 
-bool ContentSecurityPolicy::ParseFrameAncestors(
-    base::StringPiece frame_ancestors_value) {
-  // A frame-ancestors directive has already been parsed. Skip further
-  // frame-ancestors directives per
-  // https://www.w3.org/TR/CSP3/#parse-serialized-policy.
-  if (FindDirective(mojom::CSPDirective::Name::FrameAncestors,
-                    &(content_security_policy_ptr_->directives))) {
-    // TODO(arthursonzogni, lfg): Should a warning be fired to the user here?
-    return true;
-  }
-
-  auto source_list = ParseFrameAncestorsSourceList(frame_ancestors_value);
-
-  // TODO(lfg): Emit a warning to the user when parsing an invalid
-  // expression.
-  if (!source_list)
-    return false;
-
-  content_security_policy_ptr_->directives.push_back(mojom::CSPDirective::New(
-      mojom::CSPDirective::Name::FrameAncestors, std::move(source_list)));
-
-  return true;
-}
-
-bool ContentSecurityPolicy::ParseReportEndpoint(const GURL& base_url,
-                                                base::StringPiece header_value,
-                                                bool using_reporting_api) {
-  // A report-uri directive has already been parsed. Skip further directives per
-  // https://www.w3.org/TR/CSP3/#parse-serialized-policy.
-  if (!content_security_policy_ptr_->report_endpoints.empty())
-    return true;
-
-  if (!ParseReportDirective(
-          base_url, header_value, using_reporting_api,
-          &(content_security_policy_ptr_->report_endpoints))) {
-    // TODO(lfg): Emit a warning to the user when parsing an invalid
-    // expression.
-    return false;
+    content_security_policies_.push_back(
+        std::move(content_security_policy_ptr));
   }
 
   return true;

services/network/public/cpp/content_security_policy.h

@@ -23,11 +23,8 @@ class COMPONENT_EXPORT(NETWORK_CPP) ContentSecurityPolicy {
   ContentSecurityPolicy();
   ~ContentSecurityPolicy();
 
-  explicit ContentSecurityPolicy(
-      mojom::ContentSecurityPolicyPtr content_security_policy_ptr);
-  ContentSecurityPolicy(const ContentSecurityPolicy& other);
-  ContentSecurityPolicy(ContentSecurityPolicy&& other);
-  ContentSecurityPolicy& operator=(const ContentSecurityPolicy& other);
+  ContentSecurityPolicy(const ContentSecurityPolicy&) = delete;
+  ContentSecurityPolicy& operator=(const ContentSecurityPolicy&) = delete;
 
   // Parses the Content-Security-Policy headers specified in |headers| while
   // requesting |request_url|. The |request_url| is used for violation
@@ -38,24 +35,16 @@ class COMPONENT_EXPORT(NETWORK_CPP) ContentSecurityPolicy {
   // Parses a Content-Security-Policy |header|.
   bool Parse(const GURL& base_url, base::StringPiece header);
 
-  const mojom::ContentSecurityPolicyPtr& content_security_policy_ptr() {
-    return content_security_policy_ptr_;
+  const std::vector<mojom::ContentSecurityPolicyPtr>&
+  content_security_policies() {
+    return content_security_policies_;
   }
-  mojom::ContentSecurityPolicyPtr TakeContentSecurityPolicy() {
-    return std::move(content_security_policy_ptr_);
+  std::vector<mojom::ContentSecurityPolicyPtr> TakeContentSecurityPolicy() {
+    return std::move(content_security_policies_);
   }
 
  private:
-
-  // Parses the frame-ancestor directive of a Content-Security-Policy header.
-  bool ParseFrameAncestors(base::StringPiece header_value);
-
-  // Parses the report-uri directive of a Content-Security-Policy header.
-  bool ParseReportEndpoint(const GURL& base_url,
-                           base::StringPiece header_value,
-                           bool using_reporting_api);
-
-  mojom::ContentSecurityPolicyPtr content_security_policy_ptr_;
+  std::vector<mojom::ContentSecurityPolicyPtr> content_security_policies_;
 };
 
 }  // namespace network