Skip to content

13464: Unrestrict referrer hiding for top-level navigations. #7591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 3 additions & 14 deletions browser/brave_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,6 @@ void BraveContentBrowserClient::MaybeHideReferrer(
content::BrowserContext* browser_context,
const GURL& request_url,
const GURL& document_url,
bool is_main_frame,
const std::string& method,
blink::mojom::ReferrerPtr* referrer) {
DCHECK(referrer && !referrer->is_null());
#if BUILDFLAG(ENABLE_EXTENSIONS)
Expand All @@ -368,19 +366,10 @@ void BraveContentBrowserClient::MaybeHideReferrer(
HostContentSettingsMapFactory::GetForProfile(profile),
document_url);

// Some top-level navigations get empty referrers (brave/brave-browser#3422).
network::mojom::ReferrerPolicy policy = (*referrer)->policy;
if (is_main_frame &&
brave_shields::ShouldCleanReferrerForTopLevelNavigation(method,
(*referrer)->url,
request_url)) {
policy = network::mojom::ReferrerPolicy::kNever;
}

content::Referrer new_referrer;
if (brave_shields::MaybeChangeReferrer(
allow_referrers, shields_up, (*referrer)->url, request_url, policy,
&new_referrer)) {
if (brave_shields::MaybeChangeReferrer(allow_referrers, shields_up,
(*referrer)->url, request_url,
&new_referrer)) {
(*referrer)->url = new_referrer.url;
(*referrer)->policy = new_referrer.policy;
}
Expand Down
2 changes: 0 additions & 2 deletions browser/brave_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ class BraveContentBrowserClient : public ChromeContentBrowserClient {
void MaybeHideReferrer(content::BrowserContext* browser_context,
const GURL& request_url,
const GURL& document_url,
bool is_main_frame,
const std::string& method,
blink::mojom::ReferrerPtr* referrer) override;

GURL GetEffectiveURL(content::BrowserContext* browser_context,
Expand Down
23 changes: 8 additions & 15 deletions browser/brave_content_browser_client_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -496,49 +496,42 @@ IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientReferrerTest,
blink::mojom::ReferrerPtr kReferrer = blink::mojom::Referrer::New(
kDocumentUrl, network::mojom::ReferrerPolicy::kDefault);

// Cross-origin navigations don't get a referrer.
// Cross-origin navigations get an origin.
blink::mojom::ReferrerPtr referrer = kReferrer.Clone();
client()->MaybeHideReferrer(browser()->profile(), kRequestUrl, kDocumentUrl,
true, "GET", &referrer);
EXPECT_EQ(referrer->url, GURL());

// Cross-origin navigations get a truncated referrer if method is not "GET" or
// "HEAD".
referrer = kReferrer.Clone();
client()->MaybeHideReferrer(browser()->profile(), kRequestUrl, kDocumentUrl,
true, "POST", &referrer);
&referrer);
EXPECT_EQ(referrer->url, kDocumentUrl.GetOrigin());

// Same-origin navigations get full referrers.
referrer = kReferrer.Clone();
client()->MaybeHideReferrer(browser()->profile(), kSameOriginRequestUrl,
kDocumentUrl, true, "GET", &referrer);
kDocumentUrl, &referrer);
EXPECT_EQ(referrer->url, kDocumentUrl);

// Same-site navigations get truncated referrers.
referrer = kReferrer.Clone();
client()->MaybeHideReferrer(browser()->profile(), kSameSiteRequestUrl,
kDocumentUrl, true, "GET", &referrer);
kDocumentUrl, &referrer);
EXPECT_EQ(referrer->url, kDocumentUrl.GetOrigin());

// Cross-origin iframe navigations get origins.
referrer = kReferrer.Clone();
client()->MaybeHideReferrer(browser()->profile(), kRequestUrl, kDocumentUrl,
false, "GET", &referrer);
&referrer);
EXPECT_EQ(referrer->url, kDocumentUrl.GetOrigin().spec());

// Same-origin iframe navigations get full referrers.
referrer = kReferrer.Clone();
client()->MaybeHideReferrer(browser()->profile(), kSameOriginRequestUrl,
kDocumentUrl, false, "GET", &referrer);
kDocumentUrl, &referrer);
EXPECT_EQ(referrer->url, kDocumentUrl);

// Special rule for extensions.
const GURL kExtensionUrl("chrome-extension://abc/path?query");
referrer = kReferrer.Clone();
referrer->url = kExtensionUrl;
client()->MaybeHideReferrer(browser()->profile(), kRequestUrl, kExtensionUrl,
true, "GET", &referrer);
&referrer);
EXPECT_EQ(referrer->url, kExtensionUrl);

