Report COEP errors for resource loads and navigations

This CL adds specific error reason reporting for resources that get blocked due to COEP errors. Blocking either occurs in a URLLoader or in a NavigationRequest. The error is plumbed to the DevTools front-end, which should help developers identify and fix COEP issues. Note that this CL does not address CORB blocked fetches, so DevTools will report a plain failure if those get blocked. Bug: chromium:1051473 Change-Id: Idad0d1669d0f45a7116419e1935feb0bb9973e91 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2070409 Reviewed-by: Camille Lamy <clamy@chromium.org> Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Commit-Queue: Sigurd Schneider <sigurds@chromium.org> Cr-Commit-Position: refs/heads/master@{#745916}
FairyWorld · Mar 2, 2020 · f2a0495 · f2a0495
1 parent 370ebe2
commit f2a0495
Show file tree

Hide file tree

Showing 24 changed files with 298 additions and 87 deletions.
diff --git a/content/browser/cache_storage/cache_storage_dispatcher_host.cc b/content/browser/cache_storage/cache_storage_dispatcher_host.cc
@@ -139,9 +139,10 @@ bool ResponseBlockedByCrossOriginResourcePolicy(
   if (corp_header != response->headers.end())
     corp_header_value = corp_header->second;
 
-  return !CrossOriginResourcePolicy::VerifyByHeaderValue(
-      response->url_list.back(), document_origin, corp_header_value,
-      RequestMode::kNoCors, document_origin, document_coep);
+  return CrossOriginResourcePolicy::IsBlockedByHeaderValue(
+             response->url_list.back(), document_origin, corp_header_value,
+             RequestMode::kNoCors, document_origin, document_coep)
+      .has_value();
 }
 
 }  // namespace

