Add the goog-unwanted-shavar list to a new SafeBrowsing PrefixSet.

chrome/browser/safe_browsing/database_manager.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/database_manager.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 50 matching lines @@
 }
 
 bool IsExpectedThreat(
     const SBThreatType threat_type,
     const std::vector<SBThreatType>& expected_threats) {
   return expected_threats.end() != std::find(expected_threats.begin(),
                                              expected_threats.end(),
                                              threat_type);
 }
 
-// Return the list id from the first result in |full_hashes| which matches
+// Return the severest list id from the results in |full_hashes| which matches
 // |hash|, or INVALID if none match.
-safe_browsing_util::ListType GetHashThreatListType(
+safe_browsing_util::ListType GetHashSeverestThreatListType(
     const SBFullHash& hash,
     const std::vector<SBFullHashResult>& full_hashes,
     size_t* index) {
+
+  safe_browsing_util::ListType severest_threat = safe_browsing_util::INVALID;

[mattm, 2014/11/12 03:24:30]

name is a bit confusing since it's not used for the severest threats actually. 

[gab, 2014/11/12 16:05:08]

Done.

   for (size_t i = 0; i < full_hashes.size(); ++i) {
     if (SBFullHashEqual(hash, full_hashes[i].hash)) {
-      if (index)
-        *index = i;
-      return static_cast<safe_browsing_util::ListType>(full_hashes[i].list_id);
+      const safe_browsing_util::ListType threat =
+          static_cast<safe_browsing_util::ListType>(full_hashes[i].list_id);
+      switch (threat) {
+        case safe_browsing_util::INVALID:
+          // |full_hashes| should never contain INVALID as a |list_id|.
+          NOTREACHED();
+          break;
+        case safe_browsing_util::MALWARE:  // Fall through.
+        case safe_browsing_util::PHISH:  // Fall through.
+        case safe_browsing_util::BINURL:  // Fall through.
+        case safe_browsing_util::CSDWHITELIST:  // Fall through.
+        case safe_browsing_util::DOWNLOADWHITELIST:  // Fall through.
+        case safe_browsing_util::EXTENSIONBLACKLIST:  // Fall through.
+        case safe_browsing_util::SIDEEFFECTFREEWHITELIST:  // Fall through.
+        case safe_browsing_util::IPBLACKLIST:
+          if (index)
+            *index = i;
+          return threat;
+        case safe_browsing_util::UNWANTEDURL:
+          // UNWANTEDURL is considered less severe than other threats, keep
+          // looking.
+          severest_threat = threat;
+          if (index)
+            *index = i;
+          break;
+      }
     }
   }
-  return safe_browsing_util::INVALID;
+  return severest_threat;
 }
 
 // Given a URL, compare all the possible host + path full hashes to the set of
-// provided full hashes.  Returns the list id of the a matching result from
-// |full_hashes|, or INVALID if none match.
-safe_browsing_util::ListType GetUrlThreatListType(
+// provided full hashes.  Returns the list id of the severest matching result
+// from |full_hashes|, or INVALID if none match.
+safe_browsing_util::ListType GetUrlSeverestThreatListType(
     const GURL& url,
     const std::vector<SBFullHashResult>& full_hashes,
     size_t* index) {
   if (full_hashes.empty())
     return safe_browsing_util::INVALID;
 
   std::vector<std::string> patterns;
   safe_browsing_util::GeneratePatternsToCheck(url, &patterns);
 
+  safe_browsing_util::ListType severest_threat = safe_browsing_util::INVALID;
   for (size_t i = 0; i < patterns.size(); ++i) {
-    safe_browsing_util::ListType threat = GetHashThreatListType(
+    safe_browsing_util::ListType threat = GetHashSeverestThreatListType(
         SBFullHashForString(patterns[i]), full_hashes, index);
-    if (threat != safe_browsing_util::INVALID)
-      return threat;
+    switch (threat) {
+      case safe_browsing_util::INVALID:
+        // Ignore patterns with no matching threat.
+        break;
+      case safe_browsing_util::MALWARE:  // Fall through.
+      case safe_browsing_util::PHISH:  // Fall through.
+      case safe_browsing_util::BINURL:  // Fall through.
+      case safe_browsing_util::CSDWHITELIST:  // Fall through.
+      case safe_browsing_util::DOWNLOADWHITELIST:  // Fall through.
+      case safe_browsing_util::EXTENSIONBLACKLIST:  // Fall through.
+      case safe_browsing_util::SIDEEFFECTFREEWHITELIST:  // Fall through.
+      case safe_browsing_util::IPBLACKLIST:
+        return threat;
+      case safe_browsing_util::UNWANTEDURL:
+        // UNWANTEDURL is considered less severe than other threats, keep
+        // looking.
+        severest_threat = threat;
+        break;
+    }
   }
-  return safe_browsing_util::INVALID;
+  return severest_threat;
 }
 
 SBThreatType GetThreatTypeFromListType(safe_browsing_util::ListType list_type) {
   switch (list_type) {
     case safe_browsing_util::PHISH:
       return SB_THREAT_TYPE_URL_PHISHING;
     case safe_browsing_util::MALWARE:
       return SB_THREAT_TYPE_URL_MALWARE;
+    case safe_browsing_util::UNWANTEDURL:
+      return SB_THREAT_TYPE_URL_UNWANTED;
     case safe_browsing_util::BINURL:
       return SB_THREAT_TYPE_BINARY_MALWARE_URL;
     case safe_browsing_util::EXTENSIONBLACKLIST:
       return SB_THREAT_TYPE_EXTENSION;
     default:
       DVLOG(1) << "Unknown safe browsing list id " << list_type;
       return SB_THREAT_TYPE_SAFE;
   }
 }
 
 }  // namespace
 
 // static
-SBThreatType SafeBrowsingDatabaseManager::GetHashThreatType(
+SBThreatType SafeBrowsingDatabaseManager::GetHashSeverestThreatType(
     const SBFullHash& hash,
     const std::vector<SBFullHashResult>& full_hashes) {
   return GetThreatTypeFromListType(
-      GetHashThreatListType(hash, full_hashes, NULL));
+      GetHashSeverestThreatListType(hash, full_hashes, NULL));
 }
 
 // static
-SBThreatType SafeBrowsingDatabaseManager::GetUrlThreatType(
+SBThreatType SafeBrowsingDatabaseManager::GetUrlSeverestThreatType(
     const GURL& url,
     const std::vector<SBFullHashResult>& full_hashes,
     size_t* index) {
   return GetThreatTypeFromListType(
-      GetUrlThreatListType(url, full_hashes, index));
+      GetUrlSeverestThreatListType(url, full_hashes, index));
 }
 
 SafeBrowsingDatabaseManager::SafeBrowsingCheck::SafeBrowsingCheck(
     const std::vector<GURL>& urls,
     const std::vector<SBFullHash>& full_hashes,
     Client* client,
     safe_browsing_util::ListType check_type,
     const std::vector<SBThreatType>& expected_threats)
     : urls(urls),
       url_results(urls.size(), SB_THREAT_TYPE_SAFE),
@@ skipping 12 matching lines @@
 
 void SafeBrowsingDatabaseManager::Client::OnSafeBrowsingResult(
     const SafeBrowsingCheck& check) {
   DCHECK_EQ(check.urls.size(), check.url_results.size());
   DCHECK_EQ(check.full_hashes.size(), check.full_hash_results.size());
   if (!check.urls.empty()) {
     DCHECK(check.full_hashes.empty());
     switch (check.check_type) {
       case safe_browsing_util::MALWARE:
       case safe_browsing_util::PHISH:
+      case safe_browsing_util::UNWANTEDURL:
         DCHECK_EQ(1u, check.urls.size());
         OnCheckBrowseUrlResult(
             check.urls[0], check.url_results[0], check.url_metadata[0]);
         break;
       case safe_browsing_util::BINURL:
         DCHECK_EQ(check.urls.size(), check.url_results.size());
         OnCheckDownloadUrlResult(
             check.urls,
             *std::max_element(check.url_results.begin(),
                               check.url_results.end()));
@@ skipping 26 matching lines @@
     const scoped_refptr<SafeBrowsingService>& service)
     : sb_service_(service),
       database_(NULL),
       enabled_(false),
       enable_download_protection_(false),
       enable_csd_whitelist_(false),
       enable_download_whitelist_(false),
       enable_extension_blacklist_(false),
       enable_side_effect_free_whitelist_(false),
       enable_ip_blacklist_(false),
+      enable_unwanted_software_blacklist_(false),
       update_in_progress_(false),
       database_update_in_progress_(false),
       closing_database_(false),
       check_timeout_(base::TimeDelta::FromMilliseconds(kCheckTimeoutMs)) {
   DCHECK(sb_service_.get() != NULL);
 
   // Android only supports a subset of FULL_SAFE_BROWSING.
   // TODO(shess): This shouldn't be OS-driven <http://crbug.com/394379>
 #if !defined(OS_ANDROID)
   CommandLine* cmdline = CommandLine::ForCurrentProcess();
@@ skipping 16 matching lines @@
       !cmdline->HasSwitch(switches::kSbDisableExtensionBlacklist);
 
   enable_side_effect_free_whitelist_ =
       prerender::IsSideEffectFreeWhitelistEnabled() &&
       !cmdline->HasSwitch(switches::kSbDisableSideEffectFreeWhitelist);
 
   // The client-side IP blacklist feature is tightly integrated with client-side
   // phishing protection for now.
   enable_ip_blacklist_ = enable_csd_whitelist_;
 
+  // TODO(gab): Gate this on the same experiment that will soon control the UwS
+  // URL UI.
+  enable_unwanted_software_blacklist_ = true;
+
   enum SideEffectFreeWhitelistStatus {
     SIDE_EFFECT_FREE_WHITELIST_ENABLED,
     SIDE_EFFECT_FREE_WHITELIST_DISABLED,
     SIDE_EFFECT_FREE_WHITELIST_STATUS_MAX
   };
 
   SideEffectFreeWhitelistStatus side_effect_free_whitelist_status =
       enable_side_effect_free_whitelist_ ? SIDE_EFFECT_FREE_WHITELIST_ENABLED :
       SIDE_EFFECT_FREE_WHITELIST_DISABLED;
 
@@ skipping 136 matching lines @@
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (!enabled_)
     return true;
 
   if (!CanCheckUrl(url))
     return true;
 
   std::vector<SBThreatType> expected_threats;
   expected_threats.push_back(SB_THREAT_TYPE_URL_MALWARE);
   expected_threats.push_back(SB_THREAT_TYPE_URL_PHISHING);
+  expected_threats.push_back(SB_THREAT_TYPE_URL_UNWANTED);
 
   const base::TimeTicks start = base::TimeTicks::Now();
   if (!MakeDatabaseAvailable()) {
     QueuedCheck queued_check(safe_browsing_util::MALWARE,  // or PHISH
                              client,
                              url,
                              expected_threats,
                              start);
     queued_checks_.push_back(queued_check);
     return false;
   }
 
-  std::vector<SBPrefix> prefix_hits;
+  // Cache hits should be the same for both (as verified by the DCHECK below).
+  // TODO(gab): Refactor SafeBrowsingDatabase to avoid depending on this here.
   std::vector<SBFullHashResult> cache_hits;
 
-  bool prefix_match =
-      database_->ContainsBrowseUrl(url, &prefix_hits, &cache_hits);
+  std::vector<SBPrefix> browse_prefix_hits;
+  bool browse_prefix_match = database_->ContainsBrowseUrl(
+      url, &browse_prefix_hits, &cache_hits);
+
+  std::vector<SBPrefix> unwanted_prefix_hits;
+  std::vector<SBFullHashResult> other_cache_hits;

[mattm, 2014/11/12 03:24:30]

unused_cache_hits ?

[gab, 2014/11/12 16:05:08]

Done.

+  bool unwanted_prefix_match = database_->ContainsUnwantedSoftwareUrl(
+      url, &unwanted_prefix_hits, &other_cache_hits);
+
+#if defined(OS_WIN)
+  // TODO(gab): Figure out why this doesn't pass on non-win compilers (padding?)
+  // leave it here as Win-only for now to ensure changes to SBFullHashResult's
+  // format are at least caught by some bots...

[mattm, 2014/11/12 03:24:30]

It's not win specific, but it is padding, yes.  Ex you can see if fails in the
win x64 bot also.

I don't think there's a good way to do this (you could define your own copy of
what you expect the struct to be, and then compare size of that...).  But see my
comment below, anyway.

[gab, 2014/11/12 16:05:08]

Removed.

+  static_assert(sizeof(SBFullHashResult) == sizeof(SBFullHash) + sizeof(int) +
+                    sizeof(std::string),
+                "Need to update equality predicate below.");
+#endif
+  DCHECK(cache_hits.size() == other_cache_hits.size() &&
+         std::equal(cache_hits.begin(), cache_hits.end(),
+                    other_cache_hits.begin(),
+                    [](const SBFullHashResult& a, const SBFullHashResult& b) {
+                      return SBFullHashEqual(a.hash, b.hash) &&
+                          a.list_id == b.list_id && a.metadata == b.metadata;
+                    }));

[mattm, 2014/11/12 03:24:30]

Technically the cache hits could be different, since they will be run at
slightly different times, and thus the later one might ignore some cache entries
as being too old. So it's probably not a good idea to DCHECK it. I think it is
okay to just always use the first one, though.

[gab, 2014/11/12 16:05:08]

Good catch, removed.

+
+  // Merge the two pre-sorted prefix hits lists.
+  // TODO(gab): Refactor SafeBrowsingDatabase for it to return this merged list
+  // by default rather than building it here.
+  DCHECK(std::is_sorted(browse_prefix_hits.begin(), browse_prefix_hits.end()));
+  DCHECK(std::is_sorted(unwanted_prefix_hits.begin(),

[gab, 2014/11/12 16:05:08]

Also removed these two checks as std::is_sorted is a C++11 STL feature which are
not yet supported in Chromium.

Already updated the API definition to enforce this by contract anyways.

+                        unwanted_prefix_hits.end()));
+  std::vector<SBPrefix> prefix_hits(
+      browse_prefix_hits.size() + unwanted_prefix_hits.size());
+  std::merge(browse_prefix_hits.begin(), browse_prefix_hits.end(),
+             unwanted_prefix_hits.begin(), unwanted_prefix_hits.end(),
+             prefix_hits.begin());
+  prefix_hits.erase(std::unique(prefix_hits.begin(), prefix_hits.end()),
+                    prefix_hits.end());
 
   UMA_HISTOGRAM_TIMES("SB2.FilterCheck", base::TimeTicks::Now() - start);
 
-  if (!prefix_match)
+  if (!browse_prefix_match && !unwanted_prefix_match)
     return true;  // URL is okay.
 
   // Needs to be asynchronous, since we could be in the constructor of a
   // ResourceDispatcherHost event handler which can't pause there.
-  SafeBrowsingCheck* check = new SafeBrowsingCheck(std::vector<GURL>(1, url),
-                                                   std::vector<SBFullHash>(),
-                                                   client,
-                                                   safe_browsing_util::MALWARE,
-                                                   expected_threats);
+  // This check will ping the Safe Browsing servers and get all lists which it
+  // matches. These lists will then be filtered against the |expected_threats|
+  // and the result callback for MALWARE (which is the same as for PHISH and
+  // UNWANTEDURL) will eventually be invoked with the final decision.
+  SafeBrowsingCheck* check =
+      new SafeBrowsingCheck(std::vector<GURL>(1, url),
+                            std::vector<SBFullHash>(),
+                            client,
+                            safe_browsing_util::MALWARE,
+                            expected_threats);
   check->need_get_hash = cache_hits.empty();
   check->prefix_hits.swap(prefix_hits);
   check->cache_hits.swap(cache_hits);
   checks_.insert(check);
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&SafeBrowsingDatabaseManager::OnCheckDone, this, check));
 
   return false;
@@ skipping 246 matching lines @@
   startup_metric_utils::ScopedSlowStartupUMA
       scoped_timer("Startup.SlowStartupSafeBrowsingGetDatabase");
   const base::TimeTicks before = base::TimeTicks::Now();
 
   SafeBrowsingDatabase* database =
       SafeBrowsingDatabase::Create(enable_download_protection_,
                                    enable_csd_whitelist_,
                                    enable_download_whitelist_,
                                    enable_extension_blacklist_,
                                    enable_side_effect_free_whitelist_,
-                                   enable_ip_blacklist_);
+                                   enable_ip_blacklist_,
+                                   enable_unwanted_software_blacklist_);
 
   database->Init(SafeBrowsingService::GetBaseFilename());
   {
     // Acquiring the lock here guarantees correct ordering between the writes to
     // the new database object above, and the setting of |databse_| below.
     base::AutoLock lock(database_lock_);
     database_ = database;
   }
 
   BrowserThread::PostTask(
@@ skipping 45 matching lines @@
     bool is_download = check->check_type == safe_browsing_util::BINURL;
     sb_service_->protocol_manager()->GetFullHash(
         check->prefix_hits,
         base::Bind(&SafeBrowsingDatabaseManager::HandleGetHashResults,
                    base::Unretained(this),
                    check),
         is_download);
   } else {
     // We may have cached results for previous GetHash queries.  Since
     // this data comes from cache, don't histogram hits.
-    bool is_threat = HandleOneCheck(check, check->cache_hits);
-    // cache_hits should only contain hits for a fullhash we searched for, so if
-    // we got to this point it should always result in a threat match.
-    DCHECK(is_threat);
+    HandleOneCheck(check, check->cache_hits);
   }
 }
 
 void SafeBrowsingDatabaseManager::GetAllChunksFromDatabase(
     GetChunksCallback callback) {
   DCHECK_EQ(base::MessageLoop::current(),
             safe_browsing_thread_->message_loop());
 
   bool database_error = true;
   std::vector<SBListChunkRanges> lists;
@@ skipping 145 matching lines @@
 }
 
 bool SafeBrowsingDatabaseManager::HandleOneCheck(
     SafeBrowsingCheck* check,
     const std::vector<SBFullHashResult>& full_hashes) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(check);
 
   bool is_threat = false;
 
-  // TODO(shess): GetHashThreadListType() contains a loop,
-  // GetUrlThreatListType() a loop around that loop.  Having another loop out
-  // here concerns me.  It is likely that SAFE is an expected outcome, which
-  // means all of those loops run to completion.  Refactoring this to generate a
-  // set of sorted items to compare in sequence would probably improve things.
+  // TODO(shess): GetHashSeverestThreadListType() contains a loop,
+  // GetUrlSeverestThreatListType() a loop around that loop.  Having another
+  // loop out here concerns me.  It is likely that SAFE is an expected outcome,
+  // which means all of those loops run to completion.  Refactoring this to
+  // generate a set of sorted items to compare in sequence would probably
+  // improve things.
   //
   // Additionally, the set of patterns generated from the urls is very similar
   // to the patterns generated in ContainsBrowseUrl() and other database checks,
   // which are called from this code.  Refactoring that across the checks could
   // interact well with batching the checks here.
 
   for (size_t i = 0; i < check->urls.size(); ++i) {
     size_t threat_index;
     SBThreatType threat =
-        GetUrlThreatType(check->urls[i], full_hashes, &threat_index);
+        GetUrlSeverestThreatType(check->urls[i], full_hashes, &threat_index);
     if (threat != SB_THREAT_TYPE_SAFE &&
         IsExpectedThreat(threat, check->expected_threats)) {

[mattm, 2014/11/12 03:24:30]

Hmm, I suppose ideally the severest threat should only look for one of the
expected threat list, otherwise we might ignore results because it just happened
to return the value for some threat we aren't looking for, even if the one we
were looking for was also in the list.  But I guess that problem already exists,
so up to you if you want to address in this CL.

[gab, 2014/11/12 16:05:08]

Good point. Since this is an existing problem though I added a TODO and will
address in a follow-up CL.

       check->url_results[i] = threat;
       check->url_metadata[i] = full_hashes[threat_index].metadata;
       is_threat = true;
     }
   }
 
   for (size_t i = 0; i < check->full_hashes.size(); ++i) {
-    SBThreatType threat = GetHashThreatType(check->full_hashes[i], full_hashes);
+    SBThreatType threat = GetHashSeverestThreatType(check->full_hashes[i],
+                                                    full_hashes);
     if (threat != SB_THREAT_TYPE_SAFE &&
         IsExpectedThreat(threat, check->expected_threats)) {
       check->full_hash_results[i] = threat;
       is_threat = true;
     }
   }
 
   SafeBrowsingCheckDone(check);
   return is_threat;
 }
@@ skipping 96 matching lines @@
       new base::WeakPtrFactory<SafeBrowsingDatabaseManager>(this));
   checks_.insert(check);
 
   safe_browsing_thread_->message_loop()->PostTask(FROM_HERE, task);
 
   base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
       base::Bind(&SafeBrowsingDatabaseManager::TimeoutCallback,
                  check->timeout_factory_->GetWeakPtr(), check),
       check_timeout_);
 }