// Allow referrers for certain URL.
Expand All @@ -548,6 +541,6 @@ IN_PROC_BROWSER_TEST_F(BraveContentBrowserClientReferrerTest,
CONTENT_SETTING_ALLOW);
referrer = kReferrer.Clone();
client()->MaybeHideReferrer(browser()->profile(), kRequestUrl, kDocumentUrl,
true, "GET", &referrer);
&referrer);
EXPECT_EQ(referrer->url, kDocumentUrl);
}
16 changes: 7 additions & 9 deletions browser/net/brave_site_hacks_network_delegate_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,8 @@ bool ApplyPotentialReferrerBlock(std::shared_ptr<BraveRequestInfo> ctx) {

content::Referrer new_referrer;
if (brave_shields::MaybeChangeReferrer(
ctx->allow_referrers, ctx->allow_brave_shields,
GURL(ctx->referrer), ctx->request_url,
blink::ReferrerUtils::NetToMojoReferrerPolicy(ctx->referrer_policy),
&new_referrer)) {
ctx->allow_referrers, ctx->allow_brave_shields, GURL(ctx->referrer),
ctx->request_url, &new_referrer)) {
ctx->new_referrer = new_referrer.url;
return true;
}
Expand Down Expand Up @@ -179,14 +177,14 @@ int OnBeforeStartTransaction_SiteHacksWork(
// will affect performance).
// Note that this code only affects "Referer" header sent via network - we
// handle document.referer in content::NavigationRequest (see also
// BraveContentBrowserClient::MaybeHideReferrer).
// |BraveContentBrowserClient::MaybeHideReferrer|).
if (!ctx->allow_referrers && ctx->allow_brave_shields &&
ctx->redirect_source.is_valid() &&
ctx->resource_type == blink::mojom::ResourceType::kMainFrame &&
brave_shields::ShouldCleanReferrerForTopLevelNavigation(
ctx->method, ctx->redirect_source, ctx->request_url)) {
// This is hack that notifies the patched code in net::URLRequest.
ctx->removed_headers.insert("X-Brave-Clear-Referer");
!brave_shields::IsSameOriginNavigation(ctx->redirect_source,
ctx->request_url)) {
// This is a hack that notifies the network layer.
ctx->removed_headers.insert("X-Brave-Cap-Referrer");
}
return net::OK;
}
Expand Down
24 changes: 11 additions & 13 deletions chromium_src/content/browser/renderer_host/navigation_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,17 @@ GURL GetTopDocumentGURL(content::FrameTreeNode* frame_tree_node) {

} // namespace

#define BRAVE_ONREQUESTREDIRECTED_MAYBEHIDEREFERRER \
BrowserContext* browser_context = \
frame_tree_node_->navigator().GetController()->GetBrowserContext(); \
GetContentClient()->browser()->MaybeHideReferrer( \
browser_context, common_params_->url, \
GetTopDocumentGURL(frame_tree_node_), frame_tree_node_->IsMainFrame(), \
common_params_->method, &common_params_->referrer);

#define BRAVE_ONSTARTCHECKSCOMPLETE_MAYBEHIDEREFERRER \
GetContentClient()->browser()->MaybeHideReferrer( \
browser_context, common_params_->url, \
GetTopDocumentGURL(frame_tree_node_), frame_tree_node_->IsMainFrame(), \
common_params_->method, &common_params_->referrer);
#define BRAVE_ONREQUESTREDIRECTED_MAYBEHIDEREFERRER \
BrowserContext* browser_context = \
frame_tree_node_->navigator().GetController()->GetBrowserContext(); \
GetContentClient()->browser()->MaybeHideReferrer( \
browser_context, common_params_->url, \
GetTopDocumentGURL(frame_tree_node_), &common_params_->referrer);

#define BRAVE_ONSTARTCHECKSCOMPLETE_MAYBEHIDEREFERRER \
GetContentClient()->browser()->MaybeHideReferrer( \
browser_context, common_params_->url, \
GetTopDocumentGURL(frame_tree_node_), &common_params_->referrer);

#include "../../../../../content/browser/renderer_host/navigation_request.cc"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@
#define BRAVE_CONTENT_BROWSER_CLIENT_H \
virtual void MaybeHideReferrer( \
BrowserContext* browser_context, const GURL& request_url, \
const GURL& document_url, bool is_main_frame, \
const std::string& method, \
blink::mojom::ReferrerPtr* referrer) {}
const GURL& document_url, blink::mojom::ReferrerPtr* referrer) {}

