Enable verdict cache on safe browsing real time url check.

In this CL, cache_manager is introduced to safe_browsing_url_check via
chrome_content_browser_client. It must be not null when real time url
check is enabled and the profile is not deleted.

Cache is checked and stored on UI thread. url_checker doesn't own
cache_manager, so it uses a weak pointer to access it. Weak pointers
can only be accessed from the same thread it is created.

If cache_manager is able to get a valid matching verdict from the url,
regardless of whether the verdict type is safe or dangerous, it will
skip sending pings and use that verdict type as the check result.

More information can be found in go/chrome-protego-cache.

Bug: 1030989
Change-Id: I1195c4777a10de5356078746dc07119f1b0e04c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1963409
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Reviewed-by: Tim Volodine <timvolodine@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726084}

android_webview/browser/aw_content_browser_client.cc

@@ -779,7 +779,11 @@ AwContentBrowserClient::CreateURLLoaderThrottles(
             return client->GetSafeBrowsingUrlCheckerDelegate();
           },
           base::Unretained(this)),
-      wc_getter, frame_tree_node_id, browser_context->GetResourceContext()));
+      wc_getter, frame_tree_node_id, browser_context->GetResourceContext(),
+      // TODO(crbug.com/1033760): cache manager is used to perform real time url
+      // check, which is gated by UKM opted in. Since AW currently doesn't
+      // support UKM, this feature is not enabled.
+      /* cache_manager */ nullptr));
 
   if (request.resource_type ==
       static_cast<int>(content::ResourceType::kMainFrame)) {

chrome/browser/chrome_content_browser_client.cc

@@ -235,6 +235,8 @@
 #include "components/safe_browsing/db/database_manager.h"
 #include "components/safe_browsing/features.h"
 #include "components/safe_browsing/password_protection/password_protection_navigation_throttle.h"
+#include "components/safe_browsing/realtime/policy_engine.h"
+#include "components/safe_browsing/verdict_cache_manager.h"
 #include "components/security_interstitials/content/origin_policy_ui.h"
 #include "components/security_interstitials/content/ssl_cert_reporter.h"
 #include "components/security_interstitials/content/ssl_error_navigation_throttle.h"
@@ -4242,11 +4244,21 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
   bool matches_enterprise_whitelist = safe_browsing::IsURLWhitelistedByPolicy(
       request.url, *profile->GetPrefs());
   if (!matches_enterprise_whitelist) {
+    // |cache_manager| is used when real time url check is enabled.
+    base::WeakPtr<safe_browsing::VerdictCacheManager> cache_manager =
+        // |safe_browsing_service_| may be unavailable in tests.
+        safe_browsing_service_ &&
+                safe_browsing::RealTimePolicyEngine::CanPerformFullURLLookup(
+                    profile)
+            ? safe_browsing_service_->GetVerdictCacheManagerWeakPtr(profile)
+            : nullptr;
+
     result.push_back(safe_browsing::BrowserURLLoaderThrottle::Create(
         base::BindOnce(
             &ChromeContentBrowserClient::GetSafeBrowsingUrlCheckerDelegate,
             base::Unretained(this)),
-        wc_getter, frame_tree_node_id, profile->GetResourceContext()));
+        wc_getter, frame_tree_node_id, profile->GetResourceContext(),
+        cache_manager));
   }
 #endif
 

chrome/browser/safe_browsing/safe_browsing_service.cc

@@ -248,6 +248,13 @@ VerdictCacheManager* SafeBrowsingService::GetVerdictCacheManager(
   return nullptr;
 }
 
+base::WeakPtr<VerdictCacheManager>
+SafeBrowsingService::GetVerdictCacheManagerWeakPtr(Profile* profile) const {
+  if (profile->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled))
+    return services_delegate_->GetVerdictCacheManager(profile)->GetWeakPtr();
+  return nullptr;
+}
+
 BinaryUploadService* SafeBrowsingService::GetBinaryUploadService(
     Profile* profile) const {
   if (profile->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled))

chrome/browser/safe_browsing/safe_browsing_service.h

@@ -180,6 +180,8 @@ class SafeBrowsingService : public SafeBrowsingServiceInterface,
 
   // Get the cache manager by profile.
   VerdictCacheManager* GetVerdictCacheManager(Profile* profile) const;
