Skip to content
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

Don't use CONTENT_SETTING_DEFAULT for FP setting #5879

Merged
merged 1 commit into from
Jun 24, 2020
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
57 changes: 43 additions & 14 deletions components/brave_shields/browser/brave_shields_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h"
#include "brave/components/brave_shields/browser/referrer_whitelist_service.h"
#include "brave/components/brave_shields/common/brave_shield_constants.h"
#include "brave/components/brave_shields/common/brave_shield_utils.h"
#include "brave/components/brave_shields/common/features.h"
#include "brave/components/content_settings/core/common/content_settings_util.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
Expand Down Expand Up @@ -332,25 +333,53 @@ void SetFingerprintingControlType(Profile* profile,
if (!primary_pattern.IsValid())
return;

auto* map = HostContentSettingsMapFactory::GetForProfile(profile);
map->SetContentSettingCustomScope(
primary_pattern, ContentSettingsPattern::Wildcard(),
ContentSettingsType::PLUGINS, kFingerprintingV2,
GetDefaultAllowFromControlType(type));
// Clear previous value to have only one rule for one pattern.
HostContentSettingsMapFactory::GetForProfile(profile)->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need two rules for this. The secondary pattern is either "balanced" or it's a wildcard. It should never be both.

Copy link
Member Author

@simonhong simonhong Jun 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed to have one fp rule for one url. - e2213b5817d4dc3b3c12a12da47bff69295034f4

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused, it's still setting two rules here. We should never be setting both wildcard and balanced for the secondary pattern

Copy link
Member Author

@simonhong simonhong Jun 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this PR only set one rule for one url but two patterns.
If current www.brave.com has balanced, only one rule (second - balanced) is stored.
and if user set other rule(allow), it will have only one rule (second - *) is stored.
When new rule is stored previous balanced rule is not deleted automatically because both are different rule. So, clearing step is needed.
If we don't clear here, prefs could have two rules.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see. I missed the comment

SetContentSettingCustomScope(
primary_pattern,
ContentSettingsPattern::FromString("https://balanced/*"),
ContentSettingsType::PLUGINS,
kFingerprintingV2,
CONTENT_SETTING_DEFAULT);
HostContentSettingsMapFactory::GetForProfile(profile)->
SetContentSettingCustomScope(primary_pattern,
ContentSettingsPattern::Wildcard(),
ContentSettingsType::PLUGINS,
kFingerprintingV2,
CONTENT_SETTING_DEFAULT);

auto content_setting = CONTENT_SETTING_BLOCK;
auto secondary_pattern =
ContentSettingsPattern::FromString("https://balanced/*");

if (type != ControlType::DEFAULT) {
content_setting = GetDefaultBlockFromControlType(type);
secondary_pattern = ContentSettingsPattern::Wildcard();
}

HostContentSettingsMapFactory::GetForProfile(profile)->
SetContentSettingCustomScope(primary_pattern,
secondary_pattern,
ContentSettingsType::PLUGINS,
kFingerprintingV2,
content_setting);

RecordShieldsSettingChanged();
}

ControlType GetFingerprintingControlType(Profile* profile, const GURL& url) {
auto* map = HostContentSettingsMapFactory::GetForProfile(profile);
ContentSetting setting = map->GetContentSetting(
url, GURL(), ContentSettingsType::PLUGINS, kFingerprintingV2);
if (setting == CONTENT_SETTING_BLOCK) {
return ControlType::BLOCK;
} else if (setting == CONTENT_SETTING_ALLOW) {
return ControlType::ALLOW;
}
return ControlType::DEFAULT;
ContentSettingsForOneType fingerprinting_rules;
HostContentSettingsMapFactory::GetForProfile(profile)->
GetSettingsForOneType(ContentSettingsType::PLUGINS,
brave_shields::kFingerprintingV2,
&fingerprinting_rules);

ContentSetting fp_setting =
GetBraveFPContentSettingFromRules(fingerprinting_rules, url);
if (fp_setting == CONTENT_SETTING_DEFAULT)
return ControlType::DEFAULT;
return fp_setting == CONTENT_SETTING_ALLOW ? ControlType::ALLOW
: ControlType::BLOCK;
}

