Commit aad87b1d authored by Varun Khaneja's avatar Varun Khaneja Committed by Commit Bot

Start querying the high confidence allow list for Safe Browsing.

Here's how it works:
1. Client calls V4LDbM::CheckUrlForHighConfidenceAllowlist()
2. If the hash prefixes aren't ready or Safe Browsing is disabled, it
   returns NO_MATCH i.e. can't determine if the URL is safe.
3. If no match is found in the local database, it returns NO_MATCH to
   indicate the URL isn't on the allowlist.
4. If the full hash of the URL matches a full hash in the local
   database, MATCH is returned to indicate no further lookup is required.
5. If a hash prefix match is found, it initiates a full hash lookup
   and returns ASYNC to indicate that the client will be notified later.

If the returned value is ASYNC, a full hash request is sent to the
backend to check if the URL's full hash matches a full hash of a URL on
the high confidence allowlist. Once the response is received, the client's
|OnCheckUrlForHighConfidenceAllowlist| method is called with the final
result, whenever it is available (or a timeout happens).

This entire feature is behind a different Finch flag:
SafeBrowsingRealTimeUrlLookupEnabled

R=drubery

Bug: 963165, 966646
Change-Id: Ifdc24d3f0b2e68013da5080febbe2aaf94edded4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1661288
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Reviewed-by: default avatarNathan Parker <nparker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671469}
parent 6c108a6b
...@@ -236,6 +236,15 @@ bool RemoteSafeBrowsingDatabaseManager::CheckResourceUrl(const GURL& url, ...@@ -236,6 +236,15 @@ bool RemoteSafeBrowsingDatabaseManager::CheckResourceUrl(const GURL& url,
return true; return true;
} }
AsyncMatch
RemoteSafeBrowsingDatabaseManager::CheckUrlForHighConfidenceAllowlist(
const GURL& url,
Client* client) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
NOTREACHED();
return AsyncMatch::NO_MATCH;
}
bool RemoteSafeBrowsingDatabaseManager::CheckUrlForSubresourceFilter( bool RemoteSafeBrowsingDatabaseManager::CheckUrlForSubresourceFilter(
const GURL& url, const GURL& url,
Client* client) { Client* client) {
......
...@@ -48,6 +48,8 @@ class RemoteSafeBrowsingDatabaseManager : public SafeBrowsingDatabaseManager { ...@@ -48,6 +48,8 @@ class RemoteSafeBrowsingDatabaseManager : public SafeBrowsingDatabaseManager {
Client* client) override; Client* client) override;
AsyncMatch CheckCsdWhitelistUrl(const GURL& url, Client* client) override; AsyncMatch CheckCsdWhitelistUrl(const GURL& url, Client* client) override;
bool CheckResourceUrl(const GURL& url, Client* client) override; bool CheckResourceUrl(const GURL& url, Client* client) override;
AsyncMatch CheckUrlForHighConfidenceAllowlist(const GURL& url,
Client* client) override;
bool CheckUrlForSubresourceFilter(const GURL& url, Client* client) override; bool CheckUrlForSubresourceFilter(const GURL& url, Client* client) override;
bool MatchDownloadWhitelistString(const std::string& str) override; bool MatchDownloadWhitelistString(const std::string& str) override;
bool MatchDownloadWhitelistUrl(const GURL& url) override; bool MatchDownloadWhitelistUrl(const GURL& url) override;
......
...@@ -330,6 +330,7 @@ source_set("v4_local_database_manager_unittest") { ...@@ -330,6 +330,7 @@ source_set("v4_local_database_manager_unittest") {
":v4_test_util", ":v4_test_util",
"//base", "//base",
"//base/test:test_support", "//base/test:test_support",
"//components/safe_browsing:features",
"//content/test:test_support", "//content/test:test_support",
"//net", "//net",
"//net:test_support", "//net:test_support",
...@@ -412,6 +413,7 @@ source_set("unit_tests_desktop") { ...@@ -412,6 +413,7 @@ source_set("unit_tests_desktop") {
":v4_update_protocol_manager", ":v4_update_protocol_manager",
"//base", "//base",
"//components/prefs:test_support", "//components/prefs:test_support",
"//components/safe_browsing:features",
"//components/safe_browsing/common:safe_browsing_prefs", "//components/safe_browsing/common:safe_browsing_prefs",
"//content/test:test_support", "//content/test:test_support",
"//crypto", "//crypto",
......
...@@ -30,12 +30,22 @@ class SharedURLLoaderFactory; ...@@ -30,12 +30,22 @@ class SharedURLLoaderFactory;
namespace safe_browsing { namespace safe_browsing {
// Value returned by some Check*Whitelist() calls that may or may not have an // Value returned by some functions that check an allowlist and may or may not
// immediate answer. // have an immediate answer.
enum class AsyncMatch { enum class AsyncMatch {
ASYNC, // No answer yet -- Client will get a callback // If a hash prefix on the allowlist matches any of the computed hashes for
MATCH, // URL matches the list. No callback. // the URL. In this case, the callback method on the client is called back
NO_MATCH, // URL doesn't match. No callback. // later with the result.
ASYNC,
// If a full hash on the allowlist matches any of the computed hashes for the
// URL. The callback function isn't called.
MATCH,
// If Safe Browsing isn't enabled, or the allowlist hasn't been sync'd yet, or
// when no hash prefix or full hash in the allowlist matches the computed
// hashes of the URL. The callback function isn't called.
NO_MATCH,
}; };
struct V4ProtocolConfig; struct V4ProtocolConfig;
...@@ -79,6 +89,11 @@ class SafeBrowsingDatabaseManager ...@@ -79,6 +89,11 @@ class SafeBrowsingDatabaseManager
// Called when the result of checking a whitelist is known. // Called when the result of checking a whitelist is known.
// Currently only used for CSD whitelist. // Currently only used for CSD whitelist.
virtual void OnCheckWhitelistUrlResult(bool is_whitelisted) {} virtual void OnCheckWhitelistUrlResult(bool is_whitelisted) {}
// Called when the result of checking the high-confidence allowlist is
// known.
virtual void OnCheckUrlForHighConfidenceAllowlist(bool is_high_confidence) {
}
}; };
// //
...@@ -118,23 +133,23 @@ class SafeBrowsingDatabaseManager ...@@ -118,23 +133,23 @@ class SafeBrowsingDatabaseManager
// //
// Called on the IO thread to check if the given url has blacklisted APIs. // Called on the IO thread to check if the given url has blacklisted APIs.
// "client" is called asynchronously with the result when it is ready. Callers // |client| is called asynchronously with the result when it is ready. Callers
// should wait for results before calling this method a second time with the // should wait for results before calling this method a second time with the
// same client. This method has the same implementation for both the local and // same client. This method has the same implementation for both the local and
// remote database managers since it pings Safe Browsing servers directly // remote database managers since it pings Safe Browsing servers directly
// without accessing the database at all. Returns true if we can // without accessing the database at all. Returns true if we can
// synchronously determine that the url is safe. Otherwise it returns false, // synchronously determine that the url is safe. Otherwise it returns false,
// and "client" is called asynchronously with the result when it is ready. // and |client| is called asynchronously with the result when it is ready.
virtual bool CheckApiBlacklistUrl(const GURL& url, Client* client); virtual bool CheckApiBlacklistUrl(const GURL& url, Client* client);
// Check if the |url| matches any of the full-length hashes from the client- // Check if the |url| matches any of the full-length hashes from the client-
// side phishing detection whitelist. The 3-state return value indicates // side phishing detection whitelist. The 3-state return value indicates
// the result or that the Client will get a callback later with the result. // the result or that |client| will get a callback later with the result.
virtual AsyncMatch CheckCsdWhitelistUrl(const GURL& url, Client* client) = 0; virtual AsyncMatch CheckCsdWhitelistUrl(const GURL& url, Client* client) = 0;
// Called on the IO thread to check if the given url is safe or not. If we // Called on the IO thread to check if the given url is safe or not. If we
// can synchronously determine that the url is safe, CheckUrl returns true. // can synchronously determine that the url is safe, CheckUrl returns true.
// Otherwise it returns false, and "client" is called asynchronously with the // Otherwise it returns false, and |client| is called asynchronously with the
// result when it is ready. The URL will only be checked for the threat types // result when it is ready. The URL will only be checked for the threat types
// in |threat_types|. // in |threat_types|.
virtual bool CheckBrowseUrl(const GURL& url, virtual bool CheckBrowseUrl(const GURL& url,
...@@ -160,11 +175,20 @@ class SafeBrowsingDatabaseManager ...@@ -160,11 +175,20 @@ class SafeBrowsingDatabaseManager
// Called on the IO thread to check if the given url belongs to a list the // Called on the IO thread to check if the given url belongs to a list the
// subresource cares about. If the url doesn't belong to any such list and the // subresource cares about. If the url doesn't belong to any such list and the
// check can happen synchronously, returns true. Otherwise it returns false, // check can happen synchronously, returns true. Otherwise it returns false,
// and "client" is called asynchronously with the result when it is ready. // and |client| is called asynchronously with the result when it is ready.
// Returns true if the list is not yet available. // Returns true if the list is not yet available.
virtual bool CheckUrlForSubresourceFilter(const GURL& url, virtual bool CheckUrlForSubresourceFilter(const GURL& url,
Client* client) = 0; Client* client) = 0;
// Called on the IO thread to check whether |url| is safe by checking if it
// appears on a high-confidence allowlist. The 3-state return value indicates
// the result or that |client| will get a callback later with the result.
// The high confidence allowlist is a list of partial or full hashes of URLs
// that are expected to be safe so in the case of a match on this list, the
// realtime full URL Safe Browsing lookup isn't performed.
virtual AsyncMatch CheckUrlForHighConfidenceAllowlist(const GURL& url,
Client* client) = 0;
// //
// Match*(): Methods to synchronously check if various types are safe. // Match*(): Methods to synchronously check if various types are safe.
// //
......
...@@ -61,6 +61,13 @@ bool TestSafeBrowsingDatabaseManager::CheckResourceUrl(const GURL& url, ...@@ -61,6 +61,13 @@ bool TestSafeBrowsingDatabaseManager::CheckResourceUrl(const GURL& url,
return true; return true;
} }
AsyncMatch TestSafeBrowsingDatabaseManager::CheckUrlForHighConfidenceAllowlist(
const GURL& url,
Client* client) {
NOTIMPLEMENTED();
return AsyncMatch::NO_MATCH;
}
bool TestSafeBrowsingDatabaseManager::CheckUrlForSubresourceFilter( bool TestSafeBrowsingDatabaseManager::CheckUrlForSubresourceFilter(
const GURL& url, const GURL& url,
Client* client) { Client* client) {
......
...@@ -34,6 +34,8 @@ class TestSafeBrowsingDatabaseManager : public SafeBrowsingDatabaseManager { ...@@ -34,6 +34,8 @@ class TestSafeBrowsingDatabaseManager : public SafeBrowsingDatabaseManager {
bool CheckExtensionIDs(const std::set<std::string>& extension_ids, bool CheckExtensionIDs(const std::set<std::string>& extension_ids,
Client* client) override; Client* client) override;
bool CheckResourceUrl(const GURL& url, Client* client) override; bool CheckResourceUrl(const GURL& url, Client* client) override;
AsyncMatch CheckUrlForHighConfidenceAllowlist(const GURL& url,
Client* client) override;
bool CheckUrlForSubresourceFilter(const GURL& url, Client* client) override; bool CheckUrlForSubresourceFilter(const GURL& url, Client* client) override;
bool MatchDownloadWhitelistString(const std::string& str) override; bool MatchDownloadWhitelistString(const std::string& str) override;
bool MatchDownloadWhitelistUrl(const GURL& url) override; bool MatchDownloadWhitelistUrl(const GURL& url) override;
......
...@@ -2,9 +2,6 @@ ...@@ -2,9 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
// This file should not be build on Android but is currently getting built.
// TODO(vakh): Fix that: http://crbug.com/621647
#include "components/safe_browsing/db/v4_local_database_manager.h" #include "components/safe_browsing/db/v4_local_database_manager.h"
#include <utility> #include <utility>
...@@ -128,6 +125,7 @@ ThreatSeverity GetThreatSeverity(const ListIdentifier& list_id) { ...@@ -128,6 +125,7 @@ ThreatSeverity GetThreatSeverity(const ListIdentifier& list_id) {
case SUBRESOURCE_FILTER: case SUBRESOURCE_FILTER:
return 2; return 2;
case CSD_WHITELIST: case CSD_WHITELIST:
case HIGH_CONFIDENCE_ALLOWLIST:
return 3; return 3;
case SUSPICIOUS: case SUSPICIOUS:
return 4; return 4;
...@@ -395,6 +393,28 @@ bool V4LocalDatabaseManager::CheckResourceUrl(const GURL& url, Client* client) { ...@@ -395,6 +393,28 @@ bool V4LocalDatabaseManager::CheckResourceUrl(const GURL& url, Client* client) {
return HandleCheck(std::move(check)); return HandleCheck(std::move(check));
} }
AsyncMatch V4LocalDatabaseManager::CheckUrlForHighConfidenceAllowlist(
const GURL& url,
Client* client) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
DCHECK(RealTimePolicyEngine::CanPerformFullURLLookup());
StoresToCheck stores_to_check({GetUrlHighConfidenceAllowlistId()});
if (!enabled_ || !CanCheckUrl(url) ||
!AreAllStoresAvailableNow(stores_to_check)) {
// NOTE(vakh): If Safe Browsing isn't enabled yet, or if the URL isn't a
// navigation URL, or if the allowlist isn't ready yet, return NO_MATCH.
// This will lead to a full URL lookup, if other conditions are met.
return AsyncMatch::NO_MATCH;
}
std::unique_ptr<PendingCheck> check = std::make_unique<PendingCheck>(
client, ClientCallbackType::CHECK_HIGH_CONFIDENCE_ALLOWLIST,
stores_to_check, std::vector<GURL>(1, url));
return HandleWhitelistCheck(std::move(check));
}
bool V4LocalDatabaseManager::CheckUrlForSubresourceFilter(const GURL& url, bool V4LocalDatabaseManager::CheckUrlForSubresourceFilter(const GURL& url,
Client* client) { Client* client) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
...@@ -914,6 +934,16 @@ void V4LocalDatabaseManager::RespondToClient( ...@@ -914,6 +934,16 @@ void V4LocalDatabaseManager::RespondToClient(
check->most_severe_threat_type); check->most_severe_threat_type);
break; break;
case ClientCallbackType::CHECK_HIGH_CONFIDENCE_ALLOWLIST: {
DCHECK_EQ(1u, check->urls.size());
bool did_match_allowlist = check->most_severe_threat_type ==
SB_THREAT_TYPE_HIGH_CONFIDENCE_ALLOWLIST;
DCHECK(did_match_allowlist ||
check->most_severe_threat_type == SB_THREAT_TYPE_SAFE);
check->client->OnCheckUrlForHighConfidenceAllowlist(did_match_allowlist);
break;
}
case ClientCallbackType::CHECK_RESOURCE_URL: case ClientCallbackType::CHECK_RESOURCE_URL:
DCHECK_EQ(1u, check->urls.size()); DCHECK_EQ(1u, check->urls.size());
check->client->OnCheckResourceUrlResult(check->urls[0], check->client->OnCheckResourceUrlResult(check->urls[0],
...@@ -923,11 +953,11 @@ void V4LocalDatabaseManager::RespondToClient( ...@@ -923,11 +953,11 @@ void V4LocalDatabaseManager::RespondToClient(
case ClientCallbackType::CHECK_CSD_WHITELIST: { case ClientCallbackType::CHECK_CSD_WHITELIST: {
DCHECK_EQ(1u, check->urls.size()); DCHECK_EQ(1u, check->urls.size());
bool did_match_whitelist = bool did_match_allowlist =
check->most_severe_threat_type == SB_THREAT_TYPE_CSD_WHITELIST; check->most_severe_threat_type == SB_THREAT_TYPE_CSD_WHITELIST;
DCHECK(did_match_whitelist || DCHECK(did_match_allowlist ||
check->most_severe_threat_type == SB_THREAT_TYPE_SAFE); check->most_severe_threat_type == SB_THREAT_TYPE_SAFE);
check->client->OnCheckWhitelistUrlResult(did_match_whitelist); check->client->OnCheckWhitelistUrlResult(did_match_allowlist);
break; break;
} }
......
...@@ -67,6 +67,8 @@ class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager { ...@@ -67,6 +67,8 @@ class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
bool CheckExtensionIDs(const std::set<FullHash>& extension_ids, bool CheckExtensionIDs(const std::set<FullHash>& extension_ids,
Client* client) override; Client* client) override;
bool CheckResourceUrl(const GURL& url, Client* client) override; bool CheckResourceUrl(const GURL& url, Client* client) override;
AsyncMatch CheckUrlForHighConfidenceAllowlist(const GURL& url,
Client* client) override;
bool CheckUrlForSubresourceFilter(const GURL& url, Client* client) override; bool CheckUrlForSubresourceFilter(const GURL& url, Client* client) override;
bool MatchDownloadWhitelistString(const std::string& str) override; bool MatchDownloadWhitelistString(const std::string& str) override;
bool MatchDownloadWhitelistUrl(const GURL& url) override; bool MatchDownloadWhitelistUrl(const GURL& url) override;
...@@ -127,6 +129,9 @@ class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager { ...@@ -127,6 +129,9 @@ class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
// part of the CSD whitelist. // part of the CSD whitelist.
CHECK_CSD_WHITELIST, CHECK_CSD_WHITELIST,
// TODO(vakh): Explain this.
CHECK_HIGH_CONFIDENCE_ALLOWLIST,
// This represents the other cases when a check is being performed // This represents the other cases when a check is being performed
// synchronously so a client callback isn't required. For instance, when // synchronously so a client callback isn't required. For instance, when
// trying to determing if an IP address is unsafe due to hosting Malware. // trying to determing if an IP address is unsafe due to hosting Malware.
......
...@@ -51,6 +51,9 @@ const base::Feature kPasswordProtectionForSignedInUsers{ ...@@ -51,6 +51,9 @@ const base::Feature kPasswordProtectionForSignedInUsers{
"SafeBrowsingPasswordProtectionForSignedInUsers", "SafeBrowsingPasswordProtectionForSignedInUsers",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kRealTimeUrlLookupEnabled{
"SafeBrowsingRealTimeUrlLookupEnabled", base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kRealTimeUrlLookupFetchAllowlist{ const base::Feature kRealTimeUrlLookupFetchAllowlist{
"SafeBrowsingRealTimeUrlLookupFetchAllowlist", "SafeBrowsingRealTimeUrlLookupFetchAllowlist",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
...@@ -91,6 +94,7 @@ constexpr struct { ...@@ -91,6 +94,7 @@ constexpr struct {
{&kCommittedSBInterstitials, true}, {&kCommittedSBInterstitials, true},
{&kForceUseAPDownloadProtection, false}, {&kForceUseAPDownloadProtection, false},
{&kPasswordProtectionForSignedInUsers, true}, {&kPasswordProtectionForSignedInUsers, true},
{&kRealTimeUrlLookupEnabled, true},
{&kRealTimeUrlLookupFetchAllowlist, true}, {&kRealTimeUrlLookupFetchAllowlist, true},
{&kSuspiciousSiteTriggerQuotaFeature, true}, {&kSuspiciousSiteTriggerQuotaFeature, true},
{&kThreatDomDetailsTagAndAttributeFeature, false}, {&kThreatDomDetailsTagAndAttributeFeature, false},
......
...@@ -49,6 +49,10 @@ extern const base::Feature kPasswordProtectionForSignedInUsers; ...@@ -49,6 +49,10 @@ extern const base::Feature kPasswordProtectionForSignedInUsers;
// Controls the daily quota for the suspicious site trigger. // Controls the daily quota for the suspicious site trigger.
extern const base::Feature kSuspiciousSiteTriggerQuotaFeature; extern const base::Feature kSuspiciousSiteTriggerQuotaFeature;
// Controls whether the real time URL lookup is enabled. Only works if
// |kRealTimeUrlLookupFetchAllowlist| is also enabled.
extern const base::Feature kRealTimeUrlLookupEnabled;
// Controls whether the high confidence allowlist for real time URL lookup be // Controls whether the high confidence allowlist for real time URL lookup be
// fetched. // fetched.
extern const base::Feature kRealTimeUrlLookupFetchAllowlist; extern const base::Feature kRealTimeUrlLookupFetchAllowlist;
......
...@@ -14,4 +14,12 @@ bool RealTimePolicyEngine::CanFetchAllowlist() { ...@@ -14,4 +14,12 @@ bool RealTimePolicyEngine::CanFetchAllowlist() {
return base::FeatureList::IsEnabled(kRealTimeUrlLookupFetchAllowlist); return base::FeatureList::IsEnabled(kRealTimeUrlLookupFetchAllowlist);
} }
// static
bool RealTimePolicyEngine::CanPerformFullURLLookup() {
// TODO(vakh): This should also take into account whether the user is eligible
// for this service (see "Target Users" in the design doc).
return CanFetchAllowlist() &&
base::FeatureList::IsEnabled(kRealTimeUrlLookupEnabled);
}
} // namespace safe_browsing } // namespace safe_browsing
...@@ -17,6 +17,10 @@ class RealTimePolicyEngine { ...@@ -17,6 +17,10 @@ class RealTimePolicyEngine {
public: public:
// Can the high confidence allowlist be sync'd? // Can the high confidence allowlist be sync'd?
static bool CanFetchAllowlist(); static bool CanFetchAllowlist();
// Return true if the feature to enable full URL lookups is enabled and the
// allowlist fetch is enabled.
static bool CanPerformFullURLLookup();
}; // class RealTimePolicyEngine }; // class RealTimePolicyEngine
} // namespace safe_browsing } // namespace safe_browsing
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment