Avoid using ContentBrowserClient::IsIllegalOrigin in ResourceDispatcherHost.

content/browser/loader/resource_dispatcher_host_impl.h

Index: content/browser/loader/resource_dispatcher_host_impl.h
diff --git a/content/browser/loader/resource_dispatcher_host_impl.h b/content/browser/loader/resource_dispatcher_host_impl.h
index 339daec4d5d6a909fe8f05f1e134db52e5d8a370..3e217f9578f0b4343af4878f5bbe46e1a9dcf6bd 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.h
+++ b/content/browser/loader/resource_dispatcher_host_impl.h
@@ -107,7 +107,21 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
   void SetDelegate(ResourceDispatcherHostDelegate* delegate) override;
   void SetAllowCrossOriginAuthPrompt(bool value) override;
   void ClearLoginDelegateForRequest(net::URLRequest* request) override;
-
+  void AddSchemeForAccessCheck(const std::string& scheme) override;
+  void RegisterOriginForAccessChecks(
+      const ResourceContext* context,
+      const std::string& origin,
+      OriginAccessCheckMask access_check_mask) override;
+  void UnregisterOriginForAccessChecks(const ResourceContext* context,
+                                       const std::string& origin) override;
+  void AddProcessForOrigin(const ResourceContext* context,
+                           const std::string& origin,
+                           int process_id,
+                           bool owner_proces) override;
+  void RemoveProcessForOrigin(const ResourceContext* context,
+                              const std::string& origin,
+                              int process_id,
+                              bool owner_process) override;
   // Puts the resource dispatcher host in an inactive state (unable to begin
   // new requests).  Cancels all pending requests.
   void Shutdown();
@@ -292,6 +306,23 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
 
   void OnRenderFrameDeleted(const GlobalFrameRoutingId& global_routing_id);
 
+  // Checks whether the child process identified by |child_process_id| is
+  // allowed to access the |origin| and returns true if not.
+  // 1. If the |origin| scheme is not registered then it is allowed.
+  // 2. If the |origin| host is not registered, then it is denied.
+  // 3. If the |child_process_id| is in the list of owner processes for the
+  //    |origin| it is allowed. Please see OriginAccessInfo defined above for

[Charlie Reis, 2016/08/09 02:07:48]

nit: Below?

+  //    more information.
+  // 4. If the origin access mask is DENY_FOR_NON_OWNERS it is denied.
+  // 5. If the origin access mask is ALLOW_EVERYTHING it is allowed.
+  // 6. If the |child_process_id| is in the list of other processes for the
+  //    |origin| and there are publicly available resources it is allowed.

[Charlie Reis, 2016/08/09 02:07:48]

Hmm.  This is hard to understand in the abstract without having extension
concepts to map it to, which also makes it hard to reason about whether the
policy is the same as before (or if it's correct).

Do we have a sense for whether it will be possible in general to completely
avoid having extension code in the network process?  I'm wondering if it would
be easier to have a similar ContentClient structure in that process where we can
call out to the embedder and let it answer questions like this in context.

Would that be a worse tradeoff?  I'm just trying to brainstorm ways to make this
easier to understand, since this abstract mapping is hard (and may be hard to
maintain as we eliminate or change the corner cases in the extension code
itself).

+  //    Please see OriginAccessInfo defined above for more information.
+  // 5. Deny if all checks above fail.
+  bool IsIllegalOrigin(ResourceContext* context,
+                       const GURL& origin,
+                       int child_process_id);
+
  private:
   friend class ResourceDispatcherHostTest;
 
@@ -325,6 +356,43 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
   // Map from ProcessID+RouteID pair to the "most interesting" LoadState.
   typedef std::map<GlobalRoutingID, LoadInfo> LoadInfoMap;
 
+  // Contains information about an origin which is used for access checks. This
+  // means whether a process committing a URL is allowed to do so. We determine
+  // this based on whether the process has been registered for the origin.
+  // We maintain two maps
+  // 1. A map of owner processes. These are allowed to commit the URL even if
+  //    there are no publicly accessible resources.
+  // 2. A map of other processes. These are allowed to commit the URL only if
+  //    there are public resources.
+  struct OriginAccessInfo {
+    // This structure is complicated enough for clang to require the ctors to
+    // be explicitly defined in the cc file.
+    OriginAccessInfo();
+    ~OriginAccessInfo();
+    OriginAccessInfo(const OriginAccessInfo& other);
+
+    // Controls which can processes can commit the origin.
+    // By default owners can commit everything.
+    OriginAccessCheckMask access_check_mask;
+    // A process may be reused across multiple SiteInstances or routing ids.
+    // This means that a process could be added and removed multiple times for
+    // an origin via the AddProcessForOrigin() an RemoveProcessForOrigin()
+    // methods. To ensure that this works correctly we maintain a map of
+    // process id to refcount. The process id is removed from the map when the
+    // refcount drops to 0.
+    std::map<int, int> owner_processes;
+    std::map<int, int> other_processes;
+  };
+
+  // Map from the origin host (std::string) to the OriginAccessInfo structure
+  // defined above.
+  // This map is per ResourceContext.
+  typedef std::map<std::string, OriginAccessInfo> OriginAccessInfoMap;
+
+  typedef std::map<const ResourceContext*,
+                   std::unique_ptr<OriginAccessInfoMap>>
+      ResourceContextOriginMap;
+
   // ResourceLoaderDelegate implementation:
   ResourceDispatcherHostLoginDelegate* CreateLoginDelegate(
       ResourceLoader* loader,
@@ -537,6 +605,11 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
 
   CertStore* GetCertStore();
 
+  // Returns the OriginAccessInfoMap instance for the |context| passed in. This
+  // map is used to enforce access checks on web requests for some origins.
+  OriginAccessInfoMap* GetOriginAccessMapForResourceContext(
+      const ResourceContext* context);
+
   LoaderMap pending_loaders_;
 
   // Collection of temp files downloaded for child processes via
@@ -631,6 +704,14 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
   // outlive this ResourceDispatcherHostImpl.
   CertStore* cert_store_for_testing_;
 
+  // Used to check whether a request to retrieve an origin resource is allowed.
+  // This is only done for origins which are to be checked for access.
+  ResourceContextOriginMap context_origin_access_info_map_;
+
+  // This contains the set of origins we need to enforce access checks on. By
+  // default everything is allowed.
+  std::set<std::string> origins_for_access_check_;
+
   DISALLOW_COPY_AND_ASSIGN(ResourceDispatcherHostImpl);
 };