V4LDBM: Get response from GetHashManager, detect severest result

components/safe_browsing_db/v4_local_database_manager.h

 // Copyright 2016 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 COMPONENTS_SAFE_BROWSING_DB_V4_LOCAL_DATABASE_MANAGER_H_
 #define COMPONENTS_SAFE_BROWSING_DB_V4_LOCAL_DATABASE_MANAGER_H_
 
 // A class that provides the interface between the SafeBrowsing protocol manager
 // and database that holds the downloaded updates.
 
 #include <memory>
 
 #include "components/safe_browsing_db/database_manager.h"
 #include "components/safe_browsing_db/hit_report.h"
 #include "components/safe_browsing_db/v4_database.h"
 #include "components/safe_browsing_db/v4_get_hash_protocol_manager.h"
 #include "components/safe_browsing_db/v4_protocol_manager_util.h"
 #include "components/safe_browsing_db/v4_update_protocol_manager.h"
 #include "url/gurl.h"
 
 using content::ResourceType;
 
 namespace safe_browsing {
 
+typedef unsigned ThreatSeverity;
+
 // Manages the local, on-disk database of updates downloaded from the
 // SafeBrowsing service and interfaces with the protocol manager.
 class V4LocalDatabaseManager : public SafeBrowsingDatabaseManager {
  public:
+  enum class CheckType {

[Nathan Parker, 2016/09/20 17:51:09]

I think this name is a bit too general. Does this map to the use-case of the
caller, and hence to the check function they called, like
db_mananger->CheckFOO()?  Or maybe there's an equivalency set of these three:
  1) Which CheckFOO() function was called 
  2) which "CheckType" is used
  3) Which client->callback_function should be used 


There are 12 Check/Match functions (including the two IsXXXKillSwitchOn()), only
5 of which take a Client* and so would have a callback.  The others check
synchronously, and maybe assume that the DB has 32-byte hashes for that list so
no network call is possibly required.

The enum could be named ClientCallbackType, since that's how it's used.
Ultimately it'd have 5 values.


An aside: In the long run, I'd be glad to get rid of Client* and just have the
requestor pass in a callback to receive the results. But I don't think we want
to switch that now, since you need a consistent DBManager API so you can swap
the implementation cleanly.

[vakh (use Gerrit instead), 2016/09/20 18:21:17]

Yes, it is supposed to reflect the function to call on the Client object. I've
changed the name of the enum.
Ack.

[Scott Hess - ex-Googler, 2016/09/20 21:55:55]

OMG +++!!!1!111!!oneoneone

+    // This represents the case when we're trying to determine if a URL is
+    // unsafe from the following perspectives: Malware, Phishing, UwS.
+    CHECK_BROWSE_URL = 0,
+
+    // This should always be the last value.
+    CHECK_MAX
+  };
+
+  // The information we need to return the response to the SafeBrowsing client
+  // that asked for the safety reputation of a URL (or a full hash) if we can't
+  // determine that synchronously.
+  struct PendingCheck {

[Nathan Parker, 2016/09/20 17:51:09]

Will this be used just for single URL checks, or other data types (list-of-URLs,
digests, DLL-filenames, etc).

And might as well make it private.

[vakh (use Gerrit instead), 2016/09/20 18:21:17]

URLs for now. Other types later.
Done.

+    PendingCheck(CheckType check_type, Client* client, const GURL& url);
+
+    ~PendingCheck();
+
+    // Determines which funtion from the |client| needs to be called once we
+    // know whether each of the the URL(s) in |urls| is safe or unsafe.
+    CheckType check_type;
+
+    // The SafeBrowsing client that's waiting for the safe/unsafe verdict.
+    Client* client;
+
+    // The URL that are being checked for being unsafe.
+    GURL url;

[Nathan Parker, 2016/09/20 17:51:09]

Should this be a vector, like the comments above imply?

[vakh (use Gerrit instead), 2016/09/20 18:21:17]

Not now, later.

+
+    // The metadata associated with the full hash of the severest match found
+    // for that URL.
+    ThreatMetadata url_metadata;
+
+    // The threat verdict for the URL being checked.
+    SBThreatType result_threat_type;
+  };
+
   // Construct V4LocalDatabaseManager.
   // Must be initialized by calling StartOnIOThread() before using.
   V4LocalDatabaseManager(const base::FilePath& base_path);
 
   //
   // SafeBrowsingDatabaseManager implementation
   //
 
   bool IsSupported() const override;
   safe_browsing::ThreatSource GetThreatSource() const override;
