[MaliciousApkDownloadCheck] Specify sampled ping behavior

This is part 2 of 6 of relanding code that was originally landed in
crrev.com/c/6309053 but was reverted.

This CL adds methods to the DownloadProtectionDelegate interface to
allow the delegate to specify platform-specific behavior for
1) sampled pings for allowlisted downloads, and 2) sampled pings for
unsupported file types.

For Android download protection, these are both disabled:
1) sampling allowlisted downloads does not yet make sense because the
allowlist check is not yet implemented for Android download protection
(it will be in the future);
2) sampled pings for unsupported file types are not supported.

There is no behavior change for download protection on desktop
platforms.

Bug: 397407934
Change-Id: Id61a148fa6a2a88ada60c05a4193510fa01c532d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6341930
Commit-Queue: Lily Chen <chlily@chromium.org>
Reviewed-by: Daniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1431755}

chrome/browser/safe_browsing/download_protection/check_client_download_request_base.cc

@@ -162,11 +162,9 @@ bool CheckClientDownloadRequestBase::ShouldSampleUnsupportedFile(
   // all "unknown" extensions), we may want to sample it. Sampling it means
   // we'll send a "light ping" with private info removed, and we won't
   // use the verdict.
-  const FileTypePolicies* policies = FileTypePolicies::GetInstance();
   return service_ && is_extended_reporting_ && !is_incognito_ &&
-         base::RandDouble() < policies->SampledPingProbability() &&
-         policies->PingSettingForFile(filename) ==
-             DownloadFileType::SAMPLED_PING;
+         base::RandDouble() <
+             service_->delegate()->GetUnsupportedFileSampleRate(filename);
 }
 
 // If the hash of either the original file or any executables within an

chrome/browser/safe_browsing/download_protection/download_protection_delegate.h

@@ -48,6 +48,15 @@ class DownloadProtectionDelegate {
   // Returns the URL that will be contacted for download protection requests.
   virtual const GURL& GetDownloadRequestUrl() const = 0;
 
+  // Sampling rate for when an allowlisted download may generate a sampled ping,
+  // if other requirements are met.
+  virtual float GetAllowlistedDownloadSampleRate() const = 0;
+
+  // Sampling rate for when an unsupported download may generate a sampled ping,
+  // if other requirements are met.
+  virtual float GetUnsupportedFileSampleRate(
+      const base::FilePath& filename) const = 0;
+
   // Completes the network traffic annotation for CheckClientDownloadRequest.
   virtual net::NetworkTrafficAnnotationTag
   CompleteClientDownloadRequestTrafficAnnotation(

chrome/browser/safe_browsing/download_protection/download_protection_delegate_android.cc

@@ -134,4 +134,17 @@ net::NetworkTrafficAnnotationTag DownloadProtectionDelegateAndroid::
           })");
 }
 
+float DownloadProtectionDelegateAndroid::GetAllowlistedDownloadSampleRate()
+    const {
+  // TODO(chlily): The allowlist is not implemented yet for Android download
+  // protection.
+  return 0.0;
+}
+
+float DownloadProtectionDelegateAndroid::GetUnsupportedFileSampleRate(
+    const base::FilePath& filename) const {
+  // "Light" pings for a sample of unsupported files is disabled on Android.
+  return 0.0;
+}
+
 }  // namespace safe_browsing

chrome/browser/safe_browsing/download_protection/download_protection_delegate_android.h

@@ -36,6 +36,9 @@ class DownloadProtectionDelegateAndroid : public DownloadProtectionDelegate {
   CompleteClientDownloadRequestTrafficAnnotation(
       const net::PartialNetworkTrafficAnnotationTag& partial_traffic_annotation)
       const override;
+  float GetAllowlistedDownloadSampleRate() const override;
+  float GetUnsupportedFileSampleRate(
+      const base::FilePath& filename) const override;
 
  private:
   const GURL download_request_url_;

chrome/browser/safe_browsing/download_protection/download_protection_delegate_desktop.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/safe_browsing/download_protection/download_protection_util.h"
 #include "chrome/common/safe_browsing/download_type_util.h"
 #include "components/download/public/common/download_item.h"
+#include "components/safe_browsing/content/common/file_type_policies.h"
 #include "components/safe_browsing/core/common/safe_browsing_prefs.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/download_item_utils.h"
@@ -21,6 +22,9 @@ namespace {
 const char kDownloadRequestUrl[] =
     "https://sb-ssl.google.com/safebrowsing/clientreport/download";
 
+// We sample 1% of allowlisted downloads to still send out download pings.
+const double kAllowlistDownloadSampleRate = 0.01;
+
 // The API key should not change so we can construct this GURL once and cache
 // it.
 GURL ConstructDownloadRequestUrl() {
@@ -112,4 +116,22 @@ net::NetworkTrafficAnnotationTag DownloadProtectionDelegateDesktop::
           })");
 }
 
