Move blocking of top-level navigations to nested URLs with extension origins from non-extension pro…

extensions/browser/extension_navigation_throttle.cc

Index: extensions/browser/extension_navigation_throttle.cc
diff --git a/extensions/browser/extension_navigation_throttle.cc b/extensions/browser/extension_navigation_throttle.cc
index bdf1a27af56d003b7374132a946fb4c715992407..8d2061e45d3de694eac4372ecd54f7b0d4877f1a 100644
--- a/extensions/browser/extension_navigation_throttle.cc
+++ b/extensions/browser/extension_navigation_throttle.cc
@@ -14,6 +14,8 @@
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/manifest_handlers/web_accessible_resources_info.h"
+#include "extensions/common/permissions/api_permission.h"
+#include "extensions/common/permissions/permissions_data.h"
 
 namespace extensions {
 
@@ -26,14 +28,44 @@ ExtensionNavigationThrottle::~ExtensionNavigationThrottle() {}
 content::NavigationThrottle::ThrottleCheckResult
 ExtensionNavigationThrottle::WillStartRequest() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  GURL url(navigation_handle()->GetURL());
+  ExtensionRegistry* registry = ExtensionRegistry::Get(
+      navigation_handle()->GetWebContents()->GetBrowserContext());
+
+  if (navigation_handle()->IsInMainFrame()) {
+    // Block top-level navigations to blob: or filesystem: URLs with extension
+    // origin from non-extension processes.  See https://crbug.com/645028.
+    bool is_nested_url = url.SchemeIsFileSystem() || url.SchemeIsBlob();
+    bool is_extension = false;
+    if (registry) {
+      is_extension = !!registry->enabled_extensions().GetExtensionOrAppByURL(

[jam, 2016/10/11 19:09:06]

fyi this is based on translating the previous check in
ChromeExtensionsNetworkDelegateImpl::OnBeforeURLRequest, which did:
xtension_info_map_->process_map().Contains(info->GetChildID())

The map entry is inserted in 
ChromeContentBrowserClientExtensionsPart::SiteInstanceGotProcess
which calls:
registry->enabled_extensions().GetExtensionOrAppByURL(
          site_instance->GetSiteURL());
to get the extension

[nasko, 2016/10/12 17:00:35]

Ideally, I'd like to keep the process id check as well. If we have the
SiteInstance here, we can ensure that the process for the SiteInstance is in the
extensions process map, which I think is accessible on the UI thread too.

[jam, 2016/10/12 22:10:45]

The old code didn't check that it was in the map, it was checking if it's _not_
in it so I'm not sure what you mean?

I tried adding it in patchset 2 and it gave a few browser_tests failures related
to apps, i.e. for ChromeAppAPITest.InstallAndRunningState. I guess the process
map won't have entries for chrome apps processes?

[alexmos, 2016/10/12 23:13:28]

Yes, Nasko meant the reverse (what you tried).
Ah, let's not do the process check in this CL then.  We will investigate and
follow up on this separately from our side.  Seems like the SiteInstance check
should cover the cases we care about.

+          navigation_handle()->GetSiteURL());
+    }
 
-  // This method for now enforces only web_accessible_resources for navigations.
-  // Top-level navigations should always be allowed.
-  DCHECK(!navigation_handle()->IsInMainFrame());
+    url::Origin origin(url);
+    if (is_nested_url && origin.scheme() == extensions::kExtensionScheme &&
+        !is_extension) {
+      // Relax this restriction for apps that use <webview>.  See
+      // https://crbug.com/652077.
+      const extensions::Extension* extension =
+          registry->enabled_extensions().GetByID(origin.host());
+      bool has_webview_permission =
+          extension &&
+          extension->permissions_data()->HasAPIPermission(
+              extensions::APIPermission::kWebView);
+      if (!has_webview_permission)
+        return content::NavigationThrottle::CANCEL;
+    }
+
+    return content::NavigationThrottle::PROCEED;
+  }
+
+  // Now enforce web_accessible_resources for navigations. Top-level navigations
+  // should always be allowed.
 
   // If the navigation is not to a chrome-extension:// URL, no need to perform
   // any more checks.
-  if (!navigation_handle()->GetURL().SchemeIs(extensions::kExtensionScheme))
+  if (!url.SchemeIs(extensions::kExtensionScheme))
     return content::NavigationThrottle::PROCEED;
 
   // The subframe which is navigated needs to have all of its ancestors be
@@ -58,8 +90,7 @@ ExtensionNavigationThrottle::WillStartRequest() {
   content::RenderFrameHost* ancestor = navigating_frame->GetParent();
   bool external_ancestor = false;
   while (ancestor) {
-    if (ancestor->GetLastCommittedURL().GetOrigin() !=
-        navigation_handle()->GetURL().GetOrigin()) {
+    if (ancestor->GetLastCommittedURL().GetOrigin() != url.GetOrigin()) {
       // Ignore DevTools, as it is allowed to embed extension pages.
       if (!ancestor->GetLastCommittedURL().SchemeIs(
               content::kChromeDevToolsScheme)) {
@@ -75,15 +106,12 @@ ExtensionNavigationThrottle::WillStartRequest() {
 
   // Since there was at least one origin different than the navigation URL,
   // explicitly check for the resource in web_accessible_resources.
-  std::string resource_path = navigation_handle()->GetURL().path();
-  ExtensionRegistry* registry = ExtensionRegistry::Get(
-      navigation_handle()->GetWebContents()->GetBrowserContext());
+  std::string resource_path = url.path();
   if (!registry)
     return content::NavigationThrottle::BLOCK_REQUEST;
 
   const extensions::Extension* extension =
-      registry->enabled_extensions().GetByID(
-          navigation_handle()->GetURL().host());
+      registry->enabled_extensions().GetByID(url.host());
   if (!extension)
     return content::NavigationThrottle::BLOCK_REQUEST;