diff --git a/content/common/background_fetch/background_fetch_types.cc b/content/common/background_fetch/background_fetch_types.cc index 8a4561e5704d22..754e1ca70220ea 100644 --- a/content/common/background_fetch/background_fetch_types.cc +++ b/content/common/background_fetch/background_fetch_types.cc @@ -54,10 +54,11 @@ blink::mojom::FetchAPIRequestPtr BackgroundFetchSettledFetch::CloneRequest( request->mode, request->is_main_resource_load, request->destination, request->frame_type, request->url, request->method, request->headers, CloneSerializedBlob(request->blob), request->body, - request->referrer.Clone(), request->credentials_mode, request->cache_mode, - request->redirect_mode, request->integrity, request->priority, - request->fetch_window_id, request->keepalive, request->is_reload, - request->is_history_navigation, request->devtools_stack_id); + request->request_initiator, request->referrer.Clone(), + request->credentials_mode, request->cache_mode, request->redirect_mode, + request->integrity, request->priority, request->fetch_window_id, + request->keepalive, request->is_reload, request->is_history_navigation, + request->devtools_stack_id); } } // namespace content diff --git a/content/common/fetch/fetch_request_type_converters.cc b/content/common/fetch/fetch_request_type_converters.cc index ea4c9007ba8185..a04ab8c43e2606 100644 --- a/content/common/fetch/fetch_request_type_converters.cc +++ b/content/common/fetch/fetch_request_type_converters.cc @@ -27,6 +27,7 @@ blink::mojom::FetchAPIRequestPtr TypeConverter< // nullptr. if (input.request_body) output->body = input.request_body; + output->request_initiator = input.request_initiator; output->referrer = blink::mojom::Referrer::New( input.referrer, blink::ReferrerUtils::NetToMojoReferrerPolicy(input.referrer_policy)); diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc index aecb553e6b24f0..898c5b8a6bbabb 100644 --- a/net/url_request/url_request.cc +++ b/net/url_request/url_request.cc @@ -563,6 +563,7 @@ URLRequest::URLRequest(const GURL& url, url_chain_(1, url), force_ignore_site_for_cookies_(false), force_ignore_top_frame_party_for_cookies_(false), + force_main_frame_for_same_site_cookies_(false), method_("GET"), referrer_policy_( ReferrerPolicy::CLEAR_ON_TRANSITION_FROM_SECURE_TO_INSECURE), diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h index 8cca71046acf4f..90b56005b137d3 100644 --- a/net/url_request/url_request.h +++ b/net/url_request/url_request.h @@ -293,6 +293,16 @@ class NET_EXPORT URLRequest : public base::SupportsUserData { force_ignore_top_frame_party_for_cookies_ = force; } + // Indicates if the request should be treated as a main frame navigation for + // SameSite cookie computations. This flag overrides the IsolationInfo + // request type associated with fetches from a service worker context. + bool force_main_frame_for_same_site_cookies() const { + return force_main_frame_for_same_site_cookies_; + } + void set_force_main_frame_for_same_site_cookies(bool value) { + force_main_frame_for_same_site_cookies_ = value; + } + // The first-party URL policy to apply when updating the first party URL // during redirects. The first-party URL policy may only be changed before // Start() is called. @@ -913,6 +923,7 @@ class NET_EXPORT URLRequest : public base::SupportsUserData { bool force_ignore_site_for_cookies_; bool force_ignore_top_frame_party_for_cookies_; + bool force_main_frame_for_same_site_cookies_; absl::optional initiator_; GURL delegate_redirect_url_; std::string method_; // "GET", "POST", etc. Should be all uppercase. diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc index d816f325dec4b5..2e1817d58e8383 100644 --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -597,8 +597,10 @@ void URLRequestHttpJob::AddCookieHeaderAndStart() { request_->site_for_cookies())) { force_ignore_site_for_cookies = true; } - bool is_main_frame_navigation = IsolationInfo::RequestType::kMainFrame == - request_->isolation_info().request_type(); + bool is_main_frame_navigation = + IsolationInfo::RequestType::kMainFrame == + request_->isolation_info().request_type() || + request_->force_main_frame_for_same_site_cookies(); CookieOptions::SameSiteCookieContext same_site_context = net::cookie_util::ComputeSameSiteContextForRequest( request_->method(), request_->url_chain(), diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc index f493ed2afababc..6ae7ef3dee308a 100644 --- a/net/url_request/url_request_unittest.cc +++ b/net/url_request/url_request_unittest.cc @@ -2490,6 +2490,31 @@ TEST_P(URLRequestSameSiteCookiesTest, SameSiteCookies) { EXPECT_EQ(0, network_delegate.blocked_set_cookie_count()); } + // Verify that the lax cookie is sent for cross-site initiators when the + // method is "safe" and the request is being forced to be considered as a + // main frame navigation. + { + TestDelegate d; + std::unique_ptr req(default_context().CreateRequest( + test_server.GetURL(kHost, "/echoheader?Cookie"), DEFAULT_PRIORITY, &d, + TRAFFIC_ANNOTATION_FOR_TESTS)); + req->set_isolation_info(IsolationInfo::Create( + IsolationInfo::RequestType::kOther, kOrigin, kOrigin, kSiteForCookies, + {} /* party_context */)); + req->set_site_for_cookies(kSiteForCookies); + req->set_initiator(kCrossOrigin); + req->set_method("GET"); + req->set_force_main_frame_for_same_site_cookies(true); + req->Start(); + d.RunUntilComplete(); + + EXPECT_EQ(std::string::npos, + d.data_received().find("StrictSameSiteCookie=1")); + EXPECT_NE(std::string::npos, d.data_received().find("LaxSameSiteCookie=1")); + EXPECT_EQ(0, network_delegate.blocked_annotate_cookies_count()); + EXPECT_EQ(0, network_delegate.blocked_set_cookie_count()); + } + // Verify that neither cookie is sent for cross-site initiators when the // method is unsafe (e.g. POST), even if the request is a main frame // navigation. diff --git a/services/network/BUILD.gn b/services/network/BUILD.gn index 36f45d8b1bc02d..a03d089cf764a4 100644 --- a/services/network/BUILD.gn +++ b/services/network/BUILD.gn @@ -427,6 +427,7 @@ source_set("tests") { "//jingle/glue:fake_ssl_socket", "//mojo/public/cpp/bindings", "//mojo/public/cpp/system", + "//mojo/public/cpp/test_support:test_utils", "//net", "//net:extras", "//net:quic_test_tools", diff --git a/services/network/cors/cors_url_loader.cc b/services/network/cors/cors_url_loader.cc index 9b6f57365aa684..030588691a87df 100644 --- a/services/network/cors/cors_url_loader.cc +++ b/services/network/cors/cors_url_loader.cc @@ -294,6 +294,18 @@ void CorsURLLoader::FollowRedirect( const net::HttpRequestHeaders& modified_headers, const net::HttpRequestHeaders& modified_cors_exempt_headers, const absl::optional& 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/services/network/cors/cors_url_loader_factory.cc b/services/network/cors/cors_url_loader_factory.cc index f389045971e1da..ed24bbb7e88590 100644 --- a/services/network/cors/cors_url_loader_factory.cc +++ b/services/network/cors/cors_url_loader_factory.cc @@ -383,18 +383,57 @@ 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; + } + + // By default we compare the `request_initiator` to the lock below. This is + // overridden for renderer navigations, however. + absl::optional 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: - // 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; + // 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; + } + + break; case mojom::RequestMode::kSameOrigin: case mojom::RequestMode::kCors: @@ -408,11 +447,11 @@ bool CorsURLLoaderFactory::IsValidRequest(const ResourceRequest& request, } } - // Compare |request_initiator| and |request_initiator_origin_lock_|. + // Depending on the type of request, compare either `request_initiator` or + // `request.url` to `request_initiator_origin_lock_`. InitiatorLockCompatibility initiator_lock_compatibility = - VerifyRequestInitiatorLockWithPluginCheck(process_id_, - request_initiator_origin_lock_, - request.request_initiator); + VerifyRequestInitiatorLockWithPluginCheck( + process_id_, request_initiator_origin_lock_, origin_to_validate); UMA_HISTOGRAM_ENUMERATION( "NetworkService.URLLoader.RequestInitiatorOriginLockCompatibility", initiator_lock_compatibility); @@ -440,7 +479,6 @@ bool CorsURLLoaderFactory::IsValidRequest(const ResourceRequest& request, case InitiatorLockCompatibility::kIncorrectLock: // Requests from the renderer need to always specify a correct initiator. - NOTREACHED(); mojo::ReportBadMessage( "CorsURLLoaderFactory: lock VS initiator mismatch"); return false; diff --git a/services/network/cors/cors_url_loader_factory_unittest.cc b/services/network/cors/cors_url_loader_factory_unittest.cc index 5164abca8319ee..d91abe967c2420 100644 --- a/services/network/cors/cors_url_loader_factory_unittest.cc +++ b/services/network/cors/cors_url_loader_factory_unittest.cc @@ -7,6 +7,8 @@ #include "base/macros.h" #include "base/test/task_environment.h" #include "mojo/public/cpp/bindings/remote.h" +#include "mojo/public/cpp/test_support/fake_message_dispatch_context.h" +#include "mojo/public/cpp/test_support/test_utils.h" #include "net/base/load_flags.h" #include "net/proxy_resolution/configured_proxy_resolution_service.h" #include "net/test/embedded_test_server/embedded_test_server.h" @@ -111,6 +113,7 @@ class CorsURLLoaderFactoryTest : public testing::Test { private: // Test environment. base::test::TaskEnvironment task_environment_; + mojo::FakeMessageDispatchContext mojo_context_; std::unique_ptr url_request_context_; ResourceScheduler resource_scheduler_; std::unique_ptr network_service_; @@ -190,5 +193,77 @@ TEST_F(CorsURLLoaderFactoryTest, CleanupWithSharedCacheObjectInUse) { ResetFactory(); } +TEST_F(CorsURLLoaderFactoryTest, + NavigationFromRendererWithBadRequestURLOrigin) { + ResourceRequest request; + GURL url = test_server()->GetURL("/echoall"); + request.mode = mojom::RequestMode::kNavigate; + request.redirect_mode = mojom::RedirectMode::kManual; + request.destination = mojom::RequestDestination::kEmpty; + request.method = net::HttpRequestHeaders::kPostMethod; + request.url = GURL("https://some.other.origin/echoall"); + request.request_initiator = url::Origin::Create(url); + mojo::test::BadMessageObserver bad_message_observer; + CreateLoaderAndStart(request); + EXPECT_EQ("CorsURLLoaderFactory: lock VS initiator mismatch", + bad_message_observer.WaitForBadMessage()); +} + +TEST_F(CorsURLLoaderFactoryTest, NavigationFromRendererWithBadRedirectMode) { + ResourceRequest request; + GURL url = test_server()->GetURL("/echoall"); + request.mode = mojom::RequestMode::kNavigate; + request.redirect_mode = mojom::RedirectMode::kFollow; + request.destination = mojom::RequestDestination::kEmpty; + request.method = net::HttpRequestHeaders::kPostMethod; + request.url = url; + request.request_initiator = url::Origin::Create(url).DeriveNewOpaqueOrigin(); + mojo::test::BadMessageObserver bad_message_observer; + CreateLoaderAndStart(request); + EXPECT_EQ( + "CorsURLLoaderFactory: navigate from non-browser-process with " + "redirect_mode set to 'follow'", + bad_message_observer.WaitForBadMessage()); +} + +TEST_F(CorsURLLoaderFactoryTest, OriginalDestinationIsDocumentWithBadMode) { + ResourceRequest request; + GURL url = test_server()->GetURL("/echoall"); + request.mode = mojom::RequestMode::kCors; + request.redirect_mode = mojom::RedirectMode::kFollow; + request.destination = mojom::RequestDestination::kEmpty; + request.method = net::HttpRequestHeaders::kGetMethod; + request.url = url; + request.request_initiator = + url::Origin::Create(GURL("https://some.other.origin")); + request.original_destination = mojom::RequestDestination::kDocument; + mojo::test::BadMessageObserver bad_message_observer; + CreateLoaderAndStart(request); + EXPECT_EQ( + "CorsURLLoaderFactory: original_destination is unexpectedly set to " + "kDocument", + bad_message_observer.WaitForBadMessage()); +} + +TEST_F(CorsURLLoaderFactoryTest, + OriginalDestinationIsDocumentWithBadDestination) { + ResourceRequest request; + GURL url = test_server()->GetURL("/echoall"); + request.mode = mojom::RequestMode::kNavigate; + request.redirect_mode = mojom::RedirectMode::kManual; + request.destination = mojom::RequestDestination::kIframe; + request.method = net::HttpRequestHeaders::kGetMethod; + request.url = url; + request.request_initiator = + url::Origin::Create(GURL("https://some.other.origin")); + request.original_destination = mojom::RequestDestination::kDocument; + mojo::test::BadMessageObserver bad_message_observer; + CreateLoaderAndStart(request); + EXPECT_EQ( + "CorsURLLoaderFactory: original_destination is unexpectedly set to " + "kDocument", + bad_message_observer.WaitForBadMessage()); +} + } // namespace cors } // namespace network diff --git a/services/network/cors/cors_url_loader_unittest.cc b/services/network/cors/cors_url_loader_unittest.cc index 29758490b6f957..18b661eb1292cf 100644 --- a/services/network/cors/cors_url_loader_unittest.cc +++ b/services/network/cors/cors_url_loader_unittest.cc @@ -207,12 +207,16 @@ class CorsURLLoaderTest : public testing::Test { } void SetUp() override { SetUp(mojom::NetworkContextParams::New()); } - void CreateLoaderAndStart(const GURL& origin, - const GURL& url, - mojom::RequestMode mode) { + void CreateLoaderAndStart( + const GURL& origin, + const GURL& url, + mojom::RequestMode mode, + mojom::RedirectMode redirect_mode = mojom::RedirectMode::kFollow, + mojom::CredentialsMode credentials_mode = mojom::CredentialsMode::kOmit) { ResourceRequest request; request.mode = mode; - request.credentials_mode = mojom::CredentialsMode::kOmit; + request.redirect_mode = redirect_mode; + request.credentials_mode = credentials_mode; request.method = net::HttpRequestHeaders::kGetMethod; request.url = url; request.request_initiator = url::Origin::Create(origin); @@ -702,9 +706,13 @@ TEST_F(CorsURLLoaderTest, NavigateWithEarlyHints) { } TEST_F(CorsURLLoaderTest, NavigationFromRenderer) { + ResetFactory(url::Origin::Create(GURL("https://example.com/")), + kRendererProcessId); + ResourceRequest request; request.mode = mojom::RequestMode::kNavigate; - request.url = GURL("https://example.com/"); + request.redirect_mode = mojom::RedirectMode::kManual; + request.url = GURL("https://some.other.example.com/"); request.request_initiator = absl::nullopt; BadMessageTestHelper bad_message_helper; @@ -718,7 +726,7 @@ TEST_F(CorsURLLoaderTest, NavigationFromRenderer) { EXPECT_EQ(net::ERR_INVALID_ARGUMENT, client().completion_status().error_code); EXPECT_THAT(bad_message_helper.bad_message_reports(), ::testing::ElementsAre( - "CorsURLLoaderFactory: navigate from non-browser-process")); + "CorsURLLoaderFactory: lock VS initiator mismatch")); } TEST_F(CorsURLLoaderTest, SameOriginRequest) { @@ -2875,6 +2883,39 @@ TEST_F(CorsURLLoaderTest, PreflightMissingAllowOrigin) { mojom::CorsError::kPreflightMissingAllowOriginHeader))); } +TEST_F(CorsURLLoaderTest, NonBrowserNavigationRedirect) { + BadMessageTestHelper bad_message_helper; + + const GURL origin("https://example.com"); + const GURL url("https://example.com/foo.png"); + const GURL new_url("https://example.com/bar.png"); + + CreateLoaderAndStart(origin, url, mojom::RequestMode::kNavigate, + mojom::RedirectMode::kManual, + mojom::CredentialsMode::kInclude); + RunUntilCreateLoaderAndStartCalled(); + + EXPECT_EQ(1, num_created_loaders()); + EXPECT_EQ(GetRequest().url, url); + EXPECT_EQ(GetRequest().method, "GET"); + + NotifyLoaderClientOnReceiveRedirect(CreateRedirectInfo(301, "GET", new_url)); + RunUntilRedirectReceived(); + + EXPECT_TRUE(IsNetworkLoaderStarted()); + EXPECT_FALSE(client().has_received_completion()); + EXPECT_FALSE(client().has_received_response()); + EXPECT_TRUE(client().has_received_redirect()); + + FollowRedirect(); + + RunUntilComplete(); + EXPECT_THAT( + bad_message_helper.bad_message_reports(), + ::testing::ElementsAre("CorsURLLoader: navigate from non-browser-process " + "should not call FollowRedirect")); +} + } // namespace } // namespace cors diff --git a/services/network/public/cpp/resource_request.h b/services/network/public/cpp/resource_request.h index 490011956f54e3..f1432e136c6b85 100644 --- a/services/network/public/cpp/resource_request.h +++ b/services/network/public/cpp/resource_request.h @@ -144,6 +144,8 @@ 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 request_body; bool keepalive = false; bool has_user_gesture = false; diff --git a/services/network/public/cpp/url_request_mojom_traits.cc b/services/network/public/cpp/url_request_mojom_traits.cc index a1c4a34739a8fc..25b59ed3be2f81 100644 --- a/services/network/public/cpp/url_request_mojom_traits.cc +++ b/services/network/public/cpp/url_request_mojom_traits.cc @@ -218,6 +218,7 @@ bool StructTraits< out->is_fetch_like_api = data.is_fetch_like_api(); out->is_favicon = data.is_favicon(); out->obey_origin_policy = data.obey_origin_policy(); + out->original_destination = data.original_destination(); out->target_ip_address_space = data.target_ip_address_space(); return true; } diff --git a/services/network/public/cpp/url_request_mojom_traits.h b/services/network/public/cpp/url_request_mojom_traits.h index 2d9ba6918b993e..64b91e6d4a18c1 100644 --- a/services/network/public/cpp/url_request_mojom_traits.h +++ b/services/network/public/cpp/url_request_mojom_traits.h @@ -317,6 +317,10 @@ 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 absl::optional>& devtools_accepted_stream_types(const network::ResourceRequest& request) { return request.devtools_accepted_stream_types; diff --git a/services/network/public/mojom/url_request.mojom b/services/network/public/mojom/url_request.mojom index 571ecc5eac8bb2..5b1708bf03bb80 100644 --- a/services/network/public/mojom/url_request.mojom +++ b/services/network/public/mojom/url_request.mojom @@ -378,6 +378,10 @@ 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; + // Params intended to be set by a trusted requestor which sends the request // to a trusted URLLoaderFactory. Note that the main implementation of // URLLoaderFactory is the one created in the network service via diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc index 30e9ceb04fd579..2fec9ab680227f 100644 --- a/services/network/url_loader.cc +++ b/services/network/url_loader.cc @@ -610,6 +610,18 @@ URLLoader::URLLoader( if (ShouldForceIgnoreTopFramePartyForCookies()) url_request_->set_force_ignore_top_frame_party_for_cookies(true); + // 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 || (request.trusted_params && request.trusted_params->disable_secure_dns)) { url_request_->SetSecureDnsPolicy(net::SecureDnsPolicy::kDisable); diff --git a/third_party/blink/public/mojom/fetch/fetch_api_request.mojom b/third_party/blink/public/mojom/fetch/fetch_api_request.mojom index 8e30381eef6751..e952c541d5b1b9 100644 --- a/third_party/blink/public/mojom/fetch/fetch_api_request.mojom +++ b/third_party/blink/public/mojom/fetch/fetch_api_request.mojom @@ -14,6 +14,7 @@ import "services/network/public/mojom/url_request.mojom"; import "third_party/blink/public/mojom/blob/serialized_blob.mojom"; import "third_party/blink/public/mojom/loader/request_context_frame_type.mojom"; import "third_party/blink/public/mojom/loader/referrer.mojom"; +import "url/mojom/origin.mojom"; import "url/mojom/url.mojom"; @@ -162,6 +163,14 @@ struct FetchAPIRequest { SerializedBlob? blob; FetchAPIRequestBody? body; + // `request_initiator` indicates the origin that initiated the request. See + // also `network::ResourceRequest::request_initiator`, and the doc comment + // for `request_initiator` in services/network/public/mojom/url_request.mojom. + // + // Note that the origin may be missing for browser-initiated navigations + // (e.g. ones initiated from the Omnibox). + url.mojom.Origin? request_initiator; + Referrer? referrer; network.mojom.CredentialsMode credentials_mode = network.mojom.CredentialsMode.kOmit; diff --git a/third_party/blink/renderer/core/fetch/fetch_manager.cc b/third_party/blink/renderer/core/fetch/fetch_manager.cc index 30f55d7d3cea2d..b73932e10e3ba6 100644 --- a/third_party/blink/renderer/core/fetch/fetch_manager.cc +++ b/third_party/blink/renderer/core/fetch/fetch_manager.cc @@ -762,22 +762,7 @@ void FetchManager::Loader::PerformHTTPFetch() { request.SetHttpMethod(fetch_request_data_->Method()); request.SetFetchWindowId(fetch_request_data_->WindowId()); request.SetTrustTokenParams(fetch_request_data_->TrustTokenParams()); - - switch (fetch_request_data_->Mode()) { - case RequestMode::kSameOrigin: - case RequestMode::kNoCors: - case RequestMode::kCors: - case RequestMode::kCorsWithForcedPreflight: - request.SetMode(fetch_request_data_->Mode()); - break; - case RequestMode::kNavigate: - // NetworkService (i.e. CorsURLLoaderFactory::IsSane) rejects kNavigate - // requests coming from renderers, so using kSameOrigin here. - // TODO(lukasza): Tweak CorsURLLoaderFactory::IsSane to accept kNavigate - // if request_initiator and the target are same-origin. - request.SetMode(RequestMode::kSameOrigin); - break; - } + request.SetMode(fetch_request_data_->Mode()); request.SetCredentialsMode(fetch_request_data_->Credentials()); for (const auto& header : fetch_request_data_->HeaderList()->List()) { @@ -825,6 +810,8 @@ void FetchManager::Loader::PerformHTTPFetch() { UseCounter::Count(execution_context_, mojom::WebFeature::kFetchKeepalive); } + request.SetOriginalDestination(fetch_request_data_->OriginalDestination()); + // "3. Append `Host`, ..." // FIXME: Implement this when the spec is fixed. diff --git a/third_party/blink/renderer/core/fetch/fetch_request_data.cc b/third_party/blink/renderer/core/fetch/fetch_request_data.cc index 7e569ba2100aa9..314d379cee836a 100644 --- a/third_party/blink/renderer/core/fetch/fetch_request_data.cc +++ b/third_party/blink/renderer/core/fetch/fetch_request_data.cc @@ -164,6 +164,8 @@ FetchRequestData* FetchRequestData::Create( // we deprecate SetContext. request->SetDestination(fetch_api_request->destination); + if (fetch_api_request->request_initiator) + request->SetOrigin(fetch_api_request->request_initiator); request->SetReferrerString(AtomicString(Referrer::NoReferrer())); if (fetch_api_request->referrer) { if (!fetch_api_request->referrer->url.IsEmpty()) { @@ -184,6 +186,7 @@ FetchRequestData* FetchRequestData::Create( fetch_api_request->priority)); if (fetch_api_request->fetch_window_id) request->SetWindowId(fetch_api_request->fetch_window_id.value()); + return request; } @@ -205,6 +208,7 @@ FetchRequestData* FetchRequestData::CloneExceptBody() { request->integrity_ = integrity_; request->priority_ = priority_; request->importance_ = importance_; + request->original_destination_ = original_destination_; request->keepalive_ = keepalive_; request->is_history_navigation_ = is_history_navigation_; request->window_id_ = window_id_; diff --git a/third_party/blink/renderer/core/fetch/fetch_request_data.h b/third_party/blink/renderer/core/fetch/fetch_request_data.h index 02b690998d3aa7..3465196f08569d 100644 --- a/third_party/blink/renderer/core/fetch/fetch_request_data.h +++ b/third_party/blink/renderer/core/fetch/fetch_request_data.h @@ -108,6 +108,15 @@ class CORE_EXPORT FetchRequestData final void SetIntegrity(const String& integrity) { integrity_ = integrity; } ResourceLoadPriority Priority() const { return priority_; } void SetPriority(ResourceLoadPriority priority) { priority_ = priority; } + + // The original destination of a request passed through by a service worker. + void SetOriginalDestination(network::mojom::RequestDestination value) { + original_destination_ = value; + } + network::mojom::RequestDestination OriginalDestination() const { + return original_destination_; + } + bool Keepalive() const { return keepalive_; } void SetKeepalive(bool b) { keepalive_ = b; } bool IsHistoryNavigation() const { return is_history_navigation_; } @@ -174,6 +183,8 @@ class CORE_EXPORT FetchRequestData final String mime_type_; String integrity_; ResourceLoadPriority priority_; + network::mojom::RequestDestination original_destination_ = + network::mojom::RequestDestination::kEmpty; bool keepalive_; bool is_history_navigation_ = false; // A specific factory that should be used for this request instead of whatever diff --git a/third_party/blink/renderer/core/fetch/request.cc b/third_party/blink/renderer/core/fetch/request.cc index 9cf7fe8c115906..8421ed69d094b6 100644 --- a/third_party/blink/renderer/core/fetch/request.cc +++ b/third_party/blink/renderer/core/fetch/request.cc @@ -70,7 +70,8 @@ FetchRequestData* CreateCopyOfFetchRequestDataForFetch( request->SetURL(original->Url()); request->SetMethod(original->Method()); request->SetHeaderList(original->HeaderList()->Clone()); - request->SetOrigin(context->GetSecurityOrigin()); + request->SetOrigin(original->Origin() ? original->Origin() + : context->GetSecurityOrigin()); // FIXME: Set client. DOMWrapperWorld& world = script_state->World(); if (world.IsIsolatedWorld()) { @@ -97,6 +98,18 @@ FetchRequestData* CreateCopyOfFetchRequestDataForFetch( } request->SetWindowId(original->WindowId()); request->SetTrustTokenParams(original->TrustTokenParams()); + + // When a new request is created from another the destination is always reset + // to be `kEmpty`. In order to facilitate some later checks when a service + // worker forwards a navigation request we want to keep track of the + // destination of the original request. Therefore record the original + // request's destination if its non-empty, otherwise just carry forward + // whatever "original destination" value was already set. + if (original->Destination() != network::mojom::RequestDestination::kEmpty) + request->SetOriginalDestination(original->Destination()); + else + request->SetOriginalDestination(original->OriginalDestination()); + return request; } @@ -319,6 +332,9 @@ Request* Request::CreateRequestWithRequestOrString( // "If any of |init|'s members are present, then:" if (AreAnyMembersPresent(init)) { + request->SetOrigin(execution_context->GetSecurityOrigin()); + request->SetOriginalDestination(network::mojom::RequestDestination::kEmpty); + // "If |request|'s |mode| is "navigate", then set it to "same-origin". if (request->Mode() == network::mojom::RequestMode::kNavigate) request->SetMode(network::mojom::RequestMode::kSameOrigin); @@ -982,6 +998,7 @@ mojom::blink::FetchAPIRequestPtr Request::CreateFetchAPIRequest() const { fetch_api_request->integrity = request_->Integrity(); fetch_api_request->is_history_navigation = request_->IsHistoryNavigation(); fetch_api_request->destination = request_->Destination(); + fetch_api_request->request_initiator = request_->Origin(); // Strip off the fragment part of URL. So far, all callers expect the fragment // to be excluded. diff --git a/third_party/blink/renderer/modules/cache_storage/inspector_cache_storage_agent.cc b/third_party/blink/renderer/modules/cache_storage/inspector_cache_storage_agent.cc index 3ae61406740879..cad84d21d03e2b 100644 --- a/third_party/blink/renderer/modules/cache_storage/inspector_cache_storage_agent.cc +++ b/third_party/blink/renderer/modules/cache_storage/inspector_cache_storage_agent.cc @@ -268,11 +268,12 @@ class ResponsesAccumulator : public RefCounted { auto request_clone_without_body = mojom::blink::FetchAPIRequest::New( request->mode, request->is_main_resource_load, request->destination, request->frame_type, request->url, request->method, request->headers, - nullptr /* blob */, ResourceRequestBody(), request->referrer.Clone(), - request->credentials_mode, request->cache_mode, - request->redirect_mode, request->integrity, request->priority, - request->fetch_window_id, request->keepalive, request->is_reload, - request->is_history_navigation, request->devtools_stack_id); + nullptr /* blob */, ResourceRequestBody(), request->request_initiator, + request->referrer.Clone(), request->credentials_mode, + request->cache_mode, request->redirect_mode, request->integrity, + request->priority, request->fetch_window_id, request->keepalive, + request->is_reload, request->is_history_navigation, + request->devtools_stack_id); cache_remote_->Match( std::move(request), mojom::blink::CacheQueryOptions::New(), /*in_related_fetch_event=*/false, /*in_range_fetch_event=*/false, diff --git a/third_party/blink/renderer/platform/loader/fetch/resource_request.h b/third_party/blink/renderer/platform/loader/fetch/resource_request.h index 172797442ce10d..fbc30546d6ce65 100644 --- a/third_party/blink/renderer/platform/loader/fetch/resource_request.h +++ b/third_party/blink/renderer/platform/loader/fetch/resource_request.h @@ -513,6 +513,14 @@ class PLATFORM_EXPORT ResourceRequestHead { return allowHTTP1ForStreamingUpload_; } + // The original destination of a request passed through by a service worker. + network::mojom::RequestDestination GetOriginalDestination() const { + return original_destination_; + } + void SetOriginalDestination(network::mojom::RequestDestination value) { + original_destination_ = value; + } + const absl::optional& GetWebBundleTokenParams() const { return web_bundle_token_params_; @@ -603,6 +611,9 @@ class PLATFORM_EXPORT ResourceRequestHead { base::UnguessableToken fetch_window_id_; + network::mojom::RequestDestination original_destination_ = + network::mojom::RequestDestination::kEmpty; + uint64_t inspector_id_ = 0; bool is_from_origin_dirty_style_sheet_ = false; diff --git a/third_party/blink/renderer/platform/loader/fetch/url_loader/request_conversion.cc b/third_party/blink/renderer/platform/loader/fetch/url_loader/request_conversion.cc index 0ead3265450d55..7c8097f0ec3290 100644 --- a/third_party/blink/renderer/platform/loader/fetch/url_loader/request_conversion.cc +++ b/third_party/blink/renderer/platform/loader/fetch/url_loader/request_conversion.cc @@ -420,6 +420,8 @@ void PopulateResourceRequest(const ResourceRequestHead& src, dest->headers.SetHeaderIfMissing(net::HttpRequestHeaders::kAccept, network::kDefaultAcceptHeaderValue); } + + dest->original_destination = src.GetOriginalDestination(); } } // namespace blink diff --git a/third_party/blink/web_tests/external/wpt/service-workers/service-worker/navigation-headers.https.html b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/navigation-headers.https.html new file mode 100644 index 00000000000000..410b8e4631c19d --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/navigation-headers.https.html @@ -0,0 +1,558 @@ + + +Service Worker: Navigation Post Request Origin Header + + + + + + + diff --git a/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/fetch-rewrite-worker.js b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/fetch-rewrite-worker.js index 4631e83e0ceaab..20a80665270ddb 100644 --- a/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/fetch-rewrite-worker.js +++ b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/fetch-rewrite-worker.js @@ -90,8 +90,12 @@ self.addEventListener('fetch', function(event) { var request = event.request; if (url) { request = new Request(url, init); + } else if (params['change-request']) { + request = new Request(request, init); } - fetch(request).then(function(response) { + const response_promise = params['navpreload'] ? event.preloadResponse + : fetch(request); + response_promise.then(function(response) { var expectedType = params['expected_type']; if (expectedType && response.type !== expectedType) { // Resolve a JSON object with a failure instead of rejecting diff --git a/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/fetch-rewrite-worker.js.headers b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/fetch-rewrite-worker.js.headers new file mode 100644 index 00000000000000..123053b38c66a0 --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/fetch-rewrite-worker.js.headers @@ -0,0 +1,2 @@ +Content-Type: text/javascript +Service-Worker-Allowed: / diff --git a/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/form-poster.html b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/form-poster.html new file mode 100644 index 00000000000000..5d56fde19a8e4f --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/form-poster.html @@ -0,0 +1,12 @@ + + +
+ diff --git a/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/location-setter.html b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/location-setter.html new file mode 100644 index 00000000000000..fae18e8066550a --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/location-setter.html @@ -0,0 +1,10 @@ + + + diff --git a/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/navigation-headers-server.py b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/navigation-headers-server.py new file mode 100644 index 00000000000000..5b2e044f8b52a1 --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/navigation-headers-server.py @@ -0,0 +1,19 @@ +def main(request, response): + response.status = (200, b"OK") + response.headers.set(b"Content-Type", b"text/html") + return b""" + """ % (request.headers.get( + b"origin", b"not set"), request.headers.get(b"referer", b"not set"), + request.headers.get(b"sec-fetch-site", b"not set"), + request.headers.get(b"sec-fetch-mode", b"not set"), + request.headers.get(b"sec-fetch-dest", b"not set")) diff --git a/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/same-site-cookies-register.html b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/same-site-cookies-register.html new file mode 100644 index 00000000000000..084f0a08a8e64c --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/same-site-cookies-register.html @@ -0,0 +1,22 @@ + + + diff --git a/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/same-site-cookies-unregister.html b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/same-site-cookies-unregister.html new file mode 100644 index 00000000000000..cca3620b61e73c --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/resources/same-site-cookies-unregister.html @@ -0,0 +1,11 @@ + + + diff --git a/third_party/blink/web_tests/external/wpt/service-workers/service-worker/same-site-cookies.https-expected.txt b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/same-site-cookies.https-expected.txt new file mode 100644 index 00000000000000..96ebf3b64938e7 --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/same-site-cookies.https-expected.txt @@ -0,0 +1,32 @@ +This is a testharness.js-based test. +PASS Setup service workers +PASS same-origin, window.open with no service worker +PASS same-origin, window.open with fallback +PASS same-origin, window.open with passthrough +PASS same-origin, window.open with change-request +PASS same-origin, window.open with navpreload +PASS same-site, window.open with no service worker +PASS same-site, window.open with fallback +PASS same-site, window.open with passthrough +PASS same-site, window.open with change-request +PASS same-site, window.open with navpreload +PASS cross-site, window.open with no service worker +PASS cross-site, window.open with fallback +PASS cross-site, window.open with passthrough +PASS cross-site, window.open with change-request +PASS cross-site, window.open with navpreload +PASS same-origin, form post with no service worker +PASS same-origin, form post with fallback +PASS same-origin, form post with passthrough +PASS same-origin, form post with change-request +PASS same-site, form post with no service worker +PASS same-site, form post with fallback +PASS same-site, form post with passthrough +PASS same-site, form post with change-request +FAIL cross-site, form post with no service worker assert_not_equals: Unspecified-SameSite cookies are not sent with cross-site requests. got disallowed value "COOKIE_VALUE" +FAIL cross-site, form post with fallback assert_not_equals: Unspecified-SameSite cookies are not sent with cross-site requests. got disallowed value "COOKIE_VALUE" +FAIL cross-site, form post with passthrough assert_not_equals: Unspecified-SameSite cookies are not sent with cross-site requests. got disallowed value "COOKIE_VALUE" +PASS cross-site, form post with change-request +PASS Cleanup service workers +Harness: the test ran to completion. + diff --git a/third_party/blink/web_tests/external/wpt/service-workers/service-worker/same-site-cookies.https.html b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/same-site-cookies.https.html new file mode 100644 index 00000000000000..54c42c84d80231 --- /dev/null +++ b/third_party/blink/web_tests/external/wpt/service-workers/service-worker/same-site-cookies.https.html @@ -0,0 +1,215 @@ + + +Service Worker: Same-site cookie behavior + + + + + + + +