Safebrowsing: change gethash caching to match api 2.3 rules, fix some corner cases.

chrome/browser/safe_browsing/safe_browsing_database.h

 // 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.
 
 #ifndef CHROME_BROWSER_SAFE_BROWSING_SAFE_BROWSING_DATABASE_H_
 #define CHROME_BROWSER_SAFE_BROWSING_SAFE_BROWSING_DATABASE_H_
 
 #include <map>
 #include <set>
 #include <string>
@@ skipping 75 matching lines @@
   }
 
   virtual ~SafeBrowsingDatabase();
 
   // Initializes the database with the given filename.
   virtual void Init(const base::FilePath& filename) = 0;
 
   // Deletes the current database and creates a new one.
   virtual bool ResetDatabase() = 0;
 
-  // Returns false if |url| is not in the browse database.  If it
-  // returns true, then either |matching_list| is the name of the matching
-  // list, or |prefix_hits| and |full_hits| contains the matching hash
-  // prefixes.  This function is safe to call from threads other than
-  // the creation thread.
+  // Returns false if |url| is not in the browse database or already was cached
+  // as a miss.  If it returns true, |prefix_hits| contains matching hash
+  // prefixes which had no cached results and |cache_hits| contains any matching
+  // cached gethash results.  This function is safe to call from threads other
+  // than the creation thread.
   virtual bool ContainsBrowseUrl(const GURL& url,
-                                 std::string* matching_list,
                                  std::vector<SBPrefix>* prefix_hits,
-                                 std::vector<SBFullHashResult>* full_hits,
-                                 base::Time last_update) = 0;
+                                 std::vector<SBFullHashResult>* cache_hits) = 0;
 
   // Returns false if none of |urls| are in Download database. If it returns
   // true, |prefix_hits| should contain the prefixes for the URLs that were in
   // the database.  This function could ONLY be accessed from creation thread.
   virtual bool ContainsDownloadUrl(const std::vector<GURL>& urls,
                                    std::vector<SBPrefix>* prefix_hits) = 0;
 
   // Returns false if |url| is not on the client-side phishing detection
   // whitelist.  Otherwise, this function returns true.  Note: the whitelist
   // only contains full-length hashes so we don't return any prefix hit.
@@ skipping 49 matching lines @@
   virtual bool UpdateStarted(std::vector<SBListChunkRanges>* lists) = 0;
   virtual void InsertChunks(const std::string& list_name,
                             const SBChunkList& chunks) = 0;
   virtual void DeleteChunks(
       const std::vector<SBChunkDelete>& chunk_deletes) = 0;
   virtual void UpdateFinished(bool update_succeeded) = 0;
 
   // Store the results of a GetHash response. In the case of empty results, we
   // cache the prefixes until the next update so that we don't have to issue
   // further GetHash requests we know will be empty.
-  virtual void CacheHashResults(
-      const std::vector<SBPrefix>& prefixes,
-      const std::vector<SBFullHashResult>& full_hits) = 0;
+  virtual void CacheHashResults(const std::vector<SBPrefix>& prefixes,
+                                const std::vector<SBFullHashResult>& full_hits,
+                                const base::TimeDelta& cache_lifetime) = 0;
 
   // Returns true if the malware IP blacklisting killswitch URL is present
   // in the csd whitelist.
   virtual bool IsMalwareIPMatchKillSwitchOn() = 0;
 
   // The name of the bloom-filter file for the given database file.
   // NOTE(shess): OBSOLETE.  Present for deleting stale files.
   static base::FilePath BloomFilterForFilename(
       const base::FilePath& db_filename);
 
@@ skipping 96 matching lines @@
   // Create a database with a browse store. This is a legacy interface that
   // useds Sqlite.
   SafeBrowsingDatabaseNew();
 
   virtual ~SafeBrowsingDatabaseNew();
 
   // Implement SafeBrowsingDatabase interface.
   virtual void Init(const base::FilePath& filename) OVERRIDE;
   virtual bool ResetDatabase() OVERRIDE;
   virtual bool ContainsBrowseUrl(const GURL& url,
-                                 std::string* matching_list,
                                  std::vector<SBPrefix>* prefix_hits,
-                                 std::vector<SBFullHashResult>* full_hits,
-                                 base::Time last_update) OVERRIDE;
+                                 std::vector<SBFullHashResult>* cache_hits)
+      OVERRIDE;
   virtual bool ContainsDownloadUrl(const std::vector<GURL>& urls,
                                    std::vector<SBPrefix>* prefix_hits) OVERRIDE;
   virtual bool ContainsCsdWhitelistedUrl(const GURL& url) OVERRIDE;
   virtual bool ContainsDownloadWhitelistedUrl(const GURL& url) OVERRIDE;
   virtual bool ContainsDownloadWhitelistedString(
       const std::string& str) OVERRIDE;
   virtual bool ContainsExtensionPrefixes(
       const std::vector<SBPrefix>& prefixes,
       std::vector<SBPrefix>* prefix_hits) OVERRIDE;
   virtual bool ContainsSideEffectFreeWhitelistUrl(const GURL& url)  OVERRIDE;
   virtual bool ContainsMalwareIP(const std::string& ip_address) OVERRIDE;
   virtual bool UpdateStarted(std::vector<SBListChunkRanges>* lists) OVERRIDE;
   virtual void InsertChunks(const std::string& list_name,
                             const SBChunkList& chunks) OVERRIDE;
   virtual void DeleteChunks(
       const std::vector<SBChunkDelete>& chunk_deletes) OVERRIDE;
   virtual void UpdateFinished(bool update_succeeded) OVERRIDE;
