OOR-CORS: Manage per-profile access list even for NetworkService

This patch makes BrowserContext manages per-profile CORS access lists
even if NetworkService is enabled, and use it to setup initial access
lists for the non-primary NetworkContext. It will also work for
restoring per-profile CORS settings on network service restarts.

This patch makes following tests work even with
--enable-features=OutOfBlinkCors,NetworkService.

- CrossOriginReadBlockingExtensionTest.ProgrammaticContentScriptVsAppCache
- CrossOriginReadBlockingExtensionTest.WebViewContentScript
- ExtensionWebRequestApiTest.ExtensionRequests
- PlatformAppBrowserTest.Isolation

Bug: 908324, 891891
Change-Id: Ib0cfc2f5633f25187366a4d7d63168d60ea51f71
Reviewed-on: https://chromium-review.googlesource.com/c/1351208Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612567}

chrome/browser/net/profile_network_context_service.cc

@@ -33,12 +33,14 @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/network_service_instance.h"
+#include "content/public/browser/shared_cors_origin_access_list.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/service_names.mojom.h"
 #include "content/public/common/url_constants.h"
 #include "mojo/public/cpp/bindings/associated_interface_ptr.h"
 #include "net/http/http_util.h"
 #include "net/net_buildflags.h"
+#include "services/network/public/cpp/cors/origin_access_list.h"
 #include "services/network/public/cpp/features.h"
 
 #if defined(OS_CHROMEOS)
@@ -486,6 +488,14 @@ ProfileNetworkContextService::CreateNetworkContextParams(
   }
 #endif
 
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    // Should be initialized with existing per-profile CORS access lists.
+    network_context_params->cors_origin_access_list =
+        content::BrowserContext::GetSharedCorsOriginAccessList(profile_)
+            ->GetOriginAccessList()
+            .CreateCorsOriginAccessPatternsList();
+  }
+
   return network_context_params;
 }
 

content/browser/browser_context.cc

@@ -13,6 +13,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/barrier_closure.h"
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/command_line.h"
@@ -138,9 +139,6 @@ class CorsOriginPatternSetter
                        base::RetainedRef(this)));
   }
 
- private:
-  friend class base::RefCounted<CorsOriginPatternSetter>;
-
   static std::vector<network::mojom::CorsOriginPatternPtr> ClonePatterns(
       const std::vector<network::mojom::CorsOriginPatternPtr>& patterns) {
     std::vector<network::mojom::CorsOriginPatternPtr> cloned_patterns;
@@ -150,6 +148,9 @@ class CorsOriginPatternSetter
     return cloned_patterns;
   }
 
+ private:
+  friend class base::RefCounted<CorsOriginPatternSetter>;
+
   ~CorsOriginPatternSetter() { std::move(closure_).Run(); }
 
   const url::Origin source_origin_;
@@ -730,12 +731,21 @@ void BrowserContext::SetCorsOriginAccessListsForOrigin(
         ->SetForOrigin(source_origin, std::move(allow_patterns),
                        std::move(block_patterns), std::move(closure));
   } else {
+    auto barrier_closure = BarrierClosure(2, std::move(closure));
     auto setter = base::MakeRefCounted<CorsOriginPatternSetter>(
-        source_origin, std::move(allow_patterns), std::move(block_patterns),
-        std::move(closure));
+        source_origin, CorsOriginPatternSetter::ClonePatterns(allow_patterns),
+        CorsOriginPatternSetter::ClonePatterns(block_patterns),
+        barrier_closure);
     ForEachStoragePartition(
         browser_context, base::BindRepeating(&CorsOriginPatternSetter::SetLists,
                                              base::RetainedRef(setter.get())));
+
+    // Keeps per-profile access lists in the browser process to make all
+    // NetworkContext belonging to the profile to be synchronized.
+    UserDataAdapter<SharedCorsOriginAccessList>::Get(
+        browser_context, kSharedCorsOriginAccessListKey)
+        ->SetForOrigin(source_origin, std::move(allow_patterns),
+                       std::move(block_patterns), barrier_closure);
   }
 }
 