void SetHTTPSEverywhereEnabled(Profile* profile,
Expand Down
41 changes: 25 additions & 16 deletions components/brave_shields/browser/brave_shields_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -617,24 +617,33 @@ TEST_F(BraveShieldsUtilTest, SetFingerprintingControlType_Default) {
/* DEFAULT */
brave_shields::SetFingerprintingControlType(
profile(), ControlType::DEFAULT, GURL());
setting =
map->GetContentSetting(GURL(), GURL(), ContentSettingsType::PLUGINS,
brave_shields::kFingerprintingV2);
EXPECT_EQ(CONTENT_SETTING_DEFAULT, setting);
setting = map->GetContentSetting(GURL(), GURL("https://firstParty"),
ContentSettingsType::PLUGINS,
brave_shields::kFingerprintingV2);
EXPECT_EQ(CONTENT_SETTING_DEFAULT, setting);
ControlType type =
brave_shields::GetFingerprintingControlType(profile(), GURL());
EXPECT_EQ(ControlType::DEFAULT, type);

// setting should apply to all urls
setting = map->GetContentSetting(GURL("http://brave.com"), GURL(),
ContentSettingsType::PLUGINS,
brave_shields::kFingerprintingV2);
EXPECT_EQ(CONTENT_SETTING_DEFAULT, setting);
setting = map->GetContentSetting(
GURL("http://brave.com"), GURL("https://firstParty"),
ContentSettingsType::PLUGINS, brave_shields::kFingerprintingV2);
EXPECT_EQ(CONTENT_SETTING_DEFAULT, setting);
type = brave_shields::GetFingerprintingControlType(
profile(), GURL("http://brave.com"));
EXPECT_EQ(ControlType::DEFAULT, type);

/* Global ALLOW and Site explicit DEFAULT */
brave_shields::SetFingerprintingControlType(profile(), ControlType::ALLOW,
GURL());
brave_shields::SetFingerprintingControlType(profile(), ControlType::DEFAULT,
GURL("http://brave.com"));

// Site should have DEFAULT if it's explicitly set.
type = brave_shields::GetFingerprintingControlType(
profile(), GURL("http://brave.com"));
EXPECT_EQ(ControlType::DEFAULT, type);

/* Global BLOCK and Site explicit DEFAULT */
brave_shields::SetFingerprintingControlType(profile(), ControlType::BLOCK,
GURL());
// Site should have DEFAULT if it's explicitly set.
type = brave_shields::GetFingerprintingControlType(
profile(), GURL("http://brave.com"));
EXPECT_EQ(ControlType::DEFAULT, type);
}

TEST_F(BraveShieldsUtilTest, SetFingerprintingControlType_ForOrigin) {
Expand Down
4 changes: 4 additions & 0 deletions components/brave_shields/common/BUILD.gn
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
source_set("common") {
sources = [
"brave_shield_constants.h",
"brave_shield_utils.cc",
"brave_shield_utils.h",
"features.cc",
"features.h",
]

deps = [
"//base",
"//components/content_settings/core/common",
"//url",
]
}
49 changes: 49 additions & 0 deletions components/brave_shields/common/brave_shield_utils.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/* Copyright 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/components/brave_shields/common/brave_shield_utils.h"

#include "base/optional.h"
#include "components/content_settings/core/common/content_settings_pattern.h"
#include "url/gurl.h"

ContentSetting GetBraveFPContentSettingFromRules(
const ContentSettingsForOneType& fp_rules,
const GURL& primary_url) {
base::Optional<ContentSettingPatternSource> global_fp_rule;
base::Optional<ContentSettingPatternSource> global_fp_balanced_rule;

for (const auto& rule : fp_rules) {
if (rule.primary_pattern != ContentSettingsPattern::Wildcard() &&
rule.primary_pattern.Matches(primary_url)) {
if (rule.secondary_pattern ==
ContentSettingsPattern::FromString("https://balanced")) {
return CONTENT_SETTING_DEFAULT;
}
if (rule.secondary_pattern == ContentSettingsPattern::Wildcard())
return rule.GetContentSetting();
}

if (rule.primary_pattern == ContentSettingsPattern::Wildcard()) {
if (rule.secondary_pattern ==
ContentSettingsPattern::FromString("https://balanced")) {
DCHECK(!global_fp_rule);
global_fp_balanced_rule = rule;
}
if (rule.secondary_pattern == ContentSettingsPattern::Wildcard()) {
DCHECK(!global_fp_balanced_rule);
global_fp_rule = rule;
}
}
}

if (global_fp_balanced_rule)
return CONTENT_SETTING_DEFAULT;

if (global_fp_rule)
return global_fp_rule->GetContentSetting();

return CONTENT_SETTING_DEFAULT;
}
17 changes: 17 additions & 0 deletions components/brave_shields/common/brave_shield_utils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/* Copyright 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_COMPONENTS_BRAVE_SHIELDS_COMMON_BRAVE_SHIELD_UTILS_H_
#define BRAVE_COMPONENTS_BRAVE_SHIELDS_COMMON_BRAVE_SHIELD_UTILS_H_

#include "components/content_settings/core/common/content_settings.h"

class GURL;

ContentSetting GetBraveFPContentSettingFromRules(
const ContentSettingsForOneType& fp_rules,
const GURL& primary_url);

#endif // BRAVE_COMPONENTS_BRAVE_SHIELDS_COMMON_BRAVE_SHIELD_UTILS_H_
65 changes: 10 additions & 55 deletions renderer/brave_content_settings_agent_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "base/strings/utf_string_conversions.h"
#include "brave/common/render_messages.h"
#include "brave/common/shield_exceptions.h"
#include "brave/components/brave_shields/common/brave_shield_utils.h"
#include "brave/components/brave_shields/common/features.h"
#include "brave/content/common/frame_messages.h"
#include "components/content_settings/core/common/content_settings_pattern.h"
Expand Down Expand Up @@ -64,27 +65,6 @@ bool IsBraveShieldsDown(const blink::WebFrame* frame,
return setting == CONTENT_SETTING_BLOCK;
}

template <typename URL>
ContentSetting GetBraveContentSettingFromRules(
const ContentSettingsForOneType& shield_rules,
const ContentSettingsForOneType& rules,
const blink::WebFrame* frame,
const URL& secondary_url) {
// if shields is down, allow everything
if (IsBraveShieldsDown(frame, secondary_url, shield_rules))
return CONTENT_SETTING_ALLOW;

const GURL& primary_url = GetOriginOrURL(frame);
const GURL& secondary_gurl = secondary_url;
for (const auto& rule : rules) {
if (rule.primary_pattern.Matches(primary_url) &&
rule.secondary_pattern.Matches(secondary_gurl)) {
return rule.GetContentSetting();
}
}
return CONTENT_SETTING_DEFAULT;
}

} // namespace

BraveContentSettingsAgentImpl::BraveContentSettingsAgentImpl(
Expand Down Expand Up @@ -198,36 +178,6 @@ void BraveContentSettingsAgentImpl::DidBlockFingerprinting(
Send(new BraveViewHostMsg_FingerprintingBlocked(routing_id(), details));
}

ContentSetting BraveContentSettingsAgentImpl::GetFPContentSettingFromRules(
const ContentSettingsForOneType& rules,
const blink::WebFrame* frame,
const GURL& secondary_url) {

if (rules.size() == 0)
return CONTENT_SETTING_DEFAULT;

const GURL& primary_url = GetOriginOrURL(frame);

for (const auto& rule : rules) {
ContentSettingsPattern secondary_pattern = rule.secondary_pattern;
if (rule.secondary_pattern ==
ContentSettingsPattern::FromString("https://firstParty/*")) {
secondary_pattern = ContentSettingsPattern::FromString(
"[*.]" + GetOriginOrURL(frame).HostNoBrackets());
}

if (rule.primary_pattern.Matches(primary_url) &&
(secondary_pattern == ContentSettingsPattern::Wildcard() ||
secondary_pattern.Matches(secondary_url))) {
return rule.GetContentSetting();
}
}

// for cases which are third party resources and doesn't match any existing
// rules, block them by default
return CONTENT_SETTING_BLOCK;
}

bool BraveContentSettingsAgentImpl::IsBraveShieldsDown(
const blink::WebFrame* frame,
const GURL& secondary_url) {
Expand Down Expand Up @@ -259,10 +209,15 @@ BraveFarblingLevel BraveContentSettingsAgentImpl::GetBraveFarblingLevel() {

ContentSetting setting = CONTENT_SETTING_DEFAULT;
if (content_setting_rules_) {
setting = GetBraveContentSettingFromRules(
content_setting_rules_->brave_shields_rules,
content_setting_rules_->fingerprinting_rules, frame,
url::Origin(frame->GetDocument().GetSecurityOrigin()).GetURL());
if (IsBraveShieldsDown(
frame,
url::Origin(frame->GetDocument().GetSecurityOrigin()).GetURL())) {
setting = CONTENT_SETTING_ALLOW;
} else {
setting = GetBraveFPContentSettingFromRules(
content_setting_rules_->fingerprinting_rules,
GetOriginOrURL(frame));
}
}

if (setting == CONTENT_SETTING_BLOCK) {
Expand Down
5 changes: 0 additions & 5 deletions renderer/brave_content_settings_agent_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,6 @@ class BraveContentSettingsAgentImpl
const base::string16& details);

private:
ContentSetting GetFPContentSettingFromRules(
const ContentSettingsForOneType& rules,
const blink::WebFrame* frame,
const GURL& secondary_url);

bool IsBraveShieldsDown(
const blink::WebFrame* frame,
const GURL& secondary_url);
Expand Down