Use std::array in PackedListModule to contain the SHA1 hash.

This allows us to use the hashes as value types with convenient
assignment and comparison operators.

Also add myself as owner of the third_party_dlls subdirectory as
pennymac@chromium.org no longer works on Chrome.

Change-Id: I66838fd4890637bf53faa22fbcbbfcae6456b583
Reviewed-on: https://chromium-review.googlesource.com/c/1340967Reviewed-by: Cait Phillips <caitkp@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609374}

chrome/browser/conflicts/module_blacklist_cache_updater_win.cc

@@ -135,13 +135,13 @@ void PopulatePackedListModule(
       base::i18n::ToLower(module_key.module_path.BaseName().value()));
   base::SHA1HashBytes(reinterpret_cast<const uint8_t*>(module_basename.data()),
                       module_basename.length(),
-                      packed_list_module->basename_hash);
+                      &packed_list_module->basename_hash[0]);
 
   // Hash the code id.
   const std::string module_code_id = GenerateCodeId(module_key);
   base::SHA1HashBytes(reinterpret_cast<const uint8_t*>(module_code_id.data()),
                       module_code_id.length(),
-                      packed_list_module->code_id_hash);
+                      &packed_list_module->code_id_hash[0]);
 
   packed_list_module->time_date_stamp =
       CalculateTimeDateStamp(base::Time::Now());

chrome/browser/conflicts/module_blacklist_cache_updater_win_unittest.cc

@@ -349,10 +349,10 @@ TEST_F(ModuleBlacklistCacheUpdaterTest, RegisteredModules) {
   const std::string module_basename = base::UTF16ToUTF8(
       base::i18n::ToLower(module_key2.module_path.BaseName().value()));
   base::SHA1HashBytes(reinterpret_cast<const uint8_t*>(module_basename.data()),
-                      module_basename.length(), expected.basename_hash);
+                      module_basename.length(), &expected.basename_hash[0]);
   const std::string module_code_id = GenerateCodeId(module_key2);
   base::SHA1HashBytes(reinterpret_cast<const uint8_t*>(module_code_id.data()),
-                      module_code_id.length(), expected.code_id_hash);
+                      module_code_id.length(), &expected.code_id_hash[0]);
 
   EXPECT_TRUE(internal::ModuleEqual()(expected, blacklisted_modules[0]));
 }

chrome/browser/conflicts/module_blacklist_cache_util_win.cc

@@ -8,6 +8,7 @@
 #include <functional>
 #include <iterator>
 #include <string>
+#include <tuple>
 #include <utility>
 
 #include "base/files/file.h"
