Keep subdomains of an isolated origin in the isolated origin's SiteInstance.

content/browser/site_instance_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/site_instance_impl.h"
 
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "content/browser/browsing_instance.h"
 #include "content/browser/child_process_security_policy_impl.h"
@@ skipping 301 matching lines @@
   // If either URL is invalid, they aren't part of the same site.
   if (!src_url.is_valid() || !dest_url.is_valid())
     return false;
 
   // If the destination url is just a blank page, we treat them as part of the
   // same site.
   GURL blank_page(url::kAboutBlankURL);
   if (dest_url == blank_page)
     return true;
 
-  // If either URL has an isolated origin, compare origins rather than sites.
+  // If either URL matches an isolated origin, compare origins rather than
+  // sites.
   url::Origin src_origin(src_url);
   url::Origin dest_origin(dest_url);
   auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
-  if (policy->IsIsolatedOrigin(src_origin) ||
-      policy->IsIsolatedOrigin(dest_origin))
-    return src_origin == dest_origin;
+  url::Origin src_isolated_origin;
+  url::Origin dest_isolated_origin;
+  bool src_origin_is_isolated =
+      policy->GetMatchingIsolatedOrigin(src_origin, &src_isolated_origin);
+  bool dest_origin_is_isolated =
+      policy->GetMatchingIsolatedOrigin(dest_origin, &dest_isolated_origin);
+  if (src_origin_is_isolated || dest_origin_is_isolated) {
+    // Compare most specific matching origins to ensure that a subdomain of an
+    // isolated origin (e.g., https://subdomain.isolated.foo.com) also matches
+    // the isolated origin's site URL (e.g., https://isolated.foo.com).
+    return src_isolated_origin == dest_isolated_origin;
+  }
 
   // If the schemes differ, they aren't part of the same site.
   if (src_origin.scheme() != dest_origin.scheme())
     return false;
 
   return net::registry_controlled_domains::SameDomainOrHost(
       src_origin, dest_origin,
       net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);

[ncarter (slow), 2017/06/30 22:00:56]

I wonder if we should do the SameDomainOrHost check first here, before the
isolated origin match:

if (!SameDomainOrHost(...))
  return false;

That would kind of fix the co.uk case kind of automatically, and provide a
faster result in the cross-site case.

[alexmos, 2017/06/30 23:30:02]

Done.  Good idea, it makes sense to ensure that isolated origin checks can't
relax the regular site checks.

I'm still inclined to keep the restriction on not being able to add
http://co.uk/ as an isolated origin: while this fixes the subdomain matching
here, GetSiteForURL will still put http://foo.co.uk in a "http://co.uk" site
URL, which might lead to other weird behavior.  We could fix GetSiteForURL to
still prefer regular site URL when it's more specific than the isolated_origin
URL, but that gets into the behavior of process-isolating all sites on a TLD,
and I'd rather do that if/when we have a good use for it.

 }
 
 // static
 GURL SiteInstance::GetSiteForURL(BrowserContext* browser_context,
                                  const GURL& real_url) {
   // TODO(fsamuel, creis): For some reason appID is not recognized as a host.
   if (real_url.SchemeIs(kGuestScheme))
     return real_url;
 
   GURL url = SiteInstanceImpl::GetEffectiveURL(browser_context, real_url);
   url::Origin origin(url);
 
-  // Isolated origins should use the full origin as their site URL.
+  // Isolated origins should use the full origin as their site URL. A subdomain
+  // of an isolated origin should also use that isolated origin's site URL.
   auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
-  if (policy->IsIsolatedOrigin(origin))
-    return origin.GetURL();
+  url::Origin isolated_origin;
+  if (policy->GetMatchingIsolatedOrigin(url::Origin(real_url),
+                                        &isolated_origin)) {
+    return isolated_origin.GetURL();
+  }
 
   // If the url has a host, then determine the site.
   if (!origin.host().empty()) {
     // Only keep the scheme and registered domain of |origin|.
     std::string domain = net::registry_controlled_domains::GetDomainAndRegistry(
         origin.host(),
         net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
     std::string site = origin.scheme();
     site += url::kStandardSchemeSeparator;
     site += domain.empty() ? origin.host() : domain;
@@ skipping 121 matching lines @@
   // prevent the non-isolated sites from requesting resources for isolated
   // sites. https://crbug.com/509125
   if (ShouldLockToOrigin(GetBrowserContext(), site_)) {
     ChildProcessSecurityPolicyImpl* policy =
         ChildProcessSecurityPolicyImpl::GetInstance();
     policy->LockToOrigin(process_->GetID(), site_);
   }
 }
 
 }  // namespace content