-  virtual void CacheHashResults(
-      const std::vector<SBPrefix>& prefixes,
-      const std::vector<SBFullHashResult>& full_hits) OVERRIDE;
+  virtual void CacheHashResults(const std::vector<SBPrefix>& prefixes,
+                                const std::vector<SBFullHashResult>& full_hits,
+                                const base::TimeDelta& cache_lifetime) OVERRIDE;
 
   // Returns the value of malware_kill_switch_;
   virtual bool IsMalwareIPMatchKillSwitchOn() OVERRIDE;
 
  private:
   friend class SafeBrowsingDatabaseTest;
   FRIEND_TEST_ALL_PREFIXES(SafeBrowsingDatabaseTest, HashCaching);
 
   // A SafeBrowsing whitelist contains a list of whitelisted full-hashes (stored
   // in a sorted vector) as well as a boolean flag indicating whether all
   // lookups in the whitelist should be considered matches for safety.
   typedef std::pair<std::vector<SBFullHash>, bool> SBWhitelist;
 
   // This map holds a csd malware IP blacklist which maps a prefix mask
   // to a set of hashed blacklisted IP prefixes.  Each IP prefix is a hashed
   // IPv6 IP prefix using SHA-1.
   typedef std::map<std::string, base::hash_set<std::string> > IPBlacklist;
 
+  // Like ContainsBrowseUrl, but takes a vector of fullhashes instead of an URL.
+  // |full_hashes| must be sorted.

[Scott Hess - ex-Googler, 2014/04/01 22:08:36]

Perhaps describe this as a helper for ContainsBrowseUrl.  Also reference the
sort required for full_hashes.

[mattm, 2014/04/03 01:38:12]

Done.
Changed the wording to make it more obvious.

+  bool ContainsBrowseUrlHashes(const std::vector<SBFullHash>& full_hashes,
+                               std::vector<SBPrefix>* prefix_hits,
+                               std::vector<SBFullHashResult>* cache_hits);
+
   // Returns true if the whitelist is disabled or if any of the given hashes
   // matches the whitelist.
   bool ContainsWhitelistedHashes(const SBWhitelist& whitelist,
                                  const std::vector<SBFullHash>& hashes);
 
   // Return the browse_store_, download_store_, download_whitelist_store or
   // csd_whitelist_store_ based on list_id.
   SafeBrowsingStore* GetStore(int list_id);
 
   // Deletes the files on disk.
@@ skipping 46 matching lines @@
                             SafeBrowsingStore* store,
                             SBWhitelist* whitelist);
   void UpdateIpBlacklistStore();
 
   // Used to verify that various calls are made from the thread the
   // object was created on.
   base::MessageLoop* creation_loop_;
 
   // Lock for protecting access to variables that may be used on the
   // IO thread.  This includes |prefix_set_|, |full_browse_hashes_|,
-  // |pending_browse_hashes_|, |prefix_miss_cache_|, |csd_whitelist_|.
+  // |browse_gethash_cache_|, |csd_whitelist_|.
   base::Lock lookup_lock_;
 
   // Underlying persistent store for chunk data.
   // For browsing related (phishing and malware URLs) chunks and prefixes.
   base::FilePath browse_filename_;
   scoped_ptr<SafeBrowsingStore> browse_store_;
 
   // For download related (download URL and binary hash) chunks and prefixes.
   base::FilePath download_filename_;
   scoped_ptr<SafeBrowsingStore> download_store_;
@@ skipping 20 matching lines @@
   base::FilePath ip_blacklist_filename_;
   scoped_ptr<SafeBrowsingStore> ip_blacklist_store_;
 
   SBWhitelist csd_whitelist_;
   SBWhitelist download_whitelist_;
   SBWhitelist extension_blacklist_;
 
   // The IP blacklist should be small.  At most a couple hundred IPs.
   IPBlacklist ip_blacklist_;
 
-  // Cached browse store related full-hash items, ordered by prefix for
-  // efficient scanning.
-  // |full_browse_hashes_| are items from |browse_store_|,
-  // |pending_browse_hashes_| are items from |CacheHashResults()|, which
-  // will be pushed to the store on the next update.
+  // Full-hashes from browse store, ordered by prefix for efficient scanning.
   std::vector<SBAddFullHash> full_browse_hashes_;
-  std::vector<SBAddFullHash> pending_browse_hashes_;
 
-  // Cache of prefixes that returned empty results (no full hash
-  // match) to |CacheHashResults()|.  Cached to prevent asking for
-  // them every time.  Cleared on next update.
-  std::set<SBPrefix> prefix_miss_cache_;
+  // Cache of gethash results for browse store. Entries should not be used if
+  // they are older than their expire_after field.  Cached misses will have
+  // empty full_hashes field.  Cleared on each update.
+  std::map<SBPrefix, SBCachedFullHashResult> browse_gethash_cache_;
 
   // Used to schedule resetting the database because of corruption.
   base::WeakPtrFactory<SafeBrowsingDatabaseNew> reset_factory_;
 
   // Set if corruption is detected during the course of an update.
   // Causes the update functions to fail with no side effects, until
   // the next call to |UpdateStarted()|.
   bool corruption_detected_;
 
   // Set to true if any chunks are added or deleted during an update.
   // Used to optimize away database update.
   bool change_detected_;
 
   // Used to check if a prefix was in the browse database.
   base::FilePath browse_prefix_set_filename_;
   scoped_ptr<safe_browsing::PrefixSet> browse_prefix_set_;
 
   // Used to check if a prefix was in the browse database.
   base::FilePath side_effect_free_whitelist_prefix_set_filename_;
   scoped_ptr<safe_browsing::PrefixSet> side_effect_free_whitelist_prefix_set_;
 };
 
 #endif  // CHROME_BROWSER_SAFE_BROWSING_SAFE_BROWSING_DATABASE_H_