+  base::WeakPtr<VerdictCacheManager> GetVerdictCacheManagerWeakPtr(
+      Profile* profile) const;
 
   // Get the binary upload service by profile.
   BinaryUploadService* GetBinaryUploadService(Profile* profile) const;

components/safe_browsing/browser/BUILD.gn

@@ -27,6 +27,7 @@ jumbo_source_set("browser") {
     "//components/safe_browsing:features",
     "//components/safe_browsing:realtimeapi_proto",
     "//components/safe_browsing:safe_browsing",
+    "//components/safe_browsing:verdict_cache_manager",
     "//components/safe_browsing/browser:network_context",
     "//components/safe_browsing/browser:referrer_chain_provider",
     "//components/safe_browsing/common:common",

components/safe_browsing/browser/browser_url_loader_throttle.cc

@@ -12,6 +12,7 @@
 #include "components/safe_browsing/common/safebrowsing_constants.h"
 #include "components/safe_browsing/common/utils.h"
 #include "components/safe_browsing/realtime/policy_engine.h"
+#include "components/safe_browsing/verdict_cache_manager.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/web_contents.h"
 #include "net/log/net_log_event_type.h"
@@ -32,13 +33,15 @@ class BrowserURLLoaderThrottle::CheckerOnIO
       int frame_tree_node_id,
       base::RepeatingCallback<content::WebContents*()> web_contents_getter,
       base::WeakPtr<BrowserURLLoaderThrottle> throttle,
-      bool real_time_lookup_enabled)
+      bool real_time_lookup_enabled,
+      base::WeakPtr<VerdictCacheManager> cache_manager)
       : delegate_getter_(std::move(delegate_getter)),
         resource_context_(resource_context),
         frame_tree_node_id_(frame_tree_node_id),
         web_contents_getter_(web_contents_getter),
         throttle_(std::move(throttle)),
-        real_time_lookup_enabled_(real_time_lookup_enabled) {}
+        real_time_lookup_enabled_(real_time_lookup_enabled),
+        cache_manager_(cache_manager) {}
 
   // Starts the initial safe browsing check. This check and future checks may be
   // skipped after checking with the UrlCheckerDelegate.
@@ -67,7 +70,8 @@ class BrowserURLLoaderThrottle::CheckerOnIO
 
     url_checker_ = std::make_unique<SafeBrowsingUrlCheckerImpl>(
         headers, load_flags, resource_type, has_user_gesture,
-        url_checker_delegate, web_contents_getter_, real_time_lookup_enabled_);
+        url_checker_delegate, web_contents_getter_, real_time_lookup_enabled_,
+        cache_manager_);
 
     CheckUrl(url, method);
   }
@@ -135,6 +139,7 @@ class BrowserURLLoaderThrottle::CheckerOnIO
   bool skip_checks_ = false;
   base::WeakPtr<BrowserURLLoaderThrottle> throttle_;
   bool real_time_lookup_enabled_ = false;
+  base::WeakPtr<VerdictCacheManager> cache_manager_;
 };
 
 // static
