Remove ExtensionURLInfo, make security decisions in render process

chrome/renderer/extensions/dispatcher.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 "chrome/renderer/extensions/dispatcher.h"
 
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/debug/alias.h"
 #include "base/json/json_reader.h"
@@ skipping 997 matching lines @@
     // CSP blocks extension page loading by switching the extension ID to
     // "invalid". This isn't interesting.
     if (extension_id != "invalid") {
       LOG(ERROR) << "Extension \"" << extension_id << "\" not found";
       RenderThread::Get()->RecordUserMetrics("ExtensionNotFound_ED");
     }
 
     extension_id = "";
   }
 
-  ExtensionURLInfo url_info(frame->document().securityOrigin(),
-      UserScriptSlave::GetDataSourceURLForFrame(frame));
+  // Frames loaded on a unique security origin are not accessible to extensions.

[not at google - send to devlin, 2013/07/15 22:09:12]

I've convinced myself that this seems kind of arbitrary and we should be
allowing it.

+  GURL effective_frame_url;
+  if (!frame->document().securityOrigin().isUnique())
+    effective_frame_url = UserScriptSlave::GetDataSourceURLForFrame(frame);
 
-  Feature::Context context_type =
-      ClassifyJavaScriptContext(extension_id, extension_group, url_info);
+  Feature::Context context_type = ClassifyJavaScriptContext(
+      extension_id, extension_group, effective_frame_url);

[Matt Perry, 2013/07/15 21:24:31]

The call to IsSandboxedPage in ClassifyJavaScriptContext will definitely return
false in the case that the security origin is unique. I have no idea if that's
what we want.

Maybe kalman knows.

[not at google - send to devlin, 2013/07/15 22:09:12]

yes I think this is another place where isUnique isn't needed

 
   ChromeV8Context* context =
       new ChromeV8Context(v8_context, frame, extension, context_type);
   v8_context_set_.Add(context);
 
   {
     scoped_ptr<ModuleSystem> module_system(new ModuleSystem(context,
                                                             &source_map_));
     context->set_module_system(module_system.Pass());
   }
@@ skipping 87 matching lines @@
 
   VLOG(1) << "Num tracked contexts: " << v8_context_set_.size();
 }
 
 std::string Dispatcher::GetExtensionID(const WebFrame* frame, int world_id) {
   if (world_id != 0) {
     // Isolated worlds (content script).
     return user_script_slave_->GetExtensionIdForIsolatedWorld(world_id);
   }
 
+  if (frame->document().securityOrigin().isUnique())
+    return std::string();

[not at google - send to devlin, 2013/07/15 22:09:12]

also here

+
   // 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();
 }
 
 void Dispatcher::WillReleaseScriptContext(
     WebFrame* frame, v8::Handle<v8::Context> v8_context, int world_id) {
   ChromeV8Context* context = v8_context_set_.GetByV8Context(v8_context);
   if (!context)
     return;
 
@@ skipping 219 matching lines @@
   RenderThread::Get()->Send(new ExtensionHostMsg_SuspendAck(extension_id));
 }
 
 void Dispatcher::OnCancelSuspend(const std::string& extension_id) {
   DispatchEvent(extension_id, kOnSuspendCanceledEvent);
 }
 
 Feature::Context Dispatcher::ClassifyJavaScriptContext(
     const std::string& extension_id,
     int extension_group,
-    const ExtensionURLInfo& url_info) {
+    const GURL& url) {
   DCHECK_GE(extension_group, 0);
   if (extension_group == EXTENSION_GROUP_CONTENT_SCRIPTS) {
     return extensions_.Contains(extension_id) ?
         Feature::CONTENT_SCRIPT_CONTEXT : Feature::UNSPECIFIED_CONTEXT;
   }
 
   // We have an explicit check for sandboxed pages before checking whether the
   // extension is active in this process because:
   // 1. Sandboxed pages run in the same process as regular extension pages, so
   //    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 (extensions_.IsSandboxedPage(url))
     return Feature::WEB_PAGE_CONTEXT;
 
   if (IsExtensionActive(extension_id))
     return Feature::BLESSED_EXTENSION_CONTEXT;
 
-  if (extensions_.ExtensionBindingsAllowed(url_info)) {
+  if (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;
 }
 
 void Dispatcher::OnExtensionResponse(int request_id,
                                      bool success,
                                      const base::ListValue& response,
                                      const std::string& error) {
   request_sender_->HandleResponse(request_id, success, response, error);
 }
 
 bool Dispatcher::CheckContextAccessToExtensionAPI(
     const std::string& function_name, ChromeV8Context* context) const {
   if (!context) {
     DLOG(ERROR) << "Not in a v8::Context";
     return false;
   }
 
   if (!context->extension()) {
     v8::ThrowException(
         v8::Exception::Error(v8::String::New("Not in an extension.")));
     return false;
   }
 
   // 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 (frame->document().securityOrigin().isUnique() ||
+      extensions_.IsSandboxedPage(
+          UserScriptSlave::GetDataSourceURLForFrame(frame))) {

[not at google - send to devlin, 2013/07/15 22:09:12]

I don't actually think we should be preventing scripts from running in
web-sandboxed pages, just extension-sandboxed ones (then let host permissions do
the rest).

I think this would prevent extensions from running content scripts on file://
URLs which isn't desirable.

[not at google - send to devlin, 2013/07/15 22:27:10]

Ok I'm being dense and mixed up host and API permissions.

However it would prevent content scripts from calling any APIs on file:// URLs,
like chrome.runtime.sendMessage, which is nevertheless undesirable.

[jamesr, 2013/07/15 22:55:27]

It appears that chrome.runtime.sendMessage() fails in content scripts injected
into file:// URLs with or without this patch.  I agree that's bad but am not
sure exactly how it happens.  Removing this call doesn't make
chrome.runtime.sendMessage() work.

[not at google - send to devlin, 2013/07/15 23:00:53]

Yeah looks like there's an existing issue at play here too. However, as
discussed, removing the isUnque check here did make the chrome.storage functions
work?

     static const char kMessage[] =
         "%s cannot be used within a sandboxed frame.";
     std::string error_msg = base::StringPrintf(kMessage, function_name.c_str());
     v8::ThrowException(
         v8::Exception::Error(v8::String::New(error_msg.c_str())));
     return false;
   }
 
   Feature::Availability availability = context->GetAvailability(function_name);
   if (!availability.is_available()) {
@@ skipping 53 matching lines @@
     RenderView* background_view =
         ExtensionHelper::GetBackgroundPage(extension_id);
     if (background_view) {
       background_view->Send(new ExtensionHostMsg_EventAck(
           background_view->GetRoutingID()));
     }
   }
 }
 
 }  // namespace extensions