Commit d0637d14 authored by Charles Harrison's avatar Charles Harrison Committed by Commit Bot

[subresource_filter] Add a finch param to disable using rules from the ruleset

This allows us to remotely turn off filtering rules while still enabling
other features like the strengthened popup blocker.

Bug: 737737
Change-Id: Ia77acc7bdb66009fae7204a259ce571bd680ba9f
Reviewed-on: https://chromium-review.googlesource.com/553597Reviewed-by: default avatarShivani Sharma <shivanisha@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488256}
parent f3dece92
...@@ -1138,6 +1138,18 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, ...@@ -1138,6 +1138,18 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
observer.Wait(); observer.Wait();
} }
IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
DisableRuleset_SubresourceAllowed) {
Configuration config = Configuration::MakePresetForLiveRunOnPhishingSites();
config.activation_options.should_disable_ruleset_rules = true;
ResetConfiguration(std::move(config));
GURL url(GetTestUrl("subresource_filter/frame_with_included_script.html"));
ConfigureAsPhishingURL(url);
ui_test_utils::NavigateToURL(browser(), url);
EXPECT_TRUE(WasParsedScriptElementLoaded(web_contents()->GetMainFrame()));
}
IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
NoConfiguration_AllowCreatingNewWindows) { NoConfiguration_AllowCreatingNewWindows) {
base::HistogramTester tester; base::HistogramTester tester;
...@@ -1183,10 +1195,8 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, BlockCreatingNewWindows) { ...@@ -1183,10 +1195,8 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, BlockCreatingNewWindows) {
EXPECT_TRUE(content::ExecuteScriptAndExtractBool(web_contents, "openWindow()", EXPECT_TRUE(content::ExecuteScriptAndExtractBool(web_contents, "openWindow()",
&opened_window)); &opened_window));
EXPECT_FALSE(opened_window); EXPECT_FALSE(opened_window);
// Do not force the UI if the popup was the only thing disallowed. The popup
// UI is good enough.
tester.ExpectBucketCount(kSubresourceFilterActionsHistogram, kActionUIShown, tester.ExpectBucketCount(kSubresourceFilterActionsHistogram, kActionUIShown,
0); 1);
// Make sure the popup UI was shown. // Make sure the popup UI was shown.
EXPECT_TRUE(TabSpecificContentSettings::FromWebContents(web_contents) EXPECT_TRUE(TabSpecificContentSettings::FromWebContents(web_contents)
->IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS)); ->IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS));
...@@ -1223,10 +1233,8 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, BlockOpenURLFromTab) { ...@@ -1223,10 +1233,8 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, BlockOpenURLFromTab) {
content::WebContents* web_contents = content::WebContents* web_contents =
browser()->tab_strip_model()->GetActiveWebContents(); browser()->tab_strip_model()->GetActiveWebContents();
EXPECT_TRUE(content::ExecuteScript(web_contents, "openWindow()")); EXPECT_TRUE(content::ExecuteScript(web_contents, "openWindow()"));
// Do not force the UI if the popup was the only thing disallowed. The popup
// UI is good enough.
tester.ExpectBucketCount(kSubresourceFilterActionsHistogram, kActionUIShown, tester.ExpectBucketCount(kSubresourceFilterActionsHistogram, kActionUIShown,
0); 1);
EXPECT_TRUE(TabSpecificContentSettings::FromWebContents(web_contents) EXPECT_TRUE(TabSpecificContentSettings::FromWebContents(web_contents)
->IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS)); ->IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS));
......
...@@ -147,3 +147,16 @@ TEST_F(SubresourceFilterTest, SimpleDisallowedLoad_WithObserver) { ...@@ -147,3 +147,16 @@ TEST_F(SubresourceFilterTest, SimpleDisallowedLoad_WithObserver) {
EXPECT_EQ(subresource_filter::LoadPolicy::DISALLOW, EXPECT_EQ(subresource_filter::LoadPolicy::DISALLOW,
observer.GetSubframeLoadPolicy(disallowed_url).value()); observer.GetSubframeLoadPolicy(disallowed_url).value());
} }
TEST_F(SubresourceFilterTest, DisableRuleset_SubframeAllowed) {
subresource_filter::Configuration config(
subresource_filter::ActivationLevel::ENABLED,
subresource_filter::ActivationScope::ALL_SITES);
config.activation_options.should_disable_ruleset_rules = true;
scoped_configuration().ResetConfiguration(std::move(config));
GURL url("https://a.test");
ConfigureAsSubresourceFilterOnlyURL(url);
SimulateNavigateAndCommit(url, main_rfh());
EXPECT_TRUE(CreateAndNavigateDisallowedSubframe(main_rfh()));
}
...@@ -138,13 +138,14 @@ ActivationStateComputingNavigationThrottle::filter() const { ...@@ -138,13 +138,14 @@ ActivationStateComputingNavigationThrottle::filter() const {
// when activation IPCs are not sent to the render process. // when activation IPCs are not sent to the render process.
std::unique_ptr<AsyncDocumentSubresourceFilter> std::unique_ptr<AsyncDocumentSubresourceFilter>
ActivationStateComputingNavigationThrottle::ReleaseFilter() { ActivationStateComputingNavigationThrottle::ReleaseFilter() {
return will_send_activation_to_renderer_ ? std::move(async_filter_) : nullptr; return could_send_activation_to_renderer_ ? std::move(async_filter_)
: nullptr;
} }
void ActivationStateComputingNavigationThrottle:: void ActivationStateComputingNavigationThrottle::
WillSendActivationToRenderer() { CouldSendActivationToRenderer() {
DCHECK(async_filter_); DCHECK(async_filter_);
will_send_activation_to_renderer_ = true; could_send_activation_to_renderer_ = true;
} }
} // namespace subresource_filter } // namespace subresource_filter
...@@ -71,7 +71,7 @@ class ActivationStateComputingNavigationThrottle ...@@ -71,7 +71,7 @@ class ActivationStateComputingNavigationThrottle
AsyncDocumentSubresourceFilter* filter() const; AsyncDocumentSubresourceFilter* filter() const;
void WillSendActivationToRenderer(); void CouldSendActivationToRenderer();
private: private:
void OnActivationStateComputed(ActivationState state); void OnActivationStateComputed(ActivationState state);
...@@ -95,11 +95,12 @@ class ActivationStateComputingNavigationThrottle ...@@ -95,11 +95,12 @@ class ActivationStateComputingNavigationThrottle
// Callback to be run in the destructor. // Callback to be run in the destructor.
base::OnceClosure destruction_closure_; base::OnceClosure destruction_closure_;
// Becomes true when the throttle manager reaches ReadyToCommitNavigation and // Can become true when the throttle manager reaches ReadyToCommitNavigation.
// sends an activation IPC to the render process. Makes sure a caller cannot // Makes sure a caller cannot take ownership of the subresource filter unless
// take ownership of the subresource filter unless an activation IPC is sent // the throttle has reached this point. After this point the throttle manager
// to the renderer. // can send an activation IPC to the render process. Note that an IPC is not
bool will_send_activation_to_renderer_ = false; // always sent in case this activation is ignoring ruleset rules.
bool could_send_activation_to_renderer_ = false;
base::WeakPtrFactory<ActivationStateComputingNavigationThrottle> base::WeakPtrFactory<ActivationStateComputingNavigationThrottle>
weak_ptr_factory_; weak_ptr_factory_;
......
...@@ -176,7 +176,7 @@ class ActivationStateComputingNavigationThrottleTest ...@@ -176,7 +176,7 @@ class ActivationStateComputingNavigationThrottleTest
return; return;
ASSERT_EQ(navigation_handle, test_throttle_->navigation_handle()); ASSERT_EQ(navigation_handle, test_throttle_->navigation_handle());
if (test_throttle_->filter()) if (test_throttle_->filter())
test_throttle_->WillSendActivationToRenderer(); test_throttle_->CouldSendActivationToRenderer();
if (auto filter = test_throttle_->ReleaseFilter()) { if (auto filter = test_throttle_->ReleaseFilter()) {
EXPECT_NE(ActivationLevel::DISABLED, EXPECT_NE(ActivationLevel::DISABLED,
......
...@@ -105,6 +105,10 @@ bool ContentSubresourceFilterDriverFactory::AllowStrongPopupBlocking() { ...@@ -105,6 +105,10 @@ bool ContentSubresourceFilterDriverFactory::AllowStrongPopupBlocking() {
return activation_options_.should_strengthen_popup_blocker; return activation_options_.should_strengthen_popup_blocker;
} }
bool ContentSubresourceFilterDriverFactory::AllowRulesetRules() {
return !activation_options_.should_disable_ruleset_rules;
}
void ContentSubresourceFilterDriverFactory::DidStartNavigation( void ContentSubresourceFilterDriverFactory::DidStartNavigation(
content::NavigationHandle* navigation_handle) { content::NavigationHandle* navigation_handle) {
if (navigation_handle->IsInMainFrame() && if (navigation_handle->IsInMainFrame() &&
......
...@@ -70,6 +70,7 @@ class ContentSubresourceFilterDriverFactory ...@@ -70,6 +70,7 @@ class ContentSubresourceFilterDriverFactory
// ContentSubresourceFilterThrottleManager::Delegate: // ContentSubresourceFilterThrottleManager::Delegate:
void OnFirstSubresourceLoadDisallowed() override; void OnFirstSubresourceLoadDisallowed() override;
bool AllowStrongPopupBlocking() override; bool AllowStrongPopupBlocking() override;
bool AllowRulesetRules() override;
ContentSubresourceFilterThrottleManager* throttle_manager() { ContentSubresourceFilterThrottleManager* throttle_manager() {
return throttle_manager_.get(); return throttle_manager_.get();
......
...@@ -35,6 +35,10 @@ bool ContentSubresourceFilterThrottleManager::Delegate:: ...@@ -35,6 +35,10 @@ bool ContentSubresourceFilterThrottleManager::Delegate::
return false; return false;
} }
bool ContentSubresourceFilterThrottleManager::Delegate::AllowRulesetRules() {
return true;
}
ContentSubresourceFilterThrottleManager:: ContentSubresourceFilterThrottleManager::
ContentSubresourceFilterThrottleManager( ContentSubresourceFilterThrottleManager(
Delegate* delegate, Delegate* delegate,
...@@ -98,12 +102,17 @@ void ContentSubresourceFilterThrottleManager::ReadyToCommitNavigation( ...@@ -98,12 +102,17 @@ void ContentSubresourceFilterThrottleManager::ReadyToCommitNavigation(
"ContentSubresourceFilterThrottleManager::ReadyToCommitNavigation", "ContentSubresourceFilterThrottleManager::ReadyToCommitNavigation",
"activation_state", filter->activation_state().ToTracedValue()); "activation_state", filter->activation_state().ToTracedValue());
throttle->WillSendActivationToRenderer(); // Only send the IPC to the renderer if not actively ignoring rules from our
// ruleset. Note, if we ever want to do anything more complex in the renderer
content::RenderFrameHost* frame_host = // (other than just consume the rules), we will likely have to find a
navigation_handle->GetRenderFrameHost(); // different solution here.
frame_host->Send(new SubresourceFilterMsg_ActivateForNextCommittedLoad( throttle->CouldSendActivationToRenderer();
frame_host->GetRoutingID(), filter->activation_state())); if (delegate_->AllowRulesetRules()) {
content::RenderFrameHost* frame_host =
navigation_handle->GetRenderFrameHost();
frame_host->Send(new SubresourceFilterMsg_ActivateForNextCommittedLoad(
frame_host->GetRoutingID(), filter->activation_state()));
}
} }
void ContentSubresourceFilterThrottleManager::DidFinishNavigation( void ContentSubresourceFilterThrottleManager::DidFinishNavigation(
...@@ -243,11 +252,14 @@ bool ContentSubresourceFilterThrottleManager::ShouldDisallowNewWindow( ...@@ -243,11 +252,14 @@ bool ContentSubresourceFilterThrottleManager::ShouldDisallowNewWindow(
// isTrusted bit set to false. This bit is set to true if the event is // isTrusted bit set to false. This bit is set to true if the event is
// generated via a user action. See docs: // generated via a user action. See docs:
// https://developer.mozilla.org/en-US/docs/Web/API/Event/isTrusted // https://developer.mozilla.org/en-US/docs/Web/API/Event/isTrusted
bool should_block = true;
if (open_url_params) { if (open_url_params) {
return open_url_params->triggering_event_info == should_block = open_url_params->triggering_event_info ==
blink::WebTriggeringEventInfo::kFromUntrustedEvent; blink::WebTriggeringEventInfo::kFromUntrustedEvent;
} }
return true; if (should_block)
delegate_->OnFirstSubresourceLoadDisallowed();
return should_block;
} }
std::unique_ptr<SubframeNavigationFilteringThrottle> std::unique_ptr<SubframeNavigationFilteringThrottle>
...@@ -256,6 +268,8 @@ ContentSubresourceFilterThrottleManager:: ...@@ -256,6 +268,8 @@ ContentSubresourceFilterThrottleManager::
content::NavigationHandle* navigation_handle) { content::NavigationHandle* navigation_handle) {
if (navigation_handle->IsInMainFrame()) if (navigation_handle->IsInMainFrame())
return nullptr; return nullptr;
if (!delegate_->AllowRulesetRules())
return nullptr;
AsyncDocumentSubresourceFilter* parent_filter = AsyncDocumentSubresourceFilter* parent_filter =
GetParentFrameFilter(navigation_handle); GetParentFrameFilter(navigation_handle);
return parent_filter ? base::MakeUnique<SubframeNavigationFilteringThrottle>( return parent_filter ? base::MakeUnique<SubframeNavigationFilteringThrottle>(
......
...@@ -58,7 +58,13 @@ class ContentSubresourceFilterThrottleManager ...@@ -58,7 +58,13 @@ class ContentSubresourceFilterThrottleManager
// The embedder may be interested in displaying UI to the user when the // The embedder may be interested in displaying UI to the user when the
// first load is disallowed for a given page load. // first load is disallowed for a given page load.
virtual void OnFirstSubresourceLoadDisallowed() {} virtual void OnFirstSubresourceLoadDisallowed() {}
// Whether the stronger version of the popup blocker is enabled for this
// page load.
virtual bool AllowStrongPopupBlocking(); virtual bool AllowStrongPopupBlocking();
// Whether we should be using ruleset rules for this page load.
virtual bool AllowRulesetRules();
}; };
ContentSubresourceFilterThrottleManager( ContentSubresourceFilterThrottleManager(
......
...@@ -176,6 +176,9 @@ Configuration ParseExperimentalConfiguration( ...@@ -176,6 +176,9 @@ Configuration ParseExperimentalConfiguration(
ParseBool(TakeVariationParamOrReturnEmpty( ParseBool(TakeVariationParamOrReturnEmpty(
params, kStrengthenPopupBlockerParameterName)); params, kStrengthenPopupBlockerParameterName));
configuration.activation_options.should_disable_ruleset_rules =
ParseBool(TakeVariationParamOrReturnEmpty(params, kDisableRulesetRules));
// GeneralSettings: // GeneralSettings:
configuration.general_settings.ruleset_flavor = configuration.general_settings.ruleset_flavor =
TakeVariationParamOrReturnEmpty(params, kRulesetFlavorParameterName); TakeVariationParamOrReturnEmpty(params, kRulesetFlavorParameterName);
...@@ -264,6 +267,7 @@ const char kPerformanceMeasurementRateParameterName[] = ...@@ -264,6 +267,7 @@ const char kPerformanceMeasurementRateParameterName[] =
const char kSuppressNotificationsParameterName[] = "suppress_notifications"; const char kSuppressNotificationsParameterName[] = "suppress_notifications";
const char kWhitelistSiteOnReloadParameterName[] = "whitelist_site_on_reload"; const char kWhitelistSiteOnReloadParameterName[] = "whitelist_site_on_reload";
const char kStrengthenPopupBlockerParameterName[] = "strengthen_popup_blocker"; const char kStrengthenPopupBlockerParameterName[] = "strengthen_popup_blocker";
const char kDisableRulesetRules[] = "disable_ruleset_rules";
const char kRulesetFlavorParameterName[] = "ruleset_flavor"; const char kRulesetFlavorParameterName[] = "ruleset_flavor";
...@@ -316,6 +320,7 @@ bool Configuration::operator==(const Configuration& rhs) const { ...@@ -316,6 +320,7 @@ bool Configuration::operator==(const Configuration& rhs) const {
config.activation_options.should_whitelist_site_on_reload, config.activation_options.should_whitelist_site_on_reload,
config.activation_options.should_suppress_notifications, config.activation_options.should_suppress_notifications,
config.activation_options.should_strengthen_popup_blocker, config.activation_options.should_strengthen_popup_blocker,
config.activation_options.should_disable_ruleset_rules,
config.general_settings.ruleset_flavor); config.general_settings.ruleset_flavor);
}; };
return tie(*this) == tie(rhs); return tie(*this) == tie(rhs);
...@@ -349,6 +354,8 @@ std::unique_ptr<base::trace_event::TracedValue> Configuration::ToTracedValue() ...@@ -349,6 +354,8 @@ std::unique_ptr<base::trace_event::TracedValue> Configuration::ToTracedValue()
activation_options.should_whitelist_site_on_reload); activation_options.should_whitelist_site_on_reload);
value->SetBoolean("should_strengthen_popup_blocker", value->SetBoolean("should_strengthen_popup_blocker",
activation_options.should_strengthen_popup_blocker); activation_options.should_strengthen_popup_blocker);
value->SetBoolean("should_disable_ruleset_rules",
activation_options.should_disable_ruleset_rules);
value->SetString("ruleset_flavor", value->SetString("ruleset_flavor",
StreamToString(general_settings.ruleset_flavor)); StreamToString(general_settings.ruleset_flavor));
return value; return value;
......
...@@ -88,6 +88,10 @@ struct Configuration { ...@@ -88,6 +88,10 @@ struct Configuration {
// Whether to apply a more powerful popup blocker on pages with activation. // Whether to apply a more powerful popup blocker on pages with activation.
bool should_strengthen_popup_blocker = false; bool should_strengthen_popup_blocker = false;
// Whether to disable rules from the ruleset. In practice this might be used
// if e.g. only popup blocking behavior is desired.
bool should_disable_ruleset_rules = false;
}; };
// General settings that apply outside of the scope of a navigation. // General settings that apply outside of the scope of a navigation.
...@@ -208,6 +212,8 @@ extern const char kWhitelistSiteOnReloadParameterName[]; ...@@ -208,6 +212,8 @@ extern const char kWhitelistSiteOnReloadParameterName[];
extern const char kStrengthenPopupBlockerParameterName[]; extern const char kStrengthenPopupBlockerParameterName[];
extern const char kDisableRulesetRules[];
extern const char kRulesetFlavorParameterName[]; extern const char kRulesetFlavorParameterName[];
extern const char kEnablePresetsParameterName[]; extern const char kEnablePresetsParameterName[];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment