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
author: jamesr@chromium.org <jamesr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> 2013-07-18 09:28:55 +0000
committer: jamesr@chromium.org <jamesr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> 2013-07-18 09:28:55 +0000
commit: be9915fbccfaab301759501d888579976427acfb (patch)
tree: c7072e00db4305cb6b65a33895dbd1dbb47e709a /chrome/common/extensions
parent: acdaa9963d419b7b72bd8201857a23f29f626959 (diff)
download: chromium_src-be9915fbccfaab301759501d888579976427acfb.zip
chromium_src-be9915fbccfaab301759501d888579976427acfb.tar.gz
chromium_src-be9915fbccfaab301759501d888579976427acfb.tar.bz2
6 files changed, 32 insertions, 103 deletions
diff --git a/chrome/common/extensions/extension_messages.h b/chrome/common/extensions/extension_messages.h
index 37dbfdc..9403e29 100644
--- a/chrome/common/extensions/extension_messages.h
+++ b/chrome/common/extensions/extension_messages.h
@@ -80,9 +80,6 @@ IPC_STRUCT_BEGIN(ExtensionHostMsg_Request_Params)
   // extension. Or, they can originate from hosted apps or normal web pages.
   IPC_STRUCT_MEMBER(GURL, source_url)
 
-  // Web security origin of the frame the request was sent from.
-  IPC_STRUCT_MEMBER(string16, source_origin)
-
   // Unique request id to match requests and responses.
   IPC_STRUCT_MEMBER(int, request_id)
 
