OOR-CORS: Use OriginAccessList in CORSURLLoaderFactory

This makes CORSURLLoaderFactory use OriginAccessList
to check the source origin and destination URL pairs in the
allowed list.

There is no caller in production code at this moment, but
patch set to support the legacy path and NetworkService will
follow respectively.

Bug: 870172
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Iba37beb1d955edf05148ad9ac15731e64e58de1e
Reviewed-on: https://chromium-review.googlesource.com/1196702
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588782}

content/browser/loader/resource_message_filter.cc

@@ -184,7 +184,8 @@ void ResourceMessageFilter::InitializeOnIOThread() {
       std::make_unique<URLLoaderFactoryImpl>(requester_info_),
       base::BindRepeating(&ResourceDispatcherHostImpl::CancelRequest,
                           base::Unretained(ResourceDispatcherHostImpl::Get()),
-                          requester_info_->child_id()));
+                          requester_info_->child_id()),
+      nullptr);
 
   std::vector<network::mojom::URLLoaderFactoryRequest> requests =
       std::move(queued_clone_requests_);

services/network/cors/cors_url_loader.cc

@@ -8,6 +8,7 @@
 #include "net/base/load_flags.h"
 #include "services/network/cors/preflight_controller.h"
 #include "services/network/public/cpp/cors/cors.h"
