Remove ExtensionURLInfo, make security decisions in render process

When asking if an extension should have access to a given frame, we need to
consider the frame's URL and also if the frame is sandboxed. We check the latter
by asking if the frame's security origin is the unique origin. However, we can
only usefully do this in the render process when examining a frame. In the
browser process or other common code, there's no useful origin to use other than
one that duplicates information in the URL.

This does security checks in the render process before doing any URL-based
lookups and then uses URLs from that point on.

R=abarth, mpcomplete
BUG=259982,237267

Review URL: https://chromiumcodereview.appspot.com/16625012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@212302 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 63 insertions, 48 deletions

diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index 79d57a7..e17227a 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -617,7 +617,7 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
             }
             const Extension* extension =
                 g_current_client->extension_dispatcher_->extensions()->
-                    GetExtensionOrAppByURL(ExtensionURLInfo(manifest_url));
+                    GetExtensionOrAppByURL(manifest_url);
             if (!IsNaClAllowed(manifest_url,
                                app_url,
                                is_nacl_unrestricted,
@@ -860,7 +860,7 @@ void ChromeContentRendererClient::GetNavigationErrorStrings(
   if (failed_url.is_valid() &&
       !failed_url.SchemeIs(extensions::kExtensionScheme)) {
     extension = extension_dispatcher_->extensions()->GetExtensionOrAppByURL(
-        ExtensionURLInfo(failed_url));
+        failed_url);
   }
 
   bool is_post = EqualsASCII(failed_request.httpMethod(), "POST");
@@ -961,7 +961,7 @@ bool ChromeContentRendererClient::ShouldFork(WebFrame* frame,
 
   // Determine if the new URL is an extension (excluding bookmark apps).
   const Extension* new_url_extension = extensions::GetNonBookmarkAppExtension(
-      *extensions, ExtensionURLInfo(url));
+      *extensions, url);
   bool is_extension_url = !!new_url_extension;
 
   // If the navigation would cross an app extent boundary, we also need
@@ -978,8 +978,7 @@ bool ChromeContentRendererClient::ShouldFork(WebFrame* frame,
     *send_referrer = true;
 
     const Extension* extension =
-        extension_dispatcher_->extensions()->GetExtensionOrAppByURL(
-            ExtensionURLInfo(url));
+        extension_dispatcher_->extensions()->GetExtensionOrAppByURL(url);
     if (extension && extension->is_app()) {
       UMA_HISTOGRAM_ENUMERATION(
           extension->is_platform_app() ?
@@ -1140,11 +1139,10 @@ bool ChromeContentRendererClient::CrossesExtensionExtents(
     // in an extension process, we want to keep it in process to allow the
     // opener to script it.
     WebDocument opener_document = frame->opener()->document();
-    GURL opener_url = opener_document.url();
-    WebSecurityOrigin opener_origin = opener_document.securityOrigin();
-    bool opener_is_extension_url = !!extensions.GetExtensionOrAppByURL(
-        ExtensionURLInfo(opener_origin, opener_url));
     WebSecurityOrigin opener = frame->opener()->document().securityOrigin();
+    bool opener_is_extension_url =
+        !opener.isUnique() && extensions.GetExtensionOrAppByURL(
+            opener_document.url()) != NULL;
     if (!is_extension_url &&
         !opener_is_extension_url &&
         extension_dispatcher_->is_extension_process() &&
@@ -1163,8 +1161,7 @@ bool ChromeContentRendererClient::CrossesExtensionExtents(
   bool should_consider_workaround = !!frame->opener();
 
   return extensions::CrossesExtensionProcessBoundary(
-      extensions, ExtensionURLInfo(old_url), ExtensionURLInfo(new_url),
-      should_consider_workaround);
+      extensions, old_url, new_url, should_consider_workaround);
 }
 
 void ChromeContentRendererClient::SetSpellcheck(SpellCheck* spellcheck) {
diff --git a/chrome/renderer/extensions/app_bindings.cc b/chrome/renderer/extensions/app_bindings.cc
index 8f07ed2..268727a 100644
--- a/chrome/renderer/extensions/app_bindings.cc
+++ b/chrome/renderer/extensions/app_bindings.cc
@@ -119,10 +119,13 @@ void AppBindings::GetDetailsForFrame(
 
 v8::Handle<v8::Value> AppBindings::GetDetailsForFrameImpl(
     WebFrame* frame) {
+  if (frame->document().securityOrigin().isUnique())
+    return v8::Null();
+
   const Extension* extension =
       dispatcher_->extensions()->GetExtensionOrAppByURL(
-          ExtensionURLInfo(frame->document().securityOrigin(),
-                           frame->document().url()));
+          frame->document().url());
+
   if (!extension)
     return v8::Null();
 
@@ -166,11 +169,11 @@ void AppBindings::GetRunningState(
 
   // The app associated with the top level frame.
   const Extension* parent_app = extensions->GetHostedAppByURL(
-      ExtensionURLInfo(parent_frame->document().url()));
+      parent_frame->document().url());
 
   // The app associated with this frame.
-  const Extension* this_app = extensions->GetHostedAppByURL(ExtensionURLInfo(
-      context()->web_frame()->document().url()));
+  const Extension* this_app = extensions->GetHostedAppByURL(
+      context()->web_frame()->document().url());
 
   if (!this_app || !parent_app) {
     args.GetReturnValue().Set(
diff --git a/chrome/renderer/extensions/dispatcher.cc b/chrome/renderer/extensions/dispatcher.cc
index f8d9094..9db8cab 100644
--- a/chrome/renderer/extensions/dispatcher.cc
+++ b/chrome/renderer/extensions/dispatcher.cc
@@ -24,6 +24,7 @@
 #include "chrome/common/extensions/features/feature.h"
 #include "chrome/common/extensions/manifest.h"
 #include "chrome/common/extensions/manifest_handlers/externally_connectable.h"
+#include "chrome/common/extensions/manifest_handlers/sandboxed_page_info.h"
 #include "chrome/common/extensions/message_bundle.h"
 #include "chrome/common/extensions/permissions/permission_set.h"
 #include "chrome/common/extensions/permissions/permissions_data.h"
@@ -73,6 +74,7 @@
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
 #include "content/public/renderer/v8_value_converter.h"
+#include "extensions/common/constants.h"
 #include "extensions/common/view_type.h"
 #include "grit/common_resources.h"
 #include "grit/renderer_resources.h"
@@ -1015,11 +1017,10 @@ void Dispatcher::DidCreateScriptContext(
     extension_id = "";
   }
 
-  ExtensionURLInfo url_info(frame->document().securityOrigin(),
-      UserScriptSlave::GetDataSourceURLForFrame(frame));
-
-  Feature::Context context_type =
-      ClassifyJavaScriptContext(extension_id, extension_group, url_info);
+  Feature::Context context_type = ClassifyJavaScriptContext(
+      extension_id, extension_group,
+      UserScriptSlave::GetDataSourceURLForFrame(frame),
+      frame->document().securityOrigin());
 
   ChromeV8Context* context =
       new ChromeV8Context(v8_context, frame, extension, context_type);
@@ -1127,18 +1128,18 @@ std::string Dispatcher::GetExtensionID(const WebFrame* frame, int world_id) {
     return user_script_slave_->GetExtensionIdForIsolatedWorld(world_id);
   }
 
+  // TODO(kalman): Delete this check.
+  if (frame->document().securityOrigin().isUnique())
+    return std::string();
+
   // Extension pages (chrome-extension:// URLs).
   GURL frame_url = UserScriptSlave::GetDataSourceURLForFrame(frame);
-  return extensions_.GetExtensionOrAppIDByURL(
-      ExtensionURLInfo(frame->document().securityOrigin(), frame_url));
+  return extensions_.GetExtensionOrAppIDByURL(frame_url);
 }
 
 bool Dispatcher::IsWithinPlatformApp(const WebFrame* frame) {
-  // We intentionally don't use the origin parameter for ExtensionURLInfo since
-  // it would be empty (i.e. unique) for sandboxed resources and thus not match.
-  ExtensionURLInfo url_info(
-      UserScriptSlave::GetDataSourceURLForFrame(frame->top()));
-  const Extension* extension = extensions_.GetExtensionOrAppByURL(url_info);
+  GURL url(UserScriptSlave::GetDataSourceURLForFrame(frame->top()));
+  const Extension* extension = extensions_.GetExtensionOrAppByURL(url);
 
   return extension && extension->is_platform_app();
 }
@@ -1375,10 +1376,25 @@ void Dispatcher::OnCancelSuspend(const std::string& extension_id) {
   DispatchEvent(extension_id, kOnSuspendCanceledEvent);
 }
 
+// TODO(kalman): This is checking for the wrong thing, it should be checking if
+// the frame's security origin is unique. The extension sandbox directive is
+// checked for in chrome/common/extensions/csp_handler.cc.
+bool Dispatcher::IsSandboxedPage(const GURL& url) const {
+  if (url.SchemeIs(extensions::kExtensionScheme)) {
+    const Extension* extension = extensions_.GetByID(url.host());
+    if (extension) {
+      return extensions::SandboxedPageInfo::IsSandboxedPage(extension,
+                                                            url.path());
+    }
+  }
+  return false;
+}
+
 Feature::Context Dispatcher::ClassifyJavaScriptContext(
     const std::string& extension_id,
     int extension_group,
-    const ExtensionURLInfo& url_info) {
+    const GURL& url,
+    const WebKit::WebSecurityOrigin& origin) {
   DCHECK_GE(extension_group, 0);
   if (extension_group == EXTENSION_GROUP_CONTENT_SCRIPTS) {
     return extensions_.Contains(extension_id) ?
@@ -1391,20 +1407,22 @@ Feature::Context Dispatcher::ClassifyJavaScriptContext(
   //    the extension is considered active.
   // 2. ScriptContext creation (which triggers bindings injection) happens
   //    before the SecurityContext is updated with the sandbox flags (after
-  //    reading the CSP header), so url_info.url().securityOrigin() is not
-  //    unique yet.
-  if (extensions_.IsSandboxedPage(url_info))
+  //    reading the CSP header), so the caller can't check if the context's
+  //    security origin is unique yet.
+  if (IsSandboxedPage(url))
     return Feature::WEB_PAGE_CONTEXT;
 
   if (IsExtensionActive(extension_id))
     return Feature::BLESSED_EXTENSION_CONTEXT;
 
-  if (extensions_.ExtensionBindingsAllowed(url_info)) {
+  // TODO(kalman): This isUnique() check is wrong, it should be performed as
+  // part of IsSandboxedPage().
+  if (!origin.isUnique() && extensions_.ExtensionBindingsAllowed(url)) {
     return extensions_.Contains(extension_id) ?
         Feature::UNBLESSED_EXTENSION_CONTEXT : Feature::UNSPECIFIED_CONTEXT;
   }
 
-  if (url_info.url().is_valid())
+  if (url.is_valid())
     return Feature::WEB_PAGE_CONTEXT;
 
   return Feature::UNSPECIFIED_CONTEXT;
@@ -1433,9 +1451,7 @@ bool Dispatcher::CheckContextAccessToExtensionAPI(
   // Theoretically we could end up with bindings being injected into sandboxed
   // frames, for example content scripts. Don't let them execute API functions.
   WebKit::WebFrame* frame = context->web_frame();
-  ExtensionURLInfo url_info(frame->document().securityOrigin(),
-                            UserScriptSlave::GetDataSourceURLForFrame(frame));
-  if (extensions_.IsSandboxedPage(url_info)) {
+  if (IsSandboxedPage(UserScriptSlave::GetDataSourceURLForFrame(frame))) {
     static const char kMessage[] =
         "%s cannot be used within a sandboxed frame.";
     std::string error_msg = base::StringPrintf(kMessage, function_name.c_str());
diff --git a/chrome/renderer/extensions/dispatcher.h b/chrome/renderer/extensions/dispatcher.h
index a341369..b43067d 100644
--- a/chrome/renderer/extensions/dispatcher.h
+++ b/chrome/renderer/extensions/dispatcher.h
@@ -31,6 +31,7 @@ struct ExtensionMsg_Loaded_Params;
 
 namespace WebKit {
 class WebFrame;
+class WebSecurityOrigin;
 }
 
 namespace base {
@@ -230,10 +231,14 @@ class Dispatcher : public content::RenderProcessObserver {
   // are not in the same origin).
   bool IsWithinPlatformApp(const WebKit::WebFrame* frame);
 
+  bool IsSandboxedPage(const GURL& url) const;
+
   // Returns the Feature::Context type of context for a JavaScript context.
-  Feature::Context ClassifyJavaScriptContext(const std::string& extension_id,
-                                             int extension_group,
-                                             const ExtensionURLInfo& url_info);
+  Feature::Context ClassifyJavaScriptContext(
+      const std::string& extension_id,
+      int extension_group,
+      const GURL& url,
+      const WebKit::WebSecurityOrigin& origin);
 
   // Gets |field| from |object| or creates it as an empty object if it doesn't
   // exist.
diff --git a/chrome/renderer/extensions/request_sender.cc b/chrome/renderer/extensions/request_sender.cc
index 1c2cc0f..02ddac0 100644
--- a/chrome/renderer/extensions/request_sender.cc
+++ b/chrome/renderer/extensions/request_sender.cc
@@ -11,7 +11,6 @@
 #include "content/public/renderer/render_view.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
-#include "third_party/WebKit/public/web/WebSecurityOrigin.h"
 #include "third_party/WebKit/public/web/WebUserGestureIndicator.h"
 
 namespace extensions {
@@ -81,12 +80,8 @@ void RequestSender::StartRequest(Source* source,
     return;
 
   GURL source_url;
-  WebKit::WebSecurityOrigin source_origin;
-  WebKit::WebFrame* webframe = context->web_frame();
-  if (webframe) {
+  if (WebKit::WebFrame* webframe = context->web_frame())
     source_url = webframe->document().url();
-    source_origin = webframe->document().securityOrigin();
-  }
 
   InsertRequest(request_id, new PendingRequest(name, source));
 
@@ -95,7 +90,6 @@ void RequestSender::StartRequest(Source* source,
   params.arguments.Swap(value_args);
   params.extension_id = context->GetExtensionID();
   params.source_url = source_url;
-  params.source_origin = source_origin.toString();
   params.request_id = request_id;
   params.has_callback = has_callback;
   params.user_gesture =
diff --git a/chrome/renderer/extensions/resource_request_policy.cc b/chrome/renderer/extensions/resource_request_policy.cc
index 002bb46..f82e972 100644
--- a/chrome/renderer/extensions/resource_request_policy.cc
+++ b/chrome/renderer/extensions/resource_request_policy.cc
@@ -39,7 +39,7 @@ bool ResourceRequestPolicy::CanRequestResource(
   CHECK(resource_url.SchemeIs(extensions::kExtensionScheme));
 
   const Extension* extension =
-      loaded_extensions->GetExtensionOrAppByURL(ExtensionURLInfo(resource_url));
+      loaded_extensions->GetExtensionOrAppByURL(resource_url);
   if (!extension) {
     // Allow the load in the case of a non-existent extension. We'll just get a
     // 404 from the browser process.