From cf3c677de2fd4ba585237fcead536e5a9ab5b327 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Tue, 8 Jan 2019 14:39:20 -0500 Subject: [PATCH] Merge pull request #1240 from brave/referrer_block Issue 2252: Refactor the referrer hiding. --- browser/brave_content_browser_client.cc | 33 ++++++++++ browser/brave_content_browser_client.h | 9 +++ browser/net/brave_network_delegate_base.cc | 3 +- ...rave_site_hacks_network_delegate_helper.cc | 18 ++--- ...brave_site_hacks_network_delegate_helper.h | 2 - ..._hacks_network_delegate_helper_unittest.cc | 4 +- browser/net/url_context.h | 1 - .../browser/brave_shields_util.cc | 65 ++++++++++++++----- .../browser/brave_shields_util.h | 7 ++ ...ser-frame_host-navigation_request.cc.patch | 16 +++++ ...lic-browser-content_browser_client.h.patch | 18 +++++ 11 files changed, 141 insertions(+), 35 deletions(-) create mode 100644 patches/content-browser-frame_host-navigation_request.cc.patch create mode 100644 patches/content-public-browser-content_browser_client.h.patch diff --git a/browser/brave_content_browser_client.cc b/browser/brave_content_browser_client.cc index 8dc75f0e3576..12e710ece615 100644 --- a/browser/brave_content_browser_client.cc +++ b/browser/brave_content_browser_client.cc @@ -28,6 +28,7 @@ #include "chrome/common/url_constants.h" #include "components/content_settings/core/browser/cookie_settings.h" #include "content/browser/frame_host/render_frame_host_impl.h" +#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_url_handler.h" @@ -211,6 +212,38 @@ void BraveContentBrowserClient::AdjustUtilityServiceProcessCommandLine( } } +void BraveContentBrowserClient::MaybeHideReferrer( + content::BrowserContext* browser_context, + const GURL& request_url, + const GURL& document_url, + content::Referrer* referrer) { + DCHECK(referrer); + if (document_url.SchemeIs(kChromeExtensionScheme)) { + return; + } + + Profile* profile = Profile::FromBrowserContext(browser_context); + const bool allow_referrers = + brave_shields::IsAllowContentSettingsForProfile( + profile, + document_url, + document_url, + CONTENT_SETTINGS_TYPE_PLUGINS, + brave_shields::kReferrers); + const bool shields_up = + brave_shields::IsAllowContentSettingsForProfile( + profile, + document_url, + GURL(), + CONTENT_SETTINGS_TYPE_PLUGINS, + brave_shields::kBraveShields); + brave_shields::ShouldSetReferrer(allow_referrers, shields_up, + referrer->url, document_url, request_url, + request_url.GetOrigin(), + referrer->policy, + referrer); +} + GURL BraveContentBrowserClient::GetEffectiveURL( content::BrowserContext* browser_context, const GURL& url) { diff --git a/browser/brave_content_browser_client.h b/browser/brave_content_browser_client.h index bb4f2f8bda0c..3a91c96922ba 100644 --- a/browser/brave_content_browser_client.h +++ b/browser/brave_content_browser_client.h @@ -10,6 +10,10 @@ #include +namespace content { +class BrowserContext; +} + class BraveContentBrowserClient : public ChromeContentBrowserClient { public: BraveContentBrowserClient(ChromeFeatureListCreator* chrome_feature_list_creator = nullptr); @@ -55,6 +59,11 @@ class BraveContentBrowserClient : public ChromeContentBrowserClient { const service_manager::Identity& identity, base::CommandLine* command_line) override; + void MaybeHideReferrer(content::BrowserContext* browser_context, + const GURL& request_url, + const GURL& document_url, + content::Referrer* referrer) override; + GURL GetEffectiveURL(content::BrowserContext* browser_context, const GURL& url) override; diff --git a/browser/net/brave_network_delegate_base.cc b/browser/net/brave_network_delegate_base.cc index fa90815d757e..e9d478969d8f 100644 --- a/browser/net/brave_network_delegate_base.cc +++ b/browser/net/brave_network_delegate_base.cc @@ -284,8 +284,7 @@ void BraveNetworkDelegateBase::RunNextCallback( if (ctx->event_type == brave::kOnBeforeRequest) { if (!ctx->new_url_spec.empty() && - (ctx->new_url_spec != ctx->request_url.spec() || - ctx->referrer_changed) && + (ctx->new_url_spec != ctx->request_url.spec()) && IsRequestIdentifierValid(ctx->request_identifier)) { *ctx->new_url = GURL(ctx->new_url_spec); } diff --git a/browser/net/brave_site_hacks_network_delegate_helper.cc b/browser/net/brave_site_hacks_network_delegate_helper.cc index fb59bc399f8b..d7583f117b00 100644 --- a/browser/net/brave_site_hacks_network_delegate_helper.cc +++ b/browser/net/brave_site_hacks_network_delegate_helper.cc @@ -23,12 +23,15 @@ using content::BrowserThread; using content::Referrer; +namespace brave { + namespace { -bool ApplyPotentialReferrerBlock(net::URLRequest* request) { +bool ApplyPotentialReferrerBlock(std::shared_ptr ctx, + net::URLRequest* request) { DCHECK_CURRENTLY_ON(BrowserThread::IO); - GURL target_origin = GURL(request->url()).GetOrigin(); - GURL tab_origin = request->site_for_cookies().GetOrigin(); + GURL target_origin = request->url().GetOrigin(); + GURL tab_origin = ctx->tab_origin; if (tab_origin.SchemeIs(kChromeExtensionScheme)) { return false; } @@ -52,17 +55,10 @@ bool ApplyPotentialReferrerBlock(net::URLRequest* request) { } // namespace -namespace brave { - int OnBeforeURLRequest_SiteHacksWork( const ResponseCallback& next_callback, std::shared_ptr ctx) { - - if (ApplyPotentialReferrerBlock(const_cast(ctx->request))) { - ctx->new_url_spec = ctx->request_url.spec(); - ctx->referrer_changed = true; - } - + ApplyPotentialReferrerBlock(ctx, const_cast(ctx->request)); return net::OK; } diff --git a/browser/net/brave_site_hacks_network_delegate_helper.h b/browser/net/brave_site_hacks_network_delegate_helper.h index ceb9f5f18308..a94d6b1eef40 100644 --- a/browser/net/brave_site_hacks_network_delegate_helper.h +++ b/browser/net/brave_site_hacks_network_delegate_helper.h @@ -7,8 +7,6 @@ #include "brave/browser/net/url_context.h" -struct BraveRequestInfo; - namespace net { class URLRequest; } diff --git a/browser/net/brave_site_hacks_network_delegate_helper_unittest.cc b/browser/net/brave_site_hacks_network_delegate_helper_unittest.cc index ceb9695faa98..cc2b140cf561 100644 --- a/browser/net/brave_site_hacks_network_delegate_helper_unittest.cc +++ b/browser/net/brave_site_hacks_network_delegate_helper_unittest.cc @@ -265,8 +265,8 @@ TEST_F(BraveSiteHacksNetworkDelegateHelperTest, ReferrerCleared) { brave::ResponseCallback callback; int ret = brave::OnBeforeURLRequest_SiteHacksWork(callback, brave_request_info); EXPECT_EQ(ret, net::OK); - // new_url_spec must be set to trigger an internal redirect - EXPECT_STREQ(brave_request_info->new_url_spec.c_str(), url.spec().c_str()); + // new_url should not be set + EXPECT_TRUE(brave_request_info->new_url_spec.empty()); EXPECT_STREQ(request->referrer().c_str(), url.GetOrigin().spec().c_str()); }); } diff --git a/browser/net/url_context.h b/browser/net/url_context.h index ebee381a407f..68894a7afdc4 100644 --- a/browser/net/url_context.h +++ b/browser/net/url_context.h @@ -58,7 +58,6 @@ struct BraveRequestInfo { bool allow_http_upgradable_resource = false; bool allow_1p_cookies = true; bool allow_3p_cookies = false; - bool referrer_changed = false; int render_process_id = 0; int render_frame_id = 0; int frame_tree_node_id = 0; diff --git a/components/brave_shields/browser/brave_shields_util.cc b/components/brave_shields/browser/brave_shields_util.cc index 4fd315d4bcfd..6ffb1260687c 100644 --- a/components/brave_shields/browser/brave_shields_util.cc +++ b/components/brave_shields/browser/brave_shields_util.cc @@ -8,6 +8,7 @@ #include "brave/common/shield_exceptions.h" #include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h" #include "brave/components/brave_shields/common/brave_shield_constants.h" +#include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/extensions/extension_tab_util.h" #include "chrome/browser/profiles/profile_io_data.h" #include "components/content_settings/core/browser/host_content_settings_map.h" @@ -30,6 +31,8 @@ using namespace net::registry_controlled_domains; namespace brave_shields { +namespace { + bool GetDefaultFromResourceIdentifier(const std::string& resource_identifier, const GURL& primary_url, const GURL& secondary_url) { if (resource_identifier == brave_shields::kAds) { @@ -48,6 +51,31 @@ bool GetDefaultFromResourceIdentifier(const std::string& resource_identifier, return false; } +bool IsAllowContentSetting(HostContentSettingsMap* content_settings, + const GURL& primary_url, + const GURL& secondary_url, + ContentSettingsType setting_type, + const std::string& resource_identifier) { + DCHECK(content_settings); + content_settings::SettingInfo setting_info; + std::unique_ptr value = + content_settings->GetWebsiteSetting( + primary_url, secondary_url, setting_type, resource_identifier, + &setting_info); + ContentSetting setting = + content_settings::ValueToContentSetting(value.get()); + + // TODO(bbondy): Add a static RegisterUserPrefs method for shields and use + // prefs instead of simply returning true / false below. + if (setting == CONTENT_SETTING_DEFAULT) { + return GetDefaultFromResourceIdentifier(resource_identifier, primary_url, + secondary_url); + } + return setting == CONTENT_SETTING_ALLOW; +} + +} // namespace + bool IsAllowContentSettingFromIO(const net::URLRequest* request, const GURL& primary_url, const GURL& secondary_url, ContentSettingsType setting_type, @@ -66,31 +94,34 @@ bool IsAllowContentSettingFromIO(const net::URLRequest* request, secondary_url, setting_type, resource_identifier); } +bool IsAllowContentSettingsForProfile(Profile* profile, + const GURL& primary_url, + const GURL& secondary_url, + ContentSettingsType setting_type, + const std::string& resource_identifier) { + DCHECK_CURRENTLY_ON(BrowserThread::UI); + DCHECK(profile); + return IsAllowContentSetting( + HostContentSettingsMapFactory::GetForProfile(profile), + primary_url, + secondary_url, + setting_type, + resource_identifier); +} + bool IsAllowContentSettingWithIOData(ProfileIOData* io_data, const GURL& primary_url, const GURL& secondary_url, ContentSettingsType setting_type, const std::string& resource_identifier) { - if (!io_data) { return GetDefaultFromResourceIdentifier(resource_identifier, primary_url, secondary_url); } - content_settings::SettingInfo setting_info; - std::unique_ptr value = - io_data->GetHostContentSettingsMap()->GetWebsiteSetting( - primary_url, secondary_url, - setting_type, - resource_identifier, &setting_info); - ContentSetting setting = - content_settings::ValueToContentSetting(value.get()); - - // TODO(bbondy): Add a static RegisterUserPrefs method for shields and use - // prefs instead of simply returning true / false below. - if (setting == CONTENT_SETTING_DEFAULT) { - return GetDefaultFromResourceIdentifier(resource_identifier, primary_url, - secondary_url); - } - return setting == CONTENT_SETTING_ALLOW; + return IsAllowContentSetting(io_data->GetHostContentSettingsMap(), + primary_url, + secondary_url, + setting_type, + resource_identifier); } void GetRenderFrameInfo(const URLRequest* request, diff --git a/components/brave_shields/browser/brave_shields_util.h b/components/brave_shields/browser/brave_shields_util.h index db9b92153b64..42c8eb99cf2e 100644 --- a/components/brave_shields/browser/brave_shields_util.h +++ b/components/brave_shields/browser/brave_shields_util.h @@ -20,6 +20,7 @@ struct Referrer; } class GURL; +class Profile; class ProfileIOData; namespace brave_shields { @@ -29,6 +30,12 @@ bool IsAllowContentSettingWithIOData(ProfileIOData* io_data, ContentSettingsType setting_type, const std::string& resource_identifier); +bool IsAllowContentSettingsForProfile(Profile* profile, + const GURL& primary_url, + const GURL& secondary_url, + ContentSettingsType setting_type, + const std::string& resource_identifier); + bool IsAllowContentSettingFromIO(const net::URLRequest* request, const GURL& primary_url, const GURL& secondary_url, ContentSettingsType setting_type, diff --git a/patches/content-browser-frame_host-navigation_request.cc.patch b/patches/content-browser-frame_host-navigation_request.cc.patch new file mode 100644 index 000000000000..8de7dfc7e588 --- /dev/null +++ b/patches/content-browser-frame_host-navigation_request.cc.patch @@ -0,0 +1,16 @@ +diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc +index 3687817b179bdc1966d136d497bef2196dcc881e..9adb66951a8ce5769b6ecdfe5625671975ceff20 100644 +--- a/content/browser/frame_host/navigation_request.cc ++++ b/content/browser/frame_host/navigation_request.cc +@@ -1405,6 +1405,11 @@ void NavigationRequest::OnStartChecksComplete( + frame_tree_node_, begin_params_.get(), &report_raw_headers); + RenderFrameDevToolsAgentHost::OnNavigationRequestWillBeSent(*this); + ++ GetContentClient()->browser()->MaybeHideReferrer(browser_context, ++ common_params_.url, ++ top_document_url, ++ &common_params_.referrer); ++ + loader_ = NavigationURLLoader::Create( + browser_context->GetResourceContext(), partition, + std::make_unique( diff --git a/patches/content-public-browser-content_browser_client.h.patch b/patches/content-public-browser-content_browser_client.h.patch new file mode 100644 index 000000000000..21dfde61c3a9 --- /dev/null +++ b/patches/content-public-browser-content_browser_client.h.patch @@ -0,0 +1,18 @@ +diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h +index b33545b865e4677ee2e9a909d259df171c12d985..a5d8d152dbeaf53d452c66129b47453e4ae2d20a 100644 +--- a/content/public/browser/content_browser_client.h ++++ b/content/public/browser/content_browser_client.h +@@ -1364,6 +1364,13 @@ class CONTENT_EXPORT ContentBrowserClient { + // perform additional checks, such as requiring --user-data-dir flag too to + // make sure that insecure contents will not persist accidentally. + virtual bool CanIgnoreCertificateErrorIfNeeded(); ++ ++ // Brave-speicific: allows the embedder to modify the referrer string ++ // according to user preferences. ++ virtual void MaybeHideReferrer(BrowserContext* browser_context, ++ const GURL& request_url, ++ const GURL& document_url, ++ Referrer* referrer) {} + }; + + } // namespace content