diff --git a/content/browser/devtools/protocol/network_handler.cc b/content/browser/devtools/protocol/network_handler.cc
@@ -1522,13 +1522,50 @@ String blockedReason(blink::ResourceRequestBlockedReason reason) {
       return protocol::Network::BlockedReasonEnum::Other;
     case blink::ResourceRequestBlockedReason::kCollapsedByClient:
       return protocol::Network::BlockedReasonEnum::CollapsedByClient;
+    case blink::ResourceRequestBlockedReason::kCoepFrameResourceNeedsCoepHeader:
+      return protocol::Network::BlockedReasonEnum::
+          CoepFrameResourceNeedsCoepHeader;
+    case blink::ResourceRequestBlockedReason::
+        kCoopSandboxedIFrameCannotNavigateToCoopPage:
+      return protocol::Network::BlockedReasonEnum::
+          CoopSandboxedIframeCannotNavigateToCoopPage;
+    case blink::ResourceRequestBlockedReason::kCorpNotSameOrigin:
+      return protocol::Network::BlockedReasonEnum::CorpNotSameOrigin;
+    case blink::ResourceRequestBlockedReason::
+        kCorpNotSameOriginAfterDefaultedToSameOriginByCoep:
+      return protocol::Network::BlockedReasonEnum::
+          CorpNotSameOriginAfterDefaultedToSameOriginByCoep;
+    case blink::ResourceRequestBlockedReason::kCorpNotSameSite:
+      return protocol::Network::BlockedReasonEnum::CorpNotSameSite;
   }
   NOTREACHED();
   return protocol::Network::BlockedReasonEnum::Other;
 }
 
 Maybe<String> GetBlockedReasonFor(
     const network::URLLoaderCompletionStatus& status) {
+  if (status.blocked_by_response_reason) {
+    switch (*status.blocked_by_response_reason) {
+      case network::BlockedByResponseReason::kCoepFrameResourceNeedsCoepHeader:
+        return {protocol::Network::BlockedReasonEnum::
+                    CoepFrameResourceNeedsCoepHeader};
+      case network::BlockedByResponseReason::
+          kCoopSandboxedIFrameCannotNavigateToCoopPage:
+        return {protocol::Network::BlockedReasonEnum::
+                    CoopSandboxedIframeCannotNavigateToCoopPage};
+      case network::BlockedByResponseReason::
+          kCorpNotSameOriginAfterDefaultedToSameOriginByCoep:
+        return {protocol::Network::BlockedReasonEnum::
+                    CorpNotSameOriginAfterDefaultedToSameOriginByCoep};
+      case network::BlockedByResponseReason::kCorpNotSameOrigin:
+        return {protocol::Network::BlockedReasonEnum::CorpNotSameOrigin};
+        break;
+      case network::BlockedByResponseReason::kCorpNotSameSite:
+        return {protocol::Network::BlockedReasonEnum::CorpNotSameSite};
+        break;
+    }
+    NOTREACHED();
+  }
   if (status.error_code != net::ERR_BLOCKED_BY_CLIENT &&
       status.error_code != net::ERR_BLOCKED_BY_RESPONSE)
     return Maybe<String>();

diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
@@ -1791,13 +1791,14 @@ void NavigationRequest::OnResponseStarted(
         !common_params_->url.SchemeIsBlob() &&
         !common_params_->url.SchemeIs(url::kDataScheme)) {
       // The CORP check for nested navigation.
-      if (network::CrossOriginResourcePolicy::VerifyNavigation(
-              common_params_->url, GetParentFrame()->GetLastCommittedOrigin(),
-              *response_head_, GetParentFrame()->GetLastCommittedOrigin(),
-              cross_origin_embedder_policy) ==
-          network::CrossOriginResourcePolicy::kBlock) {
+      if (base::Optional<network::BlockedByResponseReason> blocked_reason =
+              network::CrossOriginResourcePolicy::IsNavigationBlocked(
+                  common_params_->url,
+                  GetParentFrame()->GetLastCommittedOrigin(), *response_head_,
+                  GetParentFrame()->GetLastCommittedOrigin(),
+                  cross_origin_embedder_policy)) {
         OnRequestFailedInternal(
-            network::URLLoaderCompletionStatus(net::ERR_BLOCKED_BY_RESPONSE),
+            network::URLLoaderCompletionStatus(*blocked_reason),
             false /* skip_throttles */, base::nullopt /* error_page_content */,
             false /* collapse_frame */);
         // DO NOT ADD CODE after this. The previous call to
@@ -1817,10 +1818,12 @@ void NavigationRequest::OnResponseStarted(
       }
       if (cross_origin_embedder_policy.value ==
           network::mojom::CrossOriginEmbedderPolicyValue::kNone) {
-        OnRequestFailedInternal(
-            network::URLLoaderCompletionStatus(net::ERR_BLOCKED_BY_RESPONSE),
-            false /* skip_throttles */, base::nullopt /* error_page_content */,
-            false /* collapse_frame */);
+        OnRequestFailedInternal(network::URLLoaderCompletionStatus(
+                                    network::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;
@@ -1837,7 +1840,9 @@ void NavigationRequest::OnResponseStarted(
         (frame_tree_node_->pending_frame_policy().sandbox_flags !=
          blink::mojom::WebSandboxFlags::kNone)) {
       OnRequestFailedInternal(
-          network::URLLoaderCompletionStatus(net::ERR_BLOCKED_BY_RESPONSE),
+          network::URLLoaderCompletionStatus(
+              network::BlockedByResponseReason::
+                  kCoopSandboxedIFrameCannotNavigateToCoopPage),
           false /* skip_throttles */, base::nullopt /* error_page_content */,
           false /* collapse_frame */);
       // DO NOT ADD CODE after this. The previous call to

diff --git a/content/renderer/loader/web_url_loader_impl.cc b/content/renderer/loader/web_url_loader_impl.cc
@@ -71,6 +71,7 @@
 #include "third_party/blink/public/common/thread_safe_browser_interface_broker_proxy.h"
 #include "third_party/blink/public/platform/file_path_conversion.h"
 #include "third_party/blink/public/platform/platform.h"
+#include "third_party/blink/public/platform/resource_request_blocked_reason.h"
 #include "third_party/blink/public/platform/url_conversion.h"
 #include "third_party/blink/public/platform/web_http_load_info.h"
 #include "third_party/blink/public/platform/web_security_origin.h"
@@ -1015,6 +1016,11 @@ WebURLError WebURLLoaderImpl::PopulateURLError(
                              : WebURLError::HasCopyInCache::kFalse;
   if (status.cors_error_status)
     return WebURLError(*status.cors_error_status, has_copy_in_cache, url);
+  if (status.blocked_by_response_reason) {
+    DCHECK_EQ(net::ERR_BLOCKED_BY_RESPONSE, status.error_code);
+    return WebURLError(*status.blocked_by_response_reason,
+                       status.resolve_error_info, has_copy_in_cache, url);
+  }
   return WebURLError(status.error_code, status.extended_error_code,
                      status.resolve_error_info, has_copy_in_cache,
                      WebURLError::IsWebSecurityViolation::kFalse, url);

diff --git a/services/network/cross_origin_read_blocking.cc b/services/network/cross_origin_read_blocking.cc
@@ -832,9 +832,9 @@ CrossOriginReadBlocking::ResponseAnalyzer::ShouldBlockBasedOnHeaders(
   // fail in the renderer), then we can let CORB filter the response without
   // caring about MIME type or sniffing.
   //
-  // To make CrossOriginResourcePolicy::Verify apply to all fetch modes in this
-  // case and not just "no-cors", we pass kNoCors as a hard-coded value.  This
-  // does not affect the usual enforcement of CORP headers.
+  // To make CrossOriginResourcePolicy::IsBlocked apply to all fetch modes in
+  // this case and not just "no-cors", we pass kNoCors as a hard-coded value.
+  // This does not affect the usual enforcement of CORP headers.
   //
   // TODO(lukasza): Once OOR-CORS launches (https://crbug.com/736308), this code
   // block will no longer be necessary since all failed CORS requests will be
@@ -844,8 +844,7 @@ CrossOriginReadBlocking::ResponseAnalyzer::ShouldBlockBasedOnHeaders(
   constexpr mojom::RequestMode kOverreachingRequestMode =
       mojom::RequestMode::kNoCors;
   // COEP is not supported when OOR-CORS is disabled.
-  if (CrossOriginResourcePolicy::kBlock ==
-      CrossOriginResourcePolicy::Verify(
+  if (CrossOriginResourcePolicy::IsBlocked(
           request_url, request_initiator, response, kOverreachingRequestMode,
           request_initiator_site_lock, CrossOriginEmbedderPolicy())) {
     // Ignore mime types and/or sniffing and have CORB block all responses with

diff --git a/services/network/public/cpp/BUILD.gn b/services/network/public/cpp/BUILD.gn
@@ -141,6 +141,7 @@ jumbo_component("cpp_base") {
   output_name = "network_cpp_base"
 
   sources = [
+    "blocked_by_response_reason.h",
     "cors/cors_error_status.cc",
     "cors/cors_error_status.h",
     "cross_origin_embedder_policy.cc",

diff --git a/services/network/public/cpp/blocked_by_response_reason.h b/services/network/public/cpp/blocked_by_response_reason.h
@@ -0,0 +1,25 @@
+// Copyright 2020 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.
+
+#ifndef SERVICES_NETWORK_PUBLIC_CPP_BLOCKED_BY_RESPONSE_REASON_H_
+#define SERVICES_NETWORK_PUBLIC_CPP_BLOCKED_BY_RESPONSE_REASON_H_
+
+namespace network {
+
+// This enum is used by to communicate the reason a request was blocked from
+// the network service to the browser. The blocking reasons pertain to
+// security features such as CORP, COEP, and COOP.
+enum class BlockedByResponseReason : int {
+  kCoepFrameResourceNeedsCoepHeader,
+  kCoopSandboxedIFrameCannotNavigateToCoopPage,
+  kCorpNotSameOrigin,
+  kCorpNotSameOriginAfterDefaultedToSameOriginByCoep,
+  kCorpNotSameSite,
+  // `kMaxValue` needs be assigned to the max value in the enum.
+  kMaxValue = kCorpNotSameSite,
+};
+
+}  // namespace network
+
+#endif  // SERVICES_NETWORK_PUBLIC_CPP_BLOCKED_BY_RESPONSE_REASON_H_
diff --git a/services/network/public/cpp/cross_origin_resource_policy.cc b/services/network/public/cpp/cross_origin_resource_policy.cc
@@ -11,6 +11,7 @@
 #include "net/http/http_response_headers.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/initiator_lock_compatibility.h"
+#include "services/network/public/cpp/url_loader_completion_status.h"
 #include "services/network/public/mojom/url_response_head.mojom.h"
 #include "url/gurl.h"
 #include "url/origin.h"
@@ -105,19 +106,21 @@ bool ShouldAllowSameSite(const url::Origin& initiator,
          target_origin.scheme() != url::kHttpsScheme;
 }
 
-CrossOriginResourcePolicy::VerificationResult VerifyInternal(
+base::Optional<BlockedByResponseReason> IsBlockedInternal(
     CrossOriginResourcePolicy::ParsedHeader policy,
     const GURL& request_url,
     const base::Optional<url::Origin>& request_initiator,
     mojom::RequestMode request_mode,
     base::Optional<url::Origin> request_initiator_site_lock,
     mojom::CrossOriginEmbedderPolicyValue embedder_policy) {
   // COEP https://mikewest.github.io/corpp/#corp-check
+  bool upgrade_to_same_origin = false;
   if ((policy == CrossOriginResourcePolicy::kNoHeader ||
        policy == CrossOriginResourcePolicy::kParsingError) &&
       embedder_policy == mojom::CrossOriginEmbedderPolicyValue::kRequireCorp) {
     DCHECK(base::FeatureList::IsEnabled(features::kCrossOriginIsolation));
     policy = CrossOriginResourcePolicy::kSameOrigin;
+    upgrade_to_same_origin = true;
   }
 
   if (policy == CrossOriginResourcePolicy::kNoHeader ||
@@ -129,7 +132,7 @@ CrossOriginResourcePolicy::VerificationResult VerifyInternal(
     //
     // From https://fetch.spec.whatwg.org/#cross-origin-resource-policy-header:
     // > 7.  Return allowed.
-    return CrossOriginResourcePolicy::kAllow;
+    return base::nullopt;
   }
 
   // From https://fetch.spec.whatwg.org/#cross-origin-resource-policy-header:
@@ -139,12 +142,16 @@ CrossOriginResourcePolicy::VerificationResult VerifyInternal(
   url::Origin initiator =
       GetTrustworthyInitiator(request_initiator_site_lock, request_initiator);
   if (initiator == target_origin)
-    return CrossOriginResourcePolicy::kAllow;
+    return base::nullopt;
 
   // From https://fetch.spec.whatwg.org/#cross-origin-resource-policy-header:
   // > 4. If policy is `same-origin`, then return blocked.
-  if (policy == CrossOriginResourcePolicy::kSameOrigin)
-    return CrossOriginResourcePolicy::kBlock;
+  if (policy == CrossOriginResourcePolicy::kSameOrigin) {
+    return upgrade_to_same_origin
+               ? BlockedByResponseReason::
+                     kCorpNotSameOriginAfterDefaultedToSameOriginByCoep
+               : BlockedByResponseReason::kCorpNotSameOrigin;
+  }
 
   // From https://fetch.spec.whatwg.org/#cross-origin-resource-policy-header:
   // > 5. If the following are true
@@ -154,12 +161,12 @@ CrossOriginResourcePolicy::VerificationResult VerifyInternal(
   // >      "none"
   // >    then return allowed.
   if (ShouldAllowSameSite(initiator, target_origin))
-    return CrossOriginResourcePolicy::kAllow;
+    return base::nullopt;
 
   // From https://fetch.spec.whatwg.org/#cross-origin-resource-policy-header:
   // > 6.  If policy is `same-site`, then return blocked.
   DCHECK_EQ(CrossOriginResourcePolicy::kSameSite, policy);
-  return CrossOriginResourcePolicy::kBlock;
+  return BlockedByResponseReason::kCorpNotSameSite;
 }
 
 }  // namespace
@@ -169,7 +176,7 @@ const char CrossOriginResourcePolicy::kHeaderName[] =
     "Cross-Origin-Resource-Policy";
 
 // static
-CrossOriginResourcePolicy::VerificationResult CrossOriginResourcePolicy::Verify(
+base::Optional<BlockedByResponseReason> CrossOriginResourcePolicy::IsBlocked(
     const GURL& request_url,
     const base::Optional<url::Origin>& request_initiator,
     const network::mojom::URLResponseHead& response,
@@ -179,7 +186,7 @@ CrossOriginResourcePolicy::VerificationResult CrossOriginResourcePolicy::Verify(
   // From https://fetch.spec.whatwg.org/#cross-origin-resource-policy-header:
   // > 1. If request’s mode is not "no-cors", then return allowed.
   if (request_mode != mojom::RequestMode::kNoCors)
-    return kAllow;
+    return base::nullopt;
 
   // From https://fetch.spec.whatwg.org/#cross-origin-resource-policy-header:
   // > 3. Let policy be the result of getting `Cross-Origin-Resource-Policy`
@@ -191,13 +198,13 @@ CrossOriginResourcePolicy::VerificationResult CrossOriginResourcePolicy::Verify(
   ParsedHeader policy =
       ParseHeaderByHttpResponseHeaders(response.headers.get());
 
-  return VerifyInternal(policy, request_url, request_initiator, request_mode,
-                        request_initiator_site_lock, embedder_policy.value);
+  return IsBlockedInternal(policy, request_url, request_initiator, request_mode,
+                           request_initiator_site_lock, embedder_policy.value);
 }
 
 // static
-CrossOriginResourcePolicy::VerificationResult
-CrossOriginResourcePolicy::VerifyByHeaderValue(
+base::Optional<BlockedByResponseReason>
+CrossOriginResourcePolicy::IsBlockedByHeaderValue(
     const GURL& request_url,
     const base::Optional<url::Origin>& request_initiator,
     base::Optional<std::string> corp_header_value,
@@ -207,17 +214,17 @@ CrossOriginResourcePolicy::VerifyByHeaderValue(
   // From https://fetch.spec.whatwg.org/#cross-origin-resource-policy-header:
   // > 1. If request’s mode is not "no-cors", then return allowed.
   if (request_mode != mojom::RequestMode::kNoCors)
-    return kAllow;
+    return base::nullopt;
 
   ParsedHeader policy = ParseHeaderByString(corp_header_value);
 
-  return VerifyInternal(policy, request_url, request_initiator, request_mode,
-                        request_initiator_site_lock, embedder_policy.value);
+  return IsBlockedInternal(policy, request_url, request_initiator, request_mode,
+                           request_initiator_site_lock, embedder_policy.value);
 }
 
 // static
-CrossOriginResourcePolicy::VerificationResult
-CrossOriginResourcePolicy::VerifyNavigation(
+base::Optional<BlockedByResponseReason>
+CrossOriginResourcePolicy::IsNavigationBlocked(
     const GURL& request_url,
     const base::Optional<url::Origin>& request_initiator,
     const network::mojom::URLResponseHead& response,
@@ -226,9 +233,9 @@ CrossOriginResourcePolicy::VerifyNavigation(
   ParsedHeader policy =
       ParseHeaderByHttpResponseHeaders(response.headers.get());
 
-  return VerifyInternal(policy, request_url, request_initiator,
-                        mojom::RequestMode::kNavigate,
-                        request_initiator_site_lock, embedder_policy.value);
+  return IsBlockedInternal(policy, request_url, request_initiator,
+                           mojom::RequestMode::kNavigate,
+                           request_initiator_site_lock, embedder_policy.value);
 }
 
 // static

diff --git a/services/network/public/cpp/cross_origin_resource_policy.h b/services/network/public/cpp/cross_origin_resource_policy.h
@@ -33,35 +33,31 @@ class COMPONENT_EXPORT(NETWORK_CPP) CrossOriginResourcePolicy {
 
   static const char kHeaderName[];
 
-  // For "no-cors" fetches, the Verify method checks whether the response has a
-  // Cross-Origin-Resource-Policy header which says the response should not be
-  // delivered to a cross-origin or cross-site context.
-  enum VerificationResult {
-    kBlock,
-    kAllow,
-  };
   // The CORP check. This returns kAllowed when |request_mode| is not kNoCors.
-  static VerificationResult Verify(
+  // For kNoCors fetches, the IsBlocked method checks whether the response has
+  // a Cross-Origin-Resource-Policy header which says the response should not be
+  // delivered to a cross-origin or cross-site context.
+  static base::Optional<BlockedByResponseReason> IsBlocked(
       const GURL& request_url,
       const base::Optional<url::Origin>& request_initiator,
       const network::mojom::URLResponseHead& response,
       mojom::RequestMode request_mode,
       base::Optional<url::Origin> request_initiator_site_lock,
-      const CrossOriginEmbedderPolicy& embedder_policy);
+      const CrossOriginEmbedderPolicy& embedder_policy) WARN_UNUSED_RESULT;
 
-  // Same with Verify() but this method can take a raw value of
-  // Cross-Origin-Resource-Policy header instead of using URLResponseHead.
-  static VerificationResult VerifyByHeaderValue(
+  // Same as IsBlocked(), but this method can take a raw value of
+  // Cross-Origin-Resource-Policy header instead of using a URLResponseHead.
+  static base::Optional<BlockedByResponseReason> IsBlockedByHeaderValue(
       const GURL& request_url,
       const base::Optional<url::Origin>& request_initiator,
       base::Optional<std::string> corp_header_value,
       mojom::RequestMode request_mode,
       base::Optional<url::Origin> request_initiator_site_lock,
-      const CrossOriginEmbedderPolicy& embedder_policy);
+      const CrossOriginEmbedderPolicy& embedder_policy) WARN_UNUSED_RESULT;
 
   // The CORP check for navigation requests. This is expected to be called
   // from the navigation algorithm.
-  static VerificationResult VerifyNavigation(
+  static base::Optional<BlockedByResponseReason> IsNavigationBlocked(
       const GURL& request_url,
       const base::Optional<url::Origin>& request_initiator,
       const network::mojom::URLResponseHead& response,