Implement support for checking bad IPs aka MatchMalwareIP

components/safe_browsing_db/v4_local_database_manager.cc

Index: components/safe_browsing_db/v4_local_database_manager.cc
diff --git a/components/safe_browsing_db/v4_local_database_manager.cc b/components/safe_browsing_db/v4_local_database_manager.cc
index b9676df6725c8013e5375a804c3ef5678a942790..9271d9291c40bfebb8d2ad9aed6b6245e6f2b565 100644
--- a/components/safe_browsing_db/v4_local_database_manager.cc
+++ b/components/safe_browsing_db/v4_local_database_manager.cc
@@ -14,8 +14,11 @@
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/sha1.h"
 #include "components/safe_browsing_db/v4_feature_list.h"
+#include "components/safe_browsing_db/v4_protocol_manager_util.h"
 #include "content/public/browser/browser_thread.h"
+#include "net/base/ip_address.h"
 
 using content::BrowserThread;
 using base::TimeTicks;
@@ -234,9 +237,22 @@ bool V4LocalDatabaseManager::MatchDownloadWhitelistUrl(const GURL& url) {
 }
 
 bool V4LocalDatabaseManager::MatchMalwareIP(const std::string& ip_address) {
-  // TODO(vakh): Implement this skeleton.
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  return false;
+  if (!enabled_) {
+    return false;
+  }
+  std::string encoded_ip;
+  if (!IPAddressToEncodedIPV6(ip_address, &encoded_ip)) {
+    return false;
+  }
+
+  std::set<FullHash> encoded_ips{encoded_ip};
+  std::unique_ptr<PendingCheck> check = base::MakeUnique<PendingCheck>(
+      nullptr, ClientCallbackType::CHECK_MALWARE_IP,
+      StoresToCheck({GetAnyIpMalwareId()}), encoded_ips);
+
+  // HandleCheck() tells us whether the resource is safe.
+  return !HandleCheck(std::move(check), true /* synchronous_response */);
 }
 
 bool V4LocalDatabaseManager::MatchModuleWhitelistString(
@@ -369,7 +385,8 @@ bool V4LocalDatabaseManager::GetPrefixMatches(
   const base::TimeTicks before = TimeTicks::Now();
   if (check->client_callback_type == ClientCallbackType::CHECK_BROWSE_URL ||
       check->client_callback_type == ClientCallbackType::CHECK_DOWNLOAD_URLS ||
-      check->client_callback_type == ClientCallbackType::CHECK_EXTENSION_IDS) {
+      check->client_callback_type == ClientCallbackType::CHECK_EXTENSION_IDS ||
+      check->client_callback_type == ClientCallbackType::CHECK_MALWARE_IP) {
     DCHECK(!check->full_hashes.empty());
 
     full_hash_to_store_and_hash_prefixes->clear();
@@ -436,8 +453,13 @@ SBThreatType V4LocalDatabaseManager::GetSBThreatTypeForList(
   return it->sb_threat_type();
 }
 
-bool V4LocalDatabaseManager::HandleCheck(std::unique_ptr<PendingCheck> check) {
+bool V4LocalDatabaseManager::HandleCheck(std::unique_ptr<PendingCheck> check,

[Nathan Parker, 2016/11/14 21:09:37]

The flow is quite different if !sychronous_respose... How about making it a
separate function?

HandleCheckSynchronously(..)

The duplicated code would be small, but it'd be easier to understand.  And then
anything that uses a whitelist (or we could call it a "fully-populated store" or
"store of full-hashes) would call that function.

[vakh (use Gerrit instead), 2016/11/15 00:36:21]

Done.

+                                         const bool synchronous_response) {
   if (!v4_database_) {
+    if (synchronous_response) {
+      return true;
+    }
+
     queued_checks_.push_back(std::move(check));
     return false;
   }
@@ -447,15 +469,37 @@ bool V4LocalDatabaseManager::HandleCheck(std::unique_ptr<PendingCheck> check) {
     return true;
   }
 
-  // Post on the IO thread to enforce async behavior.
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&V4LocalDatabaseManager::PerformFullHashCheck, this,
-                 base::Passed(std::move(check)),
-                 full_hash_to_store_and_hash_prefixes));
+  if (!synchronous_response) {
+    // Post on the IO thread to enforce async behavior.
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&V4LocalDatabaseManager::PerformFullHashCheck, this,
+                   base::Passed(std::move(check)),
+                   full_hash_to_store_and_hash_prefixes));
+  }
   return false;
 }
 
+// static
+bool V4LocalDatabaseManager::IPAddressToEncodedIPV6(
+    const std::string& ip_address,
+    std::string* encoded_ip) {
+  net::IPAddress address;
+  if (!V4ProtocolManagerUtil::GetIPV6AddressFromString(ip_address, &address)) {
+    return false;
+  }
+  std::string packed_ip = net::IPAddressToPackedString(address);
+  if (packed_ip.empty()) {
+    return false;
+  }
+
+  const std::string hash = base::SHA1HashString(packed_ip);
+  encoded_ip->resize(base::kSHA1Length + 1, '\x00');

[Scott Hess - ex-Googler, 2016/11/14 20:52:10]

Does the char matter?  AFAICT everything will be overwritten.

[vakh (use Gerrit instead), 2016/11/15 00:37:02]

Done.

+  encoded_ip->replace(0, hash.size(), hash);
+  (*encoded_ip)[20] = static_cast<unsigned char>(128);

[Scott Hess - ex-Googler, 2016/11/14 20:52:10]

I think you should consistently use hash.size() instead of base::kSHA1Length and
the literal 20.  OR you should consistently use base::kSHA1Length in all three
places and ignore (or DCHECK) hash.size().

[vakh (use Gerrit instead), 2016/11/15 00:37:02]

Done.

+  return true;
+}
+
 void V4LocalDatabaseManager::OnFullHashResponse(
     std::unique_ptr<PendingCheck> pending_check,
     const std::vector<FullHashInfo>& full_hash_infos) {