@@ -142,18 +147,20 @@ std::unique_ptr<BrowserURLLoaderThrottle> BrowserURLLoaderThrottle::Create(
     GetDelegateCallback delegate_getter,
     const base::Callback<content::WebContents*()>& web_contents_getter,
     int frame_tree_node_id,
-    content::ResourceContext* resource_context) {
+    content::ResourceContext* resource_context,
+    base::WeakPtr<VerdictCacheManager> cache_manager) {
   return base::WrapUnique<BrowserURLLoaderThrottle>(
       new BrowserURLLoaderThrottle(std::move(delegate_getter),
                                    web_contents_getter, frame_tree_node_id,
-                                   resource_context));
+                                   resource_context, cache_manager));
 }
 
 BrowserURLLoaderThrottle::BrowserURLLoaderThrottle(
     GetDelegateCallback delegate_getter,
     const base::Callback<content::WebContents*()>& web_contents_getter,
     int frame_tree_node_id,
-    content::ResourceContext* resource_context) {
+    content::ResourceContext* resource_context,
+    base::WeakPtr<VerdictCacheManager> cache_manager) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // Decide whether to do real time URL lookups or not.
@@ -165,8 +172,8 @@ BrowserURLLoaderThrottle::BrowserURLLoaderThrottle(
 
   io_checker_ = std::make_unique<CheckerOnIO>(
       std::move(delegate_getter), resource_context, frame_tree_node_id,
-      web_contents_getter, weak_factory_.GetWeakPtr(),
-      real_time_lookup_enabled);
+      web_contents_getter, weak_factory_.GetWeakPtr(), real_time_lookup_enabled,
+      cache_manager);
 }
 
 BrowserURLLoaderThrottle::~BrowserURLLoaderThrottle() {

components/safe_browsing/browser/browser_url_loader_throttle.h

@@ -29,6 +29,8 @@ namespace safe_browsing {
 
 class UrlCheckerDelegate;
 
+class VerdictCacheManager;
+
 // BrowserURLLoaderThrottle is used in the browser process to query
 // SafeBrowsing to determine whether a URL and also its redirect URLs are safe
 // to load.
@@ -48,7 +50,8 @@ class BrowserURLLoaderThrottle : public blink::URLLoaderThrottle {
       GetDelegateCallback delegate_getter,
       const base::Callback<content::WebContents*()>& web_contents_getter,
       int frame_tree_node_id,
-      content::ResourceContext* resource_context);
+      content::ResourceContext* resource_context,
+      base::WeakPtr<VerdictCacheManager> cache_manager);
 
   ~BrowserURLLoaderThrottle() override;
 
@@ -82,7 +85,8 @@ class BrowserURLLoaderThrottle : public blink::URLLoaderThrottle {
       GetDelegateCallback delegate_getter,
       const base::Callback<content::WebContents*()>& web_contents_getter,
       int frame_tree_node_id,
-      content::ResourceContext* resource_context);
+      content::ResourceContext* resource_context,
+      base::WeakPtr<VerdictCacheManager> cache_manager);
 
   // |slow_check| indicates whether it reports the result of a slow check.
   // (Please see comments of CheckerOnIO::OnCheckUrlResult() for what slow check

components/safe_browsing/browser/mojo_safe_browsing_impl.cc

@@ -149,14 +149,14 @@ void MojoSafeBrowsingImpl::CreateCheckerAndCheck(
 
   // This is not called for frame resources, and real time URL checks currently
   // only support frame resources. If we extend real time URL checks to support
-  // non-main frames, we will need to provide the user preferences regarding
-  // real time lookup here.
+  // non-main frames, we will need to provide the user preferences and cache
+  // manager regarding real time lookup here.
   auto checker_impl = std::make_unique<SafeBrowsingUrlCheckerImpl>(
       headers, static_cast<int>(load_flags), resource_type, has_user_gesture,
       delegate_,
       base::Bind(&GetWebContentsFromID, render_process_id_,
                  static_cast<int>(render_frame_id)),
-      /*real_time_lookup_enabled=*/false);
+      /*real_time_lookup_enabled=*/false, /*cache_manager=*/nullptr);
 
   checker_impl->CheckUrl(
       url, method,

components/safe_browsing/browser/safe_browsing_url_checker_impl.cc

@@ -12,6 +12,7 @@
 #include "components/safe_browsing/browser/url_checker_delegate.h"
 #include "components/safe_browsing/realtime/policy_engine.h"
 #include "components/safe_browsing/realtime/url_lookup_service.h"
+#include "components/safe_browsing/verdict_cache_manager.h"
 #include "components/safe_browsing/web_ui/constants.h"
 #include "components/safe_browsing/web_ui/safe_browsing_ui.h"
 #include "components/security_interstitials/content/unsafe_resource.h"
@@ -101,7 +102,8 @@ SafeBrowsingUrlCheckerImpl::SafeBrowsingUrlCheckerImpl(
     bool has_user_gesture,
     scoped_refptr<UrlCheckerDelegate> url_checker_delegate,
     const base::Callback<content::WebContents*()>& web_contents_getter,
-    bool real_time_lookup_enabled)
+    bool real_time_lookup_enabled,
+    base::WeakPtr<VerdictCacheManager> cache_manager_on_ui)
     : headers_(headers),
       load_flags_(load_flags),
       resource_type_(resource_type),
