Add sampling of unknown filetypes in download protection.

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 96 matching lines @@
   DOWNLOAD_URL_CHECKS_MALWARE,
 
   DOWNLOAD_HASH_CHECKS_TOTAL,
   DOWNLOAD_HASH_CHECKS_MALWARE,
 
   // Memory space for histograms is determined by the max.
   // ALWAYS ADD NEW VALUES BEFORE THIS ONE.
   DOWNLOAD_CHECKS_MAX
 };
 
-// Prepares URLs to be put into a ping message. Currently this just shortens
-// data: URIs, other URLs are included verbatim.
-std::string SanitizeUrl(const GURL& url) {
-  std::string spec = url.spec();
-  if (url.SchemeIs(url::kDataScheme)) {
-    size_t comma_pos = spec.find(',');
-    if (comma_pos != std::string::npos && comma_pos != spec.size() - 1) {
-      std::string hash_value = crypto::SHA256HashString(spec);
-      spec.erase(comma_pos + 1);
-      spec += base::HexEncode(hash_value.data(), hash_value.size());
-    }
-  }
-  return spec;
-}
-
 }  // namespace
 
 // Parent SafeBrowsing::Client class used to lookup the bad binary
 // URL and digest list.  There are two sub-classes (one for each list).
 class DownloadSBClient
     : public SafeBrowsingDatabaseManager::Client,
       public base::RefCountedThreadSafe<DownloadSBClient> {
  public:
   DownloadSBClient(
       const content::DownloadItem& item,
@@ skipping 160 matching lines @@
         type_(ClientDownloadRequest::WIN_EXECUTABLE),
         start_time_(base::TimeTicks::Now()),
         skipped_url_whitelist_(false),
         skipped_certificate_whitelist_(false),
         is_extended_reporting_(false),
         is_incognito_(false),
         weakptr_factory_(this) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
     item_->AddObserver(this);
   }
+  bool ShouldSampleUnsupportedFile(const base::FilePath filename) {

[asanka, 2016/06/20 18:15:46]

Nit: Use 'const base::FilePath&'. We aren't benefiting from copy elision here
since the return value from DownloadItem::GetTargetFilePath() is a const
reference.

[Nathan Parker, 2016/06/20 22:47:31]

Done.

+    // 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 service_ && is_extended_reporting_ && !is_incognito_ &&
+           base::RandDouble() < policies->SampledPingProbability() &&
+           policies->PingSettingForFile(filename) ==
+               DownloadFileType::SAMPLED_PING;
+  }
 
   void Start() {
     DVLOG(2) << "Starting SafeBrowsing download check for: "
              << item_->DebugString(true);
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
     if (item_->GetBrowserContext()) {
       Profile* profile =
           Profile::FromBrowserContext(item_->GetBrowserContext());
       is_extended_reporting_ = profile &&
              profile->GetPrefs()->GetBoolean(
                  prefs::kSafeBrowsingExtendedReportingEnabled);
       is_incognito_ = item_->GetBrowserContext()->IsOffTheRecord();
     }
-    // TODO(noelutz): implement some cache to make sure we don't issue the same
-    // request over and over again if a user downloads the same binary multiple
-    // times.
+
     DownloadCheckResultReason reason = REASON_MAX;
     if (!IsSupportedDownload(
         *item_, item_->GetTargetFilePath(), &reason, &type_)) {
       switch (reason) {
         case REASON_EMPTY_URL_CHAIN:
         case REASON_INVALID_URL:
         case REASON_UNSUPPORTED_URL_SCHEME:
           PostFinishTask(UNKNOWN, reason);

[asanka, 2016/06/20 18:15:46]

Can we do some sort of incomplete ping for these cases? Or is that not
necessary.

[Nathan Parker, 2016/06/20 22:47:31]

I don't really know what these are, other than file:// URLs.  They account for
0.04% of downloads.  Jialiu is splitting out file:///foo and file://host/foo, so
we'll be able to see how many of those, but I don't know that we need pings for
them.  We can't do much with file: URLs.

           return;
 
         case REASON_NOT_BINARY_FILE:
-          RecordFileExtensionType(item_->GetTargetFilePath());
-          PostFinishTask(UNKNOWN, reason);
-          return;
+          if (ShouldSampleUnsupportedFile(item_->GetTargetFilePath())) {
+            // Send a "light ping" and don't use the verdict.
+            type_ = ClientDownloadRequest::SAMPLED_UNSUPPORTED_FILE;
+            break;
+          } else {

[asanka, 2016/06/20 18:15:46]

Nit: else { unnecessary.

[Nathan Parker, 2016/06/20 22:47:31]

Done.

+            RecordFileExtensionType(item_->GetTargetFilePath());
+            PostFinishTask(UNKNOWN, reason);
+            return;
+          }
 
         default:
           // We only expect the reasons explicitly handled above.
           NOTREACHED();
       }
     }
     RecordFileExtensionType(item_->GetTargetFilePath());
 
     // Compute features from the file contents. Note that we record histograms
     // based on the result, so this runs regardless of whether the pingbacks
@@ skipping 105 matching lines @@
     std::string token;
     if (source->GetStatus().is_success() &&
         net::HTTP_OK == source->GetResponseCode()) {
       ClientDownloadResponse response;
       std::string data;
       bool got_data = source->GetResponseAsString(&data);
       DCHECK(got_data);
       if (!response.ParseFromString(data)) {
         reason = REASON_INVALID_RESPONSE_PROTO;
         result = UNKNOWN;
+      } else if (type_ == ClientDownloadRequest::SAMPLED_UNSUPPORTED_FILE) {
+        // Ignore the verdict because we were just reporting a sampled file.
+        reason = REASON_SAMPLED_UNSUPPORTED_FILE;
+        result = UNKNOWN;
       } else if (response.verdict() == ClientDownloadResponse::SAFE) {
         reason = REASON_DOWNLOAD_SAFE;
         result = SAFE;
       } else if (service_ && !service_->IsSupportedDownload(
           *item_, item_->GetTargetFilePath())) {
-        // The client of the download protection service assumes that we don't
-        // support this download so we cannot return any other verdict than
-        // UNKNOWN even if the server says it's dangerous to download this file.
-        // Note: if service_ is NULL we already cancelled the request and
-        // returned UNKNOWN.
+        // TODO(nparker): Remove this check since it should be impossible.
         reason = REASON_DOWNLOAD_NOT_SUPPORTED;
         result = UNKNOWN;
       } else if (response.verdict() == ClientDownloadResponse::DANGEROUS) {
         reason = REASON_DOWNLOAD_DANGEROUS;
         result = DANGEROUS;
         token = response.token();
       } else if (response.verdict() == ClientDownloadResponse::UNCOMMON) {
         reason = REASON_DOWNLOAD_UNCOMMON;
         result = UNCOMMON;
         token = response.token();
@@ skipping 34 matching lines @@
                                   ClientDownloadRequest::DownloadType* type) {
     if (item.GetUrlChain().empty()) {
       *reason = REASON_EMPTY_URL_CHAIN;
       return false;
     }
     const GURL& final_url = item.GetUrlChain().back();
     if (!final_url.is_valid() || final_url.is_empty()) {
       *reason = REASON_INVALID_URL;
       return false;
     }
-    if (!FileTypePolicies::GetInstance()->IsCheckedBinaryFile(target_path)) {
-      *reason = REASON_NOT_BINARY_FILE;
-      return false;
-    }
     if ((!final_url.IsStandard() && !final_url.SchemeIsBlob() &&
          !final_url.SchemeIs(url::kDataScheme)) ||
         final_url.SchemeIsFile()) {
       *reason = REASON_UNSUPPORTED_URL_SCHEME;
       return false;
     }
+    // This check should be last, so we know the earlier checks passed.
+    if (!FileTypePolicies::GetInstance()->IsCheckedBinaryFile(target_path)) {
+      *reason = REASON_NOT_BINARY_FILE;
+      return false;
+    }
     *type = download_protection_util::GetDownloadType(target_path);
     return true;
   }
 
  private:
   friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
   friend class base::DeleteHelper<CheckClientDownloadRequest>;
 
   ~CheckClientDownloadRequest() override {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ skipping 321 matching lines @@
     if (service_->IsHashManuallyBlacklisted(request.digests().sha256()))
       return true;
 
     for (auto bin_itr : request.archived_binary()) {
       if (service_->IsHashManuallyBlacklisted(bin_itr.digests().sha256()))
         return true;
     }
     return false;
   }
 
+  // Prepares URLs to be put into a ping message. Currently this just shortens
+  // data: URIs, other URLs are included verbatim. If this is a sampled binary,
+  // we'll send a lite-ping which strips all PII.
+  std::string SanitizeUrl(const GURL& url) const {
+    if (type_ == ClientDownloadRequest::SAMPLED_UNSUPPORTED_FILE)
+      return url.GetOrigin().spec();
+
+    std::string spec = url.spec();
+    if (url.SchemeIs(url::kDataScheme)) {
+      size_t comma_pos = spec.find(',');
+      if (comma_pos != std::string::npos && comma_pos != spec.size() - 1) {
+        std::string hash_value = crypto::SHA256HashString(spec);
+        spec.erase(comma_pos + 1);
+        spec += base::HexEncode(hash_value.data(), hash_value.size());
+      }
+    }
+    return spec;
+  }
+
+

[asanka, 2016/06/20 18:15:46]

Nit: Extra blank line.

[Nathan Parker, 2016/06/20 22:47:31]

Done.

   void SendRequest() {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
     // This is our last chance to check whether the request has been canceled
     // before sending it.
     if (!service_)
       return;
 
     ClientDownloadRequest request;
     if (is_extended_reporting_) {
       request.mutable_population()->set_user_population(
           ChromeUserPopulation::EXTENDED_REPORTING);
     } else {
       request.mutable_population()->set_user_population(
           ChromeUserPopulation::SAFE_BROWSING);
     }
+
     request.set_url(SanitizeUrl(item_->GetUrlChain().back()));
     request.mutable_digests()->set_sha256(item_->GetHash());
     request.set_length(item_->GetReceivedBytes());
     request.set_skipped_url_whitelist(skipped_url_whitelist_);
     request.set_skipped_certificate_whitelist(skipped_certificate_whitelist_);
     for (size_t i = 0; i < item_->GetUrlChain().size(); ++i) {
       ClientDownloadRequest::Resource* resource = request.add_resources();
       resource->set_url(SanitizeUrl(item_->GetUrlChain()[i]));
       if (i == item_->GetUrlChain().size() - 1) {
         // The last URL in the chain is the download URL.
@@ skipping 23 matching lines @@
       resource->set_url(SanitizeUrl(tab_url_));
       DVLOG(2) << "tab url " << resource->url();
       resource->set_type(ClientDownloadRequest::TAB_URL);
       if (tab_referrer_url_.is_valid()) {
         resource->set_referrer(SanitizeUrl(tab_referrer_url_));
         DVLOG(2) << "tab referrer " << resource->referrer();
       }
     }
 
     request.set_user_initiated(item_->HasUserGesture());
-    request.set_file_basename(
+    if (type_ == ClientDownloadRequest::SAMPLED_UNSUPPORTED_FILE) {
+      request.set_file_basename(
+          base::FilePath(item_->GetTargetFilePath().Extension())
+              .AsUTF8Unsafe());
+    } else {
+      request.set_file_basename(
         item_->GetTargetFilePath().BaseName().AsUTF8Unsafe());
+    }
     request.set_download_type(type_);
     if (archive_is_valid_ != ArchiveValid::UNSET)
       request.set_archive_valid(archive_is_valid_ == ArchiveValid::VALID);
     request.mutable_signature()->CopyFrom(signature_info_);
     if (image_headers_)
       request.set_allocated_image_headers(image_headers_.release());
     if (archived_executable_)
       request.mutable_archived_binary()->Swap(&archived_binary_);
     if (!request.SerializeToString(&client_download_request_data_)) {
       FinishRequest(UNKNOWN, REASON_INVALID_REQUEST_PROTO);
@@ skipping 60 matching lines @@
                                 REASON_MAX);
       if (reason != REASON_REQUEST_CANCELED) {
         UMA_HISTOGRAM_TIMES("SBClientDownload.DownloadRequestTimeoutDuration",
                             base::TimeTicks::Now() - timeout_start_time_);
       }
     }
     if (service_) {
       DVLOG(2) << "SafeBrowsing download verdict for: "
                << item_->DebugString(true) << " verdict:" << reason
                << " result:" << result;
-      UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckDownloadStats",
-                                reason,
+      UMA_HISTOGRAM_ENUMERATION("SBClientDownload.CheckDownloadStats", reason,
                                 REASON_MAX);
       callback_.Run(result);
       item_->RemoveObserver(this);
       item_ = NULL;
       DownloadProtectionService* service = service_;
       service_ = NULL;
       service->RequestFinished(this);
       // DownloadProtectionService::RequestFinished will decrement our refcount,
       // so we may be deleted now.
     } else {
@@ skipping 326 matching lines @@
       case ClientDownloadResponse::DANGEROUS:
         return DANGEROUS;
       case ClientDownloadResponse::DANGEROUS_HOST:
         return DANGEROUS_HOST;
     }
     return UNKNOWN;
   }
 
   // Given a |default_file_path| and a list of |alternate_extensions|,
   // constructs a FilePath with each possible extension and returns one that
-  // satisfies download_protection_util::IsSupportedBinaryFile(). If none are
-  // supported, returns an empty FilePath.
+  // satisfies IsCheckedBinaryFile(). If none are supported, returns an
+  // empty FilePath.
   static base::FilePath GetSupportedFilePath(
       const base::FilePath& default_file_path,
       const std::vector<base::FilePath::StringType>& alternate_extensions) {
     const FileTypePolicies* file_type_policies =
         FileTypePolicies::GetInstance();
     if (file_type_policies->IsCheckedBinaryFile(default_file_path))
       return default_file_path;
 
     for (const auto& extension : alternate_extensions) {
       base::FilePath alternative_file_path =
@@ skipping 22 matching lines @@
   CheckDownloadCallback callback_;
 
   DownloadProtectionService* service_;
   const scoped_refptr<SafeBrowsingDatabaseManager> database_manager_;
 
   // 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,
-  // safe_browsing::IsSupportedBinaryFile(supported_path_) is always true. This
+  // 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_;
 
   base::WeakPtrFactory<PPAPIDownloadRequest> weakptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PPAPIDownloadRequest);
 };
 
 DownloadProtectionService::DownloadProtectionService(
@@ skipping 78 matching lines @@
         FROM_HERE,
         base::Bind(&DownloadUrlSBClient::StartCheck, client));
 }
 
 bool DownloadProtectionService::IsSupportedDownload(
     const content::DownloadItem& item,
     const base::FilePath& target_path) const {
   DownloadCheckResultReason reason = REASON_MAX;
   ClientDownloadRequest::DownloadType type =
       ClientDownloadRequest::WIN_EXECUTABLE;
+  // TODO(nparker): Remove the CRX check here once can support
+  // 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,
     const CheckDownloadCallback& callback) {
@@ skipping 158 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