content/browser/loader/resource_message_filter.cc

@@ -21,6 +21,7 @@
 #include "content/public/browser/shared_cors_origin_access_list.h"
 #include "content/public/common/content_switches.h"
 #include "services/network/cors/cors_url_loader_factory.h"
+#include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "storage/browser/fileapi/file_system_context.h"
 
@@ -181,14 +182,21 @@ void ResourceMessageFilter::InitializeOnIOThread() {
   // The WeakPtr of the filter must be created on the IO thread. So sets the
   // WeakPtr of |requester_info_| now.
   requester_info_->set_filter(GetWeakPtr());
-  url_loader_factory_ = std::make_unique<network::cors::CorsURLLoaderFactory>(
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kDisableWebSecurity),
-      std::make_unique<URLLoaderFactoryImpl>(requester_info_),
-      base::BindRepeating(&ResourceDispatcherHostImpl::CancelRequest,
-                          base::Unretained(ResourceDispatcherHostImpl::Get()),
-                          requester_info_->child_id()),
-      &shared_cors_origin_access_list_->GetOriginAccessList());
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    // ResourceMessageFilter should not be used if NetworkService is enabled,
+    // but still some tests rely on it.
+    url_loader_factory_ =
+        std::make_unique<URLLoaderFactoryImpl>(requester_info_);
+  } else {
+    url_loader_factory_ = std::make_unique<network::cors::CorsURLLoaderFactory>(
+        base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kDisableWebSecurity),
+        std::make_unique<URLLoaderFactoryImpl>(requester_info_),
+        base::BindRepeating(&ResourceDispatcherHostImpl::CancelRequest,
+                            base::Unretained(ResourceDispatcherHostImpl::Get()),
+                            requester_info_->child_id()),
+        &shared_cors_origin_access_list_->GetOriginAccessList());
+  }
 
   std::vector<network::mojom::URLLoaderFactoryRequest> requests =
       std::move(queued_clone_requests_);

content/browser/loader/shared_cors_origin_access_list_impl.cc

@@ -22,18 +22,26 @@ void SharedCorsOriginAccessListImpl::SetForOrigin(
     std::vector<network::mojom::CorsOriginPatternPtr> block_patterns,
     base::OnceClosure closure) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK(!base::FeatureList::IsEnabled(network::features::kNetworkService));
-  base::PostTaskWithTraitsAndReply(
-      FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&SharedCorsOriginAccessListImpl::SetForOriginOnIOThread,
-                     base::RetainedRef(this), source_origin,
-                     std::move(allow_patterns), std::move(block_patterns)),
-      std::move(closure));
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    origin_access_list_.SetAllowListForOrigin(source_origin, allow_patterns);
+    origin_access_list_.SetBlockListForOrigin(source_origin, block_patterns);
+    std::move(closure).Run();
+  } else {
+    base::PostTaskWithTraitsAndReply(
+        FROM_HERE, {BrowserThread::IO},
+        base::BindOnce(&SharedCorsOriginAccessListImpl::SetForOriginOnIOThread,
+                       base::RetainedRef(this), source_origin,
+                       std::move(allow_patterns), std::move(block_patterns)),
+        std::move(closure));
+  }
 }
 
 const network::cors::OriginAccessList&
 SharedCorsOriginAccessListImpl::GetOriginAccessList() const {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService))
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  else
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
   return origin_access_list_;
 }
 
@@ -44,6 +52,7 @@ void SharedCorsOriginAccessListImpl::SetForOriginOnIOThread(
     std::vector<network::mojom::CorsOriginPatternPtr> allow_patterns,
     std::vector<network::mojom::CorsOriginPatternPtr> block_patterns) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(!base::FeatureList::IsEnabled(network::features::kNetworkService));
   origin_access_list_.SetAllowListForOrigin(source_origin, allow_patterns);
   origin_access_list_.SetBlockListForOrigin(source_origin, block_patterns);
 }

