diff options
Diffstat (limited to 'chromium/services')
7 files changed, 14 insertions, 129 deletions
diff --git a/chromium/services/network/cors/cors_url_loader.cc b/chromium/services/network/cors/cors_url_loader.cc index 7606ceb59fb..266520702c0 100644 --- a/chromium/services/network/cors/cors_url_loader.cc +++ b/chromium/services/network/cors/cors_url_loader.cc @@ -129,18 +129,6 @@ void CorsURLLoader::FollowRedirect( const net::HttpRequestHeaders& modified_headers, const net::HttpRequestHeaders& modified_cors_exempt_headers, const base::Optional<GURL>& new_url) { - // If this is a navigation from a renderer, then its a service worker - // passthrough of a navigation request. Since this case uses manual - // redirect mode FollowRedirect() should never be called. - if (process_id_ != mojom::kBrowserProcessId && - request_.mode == mojom::RequestMode::kNavigate) { - mojo::ReportBadMessage( - "CorsURLLoader: navigate from non-browser-process should not call " - "FollowRedirect"); - HandleComplete(URLLoaderCompletionStatus(net::ERR_FAILED)); - return; - } - if (!network_loader_ || !deferred_redirect_url_) { HandleComplete(URLLoaderCompletionStatus(net::ERR_FAILED)); return; diff --git a/chromium/services/network/cors/cors_url_loader_factory.cc b/chromium/services/network/cors/cors_url_loader_factory.cc index dfc110d0596..60f73825b80 100644 --- a/chromium/services/network/cors/cors_url_loader_factory.cc +++ b/chromium/services/network/cors/cors_url_loader_factory.cc @@ -379,76 +379,18 @@ bool CorsURLLoaderFactory::IsValidRequest(const ResourceRequest& request, return false; } - // The `force_main_frame_for_same_site_cookies` should only be set when a - // service worker passes through a navigation request. In this case the - // mode must be `kNavigate` and the destination must be empty. - if (request.original_destination == mojom::RequestDestination::kDocument && - (request.mode != mojom::RequestMode::kNavigate || - request.destination != mojom::RequestDestination::kEmpty)) { - mojo::ReportBadMessage( - "CorsURLLoaderFactory: original_destination is unexpectedly set to " - "kDocument"); - return false; - } - - // Validate that a navigation redirect chain is not sent for a non-navigation - // request. - if (!request.navigation_redirect_chain.empty() && - request.mode != mojom::RequestMode::kNavigate) { - mojo::ReportBadMessage( - "CorsURLLoaderFactory: navigation redirect chain set for a " - "non-navigation"); - return false; - } - - // By default we compare the `request_initiator` to the lock below. This is - // overridden for renderer navigations, however. - base::Optional<url::Origin> origin_to_validate = request.request_initiator; - // Ensure that renderer requests are covered either by CORS or CORB. if (process_id_ != mojom::kBrowserProcessId) { switch (request.mode) { case mojom::RequestMode::kNavigate: - // A navigation request from a renderer can legally occur when a service - // worker passes it through from its `FetchEvent.request` to `fetch()`. - // In this case it is making a navigation request on behalf of the - // original initiator. Since that initiator may be cross-origin, its - // possible the request's initiator will not match our lock. - // - // To make this operation safe we instead compare the request URL origin - // against the initiator lock. We can do this since service workers - // should only ever handle same-origin navigations. - // - // With this approach its possible the initiator could be spoofed by the - // renderer. However, since we have validated the request URL they can - // only every lie to the origin that they have already compromised. It - // does not allow an attacker to target other arbitrary origins. - origin_to_validate = url::Origin::Create(request.url); - - // We further validate the navigation request by ensuring it has the - // correct redirect mode. This avoids an attacker attempting to - // craft a navigation that is then automatically followed to a separate - // target origin. With manual mode the redirect will instead be - // processed as an opaque redirect response that is passed back to the - // renderer and navigation code. The redirected requested must be - // sent anew and go through this validation again. - if (request.redirect_mode != mojom::RedirectMode::kManual) { - mojo::ReportBadMessage( - "CorsURLLoaderFactory: navigate from non-browser-process with " - "redirect_mode set to 'follow'"); - return false; - } - - // Validate that a navigation redirect chain is always provided for a - // navigation request. - if (request.navigation_redirect_chain.empty()) { - mojo::ReportBadMessage( - "CorsURLLoaderFactory: navigate from non-browser-process without " - "a redirect chain provided"); - return false; - } - - break; + // Only the browser process can initiate navigations. This helps ensure + // that a malicious/compromised renderer cannot bypass CORB by issuing + // kNavigate, rather than kNoCors requests. (CORB should apply only to + // no-cors requests as tracked in https://crbug.com/953315 and as + // captured in https://fetch.spec.whatwg.org/#main-fetch). + mojo::ReportBadMessage( + "CorsURLLoaderFactory: navigate from non-browser-process"); + return false; case mojom::RequestMode::kSameOrigin: case mojom::RequestMode::kCors: @@ -462,11 +404,11 @@ bool CorsURLLoaderFactory::IsValidRequest(const ResourceRequest& request, } } - // Depending on the type of request, compare either `request_initiator` or - // `request.url` to `request_initiator_origin_lock_`. + // Compare |request_initiator| and |request_initiator_origin_lock_|. InitiatorLockCompatibility initiator_lock_compatibility = - VerifyRequestInitiatorLockWithPluginCheck( - process_id_, request_initiator_origin_lock_, origin_to_validate); + VerifyRequestInitiatorLockWithPluginCheck(process_id_, + request_initiator_origin_lock_, + request.request_initiator); UMA_HISTOGRAM_ENUMERATION( "NetworkService.URLLoader.RequestInitiatorOriginLockCompatibility", initiator_lock_compatibility); @@ -495,6 +437,7 @@ bool CorsURLLoaderFactory::IsValidRequest(const ResourceRequest& request, case InitiatorLockCompatibility::kIncorrectLock: // Requests from the renderer need to always specify a correct initiator. + NOTREACHED(); if (base::FeatureList::IsEnabled( features::kRequestInitiatorSiteLockEnfocement)) { url::debug::ScopedOriginCrashKey initiator_lock_crash_key( diff --git a/chromium/services/network/public/cpp/resource_request.h b/chromium/services/network/public/cpp/resource_request.h index 144a3942421..000ff16bbd6 100644 --- a/chromium/services/network/public/cpp/resource_request.h +++ b/chromium/services/network/public/cpp/resource_request.h @@ -78,11 +78,6 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) ResourceRequest { base::Optional<url::Origin> request_initiator; base::Optional<url::Origin> isolated_world_origin; - - // The chain of URLs seen during navigation redirects. This should only - // contain values if the mode is `RedirectMode::kNavigate`. - std::vector<GURL> navigation_redirect_chain; - GURL referrer; net::ReferrerPolicy referrer_policy = net::ReferrerPolicy::NEVER_CLEAR; net::HttpRequestHeaders headers; @@ -103,8 +98,6 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) ResourceRequest { mojom::RedirectMode redirect_mode = mojom::RedirectMode::kFollow; std::string fetch_integrity; mojom::RequestDestination destination = mojom::RequestDestination::kEmpty; - mojom::RequestDestination original_destination = - mojom::RequestDestination::kEmpty; scoped_refptr<ResourceRequestBody> request_body; bool keepalive = false; bool has_user_gesture = false; diff --git a/chromium/services/network/public/cpp/url_request_mojom_traits.cc b/chromium/services/network/public/cpp/url_request_mojom_traits.cc index 6f7283a2687..d45b19b8f61 100644 --- a/chromium/services/network/public/cpp/url_request_mojom_traits.cc +++ b/chromium/services/network/public/cpp/url_request_mojom_traits.cc @@ -202,8 +202,7 @@ bool StructTraits< !data.ReadThrottlingProfileId(&out->throttling_profile_id) || !data.ReadFetchWindowId(&out->fetch_window_id) || !data.ReadDevtoolsRequestId(&out->devtools_request_id) || - !data.ReadRecursivePrefetchToken(&out->recursive_prefetch_token) || - !data.ReadNavigationRedirectChain(&out->navigation_redirect_chain)) { + !data.ReadRecursivePrefetchToken(&out->recursive_prefetch_token)) { // Note that data.ReadTrustTokenParams is temporarily handled below. return false; } @@ -243,7 +242,6 @@ bool StructTraits< out->is_signed_exchange_prefetch_cache_enabled = data.is_signed_exchange_prefetch_cache_enabled(); out->obey_origin_policy = data.obey_origin_policy(); - out->original_destination = data.original_destination(); return true; } diff --git a/chromium/services/network/public/cpp/url_request_mojom_traits.h b/chromium/services/network/public/cpp/url_request_mojom_traits.h index df94670c006..ea302870d45 100644 --- a/chromium/services/network/public/cpp/url_request_mojom_traits.h +++ b/chromium/services/network/public/cpp/url_request_mojom_traits.h @@ -109,10 +109,6 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) const network::ResourceRequest& request) { return request.request_initiator; } - static const std::vector<GURL> navigation_redirect_chain( - const network::ResourceRequest& request) { - return request.navigation_redirect_chain; - } static const base::Optional<url::Origin>& isolated_world_origin( const network::ResourceRequest& request) { return request.isolated_world_origin; @@ -252,10 +248,6 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) static bool obey_origin_policy(const network::ResourceRequest& request) { return request.obey_origin_policy; } - static network::mojom::RequestDestination original_destination( - const network::ResourceRequest& request) { - return request.original_destination; - } static const base::Optional<network::ResourceRequest::TrustedParams>& trusted_params(const network::ResourceRequest& request) { return request.trusted_params; diff --git a/chromium/services/network/public/mojom/url_loader.mojom b/chromium/services/network/public/mojom/url_loader.mojom index 72af370c1eb..ef862c8aefc 100644 --- a/chromium/services/network/public/mojom/url_loader.mojom +++ b/chromium/services/network/public/mojom/url_loader.mojom @@ -165,15 +165,6 @@ struct URLRequest { // - URLLoaderFactoryParams::request_initiator_origin_lock url.mojom.Origin? request_initiator; - // The chain of URLs seen during navigation redirects. This should only - // contain values if the mode is `RedirectMode::kNavigate`. This list - // will contain the initial network request URL, but not URLs from previous - // state in the DOM. For example, if a frame has URL A and sets its location - // to URL B, then the redirect chain will begin with URL B. The chain also - // includes the current request URL, however, it will not reflect any changes - // made by throttles. - array<url.mojom.Url> navigation_redirect_chain; - // If this is a subresource request initiated from an isolated world (e.g. // from a content script of a Chrome Extension), then // |isolated_world_origin| indicates the origin of the isolated world. @@ -395,10 +386,6 @@ struct URLRequest { // Spec: https://wicg.github.io/origin-policy/ bool obey_origin_policy; - // The original destination of a request that was passed through by a service - // worker. - RequestDestination original_destination; - // Setting these from an untrusted URLLoader will cause the request to fail. TrustedUrlRequestParams? trusted_params; diff --git a/chromium/services/network/url_loader.cc b/chromium/services/network/url_loader.cc index 30c6473442e..36b67163922 100644 --- a/chromium/services/network/url_loader.cc +++ b/chromium/services/network/url_loader.cc @@ -537,10 +537,6 @@ URLLoader::URLLoader( url_request_->set_site_for_cookies(request.site_for_cookies); url_request_->set_force_ignore_site_for_cookies( request.force_ignore_site_for_cookies); - if (!request.navigation_redirect_chain.empty()) { - DCHECK_EQ(request.mode, mojom::RequestMode::kNavigate); - url_request_->SetURLChain(request.navigation_redirect_chain); - } url_request_->SetReferrer(request.referrer.GetAsReferrer().spec()); url_request_->set_referrer_policy(request.referrer_policy); url_request_->set_upgrade_if_insecure(request.upgrade_if_insecure); @@ -564,18 +560,6 @@ URLLoader::URLLoader( if (url_request_context_->require_network_isolation_key()) DCHECK(!url_request_->isolation_info().IsEmpty()); - // When a service worker forwards a navigation request it uses the - // service worker's IsolationInfo. This causes the cookie code to fail - // to send SameSite=Lax cookies for main-frame navigations passed through - // a service worker. To fix this we check to see if the original destination - // of the request was a main frame document and then set a flag indicating - // SameSite cookies should treat it as a main frame navigation. - if (request.mode == mojom::RequestMode::kNavigate && - request.destination == mojom::RequestDestination::kEmpty && - request.original_destination == mojom::RequestDestination::kDocument) { - url_request_->set_force_main_frame_for_same_site_cookies(true); - } - if (factory_params_->disable_secure_dns) { url_request_->SetDisableSecureDns(true); } else if (request.trusted_params) { |