Remove legacy Module Verifier.

BUG=480352,454716,412045

Review URL: https://codereview.chromium.org/1083193007

Cr-Commit-Position: refs/heads/master@{#330603}

chrome/browser/safe_browsing/incident_reporting/environment_data_collection_win.cc

@@ -130,67 +130,31 @@ void CollectModuleVerificationData(
     size_t num_modules_to_verify,
     ClientIncidentReport_EnvironmentData_Process* process) {
 #if !defined(_WIN64)
+  using ModuleState = ClientIncidentReport_EnvironmentData_Process_ModuleState;
+
   for (size_t i = 0; i < num_modules_to_verify; ++i) {
-    scoped_ptr<ClientIncidentReport_EnvironmentData_Process_ModuleState>
-        module_state(
-            new ClientIncidentReport_EnvironmentData_Process_ModuleState());
+    scoped_ptr<ModuleState> module_state(new ModuleState());
 
-    VerificationResult result = NewVerifyModule(modules_to_verify[i],
-                                                module_state.get());
+    int num_bytes_different = 0;
+    bool scan_complete = VerifyModule(modules_to_verify[i],
+                                      module_state.get(),
+                                      &num_bytes_different);
 
-    std::set<std::string> modified_exports;
-    int num_bytes = 0;
-    int modified = VerifyModule(modules_to_verify[i],
-                                &modified_exports,
-                                &num_bytes);
+    if (module_state->modified_state() == ModuleState::MODULE_STATE_UNMODIFIED)
+      continue;
 
-    if (result.state == MODULE_STATE_MODIFIED) {
+    if (module_state->modified_state() == ModuleState::MODULE_STATE_MODIFIED) {
       UMA_HISTOGRAM_COUNTS_10000(
           "ModuleIntegrityVerification.BytesModified.WithoutByteSet",
-          result.num_bytes_different);
-    }
-
-    if (modified == MODULE_STATE_MODIFIED) {
-      UMA_HISTOGRAM_COUNTS_10000(
-          "ModuleIntegrityVerification.BytesModified.WithByteSet",
-          num_bytes);
+          num_bytes_different);
     }
 
-    if (modified == MODULE_STATE_MODIFIED ||
-        result.state == MODULE_STATE_MODIFIED) {
-      int difference = abs(result.num_bytes_different - num_bytes);
-
-      if (result.num_bytes_different > num_bytes) {
-        UMA_HISTOGRAM_COUNTS_10000(
-            "ModuleIntegrityVerification.Difference.WithoutByteSet",
-            difference);
-      } else if (num_bytes > result.num_bytes_different) {
-        UMA_HISTOGRAM_COUNTS_10000(
-            "ModuleIntegrityVerification.Difference.WithByteSet",
-            difference);
-      }
-    }
-
-    if (!result.verification_completed) {
+    if (!scan_complete) {
       UMA_HISTOGRAM_ENUMERATION(
           "ModuleIntegrityVerification.RelocationsUnordered", i,
           num_modules_to_verify);
     }
 
-    if (modified == MODULE_STATE_UNMODIFIED)
-      continue;
-
-    module_state->set_name(base::WideToUTF8(modules_to_verify[i]));
-    // Add 1 to the ModuleState enum to get the corresponding value in the
-    // protobuf's ModuleState enum.
-    module_state->set_modified_state(static_cast<
-        ClientIncidentReport_EnvironmentData_Process_ModuleState_ModifiedState>(
-        modified + 1));
-    for (std::set<std::string>::iterator it = modified_exports.begin();
-         it != modified_exports.end();
-         ++it) {
-      module_state->add_modified_export(*it);
-    }
     process->mutable_module_state()->AddAllocated(module_state.release());
   }
 #endif  // _WIN64

chrome/browser/safe_browsing/incident_reporting/environment_data_collection_win_unittest.cc

@@ -208,7 +208,8 @@ TEST(SafeBrowsingEnvironmentDataCollectionWinTest, VerifyLoadedModules) {
   ASSERT_LE(1, process_report.module_state_size());
 #if !defined(ADDRESS_SANITIZER)
   EXPECT_EQ(1, process_report.module_state_size());
-  EXPECT_EQ(1, process_report.module_state(0).modified_export_size());
+  EXPECT_EQ(1, process_report.module_state(0).modification_size());
+  EXPECT_TRUE(process_report.module_state(0).modification(0).has_export_name());
 #endif
 
   EXPECT_EQ(base::WideToUTF8(kTestDllNames[0]),
@@ -219,7 +220,7 @@ TEST(SafeBrowsingEnvironmentDataCollectionWinTest, VerifyLoadedModules) {
   // See comment above about AddressSantizier.
 #if !defined(ADDRESS_SANITIZER)
   EXPECT_EQ(std::string(kTestExportName),
-            process_report.module_state(0).modified_export(0));
+            process_report.module_state(0).modification(0).export_name());
 #endif
 }
 #endif  // _WIN64

chrome/browser/safe_browsing/incident_reporting/module_integrity_verifier_win.h

@@ -7,11 +7,6 @@
 
 #include <stdint.h>
 
-#include <set>
-#include <string>
-
-#include "chrome/common/safe_browsing/csd.pb.h"
-
 namespace base {
 namespace win {
 class PEImage;
@@ -21,21 +16,7 @@ class PEImageAsData;
 
 namespace safe_browsing {
 
-// This enum defines the possible module states VerifyModule can return.
-enum ModuleState {
-  MODULE_STATE_UNKNOWN,
-  MODULE_STATE_UNMODIFIED,
-  MODULE_STATE_MODIFIED,
-};
-
-struct VerificationResult {
-  ModuleState state;
-  // The number of bytes with different values on disk and in memory.
-  int num_bytes_different;
-  // True if the relocations were ordered and the verification was fully
-  // completed.
-  bool verification_completed;
-};
+class ClientIncidentReport_EnvironmentData_Process_ModuleState;
 
 // Helper to grab the addresses and size of the code section of a PEImage.
 // Returns two addresses: one for the dll loaded as a library, the other for the
@@ -46,18 +27,15 @@ bool GetCodeAddrsAndSize(const base::win::PEImage& mem_peimage,
                          uint8_t** disk_code_addr,
                          uint32_t* code_size);
 
-// Examines the code section of the given module in memory and on disk, looking
-// for unexpected differences.  Returns a ModuleState and and a set of the
-// possibly modified exports.
-ModuleState VerifyModule(const wchar_t* module_name,
-                         std::set<std::string>* modified_exports,
-                         int* num_bytes_different);
-
 // Examines the code section of the given module in memory and on disk, looking
 // for unexpected differences and populating |module_state| in the process.
-VerificationResult NewVerifyModule(
+// Returns true if the entire image was scanned. |num_bytes_different| is
+// populated with the number of differing bytes found, even if the scan failed
+// to complete.
+bool VerifyModule(
     const wchar_t* module_name,
-    ClientIncidentReport_EnvironmentData_Process_ModuleState* module_state);
+    ClientIncidentReport_EnvironmentData_Process_ModuleState* module_state,
+    int* num_bytes_different);
 
 }  // namespace safe_browsing
 

chrome/common/safe_browsing/csd.proto

@@ -474,7 +474,7 @@ message ClientIncidentReport {
         }
         optional string name = 1;
         optional ModifiedState modified_state = 2;
-        repeated string modified_export = 3;
+        repeated string OBSOLETE_modified_export = 3;
 
         message Modification {
           optional uint32 file_offset = 1;

tools/metrics/histograms/histograms.xml

@@ -16446,6 +16446,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
 </histogram>
 
 <histogram name="ModuleIntegrityVerification.Difference" units="bytes">
+  <obsolete>
+    Deprecated and removed from code as of 05/2015.
+  </obsolete>
   <owner>anthonyvd@chromium.org</owner>
   <summary>
     Represents the difference in bytes deemed modified by the two
@@ -67783,8 +67786,12 @@ To add a new entry, add it with any value and run test to compute valid value.
       label="The version of the Module Integrity Verifier that doesn't use a
              hash set to track relocations."/>
   <suffix name="WithByteSet"
-      label="The version of the Module Integrity Verifier that uses a hash
-             set to track relocations."/>
+      label="The version of the Module Integrity Verifier that uses a hash set
+             to track relocations.">
+    <obsolete>
+      Deprecated and removed from code as of 05/2015.
+    </obsolete>
+  </suffix>
   <affected-histogram name="ModuleIntegrityVerification.BytesModified"/>
   <affected-histogram name="ModuleIntegrityVerification.Difference"/>
 </histogram_suffixes>