@@ -109,7 +111,8 @@ SafeBrowsingUrlCheckerImpl::SafeBrowsingUrlCheckerImpl(
       web_contents_getter_(web_contents_getter),
       url_checker_delegate_(std::move(url_checker_delegate)),
       database_manager_(url_checker_delegate_->GetDatabaseManager()),
-      real_time_lookup_enabled_(real_time_lookup_enabled) {}
+      real_time_lookup_enabled_(real_time_lookup_enabled),
+      cache_manager_on_ui_(cache_manager_on_ui) {}
 
 SafeBrowsingUrlCheckerImpl::~SafeBrowsingUrlCheckerImpl() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
@@ -445,6 +448,55 @@ void SafeBrowsingUrlCheckerImpl::OnCheckUrlForHighConfidenceAllowlist(
     return;
   }
 
+  base::PostTask(
+      FROM_HERE, {content::BrowserThread::UI},
+      base::BindOnce(
+          &SafeBrowsingUrlCheckerImpl::StartGetCachedRealTimeUrlVerdictOnUI,
+          weak_factory_.GetWeakPtr(), cache_manager_on_ui_, url));
+}
+
+// static
+void SafeBrowsingUrlCheckerImpl::StartGetCachedRealTimeUrlVerdictOnUI(
+    base::WeakPtr<SafeBrowsingUrlCheckerImpl> weak_checker_on_io,
+    base::WeakPtr<VerdictCacheManager> cache_manager_on_ui,
+    const GURL& url) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  std::unique_ptr<RTLookupResponse::ThreatInfo> cached_threat_info =
+      std::make_unique<RTLookupResponse::ThreatInfo>();
+  // TODO(crbug.com/1030989): Add metrics for the proportion of cache_manager
+  // not valid.
+  RTLookupResponse::ThreatInfo::VerdictType verdict_type =
+      cache_manager_on_ui
+          ? cache_manager_on_ui->GetCachedRealTimeUrlVerdict(
+                url, cached_threat_info.get())
+          : RTLookupResponse::ThreatInfo::VERDICT_TYPE_UNSPECIFIED;
+  base::PostTask(
+      FROM_HERE, {content::BrowserThread::IO},
+      base::BindOnce(
+          &SafeBrowsingUrlCheckerImpl::OnGetCachedRealTimeUrlVerdictDoneOnIO,
+          weak_checker_on_io, verdict_type, std::move(cached_threat_info),
+          url));
+}
+
+void SafeBrowsingUrlCheckerImpl::OnGetCachedRealTimeUrlVerdictDoneOnIO(
+    RTLookupResponse::ThreatInfo::VerdictType verdict_type,
+    std::unique_ptr<RTLookupResponse::ThreatInfo> cached_threat_info,
+    const GURL& url) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+  // TODO(crbug.com/1030989): Add metrics for cache hit rate and latency.
+  if (verdict_type == RTLookupResponse::ThreatInfo::SAFE) {
+    OnUrlResult(url, SB_THREAT_TYPE_SAFE, ThreatMetadata());
+    return;
+  } else if (verdict_type == RTLookupResponse::ThreatInfo::DANGEROUS) {
+    OnUrlResult(url,
+                RealTimeUrlLookupService::GetSBThreatTypeForRTThreatType(
+                    cached_threat_info->threat_type()),
+                ThreatMetadata());
+    return;
+  }
+
   RTLookupRequestCallback request_callback =
       base::Bind(&SafeBrowsingUrlCheckerImpl::OnRTLookupRequest,
                  weak_factory_.GetWeakPtr());
@@ -489,19 +541,24 @@ void SafeBrowsingUrlCheckerImpl::OnRTLookupResponse(
   }
 
   const GURL& url = urls_[next_index_].url;
