Skip to content

Commit

Permalink
Disable shields for extensions
Browse files Browse the repository at this point in the history
The reasoning is they have APIs to do much worse tracking than our shields would protect them from. Users install them knowing they will have elevated permissions

Fix brave/brave-browser#1380
  • Loading branch information
bbondy committed Nov 28, 2018
1 parent 86cf2d3 commit 8b8c9d9
Show file tree
Hide file tree
Showing 14 changed files with 62 additions and 96 deletions.
8 changes: 5 additions & 3 deletions browser/brave_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,11 @@ bool BraveContentBrowserClient::AllowAccessCookie(const GURL& url, const GURL& f
BraveShieldsWebContentsObserver::GetTabURLFromRenderFrameInfo(
render_process_id, render_frame_id).GetOrigin();
ProfileIOData* io_data = ProfileIOData::FromResourceContext(context);
bool allow_brave_shields = brave_shields::IsAllowContentSettingWithIOData(
io_data, tab_origin, tab_origin, CONTENT_SETTINGS_TYPE_PLUGINS,
brave_shields::kBraveShields);
bool allow_brave_shields =
brave_shields::IsAllowContentSettingWithIOData(
io_data, tab_origin, tab_origin, CONTENT_SETTINGS_TYPE_PLUGINS,
brave_shields::kBraveShields) &&
!first_party.SchemeIs(kChromeExtensionScheme);
bool allow_1p_cookies = brave_shields::IsAllowContentSettingWithIOData(
io_data, tab_origin, GURL("https://firstParty/"),
CONTENT_SETTINGS_TYPE_PLUGINS, brave_shields::kCookies);
Expand Down
8 changes: 1 addition & 7 deletions browser/net/brave_ad_block_tp_network_delegate_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "brave/browser/brave_browser_process_impl.h"
#include "brave/common/network_constants.h"
#include "brave/common/shield_exceptions.h"
#include "brave/common/url_util.h"
#include "brave/components/brave_shields/browser/ad_block_regional_service.h"
#include "brave/components/brave_shields/browser/ad_block_service.h"
#include "brave/components/brave_shields/browser/brave_shields_util.h"
Expand Down Expand Up @@ -99,12 +98,7 @@ void OnBeforeURLRequestAdBlockTPOnTaskRunner(std::shared_ptr<BraveRequestInfo> c
}
DCHECK(ctx->request_identifier != 0);

// For PDFJS we don't want to treat 2 different URLs as 3p for the main pdf
// load which is meant to be top level
// chrome-extension://oemmndcbldboiebfnladdacbdfmadadm/https://developer.att.com/.../file.pdf
// https://developer.att.com/.../file.pdf
// So if the tab origin is chrome-extension, set it to that of the PDF only for PDFJS
std::string tab_host = brave::GetURLOrPDFURL(ctx->tab_url).host();
std::string tab_host = ctx->tab_origin.host();
if (!g_brave_browser_process->tracking_protection_service()->
ShouldStartRequest(ctx->request_url, ctx->resource_type, tab_host)) {
ctx->new_url_spec = GetBlankDataURLForResourceType(ctx->resource_type).spec();
Expand Down
4 changes: 4 additions & 0 deletions browser/net/brave_site_hacks_network_delegate_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/strings/string_util.h"
#include "brave/common/network_constants.h"
#include "brave/common/shield_exceptions.h"
#include "brave/common/url_constants.h"
#include "brave/components/brave_shields/browser/brave_shields_util.h"
#include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h"
#include "brave/components/brave_shields/common/brave_shield_constants.h"
Expand All @@ -28,6 +29,9 @@ bool ApplyPotentialReferrerBlock(net::URLRequest* request) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
GURL target_origin = GURL(request->url()).GetOrigin();
GURL tab_origin = request->site_for_cookies().GetOrigin();
if (tab_origin.SchemeIs(kChromeExtensionScheme)) {
return false;
}
bool allow_referrers = brave_shields::IsAllowContentSettingFromIO(
request, tab_origin, tab_origin, CONTENT_SETTINGS_TYPE_PLUGINS,
brave_shields::kReferrers);
Expand Down
28 changes: 28 additions & 0 deletions browser/net/brave_site_hacks_network_delegate_helper_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -271,4 +271,32 @@ TEST_F(BraveSiteHacksNetworkDelegateHelperTest, ReferrerCleared) {
});
}

TEST_F(BraveSiteHacksNetworkDelegateHelperTest, ReferrerWouldBeClearedButExtensionSite) {
std::vector<GURL> urls({
GURL("https://digg.com/7"),
GURL("https://slashdot.org/5"),
GURL("https://bondy.brian.org")
});
std::for_each(urls.begin(), urls.end(),
[this](GURL url){
net::TestDelegate test_delegate;
std::unique_ptr<net::URLRequest> request =
context()->CreateRequest(url, net::IDLE, &test_delegate,
TRAFFIC_ANNOTATION_FOR_TESTS);
request->set_site_for_cookies(GURL("chrome-extension://aemmndcbldboiebfnladdacbdfmadadm/test.html"));
std::string original_referrer = "https://hello.brianbondy.com/about";
request->SetReferrer(original_referrer);

std::shared_ptr<brave::BraveRequestInfo>
brave_request_info(new brave::BraveRequestInfo());
brave::BraveRequestInfo::FillCTXFromRequest(request.get(), brave_request_info);
brave::ResponseCallback callback;
int ret = brave::OnBeforeURLRequest_SiteHacksWork(callback, brave_request_info);
EXPECT_EQ(ret, net::OK);
// new_url should not be set
EXPECT_TRUE(brave_request_info->new_url_spec.empty());
EXPECT_STREQ(request->referrer().c_str(), original_referrer.c_str());
});
}

} // namespace
5 changes: 3 additions & 2 deletions browser/net/url_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <string>

