Revert r184245: It broke web_accessible_resources for extension iframes.

> Non-web-accessible extension URLs should not load in non-extension processes
> 
> Blocking of URL loads in the browser process cannot be as extensive as the
> one done in the renderer process, as we don't have the frame URL and page URL
> at resource load time. I've tried to do a similar check with the data available
> on the browser side.
> 
> The main part which I'm not entirely happy about is the check for DevTools
> pages. Currently, if an extension has DevTools page, all of its resources can
> be loaded by a compromised renderer. I think this can be tightened up with a
> follow up CL, if we find a good way of distinguishing DevTools processes or
> permissioning those requests.
> 
> BUG=173688
> 
> Review URL: https://chromiumcodereview.appspot.com/12218064

BUG=179127
TBR=nasko@chromium.org

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

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

6 files changed, 5 insertions, 175 deletions

diff --git a/chrome/browser/chrome_security_exploit_browsertest.cc b/chrome/browser/chrome_security_exploit_browsertest.cc
deleted file mode 100644
index 01fd458..0000000
--- a/chrome/browser/chrome_security_exploit_browsertest.cc
+++ /dev/null
@@ -1,73 +0,0 @@
-// Copyright (c) 2013 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 "base/command_line.h"
-#include "base/stringprintf.h"
-#include "base/utf_string_conversions.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_commands.h"
-#include "chrome/browser/ui/singleton_tabs.h"
-#include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/test/base/in_process_browser_test.h"
-#include "chrome/test/base/ui_test_utils.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_types.h"
-#include "content/public/browser/resource_request_details.h"
-#include "content/public/browser/web_contents_observer.h"
-#include "content/public/common/content_switches.h"
-#include "content/public/test/browser_test_utils.h"
-#include "webkit/glue/glue_serialize.h"
-
-namespace content {
-
-// The goal of these tests is to "simulate" exploited renderer processes, which
-// can send arbitrary IPC messages and confuse browser process internal state,
-// leading to security bugs. We are trying to verify that the browser doesn't
-// perform any dangerous operations in such cases.
-// This is similar to the security_exploit_browsertest.cc tests, but also
-// includes chrome/ layer concepts such as extensions.
-class ChromeSecurityExploitBrowserTest : public InProcessBrowserTest {
- public:
-  ChromeSecurityExploitBrowserTest() {}
-
-  virtual void SetUpCommandLine(CommandLine* command_line) {
-    ASSERT_TRUE(test_server()->Start());
-    net::TestServer https_server(
-        net::TestServer::TYPE_HTTPS,
-        net::TestServer::kLocalhost,
-        base::FilePath(FILE_PATH_LITERAL("chrome/test/data")));
-    ASSERT_TRUE(https_server.Start());
-
-    // Add a host resolver rule to map all outgoing requests to the test server.
-    // This allows us to use "real" hostnames in URLs, which we can use to
-    // create arbitrary SiteInstances.
-    command_line->AppendSwitchASCII(
-        switches::kHostResolverRules,
-        "MAP * " + test_server()->host_port_pair().ToString() +
-            ",EXCLUDE localhost");
-
-    // Since we assume exploited renderer process, it can bypass the same origin
-    // policy at will. Simulate that by passing the disable-web-security flag.
-    command_line->AppendSwitch(switches::kDisableWebSecurity);
-  }
-};
-
-IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTest,
-                       ChromeExtensionResources) {
-  // Load a page that requests a chrome-extension:// image through XHR. We
-  // expect this load to fail, as it is an illegal request.
-  GURL foo("http://foo.com/files/chrome_extension_resource.html");
-
-  content::DOMMessageQueue msg_queue;
-
-  ui_test_utils::NavigateToURL(browser(), foo);
-
-  std::string status;
-  std::string expected_status("0");
-  EXPECT_TRUE(msg_queue.WaitForMessage(&status));
-  EXPECT_STREQ(status.c_str(), expected_status.c_str());
-}
-
-}
diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc
index 4072d39..dd1d218 100644
--- a/chrome/browser/extensions/extension_protocols.cc
+++ b/chrome/browser/extensions/extension_protocols.cc
@@ -26,7 +26,6 @@
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_file_util.h"
 #include "chrome/common/extensions/extension_resource.h"
-#include "chrome/common/extensions/manifest_url_handler.h"
 #include "chrome/common/extensions/web_accessible_resources_handler.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/resource_request_info.h"
