diff options
Diffstat (limited to 'chromium/components/safe_browsing/core')
47 files changed, 514 insertions, 159 deletions
diff --git a/chromium/components/safe_browsing/core/browser/BUILD.gn b/chromium/components/safe_browsing/core/browser/BUILD.gn index d931494c494..f07e6908221 100644 --- a/chromium/components/safe_browsing/core/browser/BUILD.gn +++ b/chromium/components/safe_browsing/core/browser/BUILD.gn @@ -153,6 +153,20 @@ source_set("token_fetcher_unit_tests") { ] } +source_set("token_fetcher_testing_helper") { + testonly = true + sources = [ + "test_safe_browsing_token_fetcher.cc", + "test_safe_browsing_token_fetcher.h", + ] + + deps = [ + ":token_fetcher", + "//base", + "//testing/gmock", + ] +} + source_set("download_check_result") { sources = [ "download_check_result.h" ] } diff --git a/chromium/components/safe_browsing/core/browser/db/database_manager.cc b/chromium/components/safe_browsing/core/browser/db/database_manager.cc index b87902dd2cb..3ded7349599 100644 --- a/chromium/components/safe_browsing/core/browser/db/database_manager.cc +++ b/chromium/components/safe_browsing/core/browser/db/database_manager.cc @@ -150,6 +150,11 @@ void SafeBrowsingDatabaseManager::NotifyDatabaseUpdateFinished() { update_complete_callback_list_.Notify(); } +bool SafeBrowsingDatabaseManager::IsDatabaseReady() { + DCHECK(io_task_runner()->RunsTasksInCurrentSequence()); + return enabled_; +} + SafeBrowsingDatabaseManager::SafeBrowsingApiCheck::SafeBrowsingApiCheck( const GURL& url, Client* client) diff --git a/chromium/components/safe_browsing/core/browser/db/database_manager.h b/chromium/components/safe_browsing/core/browser/db/database_manager.h index 16e3ae675b6..71cb8c4ba9b 100644 --- a/chromium/components/safe_browsing/core/browser/db/database_manager.h +++ b/chromium/components/safe_browsing/core/browser/db/database_manager.h @@ -233,10 +233,6 @@ class SafeBrowsingDatabaseManager // Returns whether download protection is enabled. virtual bool IsDownloadProtectionEnabled() const = 0; - // Returns true if URL-checking is supported on this build+device. - // If false, calls to CheckBrowseUrl may dcheck-fail. - virtual bool IsSupported() const = 0; - // // Methods to indicate when to start or suspend the SafeBrowsing operations. // These functions are always called on the IO thread. @@ -267,6 +263,9 @@ class SafeBrowsingDatabaseManager // method at the bottom of it. virtual void StopOnIOThread(bool shutdown); + // Called to check if database is ready or not. + virtual bool IsDatabaseReady(); + protected: // Bundled client info for an API abuse hash prefix check. class SafeBrowsingApiCheck { diff --git a/chromium/components/safe_browsing/core/browser/db/fake_database_manager.cc b/chromium/components/safe_browsing/core/browser/db/fake_database_manager.cc index 18265d4b36a..24fab78bfd8 100644 --- a/chromium/components/safe_browsing/core/browser/db/fake_database_manager.cc +++ b/chromium/components/safe_browsing/core/browser/db/fake_database_manager.cc @@ -94,10 +94,6 @@ safe_browsing::ThreatSource FakeSafeBrowsingDatabaseManager::GetThreatSource() return safe_browsing::ThreatSource::LOCAL_PVER4; } -bool FakeSafeBrowsingDatabaseManager::IsSupported() const { - return true; -} - // static void FakeSafeBrowsingDatabaseManager::CheckBrowseURLAsync( GURL url, diff --git a/chromium/components/safe_browsing/core/browser/db/fake_database_manager.h b/chromium/components/safe_browsing/core/browser/db/fake_database_manager.h index 23d64c88d0b..cca570959d8 100644 --- a/chromium/components/safe_browsing/core/browser/db/fake_database_manager.h +++ b/chromium/components/safe_browsing/core/browser/db/fake_database_manager.h @@ -36,7 +36,6 @@ class FakeSafeBrowsingDatabaseManager : public TestSafeBrowsingDatabaseManager { Client* client) override; bool CheckUrlForSubresourceFilter(const GURL& url, Client* client) override; safe_browsing::ThreatSource GetThreatSource() const override; - bool IsSupported() const override; private: ~FakeSafeBrowsingDatabaseManager() override; diff --git a/chromium/components/safe_browsing/core/browser/db/prefix_iterator.h b/chromium/components/safe_browsing/core/browser/db/prefix_iterator.h index cd243154dde..0d065ac6514 100644 --- a/chromium/components/safe_browsing/core/browser/db/prefix_iterator.h +++ b/chromium/components/safe_browsing/core/browser/db/prefix_iterator.h @@ -16,12 +16,13 @@ namespace safe_browsing { // The prefix iterator is used to binary search within a |HashPrefixes|. It is // essentially a random access iterator that steps |PrefixSize| steps within the // underlying buffer. -class PrefixIterator - : public std::iterator<std::random_access_iterator_tag, base::StringPiece> { +class PrefixIterator { public: - using difference_type = - typename std::iterator<std::random_access_iterator_tag, - base::StringPiece>::difference_type; + using iterator_category = std::random_access_iterator_tag; + using value_type = base::StringPiece; + using difference_type = std::ptrdiff_t; + using pointer = base::StringPiece*; + using reference = base::StringPiece&; PrefixIterator(base::StringPiece prefixes, size_t index, size_t size); PrefixIterator(const PrefixIterator& rhs); diff --git a/chromium/components/safe_browsing/core/browser/db/test_database_manager.cc b/chromium/components/safe_browsing/core/browser/db/test_database_manager.cc index b4099616098..08610aa508b 100644 --- a/chromium/components/safe_browsing/core/browser/db/test_database_manager.cc +++ b/chromium/components/safe_browsing/core/browser/db/test_database_manager.cc @@ -117,11 +117,6 @@ bool TestSafeBrowsingDatabaseManager::IsDownloadProtectionEnabled() const { return false; } -bool TestSafeBrowsingDatabaseManager::IsSupported() const { - NOTIMPLEMENTED(); - return false; -} - void TestSafeBrowsingDatabaseManager::StartOnIOThread( scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, const V4ProtocolConfig& config) { diff --git a/chromium/components/safe_browsing/core/browser/db/test_database_manager.h b/chromium/components/safe_browsing/core/browser/db/test_database_manager.h index ffa46a88a1a..a4d74b45cd7 100644 --- a/chromium/components/safe_browsing/core/browser/db/test_database_manager.h +++ b/chromium/components/safe_browsing/core/browser/db/test_database_manager.h @@ -48,7 +48,6 @@ class TestSafeBrowsingDatabaseManager : public SafeBrowsingDatabaseManager { bool MatchMalwareIP(const std::string& ip_address) override; safe_browsing::ThreatSource GetThreatSource() const override; bool IsDownloadProtectionEnabled() const override; - bool IsSupported() const override; void StartOnIOThread( scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, const V4ProtocolConfig& config) override; diff --git a/chromium/components/safe_browsing/core/browser/db/v4_database_unittest.cc b/chromium/components/safe_browsing/core/browser/db/v4_database_unittest.cc index 900b9339a70..67ae5760ece 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_database_unittest.cc +++ b/chromium/components/safe_browsing/core/browser/db/v4_database_unittest.cc @@ -32,7 +32,7 @@ class FakeV4Store : public V4Store { return hash_prefix_should_match_ ? full_hash : HashPrefix(); } - bool HasValidData() const override { return true; } + bool HasValidData() override { return true; } void set_hash_prefix_matches(bool hash_prefix_matches) { hash_prefix_should_match_ = hash_prefix_matches; diff --git a/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager.cc b/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager.cc index e7ffe69cda8..221cb5c7e8a 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager.cc +++ b/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager.cc @@ -11,6 +11,7 @@ #include "base/bind.h" #include "base/containers/contains.h" #include "base/memory/ptr_util.h" +#include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_macros.h" #include "base/strings/string_split.h" #include "base/timer/timer.h" @@ -38,6 +39,12 @@ void RecordGetHashResult(safe_browsing::V4OperationResult result) { safe_browsing::V4OperationResult::OPERATION_RESULT_MAX); } +// Record a backoff error count +void RecordBackoffErrorCountResult(size_t count) { + base::UmaHistogramCounts100( + "SafeBrowsing.V4GetHash.Result.BackoffErrorCount", count); +} + // Enumerate parsing failures for histogramming purposes. DO NOT CHANGE // THE ORDERING OF THESE VALUES. enum ParseResultType { @@ -124,6 +131,12 @@ void RecordV4GetHashCheckResult(V4GetHashCheckResultType result_type) { GET_HASH_CHECK_RESULT_MAX); } +bool ErrorIsRetriable(int net_error, int http_error) { + return (net_error == net::ERR_INTERNET_DISCONNECTED || + net_error == net::ERR_NETWORK_CHANGED) && + http_error != net::HTTP_OK; +} + const char kPermission[] = "permission"; const char kPhaPatternType[] = "pha_pattern_type"; const char kMalwareThreatType[] = "malware_threat_type"; @@ -297,6 +310,7 @@ void V4GetHashProtocolManager::GetFullHashes( if (clock_->Now() <= next_gethash_time_) { if (gethash_error_count_) { RecordGetHashResult(V4OperationResult::BACKOFF_ERROR); + backoff_error_count_++; } else { RecordGetHashResult(V4OperationResult::MIN_WAIT_DURATION_ERROR); } @@ -708,6 +722,7 @@ void V4GetHashProtocolManager::ParseMetadata(const ThreatMatch& match, void V4GetHashProtocolManager::ResetGetHashErrors() { gethash_error_count_ = 0; gethash_back_off_mult_ = 1; + backoff_error_count_ = 0; next_gethash_time_ = base::Time(); } @@ -802,11 +817,18 @@ void V4GetHashProtocolManager::OnURLLoaderCompleteInternal( Time negative_cache_expire; if (net_error == net::OK && response_code == net::HTTP_OK) { RecordGetHashResult(V4OperationResult::STATUS_200); + if (gethash_error_count_) RecordBackoffErrorCountResult(backoff_error_count_); ResetGetHashErrors(); if (!ParseHashResponse(data, &full_hash_infos, &negative_cache_expire)) { full_hash_infos.clear(); RecordGetHashResult(V4OperationResult::PARSE_ERROR); } + } else if (ErrorIsRetriable(net_error, response_code)) { + if (net_error != net::OK) { + RecordGetHashResult(V4OperationResult::RETRIABLE_NETWORK_ERROR); + } else { + RecordGetHashResult(V4OperationResult::RETRIABLE_HTTP_ERROR); + } } else { HandleGetHashError(clock_->Now()); diff --git a/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager.h b/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager.h index b51211ef2a2..2a6eb81c89a 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager.h +++ b/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager.h @@ -229,6 +229,7 @@ class V4GetHashProtocolManager { TestGetHashErrorHandlingParallelRequests); FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest, GetCachedResults); FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest, TestUpdatesAreMerged); + FRIEND_TEST_ALL_PREFIXES(V4GetHashProtocolManagerTest, TestBackoffErrorHistogramCount); friend class V4GetHashProtocolManagerTest; friend class V4GetHashProtocolManagerFuzzer; friend class V4GetHashProtocolManagerFactoryImpl; @@ -329,6 +330,9 @@ class V4GetHashProtocolManager { // response, used for request backoff timing. size_t gethash_error_count_; + // The number of backoff errors since the last successful HTTP response. + size_t backoff_error_count_ = 0; + // Multiplier for the backoff error after the second. size_t gethash_back_off_mult_; diff --git a/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager_unittest.cc b/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager_unittest.cc index 13ed4fd888d..c9e49a86309 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager_unittest.cc +++ b/chromium/components/safe_browsing/core/browser/db/v4_get_hash_protocol_manager_unittest.cc @@ -10,6 +10,7 @@ #include "base/base64.h" #include "base/bind.h" #include "base/run_loop.h" +#include "base/strings/escape.h" #include "base/test/metrics/histogram_tester.h" #include "base/test/simple_test_clock.h" #include "base/test/task_environment.h" @@ -18,7 +19,6 @@ #include "components/safe_browsing/core/browser/db/safebrowsing.pb.h" #include "components/safe_browsing/core/browser/db/util.h" #include "components/safe_browsing/core/browser/db/v4_test_util.h" -#include "net/base/escape.h" #include "net/base/load_flags.h" #include "net/base/net_errors.h" #include "services/network/public/cpp/shared_url_loader_factory.h" @@ -232,6 +232,56 @@ TEST_F(V4GetHashProtocolManagerTest, TestGetHashErrorHandlingResponseCode) { EXPECT_TRUE(callback_called()); } +TEST_F(V4GetHashProtocolManagerTest, TestBackoffErrorHistogramCount) { + std::unique_ptr<V4GetHashProtocolManager> pm(CreateProtocolManager()); + base::HistogramTester histogram_tester; + + FullHashToStoreAndHashPrefixesMap matched_locally; + matched_locally[FullHash("AHashFull")].emplace_back(GetUrlSocEngId(), + HashPrefix("AHash")); + + std::vector<FullHashInfo> expected_results; + pm->GetFullHashes( + matched_locally, {}, + base::BindRepeating(&V4GetHashProtocolManagerTest::ValidateGetV4HashResults, + base::Unretained(this), expected_results)); + + FullHashToStoreAndHashPrefixesMap matched_locally2; + matched_locally2[FullHash("AHash2Full")].emplace_back(GetUrlSocEngId(), + HashPrefix("AHash2")); + + pm->GetFullHashes(matched_locally2, {}, + base::BindRepeating( + &V4GetHashProtocolManagerTest::ValidateGetV4HashResults, + base::Unretained(this), expected_results)); + + // Failed request status should result in error. + SetupFullHashFetcherToReturnResponse(pm.get(), FullHash("AHashFull"), + net::ERR_CONNECTION_RESET, 200, + GetStockV4HashResponse()); + + EXPECT_EQ(1ul, pm->gethash_error_count_); + EXPECT_EQ(1ul, pm->gethash_back_off_mult_); + + EXPECT_TRUE(callback_called()); + + reset_callback_called(); + + pm->GetFullHashes(matched_locally2, {}, + base::BindRepeating( + &V4GetHashProtocolManagerTest::ValidateGetV4HashResults, + base::Unretained(this), expected_results)); + + EXPECT_EQ(1ul, pm->backoff_error_count_); + + SetupFullHashFetcherToReturnResponse(pm.get(), FullHash("AHash2Full"), + net::OK, 200, GetStockV4HashResponse()); + + histogram_tester.ExpectTotalCount( + "SafeBrowsing.V4GetHash.Result.BackoffErrorCount", + 1); +} + TEST_F(V4GetHashProtocolManagerTest, TestGetHashErrorHandlingParallelRequests) { std::unique_ptr<V4GetHashProtocolManager> pm(CreateProtocolManager()); std::vector<FullHashInfo> empty_results; diff --git a/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager.cc b/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager.cc index 254468c76d3..0ee0f67720c 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager.cc +++ b/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager.cc @@ -639,10 +639,6 @@ bool V4LocalDatabaseManager::IsDownloadProtectionEnabled() const { return true; } -bool V4LocalDatabaseManager::IsSupported() const { - return true; -} - void V4LocalDatabaseManager::StartOnIOThread( scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, const V4ProtocolConfig& config) { @@ -990,15 +986,18 @@ void V4LocalDatabaseManager::PerformFullHashCheck( std::unique_ptr<PendingCheck> check) { DCHECK(io_task_runner()->RunsTasksInCurrentSequence()); - DCHECK(enabled_); DCHECK(!check->full_hash_to_store_and_hash_prefixes.empty()); - FullHashToStoreAndHashPrefixesMap full_hash_to_store_and_hash_prefixes = - check->full_hash_to_store_and_hash_prefixes; - v4_get_hash_protocol_manager_->GetFullHashes( - full_hash_to_store_and_hash_prefixes, list_client_states_, - base::BindOnce(&V4LocalDatabaseManager::OnFullHashResponse, - weak_factory_.GetWeakPtr(), std::move(check))); + // If we're not enabled, we're in the middle of shutdown, so silently drop the + // check. + if (enabled_) { + FullHashToStoreAndHashPrefixesMap full_hash_to_store_and_hash_prefixes = + check->full_hash_to_store_and_hash_prefixes; + v4_get_hash_protocol_manager_->GetFullHashes( + full_hash_to_store_and_hash_prefixes, list_client_states_, + base::BindOnce(&V4LocalDatabaseManager::OnFullHashResponse, + weak_factory_.GetWeakPtr(), std::move(check))); + } } void V4LocalDatabaseManager::ProcessQueuedChecks() { diff --git a/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager.h b/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager.h index c27d2dd53c9..511c12ac084 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager.h +++ b/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager.h @@ -85,7 +85,6 @@ class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager { bool MatchMalwareIP(const std::string& ip_address) override; safe_browsing::ThreatSource GetThreatSource() const override; bool IsDownloadProtectionEnabled() const override; - bool IsSupported() const override; void StartOnIOThread( scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, diff --git a/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager_unittest.cc b/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager_unittest.cc index 8961309d140..55ec5ab867e 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager_unittest.cc +++ b/chromium/components/safe_browsing/core/browser/db/v4_local_database_manager_unittest.cc @@ -478,11 +478,6 @@ TEST_F(V4LocalDatabaseManagerTest, TestGetThreatSource) { v4_local_database_manager_->GetThreatSource()); } -TEST_F(V4LocalDatabaseManagerTest, TestIsSupported) { - WaitForTasksOnTaskRunner(); - EXPECT_TRUE(v4_local_database_manager_->IsSupported()); -} - TEST_F(V4LocalDatabaseManagerTest, TestCanCheckUrl) { WaitForTasksOnTaskRunner(); EXPECT_TRUE( diff --git a/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util.cc b/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util.cc index 4e9290e3f89..962d2b0e1bd 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util.cc +++ b/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util.cc @@ -9,6 +9,7 @@ #include "base/hash/sha1.h" #include "base/metrics/histogram_functions.h" #include "base/rand_util.h" +#include "base/strings/escape.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/time/time.h" @@ -16,7 +17,6 @@ #include "components/version_info/version_info.h" #include "crypto/sha2.h" #include "google_apis/google_api_keys.h" -#include "net/base/escape.h" #include "net/base/ip_address.h" #include "net/base/net_errors.h" #include "net/http/http_request_headers.h" @@ -47,7 +47,7 @@ std::string Unescape(const std::string& url) { int loop_var = 0; do { old_size = unescaped_str.size(); - unescaped_str = net::UnescapeBinaryURLComponent(unescaped_str); + unescaped_str = base::UnescapeBinaryURLComponent(unescaped_str); } while (old_size != unescaped_str.size() && ++loop_var <= kMaxLoopIterations); @@ -97,7 +97,7 @@ std::string GetReportUrl(const V4ProtocolConfig& config, std::string api_key = google_apis::GetAPIKey(); if (!api_key.empty()) { base::StringAppendF(&url, "&key=%s", - net::EscapeQueryParamValue(api_key, true).c_str()); + base::EscapeQueryParamValue(api_key, true).c_str()); } if (reporting_level) url.append(base::StringPrintf("&ext=%d", *reporting_level)); @@ -323,7 +323,7 @@ std::string V4ProtocolManagerUtil::ComposeUrl(const std::string& prefix, method.c_str(), request_base64.c_str()); if (!key_param.empty()) { base::StringAppendF(&url, "&key=%s", - net::EscapeQueryParamValue(key_param, true).c_str()); + base::EscapeQueryParamValue(key_param, true).c_str()); } return url; } diff --git a/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util.h b/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util.h index f304edb241e..0a350650d87 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util.h +++ b/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util.h @@ -301,9 +301,16 @@ enum V4OperationResult { // Identical operation already pending. ALREADY_PENDING_ERROR = 6, + // A network error that can be retried without backoff (e.g. + // NETWORK_DISCONNECTED). + RETRIABLE_NETWORK_ERROR = 7, + + // An HTTP error code that can be retried without backoff. + RETRIABLE_HTTP_ERROR = 8, + // Memory space for histograms is determined by the max. ALWAYS // ADD NEW VALUES BEFORE THIS ONE. - OPERATION_RESULT_MAX = 7 + OPERATION_RESULT_MAX = 9 }; // A class that provides static methods related to the Pver4 protocol. diff --git a/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util_unittest.cc b/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util_unittest.cc index a36e312a201..63dd889d1df 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util_unittest.cc +++ b/chromium/components/safe_browsing/core/browser/db/v4_protocol_manager_util_unittest.cc @@ -9,10 +9,10 @@ #include "base/base64.h" #include "base/containers/contains.h" #include "base/logging.h" +#include "base/strings/escape.h" #include "base/strings/stringprintf.h" #include "base/time/time.h" #include "components/safe_browsing/core/browser/db/v4_test_util.h" -#include "net/base/escape.h" #include "net/http/http_request_headers.h" #include "testing/gtest/include/gtest/gtest.h" diff --git a/chromium/components/safe_browsing/core/browser/db/v4_store.cc b/chromium/components/safe_browsing/core/browser/db/v4_store.cc index 9f76b1569b9..7c6f00fa918 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_store.cc +++ b/chromium/components/safe_browsing/core/browser/db/v4_store.cc @@ -9,6 +9,7 @@ #include "base/base64.h" #include "base/bind.h" +#include "base/cpu_reduction_experiment.h" #include "base/files/file_util.h" #include "base/logging.h" #include "base/metrics/histogram_functions.h" @@ -22,6 +23,7 @@ #include "components/safe_browsing/core/common/proto/webui.pb.h" #include "crypto/secure_hash.h" #include "crypto/sha2.h" +#include "third_party/abseil-cpp/absl/types/optional.h" using base::TimeTicks; @@ -204,9 +206,14 @@ void V4Store::Initialize() { RecordStoreReadResult(store_read_result); } -bool V4Store::HasValidData() const { - RecordBooleanWithAndWithoutSuffix("SafeBrowsing.V4Store.IsStoreValid", - has_valid_data_, store_path_); +bool V4Store::HasValidData() { + // Record every 256th time (`record_has_valid_data_counter_` is 8-bit). + if (++record_has_valid_data_counter_ == 1 || + // TODO(crbug.com/1295441): Remove the condition below. + !base::IsRunningCpuReductionExperiment()) { + RecordBooleanWithAndWithoutSuffix("SafeBrowsing.V4Store.IsStoreValid", + has_valid_data_, store_path_); + } return has_valid_data_; } @@ -581,7 +588,8 @@ ApplyUpdateResult V4Store::MergeUpdate(const HashPrefixMap& old_prefixes_map, // picked is not the same as merged. A picked element isn't merged if its // index is on the raw_removals list. int total_picked_from_old = 0; - const int* removals_iter = raw_removals ? raw_removals->begin() : nullptr; + auto removals_iter = + raw_removals ? absl::make_optional(raw_removals->begin()) : absl::nullopt; while (old_has_unmerged || additions_has_unmerged) { // If the same hash prefix appears in the existing store and the additions // list, something is clearly wrong. Discard the update. @@ -605,8 +613,8 @@ ApplyUpdateResult V4Store::MergeUpdate(const HashPrefixMap& old_prefixes_map, // prefix of size |next_smallest_prefix_size| from the old store. old_iterator_map[next_smallest_prefix_size] += next_smallest_prefix_size; - if (!raw_removals || removals_iter == raw_removals->end() || - *removals_iter != total_picked_from_old) { + if (!raw_removals || *removals_iter == raw_removals->end() || + **removals_iter != total_picked_from_old) { // Append the smallest hash to the appropriate list. hash_prefix_map_[next_smallest_prefix_size] += next_smallest_prefix_old; @@ -616,7 +624,7 @@ ApplyUpdateResult V4Store::MergeUpdate(const HashPrefixMap& old_prefixes_map, } } else { // Element not added to new map. Move the removals iterator forward. - removals_iter++; + (*removals_iter)++; } total_picked_from_old++; @@ -648,7 +656,7 @@ ApplyUpdateResult V4Store::MergeUpdate(const HashPrefixMap& old_prefixes_map, } } - if (raw_removals && removals_iter != raw_removals->end()) { + if (raw_removals && *removals_iter != raw_removals->end()) { return REMOVALS_INDEX_TOO_LARGE_FAILURE; } diff --git a/chromium/components/safe_browsing/core/browser/db/v4_store.h b/chromium/components/safe_browsing/core/browser/db/v4_store.h index 9b2cdb2957e..45a6056f917 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_store.h +++ b/chromium/components/safe_browsing/core/browser/db/v4_store.h @@ -188,7 +188,7 @@ class V4Store { // True if this store has valid contents, either from a successful read // from disk or a full update. This does not mean the checksum was verified. - virtual bool HasValidData() const; + virtual bool HasValidData(); const std::string& state() const { return state_; } @@ -432,6 +432,10 @@ class V4Store { // The size of the file on disk for this store. int64_t file_size_; + // A counter used to manage how frequently the value of `has_valid_data_` + // below is recorded. + uint8_t record_has_valid_data_counter_ = 0; + // True if the file was successfully read+parsed or was populated from // a full update. bool has_valid_data_; diff --git a/chromium/components/safe_browsing/core/browser/db/v4_test_util.cc b/chromium/components/safe_browsing/core/browser/db/v4_test_util.cc index aa704c4b872..6ed5f4fd2f1 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_test_util.cc +++ b/chromium/components/safe_browsing/core/browser/db/v4_test_util.cc @@ -46,7 +46,7 @@ TestV4Store::TestV4Store( TestV4Store::~TestV4Store() = default; -bool TestV4Store::HasValidData() const { +bool TestV4Store::HasValidData() { return true; } diff --git a/chromium/components/safe_browsing/core/browser/db/v4_test_util.h b/chromium/components/safe_browsing/core/browser/db/v4_test_util.h index cbfdb7f447e..accb167247b 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_test_util.h +++ b/chromium/components/safe_browsing/core/browser/db/v4_test_util.h @@ -31,7 +31,7 @@ class TestV4Store : public V4Store { const base::FilePath& store_path); ~TestV4Store() override; - bool HasValidData() const override; + bool HasValidData() override; void MarkPrefixAsBad(HashPrefix prefix); diff --git a/chromium/components/safe_browsing/core/browser/db/v4_update_protocol_manager_unittest.cc b/chromium/components/safe_browsing/core/browser/db/v4_update_protocol_manager_unittest.cc index f077d7adb42..82b6bb1d827 100644 --- a/chromium/components/safe_browsing/core/browser/db/v4_update_protocol_manager_unittest.cc +++ b/chromium/components/safe_browsing/core/browser/db/v4_update_protocol_manager_unittest.cc @@ -10,6 +10,7 @@ #include "base/base64.h" #include "base/bind.h" +#include "base/strings/escape.h" #include "base/test/test_simple_task_runner.h" #include "base/threading/thread_task_runner_handle.h" #include "base/time/time.h" @@ -18,7 +19,6 @@ #include "components/safe_browsing/core/browser/db/safebrowsing.pb.h" #include "components/safe_browsing/core/browser/db/util.h" #include "components/safe_browsing/core/browser/db/v4_test_util.h" -#include "net/base/escape.h" #include "net/base/load_flags.h" #include "net/base/net_errors.h" #include "net/http/http_status_code.h" diff --git a/chromium/components/safe_browsing/core/browser/password_protection/password_protection_request.cc b/chromium/components/safe_browsing/core/browser/password_protection/password_protection_request.cc index 5a491a4036c..c0fe7698e6d 100644 --- a/chromium/components/safe_browsing/core/browser/password_protection/password_protection_request.cc +++ b/chromium/components/safe_browsing/core/browser/password_protection/password_protection_request.cc @@ -8,6 +8,7 @@ #include "base/bind.h" #include "base/metrics/histogram_functions.h" +#include "base/strings/escape.h" #include "base/strings/strcat.h" #include "base/strings/utf_string_conversions.h" #include "build/build_config.h" @@ -18,7 +19,6 @@ #include "components/safe_browsing/core/common/safebrowsing_constants.h" #include "components/safe_browsing/core/common/utils.h" #include "components/url_formatter/url_formatter.h" -#include "net/base/escape.h" #include "net/base/load_flags.h" #include "net/http/http_request_headers.h" #include "net/http/http_status_code.h" @@ -55,7 +55,7 @@ std::vector<std::string> GetMatchingDomains( url_formatter::kFormatUrlOmitHTTPS | url_formatter::kFormatUrlOmitTrivialSubdomains | url_formatter::kFormatUrlTrimAfterHost, - net::UnescapeRule::SPACES, nullptr, nullptr, nullptr)); + base::UnescapeRule::SPACES, nullptr, nullptr, nullptr)); matching_domains.push_back(std::move(domain)); } return base::flat_set<std::string>(std::move(matching_domains)).extract(); diff --git a/chromium/components/safe_browsing/core/browser/password_protection/password_protection_service_base.cc b/chromium/components/safe_browsing/core/browser/password_protection/password_protection_service_base.cc index 0f73eeea82c..d93eb525666 100644 --- a/chromium/components/safe_browsing/core/browser/password_protection/password_protection_service_base.cc +++ b/chromium/components/safe_browsing/core/browser/password_protection/password_protection_service_base.cc @@ -12,6 +12,7 @@ #include "base/bind.h" #include "base/callback.h" #include "base/metrics/histogram_macros.h" +#include "base/strings/escape.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_split.h" #include "base/strings/string_util.h" @@ -25,7 +26,6 @@ #include "components/safe_browsing/core/common/features.h" #include "components/safe_browsing/core/common/utils.h" #include "google_apis/google_api_keys.h" -#include "net/base/escape.h" #include "net/base/url_util.h" using password_manager::metrics_util::PasswordType; @@ -270,7 +270,7 @@ GURL PasswordProtectionServiceBase::GetPasswordProtectionRequestUrl() { GURL url(kPasswordProtectionRequestUrl); std::string api_key = google_apis::GetAPIKey(); DCHECK(!api_key.empty()); - return url.Resolve("?key=" + net::EscapeQueryParamValue(api_key, true)); + return url.Resolve("?key=" + base::EscapeQueryParamValue(api_key, true)); } // static diff --git a/chromium/components/safe_browsing/core/browser/ping_manager.cc b/chromium/components/safe_browsing/core/browser/ping_manager.cc index 7db1aeb0016..456d3d157a4 100644 --- a/chromium/components/safe_browsing/core/browser/ping_manager.cc +++ b/chromium/components/safe_browsing/core/browser/ping_manager.cc @@ -11,12 +11,12 @@ #include "base/memory/ptr_util.h" #include "base/metrics/histogram_functions.h" #include "base/notreached.h" +#include "base/strings/escape.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "components/safe_browsing/core/browser/db/v4_protocol_manager_util.h" #include "components/safe_browsing/core/common/utils.h" #include "google_apis/google_api_keys.h" -#include "net/base/escape.h" #include "net/base/load_flags.h" #include "net/traffic_annotation/network_traffic_annotation.h" #include "services/network/public/cpp/resource_request.h" @@ -74,20 +74,32 @@ PingManager* PingManager::Create( const V4ProtocolConfig& config, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, std::unique_ptr<SafeBrowsingTokenFetcher> token_fetcher, - base::RepeatingCallback<bool()> get_should_fetch_access_token) { + base::RepeatingCallback<bool()> get_should_fetch_access_token, + WebUIDelegate* webui_delegate, + scoped_refptr<base::SequencedTaskRunner> ui_task_runner, + base::RepeatingCallback<ChromeUserPopulation()> + get_user_population_callback) { return new PingManager(config, url_loader_factory, std::move(token_fetcher), - get_should_fetch_access_token); + get_should_fetch_access_token, webui_delegate, + ui_task_runner, get_user_population_callback); } PingManager::PingManager( const V4ProtocolConfig& config, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, std::unique_ptr<SafeBrowsingTokenFetcher> token_fetcher, - base::RepeatingCallback<bool()> get_should_fetch_access_token) + base::RepeatingCallback<bool()> get_should_fetch_access_token, + WebUIDelegate* webui_delegate, + scoped_refptr<base::SequencedTaskRunner> ui_task_runner, + base::RepeatingCallback<ChromeUserPopulation()> + get_user_population_callback) : config_(config), url_loader_factory_(url_loader_factory), token_fetcher_(std::move(token_fetcher)), - get_should_fetch_access_token_(get_should_fetch_access_token) {} + get_should_fetch_access_token_(get_should_fetch_access_token), + webui_delegate_(webui_delegate), + ui_task_runner_(ui_task_runner), + get_user_population_callback_(get_user_population_callback) {} PingManager::~PingManager() {} @@ -139,19 +151,45 @@ void PingManager::ReportSafeBrowsingHit( } // Sends threat details for users who opt-in. -void PingManager::ReportThreatDetails(const std::string& report) { +PingManager::ReportThreatDetailsResult PingManager::ReportThreatDetails( + std::unique_ptr<ClientSafeBrowsingReportRequest> report) { + if (!get_user_population_callback_.is_null()) { + *report->mutable_population() = get_user_population_callback_.Run(); + } + + std::string serialized_report; + if (!report->SerializeToString(&serialized_report)) { + DLOG(ERROR) << "Unable to serialize the threat report."; + return ReportThreatDetailsResult::SERIALIZATION_ERROR; + } + if (serialized_report.empty()) { + DLOG(ERROR) << "The threat report is empty."; + return ReportThreatDetailsResult::EMPTY_REPORT; + } + if (get_should_fetch_access_token_.Run()) { token_fetcher_->Start( base::BindOnce(&PingManager::ReportThreatDetailsOnGotAccessToken, - weak_factory_.GetWeakPtr(), report)); + weak_factory_.GetWeakPtr(), serialized_report)); } else { std::string empty_access_token; - ReportThreatDetailsOnGotAccessToken(report, empty_access_token); + ReportThreatDetailsOnGotAccessToken(serialized_report, empty_access_token); } + + // The following is to log this ClientSafeBrowsingReportRequest on any open + // chrome://safe-browsing pages. + ui_task_runner_->PostTask( + FROM_HERE, + base::BindOnce(&WebUIDelegate::AddToCSBRRsSent, + // Unretained is okay because in practice, webui_delegate_ + // is a singleton + base::Unretained(webui_delegate_), std::move(report))); + + return ReportThreatDetailsResult::SUCCESS; } void PingManager::ReportThreatDetailsOnGotAccessToken( - const std::string& report, + const std::string& serialized_report, const std::string& access_token) { GURL report_url = ThreatDetailsUrl(); @@ -171,7 +209,7 @@ void PingManager::ReportThreatDetailsOnGotAccessToken( auto loader = network::SimpleURLLoader::Create(std::move(resource_request), kTrafficAnnotation); - loader->AttachStringForUpload(report, "application/octet-stream"); + loader->AttachStringForUpload(serialized_report, "application/octet-stream"); loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie( url_loader_factory_.get(), @@ -240,7 +278,7 @@ GURL PingManager::SafeBrowsingHitUrl( // Population_id should be URL-safe, but escape it and size-limit it // anyway since it came from outside Chrome. std::string up_str = - net::EscapeQueryParamValue(hit_report.population_id, true); + base::EscapeQueryParamValue(hit_report.population_id, true); if (up_str.size() > 512) { DCHECK(false) << "population_id is too long: " << up_str; up_str = "UP_STRING_TOO_LONG"; @@ -252,9 +290,10 @@ GURL PingManager::SafeBrowsingHitUrl( return GURL(base::StringPrintf( "%s&evts=%s&evtd=%s&evtr=%s&evhr=%s&evtb=%d&src=%s&m=%d%s", url.c_str(), threat_list.c_str(), - net::EscapeQueryParamValue(hit_report.malicious_url.spec(), true).c_str(), - net::EscapeQueryParamValue(hit_report.page_url.spec(), true).c_str(), - net::EscapeQueryParamValue(hit_report.referrer_url.spec(), true).c_str(), + base::EscapeQueryParamValue(hit_report.malicious_url.spec(), true) + .c_str(), + base::EscapeQueryParamValue(hit_report.page_url.spec(), true).c_str(), + base::EscapeQueryParamValue(hit_report.referrer_url.spec(), true).c_str(), hit_report.is_subresource, threat_source.c_str(), hit_report.is_metrics_reporting_active, user_population_comp.c_str())); } diff --git a/chromium/components/safe_browsing/core/browser/ping_manager.h b/chromium/components/safe_browsing/core/browser/ping_manager.h index 558e420b986..312c51463f6 100644 --- a/chromium/components/safe_browsing/core/browser/ping_manager.h +++ b/chromium/components/safe_browsing/core/browser/ping_manager.h @@ -18,6 +18,7 @@ #include "components/safe_browsing/core/browser/db/hit_report.h" #include "components/safe_browsing/core/browser/db/util.h" #include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h" +#include "components/safe_browsing/core/common/proto/csd.pb.h" #include "services/network/public/cpp/shared_url_loader_factory.h" #include "url/gurl.h" @@ -29,6 +30,25 @@ namespace safe_browsing { class PingManager : public KeyedService { public: + enum class ReportThreatDetailsResult { + SUCCESS = 0, + // There was a problem serializing the report to a string. + SERIALIZATION_ERROR = 1, + // The report is empty, so it is not sent. + EMPTY_REPORT = 2, + }; + + // Interface via which a client of this class can surface relevant events in + // WebUI. All methods must be called on the UI thread. + class WebUIDelegate { + public: + virtual ~WebUIDelegate() = default; + + // Track a client safe browsing report being sent. + virtual void AddToCSBRRsSent( + std::unique_ptr<ClientSafeBrowsingReportRequest> csbrr) = 0; + }; + PingManager(const PingManager&) = delete; PingManager& operator=(const PingManager&) = delete; @@ -39,7 +59,11 @@ class PingManager : public KeyedService { const V4ProtocolConfig& config, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, std::unique_ptr<SafeBrowsingTokenFetcher> token_fetcher, - base::RepeatingCallback<bool()> get_should_fetch_access_token); + base::RepeatingCallback<bool()> get_should_fetch_access_token, + WebUIDelegate* webui_delegate, + scoped_refptr<base::SequencedTaskRunner> ui_task_runner, + base::RepeatingCallback<ChromeUserPopulation()> + get_user_population_callback); void OnURLLoaderComplete(network::SimpleURLLoader* source, std::unique_ptr<std::string> response_body); @@ -53,9 +77,11 @@ class PingManager : public KeyedService { // SafeBrowsingtHitUrl. void ReportSafeBrowsingHit(const safe_browsing::HitReport& hit_report); - // Users can opt-in on the SafeBrowsing interstitial to send detailed - // threat reports. |report| is the serialized report. - void ReportThreatDetails(const std::string& report); + // Sends a detailed threat report after performing validation and adding extra + // details to the report. The returned object provides details on whether the + // report was successful. + ReportThreatDetailsResult ReportThreatDetails( + std::unique_ptr<ClientSafeBrowsingReportRequest> report); // Only used for tests void SetURLLoaderFactoryForTesting( @@ -65,13 +91,15 @@ class PingManager : public KeyedService { protected: friend class PingManagerTest; - // Constructs a PingManager with the given |config|, |url_loader_factory|, and - // access token fetching information. explicit PingManager( const V4ProtocolConfig& config, scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory, std::unique_ptr<SafeBrowsingTokenFetcher> token_fetcher, - base::RepeatingCallback<bool()> get_should_fetch_access_token); + base::RepeatingCallback<bool()> get_should_fetch_access_token, + WebUIDelegate* webui_delegate, + scoped_refptr<base::SequencedTaskRunner> ui_task_runner, + base::RepeatingCallback<ChromeUserPopulation()> + get_user_population_callback); private: FRIEND_TEST_ALL_PREFIXES(PingManagerTest, TestSafeBrowsingHitUrl); @@ -92,7 +120,7 @@ class PingManager : public KeyedService { // Once the user's access_token has been fetched by ReportThreatDetails (or // intentionally not fetched), attaches the token and sends the report. - void ReportThreatDetailsOnGotAccessToken(const std::string& report, + void ReportThreatDetailsOnGotAccessToken(const std::string& serialized_report, const std::string& access_token); // Track outstanding SafeBrowsing report fetchers for clean up. @@ -109,6 +137,17 @@ class PingManager : public KeyedService { // based on whether they're a signed-in ESB user. base::RepeatingCallback<bool()> get_should_fetch_access_token_; + // WebUIInfoSingleton extends PingManager::WebUIDelegate to enable the + // workaround of calling AddToCSBRRsSent in WebUIInfoSingleton without /core + // having a dependency on /content. + raw_ptr<WebUIDelegate> webui_delegate_; + + // The task runner for the UI thread. + scoped_refptr<base::SequencedTaskRunner> ui_task_runner_; + + // Pulls the user population. + base::RepeatingCallback<ChromeUserPopulation()> get_user_population_callback_; + base::WeakPtrFactory<PingManager> weak_factory_{this}; }; diff --git a/chromium/components/safe_browsing/core/browser/ping_manager_unittest.cc b/chromium/components/safe_browsing/core/browser/ping_manager_unittest.cc index d5a81d64059..8a063404620 100644 --- a/chromium/components/safe_browsing/core/browser/ping_manager_unittest.cc +++ b/chromium/components/safe_browsing/core/browser/ping_manager_unittest.cc @@ -5,13 +5,14 @@ #include "components/safe_browsing/core/browser/ping_manager.h" #include "base/base64.h" +#include "base/callback_helpers.h" #include "base/run_loop.h" +#include "base/strings/escape.h" #include "base/strings/stringprintf.h" #include "base/time/time.h" #include "base/values.h" #include "components/safe_browsing/core/browser/db/v4_test_util.h" #include "google_apis/google_api_keys.h" -#include "net/base/escape.h" #include "testing/gtest/include/gtest/gtest.h" using base::Time; @@ -29,12 +30,13 @@ class PingManagerTest : public testing::Test { std::string key = google_apis::GetAPIKey(); if (!key.empty()) { key_param_ = base::StringPrintf( - "&key=%s", net::EscapeQueryParamValue(key, true).c_str()); + "&key=%s", base::EscapeQueryParamValue(key, true).c_str()); } ping_manager_.reset( new PingManager(safe_browsing::GetTestV4ProtocolConfig(), nullptr, - nullptr, base::BindRepeating([]() { return false; }))); + nullptr, base::BindRepeating([]() { return false; }), + nullptr, nullptr, base::NullCallback())); } PingManager* ping_manager() { return ping_manager_.get(); } @@ -213,4 +215,12 @@ TEST_F(PingManagerTest, TestThreatDetailsUrl) { ping_manager()->ThreatDetailsUrl().spec()); } +TEST_F(PingManagerTest, TestReportThreatDetails_EmptyReport) { + std::unique_ptr<ClientSafeBrowsingReportRequest> report = + std::make_unique<ClientSafeBrowsingReportRequest>(); + PingManager::ReportThreatDetailsResult result = + ping_manager()->ReportThreatDetails(std::move(report)); + EXPECT_EQ(result, PingManager::ReportThreatDetailsResult::EMPTY_REPORT); +} + } // namespace safe_browsing diff --git a/chromium/components/safe_browsing/core/browser/realtime/BUILD.gn b/chromium/components/safe_browsing/core/browser/realtime/BUILD.gn index 924a79816db..6c91a33316a 100644 --- a/chromium/components/safe_browsing/core/browser/realtime/BUILD.gn +++ b/chromium/components/safe_browsing/core/browser/realtime/BUILD.gn @@ -87,6 +87,7 @@ source_set("unit_tests") { "//components/safe_browsing:buildflags", "//components/safe_browsing/core/browser:referrer_chain_provider", "//components/safe_browsing/core/browser:token_fetcher", + "//components/safe_browsing/core/browser:token_fetcher_testing_helper", "//components/safe_browsing/core/browser:verdict_cache_manager", "//components/safe_browsing/core/common", "//components/safe_browsing/core/common:safe_browsing_prefs", diff --git a/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_base.cc b/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_base.cc index fe620aad119..bb4dcd5dbd2 100644 --- a/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_base.cc +++ b/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_base.cc @@ -312,7 +312,7 @@ RealTimeUrlLookupServiceBase::GetCachedRealTimeUrlVerdict(const GURL& url) { void RealTimeUrlLookupServiceBase::MayBeCacheRealTimeUrlVerdict( const GURL& url, RTLookupResponse response) { - if (response.threat_info_size() > 0) { + if (cache_manager_ && response.threat_info_size() > 0) { base::SequencedTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(&VerdictCacheManager::CacheRealTimeUrlVerdict, cache_manager_->GetWeakPtr(), url, response, @@ -406,10 +406,10 @@ void RealTimeUrlLookupServiceBase::SendRequest( // NOTE: Pass |callback_task_runner| by copying it here as it's also needed // just below. - SendRequestInternal(std::move(resource_request), req_data, url, - access_token_string, std::move(response_callback), - callback_task_runner, - request->population().user_population()); + SendRequestInternal( + std::move(resource_request), req_data, url, access_token_string, + std::move(response_callback), callback_task_runner, + request->population().user_population(), is_sampled_report); callback_task_runner->PostTask( FROM_HERE, @@ -425,7 +425,8 @@ void RealTimeUrlLookupServiceBase::SendRequestInternal( absl::optional<std::string> access_token_string, RTLookupResponseCallback response_callback, scoped_refptr<base::SequencedTaskRunner> callback_task_runner, - ChromeUserPopulation::UserPopulation user_population) { + ChromeUserPopulation::UserPopulation user_population, + bool is_sampled_report) { std::unique_ptr<network::SimpleURLLoader> owned_loader = network::SimpleURLLoader::Create(std::move(resource_request), GetTrafficAnnotationTag()); @@ -439,7 +440,7 @@ void RealTimeUrlLookupServiceBase::SendRequestInternal( url_loader_factory_.get(), base::BindOnce(&RealTimeUrlLookupServiceBase::OnURLLoaderComplete, GetWeakPtr(), url, access_token_string, loader, - user_population, base::TimeTicks::Now(), + user_population, base::TimeTicks::Now(), is_sampled_report, std::move(callback_task_runner))); pending_requests_[owned_loader.release()] = std::move(response_callback); @@ -451,6 +452,7 @@ void RealTimeUrlLookupServiceBase::OnURLLoaderComplete( network::SimpleURLLoader* url_loader, ChromeUserPopulation::UserPopulation user_population, base::TimeTicks request_start_time, + bool is_sampled_report, scoped_refptr<base::SequencedTaskRunner> response_callback_task_runner, std::unique_ptr<std::string> response_body) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); @@ -466,9 +468,14 @@ void RealTimeUrlLookupServiceBase::OnURLLoaderComplete( int response_code = 0; if (url_loader->ResponseInfo() && url_loader->ResponseInfo()->headers) response_code = url_loader->ResponseInfo()->headers->response_code(); + std::string report_type_suffix = + is_sampled_report ? ".SampledPing" : ".NormalPing"; RecordNetworkResultWithAndWithoutSuffix("SafeBrowsing.RT.Network.Result", GetMetricSuffix(), net_error, response_code); + RecordHttpResponseOrErrorCode( + ("SafeBrowsing.RT.Network.Result" + report_type_suffix).c_str(), + net_error, response_code); if (response_code == net::HTTP_UNAUTHORIZED && access_token_string.has_value()) { @@ -481,6 +488,9 @@ void RealTimeUrlLookupServiceBase::OnURLLoaderComplete( response->ParseFromString(*response_body); RecordBooleanWithAndWithoutSuffix("SafeBrowsing.RT.IsLookupSuccessful", GetMetricSuffix(), is_rt_lookup_successful); + base::UmaHistogramBoolean( + "SafeBrowsing.RT.IsLookupSuccessful" + report_type_suffix, + is_rt_lookup_successful); is_rt_lookup_successful ? HandleLookupSuccess() : HandleLookupError(); MayBeCacheRealTimeUrlVerdict(url, *response); diff --git a/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_base.h b/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_base.h index 141c4fc2f8c..d9ecab77a37 100644 --- a/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_base.h +++ b/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_base.h @@ -229,7 +229,8 @@ class RealTimeUrlLookupServiceBase : public KeyedService { absl::optional<std::string> access_token_string, RTLookupResponseCallback response_callback, scoped_refptr<base::SequencedTaskRunner> callback_task_runner, - ChromeUserPopulation::UserPopulation user_population); + ChromeUserPopulation::UserPopulation user_population, + bool is_sampled_report); // Called when the response from the real-time lookup remote endpoint is // received. |url_loader| is the unowned loader that was used to send the @@ -243,6 +244,7 @@ class RealTimeUrlLookupServiceBase : public KeyedService { network::SimpleURLLoader* url_loader, ChromeUserPopulation::UserPopulation user_population, base::TimeTicks request_start_time, + bool is_sampled_report, scoped_refptr<base::SequencedTaskRunner> response_callback_task_runner, std::unique_ptr<std::string> response_body); diff --git a/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_unittest.cc b/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_unittest.cc index 323d521aa23..71dff161287 100644 --- a/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_unittest.cc +++ b/chromium/components/safe_browsing/core/browser/realtime/url_lookup_service_unittest.cc @@ -17,6 +17,7 @@ #include "components/safe_browsing/buildflags.h" #include "components/safe_browsing/core/browser/referrer_chain_provider.h" #include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h" +#include "components/safe_browsing/core/browser/test_safe_browsing_token_fetcher.h" #include "components/safe_browsing/core/browser/verdict_cache_manager.h" #include "components/safe_browsing/core/common/features.h" #include "components/safe_browsing/core/common/safe_browsing_prefs.h" @@ -45,28 +46,6 @@ constexpr char kTestReferrerUrl[] = "http://example.referrer/"; constexpr char kTestSubframeUrl[] = "http://iframe.example.test/"; constexpr char kTestSubframeReferrerUrl[] = "http://iframe.example.referrer/"; -class TestSafeBrowsingTokenFetcher : public SafeBrowsingTokenFetcher { - public: - TestSafeBrowsingTokenFetcher() = default; - ~TestSafeBrowsingTokenFetcher() override { - // Like SafeBrowsingTokenFetchTracer, trigger the callback when destroyed. - RunAccessTokenCallback(""); - } - - // SafeBrowsingTokenFetcher: - void Start(Callback callback) override { callback_ = std::move(callback); } - - void RunAccessTokenCallback(std::string token) { - if (callback_) - std::move(callback_).Run(token); - } - - MOCK_METHOD1(OnInvalidAccessToken, void(const std::string&)); - - private: - Callback callback_; -}; - class MockReferrerChainProvider : public ReferrerChainProvider { public: virtual ~MockReferrerChainProvider() = default; @@ -1149,11 +1128,6 @@ TEST_F(RealTimeUrlLookupServiceTest, TestShutdown_CallbackNotPostedOnShutdown) { TEST_F(RealTimeUrlLookupServiceTest, TestShutdown_CacheManagerReset) { GURL url("https://a.example.test/path1/path2"); - // Post a task to cache_manager_ to cache the verdict. - MayBeCacheRealTimeUrlVerdict(url, RTLookupResponse::ThreatInfo::DANGEROUS, - RTLookupResponse::ThreatInfo::SOCIAL_ENGINEERING, - 60, "a.example.test/path1/path2", - RTLookupResponse::ThreatInfo::COVERING_MATCH); // Shutdown and delete depending objects. rt_service()->Shutdown(); @@ -1161,6 +1135,12 @@ TEST_F(RealTimeUrlLookupServiceTest, TestShutdown_CacheManagerReset) { content_setting_map_->ShutdownOnUIThread(); content_setting_map_.reset(); + // Post a task to cache_manager_ to cache the verdict. + MayBeCacheRealTimeUrlVerdict(url, RTLookupResponse::ThreatInfo::DANGEROUS, + RTLookupResponse::ThreatInfo::SOCIAL_ENGINEERING, + 60, "a.example.test/path1/path2", + RTLookupResponse::ThreatInfo::COVERING_MATCH); + // The task to cache_manager_ should be cancelled and not cause crash. task_environment_.RunUntilIdle(); } @@ -1226,8 +1206,6 @@ TEST_F(RealTimeUrlLookupServiceTest, // Enable extended reporting. EnableExtendedReporting(); rt_service()->set_bypass_probability_for_tests(true); - // When feature is not enabled, a sampled ping should not be sent. - EXPECT_FALSE(CanSendRTSampleRequest()); feature_list_.InitAndDisableFeature( safe_browsing::kSendSampledPingsForProtegoAllowlistDomains); // After enabling the feature, a sampled ping should be sent. diff --git a/chromium/components/safe_browsing/core/browser/tailored_security_service/OWNERS b/chromium/components/safe_browsing/core/browser/tailored_security_service/OWNERS index 3a311df9c96..4f674dd2204 100644 --- a/chromium/components/safe_browsing/core/browser/tailored_security_service/OWNERS +++ b/chromium/components/safe_browsing/core/browser/tailored_security_service/OWNERS @@ -1,2 +1 @@ -bdea@chromium.org drubery@chromium.org diff --git a/chromium/components/safe_browsing/core/browser/tailored_security_service/tailored_security_service.h b/chromium/components/safe_browsing/core/browser/tailored_security_service/tailored_security_service.h index 3a7a7836579..2f85f3af1dd 100644 --- a/chromium/components/safe_browsing/core/browser/tailored_security_service/tailored_security_service.h +++ b/chromium/components/safe_browsing/core/browser/tailored_security_service/tailored_security_service.h @@ -183,7 +183,7 @@ class TailoredSecurityService : public KeyedService { bool is_shut_down_ = false; // The preferences for the given profile. - PrefService* prefs_; + raw_ptr<PrefService> prefs_; // This is used to observe when sync users update their Tailored Security // setting. diff --git a/chromium/components/safe_browsing/core/browser/test_safe_browsing_token_fetcher.cc b/chromium/components/safe_browsing/core/browser/test_safe_browsing_token_fetcher.cc new file mode 100644 index 00000000000..5f07f5324ed --- /dev/null +++ b/chromium/components/safe_browsing/core/browser/test_safe_browsing_token_fetcher.cc @@ -0,0 +1,27 @@ +// 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. + +#include "components/safe_browsing/core/browser/test_safe_browsing_token_fetcher.h" + +namespace safe_browsing { + +TestSafeBrowsingTokenFetcher::TestSafeBrowsingTokenFetcher() = default; +TestSafeBrowsingTokenFetcher::~TestSafeBrowsingTokenFetcher() { + // Like SafeBrowsingTokenFetchTracer, trigger the callback when destroyed. + RunAccessTokenCallback(""); +} +void TestSafeBrowsingTokenFetcher::Start(Callback callback) { + callback_ = std::move(callback); + was_start_called_ = true; +} +void TestSafeBrowsingTokenFetcher::RunAccessTokenCallback(std::string token) { + if (callback_) { + std::move(callback_).Run(token); + } +} +bool TestSafeBrowsingTokenFetcher::WasStartCalled() { + return was_start_called_; +} + +} // namespace safe_browsing diff --git a/chromium/components/safe_browsing/core/browser/test_safe_browsing_token_fetcher.h b/chromium/components/safe_browsing/core/browser/test_safe_browsing_token_fetcher.h new file mode 100644 index 00000000000..78b7cf98eb0 --- /dev/null +++ b/chromium/components/safe_browsing/core/browser/test_safe_browsing_token_fetcher.h @@ -0,0 +1,31 @@ +// 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 COMPONENTS_SAFE_BROWSING_CORE_BROWSER_TEST_SAFE_BROWSING_TOKEN_FETCHER_H_ +#define COMPONENTS_SAFE_BROWSING_CORE_BROWSER_TEST_SAFE_BROWSING_TOKEN_FETCHER_H_ + +#include "base/callback.h" +#include "components/safe_browsing/core/browser/safe_browsing_token_fetcher.h" +#include "testing/gmock/include/gmock/gmock.h" + +namespace safe_browsing { + +class TestSafeBrowsingTokenFetcher : public SafeBrowsingTokenFetcher { + public: + TestSafeBrowsingTokenFetcher(); + ~TestSafeBrowsingTokenFetcher() override; + + void Start(Callback callback) override; + void RunAccessTokenCallback(std::string token); + bool WasStartCalled(); + MOCK_METHOD1(OnInvalidAccessToken, void(const std::string&)); + + private: + Callback callback_; + bool was_start_called_ = false; +}; + +} // namespace safe_browsing + +#endif // COMPONENTS_SAFE_BROWSING_CORE_BROWSER_TEST_SAFE_BROWSING_TOKEN_FETCHER_H_ diff --git a/chromium/components/safe_browsing/core/browser/verdict_cache_manager.cc b/chromium/components/safe_browsing/core/browser/verdict_cache_manager.cc index 7ee2f1b7557..ab059db7f06 100644 --- a/chromium/components/safe_browsing/core/browser/verdict_cache_manager.cc +++ b/chromium/components/safe_browsing/core/browser/verdict_cache_manager.cc @@ -428,6 +428,11 @@ void VerdictCacheManager::Shutdown() { history_service_observation_.Reset(); pref_change_registrar_.RemoveAll(); sync_observer_.reset(); + + // Clear references to other KeyedServices. + content_settings_ = nullptr; + + is_shut_down_ = true; weak_factory_.InvalidateWeakPtrs(); } @@ -438,6 +443,9 @@ void VerdictCacheManager::CachePhishGuardVerdict( ReusedPasswordAccountType password_type, const LoginReputationClientResponse& verdict, const base::Time& receive_time) { + if (is_shut_down_) { + return; + } DCHECK(content_settings_); DCHECK(trigger_type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE || trigger_type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT); @@ -494,6 +502,9 @@ VerdictCacheManager::GetCachedPhishGuardVerdict( LoginReputationClientResponse* out_response) { DCHECK(trigger_type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE || trigger_type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT); + if (is_shut_down_) { + return LoginReputationClientResponse::VERDICT_TYPE_UNSPECIFIED; + } std::string type_key = GetKeyOfTypeFromTriggerType(trigger_type, password_type); @@ -505,6 +516,9 @@ VerdictCacheManager::GetCachedPhishGuardVerdict( size_t VerdictCacheManager::GetStoredPhishGuardVerdictCount( LoginReputationClientRequest::TriggerType trigger_type) { + if (is_shut_down_) { + return 0; + } DCHECK(content_settings_); DCHECK(trigger_type == LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE || trigger_type == LoginReputationClientRequest::PASSWORD_REUSE_EVENT); @@ -535,6 +549,9 @@ size_t VerdictCacheManager::GetStoredPhishGuardVerdictCount( } size_t VerdictCacheManager::GetStoredRealTimeUrlCheckVerdictCount() { + if (is_shut_down_) { + return 0; + } // If we have already computed this, return its value. if (stored_verdict_count_real_time_url_check_.has_value()) return stored_verdict_count_real_time_url_check_.value(); @@ -558,6 +575,9 @@ void VerdictCacheManager::CacheRealTimeUrlVerdict( const GURL& url, const RTLookupResponse& verdict, const base::Time& receive_time) { + if (is_shut_down_) { + return; + } std::vector<std::string> visited_cache_expressions; for (const auto& threat_info : verdict.threat_info()) { // If |cache_expression_match_type| is unspecified, ignore this entry. @@ -617,6 +637,9 @@ RTLookupResponse::ThreatInfo::VerdictType VerdictCacheManager::GetCachedRealTimeUrlVerdict( const GURL& url, RTLookupResponse::ThreatInfo* out_threat_info) { + if (is_shut_down_) { + return RTLookupResponse::ThreatInfo::VERDICT_TYPE_UNSPECIFIED; + } return GetMostMatchingCachedVerdictWithHostAndPathMatching< RTLookupResponse::ThreatInfo>( url, kRealTimeUrlCacheKey, content_settings_, @@ -663,6 +686,9 @@ void VerdictCacheManager::ScheduleNextCleanUpAfterInterval( } void VerdictCacheManager::CleanUpExpiredVerdicts() { + if (is_shut_down_) { + return; + } DCHECK(content_settings_); SCOPED_UMA_HISTOGRAM_TIMER("SafeBrowsing.RT.CacheManager.CleanUpTime"); CleanUpExpiredPhishGuardVerdicts(); @@ -842,6 +868,9 @@ bool VerdictCacheManager::RemoveExpiredRealTimeUrlCheckVerdicts( void VerdictCacheManager::RemoveContentSettingsOnURLsDeleted( bool all_history, const history::URLRows& deleted_rows) { + if (is_shut_down_) { + return; + } DCHECK(content_settings_); if (all_history) { diff --git a/chromium/components/safe_browsing/core/browser/verdict_cache_manager.h b/chromium/components/safe_browsing/core/browser/verdict_cache_manager.h index 1a7bda21a02..f46d70cc51d 100644 --- a/chromium/components/safe_browsing/core/browser/verdict_cache_manager.h +++ b/chromium/components/safe_browsing/core/browser/verdict_cache_manager.h @@ -203,6 +203,8 @@ class VerdictCacheManager : public history::HistoryServiceObserver, std::unique_ptr<SafeBrowsingSyncObserver> sync_observer_; + bool is_shut_down_ = false; + base::WeakPtrFactory<VerdictCacheManager> weak_factory_{this}; static bool has_artificial_unsafe_url_; diff --git a/chromium/components/safe_browsing/core/browser/verdict_cache_manager_unittest.cc b/chromium/components/safe_browsing/core/browser/verdict_cache_manager_unittest.cc index b8e1b1f7421..4be882da08a 100644 --- a/chromium/components/safe_browsing/core/browser/verdict_cache_manager_unittest.cc +++ b/chromium/components/safe_browsing/core/browser/verdict_cache_manager_unittest.cc @@ -860,4 +860,27 @@ TEST_F(VerdictCacheManagerTest, TestClearTokenOnSyncStateChanged) { ASSERT_FALSE(token.has_token_value()); } +TEST_F(VerdictCacheManagerTest, TestShutdown) { + cache_manager_->Shutdown(); + RTLookupResponse rt_response; + // Call to cache_manager after shutdown should not cause a crash. + cache_manager_->CacheRealTimeUrlVerdict(GURL("https://www.example.com/"), + rt_response, base::Time::Now()); + RTLookupResponse::ThreatInfo out_rt_verdict; + cache_manager_->GetCachedRealTimeUrlVerdict( + GURL("https://www.example.com/path"), &out_rt_verdict); + LoginReputationClientResponse pg_response; + ReusedPasswordAccountType password_type; + cache_manager_->CachePhishGuardVerdict( + LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE, password_type, + pg_response, base::Time::Now()); + cache_manager_->GetStoredPhishGuardVerdictCount( + LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE); + LoginReputationClientResponse out_pg_verdict; + cache_manager_->GetCachedPhishGuardVerdict( + GURL("https://www.example.com/path"), + LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE, password_type, + &out_pg_verdict); +} + } // namespace safe_browsing diff --git a/chromium/components/safe_browsing/core/common/fbs/client_model.fbs b/chromium/components/safe_browsing/core/common/fbs/client_model.fbs index 2cc099860a4..7bb1c227223 100644 --- a/chromium/components/safe_browsing/core/common/fbs/client_model.fbs +++ b/chromium/components/safe_browsing/core/common/fbs/client_model.fbs @@ -39,6 +39,7 @@ table ClientSideModel { tflite_model_input_width: int (deprecated); tflite_model_input_height: int (deprecated); tflite_metadata:safe_browsing.flat.TfLiteModelMetadata; + dom_model_version:int; } root_type ClientSideModel; diff --git a/chromium/components/safe_browsing/core/common/features.cc b/chromium/components/safe_browsing/core/common/features.cc index 15ac35d23fa..f516af703a2 100644 --- a/chromium/components/safe_browsing/core/common/features.cc +++ b/chromium/components/safe_browsing/core/common/features.cc @@ -46,13 +46,25 @@ extern const base::Feature kClientSideDetectionModelTag{ const base::Feature kClientSideDetectionReferrerChain{ "ClientSideDetectionReferrerChain", base::FEATURE_ENABLED_BY_DEFAULT}; +const base::Feature kClientSideDetectionKillswitch{ + "ClientSideDetectionKillswitch", +#if BUILDFLAG(IS_MAC) + base::FEATURE_ENABLED_BY_DEFAULT +#else + base::FEATURE_DISABLED_BY_DEFAULT +#endif +}; + const base::Feature kConnectorsScanningAccessToken{ - "ConnectorsScanningAccessToken", base::FEATURE_DISABLED_BY_DEFAULT}; + "ConnectorsScanningAccessToken", base::FEATURE_ENABLED_BY_DEFAULT}; -// TODO(b/197749390): Add tests for this feature being enabled when it's -// finalized. const base::Feature kConnectorsScanningReportOnlyUI{ - "ConnectorsScanningReportOnlyUI", base::FEATURE_DISABLED_BY_DEFAULT}; + "ConnectorsScanningReportOnlyUI", base::FEATURE_ENABLED_BY_DEFAULT}; + +#if BUILDFLAG(IS_ANDROID) +const base::Feature kCreateSafebrowsingOnStartup{ + "CreateSafebrowsingOnStartup", base::FEATURE_DISABLED_BY_DEFAULT}; +#endif const base::Feature kDelayedWarnings{"SafeBrowsingDelayedWarnings", base::FEATURE_DISABLED_BY_DEFAULT}; @@ -76,6 +88,10 @@ const base::Feature kEnhancedProtection { #endif }; +const base::Feature kEnhancedProtectionPhase2IOS{ + "SafeBrowsingEnhancedProtectionPhase2IOS", + base::FEATURE_DISABLED_BY_DEFAULT}; + const base::Feature kExtensionTelemetry{"SafeBrowsingExtensionTelemetry", base::FEATURE_DISABLED_BY_DEFAULT}; @@ -86,6 +102,11 @@ const base::Feature kExtensionTelemetryPersistence{ const base::FeatureParam<int> kExtensionTelemetryUploadIntervalSeconds{ &kExtensionTelemetry, "UploadIntervalSeconds", /*default_value=*/3600}; + +const base::FeatureParam<int> kExtensionTelemetryWritesPerInterval{ + &kExtensionTelemetry, "NumberOfWritesInInterval", + /*default_value=*/4}; + const base::Feature kExtensionTelemetryTabsExecuteScriptSignal{ "SafeBrowsingExtensionTelemetryTabsExecuteScriptSignal", base::FEATURE_DISABLED_BY_DEFAULT}; @@ -110,9 +131,6 @@ const base::Feature kOmitNonUserGesturesFromReferrerChain{ const base::Feature kSafeBrowsingCsbrrWithToken{ "SafeBrowsingCsbrrWithToken", base::FEATURE_DISABLED_BY_DEFAULT}; -const base::Feature kSafeBrowsingCTDownloadWarning{ - "SafeBrowsingCTDownloadWarning", base::FEATURE_DISABLED_BY_DEFAULT}; - const base::Feature kSafeBrowsingEnterpriseCsd{ "SafeBrowsingEnterpriseCsd", base::FEATURE_ENABLED_BY_DEFAULT}; @@ -129,7 +147,7 @@ const base::Feature kSafeBrowsingRemoveCookiesInAuthRequests{ const base::Feature kSendSampledPingsForProtegoAllowlistDomains{ "SafeBrowsingSendSampledPingsForProtegoAllowlistDomains", - base::FEATURE_DISABLED_BY_DEFAULT}; + base::FEATURE_ENABLED_BY_DEFAULT}; const base::Feature kSuspiciousSiteTriggerQuotaFeature{ "SafeBrowsingSuspiciousSiteTriggerQuota", base::FEATURE_ENABLED_BY_DEFAULT}; @@ -137,10 +155,6 @@ const base::Feature kSuspiciousSiteTriggerQuotaFeature{ const base::Feature kThreatDomDetailsTagAndAttributeFeature{ "ThreatDomDetailsTagAttributes", base::FEATURE_DISABLED_BY_DEFAULT}; -const base::Feature kTriggerThrottlerDailyQuotaFeature{ - "SafeBrowsingTriggerThrottlerDailyQuota", - base::FEATURE_DISABLED_BY_DEFAULT}; - const base::Feature kUseNewDownloadWarnings{"UseNewDownloadWarnings", base::FEATURE_DISABLED_BY_DEFAULT}; @@ -168,6 +182,7 @@ constexpr struct { {&kDelayedWarnings, true}, {&kDownloadBubble, true}, {&kEnhancedProtection, true}, + {&kEnhancedProtectionPhase2IOS, true}, {&kExtensionTelemetry, true}, {&kExtensionTelemetryReportContactedHosts, true}, {&kExtensionTelemetryPersistence, true}, @@ -179,7 +194,6 @@ constexpr struct { {&kSendSampledPingsForProtegoAllowlistDomains, true}, {&kSuspiciousSiteTriggerQuotaFeature, true}, {&kThreatDomDetailsTagAndAttributeFeature, false}, - {&kTriggerThrottlerDailyQuotaFeature, false}, }; // Adds the name and the enabled/disabled status of a given feature. diff --git a/chromium/components/safe_browsing/core/common/features.h b/chromium/components/safe_browsing/core/common/features.h index 8c66f7be89f..ca50054e3a1 100644 --- a/chromium/components/safe_browsing/core/common/features.h +++ b/chromium/components/safe_browsing/core/common/features.h @@ -43,6 +43,13 @@ const char kClientSideDetectionTagParamName[] = "reporter_omaha_tag"; // Enables client side detection referrer chain. extern const base::Feature kClientSideDetectionReferrerChain; +// Killswitch for client side phishing detection. Since client side models are +// run on a large fraction of navigations, crashes due to the model are very +// impactful, even if only a small fraction of users have a bad version of the +// model. This Finch flag allows us to remediate long-tail component versions +// while we fix the root cause. +extern const base::Feature kClientSideDetectionKillswitch; + // Controls whether an access token is attached to scanning requests triggered // by enterprise Connectors. extern const base::Feature kConnectorsScanningAccessToken; @@ -53,6 +60,13 @@ extern const base::Feature kConnectorsScanningAccessToken; // instead of just showing an "Open Now" button with the blocking UI. extern const base::Feature kConnectorsScanningReportOnlyUI; +// Controls whether to connect to the Safe Browsing service early on startup. +// The alternative is to connect as soon as the first Safe Browsing check is +// made associated with a URK request. Android only. On this platform getting +// the notification about the success of establishing the connection can be +// delayed by several seconds. +extern const base::Feature kCreateSafebrowsingOnStartup; + // Controls whether the delayed warning experiment is enabled. extern const base::Feature kDelayedWarnings; // True if mouse clicks should undelay the warnings immediately when delayed @@ -65,6 +79,9 @@ extern const base::Feature kDownloadBubble; // Enables Enhanced Safe Browsing. extern const base::Feature kEnhancedProtection; +// Phase 2 of Enhanced Safe Browsing changes. +extern const base::Feature kEnhancedProtectionPhase2IOS; + // Enables collection of signals related to extension activity and uploads // of telemetry reports to SB servers. extern const base::Feature kExtensionTelemetry; @@ -75,6 +92,11 @@ extern const base::Feature kExtensionTelemetryPersistence; // Specifies the upload interval for extension telemetry reports. extern const base::FeatureParam<int> kExtensionTelemetryUploadIntervalSeconds; + +// Specifies the number of writes the telemetry service will perform during +// a full upload interval. +extern const base::FeatureParam<int> kExtensionTelemetryWritesPerInterval; + // Enables collection of telemetry signal whenever an extension invokes the // tabs.executeScript API call. extern const base::Feature kExtensionTelemetryTabsExecuteScriptSignal; @@ -96,10 +118,6 @@ extern const base::Feature kOmitNonUserGesturesFromReferrerChain; // for Enhanced Safe Browsing users extern const base::Feature kSafeBrowsingCsbrrWithToken; -// Controls whether users will see an account compromise specific warning -// when Safe Browsing determines a file is associated with stealing cookies. -extern const base::Feature kSafeBrowsingCTDownloadWarning; - // Controls whether we are performing enterprise download checks for users // with the appropriate policies enabled. extern const base::Feature kSafeBrowsingEnterpriseCsd; @@ -138,16 +156,6 @@ extern const base::Feature kTailoredSecurityIntegration; // be lower case. extern const base::Feature kThreatDomDetailsTagAndAttributeFeature; -// Controls the daily quota for data collection triggers. It's a single param -// containing a comma-separated list of pairs. The format of the param is -// "T1,Q1,T2,Q2,...Tn,Qn", where Tx is a TriggerType and Qx is how many reports -// that trigger is allowed to send per day. -// TODO(crbug.com/744869): This param should be deprecated after ad sampler -// launch in favour of having a unique quota feature and param per trigger. -// Having a single shared feature makes it impossible to run multiple trigger -// trials simultaneously. -extern const base::Feature kTriggerThrottlerDailyQuotaFeature; - // Controls whether Chrome uses new download warning UX. extern const base::Feature kUseNewDownloadWarnings; diff --git a/chromium/components/safe_browsing/core/common/proto/client_model.proto b/chromium/components/safe_browsing/core/common/proto/client_model.proto index 3c74599ef8a..920711a5700 100644 --- a/chromium/components/safe_browsing/core/common/proto/client_model.proto +++ b/chromium/components/safe_browsing/core/common/proto/client_model.proto @@ -70,9 +70,11 @@ message ClientSideModel { // Page terms in page_term contain at most this many page words. required int32 max_words_per_term = 5; - // Model version number. Every model that we train should have a different - // version number and it should always be larger than the previous model - // version. + optional int32 dom_model_version = 18; + + // The overall client model version number. Every model update should have a + // different version number and it should always be larger than the previous + // model version. optional int32 version = 6; // List of known bad IP subnets. @@ -107,7 +109,7 @@ message ClientSideModel { optional TfLiteModelMetadata tflite_metadata = 17; - // next available tag number: 18 + // next available tag number: 19 } message TfLiteModelMetadata { diff --git a/chromium/components/safe_browsing/core/common/proto/csd.proto b/chromium/components/safe_browsing/core/common/proto/csd.proto index 0ab63881eee..4112daa103f 100644 --- a/chromium/components/safe_browsing/core/common/proto/csd.proto +++ b/chromium/components/safe_browsing/core/common/proto/csd.proto @@ -109,6 +109,18 @@ message ChromeUserPopulation { // Note: This field is set as repeated to support tokens from multiple // sources. repeated PageLoadToken page_load_tokens = 14; + + // The current state of account-level enhanced safe browsing (A-ESB) as is + // known by the client. This is an optional field and represents the state of + // A-ESB as the client has observed it to be. This value will be set for sync + // users as well as signed-in users. The state on the server may be + // different from the value that the client has when setting this field. + // See: go/esb-mms-integration-dd. + optional bool is_aesb_enabled = 15; + + // The time when the account-level enhanced safe browsing (A-ESB) bit state + // was last sent updated on the client. This is an optional field. + optional int64 aesb_last_update_time_windows_epoch_micros = 16; } message ClientPhishingRequest { @@ -148,6 +160,9 @@ message ClientPhishingRequest { // sent to the scorer and which resulted in client_score being computed. repeated Feature feature_map = 5; + // The version of the DOM model used for classification + optional int32 dom_model_version = 27; + // The version number of the model that was used to compute the client-score. // Copied from ClientSideModel.version(). optional int32 model_version = 6; @@ -215,7 +230,7 @@ message ClientPhishingRequest { // users. optional VisualFeatures visual_features = 26; - // next available tag number: 27. + // next available tag number: 28. } message ClientPhishingResponse { diff --git a/chromium/components/safe_browsing/core/common/safe_browsing_prefs.cc b/chromium/components/safe_browsing/core/common/safe_browsing_prefs.cc index f63da778f92..cf55188e9e5 100644 --- a/chromium/components/safe_browsing/core/common/safe_browsing_prefs.cc +++ b/chromium/components/safe_browsing/core/common/safe_browsing_prefs.cc @@ -9,6 +9,7 @@ #include "base/metrics/histogram_macros.h" #include "base/notreached.h" #include "base/strings/string_number_conversions.h" +#include "base/time/time.h" #include "components/pref_registry/pref_registry_syncable.h" #include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_service.h" @@ -111,7 +112,8 @@ const char kAccountTailoredSecurityShownNotification[] = "safebrowsing.aesb_shown_notification"; const char kEnhancedProtectionEnabledViaTailoredSecurity[] = "safebrowsing.esb_enabled_via_tailored_security"; - +const char kExtensionTelemetryLastUploadTime[] = + "safebrowsing.extension_telemetry_last_upload_time"; } // namespace prefs namespace safe_browsing { @@ -231,6 +233,17 @@ void RegisterProfilePrefs(PrefRegistrySimple* registry) { prefs::kAccountTailoredSecurityShownNotification, false); registry->RegisterBooleanPref( prefs::kEnhancedProtectionEnabledViaTailoredSecurity, false); + registry->RegisterTimePref(prefs::kExtensionTelemetryLastUploadTime, + base::Time::Now()); +} + +base::Time GetLastUploadTimeForExtensionTelemetry(PrefService& prefs) { + return (prefs.GetTime(prefs::kExtensionTelemetryLastUploadTime)); +} + +void SetLastUploadTimeForExtensionTelemetry(PrefService& prefs, + const base::Time& time) { + prefs.SetTime(prefs::kExtensionTelemetryLastUploadTime, time); } void RegisterLocalStatePrefs(PrefRegistrySimple* registry) { diff --git a/chromium/components/safe_browsing/core/common/safe_browsing_prefs.h b/chromium/components/safe_browsing/core/common/safe_browsing_prefs.h index 05136ba29e4..c32c617df1d 100644 --- a/chromium/components/safe_browsing/core/common/safe_browsing_prefs.h +++ b/chromium/components/safe_browsing/core/common/safe_browsing_prefs.h @@ -18,6 +18,10 @@ class PrefRegistrySimple; class PrefService; class GURL; +namespace base { +class Time; +} + namespace prefs { // A list of times at which CSD pings were sent. extern const char kSafeBrowsingCsdPingTimestamps[]; @@ -123,6 +127,10 @@ extern const char kAccountTailoredSecurityShownNotification[]; // account tailored security. extern const char kEnhancedProtectionEnabledViaTailoredSecurity[]; +// The last time the Extension Telemetry Service successfully +// uploaded its data. +extern const char kExtensionTelemetryLastUploadTime[]; + } // namespace prefs namespace safe_browsing { @@ -251,6 +259,14 @@ void SetExtendedReportingPrefAndMetric(PrefService* prefs, // This variant is used to simplify test code by omitting the location. void SetExtendedReportingPrefForTests(PrefService* prefs, bool value); +// Sets the last time the Extension Telemetry Service successfully uploaded +// its data. +void SetLastUploadTimeForExtensionTelemetry(PrefService& prefs, + const base::Time& time); + +// Returns the `kExtensionTelemetryLastUploadTime` user preference. +base::Time GetLastUploadTimeForExtensionTelemetry(PrefService& prefs); + // Sets the currently active Safe Browsing Enhanced Protection to the specified // value. void SetEnhancedProtectionPrefForTests(PrefService* prefs, bool value); diff --git a/chromium/components/safe_browsing/core/common/utils.cc b/chromium/components/safe_browsing/core/common/utils.cc index e9f7c2984a5..b43b1db95cb 100644 --- a/chromium/components/safe_browsing/core/common/utils.cc +++ b/chromium/components/safe_browsing/core/common/utils.cc @@ -51,7 +51,7 @@ std::string ShortURLForReporting(const GURL& url) { ChromeUserPopulation::ProfileManagementStatus GetProfileManagementStatus( const policy::BrowserPolicyConnector* bpc) { #if BUILDFLAG(IS_WIN) - if (base::IsMachineExternallyManaged()) + if (base::IsManagedDevice()) return ChromeUserPopulation::ENTERPRISE_MANAGED; else return ChromeUserPopulation::NOT_MANAGED; |