#include "brave/common/url_constants.h"
#include "brave/components/brave_shields/browser/brave_shields_util.h"
#include "brave/components/brave_shields/common/brave_shield_constants.h"
#include "content/public/browser/resource_request_info.h"
Expand All @@ -23,7 +24,6 @@ void BraveRequestInfo::FillCTXFromRequest(const net::URLRequest* request,
std::shared_ptr<brave::BraveRequestInfo> ctx) {
ctx->request_identifier = request->identifier();
ctx->request_url = request->url();
ctx->tab_url = request->site_for_cookies();
ctx->tab_origin = request->site_for_cookies().GetOrigin();
auto* request_info = content::ResourceRequestInfo::ForRequest(request);
if (request_info) {
Expand All @@ -33,7 +33,8 @@ void BraveRequestInfo::FillCTXFromRequest(const net::URLRequest* request,
&ctx->frame_tree_node_id);
ctx->allow_brave_shields = brave_shields::IsAllowContentSettingFromIO(
request, ctx->tab_origin, ctx->tab_origin, CONTENT_SETTINGS_TYPE_PLUGINS,
brave_shields::kBraveShields);
brave_shields::kBraveShields) &&
!request->site_for_cookies().SchemeIs(kChromeExtensionScheme);
ctx->allow_ads = brave_shields::IsAllowContentSettingFromIO(
request, ctx->tab_origin, ctx->tab_origin, CONTENT_SETTINGS_TYPE_PLUGINS,
brave_shields::kAds);
Expand Down
1 change: 0 additions & 1 deletion browser/net/url_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ struct BraveRequestInfo {
~BraveRequestInfo();
GURL request_url;
GURL tab_origin;
GURL tab_url;
std::string new_url_spec;
bool allow_brave_shields = true;
bool allow_ads = false;
Expand Down
2 changes: 0 additions & 2 deletions common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ source_set("common") {
"shield_exceptions.h",
"url_constants.cc",
"url_constants.h",
"url_util.cc",
"url_util.h",
]

public_deps = [
Expand Down
1 change: 1 addition & 0 deletions common/url_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "brave/common/url_constants.h"

const char kChromeExtensionScheme[] = "chrome-extension";
const char kBraveUIScheme[] = "brave";
const char kMagnetScheme[] = "magnet";
const char kWidevineMoreInfoURL[] = "https://www.eff.org/issues/drm";
Expand Down
1 change: 1 addition & 0 deletions common/url_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef BRAVE_COMMON_URL_CONSTANTS_H_
#define BRAVE_COMMON_URL_CONSTANTS_H_

extern const char kChromeExtensionScheme[];
extern const char kBraveUIScheme[];
extern const char kMagnetScheme[];
extern const char kWidevineMoreInfoURL[];
Expand Down
28 changes: 0 additions & 28 deletions common/url_util.cc

This file was deleted.

18 changes: 0 additions & 18 deletions common/url_util.h

This file was deleted.

34 changes: 0 additions & 34 deletions common/url_util_unittest.cc

This file was deleted.

19 changes: 19 additions & 0 deletions components/content_settings/core/browser/brave_cookie_settings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "brave/components/brave_shields/common/brave_shield_constants.h"
#include "brave/common/brave_cookie_blocking.h"
#include "extensions/buildflags/buildflags.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "url/gurl.h"

Expand Down Expand Up @@ -36,6 +37,24 @@ void BraveCookieSettings::GetCookieSetting(const GURL& url,
content_settings::SettingSource* source,
ContentSetting* cookie_setting) const {
DCHECK(cookie_setting);

// Auto-allow in extensions or for WebUI embedded in a secure origin.
// This matches an early return case in CookieSettings::GetCookieSetting
if (first_party_url.SchemeIs(kChromeUIScheme) &&
url.SchemeIsCryptographic()) {
*cookie_setting = CONTENT_SETTING_ALLOW;
return;
}

// This matches an early return case in CookieSettings::GetCookieSetting
#if BUILDFLAG(ENABLE_EXTENSIONS)
if (url.SchemeIs(extension_scheme_) &&
first_party_url.SchemeIs(extension_scheme_)) {
*cookie_setting = CONTENT_SETTING_ALLOW;
return;
}
#endif

CookieSettings::GetCookieSetting(url, first_party_url, source,
cookie_setting);
if (*cookie_setting == CONTENT_SETTING_BLOCK) {
Expand Down
1 change: 0 additions & 1 deletion test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ test("brave_unit_tests") {
"//brave/common/shield_exceptions_unittest.cc",
"//brave/common/tor/tor_test_constants.cc",
"//brave/common/tor/tor_test_constants.h",
"//brave/common/url_util_unittest.cc",
"//brave/components/assist_ranker/ranker_model_loader_impl_unittest.cc",
"//brave/components/brave_shields/browser/ad_block_regional_service_unittest.cc",
"//brave/components/brave_sync/bookmark_order_util_unittest.cc",
Expand Down

0 comments on commit 8b8c9d9

Please sign in to comment.