#include "../../../../../content/public/browser/content_browser_client.h"

Expand Down
15 changes: 10 additions & 5 deletions chromium_src/net/url_request/redirect_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@

#include "net/url_request/redirect_util.h"

#include "base/stl_util.h"
#include "net/url_request/url_request_job.h"

#define UpdateHttpRequest UpdateHttpRequest_ChromiumImpl
#include "../../../../net/url_request/redirect_util.cc"
#undef UpdateHttpRequest
Expand All @@ -26,12 +29,14 @@ void RedirectUtil::UpdateHttpRequest(
modified_headers,
request_headers,
should_clear_upload);
// Hack for dropping referrer at the network layer.
// Hack for capping referrers at the network layer.
if (removed_headers) {
if (removed_headers->end() != std::find(removed_headers->begin(),
removed_headers->end(),
"X-Brave-Clear-Referer")) {
const_cast<RedirectInfo&>(redirect_info).new_referrer.clear();
if (base::Contains(*removed_headers, "X-Brave-Cap-Referrer")) {
GURL capped_referrer = URLRequestJob::ComputeReferrerForPolicy(
ReferrerPolicy::REDUCE_GRANULARITY_ON_TRANSITION_CROSS_ORIGIN,
GURL(redirect_info.new_referrer), redirect_info.new_url);
const_cast<RedirectInfo&>(redirect_info).new_referrer =
capped_referrer.spec();
}
}
}
Expand Down
28 changes: 12 additions & 16 deletions components/brave_shields/browser/brave_shields_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -414,41 +414,37 @@ void DispatchBlockedEvent(const GURL& request_url,
#endif
}

bool ShouldCleanReferrerForTopLevelNavigation(const std::string& method,
const GURL& referrer_url,
const GURL& request_url) {
// See https://github.com/brave/brave-browser/issues/8696
return ((method == "GET" || method == "HEAD") &&
!net::registry_controlled_domains::SameDomainOrHost(
referrer_url, request_url,
net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES));
bool IsSameOriginNavigation(const GURL& referrer, const GURL& target_url) {
const url::Origin original_referrer = url::Origin::Create(referrer);
const url::Origin target_origin = url::Origin::Create(target_url);

return original_referrer.IsSameOriginWith(target_origin);
}

bool MaybeChangeReferrer(
bool allow_referrers,
bool shields_up,
const GURL& current_referrer,
const GURL& target_url,
network::mojom::ReferrerPolicy policy,
Referrer* output_referrer) {
DCHECK(output_referrer);
if (allow_referrers || !shields_up || current_referrer.is_empty()) {
return false;
}

const url::Origin original_referrer = url::Origin::Create(current_referrer);
const url::Origin target_origin = url::Origin::Create(target_url);

if (original_referrer.IsSameOriginWith(target_origin)) {
if (IsSameOriginNavigation(current_referrer, target_url)) {
// Do nothing for same-origin requests. This check also prevents us from
// sending referrer from HTTPS to HTTP.
return false;
}

// Cap referrer to "strict-origin-when-cross-origin" or anything more
// restrictive according do given policy.
// Cap the referrer to "strict-origin-when-cross-origin". More restrictive
// policies should be already applied.
// See https://github.com/brave/brave-browser/issues/13464
*output_referrer = Referrer::SanitizeForRequest(
target_url, Referrer(current_referrer.GetOrigin(), policy));
target_url,
Referrer(current_referrer.GetOrigin(),
network::mojom::ReferrerPolicy::kStrictOriginWhenCrossOrigin));

return true;
}
Expand Down
5 changes: 1 addition & 4 deletions components/brave_shields/browser/brave_shields_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,13 @@ void DispatchBlockedEvent(const GURL& request_url,
int frame_tree_node_id,
const std::string& block_type);

bool ShouldCleanReferrerForTopLevelNavigation(const std::string& method,
const GURL& referrer_url,
const GURL& request_url);
bool IsSameOriginNavigation(const GURL& referrer, const GURL& target_url);

bool MaybeChangeReferrer(
bool allow_referrers,
bool shields_up,
const GURL& current_referrer,
const GURL& target_url,
network::mojom::ReferrerPolicy policy,
content::Referrer* output_referrer);


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -622,10 +622,11 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsAgentImplBrowserTest,
link_url().GetOrigin().spec());
EXPECT_EQ(GetLastReferrer(same_site_url()), link_url().GetOrigin().spec());

// Cross-site navigations get no referrer.
// Cross-site navigations should follow the default referrer policy.
NavigateDirectlyToPageWithLink(cross_site_url());
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()), "");
EXPECT_EQ(GetLastReferrer(cross_site_url()), "");
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()),
link_url().GetOrigin().spec());
EXPECT_EQ(GetLastReferrer(cross_site_url()), link_url().GetOrigin().spec());
}

IN_PROC_BROWSER_TEST_F(BraveContentSettingsAgentImplBrowserTest,
Expand Down Expand Up @@ -655,10 +656,12 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsAgentImplBrowserTest,
url().GetOrigin().spec());
EXPECT_EQ(GetLastReferrer(cross_site_url()), url().GetOrigin().spec());

// Cross-site navigations get no referrer.
// Cross-site navigations should follow the default referrer policy.
RedirectToPageWithLink(redirect_to_cross_site_url(), cross_site_url());
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()), "");
EXPECT_EQ(GetLastReferrer(cross_site_url()), "");
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()),
redirect_to_cross_site_url().GetOrigin().spec());
EXPECT_EQ(GetLastReferrer(cross_site_url()),
redirect_to_cross_site_url().GetOrigin().spec());
EXPECT_EQ(GetLastReferrer(redirect_to_cross_site_url()),
link_url().spec());
}
Expand Down Expand Up @@ -696,28 +699,28 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsAgentImplBrowserTest,
url().GetOrigin().spec());
EXPECT_EQ(GetLastReferrer(cross_site_url()), url().GetOrigin().spec());

// Same-origin navigations get the original page origin as the referrer.
// Same-origin navigations get the original page URL as the referrer.
NavigateDirectlyToPageWithLink(same_origin_url());
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()), link_url().spec());
EXPECT_EQ(GetLastReferrer(same_origin_url()), link_url().spec());

// Same-site but cross-origin navigations get the original page origin as the
// referrer.
const std::string expected_referrer = link_url().GetOrigin().spec();
NavigateDirectlyToPageWithLink(same_site_url());
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()),
link_url().GetOrigin().spec());
EXPECT_EQ(GetLastReferrer(same_site_url()), link_url().GetOrigin().spec());
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()), expected_referrer);
EXPECT_EQ(GetLastReferrer(same_site_url()), expected_referrer);

// Cross-site navigations get no referrer.
// Cross-site navigations should follow the default referrer policy.
NavigateDirectlyToPageWithLink(cross_site_url());
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()), "");
EXPECT_EQ(GetLastReferrer(cross_site_url()), "");
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()), expected_referrer);
EXPECT_EQ(GetLastReferrer(cross_site_url()), expected_referrer);

// Check that a less restrictive policy is not respected.
NavigateDirectlyToPageWithLink(cross_site_url(),
"no-referrer-when-downgrade");
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()), "");
EXPECT_EQ(GetLastReferrer(cross_site_url()), "");
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()), expected_referrer);
EXPECT_EQ(GetLastReferrer(cross_site_url()), expected_referrer);

// Check that "no-referrer" policy is respected as more restrictive.
NavigateDirectlyToPageWithLink(same_origin_url(), "no-referrer");
Expand All @@ -727,6 +730,11 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsAgentImplBrowserTest,
NavigateDirectlyToPageWithLink(cross_site_url(), "no-referrer");
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()), "");
EXPECT_EQ(GetLastReferrer(cross_site_url()), "");

// Check that "same-origin" policy is respected as more restrictive.
NavigateDirectlyToPageWithLink(cross_site_url(), "same-origin");
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()), "");
EXPECT_EQ(GetLastReferrer(cross_site_url()), "");
}

IN_PROC_BROWSER_TEST_F(BraveContentSettingsAgentImplBrowserTest,
Expand All @@ -752,10 +760,12 @@ IN_PROC_BROWSER_TEST_F(BraveContentSettingsAgentImplBrowserTest,
url().GetOrigin().spec());
EXPECT_EQ(GetLastReferrer(cross_site_url()), url().GetOrigin().spec());

// Cross-site navigations get no referrer.
// Cross-site navigations should follow the default referrer policy.
RedirectToPageWithLink(redirect_to_cross_site_url(), cross_site_url());
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()), "");
EXPECT_EQ(GetLastReferrer(cross_site_url()), "");
EXPECT_EQ(ExecScriptGetStr(kReferrerScript, contents()),
redirect_to_cross_site_url().GetOrigin().spec());
EXPECT_EQ(GetLastReferrer(cross_site_url()),
redirect_to_cross_site_url().GetOrigin().spec());
// Intermidiate same-origin navigation gets full referrer.
EXPECT_EQ(GetLastReferrer(redirect_to_cross_site_url()),
link_url().spec());
Expand Down