diff --git a/chrome/common/extensions/extension_process_policy.cc b/chrome/common/extensions/extension_process_policy.cc
index b38f397..cb938ee 100644
--- a/chrome/common/extensions/extension_process_policy.cc
+++ b/chrome/common/extensions/extension_process_policy.cc
@@ -11,7 +11,7 @@
 namespace extensions {
 
 const extensions::Extension* GetNonBookmarkAppExtension(
-    const ExtensionSet& extensions, const ExtensionURLInfo& url) {
+    const ExtensionSet& extensions, const GURL& url) {
   // Exclude bookmark apps, which do not use the app process model.
   const extensions::Extension* extension =
       extensions.GetExtensionOrAppByURL(url);
@@ -22,8 +22,8 @@ const extensions::Extension* GetNonBookmarkAppExtension(
 
 bool CrossesExtensionProcessBoundary(
     const ExtensionSet& extensions,
-    const ExtensionURLInfo& old_url,
-    const ExtensionURLInfo& new_url,
+    const GURL& old_url,
+    const GURL& new_url,
     bool should_consider_workaround) {
   const extensions::Extension* old_url_extension = GetNonBookmarkAppExtension(
       extensions,
diff --git a/chrome/common/extensions/extension_process_policy.h b/chrome/common/extensions/extension_process_policy.h
index 19ad254..7231c0f 100644
--- a/chrome/common/extensions/extension_process_policy.h
+++ b/chrome/common/extensions/extension_process_policy.h
@@ -6,7 +6,7 @@
 #define CHROME_COMMON_EXTENSIONS_EXTENSION_PROCESS_POLICY_H_
 
 class ExtensionSet;
-class ExtensionURLInfo;
+class GURL;
 
 namespace extensions {
 
@@ -15,7 +15,7 @@ class Extension;
 // Returns the extension for the given URL.  Excludes extension objects for
 // bookmark apps, which do not use the app process model.
 const Extension* GetNonBookmarkAppExtension(const ExtensionSet& extensions,
-                                            const ExtensionURLInfo& url);
+                                            const GURL& url);
 
 // Check if navigating a toplevel page from |old_url| to |new_url| would cross
 // an extension process boundary (e.g. navigating from a web URL into an
@@ -25,8 +25,8 @@ const Extension* GetNonBookmarkAppExtension(const ExtensionSet& extensions,
 // http://crbug.com/59285.
 bool CrossesExtensionProcessBoundary(
     const ExtensionSet& extensions,
-    const ExtensionURLInfo& old_url,
-    const ExtensionURLInfo& new_url,
+    const GURL& old_url,
+    const GURL& new_url,
     bool should_consider_workaround);
 
 }  // namespace extensions
diff --git a/chrome/common/extensions/extension_set.cc b/chrome/common/extensions/extension_set.cc
index ee538bf..5c6861c 100644
--- a/chrome/common/extensions/extension_set.cc
+++ b/chrome/common/extensions/extension_set.cc
@@ -11,19 +11,8 @@
 #include "chrome/common/url_constants.h"
 #include "extensions/common/constants.h"
 
-using WebKit::WebSecurityOrigin;
 using extensions::Extension;
 
-ExtensionURLInfo::ExtensionURLInfo(WebSecurityOrigin origin, const GURL& url)
-  : origin_(origin),
-    url_(url) {
-  DCHECK(!origin_.isNull());
-}
-
-ExtensionURLInfo::ExtensionURLInfo(const GURL& url)
-  : url_(url) {
-}
-
 ExtensionSet::const_iterator::const_iterator() {}
 
 ExtensionSet::const_iterator::const_iterator(const const_iterator& other)
@@ -75,41 +64,28 @@ void ExtensionSet::Clear() {
   extensions_.clear();
 }
 
-std::string ExtensionSet::GetExtensionOrAppIDByURL(
-    const ExtensionURLInfo& info) const {
-  DCHECK(!info.origin().isNull());
-
-  if (info.url().SchemeIs(extensions::kExtensionScheme))
-    return info.origin().isUnique() ? std::string() : info.url().host();
+std::string ExtensionSet::GetExtensionOrAppIDByURL(const GURL& url) const {
+  if (url.SchemeIs(extensions::kExtensionScheme))
+    return url.host();
 
-  const Extension* extension = GetExtensionOrAppByURL(info);
+  const Extension* extension = GetExtensionOrAppByURL(url);
   if (!extension)
     return std::string();
 
   return extension->id();
 }
 
-const Extension* ExtensionSet::GetExtensionOrAppByURL(
-    const ExtensionURLInfo& info) const {
-  // In the common case, the document's origin will correspond to its URL,
-  // but in some rare cases involving sandboxing, the two will be different.
-  // We catch those cases by checking whether the document's origin is unique.
-  // If that's not the case, then we conclude that the document's security
-  // context is well-described by its URL and proceed to use only the URL.
-  if (!info.origin().isNull() && info.origin().isUnique())
-    return NULL;
-
-  if (info.url().SchemeIs(extensions::kExtensionScheme))
-    return GetByID(info.url().host());
+const Extension* ExtensionSet::GetExtensionOrAppByURL(const GURL& url) const {
+  if (url.SchemeIs(extensions::kExtensionScheme))
+    return GetByID(url.host());
 
-  return GetHostedAppByURL(info);
+  return GetHostedAppByURL(url);
 }
 
-const Extension* ExtensionSet::GetHostedAppByURL(
-    const ExtensionURLInfo& info) const {
+const Extension* ExtensionSet::GetHostedAppByURL(const GURL& url) const {
   for (ExtensionMap::const_iterator iter = extensions_.begin();
        iter != extensions_.end(); ++iter) {
-    if (iter->second->web_extent().MatchesURL(info.url()))
+    if (iter->second->web_extent().MatchesURL(url))
       return iter->second.get();
   }
 
@@ -129,8 +105,8 @@ const Extension* ExtensionSet::GetHostedAppByOverlappingWebExtent(
 
 bool ExtensionSet::InSameExtent(const GURL& old_url,
                                 const GURL& new_url) const {
-  return GetExtensionOrAppByURL(ExtensionURLInfo(old_url)) ==
-      GetExtensionOrAppByURL(ExtensionURLInfo(new_url));
+  return GetExtensionOrAppByURL(old_url) ==
+      GetExtensionOrAppByURL(new_url);
 }
 
 const Extension* ExtensionSet::GetByID(const std::string& id) const {
@@ -150,31 +126,16 @@ std::set<std::string> ExtensionSet::GetIDs() const {
   return ids;
 }
 
-bool ExtensionSet::ExtensionBindingsAllowed(
-    const ExtensionURLInfo& info) const {
-  if (info.origin().isUnique() || IsSandboxedPage(info))
-    return false;
-
-  if (info.url().SchemeIs(extensions::kExtensionScheme))
+bool ExtensionSet::ExtensionBindingsAllowed(const GURL& url) const {
+  if (url.SchemeIs(extensions::kExtensionScheme))
     return true;
 
   ExtensionMap::const_iterator i = extensions_.begin();
   for (; i != extensions_.end(); ++i) {
     if (i->second->location() == extensions::Manifest::COMPONENT &&
-        i->second->web_extent().MatchesURL(info.url()))
+        i->second->web_extent().MatchesURL(url))
       return true;
   }
 
   return false;
 }
-
-bool ExtensionSet::IsSandboxedPage(const ExtensionURLInfo& info) const {
-  if (info.url().SchemeIs(extensions::kExtensionScheme)) {
-    const Extension* extension = GetByID(info.url().host());
-    if (extension) {
-      return extensions::SandboxedPageInfo::IsSandboxedPage(extension,
-                                                            info.url().path());
-    }
-  }
-  return false;
-}
diff --git a/chrome/common/extensions/extension_set.h b/chrome/common/extensions/extension_set.h
index 34a39f3..e07a629 100644
--- a/chrome/common/extensions/extension_set.h
+++ b/chrome/common/extensions/extension_set.h
@@ -12,31 +12,8 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "chrome/common/extensions/extension.h"
-#include "third_party/WebKit/public/web/WebSecurityOrigin.h"
 #include "url/gurl.h"
 
-class ExtensionURLInfo {
- public:
-  // The extension system uses both a document's origin and its URL to
-  // grant permissions. Ideally, we would use only the origin, but because
-  // the web extent of a hosted app can be less than an entire origin, we
-  // take the URL into account as well
-  ExtensionURLInfo(WebKit::WebSecurityOrigin origin, const GURL& url);
-
-  // WARNING! Using this constructor can miss important security checks if
-  //          you're trying to find a running extension. For example, if the
-  //          URL in question is being rendered inside an iframe sandbox, then
-  //          we might incorrectly grant it access to powerful extension APIs.
-  explicit ExtensionURLInfo(const GURL& url);
-
-  const WebKit::WebSecurityOrigin& origin() const { return origin_; }
-  const GURL& url() const { return url_; }
-
- private:
-  WebKit::WebSecurityOrigin origin_;
-  GURL url_;
-};
-
 // The one true extension container. Extensions are identified by their id.
 // Only one extension can be in the set with a given ID.
 class ExtensionSet {
@@ -102,19 +79,17 @@ class ExtensionSet {
 
   // Returns the extension ID, or empty if none. This includes web URLs that
   // are part of an extension's web extent.
-  std::string GetExtensionOrAppIDByURL(const ExtensionURLInfo& info) const;
+  std::string GetExtensionOrAppIDByURL(const GURL& url) const;
 
   // Returns the Extension, or NULL if none.  This includes web URLs that are
   // part of an extension's web extent.
   // NOTE: This can return NULL if called before UpdateExtensions receives
   // bulk extension data (e.g. if called from
   // EventBindings::HandleContextCreated)
-  const extensions::Extension* GetExtensionOrAppByURL(
-      const ExtensionURLInfo& info) const;
+  const extensions::Extension* GetExtensionOrAppByURL(const GURL& url) const;
 
   // Returns the hosted app whose web extent contains the URL.
-  const extensions::Extension* GetHostedAppByURL(
-      const ExtensionURLInfo& info) const;
+  const extensions::Extension* GetHostedAppByURL(const GURL& url) const;
 
   // Returns a hosted app that contains any URL that overlaps with the given
   // extent, if one exists.
@@ -134,11 +109,7 @@ class ExtensionSet {
   // Returns true if |info| should get extension api bindings and be permitted
   // to make api calls. Note that this is independent of what extension
   // permissions the given extension has been granted.
-  bool ExtensionBindingsAllowed(const ExtensionURLInfo& info) const;
-
-  // Returns true if |info| is an extension page that is to be served in a
-  // unique sandboxed origin.
-  bool IsSandboxedPage(const ExtensionURLInfo& info) const;
+  bool ExtensionBindingsAllowed(const GURL& url) const;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(ExtensionSetTest, ExtensionSet);
diff --git a/chrome/common/extensions/extension_set_unittest.cc b/chrome/common/extensions/extension_set_unittest.cc
index 36dc7ea..84a9ae3 100644
--- a/chrome/common/extensions/extension_set_unittest.cc
+++ b/chrome/common/extensions/extension_set_unittest.cc
@@ -83,19 +83,19 @@ TEST(ExtensionSetTest, ExtensionSet) {
 
   // Get extension by its chrome-extension:// URL
   EXPECT_EQ(ext2, extensions.GetExtensionOrAppByURL(
-      ExtensionURLInfo(ext2->GetResourceURL("test.html"))));
+      ext2->GetResourceURL("test.html")));
   EXPECT_EQ(ext3, extensions.GetExtensionOrAppByURL(
-      ExtensionURLInfo(ext3->GetResourceURL("test.html"))));
+      ext3->GetResourceURL("test.html")));
   EXPECT_EQ(ext4, extensions.GetExtensionOrAppByURL(
-      ExtensionURLInfo(ext4->GetResourceURL("test.html"))));
+      ext4->GetResourceURL("test.html")));
 
   // Get extension by web extent.
   EXPECT_EQ(ext2, extensions.GetExtensionOrAppByURL(
-      ExtensionURLInfo(GURL("http://code.google.com/p/chromium/monkey"))));
+      GURL("http://code.google.com/p/chromium/monkey")));
   EXPECT_EQ(ext3, extensions.GetExtensionOrAppByURL(
-      ExtensionURLInfo(GURL("http://dev.chromium.org/design-docs/"))));
+      GURL("http://dev.chromium.org/design-docs/")));
   EXPECT_FALSE(extensions.GetExtensionOrAppByURL(
-      ExtensionURLInfo(GURL("http://blog.chromium.org/"))));
+      GURL("http://blog.chromium.org/")));
 
   // Test InSameExtent().
   EXPECT_TRUE(extensions.InSameExtent(
author	jamesr@chromium.org <jamesr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>	2013-07-18 09:28:55 +0000
committer	jamesr@chromium.org <jamesr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>	2013-07-18 09:28:55 +0000
commit	be9915fbccfaab301759501d888579976427acfb (patch)
tree	c7072e00db4305cb6b65a33895dbd1dbb47e709a /chrome/common/extensions
parent	acdaa9963d419b7b72bd8201857a23f29f626959 (diff)
download	chromium_src-be9915fbccfaab301759501d888579976427acfb.zip chromium_src-be9915fbccfaab301759501d888579976427acfb.tar.gz chromium_src-be9915fbccfaab301759501d888579976427acfb.tar.bz2