Addition of file digest, version and PE headers for incidents related…

… to blacklisted modules.

- Added relevant field in the protobuffer BlacklistLoadIncident inside
  ClientIncidentReport.
- Added code to compute the file digest and get the file version and PE
  headers of the blacklisted DLL.

Copied from https://codereview.chromium.org/661603002/ by georgesak@.

BUG=None
TBR=sky@chromium.org

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

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

chrome/browser/safe_browsing/binary_feature_extractor.cc

@@ -0,0 +1,40 @@
+// Copyright 2014 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/binary_feature_extractor.h"
+
+#include "base/files/file.h"
+#include "base/files/file_path.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/common/safe_browsing/csd.pb.h"
+#include "crypto/secure_hash.h"
+#include "crypto/sha2.h"
+
+namespace safe_browsing {
+
+void BinaryFeatureExtractor::ExtractDigest(
+    const base::FilePath& file_path,
+    ClientDownloadRequest_Digests* digests) {
+  base::File file(file_path, base::File::FLAG_OPEN | base::File::FLAG_READ);
+  if (file.IsValid()) {
+    const int kBufferSize = 1 << 12;
+    scoped_ptr<char[]> buf(new char[kBufferSize]);
+    scoped_ptr<crypto::SecureHash> ctx(
+        crypto::SecureHash::Create(crypto::SecureHash::SHA256));
+    int len = 0;
+    while (true) {
+      len = file.ReadAtCurrentPos(buf.get(), kBufferSize);
+      if (len <= 0)
+        break;
+      ctx->Update(buf.get(), len);
+    }
+    if (!len) {
+      uint8_t hash[crypto::kSHA256Length];
+      ctx->Finish(hash, sizeof(hash));
+      digests->set_sha256(hash, sizeof(hash));
+    }
+  }
+}
+
+}  // namespace safe_browsing

chrome/browser/safe_browsing/binary_feature_extractor.h

@@ -16,6 +16,7 @@ class FilePath;
 }
 
 namespace safe_browsing {
+class ClientDownloadRequest_Digests;
 class ClientDownloadRequest_ImageHeaders;
 class ClientDownloadRequest_SignatureInfo;
 
@@ -35,6 +36,10 @@ class BinaryFeatureExtractor
       const base::FilePath& file_path,
       ClientDownloadRequest_ImageHeaders* image_headers);
 
+  // Populates |digests.sha256| with the SHA256 digest of |file_path|.
+  virtual void ExtractDigest(const base::FilePath& file_path,
+                             ClientDownloadRequest_Digests* digests);
+
  protected:
   friend class base::RefCountedThreadSafe<BinaryFeatureExtractor>;
   virtual ~BinaryFeatureExtractor();

chrome/browser/safe_browsing/binary_feature_extractor_unittest.cc

