Revert "Fetch: Plumb request initiator through passthrough service wo…

…rkers."

This reverts commit da0a650.

Reason for revert: Failure on many bots with the following error message:
The service worker navigation preload request was cancelled before 'preloadResponse' settled. If you intend to use 'preloadResponse', use waitUntil() or respondWith() to wait for the promise to settle.", source:  (0)

Original change's description:
> Fetch: Plumb request initiator through passthrough service workers.
>
> This CL contains essentially two changes:
>
> 1. The request initiator origin is plumbed through service workers
>    that do `fetch(evt.request)`.  In addition to plumbing, this
>    requires changes to how we validate navigation requests in the
>    CorsURLLoaderFactory.
> 2. Tracks the original destination of a request passed through a
>    service worker.  This is then used in the network service to force
>    SameSite=Lax cookies to treat the request as a main frame navigation
>    where appropriate.
>
> For more detailed information about these changes please see the
> internal design doc at:
>
> https://docs.google.com/document/d/1KZscujuV7bCFEnzJW-0DaCPU-I40RJimQKoCcI0umTQ/edit?usp=sharing
>
> In addition, there is some discussion of these features in the following
> spec issues:
>
> whatwg/fetch#1321
> whatwg/fetch#1327
>
> The test includes WPT tests that verify navigation headers and SameSite
> cookies.  Note, chrome has a couple expected failures in the SameSite
> cookie tests because of the "lax-allowing-unsafe" intervention that is
> currently enabled.  See:
>
> https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/TestExpectations;l=4635;drc=e8133cbf2469adb99c6610483ab78bcfb8cc4c76
>
> Bug: 1115847,1241188
> Change-Id: I7e236fa20aeabb705aef40fcf8d5c36da6d2798c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3115917
> Reviewed-by: Matt Menke <mmenke@chromium.org>
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Reviewed-by: Nasko Oskov <nasko@chromium.org>
> Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
> Commit-Queue: Ben Kelly <wanderview@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#936029}

Bug: 1115847,1241188
Change-Id: I3044a6d20de172b4a8ab7e39a9f26191580003fa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3251692
Auto-Submit: Alan Screen <awscreen@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Alan Screen <awscreen@chromium.org>
Owners-Override: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#936125}

content/common/background_fetch/background_fetch_types.cc

@@ -54,11 +54,10 @@ 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->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);
+      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

content/common/fetch/fetch_request_type_converters.cc

@@ -27,7 +27,6 @@ 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));

net/url_request/url_request.cc

@@ -563,7 +563,6 @@ 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),

net/url_request/url_request.h

@@ -293,16 +293,6 @@ 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.
@@ -923,7 +913,6 @@ 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<url::Origin> initiator_;
   GURL delegate_redirect_url_;
   std::string method_;  // "GET", "POST", etc. Should be all uppercase.

net/url_request/url_request_http_job.cc

@@ -597,10 +597,8 @@ 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() ||
-        request_->force_main_frame_for_same_site_cookies();
+    bool is_main_frame_navigation = IsolationInfo::RequestType::kMainFrame ==
+                                    request_->isolation_info().request_type();
     CookieOptions::SameSiteCookieContext same_site_context =
         net::cookie_util::ComputeSameSiteContextForRequest(
             request_->method(), request_->url_chain(),

net/url_request/url_request_unittest.cc

@@ -2490,31 +2490,6 @@ 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<URLRequest> 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.

services/network/BUILD.gn

@@ -427,7 +427,6 @@ 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",

services/network/cors/cors_url_loader.cc

@@ -294,18 +294,6 @@ void CorsURLLoader::FollowRedirect(
     const net::HttpRequestHeaders& modified_headers,
     const net::HttpRequestHeaders& modified_cors_exempt_headers,
     const absl::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;

services/network/cors/cors_url_loader_factory.cc

@@ -383,57 +383,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;
-  }
-
-  // By default we compare the `request_initiator` to the lock below.  This is
-  // overridden for renderer navigations, however.
-  absl::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;
-        }
-
-        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:
@@ -447,11 +408,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);
@@ -479,6 +440,7 @@ 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;

services/network/cors/cors_url_loader_factory_unittest.cc

@@ -7,8 +7,6 @@
 #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"
@@ -113,7 +111,6 @@ class CorsURLLoaderFactoryTest : public testing::Test {
  private:
   // Test environment.
   base::test::TaskEnvironment task_environment_;
-  mojo::FakeMessageDispatchContext mojo_context_;
   std::unique_ptr<net::URLRequestContext> url_request_context_;
   ResourceScheduler resource_scheduler_;
   std::unique_ptr<NetworkService> network_service_;
@@ -193,77 +190,5 @@ 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

services/network/cors/cors_url_loader_unittest.cc

@@ -207,16 +207,12 @@ class CorsURLLoaderTest : public testing::Test {
   }
   void SetUp() override { SetUp(mojom::NetworkContextParams::New()); }
 
-  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) {
+  void CreateLoaderAndStart(const GURL& origin,
+                            const GURL& url,
+                            mojom::RequestMode mode) {
     ResourceRequest request;
     request.mode = mode;
-    request.redirect_mode = redirect_mode;
-    request.credentials_mode = credentials_mode;
+    request.credentials_mode = mojom::CredentialsMode::kOmit;
     request.method = net::HttpRequestHeaders::kGetMethod;
     request.url = url;
     request.request_initiator = url::Origin::Create(origin);
@@ -706,13 +702,9 @@ 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.redirect_mode = mojom::RedirectMode::kManual;
-  request.url = GURL("https://some.other.example.com/");
+  request.url = GURL("https://example.com/");
   request.request_initiator = absl::nullopt;
 
   BadMessageTestHelper bad_message_helper;
@@ -726,7 +718,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: lock VS initiator mismatch"));
+                  "CorsURLLoaderFactory: navigate from non-browser-process"));
 }
 
 TEST_F(CorsURLLoaderTest, SameOriginRequest) {
@@ -2883,39 +2875,6 @@ 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