diff options
23 files changed, 315 insertions, 61 deletions
diff --git a/chrome/browser/browsing_instance.cc b/chrome/browser/browsing_instance.cc index ff3346a..8aeb69c 100644 --- a/chrome/browser/browsing_instance.cc +++ b/chrome/browser/browsing_instance.cc @@ -8,6 +8,7 @@ #include "chrome/browser/profile.h" #include "chrome/browser/renderer_host/site_instance.h" #include "chrome/common/chrome_switches.h" +#include "chrome/common/extensions/extension.h" #include "chrome/common/url_constants.h" /*static*/ @@ -31,9 +32,11 @@ bool BrowsingInstance::ShouldUseProcessPerSite(const GURL& url) { // process creation logic in RenderProcessHost, so we do not need to worry // about it here. if (url.SchemeIs(chrome::kChromeUIScheme) || - url.SchemeIs(chrome::kExtensionScheme)) + url.SchemeIs(chrome::kExtensionScheme) || + Extension::IsGalleryURL(url)) // Always consolidate instances of the new tab page (and instances of any - // other internal resource urls), as well as extensions. + // other internal resource urls), as well as extensions and extension + // gallery pages, which have special install privileges. return true; // TODO(creis): List any other special cases that we want to limit to a diff --git a/chrome/browser/child_process_security_policy.cc b/chrome/browser/child_process_security_policy.cc index 635e0cf..6d5482b 100644 --- a/chrome/browser/child_process_security_policy.cc +++ b/chrome/browser/child_process_security_policy.cc @@ -19,7 +19,8 @@ class ChildProcessSecurityPolicy::SecurityState { public: SecurityState() : enabled_bindings_(0), - can_read_raw_cookies_(false) { } + can_read_raw_cookies_(false), + can_install_extensions_silently_(false) { } ~SecurityState() { scheme_policy_.clear(); } @@ -47,6 +48,10 @@ class ChildProcessSecurityPolicy::SecurityState { can_read_raw_cookies_ = true; } + void GrantInstallExtensionsSilently() { + can_install_extensions_silently_ = true; + } + void RevokeReadRawCookies() { can_read_raw_cookies_ = false; } @@ -76,6 +81,10 @@ class ChildProcessSecurityPolicy::SecurityState { return BindingsPolicy::is_extension_enabled(enabled_bindings_); } + bool can_install_extensions_silently() const { + return can_install_extensions_silently_; + } + bool can_read_raw_cookies() const { return can_read_raw_cookies_; } @@ -98,6 +107,8 @@ class ChildProcessSecurityPolicy::SecurityState { bool can_read_raw_cookies_; + bool can_install_extensions_silently_; + DISALLOW_COPY_AND_ASSIGN(SecurityState); }; @@ -268,6 +279,17 @@ void ChildProcessSecurityPolicy::GrantExtensionBindings(int renderer_id) { state->second->GrantBindings(BindingsPolicy::EXTENSION); } +void ChildProcessSecurityPolicy::GrantInstallExtensionsSilently( + int renderer_id) { + AutoLock lock(lock_); + + SecurityStateMap::iterator state = security_state_.find(renderer_id); + if (state == security_state_.end()) + return; + + state->second->GrantInstallExtensionsSilently(); +} + void ChildProcessSecurityPolicy::GrantReadRawCookies(int renderer_id) { AutoLock lock(lock_); @@ -362,6 +384,16 @@ bool ChildProcessSecurityPolicy::HasExtensionBindings(int renderer_id) { return state->second->has_extension_bindings(); } +bool ChildProcessSecurityPolicy::CanInstallExtensionsSilently(int renderer_id) { + AutoLock lock(lock_); + + SecurityStateMap::iterator state = security_state_.find(renderer_id); + if (state == security_state_.end()) + return false; + + return state->second->can_install_extensions_silently(); +} + bool ChildProcessSecurityPolicy::CanReadRawCookies(int renderer_id) { AutoLock lock(lock_); diff --git a/chrome/browser/child_process_security_policy.h b/chrome/browser/child_process_security_policy.h index 9cde10a..20ff53b 100644 --- a/chrome/browser/child_process_security_policy.h +++ b/chrome/browser/child_process_security_policy.h @@ -80,6 +80,11 @@ class ChildProcessSecurityPolicy { // Grant this renderer the ability to use extension Bindings. void GrantExtensionBindings(int renderer_id); + // Grant bindings to this renderer for the ability to silently install + // extensions. Granting this permission to a renderer requires that the + // renderer ONLY be used for gallery URLS. + void GrantInstallExtensionsSilently(int renderer_id); + // Grant this renderer the ability to read raw cookies. void GrantReadRawCookies(int renderer_id); @@ -101,11 +106,17 @@ class ChildProcessSecurityPolicy { // allowed to use DOMUIBindings. bool HasDOMUIBindings(int renderer_id); - // Returns true if the specified renderer_id has been granted DOMUIBindings. - // The browser should check this property before assuming the renderer is - // allowed to use extension bindings. + // Returns true if the specified renderer_id has been granted extension + // bindings. The browser should check this property before assuming the + // renderer is allowed to use extension bindings. bool HasExtensionBindings(int renderer_id); + // Returns true if the specified renderer_id has been granted + // InstallExtensionsSilently. The browser should check this property before + // allowing an extension to be installed without prompting the user + // for confirmation. + bool CanInstallExtensionsSilently(int renderer_id); + // Returns true if the specified renderer_id has been granted ReadRawCookies. bool CanReadRawCookies(int renderer_id); diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc index af7c9d5..28de0fe 100644 --- a/chrome/browser/download/download_manager.cc +++ b/chrome/browser/download/download_manager.cc @@ -576,8 +576,8 @@ void DownloadManager::StartDownload(DownloadCreateInfo* info) { if (IsDangerous(info->suggested_path.BaseName())) info->is_dangerous = true; else if (IsExtensionInstall(info) && - !ExtensionsService::IsDownloadFromGallery(info->url, - info->referrer_url)) { + !Extension::IsDownloadFromGallery(info->url, + info->referrer_url)) { info->is_dangerous = true; } } diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc index 8eba54f..7af9133 100644 --- a/chrome/browser/extensions/extensions_service.cc +++ b/chrome/browser/extensions/extensions_service.cc @@ -81,26 +81,6 @@ class InstalledExtensionSet { const char* ExtensionsService::kInstallDirectoryName = "Extensions"; const char* ExtensionsService::kCurrentVersionFileName = "Current Version"; -// static -bool ExtensionsService::IsDownloadFromGallery(const GURL& download_url, - const GURL& referrer_url) { - if (StartsWithASCII(download_url.spec(), - extension_urls::kMiniGalleryDownloadPrefix, false) && - StartsWithASCII(referrer_url.spec(), - extension_urls::kMiniGalleryBrowsePrefix, false)) { - return true; - } - - if (StartsWithASCII(download_url.spec(), - extension_urls::kGalleryDownloadPrefix, false) && - StartsWithASCII(referrer_url.spec(), - extension_urls::kGalleryBrowsePrefix, false)) { - return true; - } - - return false; -} - ExtensionsService::ExtensionsService(Profile* profile, const CommandLine* command_line, PrefService* prefs, diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h index 5b0252d..79fdc10 100644 --- a/chrome/browser/extensions/extensions_service.h +++ b/chrome/browser/extensions/extensions_service.h @@ -71,11 +71,6 @@ class ExtensionsService // The name of the file that the current active version number is stored in. static const char* kCurrentVersionFileName; - // Determine if a given extension download should be treated as if it came - // from the gallery. - static bool IsDownloadFromGallery(const GURL& download_url, - const GURL& referrer_url); - ExtensionsService(Profile* profile, const CommandLine* command_line, PrefService* prefs, diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc index c7325ad..2ac3299 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.cc +++ b/chrome/browser/renderer_host/browser_render_process_host.cc @@ -362,6 +362,10 @@ void BrowserRenderProcessHost::ReceivedBadMessage(uint32 msg_type) { BadMessageTerminateProcess(msg_type, GetHandle()); } +void BrowserRenderProcessHost::PolicyViolated(const std::string& policy_name) { + PolicyViolationTerminateProcess(policy_name, GetHandle()); +} + void BrowserRenderProcessHost::ViewCreated() { visited_link_updater_->ReceiverReady(this); } @@ -800,6 +804,19 @@ void BrowserRenderProcessHost::BadMessageTerminateProcess( base::KillProcess(process, ResultCodes::KILLED_BAD_MESSAGE, false); } +// Static. This function can be called from any thread. +void BrowserRenderProcessHost::PolicyViolationTerminateProcess( + const std::string& policy_name, base::ProcessHandle process) { + LOG(ERROR) << "child process policy " << policy_name << " violated. " + << "terminating renderer."; + if (run_renderer_in_process()) { + // In single process mode it is better if we don't suicide but just crash. + CHECK(false); + } + NOTREACHED(); + base::KillProcess(process, ResultCodes::KILLED_POLICY_VIOLATION, false); +} + void BrowserRenderProcessHost::OnChannelError() { // Our child process has died. If we didn't expect it, it's a crash. // In any case, we need to let everyone know it's gone. diff --git a/chrome/browser/renderer_host/browser_render_process_host.h b/chrome/browser/renderer_host/browser_render_process_host.h index cd94bf5..89e4d82 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.h +++ b/chrome/browser/renderer_host/browser_render_process_host.h @@ -67,6 +67,7 @@ class BrowserRenderProcessHost : public RenderProcessHost, const base::TimeDelta& max_delay, IPC::Message* msg); virtual void ReceivedBadMessage(uint32 msg_type); + virtual void PolicyViolated(const std::string& policy_name); virtual void WidgetRestored(); virtual void WidgetHidden(); virtual void ViewCreated(); @@ -92,6 +93,13 @@ class BrowserRenderProcessHost : public RenderProcessHost, static void BadMessageTerminateProcess(uint32 msg_type, base::ProcessHandle renderer); + // Called to terminate a renderer which has violated the conditions of a + // security policy privilege. This function can be safely called from any + // thread. + static void PolicyViolationTerminateProcess(const std::string& policy_name, + base::ProcessHandle renderer); + + // NotificationObserver implementation. virtual void Observe(NotificationType type, const NotificationSource& source, diff --git a/chrome/browser/renderer_host/mock_render_process_host.cc b/chrome/browser/renderer_host/mock_render_process_host.cc index 8be0416..96df560 100644 --- a/chrome/browser/renderer_host/mock_render_process_host.cc +++ b/chrome/browser/renderer_host/mock_render_process_host.cc @@ -41,6 +41,10 @@ void MockRenderProcessHost::ReceivedBadMessage(uint32 msg_type) { ++bad_msg_count_; } +void MockRenderProcessHost::PolicyViolated(const std::string& policy_name) { + NOTIMPLEMENTED(); +} + void MockRenderProcessHost::WidgetRestored() { } diff --git a/chrome/browser/renderer_host/mock_render_process_host.h b/chrome/browser/renderer_host/mock_render_process_host.h index aa04651..1ee1905 100644 --- a/chrome/browser/renderer_host/mock_render_process_host.h +++ b/chrome/browser/renderer_host/mock_render_process_host.h @@ -39,6 +39,7 @@ class MockRenderProcessHost : public RenderProcessHost { const base::TimeDelta& max_delay, IPC::Message* msg); virtual void ReceivedBadMessage(uint32 msg_type); + virtual void PolicyViolated(const std::string& policy_name); virtual void WidgetRestored(); virtual void WidgetHidden(); virtual void ViewCreated(); diff --git a/chrome/browser/renderer_host/render_process_host.cc b/chrome/browser/renderer_host/render_process_host.cc index 5f9363e..7418a8b 100644 --- a/chrome/browser/renderer_host/render_process_host.cc +++ b/chrome/browser/renderer_host/render_process_host.cc @@ -59,11 +59,20 @@ static bool IsSuitableHost(RenderProcessHost* host, Profile* profile, if (host->profile() != profile) return false; + // We classify renderers according to their highest privilege, and try + // to group pages into renderers with similar privileges. + // Note: it may be possible for a renderer to have both DOMUI and EXTENSION + // privileges, in which case we call it an "extension" renderer. + // TYPE_EXTENSION_GALLERY should never be TYPE_DOMUI and/or TYPE_EXTENSION + // as well. RenderProcessHost::Type host_type = RenderProcessHost::TYPE_NORMAL; + if (ChildProcessSecurityPolicy::GetInstance()-> + CanInstallExtensionsSilently(host->id())) + host_type = RenderProcessHost::TYPE_EXTENSION_GALLERY; if (ChildProcessSecurityPolicy::GetInstance()->HasDOMUIBindings(host->id())) host_type = RenderProcessHost::TYPE_DOMUI; if (ChildProcessSecurityPolicy::GetInstance()-> - HasExtensionBindings(host->id())) + HasExtensionBindings(host->id())) host_type = RenderProcessHost::TYPE_EXTENSION; return host_type == type; diff --git a/chrome/browser/renderer_host/render_process_host.h b/chrome/browser/renderer_host/render_process_host.h index aad1f83..0949ffb 100644 --- a/chrome/browser/renderer_host/render_process_host.h +++ b/chrome/browser/renderer_host/render_process_host.h @@ -38,12 +38,16 @@ class RenderProcessHost : public IPC::Channel::Sender, // We classify renderers according to their highest privilege, and try // to group pages into renderers with similar privileges. - // Note: it may be possible for a renderer to have multiple privileges, - // in which case we call it an "extension" renderer. + // Note: it may be possible for a renderer to have both DOMUI and EXTENSION + // privileges, in which case we call it an "extension" renderer. + // TYPE_EXTENSION_GALLERY should never be TYPE_DOMUI and/or TYPE_EXTENSION + // as well. enum Type { - TYPE_NORMAL, // Normal renderer, no extra privileges. - TYPE_DOMUI, // Renderer with DOMUI privileges, like New Tab. - TYPE_EXTENSION, // Renderer with extension privileges. + TYPE_NORMAL, // Normal renderer, no extra privileges. + TYPE_EXTENSION_GALLERY, // Renderer with silent extension installation + // privileges. + TYPE_DOMUI, // Renderer with DOMUI privileges, like New Tab. + TYPE_EXTENSION // Renderer with extension privileges. }; // Details for RENDERER_PROCESS_CLOSED notifications. @@ -175,6 +179,9 @@ class RenderProcessHost : public IPC::Channel::Sender, // Called when a received message cannot be decoded. virtual void ReceivedBadMessage(uint32 msg_type) = 0; + // Called when a renderer security policy is violated. + virtual void PolicyViolated(const std::string& policy_name) = 0; + // Track the count of visible widgets. Called by listeners to register and // unregister visibility. virtual void WidgetRestored() = 0; diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc index b414aea..070e317 100644 --- a/chrome/browser/renderer_host/render_view_host.cc +++ b/chrome/browser/renderer_host/render_view_host.cc @@ -30,6 +30,7 @@ #include "chrome/browser/renderer_host/render_widget_host_view.h" #include "chrome/browser/renderer_host/site_instance.h" #include "chrome/common/bindings_policy.h" +#include "chrome/common/extensions/extension.h" #include "chrome/common/notification_details.h" #include "chrome/common/notification_service.h" #include "chrome/common/notification_type.h" @@ -966,6 +967,14 @@ void RenderViewHost::OnMsgNavigate(const IPC::Message& msg) { const int renderer_id = process()->id(); ChildProcessSecurityPolicy* policy = ChildProcessSecurityPolicy::GetInstance(); + // Gallery URLs are granted CanInstallExtensionsSilently. RenderView delegates + // navigations to & from gallery URLs to the browser process so it can swap + // processes and ensure that non-gallery URLs do not acquire this privilege. + // This is an extra-paranoid check on a privileged renderer navigating away. + if (policy->CanInstallExtensionsSilently(renderer_id) && + !Extension::IsGalleryURL(validated_params.url)) { + process()->PolicyViolated("can_silently_install_extensions"); + } // Without this check, an evil renderer can trick the browser into creating // a navigation entry for a banned URL. If the user clicks the back button // followed by the forward button (or clicks reload, or round-trips through diff --git a/chrome/browser/renderer_host/site_instance.cc b/chrome/browser/renderer_host/site_instance.cc index 337ce3d..115845f 100644 --- a/chrome/browser/renderer_host/site_instance.cc +++ b/chrome/browser/renderer_host/site_instance.cc @@ -7,6 +7,8 @@ #include "chrome/browser/browsing_instance.h" #include "chrome/browser/dom_ui/dom_ui_factory.h" #include "chrome/browser/renderer_host/browser_render_process_host.h" +#include "chrome/common/extensions/extension.h" +#include "chrome/common/extensions/extension_constants.h" #include "chrome/common/url_constants.h" #include "chrome/common/notification_service.h" #include "net/base/registry_controlled_domain.h" @@ -115,6 +117,16 @@ GURL SiteInstance::GetSiteForURL(const GURL& url) { // URLs with no host should have an empty site. GURL site; + // Hack warning: Extension gallery URLs are special-cased here. Extension + // gallery pages are granted special privileges to install extensions without + // a warning dialog. It is important that no other URLs share the render + // with these privileges. However, since the gallery is a sub-domain of + // google.com, the SiteInstance is specially treated to avoid having other + // google.com subdomains be grouped with it. Generalizing this kind of special + // case in the future seems desirable if we come upon another similar need. + if (Extension::IsGalleryURL(url)) + return GURL(extension_urls::kGalleryBrowsePrefix); + // TODO(creis): For many protocols, we should just treat the scheme as the // site, since there is no host. e.g., file:, about:, chrome: @@ -164,6 +176,10 @@ bool SiteInstance::IsSameWebSite(const GURL& url1, const GURL& url2) { if (url1.scheme() != url2.scheme()) return false; + // Hack. Special case Extension gallery URLs. See note in GetSiteForURL(). + if (Extension::IsGalleryURL(url1) || Extension::IsGalleryURL(url2)) + return Extension::IsGalleryURL(url1) && Extension::IsGalleryURL(url2); + return net::RegistryControlledDomainService::SameDomainOrHost(url1, url2); } @@ -179,6 +195,9 @@ RenderProcessHost::Type SiteInstance::GetRendererType() { if (DOMUIFactory::HasDOMUIScheme(site_)) return RenderProcessHost::TYPE_DOMUI; + if (Extension::IsGalleryURL(site_)) + return RenderProcessHost::TYPE_EXTENSION_GALLERY; + return RenderProcessHost::TYPE_NORMAL; } diff --git a/chrome/browser/renderer_host/test/test_render_view_host.cc b/chrome/browser/renderer_host/test/test_render_view_host.cc index e9e848f..85de55a 100644 --- a/chrome/browser/renderer_host/test/test_render_view_host.cc +++ b/chrome/browser/renderer_host/test/test_render_view_host.cc @@ -15,15 +15,11 @@ TestRenderViewHost::TestRenderViewHost(SiteInstance* instance, RenderViewHostDelegate* delegate, int routing_id) : RenderViewHost(instance, delegate, routing_id), - render_view_created_(false), - delete_counter_(NULL) { + render_view_created_(false) { set_view(new TestRenderWidgetHostView(this)); } TestRenderViewHost::~TestRenderViewHost() { - if (delete_counter_) - ++*delete_counter_; - // Since this isn't a traditional view, we have to delete it. delete view(); } diff --git a/chrome/browser/renderer_host/test/test_render_view_host.h b/chrome/browser/renderer_host/test/test_render_view_host.h index b1458a1..9d3b4f4 100644 --- a/chrome/browser/renderer_host/test/test_render_view_host.h +++ b/chrome/browser/renderer_host/test/test_render_view_host.h @@ -117,11 +117,6 @@ class TestRenderViewHost : public RenderViewHost { // This is a helper function for simulating the most common types of loads. void SendNavigate(int page_id, const GURL& url); - // If set, *delete_counter is incremented when this object destructs. - void set_delete_counter(int* delete_counter) { - delete_counter_ = delete_counter; - } - // Sets whether the RenderView currently exists or not. This controls the // return value from IsRenderViewLive, which the rest of the system uses to // check whether the RenderView has crashed or not. @@ -141,9 +136,6 @@ class TestRenderViewHost : public RenderViewHost { // respond to IsRenderViewLive appropriately. bool render_view_created_; - // See set_delete_counter() above. May be NULL. - int* delete_counter_; - DISALLOW_COPY_AND_ASSIGN(TestRenderViewHost); }; diff --git a/chrome/browser/tab_contents/render_view_host_manager.cc b/chrome/browser/tab_contents/render_view_host_manager.cc index 8622f39..f44ac31 100644 --- a/chrome/browser/tab_contents/render_view_host_manager.cc +++ b/chrome/browser/tab_contents/render_view_host_manager.cc @@ -16,6 +16,7 @@ #include "chrome/browser/tab_contents/navigation_controller.h" #include "chrome/browser/tab_contents/navigation_entry.h" #include "chrome/common/chrome_switches.h" +#include "chrome/common/extensions/extension.h" #include "chrome/common/notification_service.h" #include "chrome/common/notification_type.h" #include "chrome/common/render_messages.h" @@ -301,6 +302,12 @@ bool RenderViewHostManager::ShouldSwapProcessesForNavigation( return true; } + // Extension gallery pages are granted special install privileges for + // extensions. Thus, gallery pages must be contained in a separate process. + if (Extension::IsGalleryURL(cur_entry->url()) != + Extension::IsGalleryURL(new_entry->url())) + return true; + return false; } diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index 964bff1..9a0df89 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -18,6 +18,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/cert_store.h" #include "chrome/browser/character_encoding.h" +#include "chrome/browser/child_process_security_policy.h" #include "chrome/browser/debugger/devtools_manager.h" #include "chrome/browser/dom_operation_notification_details.h" #include "chrome/browser/dom_ui/dom_ui.h" @@ -55,6 +56,7 @@ #include "chrome/browser/thumbnail_store.h" #include "chrome/browser/search_engines/template_url_fetcher.h" #include "chrome/browser/search_engines/template_url_model.h" +#include "chrome/common/bindings_policy.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/extensions/extension_action.h" #include "chrome/common/notification_service.h" @@ -2518,6 +2520,15 @@ bool TabContents::CreateRenderViewForRenderManager( render_view_host->AllowBindings( render_manager_.pending_dom_ui()->bindings()); + // If the pending url is to an extensions gallery page, enable the silent + // install bindings privilege. It is important to enforce that this renderer + // never get used for non-gallery URLs. + if (!process()->run_renderer_in_process() && + !process()->HasConnection() && + Extension::IsGalleryURL(controller_.pending_entry()->url())) + ChildProcessSecurityPolicy::GetInstance()->GrantInstallExtensionsSilently( + render_view_host->process()->id()); + RenderWidgetHostView* rwh_view = view_->CreateViewForWidget(render_view_host); scoped_refptr<URLRequestContextGetter> request_context = request_context_; diff --git a/chrome/browser/tab_contents/web_contents_unittest.cc b/chrome/browser/tab_contents/web_contents_unittest.cc index 9b59406..e4cb6533 100644 --- a/chrome/browser/tab_contents/web_contents_unittest.cc +++ b/chrome/browser/tab_contents/web_contents_unittest.cc @@ -13,6 +13,10 @@ #include "chrome/browser/tab_contents/navigation_entry.h" #include "chrome/browser/tab_contents/test_tab_contents.h" #include "chrome/common/chrome_paths.h" +#include "chrome/common/extensions/extension.h" +#include "chrome/common/extensions/extension_constants.h" +#include "chrome/common/notification_service.h" +#include "chrome/common/notification_type.h" #include "chrome/common/pref_service.h" #include "chrome/common/render_messages.h" #include "chrome/common/url_constants.h" @@ -208,6 +212,38 @@ class TabContentsTest : public RenderViewHostTestHarness { ChromeThread ui_thread_; }; +// Used to block until a navigation completes. +class RenderViewHostDeletedObserver : public NotificationObserver { + public: + RenderViewHostDeletedObserver(TabContents* contents, + RenderViewHost* render_view_host) + : render_view_host_(render_view_host), + deleted_(false) { + registrar_.Add(this, NotificationType::RENDER_VIEW_HOST_DELETED, + Source<TabContents>(contents)); + } + + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + if (type == NotificationType::RENDER_VIEW_HOST_DELETED && + render_view_host_ == Details<RenderViewHost>(details).ptr()) { + deleted_ = true; + } + } + + bool deleted() { return deleted_; } + + private: + NotificationRegistrar registrar_; + + RenderViewHost* render_view_host_; + + bool deleted_; + + DISALLOW_COPY_AND_ASSIGN(RenderViewHostDeletedObserver); +}; + // Test to make sure that title updates get stripped of whitespace. TEST_F(TabContentsTest, UpdateTitle) { ViewHostMsg_FrameNavigate_Params params; @@ -277,8 +313,7 @@ TEST_F(TabContentsTest, SimpleNavigation) { TEST_F(TabContentsTest, CrossSiteBoundaries) { contents()->transition_cross_site = true; TestRenderViewHost* orig_rvh = rvh(); - int orig_rvh_delete_count = 0; - orig_rvh->set_delete_counter(&orig_rvh_delete_count); + RenderViewHostDeletedObserver orig_rvh_deleted_observer(contents(), orig_rvh); SiteInstance* instance1 = contents()->GetSiteInstance(); // Navigate to URL. First URL should use first RenderViewHost. @@ -296,8 +331,8 @@ TEST_F(TabContentsTest, CrossSiteBoundaries) { controller().LoadURL(url2, GURL(), PageTransition::TYPED); EXPECT_TRUE(contents()->cross_navigation_pending()); TestRenderViewHost* pending_rvh = contents()->pending_rvh(); - int pending_rvh_delete_count = 0; - pending_rvh->set_delete_counter(&pending_rvh_delete_count); + RenderViewHostDeletedObserver pending_rvh_deleted_observer(contents(), + pending_rvh); // DidNavigate from the pending page ViewHostMsg_FrameNavigate_Params params2; @@ -309,7 +344,7 @@ TEST_F(TabContentsTest, CrossSiteBoundaries) { EXPECT_EQ(pending_rvh, contents()->render_view_host()); EXPECT_NE(instance1, instance2); EXPECT_TRUE(contents()->pending_rvh() == NULL); - EXPECT_EQ(orig_rvh_delete_count, 1); + EXPECT_TRUE(orig_rvh_deleted_observer.deleted()); // Going back should switch SiteInstances again. The first SiteInstance is // stored in the NavigationEntry, so it should be the same as at the start. @@ -321,17 +356,74 @@ TEST_F(TabContentsTest, CrossSiteBoundaries) { contents()->TestDidNavigate(goback_rvh, params1); EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(goback_rvh, contents()->render_view_host()); - EXPECT_EQ(pending_rvh_delete_count, 1); + EXPECT_TRUE(pending_rvh_deleted_observer.deleted()); EXPECT_EQ(instance1, contents()->GetSiteInstance()); } +// Test that extension gallery URLs are isolated into their own SiteInstance +// (distinct, in particular, from other other *.google.com domains). +TEST_F(TabContentsTest, IsolateGalleryURLs) { + contents()->transition_cross_site = true; + TestRenderViewHost* orig_rvh = rvh(); + RenderViewHostDeletedObserver orig_rvh_deleted_observer(contents(), orig_rvh); + SiteInstance* instance1 = contents()->GetSiteInstance(); + + // Navigate to first non-gallery URL. + const GURL url("https://calendar.google.com"); + controller().LoadURL(url, GURL(), PageTransition::TYPED); + ViewHostMsg_FrameNavigate_Params params1; + InitNavigateParams(¶ms1, 1, url); + contents()->TestDidNavigate(orig_rvh, params1); + + EXPECT_FALSE(contents()->cross_navigation_pending()); + EXPECT_EQ(orig_rvh, contents()->render_view_host()); + + // Navigate to gallery browser url. + const GURL url2(extension_urls::kGalleryBrowsePrefix); + controller().LoadURL(url2, GURL(), PageTransition::TYPED); + EXPECT_TRUE(contents()->cross_navigation_pending()); + TestRenderViewHost* pending_rvh = contents()->pending_rvh(); + RenderViewHostDeletedObserver pending_rvh_deleted_observer(contents(), + pending_rvh); + + ViewHostMsg_FrameNavigate_Params params2; + InitNavigateParams(¶ms2, 1, url2); + contents()->TestDidNavigate(pending_rvh, params2); + SiteInstance* instance2 = contents()->GetSiteInstance(); + + EXPECT_FALSE(contents()->cross_navigation_pending()); + EXPECT_EQ(pending_rvh, contents()->render_view_host()); + EXPECT_NE(instance1, instance2); + EXPECT_TRUE(contents()->pending_rvh() == NULL); + EXPECT_TRUE(orig_rvh_deleted_observer.deleted()); + + // Navigate again to another non-gallery google url + const GURL url3("https://mail.google.com"); + controller().LoadURL(url3, GURL(), PageTransition::TYPED); + EXPECT_TRUE(contents()->cross_navigation_pending()); + TestRenderViewHost* pending_rvh2 = contents()->pending_rvh(); + + ViewHostMsg_FrameNavigate_Params params3; + InitNavigateParams(¶ms3, 1, url3); + contents()->TestDidNavigate(pending_rvh2, params3); + SiteInstance* instance3 = contents()->GetSiteInstance(); + + EXPECT_FALSE(contents()->cross_navigation_pending()); + EXPECT_EQ(pending_rvh2, contents()->render_view_host()); + EXPECT_NE(instance2, instance3); + // TODO(creis): This should actually be EXPECT_EQ. see crbug.com/29146. + // e.g. This first and third instances *should* be the same. + EXPECT_NE(instance1, instance3); + EXPECT_TRUE(contents()->pending_rvh() == NULL); + EXPECT_TRUE(pending_rvh_deleted_observer.deleted()); +} + // Test that navigating across a site boundary after a crash creates a new // RVH without requiring a cross-site transition (i.e., PENDING state). TEST_F(TabContentsTest, CrossSiteBoundariesAfterCrash) { contents()->transition_cross_site = true; TestRenderViewHost* orig_rvh = rvh(); - int orig_rvh_delete_count = 0; - orig_rvh->set_delete_counter(&orig_rvh_delete_count); + RenderViewHostDeletedObserver orig_rvh_deleted_observer(contents(), orig_rvh); SiteInstance* instance1 = contents()->GetSiteInstance(); // Navigate to URL. First URL should use first RenderViewHost. @@ -354,7 +446,7 @@ TEST_F(TabContentsTest, CrossSiteBoundariesAfterCrash) { EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_TRUE(contents()->pending_rvh() == NULL); EXPECT_NE(orig_rvh, new_rvh); - EXPECT_EQ(orig_rvh_delete_count, 1); + EXPECT_TRUE(orig_rvh_deleted_observer.deleted()); // DidNavigate from the new page ViewHostMsg_FrameNavigate_Params params2; diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc index 94d6de2..04ae051c 100644 --- a/chrome/common/extensions/extension.cc +++ b/chrome/common/extensions/extension.cc @@ -128,6 +128,32 @@ const std::string Extension::VersionString() const { } // static +bool Extension::IsGalleryURL(const GURL& url) { + return StartsWithASCII(url.spec(), + std::string(extension_urls::kGalleryBrowsePrefix), true); +} + +// static +bool Extension::IsDownloadFromGallery(const GURL& download_url, + const GURL& referrer_url) { + if (StartsWithASCII(download_url.spec(), + extension_urls::kMiniGalleryDownloadPrefix, false) && + StartsWithASCII(referrer_url.spec(), + extension_urls::kMiniGalleryBrowsePrefix, false)) { + return true; + } + + if (StartsWithASCII(download_url.spec(), + extension_urls::kGalleryDownloadPrefix, false) && + StartsWithASCII(referrer_url.spec(), + extension_urls::kGalleryBrowsePrefix, false)) { + return true; + } + + return false; +} + +// static bool Extension::IsExtension(const FilePath& file_name) { return file_name.MatchesExtension( FilePath::StringType(FILE_PATH_LITERAL(".")) + diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h index 331de16..28bffc1 100644 --- a/chrome/common/extensions/extension.h +++ b/chrome/common/extensions/extension.h @@ -122,6 +122,14 @@ class Extension { explicit Extension(const FilePath& path); virtual ~Extension(); + // Determine if a given |url| is a gallery browse URL. + static bool IsGalleryURL(const GURL& url); + + // Determine if a given extension download should be treated as if it came + // from the theme gallery. + static bool IsDownloadFromGallery(const GURL& download_url, + const GURL& referrer_url); + // Checks to see if the extension has a valid ID. static bool IdIsValid(const std::string& id); diff --git a/chrome/common/result_codes.h b/chrome/common/result_codes.h index abaf5a7..fe148d9 100644 --- a/chrome/common/result_codes.h +++ b/chrome/common/result_codes.h @@ -39,6 +39,8 @@ class ResultCodes { UNINSTALL_DELETE_PROFILE, // Delete profile as well during uninstall. UNSUPPORTED_PARAM, // Command line parameter is not supported. KILLED_BAD_MESSAGE, // A bad message caused the process termination. + KILLED_POLICY_VIOLATION, // A renderer violated the contract of a policy + // permission that it had been granted. IMPORTER_CANCEL, // The user canceled the browser import. IMPORTER_HUNG, // Browser import hung and was killed. RESPAWN_FAILED, // Trying to restart the browser we crashed. diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc index c47cd25..e368552 100644 --- a/chrome/renderer/render_view.cc +++ b/chrome/renderer/render_view.cc @@ -30,6 +30,7 @@ #include "chrome/common/child_process_logging.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_constants.h" +#include "chrome/common/extensions/extension.h" #include "chrome/common/jstemplate_builder.h" #include "chrome/common/page_zoom.h" #include "chrome/common/plugin_messages.h" @@ -1957,6 +1958,30 @@ WebNavigationPolicy RenderView::decidePolicyForNavigation( } } + // Extension gallery URLs are granted special permission to silently install + // extensions. If the navigation is either from or to a gallery URL, kick + // it up to browser so that the renderer process can be properly managed + // (i.e. display gallery urls in a seperate process that contains nothing + // else). + if (default_policy == WebKit::WebNavigationPolicyCurrentTab && + (is_content_initiated || is_redirect) && frame->parent() == NULL && + Extension::IsGalleryURL(url) != Extension::IsGalleryURL(frame->url())) { + // TODO(rafaelw): is it OK to use frame->url() as referrer rather than + // GURL() (as above)? + OpenURL(url, frame->url(), default_policy); + return WebKit::WebNavigationPolicyIgnore; // Suppress the load here. + } + + // The renderer for the extension gallery should not allow any non-gallery + // subframe navigations, since the frames would also have elevated + // permissions. + if (default_policy == WebKit::WebNavigationPolicyCurrentTab && + frame->parent() != NULL && + Extension::IsGalleryURL(frame->top()->url()) && + !Extension::IsGalleryURL(url)) { + return WebKit::WebNavigationPolicyIgnore; // Ignore the navigation. + } + // Detect when a page is "forking" a new tab that can be safely rendered in // its own process. This is done by sites like Gmail that try to open links // in new windows without script connections back to the original page. We |