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

Re-enable Safe Browsing download protection #6763

Merged
merged 4 commits into from
Oct 19, 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
11 changes: 11 additions & 0 deletions browser/net/brave_block_safebrowsing_urls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ namespace brave {
const char kDummyUrl[] = "https://no-thanks.invalid";

bool IsSafeBrowsingReportingURL(const GURL& gurl) {
static const std::vector<URLPattern> allowed_patterns({
URLPattern(
URLPattern::SCHEME_HTTPS,
"https://sb-ssl.google.com/safebrowsing/clientreport/download*"),
});
static const std::vector<URLPattern> reporting_patterns({
URLPattern(URLPattern::SCHEME_HTTPS,
"https://sb-ssl.google.com/safebrowsing/clientreport/*"),
Expand All @@ -26,6 +31,12 @@ bool IsSafeBrowsingReportingURL(const GURL& gurl) {
URLPattern(URLPattern::SCHEME_HTTPS,
"https://safebrowsing.google.com/safebrowsing/uploads/*"),
});

if (std::any_of(
allowed_patterns.begin(), allowed_patterns.end(),
[&gurl](URLPattern pattern) { return pattern.MatchesURL(gurl); })) {
return false;
}
return std::any_of(
reporting_patterns.begin(), reporting_patterns.end(),
[&gurl](URLPattern pattern) { return pattern.MatchesURL(gurl); });
Expand Down
7 changes: 6 additions & 1 deletion browser/net/brave_block_safebrowsing_urls_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,22 @@ TEST(BraveBlockReportingUrlsHelperTest, PreserveNormalUrls) {

TEST(BraveBlockReportingUrlsHelperTest, CancelReportingUrl) {
const std::vector<const std::string> reportingUrls({
"https://sb-ssl.google.com/safebrowsing/clientreport/download",
"https://sb-ssl.google.com/safebrowsing/clientreport/chrome-cct",
"https://sb-ssl.google.com/safebrowsing/clientreport/chrome-reset",
"https://sb-ssl.google.com/safebrowsing/clientreport/chrome-sw-reporter",
"https://sb-ssl.google.com/safebrowsing/clientreport/incident",
"https://sb-ssl.google.com/safebrowsing/clientreport/login",
"https://sb-ssl.google.com/safebrowsing/clientreport/phishing",
"https://sb-ssl.google.com/safebrowsing/clientreport/malware-check",
"https://safebrowsing.google.com/safebrowsing/uploads/app",
"https://safebrowsing.google.com/safebrowsing/uploads/chrome",
"https://safebrowsing.google.com/safebrowsing/uploads/scan",
"https://safebrowsing.google.com/safebrowsing/uploads/webprotect",
"https://safebrowsing.google.com/safebrowsing/report",
"https://safebrowsing.google.com/safebrowsing/clientreport/malware",
"https://safebrowsing.google.com/safebrowsing/uploads/chrome",
"https://safebrowsing.google.com/safebrowsing/clientreport/crx-list-info",
"https://safebrowsing.google.com/safebrowsing/clientreport/realtime",
});

for (const auto& url : reportingUrls) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,8 @@ int OnBeforeURLRequest_StaticRedirectWorkForGURL(
}

if (safebrowsingfilecheck_pattern.MatchesHost(request_url)) {
// TODO(@fmarier): Re-enable download protection once we have
// truncated the list of metadata that it sends to the server
// (brave/brave-browser#6267).
//
// replacements.SetHostStr(kBraveSafeBrowsingFileCheckProxy);
// *new_url = request_url.ReplaceComponents(replacements);
replacements.SetHostStr(kBraveSafeBrowsingFileCheckProxy);
*new_url = request_url.ReplaceComponents(replacements);
return net::OK;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,6 @@ TEST(BraveStaticRedirectNetworkDelegateHelperTest,
EXPECT_EQ(rc, net::OK);
}

// TODO(@fmarier): Re-enable download protection once we have
// truncated the list of metadata that it sends to the server
// (brave/brave-browser#6267).
#if 0
TEST(BraveStaticRedirectNetworkDelegateHelperTest,
ModifySafeBrowsingFileCheckURL) {
const GURL url(
Expand All @@ -288,7 +284,6 @@ TEST(BraveStaticRedirectNetworkDelegateHelperTest,
EXPECT_EQ(request_info->new_url_spec, expected_url);
EXPECT_EQ(rc, net::OK);
}
#endif // 0

#if BUILDFLAG(ENABLE_BRAVE_TRANSLATE_GO)
TEST(BraveStaticRedirectNetworkDelegateHelperTest, RedirectTranslate) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/* Copyright (c) 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 https://mozilla.org/MPL/2.0/. */

#define RegisterFileTypePoliciesComponent \
RegisterFileTypePoliciesComponent_ChromiumImpl
#include "../../../../../chrome/browser/component_updater/file_type_policies_component_installer.cc" // NOLINT
#undef RegisterFileTypePoliciesComponent

#include "chrome/browser/component_updater/component_updater_utils.h"
#include "components/component_updater/component_updater_service.h"

namespace component_updater {

const char kFileTypePoliciesComponentId[] = "khaoiebndkojlmppeemjhbpbandiljpe";

void OnFileTypePoliciesRegistered() {
component_updater::BraveOnDemandUpdate(kFileTypePoliciesComponentId);
}

void RegisterFileTypePoliciesComponent(ComponentUpdateService* cus,
const base::FilePath& user_data_dir) {
auto installer = base::MakeRefCounted<ComponentInstaller>(
std::make_unique<FileTypePoliciesComponentInstallerPolicy>());
installer->Register(cus, base::Bind(&OnFileTypePoliciesRegistered));
}

} // namespace component_updater
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/* Copyright (c) 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 https://mozilla.org/MPL/2.0/. */

#include "components/safe_browsing/core/proto/csd.pb.h"

#define BRAVE_SEND_REQUEST_FILTER BraveFilterRequest(request.get());

namespace safe_browsing {

void BraveFilterRequest(ClientDownloadRequest* request) {
request->set_url(""); // URL must be present or we get a 400.
request->clear_file_basename();
request->clear_locale();
request->clear_resources(); // Contains URLs and referrers
request->clear_referrer_chain();

// Filter binaries within archives.
for (int i = 0; i < request->archived_binary_size(); i++) {
ClientDownloadRequest_ArchivedBinary* archived_binary =
request->mutable_archived_binary(i);
archived_binary->clear_file_basename();
}
}

} // namespace safe_browsing

#include "../../../../../../chrome/browser/safe_browsing/download_protection/check_client_download_request_base.cc" // NOLINT
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/* Copyright (c) 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 https://mozilla.org/MPL/2.0/. */

#include "base/path_service.h"
#include "base/strings/stringprintf.h"
#include "brave/common/brave_paths.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/network_session_configurator/common/network_switches.h"
#include "components/prefs/pref_service.h"
#include "components/safe_browsing/content/web_ui/safe_browsing_ui.h"
#include "components/safe_browsing/core/proto/csd.pb.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/download_manager.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/download_test_observer.h"
#include "content/public/test/test_utils.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/default_handlers.h"
#include "net/test/embedded_test_server/http_request.h"
#include "ui/base/window_open_disposition.h"

class BraveCheckClientDownloadRequestBaseBrowserTest
: public InProcessBrowserTest {
public:
BraveCheckClientDownloadRequestBaseBrowserTest()
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}

void SetUpOnMainThread() override {
InProcessBrowserTest::SetUpOnMainThread();

host_resolver()->AddRule("*", "127.0.0.1");

browser()->profile()->GetPrefs()->SetBoolean(prefs::kPromptForDownload,
false);

brave::RegisterPathProvider();
base::PathService::Get(brave::DIR_TEST_DATA, &test_data_dir_);
https_server_.ServeFilesFromDirectory(test_data_dir_);
https_server_.AddDefaultHandlers(GetChromeTestDataDir());
safe_browsing::WebUIInfoSingleton::GetInstance()->AddListenerForTesting();

ASSERT_TRUE(https_server_.Start());

download_url_ = https_server_.GetURL("a.com", "/test.exe");
}

void SetUpCommandLine(base::CommandLine* command_line) override {
InProcessBrowserTest::SetUpCommandLine(command_line);
// This is needed to load pages from "domain.com" without an interstitial.
command_line->AppendSwitch(switches::kIgnoreCertificateErrors);
}

const net::EmbeddedTestServer& https_server() { return https_server_; }
const GURL& download_url() { return download_url_; }

private:
GURL download_url_;
base::FilePath test_data_dir_;

net::test_server::EmbeddedTestServer https_server_;
};

IN_PROC_BROWSER_TEST_F(BraveCheckClientDownloadRequestBaseBrowserTest,
FilterRequest) {
ui_test_utils::DownloadURL(browser(), download_url());

const std::vector<std::unique_ptr<safe_browsing::ClientDownloadRequest>>&
requests = safe_browsing::WebUIInfoSingleton::GetInstance()
->client_download_requests_sent();

ASSERT_EQ(requests.size(), 1u);

EXPECT_TRUE(requests[0]->has_url());
EXPECT_EQ(requests[0]->url(), "");
EXPECT_FALSE(requests[0]->has_locale());
EXPECT_FALSE(requests[0]->has_file_basename());
EXPECT_EQ(requests[0]->referrer_chain_size(), 0);
EXPECT_EQ(requests[0]->resources_size(), 0);
}
3 changes: 2 additions & 1 deletion common/network_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ const char kJSDataURLPrefix[] = "data:application/javascript;base64,";
const char kGeoLocationsPattern[] =
"https://www.googleapis.com/geolocation/v1/geolocate?key=*";
const char kSafeBrowsingPrefix[] = "https://safebrowsing.googleapis.com/";
const char kSafeBrowsingFileCheckPrefix[] = "https://sb-ssl.google.com/";
const char kSafeBrowsingFileCheckPrefix[] =
"https://sb-ssl.google.com/safebrowsing/clientreport/download";
const char kCRLSetPrefix1[] =
"*://dl.google.com/release2/chrome_component/*crl-set*";
const char kCRLSetPrefix2[] =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
diff --git a/chrome/browser/safe_browsing/download_protection/check_client_download_request_base.cc b/chrome/browser/safe_browsing/download_protection/check_client_download_request_base.cc
index 4f7fd86c5e6edd387254d0eab6aa399db5e42105..1edf51b8e80c967e275e327d8143973ec4881871 100644
--- a/chrome/browser/safe_browsing/download_protection/check_client_download_request_base.cc
+++ b/chrome/browser/safe_browsing/download_protection/check_client_download_request_base.cc
@@ -554,6 +554,7 @@ void CheckClientDownloadRequestBase::SendRequest() {
request->set_archive_directory_count(directory_count_);
request->set_request_ap_verdicts(is_under_advanced_protection_);

+ BRAVE_SEND_REQUEST_FILTER
fmarier marked this conversation as resolved.
Show resolved Hide resolved
if (!request->SerializeToString(&client_download_request_data_)) {
FinishRequest(DownloadCheckResult::UNKNOWN, REASON_INVALID_REQUEST_PROTO);
return;
1 change: 1 addition & 0 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,7 @@ if (!is_android) {
"//brave/browser/ui/webui/brave_welcome_ui_browsertest.cc",
"//brave/chromium_src/chrome/browser/media/router/media_router_feature_browsertest.cc",
"//brave/chromium_src/chrome/browser/profiles/profile_window_browsertest.cc",
"//brave/chromium_src/chrome/browser/safe_browsing/download_protection/check_client_download_request_base_browsertest.cc",
"//brave/chromium_src/chrome/browser/ui/views/location_bar/location_bar_view_browsertest.cc",
"//brave/chromium_src/chrome/browser/ui/views/tabs/tab_hover_card_bubble_view_browsertest.cc",
"//brave/chromium_src/components/content_settings/core/browser/brave_content_settings_registry_browsertest.cc",
Expand Down
Binary file added test/data/test.exe
Binary file not shown.