Skip to content

Commit

Permalink
HTTPS-Only Mode: Make HTTP allowlist per-profile instead of per-tab
Browse files Browse the repository at this point in the history
HTTPS-Only Mode has an allowlist that stores the user decisions when
the user clicks through an HTTPS-Only Mode interstitial. If the user
navigates to the same domain later, this allowlist is checked to decide
whether to allow the navigation over HTTP or to upgrade it to HTTPS.

The allowlist is currently per-tab, meaning it doesn't carry over to
other tabs. To fix this, this CL introduces a new KeyedService called
HttpsUpgradeService which replaces the current allowlist. The service
keeps track of allowlist decisions per-profile.

Future CLs will also make the following changes:
- Allowlist entries will be saved to content settings when not in
incognito mode (same as desktop).
- Allowlist entries will have a timeout (same as desktop)
- Test methods like SetHttpPortForTesting() will be moved from the tab
helper to the service so that different tabs can read the same values
at once. This will allow us to test the feature with multiple tabs.

Bug: 1302509
Change-Id: Ia6e11991400de50a6136d579c7fc90ed8833a468
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3633920
Reviewed-by: Ali Juma <ajuma@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Commit-Queue: Mustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001777}
  • Loading branch information
meacer authored and Chromium LUCI CQ committed May 10, 2022
1 parent 283f110 commit 791ebc7
Show file tree
Hide file tree
Showing 21 changed files with 318 additions and 103 deletions.
6 changes: 6 additions & 0 deletions ios/chrome/browser/https_upgrades/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,15 @@ source_set("https_upgrades") {
sources = [
"https_only_mode_upgrade_tab_helper.h",
"https_only_mode_upgrade_tab_helper.mm",
"https_upgrade_service_factory.h",
"https_upgrade_service_factory.mm",
"https_upgrade_service_impl.h",
"https_upgrade_service_impl.mm",
]
deps = [
"//base",
"//components/keyed_service/core",
"//components/keyed_service/ios",
"//components/prefs:prefs",
"//components/security_interstitials/core",
"//ios/chrome/browser:pref_names",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ class HttpsOnlyModeUpgradeTabHelper
void SetFallbackDelayForTesting(base::TimeDelta delay);
// Returns true if the upgrade timer is running.
bool IsTimerRunningForTesting() const;
// Clears the allowlist that contains domains allowed over HTTP.
void ClearAllowlistForTesting();

private:
HttpsOnlyModeUpgradeTabHelper(web::WebState* web_state,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,17 @@
#include "components/prefs/pref_service.h"
#include "components/security_interstitials/core/https_only_mode_metrics.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/https_upgrades/https_upgrade_service_factory.h"
#include "ios/chrome/browser/pref_names.h"
#import "ios/chrome/browser/prerender/prerender_service.h"
#import "ios/chrome/browser/prerender/prerender_service_factory.h"
#import "ios/components/security_interstitials/https_only_mode/https_only_mode_allowlist.h"
#import "ios/components/security_interstitials/https_only_mode/https_only_mode_blocking_page.h"
#include "ios/components/security_interstitials/https_only_mode/https_only_mode_container.h"
#import "ios/components/security_interstitials/https_only_mode/https_only_mode_container.h"
#import "ios/components/security_interstitials/https_only_mode/https_only_mode_controller_client.h"
#include "ios/components/security_interstitials/https_only_mode/https_only_mode_error.h"
#import "ios/components/security_interstitials/https_only_mode/https_only_mode_error.h"
#import "ios/components/security_interstitials/https_only_mode/https_upgrade_service.h"
#import "ios/web/public/navigation/navigation_context.h"
#include "ios/web/public/navigation/navigation_item.h"
#include "ios/web/public/navigation/navigation_manager.h"
Expand Down Expand Up @@ -102,6 +103,12 @@ void RecordUMA(Event event) {
return timer_.IsRunning();
}

void HttpsOnlyModeUpgradeTabHelper::ClearAllowlistForTesting() {
HttpsUpgradeService* service = HttpsUpgradeServiceFactory::GetForBrowserState(
web_state()->GetBrowserState());
service->ClearAllowlist();
}

bool HttpsOnlyModeUpgradeTabHelper::IsFakeHTTPSForTesting(
const GURL& url) const {
return url.IntPort() == https_port_for_testing_;
Expand All @@ -110,9 +117,9 @@ void RecordUMA(Event event) {
bool HttpsOnlyModeUpgradeTabHelper::IsHttpAllowedForUrl(const GURL& url) const {
// TODO(crbug.com/1302509): Allow HTTP for IP addresses when not running
// tests. If the URL is in the allowlist, don't show any warning.
HttpsOnlyModeAllowlist* allow_list =
HttpsOnlyModeAllowlist::FromWebState(web_state());
return allow_list->IsHttpAllowedForHost(url.host());
HttpsUpgradeService* service = HttpsUpgradeServiceFactory::GetForBrowserState(
web_state()->GetBrowserState());
return service->IsHttpAllowedForHost(url.host());
}

// static
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@
#include "base/test/task_environment.h"
#include "components/prefs/pref_service.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#include "ios/chrome/browser/https_upgrades/https_upgrade_service_factory.h"
#include "ios/chrome/browser/pref_names.h"
#import "ios/chrome/browser/prerender/fake_prerender_service.h"
#import "ios/chrome/browser/prerender/prerender_service.h"
#import "ios/chrome/browser/prerender/prerender_service_factory.h"
#include "ios/components/security_interstitials/https_only_mode/https_only_mode_allowlist.h"
#include "ios/components/security_interstitials/https_only_mode/https_only_mode_container.h"
#include "ios/components/security_interstitials/https_only_mode/https_upgrade_service.h"
#import "ios/web/public/navigation/web_state_policy_decider.h"
#import "ios/web/public/test/fakes/fake_navigation_manager.h"
#import "ios/web/public/test/fakes/fake_web_state.h"
Expand All @@ -30,25 +31,54 @@
return std::make_unique<FakePrerenderService>();
}

class FakeHttpsUpgradeService : public HttpsUpgradeService {
public:
bool IsHttpAllowedForHost(const std::string& host) const override {
return base::Contains(allowed_http_hosts_, host);
}

void AllowHttpForHost(const std::string& host) override {
allowed_http_hosts_.insert(host);
};

void ClearAllowlist() override { allowed_http_hosts_.clear(); }

private:
std::set<std::string> allowed_http_hosts_;
};

std::unique_ptr<KeyedService> BuildFakeHttpsUpgradeService(
web::BrowserState* context) {
return std::make_unique<FakeHttpsUpgradeService>();
}

class HttpsOnlyModeUpgradeTabHelperTest : public PlatformTest {
protected:
HttpsOnlyModeUpgradeTabHelperTest() {
TestChromeBrowserState::Builder builder;
builder.AddTestingFactory(PrerenderServiceFactory::GetInstance(),
base::BindRepeating(&BuildFakePrerenderService));
builder.AddTestingFactory(
HttpsUpgradeServiceFactory::GetInstance(),
base::BindRepeating(&BuildFakeHttpsUpgradeService));

browser_state_ = builder.Build();
web_state_.SetBrowserState(browser_state_.get());

HttpsOnlyModeUpgradeTabHelper::CreateForWebState(
&web_state_, browser_state_->GetPrefs());
HttpsOnlyModeContainer::CreateForWebState(&web_state_);
HttpsOnlyModeAllowlist::CreateForWebState(&web_state_);
allowlist_ = HttpsOnlyModeAllowlist::FromWebState(&web_state_);

browser_state_->GetPrefs()->SetBoolean(prefs::kHttpsOnlyModeEnabled, true);
}

void TearDown() override {
HttpsUpgradeService* service =
HttpsUpgradeServiceFactory::GetForBrowserState(
web_state_.GetBrowserState());
service->ClearAllowlist();
}

// Helper function that calls into WebState::ShouldAllowResponse with the
// given |url| and |for_main_frame|, waits for the callback with the decision
// to be called, and returns the decision.
Expand All @@ -75,13 +105,10 @@
return policy_decision;
}

HttpsOnlyModeAllowlist* allowlist() { return allowlist_; }

base::HistogramTester histogram_tester_;
web::FakeWebState web_state_;

private:
HttpsOnlyModeAllowlist* allowlist_;
std::unique_ptr<ChromeBrowserState> browser_state_;
base::test::TaskEnvironment task_environment_;
};
Expand Down Expand Up @@ -116,7 +143,9 @@
.ShouldAllowNavigation());

// Allowlisted hosts shouldn't be blocked.
allowlist()->AllowHttpForHost("example.com");
HttpsUpgradeService* service = HttpsUpgradeServiceFactory::GetForBrowserState(
web_state_.GetBrowserState());
service->AllowHttpForHost("example.com");
EXPECT_TRUE(ShouldAllowResponseUrl(http_url, /*main_frame=*/true)
.ShouldAllowNavigation());
}
Expand Down
42 changes: 42 additions & 0 deletions ios/chrome/browser/https_upgrades/https_upgrade_service_factory.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef IOS_CHROME_BROWSER_HTTPS_UPGRADES_HTTPS_UPGRADE_SERVICE_FACTORY_H_
#define IOS_CHROME_BROWSER_HTTPS_UPGRADES_HTTPS_UPGRADE_SERVICE_FACTORY_H_

#include <memory>

#include "base/no_destructor.h"
#include "components/keyed_service/ios/browser_state_keyed_service_factory.h"
#include "ios/chrome/browser/browser_state/browser_state_otr_helper.h"
#import "ios/chrome/browser/https_upgrades/https_upgrade_service_impl.h"

// Singleton that owns all HttpsUpgradeService and associates them with
// ChromeBrowserState.
class HttpsUpgradeServiceFactory : public BrowserStateKeyedServiceFactory {
public:
static HttpsUpgradeService* GetForBrowserState(
web::BrowserState* browser_state);
static HttpsUpgradeServiceFactory* GetInstance();

HttpsUpgradeServiceFactory(const HttpsUpgradeServiceFactory&) = delete;
HttpsUpgradeServiceFactory& operator=(const HttpsUpgradeServiceFactory&) =
delete;

private:
friend class base::NoDestructor<HttpsUpgradeServiceFactory>;

HttpsUpgradeServiceFactory();
~HttpsUpgradeServiceFactory() override;

// BrowserStateKeyedServiceFactory implementation.
std::unique_ptr<KeyedService> BuildServiceInstanceFor(
web::BrowserState* context) const override;
web::BrowserState* GetBrowserStateToUse(
web::BrowserState* context) const override;

bool ServiceIsNULLWhileTesting() const override;
};

#endif // IOS_CHROME_BROWSER_HTTPS_UPGRADES_HTTPS_UPGRADE_SERVICE_FACTORY_H_
48 changes: 48 additions & 0 deletions ios/chrome/browser/https_upgrades/https_upgrade_service_factory.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#import "ios/chrome/browser/https_upgrades/https_upgrade_service_factory.h"

#include "base/no_destructor.h"
#include "components/keyed_service/ios/browser_state_dependency_manager.h"
#include "ios/web/public/browser_state.h"

#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif

// static
HttpsUpgradeService* HttpsUpgradeServiceFactory::GetForBrowserState(
web::BrowserState* browser_state) {
return static_cast<HttpsUpgradeService*>(
GetInstance()->GetServiceForBrowserState(browser_state, true));
}

// static
HttpsUpgradeServiceFactory* HttpsUpgradeServiceFactory::GetInstance() {
static base::NoDestructor<HttpsUpgradeServiceFactory> instance;
return instance.get();
}

HttpsUpgradeServiceFactory::HttpsUpgradeServiceFactory()
: BrowserStateKeyedServiceFactory(
"HttpsUpgradeService",
BrowserStateDependencyManager::GetInstance()) {}

HttpsUpgradeServiceFactory::~HttpsUpgradeServiceFactory() {}

std::unique_ptr<KeyedService>
HttpsUpgradeServiceFactory::BuildServiceInstanceFor(
web::BrowserState* context) const {
return std::make_unique<HttpsUpgradeServiceImpl>(context);
}

web::BrowserState* HttpsUpgradeServiceFactory::GetBrowserStateToUse(
web::BrowserState* context) const {
return GetBrowserStateOwnInstanceInIncognito(context);
}

bool HttpsUpgradeServiceFactory::ServiceIsNULLWhileTesting() const {
return false;
}
40 changes: 40 additions & 0 deletions ios/chrome/browser/https_upgrades/https_upgrade_service_impl.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef IOS_CHROME_BROWSER_HTTPS_UPGRADES_HTTPS_UPGRADE_SERVICE_IMPL_H_
#define IOS_CHROME_BROWSER_HTTPS_UPGRADES_HTTPS_UPGRADE_SERVICE_IMPL_H_

#include <set>
#include <string>

#include "components/keyed_service/core/keyed_service.h"
#include "ios/components/security_interstitials/https_only_mode/https_upgrade_service.h"

namespace web {
class BrowserState;
}

// HttpsUpgradeServiceImpl tracks the allowlist decisions for HTTPS-Only mode.
// Decisions are scoped to the host.
class HttpsUpgradeServiceImpl : public HttpsUpgradeService {
public:
HttpsUpgradeServiceImpl(web::BrowserState* context);
~HttpsUpgradeServiceImpl() override;

// Returns whether |host| can be loaded over http://.
bool IsHttpAllowedForHost(const std::string& host) const override;

// Allows future navigations to |host| over http://.
void AllowHttpForHost(const std::string& host) override;

void ClearAllowlist() override;

private:
// Set of allowlisted hostnames.
std::set<std::string> allowed_http_hosts_;

web::BrowserState* context_;
};

#endif // IOS_CHROME_BROWSER_HTTPS_UPGRADES_HTTPS_UPGRADE_SERVICE_IMPL_H_
32 changes: 32 additions & 0 deletions ios/chrome/browser/https_upgrades/https_upgrade_service_impl.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#import "ios/chrome/browser/https_upgrades/https_upgrade_service_impl.h"

#include "base/containers/contains.h"
#include "ios/chrome/browser/browser_state/chrome_browser_state.h"

#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif

HttpsUpgradeServiceImpl::HttpsUpgradeServiceImpl(web::BrowserState* context)
: context_(context) {
DCHECK(context_);
}

HttpsUpgradeServiceImpl::~HttpsUpgradeServiceImpl() = default;

bool HttpsUpgradeServiceImpl::IsHttpAllowedForHost(
const std::string& host) const {
return base::Contains(allowed_http_hosts_, host);
}

void HttpsUpgradeServiceImpl::AllowHttpForHost(const std::string& host) {
allowed_http_hosts_.insert(host);
}

void HttpsUpgradeServiceImpl::ClearAllowlist() {
allowed_http_hosts_.clear();
}
2 changes: 0 additions & 2 deletions ios/chrome/browser/tabs/tab_helper_util.mm
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@
#import "ios/chrome/browser/web/session_state/web_session_state_tab_helper.h"
#import "ios/chrome/browser/web/web_performance_metrics/web_performance_metrics_tab_helper.h"
#import "ios/chrome/browser/webui/net_export_tab_helper.h"
#import "ios/components/security_interstitials/https_only_mode/https_only_mode_allowlist.h"
#import "ios/components/security_interstitials/https_only_mode/https_only_mode_container.h"
#import "ios/components/security_interstitials/ios_blocking_page_tab_helper.h"
#import "ios/components/security_interstitials/lookalikes/lookalike_url_container.h"
Expand Down Expand Up @@ -239,7 +238,6 @@ void AttachTabHelpers(web::WebState* web_state, bool for_prerender) {
HttpsOnlyModeUpgradeTabHelper::CreateForWebState(web_state,
browser_state->GetPrefs());
HttpsOnlyModeContainer::CreateForWebState(web_state);
HttpsOnlyModeAllowlist::CreateForWebState(web_state);

if (IsWebChannelsEnabled()) {
FollowTabHelper::CreateForWebState(web_state);
Expand Down
1 change: 1 addition & 0 deletions ios/chrome/browser/web/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ source_set("web_internal") {
"//ios/chrome/browser/browser_state",
"//ios/chrome/browser/content_settings",
"//ios/chrome/browser/follow",
"//ios/chrome/browser/https_upgrades",
"//ios/chrome/browser/link_to_text",
"//ios/chrome/browser/ntp",
"//ios/chrome/browser/passwords",
Expand Down
6 changes: 5 additions & 1 deletion ios/chrome/browser/web/chrome_web_client.mm
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "ios/chrome/browser/chrome_url_constants.h"
#include "ios/chrome/browser/content_settings/host_content_settings_map_factory.h"
#import "ios/chrome/browser/follow/follow_java_script_feature.h"
#import "ios/chrome/browser/https_upgrades/https_upgrade_service_factory.h"
#include "ios/chrome/browser/ios_chrome_main_parts.h"
#import "ios/chrome/browser/link_to_text/link_to_text_java_script_feature.h"
#include "ios/chrome/browser/ntp/browser_policy_new_tab_page_rewriter.h"
Expand Down Expand Up @@ -60,6 +61,7 @@
#import "ios/components/security_interstitials/https_only_mode/https_only_mode_container.h"
#import "ios/components/security_interstitials/https_only_mode/https_only_mode_controller_client.h"
#import "ios/components/security_interstitials/https_only_mode/https_only_mode_error.h"
#import "ios/components/security_interstitials/https_only_mode/https_upgrade_service.h"
#import "ios/components/security_interstitials/ios_blocking_page_tab_helper.h"
#import "ios/components/security_interstitials/lookalikes/lookalike_url_blocking_page.h"
#import "ios/components/security_interstitials/lookalikes/lookalike_url_container.h"
Expand Down Expand Up @@ -162,11 +164,13 @@
// Fetch the HTTP URL from the container.
HttpsOnlyModeContainer* container =
HttpsOnlyModeContainer::FromWebState(web_state);
HttpsUpgradeService* service = HttpsUpgradeServiceFactory::GetForBrowserState(
web_state->GetBrowserState());

// Construct the blocking page and associate it with the WebState.
std::unique_ptr<security_interstitials::IOSSecurityInterstitialPage> page =
std::make_unique<HttpsOnlyModeBlockingPage>(
web_state, container->http_url(),
web_state, container->http_url(), service,
std::make_unique<HttpsOnlyModeControllerClient>(
web_state, container->http_url(),
GetApplicationContext()->GetApplicationLocale()));
Expand Down
1 change: 1 addition & 0 deletions ios/chrome/browser/web/https_only_mode_app_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
+ (void)useFakeHTTPSForTesting:(bool)useFakeHTTPSForTesting;
+ (void)setFallbackDelayForTesting:(int)fallbackDelayInMilliseconds;
+ (BOOL)isTimerRunning;
+ (void)clearAllowlist;

@end

Expand Down

0 comments on commit 791ebc7

Please sign in to comment.