@@ -0,0 +1,101 @@
+// Copyright 2014 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/binary_feature_extractor.h"
+
+#include "base/base_paths.h"
+#include "base/files/file.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/path_service.h"
+#include "chrome/common/safe_browsing/csd.pb.h"
+#include "crypto/sha2.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace safe_browsing {
+
+class BinaryFeatureExtractorTest : public testing::Test {
+ protected:
+  BinaryFeatureExtractorTest() : extractor_(new BinaryFeatureExtractor()) {}
+
+  void SetUp() override {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    path_ = temp_dir_.path().Append(FILE_PATH_LITERAL("file.dll"));
+  }
+
+  // Writes |size| bytes from |data| to |path_|.
+  void WriteFileToHash(const char* data, int size) {
+    base::File file(path_, base::File::FLAG_CREATE | base::File::FLAG_WRITE);
+    ASSERT_TRUE(file.IsValid());
+    ASSERT_EQ(size, file.WriteAtCurrentPos(data, size));
+  }
+
+  // Verifies that |path_| hashes to |digest|.
+  void ExpectFileDigestEq(const uint8_t* digest) {
+    ClientDownloadRequest_Digests digests;
+    extractor_->ExtractDigest(path_, &digests);
+    EXPECT_TRUE(digests.has_sha256());
+    EXPECT_EQ(std::string(reinterpret_cast<const char*>(digest),
+                          crypto::kSHA256Length),
+              digests.sha256());
+  }
+
+  static const int kBlockSize = 1 << 12;
+  scoped_refptr<BinaryFeatureExtractor> extractor_;
+  base::ScopedTempDir temp_dir_;
+
+  // The path to a file that may be hashed.
+  base::FilePath path_;
+};
+
+TEST_F(BinaryFeatureExtractorTest, ExtractDigestNoFile) {
+  base::FilePath no_file =
+      temp_dir_.path().Append(FILE_PATH_LITERAL("does_not_exist.dll"));
+
+  ClientDownloadRequest_Digests digests;
+  extractor_->ExtractDigest(no_file, &digests);
+  EXPECT_FALSE(digests.has_sha256());
+}
+
+// Hash a file that is less than 1 4k block.
+TEST_F(BinaryFeatureExtractorTest, ExtractSmallDigest) {
+  static const uint8_t kDigest[] = {
+      0x70, 0x27, 0x7b, 0xad, 0xfc, 0xb9, 0x97, 0x6b, 0x24, 0xf9, 0x80,
+      0x22, 0x26, 0x2c, 0x31, 0xea, 0x8f, 0xb2, 0x1f, 0x54, 0x93, 0x6b,
+      0x69, 0x8b, 0x5d, 0x54, 0xd4, 0xd4, 0x21, 0x0b, 0x98, 0xb7};
+
+  static const char kFileData[] = {"The mountains are robotic."};
+  static const int kDataLen = sizeof(kFileData) - 1;
+  WriteFileToHash(kFileData, kDataLen);
+  ExpectFileDigestEq(kDigest);
+}
+
+// Hash a file that is exactly 1 4k block.
+TEST_F(BinaryFeatureExtractorTest, ExtractOneBlockDigest) {
+  static const uint8_t kDigest[] = {
+      0x4f, 0x93, 0x6e, 0xee, 0x89, 0x55, 0xa5, 0xe7, 0x46, 0xd0, 0x61,
+      0x43, 0x54, 0x5f, 0x33, 0x7b, 0xdc, 0x30, 0x3a, 0x4b, 0x18, 0xb4,
+      0x82, 0x20, 0xe3, 0x93, 0x4c, 0x65, 0xe0, 0xc1, 0xc0, 0x19};
+
+  const int kDataLen = kBlockSize;
+  scoped_ptr<char[]> data(new char[kDataLen]);
+  memset(data.get(), 71, kDataLen);
+  WriteFileToHash(data.get(), kDataLen);
+  ExpectFileDigestEq(kDigest);
+}
+
+// Hash a file that is larger than 1 4k block.
+TEST_F(BinaryFeatureExtractorTest, ExtractBigBlockDigest) {
+  static const uint8_t kDigest[] = {
+      0xda, 0xae, 0xa0, 0xd5, 0x3b, 0xce, 0x0b, 0x4e, 0x5f, 0x5d, 0x0b,
+      0xc7, 0x6a, 0x69, 0x0e, 0xf1, 0x8b, 0x2d, 0x20, 0xcd, 0xf2, 0x6d,
+      0x33, 0xa7, 0x70, 0xf3, 0x6b, 0x85, 0xbf, 0xce, 0x9d, 0x5c};
+
+  const int kDataLen = kBlockSize + 1;
+  scoped_ptr<char[]> data(new char[kDataLen]);
+  memset(data.get(), 71, kDataLen);
+  WriteFileToHash(data.get(), kDataLen);
+  ExpectFileDigestEq(kDigest);
+}
+
+}  // namespace safe_browsing

chrome/browser/safe_browsing/incident_reporting/blacklist_load_analyzer_win.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/safe_browsing/incident_reporting/blacklist_load_analyzer.h"
 