@@ -237,30 +238,15 @@ int64_t CalculateExpectedFileSize(
 bool ModuleLess::operator()(
     const third_party_dlls::PackedListModule& lhs,
     const third_party_dlls::PackedListModule& rhs) const {
-  auto is_less = [](const auto& lhs, const auto& rhs) {
-    return std::lexicographical_compare(std::begin(lhs), std::end(lhs),
-                                        std::begin(rhs), std::end(rhs));
-  };
-
-  if (is_less(lhs.basename_hash, rhs.basename_hash))
-    return true;
-
-  if (is_less(rhs.basename_hash, lhs.basename_hash))
-    return false;
-
-  return is_less(lhs.code_id_hash, rhs.code_id_hash);
+  return std::tie(lhs.basename_hash, lhs.code_id_hash) <
+         std::tie(rhs.basename_hash, rhs.code_id_hash);
 }
 
 bool ModuleEqual::operator()(
     const third_party_dlls::PackedListModule& lhs,
     const third_party_dlls::PackedListModule& rhs) const {
-  auto are_equal = [](const auto& lhs, const auto& rhs) {
-    return std::equal(std::begin(lhs), std::end(lhs), std::begin(rhs),
-                      std::end(rhs));
-  };
-
-  return are_equal(lhs.basename_hash, rhs.basename_hash) &&
-         are_equal(lhs.code_id_hash, rhs.code_id_hash);
+  return lhs.basename_hash == rhs.basename_hash &&
+         lhs.code_id_hash == rhs.code_id_hash;
 }
 
 bool ModuleTimeDateStampGreater::operator()(

chrome/browser/conflicts/module_blacklist_cache_util_win_unittest.cc

@@ -18,6 +18,7 @@
 #include "base/stl_util.h"
 #include "base/time/time.h"
 #include "chrome/browser/conflicts/module_list_filter_win.h"
+#include "chrome_elf/sha1/sha1.h"
 #include "chrome_elf/third_party_dlls/packed_list_format.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -42,9 +43,7 @@ std::vector<third_party_dlls::PackedListModule> CreateUniqueModuleEntries(
 
   for (auto& entry : entries) {
     // Fill up each bytes for both SHA1 hashes.
-    for (size_t i = 0;
-         i < arraysize(third_party_dlls::PackedListModule::basename_hash);
-         ++i) {
+    for (size_t i = 0; i < elf_sha1::kSHA1Length; ++i) {
       entry.basename_hash[i] = byte_distribution(random_engine);
       entry.code_id_hash[i] = byte_distribution(random_engine);
     }
@@ -186,9 +185,11 @@ class FakeModuleListFilter : public ModuleListFilter {
 
   void AddWhitelistedModule(const third_party_dlls::PackedListModule& module) {
     whitelisted_modules_.emplace(
-        base::StringPiece(reinterpret_cast<const char*>(module.basename_hash),
+        base::StringPiece(
+            reinterpret_cast<const char*>(&module.basename_hash[0]),
             base::size(module.basename_hash)),
-        base::StringPiece(reinterpret_cast<const char*>(module.code_id_hash),
+        base::StringPiece(
+            reinterpret_cast<const char*>(&module.code_id_hash[0]),
             base::size(module.basename_hash)));
   }
 

chrome_elf/OWNERS

 caitkp@chromium.org
 pennymac@chromium.org
+pmonette@chromium.org
 robertshield@chromium.org
 
 # TEAM: security-dev@chromium.org

chrome_elf/sha1/sha1.cc

@@ -205,26 +205,11 @@ void SHA1HashBytes(const unsigned char* data, size_t len, unsigned char* hash) {
 //------------------------------------------------------------------------------
 // Public functions
 //------------------------------------------------------------------------------
-
-int CompareHashes(const uint8_t* first, const uint8_t* second) {
-  // Compare bytes, high-order to low-order.
-  for (size_t i = 0; i < kSHA1Length; ++i) {
-    if (first[i] < second[i])
-      return -1;
-    if (first[i] > second[i])
-      return 1;
-    // else they are equal, continue;
-  }
-
-  return 0;
-}
-
-std::string SHA1HashString(const std::string& str) {
-  char hash[kSHA1Length] = {};
+Digest SHA1HashString(const std::string& str) {
+  Digest digest;
   SHA1HashBytes(reinterpret_cast<const unsigned char*>(str.c_str()),
-                str.length(), reinterpret_cast<unsigned char*>(hash));
-
-  return std::string(hash, kSHA1Length);
+                str.length(), reinterpret_cast<unsigned char*>(&digest[0]));
+  return digest;
 }
 
 }  // namespace elf_sha1

chrome_elf/sha1/sha1.h

@@ -11,6 +11,7 @@
 
 #include <stddef.h>
 
+#include <array>
 #include <string>
 
 namespace elf_sha1 {
@@ -18,16 +19,10 @@ namespace elf_sha1 {
 // Length in bytes of a SHA-1 hash.
 constexpr size_t kSHA1Length = 20;
 
-// Compare two hashes.
-// - Returns -1 if |first| < |second|
-// - Returns 0 if |first| == |second|
-// - Returns 1 if |first| > |second|
-// Note: Arguments should be kSHA1Length long.
-int CompareHashes(const uint8_t* first, const uint8_t* second);
+using Digest = std::array<uint8_t, kSHA1Length>;
 
-// Computes the SHA1 hash of the input string |str| and returns the full
-// hash.  The returned SHA1 will be 20 bytes in length.
-std::string SHA1HashString(const std::string& str);
+// Returns the computed SHA1 of the input string |str|.
+Digest SHA1HashString(const std::string& str);
 
 }  // namespace elf_sha1
 

chrome_elf/sha1/sha1_unittest.cc

@@ -20,12 +20,11 @@ TEST(SHA1Test, Test1) {
   // Example A.1 from FIPS 180-2: one-block message.
   std::string input = "abc";
 
-  int expected[] = {0xa9, 0x99, 0x3e, 0x36, 0x47, 0x06, 0x81, 0x6a, 0xba, 0x3e,
-                    0x25, 0x71, 0x78, 0x50, 0xc2, 0x6c, 0x9c, 0xd0, 0xd8, 0x9d};
+  elf_sha1::Digest expected = {0xa9, 0x99, 0x3e, 0x36, 0x47, 0x06, 0x81,
+                               0x6a, 0xba, 0x3e, 0x25, 0x71, 0x78, 0x50,
+                               0xc2, 0x6c, 0x9c, 0xd0, 0xd8, 0x9d};
 
-  std::string output = elf_sha1::SHA1HashString(input);
-  for (size_t i = 0; i < elf_sha1::kSHA1Length; i++)
-    EXPECT_EQ(expected[i], output[i] & 0xFF);
+  EXPECT_EQ(elf_sha1::SHA1HashString(input), expected);
 }
 
 TEST(SHA1Test, Test2) {
@@ -33,24 +32,22 @@ TEST(SHA1Test, Test2) {
   std::string input =
       "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq";
 
-  int expected[] = {0x84, 0x98, 0x3e, 0x44, 0x1c, 0x3b, 0xd2, 0x6e, 0xba, 0xae,
-                    0x4a, 0xa1, 0xf9, 0x51, 0x29, 0xe5, 0xe5, 0x46, 0x70, 0xf1};
+  elf_sha1::Digest expected = {0x84, 0x98, 0x3e, 0x44, 0x1c, 0x3b, 0xd2,
+                               0x6e, 0xba, 0xae, 0x4a, 0xa1, 0xf9, 0x51,
+                               0x29, 0xe5, 0xe5, 0x46, 0x70, 0xf1};
 
-  std::string output = elf_sha1::SHA1HashString(input);
-  for (size_t i = 0; i < elf_sha1::kSHA1Length; i++)
-    EXPECT_EQ(expected[i], output[i] & 0xFF);
+  EXPECT_EQ(elf_sha1::SHA1HashString(input), expected);
 }
 
 TEST(SHA1Test, Test3) {
   // Example A.3 from FIPS 180-2: long message.
   std::string input(1000000, 'a');
 
-  int expected[] = {0x34, 0xaa, 0x97, 0x3c, 0xd4, 0xc4, 0xda, 0xa4, 0xf6, 0x1e,
-                    0xeb, 0x2b, 0xdb, 0xad, 0x27, 0x31, 0x65, 0x34, 0x01, 0x6f};
+  elf_sha1::Digest expected = {0x34, 0xaa, 0x97, 0x3c, 0xd4, 0xc4, 0xda,
+                               0xa4, 0xf6, 0x1e, 0xeb, 0x2b, 0xdb, 0xad,
+                               0x27, 0x31, 0x65, 0x34, 0x01, 0x6f};
 
-  std::string output = elf_sha1::SHA1HashString(input);
-  for (size_t i = 0; i < elf_sha1::kSHA1Length; i++)
-    EXPECT_EQ(expected[i], output[i] & 0xFF);
+  EXPECT_EQ(elf_sha1::SHA1HashString(input), expected);
 }
 
 }  // namespace

chrome_elf/third_party_dlls/hook.cc

@@ -294,25 +294,24 @@ NTSTATUS NewNtMapViewOfSectionImpl(
   }
 
   // Note that one of either image_name or section_basename can be empty.
-  std::string image_name_hash;
+  elf_sha1::Digest image_name_hash;
   if (!image_name.empty())
     image_name_hash = elf_sha1::SHA1HashString(image_name);
-  std::string section_basename_hash;
+  elf_sha1::Digest section_basename_hash;
   if (!section_basename.empty())
     section_basename_hash = elf_sha1::SHA1HashString(section_basename);
-  std::string fingerprint_hash =
-      GetFingerprintString(time_date_stamp, image_size);
-  fingerprint_hash = elf_sha1::SHA1HashString(fingerprint_hash);
+  elf_sha1::Digest fingerprint_hash = elf_sha1::SHA1HashString(
+      GetFingerprintString(time_date_stamp, image_size));
 
   // Check sources for blacklist decision.
   bool block = false;
 
-  if (!image_name_hash.empty() &&
+  if (!image_name.empty() &&
       IsModuleListed(image_name_hash, fingerprint_hash)) {
     // 1) Third-party DLL blacklist, check for image name from PE header.
     block = true;
-  } else if (!section_basename_hash.empty() &&
-             section_basename_hash.compare(image_name_hash) != 0 &&
+  } else if (!section_basename.empty() &&
+             section_basename_hash != image_name_hash &&
              IsModuleListed(section_basename_hash, fingerprint_hash)) {
     // 2) Third-party DLL blacklist, check for image name from the section.
     block = true;

chrome_elf/third_party_dlls/main_unittest.cc

@@ -6,6 +6,8 @@
 
 #include <windows.h>
 
+#include <string>
+
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
@@ -98,15 +100,11 @@ PackedListModule GeneratePackedListModule(const std::string& image_name,
   // Internally, an empty string should not be passed in here.
   assert(!image_name.empty());
 
-  // SHA1 hash the two strings, and copy them into the new struct.
-  std::string code_id = GetFingerprintString(timedatestamp, imagesize);
-  code_id = elf_sha1::SHA1HashString(code_id);
-  std::string name_hash = elf_sha1::SHA1HashString(image_name);
-
+  // SHA1 hash the two strings into the new struct.
   PackedListModule packed_module;
-  ::memcpy(packed_module.code_id_hash, code_id.data(), elf_sha1::kSHA1Length);
-  ::memcpy(packed_module.basename_hash, name_hash.data(),
-           elf_sha1::kSHA1Length);
+  packed_module.code_id_hash =
+      elf_sha1::SHA1HashString(GetFingerprintString(timedatestamp, imagesize));
+  packed_module.basename_hash = elf_sha1::SHA1HashString(image_name);
 
   return packed_module;
 }
@@ -480,15 +478,14 @@ TEST_F(ThirdPartyTest, SHA1SanityCheck) {
   // Get hashes from base_sha1.
   const std::string module_basename_hash =
       base::SHA1HashString(base::UTF16ToUTF8(kChineseUnicode));
-  const std::string module_code_id_hash =
-      base::SHA1HashString(base::StringPrintf(
-          "%08lX%lx", module_data.timedatestamp, module_data.imagesize));
+  const std::string module_code_id_hash = base::SHA1HashString(
+      GetFingerprintString(module_data.timedatestamp, module_data.imagesize));
 
   // Compare the hashes.
-  EXPECT_EQ(::memcmp(elf_sha1_generated.basename_hash,
+  EXPECT_EQ(::memcmp(&elf_sha1_generated.basename_hash[0],
                      module_basename_hash.data(), elf_sha1::kSHA1Length),
             0);
-  EXPECT_EQ(::memcmp(elf_sha1_generated.code_id_hash,
+  EXPECT_EQ(::memcmp(&elf_sha1_generated.code_id_hash[0],
                      module_code_id_hash.data(), elf_sha1::kSHA1Length),
             0);
 }

chrome_elf/third_party_dlls/packed_list_file.cc

@@ -42,7 +42,7 @@ std::wstring& GetBlFilePath() {
 // std::equal_range/std::is_sorted. Must return TRUE if lhs < rhs.
 bool HashBinaryPredicate(const PackedListModule& lhs,
                          const PackedListModule& rhs) {
-  return elf_sha1::CompareHashes(lhs.basename_hash, rhs.basename_hash) < 0;
+  return lhs.basename_hash < rhs.basename_hash;
 }
 
 // Given a file opened for read, pull in the packed list.
@@ -172,19 +172,16 @@ ThirdPartyStatus InitInternal() {
 // Public defines & functions
 //------------------------------------------------------------------------------
 
-bool IsModuleListed(const std::string& basename_hash,
-                    const std::string& fingerprint_hash) {
+bool IsModuleListed(const elf_sha1::Digest& basename_hash,
+                    const elf_sha1::Digest& fingerprint_hash) {
   assert(g_initialized);
-  assert(!basename_hash.empty() && !fingerprint_hash.empty());
-  assert(basename_hash.length() == elf_sha1::kSHA1Length &&
-         fingerprint_hash.length() == elf_sha1::kSHA1Length);
 
   if (!g_bl_module_array_size)
     return false;
 
   PackedListModule target = {};
-  ::memcpy(target.basename_hash, basename_hash.data(), elf_sha1::kSHA1Length);
-  ::memcpy(target.code_id_hash, fingerprint_hash.data(), elf_sha1::kSHA1Length);
+  target.basename_hash = basename_hash;
+  target.code_id_hash = fingerprint_hash;
 
   // Binary search for primary hash (basename).  There can be more than one
   // match.
@@ -194,7 +191,7 @@ bool IsModuleListed(const std::string& basename_hash,
 
   // Search for secondary hash.
   for (PackedListModule* i = pair.first; i != pair.second; ++i) {
-    if (!elf_sha1::CompareHashes(target.code_id_hash, i->code_id_hash))
+    if (target.code_id_hash == i->code_id_hash)
       return true;
   }
 

chrome_elf/third_party_dlls/packed_list_file.h

@@ -7,14 +7,15 @@
 
 #include <string>
 
+#include "chrome_elf/sha1/sha1.h"
 #include "chrome_elf/third_party_dlls/status_codes.h"
 
 namespace third_party_dlls {
 
 // Look up a binary based on the required data points.
 // - Returns true if match found in the list.
-bool IsModuleListed(const std::string& basename_hash,
-                    const std::string& fingerprint_hash);
+bool IsModuleListed(const elf_sha1::Digest& basename_hash,
+                    const elf_sha1::Digest& fingerprint_hash);
 
 // Get the full path of the blacklist file used.
 std::wstring GetBlFilePathUsed();

chrome_elf/third_party_dlls/packed_list_file_unittest.cc

@@ -79,8 +79,6 @@ bool GetTestModules(std::vector<TestModule>* test_modules,
   base::FilePath path;
   char buffer[kPageSize];
   PIMAGE_NT_HEADERS nt_headers = nullptr;
-  std::string code_id;
-  std::string basename_hash;
   test_modules->clear();
   packed_modules->clear();
 
@@ -106,15 +104,11 @@ bool GetTestModules(std::vector<TestModule>* test_modules,
     test_modules->push_back(test_module);
 
     // SHA1 hash the two strings, and copy them into the module array.
-    code_id = base::StringPrintf("%08lX%lx", test_module.timedatestamp,
-                                 test_module.imagesize);
-    code_id = elf_sha1::SHA1HashString(code_id);
-    basename_hash = elf_sha1::SHA1HashString(test_module.basename);
-
     PackedListModule packed_module;
-    ::memcpy(packed_module.code_id_hash, code_id.data(), elf_sha1::kSHA1Length);
-    ::memcpy(packed_module.basename_hash, basename_hash.data(),
-             elf_sha1::kSHA1Length);
+    packed_module.code_id_hash = elf_sha1::SHA1HashString(
+        GetFingerprintString(test_module.timedatestamp, test_module.imagesize));
+    packed_module.basename_hash =
+        elf_sha1::SHA1HashString(test_module.basename);
     packed_modules->push_back(packed_module);
   }
 
@@ -198,22 +192,18 @@ TEST_F(ThirdPartyFileTest, Success) {
   // Init.
   ASSERT_EQ(InitFromFile(), ThirdPartyStatus::kSuccess);
 
-  std::string fingerprint_hash;
-  std::string name_hash;
-
   // Test matching.
   for (const auto& test_module : GetTestArray()) {
-    fingerprint_hash =
-        GetFingerprintString(test_module.timedatestamp, test_module.imagesize);
-    fingerprint_hash = elf_sha1::SHA1HashString(fingerprint_hash);
-    name_hash = elf_sha1::SHA1HashString(test_module.basename);
+    elf_sha1::Digest name_hash = elf_sha1::SHA1HashString(test_module.basename);
+    elf_sha1::Digest fingerprint_hash = elf_sha1::SHA1HashString(
+        GetFingerprintString(test_module.timedatestamp, test_module.imagesize));
     EXPECT_TRUE(IsModuleListed(name_hash, fingerprint_hash));
   }
 
   // Test a failure to match.
-  fingerprint_hash = GetFingerprintString(0x12345678, 1337);
-  fingerprint_hash = elf_sha1::SHA1HashString(fingerprint_hash);
-  name_hash = elf_sha1::SHA1HashString("booya.dll");
+  elf_sha1::Digest name_hash = elf_sha1::SHA1HashString("booya.dll");
+  elf_sha1::Digest fingerprint_hash =
+      elf_sha1::SHA1HashString(GetFingerprintString(0x12345678, 1337));
   EXPECT_FALSE(IsModuleListed(name_hash, fingerprint_hash));
 }
 
@@ -222,9 +212,9 @@ TEST_F(ThirdPartyFileTest, NoFiles) {
   // kFileNotFound is a non-fatal status code.
   ASSERT_EQ(InitFromFile(), ThirdPartyStatus::kFileNotFound);
 
-  std::string fingerprint_hash = GetFingerprintString(0x12345678, 1337);
-  fingerprint_hash = elf_sha1::SHA1HashString(fingerprint_hash);
-  std::string name_hash = elf_sha1::SHA1HashString("booya.dll");
+  elf_sha1::Digest name_hash = elf_sha1::SHA1HashString("booya.dll");
+  elf_sha1::Digest fingerprint_hash =
+      elf_sha1::SHA1HashString(GetFingerprintString(0x12345678, 1337));
   EXPECT_FALSE(IsModuleListed(name_hash, fingerprint_hash));
 }
 

chrome_elf/third_party_dlls/packed_list_format.h

@@ -10,6 +10,7 @@
 #ifndef CHROME_ELF_THIRD_PARTY_DLLS_PACKED_LIST_FORMAT_H_
 #define CHROME_ELF_THIRD_PARTY_DLLS_PACKED_LIST_FORMAT_H_
 
+#include <array>
 #include <string>
 
 #include <stdint.h>
@@ -53,9 +54,9 @@ struct PackedListMetadata {
 
 struct PackedListModule {
   // SHA1 of lowercase, UTF-8 basename (no path).
-  uint8_t basename_hash[elf_sha1::kSHA1Length];
+  elf_sha1::Digest basename_hash;
   // Code ID. Get the formatted string via GetFingerprintString(), then SHA1.
-  uint8_t code_id_hash[elf_sha1::kSHA1Length];
+  elf_sha1::Digest code_id_hash;
   // A timestamp used for tracking "last attempted load".  Used to manage
   // lifetime of entries in the local caches.
   uint32_t time_date_stamp;