@@ skipping 22 matching lines @@
  protected:
   std::unordered_set<UpdateListIdentifier> GetStoresForFullHashRequests()
       override;
 
  private:
   friend class V4LocalDatabaseManagerTest;
   void SetTaskRunnerForTest(
       const scoped_refptr<base::SequencedTaskRunner>& task_runner) {
     task_runner_ = task_runner;
   }
+  FRIEND_TEST_ALL_PREFIXES(V4LocalDatabaseManagerTest,
+                           TestGetSeverestThreatTypeAndMetadata);
+
+  // The set of clients awaiting a full hash response.
+  typedef std::unordered_set<Client*> PendingClients;
 
   ~V4LocalDatabaseManager() override;
 
   // The callback called each time the protocol manager downloads updates
   // successfully.
   void UpdateRequestCompleted(
       std::unique_ptr<ParsedServerResponse> parsed_server_response);
 
   void SetupUpdateProtocolManager(
       net::URLRequestContextGetter* request_context_getter,
       const V4ProtocolConfig& config);
 
   void SetupDatabase();
 
-  void OnFullHashResponse(const std::vector<FullHashInfo>& full_hash_infos);
+  // Called when the |v4_get_hash_protocol_manager_| has the full hash response
+  // avaialble for the URL that we requested. It determines the severest
+  // threat type and responds to the |client| with that information.
+  void OnFullHashResponse(std::unique_ptr<PendingCheck> pending_check,
+                          const std::vector<FullHashInfo>& full_hash_infos);
 
+  // Called when all the stores managed by the database have been read from
+  // disk after startup and the database is ready for use.
   void DatabaseReady(std::unique_ptr<V4Database> v4_database);
 
   // Called when the database has been updated and schedules the next update.
   void DatabaseUpdated();
 
+  // Calls the appopriate method on the |client| object, based on the contents
+  // of |pending_check|.
+  void RespondToClient(std::unique_ptr<PendingCheck> pending_check);
+
+  // Finds the most severe |SBThreatType| and the corresponding |metadata| from
+  // |full_hash_infos|.
+  static void GetSeverestThreatTypeAndMetadata(
+      SBThreatType* result_threat_type,
+      ThreatMetadata* metadata,
+      const std::vector<FullHashInfo>& full_hash_infos);
+
   // The base directory under which to create the files that contain hashes.
   const base::FilePath base_path_;
 
   // Whether the service is running.
   bool enabled_;
 
-  // Stores the current status of the lists to download from the SafeBrowsing
-  // servers.
-  // TODO(vakh): current_list_states_ doesn't really belong here.
-  // It should come through the database, from the various V4Stores.
-  base::hash_map<UpdateListIdentifier, std::string> current_list_states_;
+  // The set of clients that are waiting for a gull hash response from the

[Nathan Parker, 2016/09/20 17:51:09]

nit: s/gull/full

[vakh (use Gerrit instead), 2016/09/20 18:21:17]

Done.

+  // SafeBrowsing service.
+  PendingClients pending_clients_;
 
   // The list of stores to manage (for hash prefixes and full hashes), along
   // with the corresponding filename on disk for each of them.
   StoreIdAndFileNames store_id_file_names_;
 
   // The protocol manager that downloads the hash prefix updates.
   std::unique_ptr<V4UpdateProtocolManager> v4_update_protocol_manager_;
 
   // The database that manages the stores containing the hash prefix updates.
   // All writes to this variable must happen on the IO thread only.
   std::unique_ptr<V4Database> v4_database_;
 
   // Called when the V4Database has finished applying the latest update and is
   // ready to process next update.
   DatabaseUpdatedCallback db_updated_callback_;
 
   // The sequenced task runner for running safe browsing database operations.
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
 
   friend class base::RefCountedThreadSafe<V4LocalDatabaseManager>;
   DISALLOW_COPY_AND_ASSIGN(V4LocalDatabaseManager);
 };  // class V4LocalDatabaseManager
 
 }  // namespace safe_browsing
 
 #endif  // COMPONENTS_SAFE_BROWSING_DB_V4_LOCAL_DATABASE_MANAGER_H_