diff --git a/browser/brave_content_browser_client.cc b/browser/brave_content_browser_client.cc index 86e10a979f7a..8996f0c5f77f 100644 --- a/browser/brave_content_browser_client.cc +++ b/browser/brave_content_browser_client.cc @@ -274,6 +274,7 @@ void BraveContentBrowserClient::MaybeHideReferrer( content::BrowserContext* browser_context, const GURL& request_url, const GURL& document_url, + bool is_main_frame, content::Referrer* referrer) { DCHECK(referrer); if (document_url.SchemeIs(kChromeExtensionScheme)) { @@ -295,12 +296,19 @@ void BraveContentBrowserClient::MaybeHideReferrer( GURL(), CONTENT_SETTINGS_TYPE_PLUGINS, brave_shields::kBraveShields); + + // Top-level navigations get empty referrers (brave/brave-browser#3422). + GURL replacement_referrer_url; + if (!is_main_frame) { + // But iframe navigations get spoofed instead (brave/brave-browser#3988). + replacement_referrer_url = request_url.GetOrigin(); + } brave_shields::ShouldSetReferrer(allow_referrers, shields_up, referrer->url, document_url, request_url, - request_url.GetOrigin(), + replacement_referrer_url, referrer->policy, referrer); } diff --git a/browser/brave_content_browser_client.h b/browser/brave_content_browser_client.h index 685dd99b7237..3514f92658d4 100644 --- a/browser/brave_content_browser_client.h +++ b/browser/brave_content_browser_client.h @@ -73,6 +73,7 @@ class BraveContentBrowserClient : public ChromeContentBrowserClient { void MaybeHideReferrer(content::BrowserContext* browser_context, const GURL& request_url, const GURL& document_url, + bool is_main_frame, content::Referrer* referrer) override; GURL GetEffectiveURL(content::BrowserContext* browser_context, diff --git a/browser/brave_content_browser_client_browsertest.cc b/browser/brave_content_browser_client_browsertest.cc index f4b551478811..d29f4076eedc 100644 --- a/browser/brave_content_browser_client_browsertest.cc +++ b/browser/brave_content_browser_client_browsertest.cc @@ -474,22 +474,45 @@ IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientReferrerTest, DefaultBehaviour) { const GURL kRequestUrl("http://request.com/path?query"); const GURL kDocumentUrl("http://document.com/path?query"); + const GURL kSameOriginRequestUrl("http://document.com/different/path"); content::Referrer kReferrer(kDocumentUrl, network::mojom::ReferrerPolicy::kDefault); - // Should be hidden by default. + // Cross-origin navigations don't get a referrer. content::Referrer referrer = kReferrer; client()->MaybeHideReferrer(browser()->profile(), - kRequestUrl, kDocumentUrl, + kRequestUrl, kDocumentUrl, true, + &referrer); + EXPECT_EQ(referrer.url, GURL()); + + // Same-origin navigations get full referrers. + referrer = kReferrer; + client()->MaybeHideReferrer(browser()->profile(), + kSameOriginRequestUrl, kDocumentUrl, true, + &referrer); + EXPECT_EQ(referrer.url, kDocumentUrl); + + // Cross-origin iframe navigations get a spoofed referrer. + referrer = kReferrer; + client()->MaybeHideReferrer(browser()->profile(), + kRequestUrl, kDocumentUrl, false, &referrer); EXPECT_EQ(referrer.url, kRequestUrl.GetOrigin()); + // Same-origin iframe navigations get full referrers. + referrer = kReferrer; + client()->MaybeHideReferrer(browser()->profile(), + kSameOriginRequestUrl, kDocumentUrl, false, + &referrer); + EXPECT_EQ(referrer.url, kDocumentUrl); + // Special rule for extensions. const GURL kExtensionUrl("chrome-extension://abc/path?query"); + referrer = kReferrer; referrer.url = kExtensionUrl; client()->MaybeHideReferrer(browser()->profile(), - kRequestUrl, kExtensionUrl, + kRequestUrl, kExtensionUrl, true, &referrer); EXPECT_EQ(referrer.url, kExtensionUrl); @@ -501,7 +524,7 @@ IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientReferrerTest, brave_shields::kReferrers, CONTENT_SETTING_ALLOW); referrer = kReferrer; client()->MaybeHideReferrer(browser()->profile(), - kRequestUrl, kDocumentUrl, + kRequestUrl, kDocumentUrl, true, &referrer); EXPECT_EQ(referrer.url, kDocumentUrl); } diff --git a/patches/content-browser-frame_host-navigation_request.cc.patch b/patches/content-browser-frame_host-navigation_request.cc.patch index 70e3988b78e7..19c58b89108e 100644 --- a/patches/content-browser-frame_host-navigation_request.cc.patch +++ b/patches/content-browser-frame_host-navigation_request.cc.patch @@ -1,14 +1,15 @@ diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc -index fef9c1c52e03646efc5f9cfb1e5acf2d662e2e22..3dcd99a6fcbb3e0885cf16d345eef96ff639db37 100644 +index fef9c1c52e03646efc5f9cfb1e5acf2d662e2e22..0b24253a3979511e4ce036522ed1aa3b98a4ec36 100644 --- a/content/browser/frame_host/navigation_request.cc +++ b/content/browser/frame_host/navigation_request.cc -@@ -1589,6 +1589,11 @@ void NavigationRequest::OnStartChecksComplete( +@@ -1589,6 +1589,12 @@ void NavigationRequest::OnStartChecksComplete( headers.MergeFrom(navigation_handle_->TakeModifiedRequestHeaders()); begin_params_->headers = headers.ToString(); + GetContentClient()->browser()->MaybeHideReferrer(browser_context, + common_params_.url, + top_document_url, ++ frame_tree_node_->IsMainFrame(), + &common_params_.referrer); + loader_ = NavigationURLLoader::Create( diff --git a/patches/content-public-browser-content_browser_client.h.patch b/patches/content-public-browser-content_browser_client.h.patch index e08b2bf8a859..a466f33a110a 100644 --- a/patches/content-public-browser-content_browser_client.h.patch +++ b/patches/content-public-browser-content_browser_client.h.patch @@ -1,16 +1,17 @@ diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h -index 009991aa57e8f543d2fdb7095b2fe2c863373d71..00ab100aa8c17e4614f808ac2c0e2eea709c8243 100644 +index 009991aa57e8f543d2fdb7095b2fe2c863373d71..fddf6126c84e95c057301d69d7d297180c618050 100644 --- a/content/public/browser/content_browser_client.h +++ b/content/public/browser/content_browser_client.h -@@ -1516,6 +1516,13 @@ class CONTENT_EXPORT ContentBrowserClient { +@@ -1516,6 +1516,14 @@ class CONTENT_EXPORT ContentBrowserClient { virtual ui::AXMode GetAXModeForBrowserContext( BrowserContext* browser_context); -+ // Brave-speicific: allows the embedder to modify the referrer string ++ // Brave-specific: 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, ++ bool is_main_frame, + Referrer* referrer) {} + #if defined(OS_ANDROID) diff --git a/renderer/brave_content_settings_observer_browsertest.cc b/renderer/brave_content_settings_observer_browsertest.cc index dec12f801b06..945b260e1e7e 100644 --- a/renderer/brave_content_settings_observer_browsertest.cc +++ b/renderer/brave_content_settings_observer_browsertest.cc @@ -17,6 +17,8 @@ #include "components/content_settings/core/common/content_settings.h" #include "components/content_settings/core/common/content_settings_types.h" #include "net/dns/mock_host_resolver.h" +#include "net/http/http_request_headers.h" +#include "net/test/embedded_test_server/http_request.h" const char kIframeID[] = "test"; @@ -61,10 +63,16 @@ class BraveContentSettingsObserverBrowserTest : public InProcessBrowserTest { base::PathService::Get(brave::DIR_TEST_DATA, &test_data_dir); embedded_test_server()->ServeFilesFromDirectory(test_data_dir); + embedded_test_server()->RegisterRequestMonitor( + base::BindRepeating( + &BraveContentSettingsObserverBrowserTest::SaveReferrer, + base::Unretained(this))); + ASSERT_TRUE(embedded_test_server()->Start()); url_ = embedded_test_server()->GetURL("a.com", "/iframe.html"); iframe_url_ = embedded_test_server()->GetURL("b.com", "/simple.html"); + image_url_ = embedded_test_server()->GetURL("b.com", "/logo.png"); top_level_page_pattern_ = ContentSettingsPattern::FromString("http://a.com/*"); iframe_pattern_ = ContentSettingsPattern::FromString("http://b.com/*"); @@ -72,6 +80,37 @@ class BraveContentSettingsObserverBrowserTest : public InProcessBrowserTest { ContentSettingsPattern::FromString("https://firstParty/*"); } + void SaveReferrer(const net::test_server::HttpRequest& request) { + base::AutoLock auto_lock(last_referrers_lock_); + + // Replace "127.0.0.1:" with the hostnames used in this test. + net::test_server::HttpRequest::HeaderMap::const_iterator pos = + request.headers.find(net::HttpRequestHeaders::kHost); + GURL::Replacements replace_host; + if (pos != request.headers.end()) { + replace_host.SetHostStr(pos->second); + replace_host.SetPortStr(""); // Host header includes the port already. + } + GURL request_url = request.GetURL(); + request_url = request_url.ReplaceComponents(replace_host); + + pos = request.headers.find(net::HttpRequestHeaders::kReferer); + if (pos == request.headers.end()) { + last_referrers_[request_url] = ""; // no referrer + } else { + last_referrers_[request_url] = pos->second; + } + } + + std::string GetLastReferrer(const GURL& url) const { + base::AutoLock auto_lock(last_referrers_lock_); + auto pos = last_referrers_.find(url); + if (pos == last_referrers_.end()) { + return "(missing)"; // Fail test if we haven't seen this URL before. + } + return pos->second; + } + void TearDown() override { browser_content_client_.reset(); content_client_.reset(); @@ -79,6 +118,18 @@ class BraveContentSettingsObserverBrowserTest : public InProcessBrowserTest { const GURL& url() { return url_; } const GURL& iframe_url() { return iframe_url_; } + const GURL& image_url() { return image_url_; } + + std::string create_image_script() { + std::string s; + s = " var img = document.createElement('img');" + " img.onload = function () {" + " domAutomationController.send(img.src);" + " };" + " img.src = '" + image_url().spec() + "';" + " document.body.appendChild(img);"; + return s; + } const ContentSettingsPattern& top_level_page_pattern() { return top_level_page_pattern_; @@ -269,11 +320,14 @@ class BraveContentSettingsObserverBrowserTest : public InProcessBrowserTest { private: GURL url_; GURL iframe_url_; + GURL image_url_; ContentSettingsPattern top_level_page_pattern_; ContentSettingsPattern first_party_pattern_; ContentSettingsPattern iframe_pattern_; std::unique_ptr content_client_; std::unique_ptr browser_content_client_; + mutable base::Lock last_referrers_lock_; + std::map last_referrers_; base::ScopedTempDir temp_user_data_dir_; }; @@ -472,35 +526,66 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsObserverBrowserTest, EXPECT_EQ(settings.size(), 0u) << "There should not be any visible referrer rules."; + // The initial navigation doesn't have a referrer. NavigateToPageWithIframe(); - EXPECT_STREQ(ExecScriptGetStr(kReferrerScript, contents()).c_str(), ""); + EXPECT_TRUE(GetLastReferrer(url()).empty()); + + // Sub-resources loaded within the page get their referrer spoofed. + EXPECT_EQ(ExecScriptGetStr(create_image_script(), contents()), + image_url().spec()); + EXPECT_EQ(GetLastReferrer(image_url()), iframe_url().GetOrigin().spec()); + + // Cross-origin iframe navigations get their referrer spoofed. ASSERT_TRUE(NavigateIframeToURL(contents(), kIframeID, iframe_url())); ASSERT_EQ(child_frame()->GetLastCommittedURL(), iframe_url()); EXPECT_STREQ(ExecScriptGetStr(kReferrerScript, child_frame()).c_str(), iframe_url().GetOrigin().spec().c_str()); + EXPECT_EQ(GetLastReferrer(iframe_url()), iframe_url().GetOrigin().spec()); } IN_PROC_BROWSER_TEST_F(BraveContentSettingsObserverBrowserTest, BlockReferrer) { BlockReferrers(); + + // The initial navigation doesn't have a referrer. NavigateToPageWithIframe(); EXPECT_STREQ(ExecScriptGetStr(kReferrerScript, contents()).c_str(), ""); + EXPECT_TRUE(GetLastReferrer(url()).empty()); + + // Sub-resources loaded within the page get their referrer spoofed. + EXPECT_EQ(ExecScriptGetStr(create_image_script(), contents()), + image_url().spec()); + EXPECT_EQ(GetLastReferrer(image_url()), image_url().GetOrigin().spec()); + + // Cross-origin iframe navigations get their referrer spoofed. ASSERT_TRUE(NavigateIframeToURL(contents(), kIframeID, iframe_url())); ASSERT_EQ(child_frame()->GetLastCommittedURL(), iframe_url()); EXPECT_STREQ(ExecScriptGetStr(kReferrerScript, child_frame()).c_str(), iframe_url().GetOrigin().spec().c_str()); + EXPECT_EQ(GetLastReferrer(iframe_url()), iframe_url().GetOrigin().spec()); } IN_PROC_BROWSER_TEST_F(BraveContentSettingsObserverBrowserTest, AllowReferrer) { AllowReferrers(); - NavigateToPageWithIframe(); + // The initial navigation doesn't have a referrer. + NavigateToPageWithIframe(); EXPECT_STREQ(ExecScriptGetStr(kReferrerScript, contents()).c_str(), ""); + EXPECT_TRUE(GetLastReferrer(url()).empty()); + + // Sub-resources loaded within the page get the page URL as referrer. + EXPECT_EQ(ExecScriptGetStr(create_image_script(), contents()), + image_url().spec()); + EXPECT_EQ(GetLastReferrer(image_url()), url().spec()); + + // A cross-origin iframe navigation gets the URL of the first one as + // referrer. ASSERT_TRUE(NavigateIframeToURL(contents(), kIframeID, iframe_url())); ASSERT_EQ(child_frame()->GetLastCommittedURL(), iframe_url()); + EXPECT_EQ(GetLastReferrer(iframe_url()), url()); EXPECT_STREQ(ExecScriptGetStr(kReferrerScript, child_frame()).c_str(), url().spec().c_str()); } @@ -508,11 +593,23 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsObserverBrowserTest, AllowReferrer) { IN_PROC_BROWSER_TEST_F(BraveContentSettingsObserverBrowserTest, BlockReferrerShieldsDown) { BlockReferrers(); ShieldsDown(); + + // The initial navigation doesn't have a referrer. NavigateToPageWithIframe(); EXPECT_STREQ(ExecScriptGetStr(kReferrerScript, contents()).c_str(), ""); + EXPECT_TRUE(GetLastReferrer(url()).empty()); + + // Sub-resources loaded within the page get the page URL as referrer. + EXPECT_EQ(ExecScriptGetStr(create_image_script(), contents()), + image_url().spec()); + EXPECT_EQ(GetLastReferrer(image_url()), url().spec()); + + // A cross-origin iframe navigation gets the URL of the first one as + // referrer. ASSERT_TRUE(NavigateIframeToURL(contents(), kIframeID, iframe_url())); ASSERT_EQ(child_frame()->GetLastCommittedURL(), iframe_url()); + EXPECT_EQ(GetLastReferrer(iframe_url()), url()); EXPECT_STREQ(ExecScriptGetStr(kReferrerScript, child_frame()).c_str(), url().spec().c_str()); }