-  // TODO(crbug.com/1033692): Only take the first threat info into account
-  // because threat infos are returned in decreasing order of severity. Consider
-  // extend it to support multiple threat types.
-  if (response && (response->threat_info_size() > 0) &&
-      (response->threat_info(0).verdict_type() ==
-       RTLookupResponse::ThreatInfo::DANGEROUS)) {
-    OnUrlResult(url,
-                RealTimeUrlLookupService::GetSBThreatTypeForRTThreatType(
-                    response->threat_info(0).threat_type()),
-                ThreatMetadata());
-  } else {
-    OnUrlResult(url, SB_THREAT_TYPE_SAFE, ThreatMetadata());
+
+  SBThreatType sb_threat_type = SB_THREAT_TYPE_SAFE;
+  if (response && (response->threat_info_size() > 0)) {
+    base::PostTask(FROM_HERE, {content::BrowserThread::UI},
+                   base::BindOnce(&VerdictCacheManager::CacheRealTimeUrlVerdict,
+                                  cache_manager_on_ui_, url, *response,
+                                  base::Time::Now()));
+
+    // TODO(crbug.com/1033692): Only take the first threat info into account
+    // because threat infos are returned in decreasing order of severity.
+    // Consider extend it to support multiple threat types.
+    if (response->threat_info(0).verdict_type() ==
+        RTLookupResponse::ThreatInfo::DANGEROUS) {
+      sb_threat_type = RealTimeUrlLookupService::GetSBThreatTypeForRTThreatType(
+          response->threat_info(0).threat_type());
+    }
   }
+  OnUrlResult(url, sb_threat_type, ThreatMetadata());
 }
 
 void SafeBrowsingUrlCheckerImpl::SetWebUIToken(int token) {

components/safe_browsing/browser/safe_browsing_url_checker_impl.h

@@ -27,6 +27,8 @@ namespace safe_browsing {
 
 class UrlCheckerDelegate;
 
+class VerdictCacheManager;
+
 // A SafeBrowsingUrlCheckerImpl instance is used to perform SafeBrowsing check
 // for a URL and its redirect URLs. It implements Mojo interface so that it can
 // be used to handle queries from renderers. But it is also used to handle
@@ -69,7 +71,8 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
       bool has_user_gesture,
       scoped_refptr<UrlCheckerDelegate> url_checker_delegate,
       const base::Callback<content::WebContents*()>& web_contents_getter,
-      bool real_time_lookup_enabled);
+      bool real_time_lookup_enabled,
+      base::WeakPtr<VerdictCacheManager> cache_manager_on_ui);
 
   ~SafeBrowsingUrlCheckerImpl() override;
 
@@ -114,9 +117,22 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
   void OnCheckBrowseUrlResult(const GURL& url,
                               SBThreatType threat_type,
                               const ThreatMetadata& metadata) override;
-
   void OnCheckUrlForHighConfidenceAllowlist(bool did_match_allowlist) override;
 
+  // This function has to be static because it is called in UI thread,
+  // |weak_checker_on_io| can only be accessed from IO thread.
+  // This function is called if the url doesn't match the allowlist.
+  static void StartGetCachedRealTimeUrlVerdictOnUI(
+      base::WeakPtr<SafeBrowsingUrlCheckerImpl> weak_checker_on_io,
+      base::WeakPtr<VerdictCacheManager> cache_manager_on_ui,
+      const GURL& url);
+
+  // This function will start real time url lookup if there is no cache match.
+  void OnGetCachedRealTimeUrlVerdictDoneOnIO(
+      RTLookupResponse::ThreatInfo::VerdictType verdict_type,
+      std::unique_ptr<RTLookupResponse::ThreatInfo> cached_threat_info,
+      const GURL& url);
+
   void OnTimeout();
 
   void OnUrlResult(const GURL& url,
@@ -202,6 +218,11 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
   // Whether real time lookup is enabled for this request.
   bool real_time_lookup_enabled_;
 
+  // Unowned object used for getting and storing real time url check cache.
+  // Must be NOT nullptr when real time url check is enabled and profile is not
+  // delete. Can only be accessed in UI thread.
+  base::WeakPtr<VerdictCacheManager> cache_manager_on_ui_;
+
   base::WeakPtrFactory<SafeBrowsingUrlCheckerImpl> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingUrlCheckerImpl);

weblayer/browser/safe_browsing/safe_browsing_service.cc

@@ -80,7 +80,11 @@ SafeBrowsingService::CreateURLLoaderThrottle(
             return sb_service->GetSafeBrowsingUrlCheckerDelegate();
           },
           base::Unretained(this)),
-      wc_getter, frame_tree_node_id, resource_context);
+      wc_getter, frame_tree_node_id, resource_context,
+      // cache_manager is used to perform real time url check, which is gated by
+      // UKM opted in. Since WebLayer currently doesn't support UKM, this
+      // feature is not enabled.
+      /*cache_manager*/ nullptr);
 }
 
 scoped_refptr<safe_browsing::UrlCheckerDelegate>