+float DownloadProtectionDelegateDesktop::GetAllowlistedDownloadSampleRate()
+    const {
+  return kAllowlistDownloadSampleRate;
+}
+
+float DownloadProtectionDelegateDesktop::GetUnsupportedFileSampleRate(
+    const base::FilePath& filename) const {
+  // If this extension is specifically marked as SAMPLED_PING (as are
+  // all "unknown" extensions), we may want to sample it. Sampling it means
+  // we'll send a "light ping" with private info removed, and we won't
+  // use the verdict.
+  const FileTypePolicies* policies = FileTypePolicies::GetInstance();
+  return policies->PingSettingForFile(filename) ==
+                 DownloadFileType::SAMPLED_PING
+             ? policies->SampledPingProbability()
+             : 0.0;
+}
+
 }  // namespace safe_browsing

chrome/browser/safe_browsing/download_protection/download_protection_delegate_desktop.h

@@ -34,6 +34,9 @@ class DownloadProtectionDelegateDesktop : public DownloadProtectionDelegate {
   CompleteClientDownloadRequestTrafficAnnotation(
       const net::PartialNetworkTrafficAnnotationTag& partial_traffic_annotation)
       const override;
+  float GetAllowlistedDownloadSampleRate() const override;
+  float GetUnsupportedFileSampleRate(
+      const base::FilePath& filename) const override;
 
  private:
   const GURL download_request_url_;

chrome/browser/safe_browsing/download_protection/download_protection_service.cc

@@ -79,8 +79,6 @@ inline constexpr int kDownloadAttributionUserGestureLimitForExtendedReporting =
     5;
 
 const int64_t kDownloadRequestTimeoutMs = 7000;
-// We sample 1% of allowlisted downloads to still send out download pings.
-const double kAllowlistDownloadSampleRate = 0.01;
 
 bool IsDownloadSecuritySensitive(safe_browsing::DownloadCheckResult result) {
   using Result = safe_browsing::DownloadCheckResult;
@@ -145,7 +143,6 @@ DownloadProtectionService::DownloadProtectionService(
               {base::MayBlock(), base::TaskPriority::BEST_EFFORT})
               .get())),
 #endif
-      allowlist_sample_rate_(kAllowlistDownloadSampleRate),
       weak_ptr_factory_(this) {
   CHECK(delegate_);
   if (sb_service) {
@@ -445,6 +442,11 @@ void DownloadProtectionService::ShowDetailsForDownload(
       /*navigation_handle_callback=*/{});
 }
 
+double DownloadProtectionService::allowlist_sample_rate() const {
+  return allowlist_sample_rate_.value_or(
+      delegate()->GetAllowlistedDownloadSampleRate());
+}
+
 // static
 void DownloadProtectionService::SetDownloadProtectionData(
     download::DownloadItem* item,

chrome/browser/safe_browsing/download_protection/download_protection_service.h

@@ -171,6 +171,7 @@ class DownloadProtectionService {
   bool enabled() const { return enabled_; }
 
   DownloadProtectionDelegate* delegate() { return delegate_.get(); }
+  const DownloadProtectionDelegate* delegate() const { return delegate_.get(); }
 
   // Returns the URL that will be contacted for download protection requests.
   const GURL& GetDownloadRequestUrl() const;
@@ -200,7 +201,7 @@ class DownloadProtectionService {
   base::CallbackListSubscription RegisterPPAPIDownloadRequestCallback(
       const PPAPIDownloadRequestCallback& callback);
 
-  double allowlist_sample_rate() const { return allowlist_sample_rate_; }
+  double allowlist_sample_rate() const;
 
   static void SetDownloadProtectionData(
       download::DownloadItem* item,
@@ -451,7 +452,8 @@ class DownloadProtectionService {
   std::set<std::string> manual_blocklist_hashes_;
 
   // Rate of allowlisted downloads we sample to send out download ping.
-  double allowlist_sample_rate_;
+  // Overrides the value provided by the delegate. Intended for testing only.
+  std::optional<double> allowlist_sample_rate_ = std::nullopt;
 
   // DownloadProtectionObserver to send real time reports for dangerous download
   // events and handle special user actions on the download.

chrome/browser/safe_browsing/download_protection/download_protection_service_unittest.cc

@@ -539,6 +539,8 @@ class DownloadProtectionServiceTestBase
     download_service_->allowlist_sample_rate_ = target_rate;
   }
 
+  // Note: This only works for desktop platforms where the sampling rate depends
+  // upon FileTypePolicies.
   void SetBinarySamplingProbability(double target_rate) {
     std::unique_ptr<DownloadFileTypeConfig> config =
         policies_.DuplicateConfig();