Skip to content

Commit

Permalink
Support desktop safebrowsing on Android behind compile flag.
Browse files Browse the repository at this point in the history
This changes a bunch of code previously gated on using the local DB to
instead be gated on the FULL_SAFE_BROWSING flag as some features aren't
supported on Android regardless of localness of the database.

This CL also caps the database entry counts to the same (or larger)
values than those used by GMS safe browsing by default. This currently
results in a DB size of approximately 203KB.

Tested locally on a device without GMS, and safe browsing appears
to work correctly.

Bug: 928236

Change-Id: Ic855c775fb5dcdf8f3ea8e37056cfe0f3d583dd5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1452062
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638912}
  • Loading branch information
Michael Thiessen authored and Commit Bot committed Mar 8, 2019
1 parent 48c6b86 commit 4c9e4a8
Show file tree
Hide file tree
Showing 26 changed files with 128 additions and 43 deletions.
2 changes: 2 additions & 0 deletions build/config/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ config("feature_flags") {
defines += [ "SAFE_BROWSING_DB_LOCAL" ]
} else if (safe_browsing_mode == 2) {
defines += [ "SAFE_BROWSING_DB_REMOTE" ]
} else if (safe_browsing_mode == 3) {
defines += [ "SAFE_BROWSING_DB_LOCAL" ]
}
if (is_official_build) {
defines += [ "OFFICIAL_BUILD" ]
Expand Down
10 changes: 7 additions & 3 deletions build/config/features.gni
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,16 @@ declare_args() {

# Variable safe_browsing is used to control the build time configuration for
# safe browsing feature. Safe browsing can be compiled in 3 different levels:
# 0 disables it, 1 enables it fully, and 2 enables mobile protection via an
# external API.
# 0 disables it, 1 enables it fully, 2 enables mobile protection via an
# external API, and 3 enables mobile protection via internal API.
if (is_ios || is_chromecast) {
safe_browsing_mode = 0
} else if (is_android) {
safe_browsing_mode = 2
if (notouch_build) {
safe_browsing_mode = 3
} else {
safe_browsing_mode = 2
}
} else {
safe_browsing_mode = 1
}
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4203,7 +4203,7 @@ ChromeContentBrowserClient::CreateThrottlesForNavigation(
throttles.push_back(std::move(background_tab_navigation_throttle));
#endif

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
std::unique_ptr<content::NavigationThrottle>
password_protection_navigation_throttle =
safe_browsing::MaybeCreateNavigationThrottle(handle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
#include "third_party/re2/src/re2/re2.h"
#include "url/url_constants.h"

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
#include "chrome/browser/safe_browsing/advanced_protection_status_manager.h"
#include "chrome/browser/safe_browsing/chrome_password_protection_service.h"
#endif
Expand Down Expand Up @@ -436,7 +436,7 @@ void ChromePasswordManagerClient::PasswordWasAutofilled(
void ChromePasswordManagerClient::CheckSafeBrowsingReputation(
const GURL& form_action,
const GURL& frame_url) {
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
safe_browsing::PasswordProtectionService* pps =
GetPasswordProtectionService();
if (pps) {
Expand All @@ -451,7 +451,7 @@ void ChromePasswordManagerClient::HidePasswordGenerationPopup() {
popup_controller_->HideAndDestroy();
}

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
safe_browsing::PasswordProtectionService*
ChromePasswordManagerClient::GetPasswordProtectionService() const {
return safe_browsing::ChromePasswordProtectionService::
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class ChromePasswordManagerClient

void HidePasswordGenerationPopup();

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
safe_browsing::PasswordProtectionService* GetPasswordProtectionService()
const override;

Expand Down
10 changes: 10 additions & 0 deletions chrome/browser/safe_browsing/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,16 @@ jumbo_static_library("safe_browsing") {
"telemetry/android/android_telemetry_service.h",
]
deps += [ "//components/safe_browsing/android:safe_browsing_mobile" ]
} else if (safe_browsing_mode == 3) {
sources += [
"../loader/safe_browsing_resource_throttle.cc",
"../loader/safe_browsing_resource_throttle.h",
"android/services_delegate_android.cc",
"android/services_delegate_android.h",
"telemetry/android/android_telemetry_service.cc",
"telemetry/android/android_telemetry_service.h",
]
deps += [ "//components/safe_browsing/db:db" ]
}
}
}
Expand Down
16 changes: 16 additions & 0 deletions chrome/browser/safe_browsing/android/services_delegate_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
#include "base/logging.h"
#include "base/memory/ptr_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
#include "chrome/browser/safe_browsing/telemetry/android/android_telemetry_service.h"
#include "chrome/browser/safe_browsing/telemetry/telemetry_service.h"
#include "components/safe_browsing/android/remote_database_manager.h"
#include "components/safe_browsing/db/v4_local_database_manager.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/download_manager.h"
Expand Down Expand Up @@ -52,10 +54,24 @@ ServicesDelegateAndroid::database_manager() const {

void ServicesDelegateAndroid::Initialize() {
if (!database_manager_set_for_tests_) {
#if defined(SAFE_BROWSING_DB_REMOTE)
database_manager_ =
base::WrapRefCounted(new RemoteSafeBrowsingDatabaseManager());
#else
database_manager_ = V4LocalDatabaseManager::Create(
SafeBrowsingService::GetBaseFilename(),
base::BindRepeating(
&ServicesDelegateAndroid::GetEstimatedExtendedReportingLevel,
base::Unretained(this)));
#endif
}
}

ExtendedReportingLevel
ServicesDelegateAndroid::GetEstimatedExtendedReportingLevel() const {
return safe_browsing_service_->estimated_extended_reporting_by_prefs();
}

void ServicesDelegateAndroid::SetDatabaseManagerForTest(
SafeBrowsingDatabaseManager* database_manager) {
database_manager_set_for_tests_ = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "base/macros.h"
#include "chrome/browser/safe_browsing/services_delegate.h"
#include "components/safe_browsing/common/safe_browsing_prefs.h"

namespace safe_browsing {

Expand Down Expand Up @@ -55,6 +56,14 @@ class ServicesDelegateAndroid : public ServicesDelegate {

std::string GetSafetyNetId() const override;

// Reports the current extended reporting level. Note that this is an
// estimation and may not always be correct. It is possible that the
// estimation finds both Scout and legacy extended reporting to be enabled.
// This can happen, for instance, if one profile has Scout enabled and another
// has legacy extended reporting enabled. In such a case, this method reports
// LEGACY as the current level.
ExtendedReportingLevel GetEstimatedExtendedReportingLevel() const;

SafeBrowsingService* const safe_browsing_service_;

// The telemetry service tied to the current profile.
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/ssl/security_state_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
#include "chrome/browser/chromeos/policy/policy_cert_service_factory.h"
#endif // defined(OS_CHROMEOS)

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
#include "chrome/browser/safe_browsing/chrome_password_protection_service.h"
#endif

Expand Down Expand Up @@ -210,7 +210,7 @@ SecurityStateTabHelper::GetMaliciousContentStatus() const {
case safe_browsing::SB_THREAT_TYPE_URL_UNWANTED:
return security_state::MALICIOUS_CONTENT_STATUS_UNWANTED_SOFTWARE;
case safe_browsing::SB_THREAT_TYPE_SIGN_IN_PASSWORD_REUSE:
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
if (safe_browsing::ChromePasswordProtectionService::
ShouldShowPasswordReusePageInfoBubble(
web_contents(),
Expand All @@ -224,7 +224,7 @@ SecurityStateTabHelper::GetMaliciousContentStatus() const {
return security_state::MALICIOUS_CONTENT_STATUS_SOCIAL_ENGINEERING;
#endif
case safe_browsing::SB_THREAT_TYPE_ENTERPRISE_PASSWORD_REUSE:
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
if (safe_browsing::ChromePasswordProtectionService::
ShouldShowPasswordReusePageInfoBubble(
web_contents(),
Expand Down
18 changes: 9 additions & 9 deletions chrome/browser/ui/page_info/page_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,18 @@
#include "chrome/browser/ui/page_info/page_info_infobar_delegate.h"
#endif

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
#include "chrome/browser/safe_browsing/chrome_password_protection_service.h"
#endif

using base::ASCIIToUTF16;
using base::UTF8ToUTF16;
using base::UTF16ToUTF8;
using content::BrowserThread;
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
using PasswordReuseEvent =
safe_browsing::LoginReputationClientRequest::PasswordReuseEvent;
#endif // SAFE_BROWSING_DB_LOCAL
#endif // FULL_SAFE_BROWSING

namespace {

Expand Down Expand Up @@ -345,7 +345,7 @@ PageInfo::PageInfo(PageInfoUI* ui,
did_revoke_user_ssl_decisions_(false),
profile_(profile),
security_level_(security_state::NONE),
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
password_protection_service_(
safe_browsing::ChromePasswordProtectionService::
GetPasswordProtectionService(profile_)),
Expand Down Expand Up @@ -555,7 +555,7 @@ void PageInfo::OpenSiteSettingsView() {

void PageInfo::OnChangePasswordButtonPressed(
content::WebContents* web_contents) {
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
DCHECK(password_protection_service_);
DCHECK(site_identity_status_ == SITE_IDENTITY_STATUS_SIGN_IN_PASSWORD_REUSE ||
site_identity_status_ ==
Expand All @@ -572,7 +572,7 @@ void PageInfo::OnChangePasswordButtonPressed(

void PageInfo::OnWhitelistPasswordReuseButtonPressed(
content::WebContents* web_contents) {
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
DCHECK(password_protection_service_);
DCHECK(site_identity_status_ == SITE_IDENTITY_STATUS_SIGN_IN_PASSWORD_REUSE ||
site_identity_status_ ==
Expand Down Expand Up @@ -963,7 +963,7 @@ void PageInfo::PresentSiteIdentity() {
info.show_ssl_decision_revoke_button = show_ssl_decision_revoke_button_;
info.show_change_password_buttons = show_change_password_buttons_;
ui_->SetIdentityInfo(info);
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
if (password_protection_service_ && show_change_password_buttons_) {
if (site_identity_status_ == SITE_IDENTITY_STATUS_SIGN_IN_PASSWORD_REUSE) {
safe_browsing::LogWarningAction(
Expand Down Expand Up @@ -1027,7 +1027,7 @@ void PageInfo::GetSiteIdentityByMaliciousContentStatus(
l10n_util::GetStringUTF16(IDS_PAGE_INFO_UNWANTED_SOFTWARE_DETAILS);
break;
case security_state::MALICIOUS_CONTENT_STATUS_SIGN_IN_PASSWORD_REUSE:
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
*status = PageInfo::SITE_IDENTITY_STATUS_SIGN_IN_PASSWORD_REUSE;
// |password_protection_service_| may be null in test.
*details = password_protection_service_
Expand All @@ -1037,7 +1037,7 @@ void PageInfo::GetSiteIdentityByMaliciousContentStatus(
#endif
break;
case security_state::MALICIOUS_CONTENT_STATUS_ENTERPRISE_PASSWORD_REUSE:
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
*status = PageInfo::SITE_IDENTITY_STATUS_ENTERPRISE_PASSWORD_REUSE;
// |password_protection_service_| maybe null in test.
*details = password_protection_service_
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/page_info/page_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ class PageInfo : public TabSpecificContentSettings::SiteDataObserver,

security_state::SecurityLevel security_level_;

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
// Used to handle changing password, and whitelisting site.
safe_browsing::ChromePasswordProtectionService* password_protection_service_;
#endif
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/ui/page_info/page_info_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
#include "ui/gfx/paint_vector_icon.h"
#endif

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
#include "components/safe_browsing/password_protection/password_protection_service.h"
#endif

Expand Down Expand Up @@ -288,12 +288,12 @@ PageInfoUI::GetSecurityDescription(const IdentityInfo& identity_info) const {
IDS_PAGE_INFO_UNWANTED_SOFTWARE_SUMMARY,
IDS_PAGE_INFO_UNWANTED_SOFTWARE_DETAILS);
case PageInfo::SITE_IDENTITY_STATUS_SIGN_IN_PASSWORD_REUSE:
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
return CreateSecurityDescriptionForPasswordReuse(
/*is_enterprise_password=*/false);
#endif
case PageInfo::SITE_IDENTITY_STATUS_ENTERPRISE_PASSWORD_REUSE:
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
return CreateSecurityDescriptionForPasswordReuse(
/*is_enterprise_password=*/true);
#endif
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/page_info/page_info_ui.h
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ class PageInfoUI {
std::unique_ptr<PageInfoUI::SecurityDescription> GetSecurityDescription(
const IdentityInfo& identity_info) const;

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
// Creates security description for password reuse case.
virtual std::unique_ptr<PageInfoUI::SecurityDescription>
CreateSecurityDescriptionForPasswordReuse(
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/views/page_info/page_info_bubble_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
#include "ui/views/window/dialog_client_view.h"
#include "url/gurl.h"

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
#include "chrome/browser/safe_browsing/chrome_password_protection_service.h"
#endif

Expand Down Expand Up @@ -902,7 +902,7 @@ void PageInfoBubbleView::LayoutPermissionsLikeUiRow(views::GridLayout* layout,
permissions_set->AddPaddingColumn(views::GridLayout::kFixedSize, side_margin);
}

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
std::unique_ptr<PageInfoUI::SecurityDescription>
PageInfoBubbleView::CreateSecurityDescriptionForPasswordReuse(
bool is_enterprise_password) const {
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/views/page_info/page_info_bubble_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class PageInfoBubbleView : public PageInfoBubbleViewBase,
bool is_list_empty,
int column_id);

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
std::unique_ptr<PageInfoUI::SecurityDescription>
CreateSecurityDescriptionForPasswordReuse(
bool is_enterprise_password) const override;
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@
#include "extensions/common/manifest.h"
#endif

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
#include "chrome/browser/safe_browsing/chrome_password_protection_service.h"
#include "chrome/browser/ui/webui/reset_password/reset_password_ui.h"
#include "components/safe_browsing/features.h"
Expand Down Expand Up @@ -687,7 +687,7 @@ WebUIFactoryFunction GetWebUIFactoryFunction(WebUI* web_ui,
return &NewWebUI<MediaEngagementUI>;
}

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
bool enable_reset_password =
base::FeatureList::IsEnabled(
safe_browsing::kForceEnableResetPasswordWebUI) ||
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/webui/settings/md_settings_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@
#include "chrome/browser/ui/webui/settings/printing_handler.h"
#endif

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(SAFE_BROWSING_FULL)
#include "chrome/browser/safe_browsing/chrome_password_protection_service.h"
#include "chrome/browser/ui/webui/settings/change_password_handler.h"
#endif
Expand Down Expand Up @@ -268,7 +268,7 @@ MdSettingsUI::MdSettingsUI(content::WebUI* web_ui)
#endif // OS_WIN && defined(GOOGLE_CHROME_BUILD)

bool password_protection_available = false;
#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(SAFE_BROWSING_FULL)
safe_browsing::ChromePasswordProtectionService* password_protection =
safe_browsing::ChromePasswordProtectionService::
GetPasswordProtectionService(profile);
Expand Down
2 changes: 1 addition & 1 deletion chrome/renderer/chrome_content_renderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ void ChromeContentRendererClient::RenderFrameCreated(
new nacl::NaClHelper(render_frame);
#endif

#if defined(FULL_SAFE_BROWSING) || defined(SAFE_BROWSING_DB_REMOTE)
#if defined(SAFE_BROWSING_DB_LOCAL) || defined(SAFE_BROWSING_DB_REMOTE)
safe_browsing::ThreatDOMDetails::Create(render_frame, registry);
#endif

Expand Down
3 changes: 1 addition & 2 deletions components/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,7 @@ test("components_unittests") {
if (enable_print_preview) {
deps += [ "//components/pwg_encoder:unit_tests" ]
}

if (safe_browsing_mode == 1) {
if (safe_browsing_mode == 1 || safe_browsing_mode == 3) {
deps += [ "//components/safe_browsing/db:unit_tests_desktop" ]
} else if (safe_browsing_mode == 2) {
deps += [ "//components/safe_browsing/android:unit_tests_mobile" ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class FaviconService;

class GURL;

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
namespace safe_browsing {
class PasswordProtectionService;
}
Expand Down Expand Up @@ -224,7 +224,7 @@ class PasswordManagerClient {
// Returns the current best guess as to the page's display language.
virtual std::string GetPageLanguage() const;

#if defined(SAFE_BROWSING_DB_LOCAL)
#if defined(FULL_SAFE_BROWSING)
// Return the PasswordProtectionService associated with this instance.
virtual safe_browsing::PasswordProtectionService*
GetPasswordProtectionService() const = 0;
Expand Down
Loading

0 comments on commit 4c9e4a8

Please sign in to comment.