@@ -270,7 +269,6 @@ bool ExtensionCanLoadInIncognito(const ResourceRequestInfo* info,
 // first need to find a way to get CanLoadInIncognito state into the renderers.
 bool AllowExtensionResourceLoad(net::URLRequest* request,
                                 bool is_incognito,
-                                const Extension* extension,
                                 ExtensionInfoMap* extension_info_map) {
   const ResourceRequestInfo* info = ResourceRequestInfo::ForRequest(request);
 
@@ -287,57 +285,6 @@ bool AllowExtensionResourceLoad(net::URLRequest* request,
     return false;
   }
 
-  // The following checks are meant to replicate similar set of checks in the
-  // renderer process, performed by ResourceRequestPolicy::CanRequestResource.
-  // These are not exactly equivalent, because we don't have the same bits of
-  // information. The two checks need to be kept in sync as much as possible, as
-  // an exploited renderer can bypass the checks in ResourceRequestPolicy.
-
-  // Check if the extension for which this request is made is indeed loaded in
-  // the process sending the request. If not, we need to explicitly check if
-  // the resource is explicitly accessible or fits in a set of exception cases.
-  // Note: This allows a case where two extensions execute in the same renderer
-  // process to request each other's resources. We can't do more precise check,
-  // since the renderer can lie which extension has made the request.
-  if (extension_info_map->process_map().Contains(
-      request->url().host(), info->GetChildID())) {
-    return true;
-  }
-
-  if (!content::PageTransitionIsWebTriggerable(info->GetPageTransition()))
-    return false;
-
-  // The following checks require that we have an actual extension object. If we
-  // don't have it, allow the request handling to continue with the rest of the
-  // checks.
-  if (!extension)
-    return true;
-
-  // Disallow loading of packaged resources for hosted apps. We don't allow
-  // hybrid hosted/packaged apps. The one exception is access to icons, since
-  // some extensions want to be able to do things like create their own
-  // launchers.
-  std::string resource_root_relative_path =
-      request->url().path().empty() ? "" : request->url().path().substr(1);
-  if (extension->is_hosted_app() &&
-      !extensions::IconsInfo::GetIcons(extension).ContainsPath(
-          resource_root_relative_path)) {
-    LOG(ERROR) << "Denying load of " << request->url().spec() << " from "
-               << "hosted app.";
-    return false;
-  }
-
-  // If the resource is not expicitly marked as web accessible, it should only
-  // be allowed if it is being loaded by DevTools. A close approximation is
-  // checking if the extension contains DevTools page.
-  // IsResourceWebAccessible already does the manifest version check, so no
-  // need to explicitly do it.
-  if (!extensions::WebAccessibleResourcesInfo::IsResourceWebAccessible(
-          extension, request->url().path()) &&
-      extensions::ManifestURL::GetDevToolsPage(extension).is_empty()) {
-    return false;
-  }
-
   return true;
 }
 
@@ -379,18 +326,17 @@ class ExtensionProtocolHandler
 net::URLRequestJob*
 ExtensionProtocolHandler::MaybeCreateJob(
     net::URLRequest* request, net::NetworkDelegate* network_delegate) const {
-  // chrome-extension://extension-id/resource/path.js
-  const std::string& extension_id = request->url().host();
-  const Extension* extension =
-      extension_info_map_->extensions().GetByID(extension_id);
-
   // TODO(mpcomplete): better error code.
   if (!AllowExtensionResourceLoad(
-           request, is_incognito_, extension, extension_info_map_)) {
+           request, is_incognito_, extension_info_map_)) {
     return new net::URLRequestErrorJob(
         request, network_delegate, net::ERR_ADDRESS_UNREACHABLE);
   }
 
+  // chrome-extension://extension-id/resource/path.js
+  const std::string& extension_id = request->url().host();
+  const Extension* extension =
+      extension_info_map_->extensions().GetByID(extension_id);
   base::FilePath directory_path;
   if (extension)
     directory_path = extension->path();
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 0b28020..ec2795a 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1040,7 +1040,6 @@
         'browser/chrome_content_browser_client_browsertest.cc',
         'browser/chrome_main_browsertest.cc',
         'browser/chrome_plugin_browsertest.cc',
-        'browser/chrome_security_exploit_browsertest.cc',
         'browser/chrome_switches_browsertest.cc',
         'browser/chromeos/accessibility/magnification_manager_browsertest.cc',
         'browser/chromeos/app_mode/kiosk_app_manager_browsertest.cc',
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index da65d7d1..03e8804 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -53,9 +53,6 @@ class APIPermissionSet;
 class PermissionSet;
 
 // Represents a Chrome extension.
-// Once created, an Extension object is immutable, with the exception of its
-// RuntimeData. This makes it safe to use on any thread, since access to the
-// RuntimeData is protected by a lock.
 class Extension : public base::RefCountedThreadSafe<Extension> {
  public:
   struct ManifestData;
diff --git a/chrome/renderer/extensions/resource_request_policy.cc b/chrome/renderer/extensions/resource_request_policy.cc
index 5843d8d..756844f 100644
--- a/chrome/renderer/extensions/resource_request_policy.cc
+++ b/chrome/renderer/extensions/resource_request_policy.cc
@@ -24,12 +24,6 @@
 
 namespace extensions {
 
-// This method does a security check whether chrome-extension:// URLs can be
-// requested by the renderer. Since this is in an untrusted process, the browser
-// has a similar check to enforce the policy, in case this process is exploited.
-// If you are changing this function, ensure equivalent checks are added to
-// extension_protocols.cc AllowExtensionResourceLoad.
-
 // static
 bool ResourceRequestPolicy::CanRequestResource(
     const GURL& resource_url,
diff --git a/chrome/test/data/chrome_extension_resource.html b/chrome/test/data/chrome_extension_resource.html
deleted file mode 100644
index 34c996c..0000000
--- a/chrome/test/data/chrome_extension_resource.html
+++ /dev/null
@@ -1,33 +0,0 @@
-<html>
-<head>
-<script>
-var xhrStatus = -1;
-var imgUrl = 'chrome-extension://eemcgdkfndhakfknompkggombfjjjeno/images/bookmark_manager_recent.png';
-
-window.onload = function() {
-  // The call to pushState with chrome-extension:// URL will succeed, since the
-  // test uses --disable-web-security.
-  history.pushState('', '',
-      'chrome-extension://eemcgdkfndhakfknompkggombfjjjeno/main.html');
-  var xhr = new XMLHttpRequest();
-  xhr.onreadystatechange = function() {
-    if (xhr.readyState == 4) {
-      xhrStatus = xhr.status;    
-      if (xhrStatus == 200) {
-        document.getElementById('star').src =
-            window.URL.createObjectURL(this.response);
-      }
-      domAutomationController.setAutomationId(0);
-      domAutomationController.send(xhr.status);
-    }
-  }
-  xhr.open('GET', imgUrl);
-  xhr.responseType = 'blob';
-  xhr.send();
-}
-</script>
-</head>
-<body>
-<img id='star'>
-</body>
-</html>