Enforce presence of |request_initiator_site_lock| in renderer factories.

Thanks to an earlier CL (https://crrev.com/c/2274591), this CL can
modify CorsURLLoaderFactory::IsValidRequest to enforce the presence of
|request_initiator_site_lock| (see the mojo::ReportBadMessage call in
the InitiatorLockCompatibility::kNoLock case).  This is a big security
win - it means that the |request_initiator| can no longer be spoofed on
the Android platform (the CL modifies
//docs/security/compromised-renderers.md accordingly).

Bug: 1114906
Change-Id: I9e52ea33d8e929e31c1b4bfaf8fc85e5d2e185f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2347195
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799116}

content/renderer/loader/child_url_loader_factory_bundle.cc

@@ -242,13 +242,20 @@ network::mojom::URLLoaderFactory* ChildURLLoaderFactoryBundle::GetFactory(
 
   // All renderer-initiated requests need to provide a value for
   // |request_initiator| - this is enforced by
-  // CorsURLLoaderFactory::IsValidRequest.
+  // CorsURLLoaderFactory::IsValidRequest (see the
+  // InitiatorLockCompatibility::kNoInitiator case).
   DCHECK(request.request_initiator.has_value());
-  if (is_deprecated_process_wide_factory_ &&
-      !request.request_initiator->opaque()) {
-    NOTREACHED();
-    ScopedRequestCrashKeys crash_keys(request);
-    base::debug::DumpWithoutCrashing();
+  if (is_deprecated_process_wide_factory_) {
+    // The CHECK condition below (in a Renderer process) is also enforced later
+    // (in the NetworkService process) by CorsURLLoaderFactory::IsValidRequest
+    // (see the InitiatorLockCompatibility::kNoLock case) - this enforcement may
+    // result in a renderer kill when the NetworkService is hosted in a separate
+    // process from the Browser process.  Despite the redundancy, we want to
+    // also have the CHECK below, so that the Renderer process terminates
+    // earlier, with a callstack that (unlike the NetworkService
+    // mojo::ReportBadMessage) is hopefully useful for tracking down the source
+    // of the problem.
+    CHECK(request.request_initiator->opaque());
   }
 
   InitDirectNetworkFactoryIfNecessary();

docs/security/compromised-renderers.md

@@ -381,10 +381,6 @@ below.
   Some web storage protections depend on `CanAccessDataForOrigin` calls
   on the IO thread.
   See also https://crbug.com/764958.
-- `request_initiator_site_lock` may be missing in unlocked renderer
-  processes on Android (for example affecting protections of CORB, CORP,
-  Sec-Fetch-Site and in the future SameSite cookies and Origin
-  protections).  See also https://crbug.com/1114906.
 
 
 ## Renderer processes hosting DevTools frontend

services/network/cors/cors_url_loader_factory.cc

@@ -382,13 +382,13 @@ bool CorsURLLoaderFactory::IsValidRequest(const ResourceRequest& request,
       break;
 
     case InitiatorLockCompatibility::kNoLock:
-      // TODO(lukasza): https://crbug.com/1114906: Browser process should always
-      // specify the request_initiator_site_lock in URLLoaderFactories given to
-      // a renderer process.  Once issues blocking https://crbug.com/1114906 are
-      // fixed, the case below should return |false| and call
-      // mojo::ReportBadMessage.
+      // |request_initiator_site_lock| should always be set in a
+      // URLLoaderFactory vended to a renderer process.  See also
+      // https://crbug.com/1114906.
       NOTREACHED();
-      break;
+      mojo::ReportBadMessage(
+          "CorsURLLoaderFactory: no initiator lock in a renderer request");
+      return false;
 
     case InitiatorLockCompatibility::kNoInitiator:
       // Requests from the renderer need to always specify an initiator.

services/network/public/cpp/initiator_lock_compatibility.h

@@ -24,8 +24,8 @@ enum class InitiatorLockCompatibility {
   // |request_initiator_site_lock| doesn't apply.
   kBrowserProcess = 0,
 
-  // |request_initiator_site_lock| is missing - see https://crbug.com/1098938
-  // and RenderProcessHostImpl::CreateURLLoaderFactoryForRendererProcess.
+  // |request_initiator_site_lock| is missing.  For historical context see
+  // https://crbug.com/1098938.
   kNoLock = 1,
 
   // |request_initiator| is missing.  This indicates that the renderer has a bug