content/public/browser/shared_cors_origin_access_list.h

@@ -22,8 +22,14 @@ namespace content {
 
 // A public interface to manage CORS origin access lists on the UI thread.
 // The shared network::cors::OriginAccessList instance can only be accessed on
-// the IO thread. Callers on UI thread must use this wrapper class.
-// TODO(toyoshim): Remove once the NetworkService is enabled.
+// the IO thread if NetworkService is not enabled. Callers on UI thread must use
+// this wrapper class to make it work with and without NetworkService until
+// NetworkService is fully enabled. If NetworkService is enabled,
+// network::cors::OriginAccessList is accessed only on the UI thread, and all
+// calls can be finished synchronously. This is used for remembering per-profile
+// access lists in the browser process.
+// TODO(toyoshim): Remove this class, and use network::cors::OriginAccessList
+// directly once NetworkService is fully enabled.
 class CONTENT_EXPORT SharedCorsOriginAccessList
     : public base::RefCountedThreadSafe<SharedCorsOriginAccessList> {
  public:
@@ -33,7 +39,7 @@ class CONTENT_EXPORT SharedCorsOriginAccessList
   // instance so that its IsAllowed() method works for all users that refer the
   // shared network::cors::OriginAccessList instance returned by
   // origin_access_list() below. |allow_patterns| and |block_patterns| will be
-  // moved so to pass the lists to the IO thread.
+  // moved so to pass the lists to the IO thread if NetworkService is disabled.
   // Should be called on the UI thread, and |closure| runs on the UI thread too.
   virtual void SetForOrigin(
       const url::Origin& source_origin,

services/network/network_context.cc

@@ -526,6 +526,8 @@ NetworkContext::NetworkContext(
       url_request_context_->net_log(), url_request_context_);
   resource_scheduler_ =
       std::make_unique<ResourceScheduler>(enable_resource_scheduler_);
+
+  InitializeCorsOriginAccessList();
 }
 
 // TODO(mmenke): Share URLRequestContextBulder configuration between two
@@ -553,6 +555,8 @@ NetworkContext::NetworkContext(
       url_request_context_->net_log(), url_request_context_);
   resource_scheduler_ =
       std::make_unique<ResourceScheduler>(enable_resource_scheduler_);
+
+  InitializeCorsOriginAccessList();
 }
 
 NetworkContext::NetworkContext(NetworkService* network_service,
@@ -2172,4 +2176,14 @@ void NetworkContext::TrustAnchorUsed() {
 }
 #endif
 
+void NetworkContext::InitializeCorsOriginAccessList() {
+  for (const auto& pattern : params_->cors_origin_access_list) {
+    url::Origin origin = url::Origin::Create(GURL(pattern->source_origin));
+    cors_origin_access_list_.SetAllowListForOrigin(origin,
+                                                   pattern->allow_patterns);
+    cors_origin_access_list_.SetBlockListForOrigin(origin,
+                                                   pattern->block_patterns);
+  }
+}
+
 }  // namespace network

services/network/network_context.h

@@ -412,6 +412,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
   void OnSetExpectCTTestReportFailure();
 #endif  // BUILDFLAG(IS_CT_SUPPORTED)
 
+  void InitializeCorsOriginAccessList();
+
   NetworkService* const network_service_;
 
   mojom::NetworkContextClientPtr client_;

services/network/public/mojom/network_context.mojom

@@ -307,6 +307,10 @@ struct NetworkContextParams {
   // verification, we should consider using each URLRequestContext to do its own
   // validation.
   bool primary_network_context = false;
+
+  // Specifies the initial set of allowed and blocked origins for the
+  // URLLoaderFactory consumers to access beyond the same-origin-policy.
+  array<CorsOriginAccessPatterns> cors_origin_access_list;
 };
 
 struct NetworkConditions {