+#include "base/file_version_info.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
@@ -49,22 +50,51 @@ void VerifyBlacklistLoadState(const AddIncidentCallback& callback) {
 
     const bool blacklist_intialized = blacklist::IsBlacklistInitialized();
 
-    std::vector<base::string16>::const_iterator module_iter(
-        module_names.begin());
-    for (; module_iter != module_names.end(); ++module_iter) {
+    for (const auto& module_name : module_names) {
       scoped_ptr<ClientIncidentReport_IncidentData> incident_data(
           new ClientIncidentReport_IncidentData());
       ClientIncidentReport_IncidentData_BlacklistLoadIncident* blacklist_load =
           incident_data->mutable_blacklist_load();
 
-      base::FilePath module_path(*module_iter);
-      path_sanitizer.StripHomeDirectory(&module_path);
+      const base::FilePath module_path(module_name);
 
-      blacklist_load->set_path(base::WideToUTF8(module_path.value()));
-      // TODO(robertshield): Add computation of file digest and version here.
+      // Sanitized path.
+      base::FilePath sanitized_path(module_path);
+      path_sanitizer.StripHomeDirectory(&sanitized_path);
+      blacklist_load->set_path(base::WideToUTF8(sanitized_path.value()));
 
+      // Digest.
+      scoped_refptr<safe_browsing::BinaryFeatureExtractor>
+          binary_feature_extractor(new BinaryFeatureExtractor());
+      base::TimeTicks start_time = base::TimeTicks::Now();
+      binary_feature_extractor->ExtractDigest(module_path,
+                                              blacklist_load->mutable_digest());
+      UMA_HISTOGRAM_TIMES("SBIRS.BLAHashTime",
+                          base::TimeTicks::Now() - start_time);
+
+      // Version.
+      scoped_ptr<FileVersionInfo> version_info(
+          FileVersionInfo::CreateFileVersionInfo(module_path));
+      if (version_info) {
+        std::wstring file_version = version_info->file_version();
+        if (!file_version.empty())
+          blacklist_load->set_version(base::WideToUTF8(file_version));
+      }
+
+      // Initialized state.
       blacklist_load->set_blacklist_initialized(blacklist_intialized);
 
+      // Signature.
+      start_time = base::TimeTicks::Now();
+      binary_feature_extractor->CheckSignature(
+          module_path, blacklist_load->mutable_signature());
+      UMA_HISTOGRAM_TIMES("SBIRS.BLASignatureTime",
+                          base::TimeTicks::Now() - start_time);
+
+      // Image headers.
+      binary_feature_extractor->ExtractImageHeaders(
+          module_path, blacklist_load->mutable_image_headers());
+
       // Send the report.
       callback.Run(incident_data.Pass());
     }

chrome/chrome.gyp

@@ -510,9 +510,11 @@
           'type': 'executable',
           'dependencies': [
             '../base/base.gyp:base',
+            '../crypto/crypto.gyp:crypto',
             'safe_browsing_proto',
           ],
           'sources': [
+            'browser/safe_browsing/binary_feature_extractor.cc',
             'browser/safe_browsing/binary_feature_extractor.h',
             'browser/safe_browsing/binary_feature_extractor_win.cc',
             'browser/safe_browsing/pe_image_reader_win.cc',

chrome/chrome_browser.gypi

@@ -2287,6 +2287,7 @@
       'browser/download/download_completion_blocker.h',
       'browser/renderer_host/safe_browsing_resource_throttle.cc',
       'browser/renderer_host/safe_browsing_resource_throttle.h',
+      'browser/safe_browsing/binary_feature_extractor.cc',
       'browser/safe_browsing/binary_feature_extractor.h',
       'browser/safe_browsing/binary_feature_extractor_posix.cc',
       'browser/safe_browsing/binary_feature_extractor_win.cc',

chrome/chrome_tests_unit.gypi

@@ -659,6 +659,7 @@
       'browser/resources/print_preview/print_preview_utils_unittest.gtestjs',
       'browser/resources_util_unittest.cc',
       'browser/rlz/rlz_unittest.cc',
+      'browser/safe_browsing/binary_feature_extractor_unittest.cc',
       'browser/safe_browsing/binary_feature_extractor_win_unittest.cc',
       'browser/safe_browsing/browser_feature_extractor_unittest.cc',
       'browser/safe_browsing/chunk_range_unittest.cc',

chrome/common/safe_browsing/csd.proto

@@ -361,6 +361,8 @@ message ClientIncidentReport {
       optional ClientDownloadRequest.Digests digest = 2;
       optional string version = 3;
       optional bool blacklist_initialized = 4;
+      optional ClientDownloadRequest.SignatureInfo signature = 5;
+      optional ClientDownloadRequest.ImageHeaders image_headers = 6;
     }
     message OmniboxInteractionIncident {
       optional string origin = 1;

tools/metrics/histograms/histograms.xml

@@ -31063,6 +31063,20 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="SBIRS.BLAHashTime" units="milliseconds">
+  <owner>grt@google.com</owner>
+  <summary>
+    The elapsed time to compute the hash of a blacklisted module.
+  </summary>
+</histogram>
+
+<histogram name="SBIRS.BLASignatureTime" units="milliseconds">
+  <owner>grt@google.com</owner>
+  <summary>
+    The elapsed time to validate the signature of a blacklisted module.
+  </summary>
+</histogram>
+
 <histogram name="SBIRS.DiscardedIncident" enum="IncidentType">
   <owner>grt@google.com</owner>
   <summary>