+#include "services/network/public/cpp/cors/origin_access_list.h"
 #include "url/url_util.h"
 
 namespace network {
@@ -16,25 +17,6 @@ namespace cors {
 
 namespace {
 
-// This should be identical to CalculateCORSFlag defined in
-// //third_party/blink/renderer/platform/loader/cors/cors.cc.
-bool CalculateCORSFlag(const ResourceRequest& request) {
-  if (request.fetch_request_mode == mojom::FetchRequestMode::kNavigate ||
-      request.fetch_request_mode == mojom::FetchRequestMode::kNoCORS) {
-    return false;
-  }
-  // CORS needs a proper origin (including a unique opaque origin). If the
-  // request doesn't have one, CORS should not work.
-  DCHECK(request.request_initiator);
-
-  if (request.url.SchemeIs(url::kDataScheme))
-    return false;
-
-  url::Origin url_origin = url::Origin::Create(request.url);
-  url::Origin security_origin(request.request_initiator.value());
-  return !security_origin.IsSameOriginWith(url_origin);
-}
-
 base::Optional<std::string> GetHeaderString(
     const scoped_refptr<net::HttpResponseHeaders>& headers,
     const std::string& header_name) {
@@ -85,7 +67,8 @@ CORSURLLoader::CORSURLLoader(
     mojom::URLLoaderClientPtr client,
     const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
     mojom::URLLoaderFactory* network_loader_factory,
-    const base::RepeatingCallback<void(int)>& request_finalizer)
+    const base::RepeatingCallback<void(int)>& request_finalizer,
+    const OriginAccessList* origin_access_list)
     : binding_(this, std::move(loader_request)),
       routing_id_(routing_id),
       request_id_(request_id),
@@ -95,13 +78,14 @@ CORSURLLoader::CORSURLLoader(
       network_client_binding_(this),
       request_(resource_request),
       forwarding_client_(std::move(client)),
-      fetch_cors_flag_(CalculateCORSFlag(resource_request)),
       request_finalizer_(request_finalizer),
       traffic_annotation_(traffic_annotation),
+      origin_access_list_(origin_access_list),
       weak_factory_(this) {
   binding_.set_connection_error_handler(base::BindOnce(
       &CORSURLLoader::OnConnectionError, base::Unretained(this)));
   DCHECK(network_loader_factory_);
+  SetCORSFlagIfNeeded();
 }
 
 CORSURLLoader::~CORSURLLoader() = default;
@@ -149,7 +133,7 @@ void CORSURLLoader::FollowRedirect(
   request_.referrer = GURL(redirect_info_.new_referrer);
   request_.referrer_policy = redirect_info_.new_referrer_policy;
   const bool original_fetch_cors_flag = fetch_cors_flag_;
-  fetch_cors_flag_ = fetch_cors_flag_ || CalculateCORSFlag(request_);
+  SetCORSFlagIfNeeded();
 
   // We cannot use FollowRedirect for a request with preflight (i.e., when both
   // |fetch_cors_flag_| and |NeedsPreflight(request_)| are true).
@@ -457,6 +441,38 @@ void CORSURLLoader::OnConnectionError() {
   HandleComplete(URLLoaderCompletionStatus(net::ERR_FAILED));
 }
 
+// This should be identical to CalculateCORSFlag defined in
+// //third_party/blink/renderer/platform/loader/cors/cors.cc.
+void CORSURLLoader::SetCORSFlagIfNeeded() {
+  if (fetch_cors_flag_)
+    return;
+
+  if (request_.fetch_request_mode == mojom::FetchRequestMode::kNavigate ||
+      request_.fetch_request_mode == mojom::FetchRequestMode::kNoCORS) {
+    return;
+  }
+
+  if (request_.url.SchemeIs(url::kDataScheme))
+    return;
+
+  // CORS needs a proper origin (including a unique opaque origin). If the
+  // request doesn't have one, CORS should not work.
+  DCHECK(request_.request_initiator);
+
+  // The source origin and destination URL pair may be in the allow list.
+  if (origin_access_list_ && origin_access_list_->IsAllowed(
+                                 *request_.request_initiator, request_.url)) {
+    return;
+  }
+
+  if (request_.request_initiator->IsSameOriginWith(
+          url::Origin::Create(request_.url))) {
+    return;
+  }
+
+  fetch_cors_flag_ = true;
+}
+
 }  // namespace cors
 
 }  // namespace network

services/network/cors/cors_url_loader.h

@@ -19,6 +19,8 @@ namespace network {
 
 namespace cors {
 
+class OriginAccessList;
+
 // Wrapper class that adds cross-origin resource sharing capabilities
 // (https://fetch.spec.whatwg.org/#http-cors-protocol), delegating requests as
 // well as potential preflight requests to the supplied
@@ -43,7 +45,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CORSURLLoader
       mojom::URLLoaderClientPtr client,
       const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
       mojom::URLLoaderFactory* network_loader_factory,
-      const base::RepeatingCallback<void(int)>& request_finalizer);
+      const base::RepeatingCallback<void(int)>& request_finalizer,
+      const OriginAccessList* origin_access_list);
 
   ~CORSURLLoader() override;
 
@@ -89,6 +92,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CORSURLLoader
 
   void OnConnectionError();
 
+  void SetCORSFlagIfNeeded();
+
   mojo::Binding<mojom::URLLoader> binding_;
 
   // We need to save these for redirect.
@@ -125,7 +130,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CORSURLLoader
   bool is_waiting_follow_redirect_call_ = false;
 
   // Corresponds to the Fetch spec, https://fetch.spec.whatwg.org/.
-  bool fetch_cors_flag_;
+  bool fetch_cors_flag_ = false;
 
   net::RedirectInfo redirect_info_;
 
@@ -142,6 +147,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CORSURLLoader
   // We need to save this for redirect.
   net::MutableNetworkTrafficAnnotationTag traffic_annotation_;
 
+  // Outlives |this|.
+  const OriginAccessList* const origin_access_list_;
+
   // Used to run asynchronous class instance bound callbacks safely.
   base::WeakPtrFactory<CORSURLLoader> weak_factory_;
 

services/network/cors/cors_url_loader_factory.cc

@@ -23,14 +23,16 @@ CORSURLLoaderFactory::CORSURLLoaderFactory(
     NetworkContext* context,
     mojom::URLLoaderFactoryParamsPtr params,
     scoped_refptr<ResourceSchedulerClient> resource_scheduler_client,
-    mojom::URLLoaderFactoryRequest request)
+    mojom::URLLoaderFactoryRequest request,
+    const OriginAccessList* origin_access_list)
     : context_(context),
       disable_web_security_(params && params->disable_web_security),
       network_loader_factory_(std::make_unique<network::URLLoaderFactory>(
           context,
           std::move(params),
           std::move(resource_scheduler_client),
-          this)) {
+          this)),
+      origin_access_list_(origin_access_list) {
   DCHECK(context_);
   bindings_.AddBinding(this, std::move(request));
   bindings_.set_connection_error_handler(base::BindRepeating(
@@ -40,10 +42,12 @@ CORSURLLoaderFactory::CORSURLLoaderFactory(
 CORSURLLoaderFactory::CORSURLLoaderFactory(
     bool disable_web_security,
     std::unique_ptr<mojom::URLLoaderFactory> network_loader_factory,
-    const base::RepeatingCallback<void(int)>& preflight_finalizer)
+    const base::RepeatingCallback<void(int)>& preflight_finalizer,
+    const OriginAccessList* origin_access_list)
     : disable_web_security_(disable_web_security),
       network_loader_factory_(std::move(network_loader_factory)),
-      preflight_finalizer_(preflight_finalizer) {}
+      preflight_finalizer_(preflight_finalizer),
+      origin_access_list_(origin_access_list) {}
 
 CORSURLLoaderFactory::~CORSURLLoaderFactory() = default;
 
@@ -80,7 +84,8 @@ void CORSURLLoaderFactory::CreateLoaderAndStart(
         base::BindOnce(&CORSURLLoaderFactory::DestroyURLLoader,
                        base::Unretained(this)),
         resource_request, std::move(client), traffic_annotation,
-        network_loader_factory_.get(), preflight_finalizer_);
+        network_loader_factory_.get(), preflight_finalizer_,
+        origin_access_list_);
     auto* raw_loader = loader.get();
     OnLoaderCreated(std::move(loader));
     raw_loader->Start();

services/network/cors/cors_url_loader_factory.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "mojo/public/cpp/bindings/strong_binding_set.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
+#include "services/network/public/cpp/cors/origin_access_list.h"
 #include "services/network/public/mojom/network_context.mojom.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
 
@@ -31,18 +32,21 @@ namespace cors {
 class COMPONENT_EXPORT(NETWORK_SERVICE) CORSURLLoaderFactory final
     : public mojom::URLLoaderFactory {
  public:
+  // |origin_access_list| should always outlive this factory instance.
   // Used by network::NetworkContext.
   CORSURLLoaderFactory(
       NetworkContext* context,
       mojom::URLLoaderFactoryParamsPtr params,
       scoped_refptr<ResourceSchedulerClient> resource_scheduler_client,
-      mojom::URLLoaderFactoryRequest request);
+      mojom::URLLoaderFactoryRequest request,
+      const OriginAccessList* origin_access_list);
   // Used by content::ResourceMessageFilter.
   // TODO(yhirano): Remove this once when the network service is fully enabled.
   CORSURLLoaderFactory(
       bool disable_web_security,
       std::unique_ptr<mojom::URLLoaderFactory> network_loader_factory,
-      const base::RepeatingCallback<void(int)>& preflight_finalizer);
+      const base::RepeatingCallback<void(int)>& preflight_finalizer,
+      const OriginAccessList* origin_access_list);
   ~CORSURLLoaderFactory() override;
 
   void OnLoaderCreated(std::unique_ptr<mojom::URLLoader> loader);
@@ -83,6 +87,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CORSURLLoaderFactory final
   // Used when constructed by ResourceMessageFilter.
   base::RepeatingCallback<void(int)> preflight_finalizer_;
 
+  // Accessed by instances in |loaders_| too. Since the factory outlives them,
+  // it's safe.
+  const OriginAccessList* const origin_access_list_;
+
   DISALLOW_COPY_AND_ASSIGN(CORSURLLoaderFactory);
 };
 

services/network/cors/cors_url_loader_unittest.cc

@@ -121,7 +121,8 @@ class CORSURLLoaderTest : public testing::Test {
         std::make_unique<TestURLLoaderFactory>();
     test_url_loader_factory_ = factory->GetWeakPtr();
     cors_url_loader_factory_ = std::make_unique<CORSURLLoaderFactory>(
-        false, std::move(factory), base::RepeatingCallback<void(int)>());
+        false, std::move(factory), base::RepeatingCallback<void(int)>(),
+        &origin_access_list_);
   }
 
  protected:
@@ -214,6 +215,14 @@ class CORSURLLoaderTest : public testing::Test {
     test_cors_loader_client_.RunUntilRedirectReceived();
   }
 
+  void AddAllowListEntryForOrigin(const url::Origin& source_origin,
+                                  const std::string& protocol,
+                                  const std::string& domain,
+                                  bool allow_subdomains) {
+    origin_access_list_.AddAllowListEntryForOrigin(source_origin, protocol,
+                                                   domain, allow_subdomains);
+  }
+
   static net::RedirectInfo CreateRedirectInfo(
       int status_code,
       base::StringPiece method,
@@ -248,6 +257,9 @@ class CORSURLLoaderTest : public testing::Test {
   // TestURLLoaderClient that records callback activities.
   TestURLLoaderClient test_cors_loader_client_;
 
+  // Holds for allowed origin access lists.
+  OriginAccessList origin_access_list_;
+
   DISALLOW_COPY_AND_ASSIGN(CORSURLLoaderTest);
 };
 
@@ -980,6 +992,32 @@ TEST_F(CORSURLLoaderTest, FollowErrorRedirect) {
   EXPECT_EQ(net::ERR_FAILED, client().completion_status().error_code);
 }
 
+// Tests if OriginAccessList is actually used to decide the cors flag.
+// Does not verify detailed functionalities that are verified in
+// OriginAccessListTest.
+TEST_F(CORSURLLoaderTest, OriginAccessList) {
+  const GURL origin("http://example.com");
+  const GURL url("http://other.com/foo.png");
+
+  // Adds an entry to allow the cross origin request beyond the CORS
+  // rules.
+  AddAllowListEntryForOrigin(url::Origin::Create(origin), url.scheme(),
+                             url.host(), false);
+
+  CreateLoaderAndStart(origin, url, mojom::FetchRequestMode::kCORS);
+
+  NotifyLoaderClientOnReceiveResponse();
+  NotifyLoaderClientOnComplete(net::OK);
+
+  RunUntilComplete();
+
+  EXPECT_TRUE(IsNetworkLoaderStarted());
+  EXPECT_FALSE(client().has_received_redirect());
+  EXPECT_TRUE(client().has_received_response());
+  EXPECT_TRUE(client().has_received_completion());
+  EXPECT_EQ(net::OK, client().completion_status().error_code);
+}
+
 }  // namespace
 
 }  // namespace cors

services/network/network_context.cc

@@ -521,7 +521,7 @@ void NetworkContext::CreateURLLoaderFactory(
     scoped_refptr<ResourceSchedulerClient> resource_scheduler_client) {
   url_loader_factories_.emplace(std::make_unique<cors::CORSURLLoaderFactory>(
       this, std::move(params), std::move(resource_scheduler_client),
-      std::move(request)));
+      std::move(request), nullptr));
 }
 
 void NetworkContext::CreateURLLoaderFactory(

services/network/public/cpp/cors/origin_access_list.h

@@ -19,9 +19,8 @@ namespace network {
 
 namespace cors {
 
-// A class to manage origin access whitelisting. It manages two lists for
-// whitelisting and blacklisting. If these lists conflict, blacklisting will be
-// respected. These lists are managed per source-origin basis.
+// A class to manage origin access allow / block lists. If these lists conflict,
+// blacklisting is respected. These lists are managed per source-origin basis.
 class COMPONENT_EXPORT(NETWORK_CPP) OriginAccessList {
  public:
   OriginAccessList();