chrome-search: should not be display-isolated

chrome/renderer/chrome_content_renderer_client.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/chrome_content_renderer_client.h"
 
 #include <string>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 230 matching lines @@
     thread->GetChannel()->set_outgoing_message_filter(LoadExternalIPCFuzzer());
   }
   // chrome:, chrome-search:, chrome-devtools:, and chrome-internal: pages
   // should not be accessible by normal content, and should also be unable to
   // script anything but themselves (to help limit the damage that a corrupt
   // page could cause).
   WebString chrome_ui_scheme(ASCIIToUTF16(chrome::kChromeUIScheme));
   WebSecurityPolicy::registerURLSchemeAsDisplayIsolated(chrome_ui_scheme);
 
   WebString chrome_search_scheme(ASCIIToUTF16(chrome::kChromeSearchScheme));
-  WebSecurityPolicy::registerURLSchemeAsDisplayIsolated(chrome_search_scheme);
+  if (!command_line->HasSwitch(switches::kInstantProcess))

[Charlie Reis, 2013/03/21 00:08:17]

Why would we register this (or the one above) at all in a non-instant process?

[dhollowa, 2013/03/21 01:30:19]

The one above, chrome:, is linked from external places I suppose, like help
pages.  But has nothing to do with Instant.

The registering of the chrome-search: protocol in general is a somewhat global
affair, ProfileIOData::IsHandledProtocol for example, so is cumbersome at best
to avoid.

A cleaner approach is to use this display-isolated setting here to disallow
access except from chrome-search: and Instant pages.

To reiterate, a non-Instant process, when trying to access
chrome-search://something, will bump up against the following:

  1. Renderer: the display-isolated check in WebKit, so will be denied,
  2. Browser: (Assuming they got by #1), the scheme checks in
URLDataSource::ShouldServiceRequest,
  3. Browser: for specific sub-classes of URLDataSource, like ThemeSource there
are additional Insant-PID checks that make sure the request is coming from a
blessed Instant process.

So with all this, I believe we're good.

[Charlie Reis, 2013/03/21 18:06:42]

My mistake; I was misreading that.
Thanks for the explanation.  I don't think it's obvious from reading this code,
so could you add a comment here explaining that the Instant process can *only*
display the content (not read it) and other processes can't display it or read
it.

I think it would also be worthwhile to add some of this context to the
kChromeSearchScheme declaration in url_constants.h, to avoid confusion about how
we expect this scheme to be handled.

+    WebSecurityPolicy::registerURLSchemeAsDisplayIsolated(chrome_search_scheme);
 
   WebString dev_tools_scheme(ASCIIToUTF16(chrome::kChromeDevToolsScheme));
   WebSecurityPolicy::registerURLSchemeAsDisplayIsolated(dev_tools_scheme);
 
   WebString internal_scheme(ASCIIToUTF16(chrome::kChromeInternalScheme));
   WebSecurityPolicy::registerURLSchemeAsDisplayIsolated(internal_scheme);
 
 #if defined(OS_CHROMEOS)
   WebString drive_scheme(ASCIIToUTF16(chrome::kDriveScheme));
   WebSecurityPolicy::registerURLSchemeAsLocal(drive_scheme);
@@ skipping 865 matching lines @@
     return false;
 
   if (container->element().shadowHost().isNull())
     return false;
 
   return container->element().shadowHost().tagName().equals(
       WebString::fromUTF8(kWebViewTagName));
 }
 
 }  // namespace chrome