Measure how often downloaded executables match the download whitelist.

This CL also tells all Chrome users who have SafeBrowsing turned on to
download this new whitelist.

BUG=
TEST=SafeBrowsingProtocolParsingTest


Review URL: http://codereview.chromium.org/8417040

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@107885 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 76 insertions, 24 deletions

diff --git a/chrome/browser/safe_browsing/download_protection_service.cc b/chrome/browser/safe_browsing/download_protection_service.cc
index 251b87217..f87fb0b 100644
--- a/chrome/browser/safe_browsing/download_protection_service.cc
+++ b/chrome/browser/safe_browsing/download_protection_service.cc
@@ -67,7 +67,8 @@ class DownloadProtectionService::CheckClientDownloadRequest
         callback_(callback),
         service_(service),
         sb_service_(sb_service),
-        pingback_enabled_(service_->enabled()) {
+        pingback_enabled_(service_->enabled()),
+        is_signed_(false) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   }
 
@@ -162,15 +163,14 @@ class DownloadProtectionService::CheckClientDownloadRequest
 
   void ExtractFileFeatures() {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-    bool is_signed;
     if (safe_browsing::signature_util::IsSigned(info_.local_file)) {
       VLOG(2) << "Downloaded a signed binary: " << info_.local_file.value();
-      is_signed = true;
+      is_signed_ = true;
     } else {
       VLOG(2) << "Downloaded an unsigned binary: " << info_.local_file.value();
-      is_signed = false;
+      is_signed_ = false;
     }
-    UMA_HISTOGRAM_BOOLEAN("SBClientDownload.SignedBinaryDownload", is_signed);
+    UMA_HISTOGRAM_BOOLEAN("SBClientDownload.SignedBinaryDownload", is_signed_);
 
     // TODO(noelutz): DownloadInfo should also contain the IP address of every
     // URL in the redirect chain.  We also should check whether the download
@@ -184,7 +184,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
   void CheckWhitelists() {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
     DownloadCheckResultReason reason = REASON_MAX;
-    if (!pingback_enabled_ || !sb_service_.get()) {
+    if (!sb_service_.get()) {
       reason = REASON_SB_DISABLED;
     } else {
       for (size_t i = 0; i < info_.download_url_chain.size(); ++i) {
@@ -194,25 +194,30 @@ class DownloadProtectionService::CheckClientDownloadRequest
           break;
         }
       }
-      if (info_.referrer_url.is_valid() &&
+      if (info_.referrer_url.is_valid() && reason == REASON_MAX &&
           sb_service_->MatchDownloadWhitelistUrl(info_.referrer_url)) {
         reason = REASON_WHITELISTED_REFERRER;
       }
+      if (reason != REASON_MAX || is_signed_) {
+        UMA_HISTOGRAM_COUNTS("SBClientDownload.SignedOrWhitelistedDownload", 1);
+      }
     }
     if (reason != REASON_MAX) {
       RecordStats(reason);
       PostFinishTask(SAFE);
-      return;
+    } else if (!pingback_enabled_) {
+      RecordStats(REASON_SB_DISABLED);
+      PostFinishTask(SAFE);
+    } else {
+      // TODO(noelutz): check signature and CA against whitelist.
+
+      // The URLFetcher is owned by the UI thread, so post a message to
+      // start the pingback.
+      BrowserThread::PostTask(
+          BrowserThread::UI,
+          FROM_HERE,
+          base::Bind(&CheckClientDownloadRequest::SendRequest, this));
     }
-
-    // TODO(noelutz): check signature and CA against whitelist.
-
-    // The URLFetcher is owned by the UI thread, so post a message to
-    // start the pingback.
-    BrowserThread::PostTask(
-        BrowserThread::UI,
-        FROM_HERE,
-        base::Bind(&CheckClientDownloadRequest::SendRequest, this));
   }
 
   void SendRequest() {
@@ -221,6 +226,7 @@ class DownloadProtectionService::CheckClientDownloadRequest
     // This is our last chance to check whether the request has been canceled
     // before sending it.
     if (!service_) {
+      RecordStats(REASON_REQUEST_CANCELED);
       FinishRequest(SAFE);
       return;
     }
@@ -289,7 +295,8 @@ class DownloadProtectionService::CheckClientDownloadRequest
   // Will be NULL if the request has been canceled.
   DownloadProtectionService* service_;
   scoped_refptr<SafeBrowsingService> sb_service_;
-  bool pingback_enabled_;
+  const bool pingback_enabled_;
+  bool is_signed_;
   scoped_ptr<content::URLFetcher> fetcher_;
 
   DISALLOW_COPY_AND_ASSIGN(CheckClientDownloadRequest);
diff --git a/chrome/browser/safe_browsing/download_protection_service.h b/chrome/browser/safe_browsing/download_protection_service.h
index ba186b9..a2d87b5 100644
--- a/chrome/browser/safe_browsing/download_protection_service.h
+++ b/chrome/browser/safe_browsing/download_protection_service.h
@@ -97,6 +97,7 @@ class DownloadProtectionService {
     REASON_SERVER_PING_FAILED,
     REASON_INVALID_RESPONSE_PROTO,
     REASON_NOT_BINARY_FILE,
+    REASON_REQUEST_CANCELED,
     REASON_MAX  // Always add new values before this one.
   };
 
diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc
index ac4ca9d..5de97f3 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -432,7 +432,7 @@ bool SafeBrowsingProtocolManager::HandleServiceResponse(const GURL& url,
         std::string data_str;
         data_str.assign(data, length);
         std::string encoded_chunk;
-        base::Base64Encode(data, &encoded_chunk);
+        base::Base64Encode(data_str, &encoded_chunk);
         VLOG(1) << "ParseChunk error for chunk: " << chunk_url.url
                 << ", client_key: " << client_key_
                 << ", wrapped_key: " << wrapped_key_
diff --git a/chrome/browser/safe_browsing/protocol_parser.cc b/chrome/browser/safe_browsing/protocol_parser.cc
index c6edf11..f09a26f 100644
--- a/chrome/browser/safe_browsing/protocol_parser.cc
+++ b/chrome/browser/safe_browsing/protocol_parser.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 //
@@ -335,8 +335,9 @@ bool SafeBrowsingProtocolParser::ParseAddChunk(const std::string& list_name,
   SBEntry::Type type = hash_len == sizeof(SBPrefix) ?
       SBEntry::ADD_PREFIX : SBEntry::ADD_FULL_HASH;
 
-  if (list_name == safe_browsing_util::kBinHashList) {
-    // kBinHashList only contains prefixes, no HOSTKEY and COUNT.
+  if (list_name == safe_browsing_util::kBinHashList ||
+      list_name == safe_browsing_util::kDownloadWhiteList) {
+    // These lists only contain prefixes, no HOSTKEY and COUNT.
     DCHECK_EQ(0, remaining % hash_len);
     prefix_count = remaining / hash_len;
     SBChunkHost chunk_host;
diff --git a/chrome/browser/safe_browsing/protocol_parser_unittest.cc b/chrome/browser/safe_browsing/protocol_parser_unittest.cc
index bdc6356..fbec063 100644
--- a/chrome/browser/safe_browsing/protocol_parser_unittest.cc
+++ b/chrome/browser/safe_browsing/protocol_parser_unittest.cc
@@ -924,3 +924,43 @@ TEST(SafeBrowsingProtocolParsingTest, TestSubBinHashChunk) {
   EXPECT_EQ(entry->ChunkIdAtPrefix(1), 0x32323232);
   EXPECT_EQ(entry->PrefixAt(1), 0x6e6e6e6e);
 }
+
+TEST(SafeBrowsingProtocolParsingTest, TestAddDownloadWhitelistChunk) {
+  std::string add_chunk("a:1:32:32\nxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
+                        "a:2:32:64\nyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy"
+                        "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz");
+  // Run the parse.
+  SafeBrowsingProtocolParser parser;
+  bool re_key = false;
+  SBChunkList chunks;
+  bool result = parser.ParseChunk(
+      safe_browsing_util::kDownloadWhiteList,
+      add_chunk.data(),
+      static_cast<int>(add_chunk.length()),
+      "", "", &re_key, &chunks);
+  EXPECT_TRUE(result);
+  EXPECT_FALSE(re_key);
+  EXPECT_EQ(chunks.size(), 2U);
+  EXPECT_EQ(chunks[0].chunk_number, 1);
+  EXPECT_EQ(chunks[0].hosts.size(), 1U);
+  EXPECT_EQ(chunks[0].hosts[0].host, 0);
+  SBEntry* entry = chunks[0].hosts[0].entry;
+  EXPECT_TRUE(entry->IsAdd());
+  EXPECT_FALSE(entry->IsPrefix());
+  EXPECT_EQ(entry->prefix_count(), 1);
+  SBFullHash full;
+  memcpy(full.full_hash, "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", 32);
+  EXPECT_TRUE(entry->FullHashAt(0) == full);
+
+  EXPECT_EQ(chunks[1].chunk_number, 2);
+  EXPECT_EQ(chunks[1].hosts.size(), 1U);
+  EXPECT_EQ(chunks[1].hosts[0].host, 0);
+  entry = chunks[1].hosts[0].entry;
+  EXPECT_TRUE(entry->IsAdd());
+  EXPECT_FALSE(entry->IsPrefix());
+  EXPECT_EQ(entry->prefix_count(), 2);
+  memcpy(full.full_hash, "yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy", 32);
+  EXPECT_TRUE(entry->FullHashAt(0) == full);
+  memcpy(full.full_hash, "zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz", 32);
+  EXPECT_TRUE(entry->FullHashAt(1) == full);
+}
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.cc b/chrome/browser/safe_browsing/safe_browsing_service.cc
index 3f99699..c8a4ea4 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_service.cc
@@ -913,8 +913,11 @@ void SafeBrowsingService::Start() {
       !cmdline->HasSwitch(switches::kDisableClientSidePhishingDetection);
 #endif
 
-  enable_download_whitelist_ = cmdline->HasSwitch(
-      switches::kEnableImprovedDownloadProtection);
+  // TODO(noelutz): remove this boolean variable since it should always be true
+  // if SafeBrowsing is enabled.  Unfortunately, we have no test data for this
+  // list right now.  This means that we need to be able to disable this list
+  // for the SafeBrowsing test to pass.
+  enable_download_whitelist_ = enable_csd_whitelist_;
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,