From f2a049571d8e6cd9f1526b847b02569f61ce1250 Mon Sep 17 00:00:00 2001 From: Sigurd Schneider Date: Mon, 2 Mar 2020 13:06:54 +0000 Subject: [PATCH] Report COEP errors for resource loads and navigations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Łukasz Anforowicz Reviewed-by: Kinuko Yasuda Reviewed-by: Mike West Commit-Queue: Sigurd Schneider Cr-Commit-Position: refs/heads/master@{#745916} --- .../cache_storage_dispatcher_host.cc | 7 +-- .../devtools/protocol/network_handler.cc | 37 +++++++++++++ .../browser/frame_host/navigation_request.cc | 27 ++++++---- .../renderer/loader/web_url_loader_impl.cc | 6 +++ .../network/cross_origin_read_blocking.cc | 9 ++-- services/network/public/cpp/BUILD.gn | 1 + .../public/cpp/blocked_by_response_reason.h | 25 +++++++++ .../cpp/cross_origin_resource_policy.cc | 49 +++++++++-------- .../public/cpp/cross_origin_resource_policy.h | 24 ++++----- .../cross_origin_resource_policy_unittest.cc | 24 ++++----- .../public/cpp/network_ipc_param_traits.h | 4 ++ .../cpp/url_loader_completion_status.cc | 7 +++ .../public/cpp/url_loader_completion_status.h | 10 ++++ services/network/url_loader.cc | 34 +++++++----- services/network/url_loader.h | 6 ++- .../devtools_protocol/browser_protocol.pdl | 5 ++ third_party/blink/public/platform/DEPS | 1 + .../resource_request_blocked_reason.h | 5 ++ .../blink/public/platform/web_url_error.h | 14 +++++ .../core/inspector/inspector_network_agent.cc | 17 +++++- .../fetch_respond_with_observer.cc | 5 +- .../platform/exported/web_url_error.cc | 12 +++++ .../platform/loader/fetch/resource_error.cc | 53 ++++++++++++++++++- .../platform/loader/fetch/resource_error.h | 3 ++ 24 files changed, 298 insertions(+), 87 deletions(-) create mode 100644 services/network/public/cpp/blocked_by_response_reason.h diff --git a/content/browser/cache_storage/cache_storage_dispatcher_host.cc b/content/browser/cache_storage/cache_storage_dispatcher_host.cc index 53b5943f7e2036..a89263b1bf246e 100644 --- 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 index 1c2b26daa44225..ea86f7367ddcfa 100644 --- a/content/browser/devtools/protocol/network_handler.cc +++ b/content/browser/devtools/protocol/network_handler.cc @@ -1522,6 +1522,21 @@ 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; @@ -1529,6 +1544,28 @@ String blockedReason(blink::ResourceRequestBlockedReason reason) { Maybe 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(); diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc index 113700fbfbbb61..6075d02f8bf5ed 100644 --- 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 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 index d641eaa52267f7..b5a203b4b20343 100644 --- 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 index 290e84c13b7f81..112e5fc558878f 100644 --- 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 index 86500a3b52d6f3..624a2f06a62e49 100644 --- 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 new file mode 100644 index 00000000000000..1fcd635774a5e1 --- /dev/null +++ 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 index 261d26da01d3b1..a5b813a5169d0b 100644 --- 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,7 +106,7 @@ bool ShouldAllowSameSite(const url::Origin& initiator, target_origin.scheme() != url::kHttpsScheme; } -CrossOriginResourcePolicy::VerificationResult VerifyInternal( +base::Optional IsBlockedInternal( CrossOriginResourcePolicy::ParsedHeader policy, const GURL& request_url, const base::Optional& request_initiator, @@ -113,11 +114,13 @@ CrossOriginResourcePolicy::VerificationResult VerifyInternal( base::Optional 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 CrossOriginResourcePolicy::IsBlocked( const GURL& request_url, const base::Optional& 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 +CrossOriginResourcePolicy::IsBlockedByHeaderValue( const GURL& request_url, const base::Optional& request_initiator, base::Optional 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 +CrossOriginResourcePolicy::IsNavigationBlocked( const GURL& request_url, const base::Optional& 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 index 79997fc03736de..1d7f19181d97dd 100644 --- 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 IsBlocked( const GURL& request_url, const base::Optional& request_initiator, const network::mojom::URLResponseHead& response, mojom::RequestMode request_mode, base::Optional 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 IsBlockedByHeaderValue( const GURL& request_url, const base::Optional& request_initiator, base::Optional corp_header_value, mojom::RequestMode request_mode, base::Optional 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 IsNavigationBlocked( const GURL& request_url, const base::Optional& request_initiator, const network::mojom::URLResponseHead& response, diff --git a/services/network/public/cpp/cross_origin_resource_policy_unittest.cc b/services/network/public/cpp/cross_origin_resource_policy_unittest.cc index 45e9d60afe4d5d..2cf46b85ccffbb 100644 --- a/services/network/public/cpp/cross_origin_resource_policy_unittest.cc +++ b/services/network/public/cpp/cross_origin_resource_policy_unittest.cc @@ -147,26 +147,27 @@ TEST(CrossOriginResourcePolicyTest, WithCOEP) { url::Origin another_origin = url::Origin::Create(GURL("https://www2.example.com")); - constexpr auto kAllow = CrossOriginResourcePolicy::kAllow; - constexpr auto kBlock = CrossOriginResourcePolicy::kBlock; + constexpr auto kAllow = base::nullopt; using mojom::RequestMode; struct TestCase { const RequestMode request_mode; const url::Origin origin; mojom::URLResponseHeadPtr response_info; - const CrossOriginResourcePolicy::VerificationResult - expectation_with_coep_none; - const CrossOriginResourcePolicy::VerificationResult + const base::Optional expectation_with_coep_none; + const base::Optional expectation_with_coep_require_corp; } test_cases[] = { // We don't have a cross-origin-resource-policy header on a response. That - // leads to kBlock when COEP: kRequireCorp is used. - {RequestMode::kNoCors, another_origin, corp_none.Clone(), kAllow, kBlock}, + // leads to blocking when COEP: kRequireCorp is used. + {RequestMode::kNoCors, another_origin, corp_none.Clone(), kAllow, + BlockedByResponseReason:: + kCorpNotSameOriginAfterDefaultedToSameOriginByCoep}, // We have "cross-origin-resource-policy: same-origin", so regardless of // COEP the response is blocked. - {RequestMode::kNoCors, another_origin, corp_same_origin.Clone(), kBlock, - kBlock}, + {RequestMode::kNoCors, another_origin, corp_same_origin.Clone(), + BlockedByResponseReason::kCorpNotSameOrigin, + BlockedByResponseReason::kCorpNotSameOrigin}, // We have "cross-origin-resource-policy: cross-origin", so regardless of // COEP the response is allowed. {RequestMode::kNoCors, another_origin, corp_cross_origin.Clone(), kAllow, @@ -184,13 +185,12 @@ TEST(CrossOriginResourcePolicyTest, WithCOEP) { for (const auto& test_case : test_cases) { CrossOriginEmbedderPolicy embedder_policy; EXPECT_EQ(test_case.expectation_with_coep_none, - CrossOriginResourcePolicy::Verify( + CrossOriginResourcePolicy::IsBlocked( destination, test_case.origin, *test_case.response_info, test_case.request_mode, test_case.origin, embedder_policy)); - embedder_policy.value = mojom::CrossOriginEmbedderPolicyValue::kRequireCorp; EXPECT_EQ(test_case.expectation_with_coep_require_corp, - CrossOriginResourcePolicy::Verify( + CrossOriginResourcePolicy::IsBlocked( destination, test_case.origin, *test_case.response_info, test_case.request_mode, test_case.origin, embedder_policy)); } diff --git a/services/network/public/cpp/network_ipc_param_traits.h b/services/network/public/cpp/network_ipc_param_traits.h index 8c55eaf0f1332b..5cc7dcc17e1c42 100644 --- a/services/network/public/cpp/network_ipc_param_traits.h +++ b/services/network/public/cpp/network_ipc_param_traits.h @@ -91,6 +91,9 @@ IPC_ENUM_TRAITS_MAX_VALUE(network::mojom::RequestMode, IPC_ENUM_TRAITS_MAX_VALUE(network::mojom::CorsPreflightPolicy, network::mojom::CorsPreflightPolicy::kMaxValue) +IPC_ENUM_TRAITS_MAX_VALUE(network::BlockedByResponseReason, + network::BlockedByResponseReason::kMaxValue) + IPC_STRUCT_TRAITS_BEGIN(network::CorsErrorStatus) IPC_STRUCT_TRAITS_MEMBER(cors_error) IPC_STRUCT_TRAITS_MEMBER(failed_parameter) @@ -106,6 +109,7 @@ IPC_STRUCT_TRAITS_BEGIN(network::URLLoaderCompletionStatus) IPC_STRUCT_TRAITS_MEMBER(decoded_body_length) IPC_STRUCT_TRAITS_MEMBER(cors_error_status) IPC_STRUCT_TRAITS_MEMBER(ssl_info) + IPC_STRUCT_TRAITS_MEMBER(blocked_by_response_reason) IPC_STRUCT_TRAITS_MEMBER(should_report_corb_blocking) IPC_STRUCT_TRAITS_MEMBER(proxy_server) IPC_STRUCT_TRAITS_MEMBER(resolve_error_info) diff --git a/services/network/public/cpp/url_loader_completion_status.cc b/services/network/public/cpp/url_loader_completion_status.cc index 255351258b28d9..8cc6c810b50890 100644 --- a/services/network/public/cpp/url_loader_completion_status.cc +++ b/services/network/public/cpp/url_loader_completion_status.cc @@ -21,6 +21,12 @@ URLLoaderCompletionStatus::URLLoaderCompletionStatus( cors_error_status = error; } +URLLoaderCompletionStatus::URLLoaderCompletionStatus( + const BlockedByResponseReason& reason) + : URLLoaderCompletionStatus(net::ERR_BLOCKED_BY_RESPONSE) { + blocked_by_response_reason = reason; +} + URLLoaderCompletionStatus::~URLLoaderCompletionStatus() = default; bool URLLoaderCompletionStatus::operator==( @@ -33,6 +39,7 @@ bool URLLoaderCompletionStatus::operator==( encoded_body_length == rhs.encoded_body_length && decoded_body_length == rhs.decoded_body_length && cors_error_status == rhs.cors_error_status && + blocked_by_response_reason == rhs.blocked_by_response_reason && should_report_corb_blocking == rhs.should_report_corb_blocking && proxy_server == rhs.proxy_server; } diff --git a/services/network/public/cpp/url_loader_completion_status.h b/services/network/public/cpp/url_loader_completion_status.h index 89ee7319d6310b..aeb3f068f70a92 100644 --- a/services/network/public/cpp/url_loader_completion_status.h +++ b/services/network/public/cpp/url_loader_completion_status.h @@ -14,6 +14,7 @@ #include "net/base/proxy_server.h" #include "net/dns/public/resolve_error_info.h" #include "net/ssl/ssl_info.h" +#include "services/network/public/cpp/blocked_by_response_reason.h" #include "services/network/public/cpp/cors/cors_error_status.h" #include "services/network/public/mojom/cors.mojom-shared.h" @@ -34,6 +35,11 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) URLLoaderCompletionStatus { // base::TimeTicks::Now() to |completion_time|. explicit URLLoaderCompletionStatus(const CorsErrorStatus& error); + // Sets ERR_BLOCKED_BY_RESPONSE to |error_code|, |reason| to + // |blocked_by_response_reason|, and base::TimeTicks::Now() to + // |completion_time|. + explicit URLLoaderCompletionStatus(const BlockedByResponseReason& reason); + ~URLLoaderCompletionStatus(); bool operator==(const URLLoaderCompletionStatus& rhs) const; @@ -65,6 +71,10 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) URLLoaderCompletionStatus { // Optional SSL certificate info. base::Optional ssl_info; + // More detailed reason for failing the response with + // ERR_net::ERR_BLOCKED_BY_RESPONSE |error_code|. + base::Optional blocked_by_response_reason; + // Set when response blocked by CORB needs to be reported to the DevTools // console. bool should_report_corb_blocking = false; diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc index e8c902a95d23d7..f24618aea80e04 100644 --- a/services/network/url_loader.cc +++ b/services/network/url_loader.cc @@ -958,12 +958,14 @@ void URLLoader::OnReceivedRedirect(net::URLRequest* url_request, factory_params_->client_security_state ? factory_params_->client_security_state->cross_origin_embedder_policy : kEmpty; - if (CrossOriginResourcePolicy::kBlock == - CrossOriginResourcePolicy::Verify( - url_request_->url(), url_request_->initiator(), *response, - request_mode_, factory_params_->request_initiator_site_lock, - cross_origin_embedder_policy)) { - CompleteBlockedResponse(net::ERR_BLOCKED_BY_RESPONSE, false); + + if (base::Optional blocked_reason = + CrossOriginResourcePolicy::IsBlocked( + url_request_->url(), url_request_->initiator(), *response, + request_mode_, factory_params_->request_initiator_site_lock, + cross_origin_embedder_policy)) { + CompleteBlockedResponse(net::ERR_BLOCKED_BY_RESPONSE, false, + blocked_reason); DeleteSelf(); return; } @@ -1119,12 +1121,13 @@ void URLLoader::OnResponseStarted(net::URLRequest* url_request, int net_error) { factory_params_->client_security_state ? factory_params_->client_security_state->cross_origin_embedder_policy : kEmpty; - if (CrossOriginResourcePolicy::kBlock == - CrossOriginResourcePolicy::Verify( - url_request_->url(), url_request_->initiator(), *response_, - request_mode_, factory_params_->request_initiator_site_lock, - cross_origin_embedder_policy)) { - CompleteBlockedResponse(net::ERR_BLOCKED_BY_RESPONSE, false); + if (base::Optional blocked_reason = + CrossOriginResourcePolicy::IsBlocked( + url_request_->url(), url_request_->initiator(), *response_, + request_mode_, factory_params_->request_initiator_site_lock, + cross_origin_embedder_policy)) { + CompleteBlockedResponse(net::ERR_BLOCKED_BY_RESPONSE, false, + blocked_reason); DeleteSelf(); return; } @@ -1772,8 +1775,10 @@ void URLLoader::OnHeadersReceivedComplete( std::move(callback).Run(result); } -void URLLoader::CompleteBlockedResponse(int error_code, - bool should_report_corb_blocking) { +void URLLoader::CompleteBlockedResponse( + int error_code, + bool should_report_corb_blocking, + base::Optional reason) { // The response headers and body shouldn't yet be sent to the URLLoaderClient. DCHECK(response_); DCHECK(consumer_handle_.is_valid()); @@ -1786,6 +1791,7 @@ void URLLoader::CompleteBlockedResponse(int error_code, status.encoded_body_length = 0; status.decoded_body_length = 0; status.should_report_corb_blocking = should_report_corb_blocking; + status.blocked_by_response_reason = reason; url_loader_client_->OnComplete(status); // Reset the connection to the URLLoaderClient. This helps ensure that we diff --git a/services/network/url_loader.h b/services/network/url_loader.h index d2ebc94fdc3a6a..8ff1c814e193b9 100644 --- a/services/network/url_loader.h +++ b/services/network/url_loader.h @@ -270,8 +270,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader const base::Optional& headers, const base::Optional& preserve_fragment_on_redirect_url); - void CompleteBlockedResponse(int error_code, - bool should_report_corb_blocking); + void CompleteBlockedResponse( + int error_code, + bool should_report_corb_blocking, + base::Optional reason = base::nullopt); enum BlockResponseForCorbResult { // Returned when caller of BlockResponseForCorb doesn't need to continue, diff --git a/third_party/blink/public/devtools_protocol/browser_protocol.pdl b/third_party/blink/public/devtools_protocol/browser_protocol.pdl index 1b5f7f84d7da8e..d7f9c2aff4c158 100644 --- a/third_party/blink/public/devtools_protocol/browser_protocol.pdl +++ b/third_party/blink/public/devtools_protocol/browser_protocol.pdl @@ -4060,6 +4060,11 @@ domain Network subresource-filter content-type collapsed-by-client + coep-frame-resource-needs-coep-header + coop-sandboxed-iframe-cannot-navigate-to-coop-page + corp-not-same-origin + corp-not-same-origin-after-defaulted-to-same-origin-by-coep + corp-not-same-site # HTTP response data. type Response extends object diff --git a/third_party/blink/public/platform/DEPS b/third_party/blink/public/platform/DEPS index 7920e5df5f53cb..1fa03e81cc0601 100644 --- a/third_party/blink/public/platform/DEPS +++ b/third_party/blink/public/platform/DEPS @@ -33,6 +33,7 @@ include_rules = [ "+net/cert", "+net/dns/public", "+net/http", + "+services/network/public/cpp/blocked_by_response_reason.h", "+services/network/public/cpp/cors/cors_error_status.h", "+services/network/public/cpp/cors/preflight_result.h", "+services/network/public/cpp/shared_url_loader_factory.h", diff --git a/third_party/blink/public/platform/resource_request_blocked_reason.h b/third_party/blink/public/platform/resource_request_blocked_reason.h index 09ef9ba7c3ab00..db14205ea69c3b 100644 --- a/third_party/blink/public/platform/resource_request_blocked_reason.h +++ b/third_party/blink/public/platform/resource_request_blocked_reason.h @@ -17,6 +17,11 @@ enum class ResourceRequestBlockedReason { kSubresourceFilter, kContentType, kCollapsedByClient, + kCoepFrameResourceNeedsCoepHeader, + kCoopSandboxedIFrameCannotNavigateToCoopPage, + kCorpNotSameOrigin, + kCorpNotSameOriginAfterDefaultedToSameOriginByCoep, + kCorpNotSameSite, }; } // namespace blink diff --git a/third_party/blink/public/platform/web_url_error.h b/third_party/blink/public/platform/web_url_error.h index 8ede3955f7eaf1..b7b3a60b3744b3 100644 --- a/third_party/blink/public/platform/web_url_error.h +++ b/third_party/blink/public/platform/web_url_error.h @@ -34,6 +34,7 @@ #include "base/logging.h" #include "base/optional.h" #include "net/dns/public/resolve_error_info.h" +#include "services/network/public/cpp/blocked_by_response_reason.h" #include "services/network/public/cpp/cors/cors_error_status.h" #include "third_party/blink/public/platform/web_url.h" @@ -61,6 +62,11 @@ struct WebURLError { HasCopyInCache, IsWebSecurityViolation, const WebURL&); + BLINK_PLATFORM_EXPORT WebURLError( + network::BlockedByResponseReason blocked_reason, + net::ResolveErrorInfo resolve_error_info, + HasCopyInCache, + const WebURL&); BLINK_PLATFORM_EXPORT WebURLError(const network::CorsErrorStatus&, HasCopyInCache, const WebURL&); @@ -76,6 +82,10 @@ struct WebURLError { const base::Optional cors_error_status() const { return cors_error_status_; } + const base::Optional + blocked_by_response_reason() const { + return blocked_by_response_reason_; + } private: // A numeric error code detailing the reason for this error. The value must @@ -100,6 +110,10 @@ struct WebURLError { // Optional CORS error details. base::Optional cors_error_status_; + + // More detailed reason for failing the response with + // ERR_net::ERR_BLOCKED_BY_RESPONSE |error_code|. + base::Optional blocked_by_response_reason_; }; } // namespace blink diff --git a/third_party/blink/renderer/core/inspector/inspector_network_agent.cc b/third_party/blink/renderer/core/inspector/inspector_network_agent.cc index 88104360ea051e..b87b0b6c8c21d7 100644 --- a/third_party/blink/renderer/core/inspector/inspector_network_agent.cc +++ b/third_party/blink/renderer/core/inspector/inspector_network_agent.cc @@ -363,6 +363,21 @@ String BuildBlockedReason(ResourceRequestBlockedReason reason) { return protocol::Network::BlockedReasonEnum::Other; case 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; @@ -1107,7 +1122,7 @@ void InspectorNetworkAgent::DidFailLoading(uint64_t identifier, bool canceled = error.IsCancellation(); base::Optional resource_request_blocked_reason = error.GetResourceRequestBlockedReason(); - blink::protocol::Maybe blocked_reason; + protocol::Maybe blocked_reason; if (resource_request_blocked_reason) { blocked_reason = BuildBlockedReason(resource_request_blocked_reason.value()); diff --git a/third_party/blink/renderer/modules/service_worker/fetch_respond_with_observer.cc b/third_party/blink/renderer/modules/service_worker/fetch_respond_with_observer.cc index 5b88eab635f11b..0dbf2a03747326 100644 --- a/third_party/blink/renderer/modules/service_worker/fetch_respond_with_observer.cc +++ b/third_party/blink/renderer/modules/service_worker/fetch_respond_with_observer.cc @@ -322,10 +322,9 @@ void FetchRespondWithObserver::OnResponseFulfilled( // Hence we provide |initiator_origin| as |request_initiator_site_lock|. auto initiator_origin = url::Origin::Create(GURL(service_worker_global_scope->Url())); - if (network::CrossOriginResourcePolicy::VerifyByHeaderValue( + if (network::CrossOriginResourcePolicy::IsBlockedByHeaderValue( request_url_, initiator_origin, corp_header_value, request_mode_, - initiator_origin, requestor_coep_) != - network::CrossOriginResourcePolicy::VerificationResult::kAllow) { + initiator_origin, requestor_coep_)) { OnResponseRejected(ServiceWorkerResponseError::kDisallowedByCorp); return; } diff --git a/third_party/blink/renderer/platform/exported/web_url_error.cc b/third_party/blink/renderer/platform/exported/web_url_error.cc index dde6ef59144f96..c06728263cb52c 100644 --- a/third_party/blink/renderer/platform/exported/web_url_error.cc +++ b/third_party/blink/renderer/platform/exported/web_url_error.cc @@ -29,6 +29,18 @@ WebURLError::WebURLError(int reason, DCHECK_NE(reason_, 0); } +WebURLError::WebURLError(network::BlockedByResponseReason blocked_reason, + net::ResolveErrorInfo resolve_error_info, + HasCopyInCache has_copy_in_cache, + const WebURL& url) + : reason_(net::ERR_BLOCKED_BY_RESPONSE), + extended_reason_(0), + resolve_error_info_(resolve_error_info), + has_copy_in_cache_(has_copy_in_cache == HasCopyInCache::kTrue), + is_web_security_violation_(false), + url_(url), + blocked_by_response_reason_(blocked_reason) {} + WebURLError::WebURLError(const network::CorsErrorStatus& cors_error_status, HasCopyInCache has_copy_in_cache, const WebURL& url) diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_error.cc b/third_party/blink/renderer/platform/loader/fetch/resource_error.cc index 9d9e7d7dcc4328..a6706847ef4ecc 100644 --- a/third_party/blink/renderer/platform/loader/fetch/resource_error.cc +++ b/third_party/blink/renderer/platform/loader/fetch/resource_error.cc @@ -100,7 +100,8 @@ ResourceError::ResourceError(const WebURLError& error) failing_url_(error.url()), is_access_check_(error.is_web_security_violation()), has_copy_in_cache_(error.has_copy_in_cache()), - cors_error_status_(error.cors_error_status()) { + cors_error_status_(error.cors_error_status()), + blocked_by_response_reason_(error.blocked_by_response_reason()) { DCHECK_NE(error_code_, 0); InitializeDescription(); } @@ -183,12 +184,45 @@ bool ResourceError::ShouldCollapseInitiator() const { ResourceRequestBlockedReason::kCollapsedByClient; } +namespace { + +blink::ResourceRequestBlockedReason +BlockedByResponseReasonToResourceRequestBlockedReason( + network::BlockedByResponseReason reason) { + switch (reason) { + case network::BlockedByResponseReason::kCoepFrameResourceNeedsCoepHeader: + return blink::ResourceRequestBlockedReason:: + kCoepFrameResourceNeedsCoepHeader; + case network::BlockedByResponseReason:: + kCoopSandboxedIFrameCannotNavigateToCoopPage: + return blink::ResourceRequestBlockedReason:: + kCoopSandboxedIFrameCannotNavigateToCoopPage; + case network::BlockedByResponseReason::kCorpNotSameOrigin: + return blink::ResourceRequestBlockedReason::kCorpNotSameOrigin; + case network::BlockedByResponseReason:: + kCorpNotSameOriginAfterDefaultedToSameOriginByCoep: + return blink::ResourceRequestBlockedReason:: + kCorpNotSameOriginAfterDefaultedToSameOriginByCoep; + break; + case network::BlockedByResponseReason::kCorpNotSameSite: + return blink::ResourceRequestBlockedReason::kCorpNotSameSite; + break; + } + NOTREACHED(); + return blink::ResourceRequestBlockedReason::kOther; +} + +} // namespace base::Optional ResourceError::GetResourceRequestBlockedReason() const { if (error_code_ != net::ERR_BLOCKED_BY_CLIENT && error_code_ != net::ERR_BLOCKED_BY_RESPONSE) { return base::nullopt; } + if (blocked_by_response_reason_) { + return BlockedByResponseReasonToResourceRequestBlockedReason( + *blocked_by_response_reason_); + } return static_cast(extended_error_code_); } @@ -222,6 +256,23 @@ String DescriptionForBlockedByClientOrResponse(int error, int extended_error) { case ResourceRequestBlockedReason::kCollapsedByClient: detail = "Collapsed"; break; + case ResourceRequestBlockedReason::kCoepFrameResourceNeedsCoepHeader: + detail = "ResponseNeedsCrossOriginEmbedderPolicy"; + break; + case ResourceRequestBlockedReason:: + kCoopSandboxedIFrameCannotNavigateToCoopPage: + detail = "SandboxedIFrameCannotNavigateToOriginIsolatedPage"; + break; + case ResourceRequestBlockedReason::kCorpNotSameOrigin: + detail = "NotSameOrigin"; + break; + case ResourceRequestBlockedReason:: + kCorpNotSameOriginAfterDefaultedToSameOriginByCoep: + detail = "NotSameOriginAfterDefaultedToSameOriginByCoep"; + break; + case ResourceRequestBlockedReason::kCorpNotSameSite: + detail = "NotSameSite"; + break; } return WebString::FromASCII(net::ErrorToString(error) + "." + detail); } diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_error.h b/third_party/blink/renderer/platform/loader/fetch/resource_error.h index 92ea2a258775fe..19c60919c24090 100644 --- a/third_party/blink/renderer/platform/loader/fetch/resource_error.h +++ b/third_party/blink/renderer/platform/loader/fetch/resource_error.h @@ -30,6 +30,7 @@ #include #include "base/optional.h" #include "net/dns/public/resolve_error_info.h" +#include "services/network/public/cpp/blocked_by_response_reason.h" #include "services/network/public/cpp/cors/cors_error_status.h" #include "third_party/blink/renderer/platform/platform_export.h" #include "third_party/blink/renderer/platform/weborigin/kurl.h" @@ -107,6 +108,8 @@ class PLATFORM_EXPORT ResourceError final { bool has_copy_in_cache_ = false; bool blocked_by_subresource_filter_ = false; base::Optional cors_error_status_; + + base::Optional blocked_by_response_reason_; }; inline bool operator==(const ResourceError& a, const ResourceError& b) {