COEP should work only on secure contexts

whatwg/html#4930 (We don't have a spec text yet). Bug: 1086066 Change-Id: I0d51df12af686dc4f1bbad252e02f9bf64981b56 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216833 Commit-Queue: Yutaka Hirano <[email protected]> Reviewed-by: Arthur Sonzogni <[email protected]> Cr-Commit-Position: refs/heads/master@{#775421}
Yannic · Jun 5, 2020 · 6585711 · 6585711
1 parent b7bb584
commit 6585711
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 40 deletions.
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
@@ -101,6 +101,7 @@
 #include "services/network/public/cpp/content_security_policy/content_security_policy.h"
 #include "services/network/public/cpp/cross_origin_resource_policy.h"
 #include "services/network/public/cpp/features.h"
+#include "services/network/public/cpp/is_potentially_trustworthy.h"
 #include "services/network/public/cpp/resource_request_body.h"
 #include "services/network/public/cpp/url_loader_completion_status.h"
 #include "services/network/public/cpp/web_sandbox_flags.h"
@@ -2063,49 +2064,58 @@ void NavigationRequest::OnResponseStarted(
       response_head_->parsed_headers->cross_origin_embedder_policy;
   if (base::FeatureList::IsEnabled(
           network::features::kCrossOriginEmbedderPolicy)) {
-    // https://mikewest.github.io/corpp/#process-navigation-response
-    if (auto* const parent_frame = GetParentFrame()) {
-      const auto& parent_coep = parent_frame->cross_origin_embedder_policy();
-      const auto& url = common_params_->url;
-      constexpr auto kRequireCorp =
-          network::mojom::CrossOriginEmbedderPolicyValue::kRequireCorp;
-      constexpr auto kNone =
-          network::mojom::CrossOriginEmbedderPolicyValue::kNone;
-
-      // Some special URLs not loaded using the network are inheriting the
-      // Cross-Origin-Embedder-Policy header from their parent.
-      const bool has_allowed_scheme =
-          url.SchemeIsBlob() || url.SchemeIs(url::kDataScheme) ||
-          GetContentClient()
-              ->browser()
-              ->ShouldInheritCrossOriginEmbedderPolicyImplicitly(url);
-      if (parent_coep.value == kRequireCorp && has_allowed_scheme) {
-        cross_origin_embedder_policy.value = kRequireCorp;
-      }
+    const auto& url = common_params_->url;
+    // https://w3c.github.io/webappsec-secure-contexts/#is-url-trustworthy
+    // returns "Potentially Trustworthy" for data URLs, but
+    // network::IsUrlPotentiallyTrustworthy returns false, so we need this
+    // extra condition.
+    if (network::IsUrlPotentiallyTrustworthy(url) ||
+        url.SchemeIs(url::kDataScheme)) {
+      // https://mikewest.github.io/corpp/#process-navigation-response
+      if (auto* const parent = GetParentFrame()) {
+        const auto& parent_coep = parent->cross_origin_embedder_policy();
+        constexpr auto kRequireCorp =
+            network::mojom::CrossOriginEmbedderPolicyValue::kRequireCorp;
+        constexpr auto kNone =
+            network::mojom::CrossOriginEmbedderPolicyValue::kNone;
+
+        // Some special URLs not loaded using the network are inheriting the
+        // Cross-Origin-Embedder-Policy header from their parent.
+        const bool has_allowed_scheme =
+            url.SchemeIsBlob() || url.SchemeIs(url::kDataScheme) ||
+            GetContentClient()
+                ->browser()
+                ->ShouldInheritCrossOriginEmbedderPolicyImplicitly(url);
+        if (parent_coep.value == kRequireCorp && has_allowed_scheme) {
+          cross_origin_embedder_policy.value = kRequireCorp;
+        }
 
-      auto* const coep_reporter = parent_frame->coep_reporter();
-      if (parent_coep.report_only_value == kRequireCorp &&
-          !has_allowed_scheme && cross_origin_embedder_policy.value == kNone &&
-          coep_reporter) {
-        coep_reporter->QueueNavigationReport(redirect_chain_[0],
-                                             /*report_only=*/true);
-      }
-      if (parent_coep.value == kRequireCorp &&
-          cross_origin_embedder_policy.value == kNone) {
-        if (coep_reporter) {
+        auto* const coep_reporter = parent->coep_reporter();
+        if (parent_coep.report_only_value == kRequireCorp &&
+            !has_allowed_scheme &&
+            cross_origin_embedder_policy.value == kNone && coep_reporter) {
           coep_reporter->QueueNavigationReport(redirect_chain_[0],
-                                               /*report_only=*/false);
+                                               /*report_only=*/true);
+        }
+        if (parent_coep.value == kRequireCorp &&
+            cross_origin_embedder_policy.value == kNone) {
+          if (coep_reporter) {
+            coep_reporter->QueueNavigationReport(redirect_chain_[0],
+                                                 /*report_only=*/false);
+          }
+          OnRequestFailedInternal(network::URLLoaderCompletionStatus(
+                                      network::mojom::BlockedByResponseReason::
+                                          kCoepFrameResourceNeedsCoepHeader),
+                                  false /* skip_throttles */,
+                                  base::nullopt /* error_page_content */,
+                                  false /* collapse_frame */);
+          // DO NOT ADD CODE after this. The previous call to
+          // OnRequestFailedInternal has destroyed the NavigationRequest.
+          return;
         }
-        OnRequestFailedInternal(network::URLLoaderCompletionStatus(
-                                    network::mojom::BlockedByResponseReason::
-                                        kCoepFrameResourceNeedsCoepHeader),
-                                false /* skip_throttles */,
-                                base::nullopt /* error_page_content */,
-                                false /* collapse_frame */);
-        // DO NOT ADD CODE after this. The previous call to
-        // OnRequestFailedInternal has destroyed the NavigationRequest.
-        return;
       }
+    } else {
+      cross_origin_embedder_policy = network::CrossOriginEmbedderPolicy();
     }
   }
 

diff --git a/third_party/blink/web_tests/TestExpectations b/third_party/blink/web_tests/TestExpectations
@@ -5790,7 +5790,6 @@ crbug.com/1061131 [ Mac ] editing/selection/replaced-boundaries-1.html [ Pass Fa
 
 crbug.com/1058888 [ Linux ] animations/animationworklet/peek-updated-composited-property-on-main.html [ Pass Failure ]
 
-crbug.com/1086066 external/wpt/html/cross-origin-embedder-policy/no-secure-context.html [ Timeout ]
 crbug.com/1015187 external/wpt/html/cross-origin-embedder-policy/none.https.html [ Pass Timeout ]
 crbug.com/1013523 external/wpt/html/cross-origin-embedder-policy/require-corp.https.html [ Failure Pass ]
 crbug.com/626703 [ Debug ] external/wpt/html/cross-origin-embedder-policy/reporting-navigation.https.html [ Pass Failure ]