Sample 1% url whitelisted PPAPI downloads to ping safe browsing server

chrome/browser/safe_browsing/download_protection_service.cc

 // Copyright (c) 2012 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 "chrome/browser/safe_browsing/download_protection_service.h"
 
 #include <stddef.h>
 
 #include <memory>
 
@@ skipping 54 matching lines @@
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_status.h"
 
 #if defined(OS_MACOSX)
 #include "chrome/browser/safe_browsing/sandboxed_dmg_analyzer_mac.h"
 #endif
 
 using content::BrowserThread;
 
 namespace {

[Lei Zhang, 2016/07/14 23:56:11]

Add some blank lines on the insides of the namespace.

[Jialiu Lin, 2016/07/15 01:20:05]

Done.

 static const int64_t kDownloadRequestTimeoutMs = 7000;
 // We sample 1% of whitelisted downloads to still send out download pings.
 static const double kWhitelistDownloadSampleRate = 0.01;
+
+enum WhitelistType {
+  NO_WHITELIST_MATCH,
+  URL_WHITELIST,
+  SIGNATURE_WHITELIST,
+  WHITELIST_TYPE_MAX
+};
+
+static void RecordCountOfWhitelistedDownload(WhitelistType type) {

[Lei Zhang, 2016/07/14 23:56:11]

static inside an anonymous namespace is redundant. Ditto nitto above.

[Jialiu Lin, 2016/07/15 01:20:05]

Done.

+  UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckWhitelistResult", type,
+                            WHITELIST_TYPE_MAX);
+}
 }  // namespace
 
 namespace safe_browsing {
 
 const char DownloadProtectionService::kDownloadRequestUrl[] =
     "https://sb-ssl.google.com/safebrowsing/clientreport/download";
 
 const void* const DownloadProtectionService::kDownloadPingTokenKey
     = &kDownloadPingTokenKey;
 
@@ skipping 657 matching lines @@
       } else {
         PostFinishTask(UNKNOWN, REASON_ARCHIVE_WITHOUT_BINARIES);
         return;
       }
     }
 
     OnFileFeatureExtractionDone();
   }
 #endif  // defined(OS_MACOSX)
 
-  enum WhitelistType {
-    NO_WHITELIST_MATCH,
-    URL_WHITELIST,
-    SIGNATURE_WHITELIST,
-    WHITELIST_TYPE_MAX
-  };
-
-  static void RecordCountOfWhitelistedDownload(WhitelistType type) {
-    UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckWhitelistResult",
-                              type,
-                              WHITELIST_TYPE_MAX);
-  }
-
-  virtual bool ShouldSampleWhitelistedDownload() {
+  bool ShouldSampleWhitelistedDownload() {
     // We currently sample 1% whitelisted downloads from users who opted
     // in extended reporting and are not in incognito mode.
     return service_ && is_extended_reporting_ && !is_incognito_ &&
         base::RandDouble() < service_->whitelist_sample_rate();
   }
 
   void CheckWhitelists() {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
     if (!database_manager_.get()) {
@@ skipping 406 matching lines @@
     REQUEST_MALFORMED,
     FETCH_FAILED,
     RESPONSE_MALFORMED,
     SUCCEEDED
   };
 
   PPAPIDownloadRequest(
       const GURL& requestor_url,
       const base::FilePath& default_file_path,
       const std::vector<base::FilePath::StringType>& alternate_extensions,
+      Profile* profile,
       const CheckDownloadCallback& callback,
       DownloadProtectionService* service,
       scoped_refptr<SafeBrowsingDatabaseManager> database_manager)
       : requestor_url_(requestor_url),
         default_file_path_(default_file_path),
         alternate_extensions_(alternate_extensions),
         callback_(callback),
         service_(service),
         database_manager_(database_manager),
         start_time_(base::TimeTicks::Now()),
         supported_path_(
             GetSupportedFilePath(default_file_path, alternate_extensions)),
-        weakptr_factory_(this) {}
+        sample_url_whitelist_(false),
+        weakptr_factory_(this) {
+    is_extended_reporting_ = profile &&
+                             profile->GetPrefs()->GetBoolean(
+                                 prefs::kSafeBrowsingExtendedReportingEnabled);
+    is_incognito_ = profile && profile->IsOffTheRecord();
+  }
 
   ~PPAPIDownloadRequest() override {
     if (fetcher_ && !callback_.is_null())
       Finish(RequestOutcome::REQUEST_DESTROYED, UNKNOWN);
   }
 
   // Start the process of checking the download request. The callback passed as
   // the |callback| parameter to the constructor will be invoked with the result
   // of the check at some point in the future.
   //
@@ skipping 29 matching lines @@
             service_->download_request_timeout_ms()));
 
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::Bind(&PPAPIDownloadRequest::CheckWhitelistsOnIOThread,
                    requestor_url_, database_manager_,
                    weakptr_factory_.GetWeakPtr()));
   }
 
  private:
+  bool ShouldSampleWhitelistedDownload() {
+    // We currently sample 1% whitelisted downloads from users who opted
+    // in extended reporting and are not in incognito mode.
+    return service_ && !is_incognito_ && is_extended_reporting_ &&
+           base::RandDouble() < service_->whitelist_sample_rate();
+  }
+
   // Whitelist checking needs to the done on the IO thread.
   static void CheckWhitelistsOnIOThread(
       const GURL& requestor_url,
       scoped_refptr<SafeBrowsingDatabaseManager> database_manager,
       base::WeakPtr<PPAPIDownloadRequest> download_request) {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
     DVLOG(2) << " checking whitelists for requestor URL:" << requestor_url;
 
     bool url_was_whitelisted =
         requestor_url.is_valid() && database_manager &&
         database_manager->MatchDownloadWhitelistUrl(requestor_url);
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&PPAPIDownloadRequest::WhitelistCheckComplete,
                    download_request, url_was_whitelisted));
   }
 
   void WhitelistCheckComplete(bool was_on_whitelist) {
     DVLOG(2) << __FUNCTION__ << " was_on_whitelist:" << was_on_whitelist;
     if (was_on_whitelist) {
-      // TODO(asanka): Should sample whitelisted downloads based on
-      // service_->whitelist_sample_rate(). http://crbug.com/610924
-      Finish(RequestOutcome::WHITELIST_HIT, SAFE);
-      return;
+      RecordCountOfWhitelistedDownload(URL_WHITELIST);
+      if (ShouldSampleWhitelistedDownload()) {

[Lei Zhang, 2016/07/14 23:56:11]

Logic might flow more easily if written as:

if (!Foo()) {
  DoBar();
  return;
}
DoFoo();

[Jialiu Lin, 2016/07/15 01:20:05]

Done.

+        sample_url_whitelist_ = true;
+      } else {
+        Finish(RequestOutcome::WHITELIST_HIT, SAFE);
+        return;
+      }
+    } else {
+      RecordCountOfWhitelistedDownload(NO_WHITELIST_MATCH);
     }
 
     // Not on whitelist, so we are going to check with the SafeBrowsing
     // backend.
     SendRequest();
   }
 
   void SendRequest() {
     DVLOG(2) << __FUNCTION__;
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
     ClientDownloadRequest request;
     request.set_download_type(ClientDownloadRequest::PPAPI_SAVE_REQUEST);
     ClientDownloadRequest::Resource* resource = request.add_resources();
     resource->set_type(ClientDownloadRequest::PPAPI_DOCUMENT);
     resource->set_url(requestor_url_.spec());
     request.set_url(requestor_url_.spec());
     request.set_file_basename(supported_path_.BaseName().AsUTF8Unsafe());
     request.set_length(0);
     request.mutable_digests()->set_md5(std::string());
+    request.set_skipped_url_whitelist(sample_url_whitelist_);
+    // Download protection does not check certificate whitelist for PPAPI
+    // downloads.
+    request.set_skipped_certificate_whitelist(false);
     for (const auto& alternate_extension : alternate_extensions_) {
       if (alternate_extension.empty())
         continue;
       DCHECK_EQ(base::FilePath::kExtensionSeparator, alternate_extension[0]);
       *(request.add_alternate_extensions()) =
           base::FilePath(alternate_extension).AsUTF8Unsafe();
     }
     if (supported_path_ != default_file_path_) {
       *(request.add_alternate_extensions()) =
           base::FilePath(default_file_path_.FinalExtension()).AsUTF8Unsafe();
@@ skipping 130 matching lines @@
   // Time request was started.
   const base::TimeTicks start_time_;
 
   // A download path that is supported by SafeBrowsing. This is determined by
   // invoking GetSupportedFilePath(). If non-empty,
   // IsCheckedBinaryFile(supported_path_) is always true. This
   // path is therefore used as the download target when sending the SafeBrowsing
   // ping.
   const base::FilePath supported_path_;
 
+  bool sample_url_whitelist_;
+
+  bool is_extended_reporting_;
+
+  bool is_incognito_;
+
   base::WeakPtrFactory<PPAPIDownloadRequest> weakptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PPAPIDownloadRequest);
 };
 
 DownloadProtectionService::DownloadProtectionService(
     SafeBrowsingService* sb_service)
     : request_context_getter_(sb_service ? sb_service->url_request_context()
                                          : nullptr),
       enabled_(false),
@@ skipping 85 matching lines @@
   // UNKNOWN types properly.  http://crbug.com/581044
   return (CheckClientDownloadRequest::IsSupportedDownload(
               item, target_path, &reason, &type) &&
           (ClientDownloadRequest::CHROME_EXTENSION != type));
 }
 
 void DownloadProtectionService::CheckPPAPIDownloadRequest(
     const GURL& requestor_url,
     const base::FilePath& default_file_path,
     const std::vector<base::FilePath::StringType>& alternate_extensions,
+    Profile* profile,
     const CheckDownloadCallback& callback) {
   DVLOG(1) << __FUNCTION__ << " url:" << requestor_url
            << " default_file_path:" << default_file_path.value();
   std::unique_ptr<PPAPIDownloadRequest> request(new PPAPIDownloadRequest(
-      requestor_url, default_file_path, alternate_extensions, callback, this,
-      database_manager_));
+      requestor_url, default_file_path, alternate_extensions, profile, callback,
+      this, database_manager_));
   PPAPIDownloadRequest* request_copy = request.get();
   auto insertion_result = ppapi_download_requests_.insert(
       std::make_pair(request_copy, std::move(request)));
   DCHECK(insertion_result.second);
   insertion_result.first->second->Start();
 }
 
 DownloadProtectionService::ClientDownloadRequestSubscription
 DownloadProtectionService::RegisterClientDownloadRequestCallback(
     const ClientDownloadRequestCallback& callback) {
@@ skipping 145 matching lines @@
 GURL DownloadProtectionService::GetDownloadRequestUrl() {
   GURL url(kDownloadRequestUrl);
   std::string api_key = google_apis::GetAPIKey();
   if (!api_key.empty())
     url = url.Resolve("?key=" + net::EscapeQueryParamValue(api_key, true));
 
   return url;
 }
 
 }  // namespace safe_browsing