Skip to content

Commit

Permalink
[WebLayer] Add support for Gaia-keyed safe browsing realtime URL lookups
Browse files Browse the repository at this point in the history
This CL adds support for Gaia-keyed safe browsing realtime URL lookups
to WebLayer via a //weblayer-specific implementation of
SafeBrowsingTokenFetcher that obtains access tokens from the WebLayer
embedder. That implementation follows the design of the
SafeBrowsingTokenFetcher implementation that Chrome uses, relying on the
shared SafeBrowsingTokenFetchTracker helper class to time out access
token requests after a given delay.

This CL supplies a SafeBrowsingTokenFetcherImpl instance to
RealTimeUrlLookupService but does not actually enable access token
fetches in the context of WebLayer, as the specific flow for enabling
this functionality is still under consideration.

This CL also adds tests of the new functionality:
- unittests of SafeBrowsingTokenFetcherImpl
- browsertests that verify that safe browsing access token fetches
  occur/don't occur as part of navigations as expected in various
  configurations of safe browsing.

Bug: 1080748
Change-Id: If39c59f44dc469fdd6a22ae4dfa5c2d48e03e158
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2652466
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Xinghui Lu <xinghuilu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#848040}
  • Loading branch information
colinblundell authored and Chromium LUCI CQ committed Jan 28, 2021
1 parent 6a5d3a3 commit 5982efd
Show file tree
Hide file tree
Showing 8 changed files with 590 additions and 7 deletions.
2 changes: 2 additions & 0 deletions weblayer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,8 @@ source_set("weblayer_lib_base") {
"browser/safe_browsing/safe_browsing_subresource_helper.h",
"browser/safe_browsing/safe_browsing_tab_observer.cc",
"browser/safe_browsing/safe_browsing_tab_observer.h",
"browser/safe_browsing/safe_browsing_token_fetcher_impl.cc",
"browser/safe_browsing/safe_browsing_token_fetcher_impl.h",
"browser/safe_browsing/safe_browsing_ui_manager.cc",
"browser/safe_browsing/safe_browsing_ui_manager.h",
"browser/safe_browsing/url_checker_delegate_impl.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
#include "weblayer/browser/browser_context_impl.h"
#include "weblayer/browser/browser_process.h"
#include "weblayer/browser/feature_list_creator.h"
#include "weblayer/browser/profile_impl.h"
#include "weblayer/browser/safe_browsing/safe_browsing_service.h"
#include "weblayer/browser/safe_browsing/safe_browsing_token_fetcher_impl.h"
#include "weblayer/browser/verdict_cache_manager_factory.h"

namespace weblayer {
Expand Down Expand Up @@ -53,10 +55,14 @@ KeyedService* RealTimeUrlLookupServiceFactory::BuildServiceInstanceFor(
// History sync is never enabled in WebLayer.
base::BindRepeating([]() { return false; }),
static_cast<BrowserContextImpl*>(context)->pref_service(),
// TODO(crbug.com/1080748): Bring up token fetching for URL lookups and
// configure the following two parameters accordingly.
nullptr /* token_fetcher */,
base::BindRepeating([](bool) { return false; }),
std::make_unique<SafeBrowsingTokenFetcherImpl>(base::BindRepeating(
&ProfileImpl::access_token_fetch_delegate,
base::Unretained(ProfileImpl::FromBrowserContext(context)))),
// TODO(crbug.com/1171215): Change this to production mechanism for
// enabling Gaia-keyed URL lookups once that mechanism is determined.
base::BindRepeating(&RealTimeUrlLookupServiceFactory::
access_token_fetches_enabled_for_testing,
base::Unretained(this)),
safe_browsing::GetProfileManagementStatus(nullptr),
false /* is_under_advanced_protection */,
static_cast<BrowserContextImpl*>(context)->IsOffTheRecord(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ class RealTimeUrlLookupServiceFactory
// Get the singleton instance.
static RealTimeUrlLookupServiceFactory* GetInstance();

// TODO(crbug.com/1171215): Remove this once browsertests can enable this
// functionality via the production mechanism for doing so.
void set_access_token_fetches_enabled_for_testing() {
access_token_fetches_enabled_for_testing_ = true;
}

private:
friend struct base::DefaultSingletonTraits<RealTimeUrlLookupServiceFactory>;

Expand All @@ -46,6 +52,17 @@ class RealTimeUrlLookupServiceFactory
// BrowserContextKeyedServiceFactory:
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* context) const override;

// TODO(crbug.com/1171215): Remove this once browsertests can enable this
// functionality via the production mechanism for doing so.
bool access_token_fetches_enabled_for_testing(
bool user_has_enabled_enhanced_protection) const {
return access_token_fetches_enabled_for_testing_;
}

// TODO(crbug.com/1171215): Remove this once browsertests can enable this
// functionality via the production mechanism for doing so.
bool access_token_fetches_enabled_for_testing_ = false;
};

} // namespace weblayer
Expand Down
133 changes: 133 additions & 0 deletions weblayer/browser/safe_browsing/safe_browsing_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "components/prefs/pref_service.h"
#include "components/safe_browsing/android/safe_browsing_api_handler.h"
#include "components/safe_browsing/content/base_blocking_page.h"
#include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h"
#include "components/safe_browsing/core/db/v4_protocol_manager_util.h"
#include "components/security_interstitials/content/security_interstitial_page.h"
#include "components/security_interstitials/content/security_interstitial_tab_helper.h"
Expand All @@ -17,12 +18,15 @@
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/test_utils.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "weblayer/browser/browser_context_impl.h"
#include "weblayer/browser/browser_impl.h"
#include "weblayer/browser/profile_impl.h"
#include "weblayer/browser/safe_browsing/real_time_url_lookup_service_factory.h"
#include "weblayer/browser/safe_browsing/safe_browsing_blocking_page.h"
#include "weblayer/browser/tab_impl.h"
#include "weblayer/public/google_account_access_token_fetch_delegate.h"
#include "weblayer/public/navigation.h"
#include "weblayer/public/navigation_controller.h"
#include "weblayer/public/navigation_observer.h"
Expand All @@ -37,6 +41,43 @@ namespace weblayer {

namespace {

// Implementation of GoogleAccountAccessTokenFetchDelegate used to exercise safe
// browsing access token fetches.
class TestAccessTokenFetchDelegate
: public GoogleAccountAccessTokenFetchDelegate {
public:
TestAccessTokenFetchDelegate() = default;
~TestAccessTokenFetchDelegate() override = default;

// GoogleAccountAccessTokenFetchDelegate:
void FetchAccessToken(const std::set<std::string>& scopes,
OnTokenFetchedCallback callback) override {
has_received_request_ = true;
scopes_from_most_recent_request_ = scopes;

if (should_respond_to_request_) {
std::move(callback).Run("token");
} else {
outstanding_request_ = std::move(callback);
}
}

void set_should_respond_to_request(bool should_respond) {
should_respond_to_request_ = should_respond;
}

bool has_received_request() { return has_received_request_; }
const std::set<std::string>& scopes_from_most_recent_request() {
return scopes_from_most_recent_request_;
}

private:
bool should_respond_to_request_ = false;
bool has_received_request_ = false;
std::set<std::string> scopes_from_most_recent_request_;
OnTokenFetchedCallback outstanding_request_;
};

// Observer customized for safe browsing navigation failures.
class SafeBrowsingErrorNavigationObserver : public NavigationObserver {
public:
Expand Down Expand Up @@ -125,11 +166,24 @@ class SafeBrowsingBrowserTest : public WebLayerBrowserTest {
InitializeOnMainThread();
// Safe Browsing is enabled by default
ASSERT_TRUE(GetSafeBrowsingEnabled());

profile()->SetGoogleAccountAccessTokenFetchDelegate(
&access_token_fetch_delegate_);
}

void TearDown() override {
profile()->SetGoogleAccountAccessTokenFetchDelegate(nullptr);
}

void InitializeOnMainThread() {
NavigateAndWaitForCompletion(GURL("about:blank"), shell());
safe_browsing::SafeBrowsingApiHandler::SetInstance(fake_handler_.get());

// Some tests need to be able to navigate to URLs on domains that are not
// explicitly localhost (e.g., so that realtime URL lookups occur on these
// navigations).
host_resolver()->AddRule("*", "127.0.0.1");

ASSERT_TRUE(embedded_test_server()->Start());
url_ = embedded_test_server()->GetURL("/simple_page.html");
}
Expand All @@ -139,6 +193,16 @@ class SafeBrowsingBrowserTest : public WebLayerBrowserTest {
value);
}

void SetRealTimeURLLookupsEnabled(bool value) {
GetProfile()->SetBooleanSetting(
SettingType::REAL_TIME_SAFE_BROWSING_ENABLED, value);
}

void EnableSafeBrowsingAccessTokenFetches() {
RealTimeUrlLookupServiceFactory::GetInstance()
->set_access_token_fetches_enabled_for_testing();
}

bool GetSafeBrowsingEnabled() {
return GetProfile()->GetBooleanSetting(
SettingType::BASIC_SAFE_BROWSING_ENABLED);
Expand Down Expand Up @@ -209,7 +273,18 @@ class SafeBrowsingBrowserTest : public WebLayerBrowserTest {
std::unique_ptr<FakeSafeBrowsingApiHandler> fake_handler_;
GURL url_;

ProfileImpl* profile() {
auto* tab_impl = static_cast<TabImpl*>(shell()->tab());
return tab_impl->profile();
}

TestAccessTokenFetchDelegate* access_token_fetch_delegate() {
return &access_token_fetch_delegate_;
}

private:
TestAccessTokenFetchDelegate access_token_fetch_delegate_;

DISALLOW_COPY_AND_ASSIGN(SafeBrowsingBrowserTest);
};

Expand Down Expand Up @@ -340,4 +415,62 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingDisabledBrowserTest,
safe_browsing::SB_THREAT_TYPE_URL_MALWARE, false);
}

IN_PROC_BROWSER_TEST_F(SafeBrowsingBrowserTest,
NoAccessTokenFetchWhenSafeBrowsingNotEnabled) {
GURL a_url(embedded_test_server()->GetURL("a.com", "/simple_page.html"));
NavigateAndWaitForCompletion(a_url, shell()->tab());

EXPECT_FALSE(access_token_fetch_delegate()->has_received_request());
}

IN_PROC_BROWSER_TEST_F(SafeBrowsingBrowserTest,
NoAccessTokenFetchInBasicSafeBrowsing) {
SetSafeBrowsingEnabled(true);

GURL a_url(embedded_test_server()->GetURL("a.com", "/simple_page.html"));
NavigateAndWaitForCompletion(a_url, shell()->tab());

EXPECT_FALSE(access_token_fetch_delegate()->has_received_request());
}

IN_PROC_BROWSER_TEST_F(SafeBrowsingBrowserTest,
NoAccessTokenFetchInRealTimeUrlLookupsUnlessEnabled) {
SetRealTimeURLLookupsEnabled(true);

GURL a_url(embedded_test_server()->GetURL("a.com", "/simple_page.html"));
NavigateAndWaitForCompletion(a_url, shell()->tab());

EXPECT_FALSE(access_token_fetch_delegate()->has_received_request());

EnableSafeBrowsingAccessTokenFetches();
access_token_fetch_delegate()->set_should_respond_to_request(true);

GURL b_url(embedded_test_server()->GetURL("a.com", "/simple_page.html"));
NavigateAndWaitForCompletion(a_url, shell()->tab());

std::set<std::string> safe_browsing_scopes = {safe_browsing::kAPIScope};
EXPECT_TRUE(access_token_fetch_delegate()->has_received_request());
EXPECT_EQ(safe_browsing_scopes,
access_token_fetch_delegate()->scopes_from_most_recent_request());
}

// Tests that even if the embedder does not respond to an access token fetch
// that is made by safe browsing as part of a navigation, the navigation
// completes due to Safe Browsing's timing out the access token fetch.
IN_PROC_BROWSER_TEST_F(
SafeBrowsingBrowserTest,
UnfulfilledAccessTokenFetchTimesOutAndNavigationCompletes) {
SetRealTimeURLLookupsEnabled(true);
EnableSafeBrowsingAccessTokenFetches();
access_token_fetch_delegate()->set_should_respond_to_request(false);

GURL a_url(embedded_test_server()->GetURL("a.com", "/simple_page.html"));
NavigateAndWaitForCompletion(a_url, shell()->tab());

std::set<std::string> safe_browsing_scopes = {safe_browsing::kAPIScope};
EXPECT_TRUE(access_token_fetch_delegate()->has_received_request());
EXPECT_EQ(safe_browsing_scopes,
access_token_fetch_delegate()->scopes_from_most_recent_request());
}

} // namespace weblayer
61 changes: 61 additions & 0 deletions weblayer/browser/safe_browsing/safe_browsing_token_fetcher_impl.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright 2021 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.

#include "weblayer/browser/safe_browsing/safe_browsing_token_fetcher_impl.h"

#include "base/bind.h"
#include "base/memory/weak_ptr.h"
#include "weblayer/public/google_account_access_token_fetch_delegate.h"

namespace weblayer {

SafeBrowsingTokenFetcherImpl::SafeBrowsingTokenFetcherImpl(
const AccessTokenFetchDelegateGetter& delegate_getter)
: delegate_getter_(delegate_getter) {}

SafeBrowsingTokenFetcherImpl::~SafeBrowsingTokenFetcherImpl() = default;

void SafeBrowsingTokenFetcherImpl::Start(Callback callback) {
auto* delegate = delegate_getter_.Run();

if (!delegate) {
std::move(callback).Run("");
return;
}

// NOTE: base::Unretained() is safe below as this object owns
// |token_fetch_tracker_|, and the callback will not be invoked after
// |token_fetch_tracker_| is destroyed.
const int request_id = token_fetch_tracker_.StartTrackingTokenFetch(
std::move(callback),
base::BindOnce(&SafeBrowsingTokenFetcherImpl::OnTokenTimeout,
base::Unretained(this)));
request_ids_.insert(request_id);

// In contrast, this object does *not* have a determined lifetime relationship
// with |delegate|.
delegate->FetchAccessToken(
{safe_browsing::kAPIScope},
base::BindOnce(&SafeBrowsingTokenFetcherImpl::OnTokenFetched,
weak_ptr_factory_.GetWeakPtr(), request_id));
}

void SafeBrowsingTokenFetcherImpl::OnTokenFetched(
int request_id,
const std::string& access_token) {
if (!request_ids_.count(request_id)) {
// The request timed out before the delegate responded; nothing to do.
return;
}

request_ids_.erase(request_id);

token_fetch_tracker_.OnTokenFetchComplete(request_id, access_token);
}

void SafeBrowsingTokenFetcherImpl::OnTokenTimeout(int request_id) {
request_ids_.erase(request_id);
}

} // namespace weblayer
60 changes: 60 additions & 0 deletions weblayer/browser/safe_browsing/safe_browsing_token_fetcher_impl.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright 2021 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 WEBLAYER_BROWSER_SAFE_BROWSING_SAFE_BROWSING_TOKEN_FETCHER_IMPL_H_
#define WEBLAYER_BROWSER_SAFE_BROWSING_SAFE_BROWSING_TOKEN_FETCHER_IMPL_H_

#include <memory>

#include "base/containers/flat_map.h"
#include "base/memory/weak_ptr.h"
#include "components/safe_browsing/core/browser/safe_browsing_token_fetch_tracker.h"
#include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h"

namespace weblayer {

class GoogleAccountAccessTokenFetchDelegate;

// This class fetches access tokens for Safe Browsing via a
// GoogleAccountAccessTokenFetcherDelegate.
class SafeBrowsingTokenFetcherImpl
: public safe_browsing::SafeBrowsingTokenFetcher {
public:
using AccessTokenFetchDelegateGetter =
base::RepeatingCallback<GoogleAccountAccessTokenFetchDelegate*()>;

// Create a SafeBrowsingTokenFetcherImpl that makes access token requests via
// the object returned by |delegate_getter|. |delegate_getter| may return
// null, in which case this object will return the empty string for access
// token requests. This object will not cache the pointer returned by
// |delegate_getter| but will instead invoke it on every access token request,
// as that object might change over time.
// NOTE: In production the getter is
// ProfileImpl::access_token_fetcher_delegate(); this level of indirection is
// present to support unittests.
explicit SafeBrowsingTokenFetcherImpl(
const AccessTokenFetchDelegateGetter& delegate_getter);
~SafeBrowsingTokenFetcherImpl() override;

// SafeBrowsingTokenFetcher:
void Start(Callback callback) override;

private:
void OnTokenFetched(int request_id, const std::string& access_token);
void OnTokenTimeout(int request_id);

AccessTokenFetchDelegateGetter delegate_getter_;

safe_browsing::SafeBrowsingTokenFetchTracker token_fetch_tracker_;

// IDs of outstanding client access token requests being tracked by
// |token_fetch_tracker_|.
std::set<int> request_ids_;

base::WeakPtrFactory<SafeBrowsingTokenFetcherImpl> weak_ptr_factory_{this};
};

} // namespace weblayer

#endif // WEBLAYER_BROWSER_SAFE_BROWSING_SAFE_BROWSING_TOKEN_FETCHER_IMPL_H_
Loading

0 comments on commit 5982efd

Please sign in to comment.