Validate the Origin HTTP header in the browser process.

chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc

Index: chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
diff --git a/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc b/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
index 67ac9277f117fb0b1f35de2900b1e16b3c13fdae..5f51a025f37f0c1332ebb709f2caa84d52eae2cf 100644
--- a/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
+++ b/chrome/browser/extensions/chrome_content_browser_client_extensions_part.cc
@@ -35,10 +35,12 @@
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/guest_view/extensions_guest_view_message_filter.h"
+#include "extensions/browser/guest_view/web_view/web_view_renderer_state.h"
 #include "extensions/browser/info_map.h"
 #include "extensions/browser/io_thread_extension_message_filter.h"
 #include "extensions/browser/view_type_utils.h"
 #include "extensions/common/constants.h"
+#include "extensions/common/manifest_constants.h"
 #include "extensions/common/manifest_handlers/app_isolation_info.h"
 #include "extensions/common/manifest_handlers/background_info.h"
 #include "extensions/common/manifest_handlers/web_accessible_resources_info.h"
@@ -184,6 +186,8 @@ bool ChromeContentBrowserClientExtensionsPart::ShouldUseProcessPerSite(
 // static
 bool ChromeContentBrowserClientExtensionsPart::CanCommitURL(
     content::RenderProcessHost* process_host, const GURL& url) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   // We need to let most extension URLs commit in any process, since this can
   // be allowed due to web_accessible_resources.  Most hosted app URLs may also
   // load in any process (e.g., in an iframe).  However, the Chrome Web Store
@@ -205,6 +209,52 @@ bool ChromeContentBrowserClientExtensionsPart::CanCommitURL(
   return true;
 }
 
+bool ChromeContentBrowserClientExtensionsPart::IsIllegalOrigin(
+    content::ResourceContext* resource_context,
+    int child_process_id,
+    const GURL& origin) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  // Consider non-extension URLs safe; they will be checked elsewhere.
+  if (!origin.SchemeIs(extensions::kExtensionScheme))
+    return false;
+
+  // If there is no extension installed for the URL, it couldn't have committed.
+  // (If the extension was recently uninstalled, the tab would have closed.)

[Charlie Reis, 2015/08/17 18:37:30]

@kalman: Is this a safe assumption?  I'd like to kill renderer processes if they
claim to be making an XHR from a chrome-extension:// URL that isn't installed.

[not at google - send to devlin, 2015/08/17 19:42:50]

Modulo race conditions (content API != tabbed UI probably) yes this should be
fine... though I believe right now there is a bug where it can happen with
service workers (https://code.google.com/p/chromium/issues/detail?id=519326).

you also might want to double-check that XHR from content scripts don't get
checked here. I know that blink treats these as coming from the extension's
origin (and adds host permissions accordingly) but I don't know whether it
filters all the way up here.

if XHR is even affected by this.

[Charlie Reis, 2015/08/17 19:49:59]

Hmm, I'll check with Devlin about this.  Thanks.
Whoa.  I did not realize that, though I don't think it should cause a problem. 
We only say it's illegal if the chrome-extension ID isn't installed or is a
platform app (which can't use content scripts).
Yep, this is used for XHRs.

+  ProfileIOData* io_data = ProfileIOData::FromResourceContext(resource_context);
+  extensions::InfoMap* extension_info_map = io_data->GetExtensionInfoMap();
+  const extensions::Extension* extension =
+      extension_info_map->extensions().GetExtensionOrAppByURL(origin);
+  if (!extension)
+    return true;
+
+  // Check for platform app origins.  These can only be committed by the app
+  // itself, or by one if its guests if there are accessible_resources.

[not at google - send to devlin, 2015/08/17 19:42:50]

TODO(creis): Remove the platform_app restriction once site isolation is in place
for all extensions.

?

in fact once site isolation is in place, the rest of this method can basically
be

return process_map.Contains(extension->id(), child_process_id);

?

[Charlie Reis, 2015/08/17 19:49:59]

I don't think this is true.

Platform apps have a different type of accessible resource that their guest
processes are allowed to request.  That's what the checks below are for (not web
accessible resources).

See "Accessing packaged resources" on
https://developer.chrome.com/apps/tags/webview.  (This is what I had lfg@
reviewing.)

[not at google - send to devlin, 2015/08/17 20:13:28]

Got it. I did read "webview accessible resources" as "web accessible resources".
I may have also been reading this CL wrong - as though it were a check for
processes that are allow to host a resource, rather than a check for resources a
process is allowed to request.

+  const extensions::ProcessMap& process_map = extension_info_map->process_map();
+  if (extension->is_platform_app() &&

[not at google - send to devlin, 2015/08/17 20:13:28]

I would prefer a check for "does this extension have access to the webview API"
not "is this extension a platform app" because there are extensions which are
whitelisted for webview, and there are platform apps which don't have webviews.
Maybe you could tighten the condition a bit in that case?

[Charlie Reis, 2015/08/17 21:19:59]

Actually, platform apps are exactly what I'm trying to validate (for
https://crbug.com/513502).  Ideally, this check would catch any non-platform-app
process that claims to be making an XHR from a platform app.  Such a check would
imply that any XHR Origin header with a platform app's origin is actually being
made by the app and not by a compromised renderer process (which is the goal).

I had to add the exception for webview accessible resources because those can be
made from non-platform-app processes.  That weakens the security guarantee
slightly for platform apps with webviews and webview accessible resources, but
it's the best we can do. 

(In contrast, this logic is not trying to do any validation for general
extensions, whether they have webviews or not.)

+      !process_map.Contains(extension->id(), child_process_id)) {
+    // This is a platform app origin not in the app's own process.  If there are
+    // no accessible resources, this is illegal.

[not at google - send to devlin, 2015/08/17 19:42:50]

Platform apps shouldn't even have accessible resources:

https://code.google.com/p/chromium/codesearch#chromium/src/extensions/common/...

the exception being component platform apps - which means that the only way this
block will continue is if this platform app is a component platform app.

[lfg, 2015/08/17 19:51:35]

The check is for webview-accessible resources (i.e. chrome-extension://
resources loaded inside a <webview> guest process).

+    if (!extension->GetManifestData(manifest_keys::kWebviewAccessibleResources))

[not at google - send to devlin, 2015/08/17 19:42:49]

A better check is 
WebAccessibleResourcesInfo::HasWebAccessibleResources(extension).

+      return true;
+
+    // If there are accessible resources, the origin is only legal if the given
+    // process is a guest of the app.

[not at google - send to devlin, 2015/08/17 19:42:49]

I don't follow this.

+    std::string owner_extension_id;
+    int owner_process_id;
+    WebViewRendererState::GetInstance()->GetOwnerInfo(
+        child_process_id, &owner_process_id, &owner_extension_id);
+    const Extension* owner_extension =
+        extension_info_map->extensions().GetByID(owner_extension_id);
+    return !owner_extension || owner_extension != extension;
+  }
+
+  // With only the origin and not the full URL, we don't have enough information
+  // to validate hosted apps or web_accessible_resources in normal extensions.
+  // Assume they're legal.
+  return false;
+}
+
 // static
 bool ChromeContentBrowserClientExtensionsPart::IsSuitableHost(
     Profile* profile,