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 94 matching lines @@
   }
   return safe_browsing_util::INVALID;
 }
 
 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;
   }
 }
 
@@ skipping 39 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_;
 
+  //FIXME: Figure out how to turn this on and off. It looks like the malware
+  //list is always on, should the uslist also be?
+  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;
-  std::vector<SBFullHashResult> cache_hits;
+  std::vector<SBPrefix> browse_prefix_hits;
+  std::vector<SBFullHashResult> browse_cache_hits;
+  bool browse_prefix_match = database_->ContainsBrowseUrl(
+      url, &browse_prefix_hits, &browse_cache_hits);
 
-  bool prefix_match =
-      database_->ContainsBrowseUrl(url, &prefix_hits, &cache_hits);
+  std::vector<SBPrefix> unwanted_prefix_hits;
+  std::vector<SBFullHashResult> unwanted_cache_hits;
+  bool unwanted_prefix_match = database_->ContainsUnwantedSoftwareUrl(
+      url, &unwanted_prefix_hits, &unwanted_cache_hits);
 
   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);
-  check->need_get_hash = cache_hits.empty();
-  check->prefix_hits.swap(prefix_hits);
-  check->cache_hits.swap(cache_hits);
-  checks_.insert(check);
+  if (browse_prefix_match) {
+    // 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);
+    check->need_get_hash = browse_cache_hits.empty();
+    check->prefix_hits.swap(browse_prefix_hits);
+    check->cache_hits.swap(browse_cache_hits);
+    checks_.insert(check);
 
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&SafeBrowsingDatabaseManager::OnCheckDone, this, check));
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&SafeBrowsingDatabaseManager::OnCheckDone, this, check));
+  }
+
+  if (unwanted_prefix_match) {
+    // 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::UNWANTEDURL,
+                              expected_threats);
+    check->need_get_hash = unwanted_cache_hits.empty();
+    check->prefix_hits.swap(unwanted_prefix_hits);
+    check->cache_hits.swap(unwanted_cache_hits);
+    checks_.insert(check);

[mattm, 2014/11/06 01:06:11]

This is going to be a problem if both browse_prefix_match and
unwanted_prefix_match are true. The client isn't expecting to get two callbacks
for the same check.

[mattm, 2014/11/06 01:22:36]

Actually there should be no need for the separate checks, a single gethash check
always gets results for all lists, so you can have a single "if
(browse_prefix_match || unwanted_prefix_match)" since expected_threats contains
all three types.

[gab, 2014/11/06 17:47:46]

I caught on to do the same thing yesterday after sending this, I decided to
split it into two checks made by the client
SafeBrowsingDatabaseManager::CheckBrowseUrl and
SafeBrowsingDatabaseManager::CheckUnwantedSoftwareUrl.
Then the client (SafeBrowsingResourceThrottle) expects two callbacks and
forwards the most severe onwards (or the most severe to-date in the event of a
timeout).

Does that make sense?

I don't quite see how we could check two lists in a single SafeBrowsingCheck
here otherwise?

[gab, 2014/11/07 00:12:09]

Implementation in patch set 6.

[mattm, 2014/11/07 00:27:09]

SafeBrowsingCheck actually checks all lists, then filters the results based on
expected_threats. So you just combine the prefix_hits and cache_hits of the two
ContainsBrowseUrl and ContainsUWSUrl.

The one niggle is the "check_type" of SafeBrowsingCheck, but this is only used
to decide which callback to call, and as  the patch to
SafeBrowsingDatabaseManager::Client::OnSafeBrowsingResult above shows, you call
the same method in either case. Therefore using a single SafeBrowsingCheck with
check_type=MALWARE should be fine.

[gab, 2014/11/07 18:54:17]

I see, as discussed yesterday, makes sense. See latest update.

+
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&SafeBrowsingDatabaseManager::OnCheckDone, this, check));
+  }
 
   return false;
 }
 
 void SafeBrowsingDatabaseManager::CancelCheck(Client* client) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   for (CurrentChecks::iterator i = checks_.begin(); i != checks_.end(); ++i) {
     // We can't delete matching checks here because the db thread has a copy of
     // the pointer.  Instead, we simply NULL out the client, and when the db
     // thread calls us back, we'll clean up the check.
@@ skipping 238 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 355 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_);
 }