Revert "[chrome_elf, third-party block support] full hook testing."

This reverts commit 2c24de69.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 562467 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzJjMjRkZTY5NTMyY2IyNmNjMTk5YmIwN2Y5ZWJmYTY3MzBiYzI2ZTIM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.win/Win7%20Tests%20%28dbg%29%281%29/69346

Sample Failed Step: chrome_elf_unittests

Original change's description:
> [chrome_elf, third-party block support] full hook testing.
> 
> Adding some full tests, including some corner cases.  These tests
> initialize the whole third_party_dlls project, to exercise the new hook.
> 
> Non-test adjustments:
> - pe_image_safe: |directory| == 0 is valid!  (export dir)
> - hook:  Exposed a GetDataFromImageForTesting() function.
>   Also a few fixes in the hook shim:
>   * Use the PEImageSafe constructor that takes an HMODULE.
>   * Use the new GetFingerprintString() api.
>   * Don't hash an empty image or section name.
>   * Pass a name hash instead of non-hashed into LogLoadAttempt
>     if a DLL was allowed.
> - packed_list_file:
>   * GetFingerprintHash() removed from this project.
>   * Also, gracefully handle a blacklist file that exists, but is empty.
> - packed_list_format:
>   * Added "new" GetFingerprintString() API here.
>     It centralizes the format string without doing the hash.
> 
> Test: chrome_elf_unittests.exe, ThirdPartyTest.*
> Bug: 769590
> Change-Id: I592f76b884312bb267eb6747a478e271fc96e689
> Reviewed-on: https://chromium-review.googlesource.com/1069694
> Commit-Queue: Penny MacNeil <pennymac@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#562467}

Change-Id: I46cf1feddbc80a60ca92b44152d2f7ad0ef865e7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 769590
Reviewed-on: https://chromium-review.googlesource.com/1077029
Cr-Commit-Position: refs/heads/master@{#562542}

chrome_elf/BUILD.gn

@@ -290,7 +290,6 @@ test("chrome_elf_unittests") {
     "third_party_dlls/imes_unittest.cc",
     "third_party_dlls/logs_unittest.cc",
     "third_party_dlls/main_unittest.cc",
-    "third_party_dlls/main_unittest_exe.h",
     "third_party_dlls/packed_list_file_unittest.cc",
   ]
   include_dirs = [ "$target_gen_dir" ]
@@ -321,8 +320,6 @@ test("chrome_elf_unittests") {
     ":blacklist_test_dll_2",
     ":blacklist_test_dll_3",
     ":chrome_elf",
-    ":main_unittest_dll_1",
-    ":main_unittest_dll_2",
     ":third_party_dlls_test_exe",
   ]
 }
@@ -365,36 +362,13 @@ test("chrome_elf_import_unittests") {
   ]
 }
 
-shared_library("main_unittest_dll_1") {
-  testonly = true
-  sources = [
-    "third_party_dlls/main_unittest_dll_1.cc",
-  ]
-  deps = [
-    "//build/config:exe_and_shlib_deps",
-  ]
-}
-
-shared_library("main_unittest_dll_2") {
-  testonly = true
-  sources = [
-    "third_party_dlls/main_unittest_dll_2.cc",
-    "third_party_dlls/main_unittest_dll_2.def",
-  ]
-  deps = [
-    "//build/config:exe_and_shlib_deps",
-  ]
-}
-
 executable("third_party_dlls_test_exe") {
   testonly = true
   sources = [
     "third_party_dlls/main_unittest_exe.cc",
-    "third_party_dlls/main_unittest_exe.h",
   ]
   deps = [
     ":third_party_dlls",
-    "//base",
     "//build/config:exe_and_shlib_deps",
     "//build/win:default_exe_manifest",
     "//chrome/install_static:install_static_util",

chrome_elf/pe_image_safe/pe_image_safe.cc

@@ -128,7 +128,7 @@ void* PEImageSafe::RVAToAddr(DWORD rva) {
 
 void* PEImageSafe::GetImageDirectoryEntryAddr(int directory,
                                               DWORD* directory_size) {
-  assert(directory >= 0 && directory < IMAGE_NUMBEROF_DIRECTORY_ENTRIES &&
+  assert(directory > 0 && directory < IMAGE_NUMBEROF_DIRECTORY_ENTRIES &&
          ldr_image_mapping_);
 
   // GetOptionalHeader() validates the optional header.

chrome_elf/pe_image_safe/pe_image_safe.h

@@ -45,8 +45,6 @@ class PEImageSafe {
   // Some functions can only be used on images that have been memory mapped by
   // the NT Loader (e.g. LoadLibrary).  This constructor must be used to enable
   // that functionality.
-  // - Note: Full load or LOAD_LIBRARY_AS_IMAGE_RESOURCE required.
-  // LOAD_LIBRARY_AS_DATAFILE* is not sufficient for some APIs.
   PEImageSafe(HMODULE buffer, DWORD buffer_size)
       : image_(buffer), image_size_(buffer_size) {
     ldr_image_mapping_ = !LDR_IS_DATAFILE(buffer);

chrome_elf/third_party_dlls/DEPS

@@ -15,7 +15,7 @@ include_rules = [
   "+chrome_elf/third_party_dlls",
 ]
 specific_include_rules = {
-  ".*_unittest.*": [
+  ".*_unittest\.cc": [
     "+base",
   ]
 }
\ No newline at end of file

chrome_elf/third_party_dlls/hook.cc

@@ -16,7 +16,6 @@
 #include "chrome_elf/third_party_dlls/logs.h"
 #include "chrome_elf/third_party_dlls/main.h"
 #include "chrome_elf/third_party_dlls/packed_list_file.h"
-#include "chrome_elf/third_party_dlls/packed_list_format.h"
 #include "sandbox/win/src/interception_internal.h"
 #include "sandbox/win/src/internal_types.h"
 #include "sandbox/win/src/nt_internals.h"
@@ -166,8 +165,7 @@ bool GetDataFromImage(PVOID buffer,
   section_path->clear();
   section_basename->clear();
 
-  pe_image_safe::PEImageSafe image(reinterpret_cast<HMODULE>(buffer),
-                                   buffer_size);
+  pe_image_safe::PEImageSafe image(buffer, buffer_size);
   PIMAGE_FILE_HEADER file_header = image.GetFileHeader();
   if (!file_header ||
       image.GetImageBitness() == pe_image_safe::ImageBitness::kUnknown) {
@@ -206,7 +204,6 @@ bool GetDataFromImage(PVOID buffer,
 
   // For now, consider it a success if at least one source results in a name.
   // Allow for the rare case of one or the other not being there.
-  // (E.g.: a module could have no export directory.)
   if (image_name->empty() && temp_section_path.empty())
     return false;
 
@@ -281,16 +278,13 @@ NTSTATUS NewNtMapViewOfSectionImpl(
     return ret;
   }
 
-  // Note that one of either image_name or section_basename can be empty.
-  std::string image_name_hash;
-  if (!image_name.empty())
-    image_name_hash = elf_sha1::SHA1HashString(image_name);
-  std::string section_basename_hash;
-  if (!section_basename.empty())
-    section_basename_hash = elf_sha1::SHA1HashString(section_basename);
+  // Note that one of either image_name or section_basename can be empty, and
+  // the resulting hash string would be empty as well.
+  std::string image_name_hash = elf_sha1::SHA1HashString(image_name);
+  std::string section_basename_hash =
+      elf_sha1::SHA1HashString(section_basename);
   std::string fingerprint_hash =
-      GetFingerprintString(image_size, time_date_stamp);
-  fingerprint_hash = elf_sha1::SHA1HashString(fingerprint_hash);
+      GetFingerprintHash(image_size, time_date_stamp);
 
   // Check sources for blacklist decision.
   bool block = false;
@@ -321,8 +315,7 @@ NTSTATUS NewNtMapViewOfSectionImpl(
     // No block.
     // Ensure a non-null image name for the log.  Prefer the section basename
     // (to match the path).
-    name_matched =
-        section_basename.empty() ? &image_name_hash : &section_basename_hash;
+    name_matched = section_basename.empty() ? &image_name : &section_basename;
   }
   // IME is an explicit whitelist.
   // TODO(pennymac): create an explicit allow LogType?
@@ -469,18 +462,4 @@ HookStatus ApplyHook() {
   return HookStatus::kSuccess;
 }
 
-bool GetDataFromImageForTesting(PVOID buffer,
-                                DWORD buffer_size,
-                                DWORD* time_date_stamp,
-                                DWORD* image_size,
-                                std::string* image_name,
-                                std::string* section_path,
-                                std::string* section_basename) {
-  if (!g_nt_query_virtual_memory_func)
-    InitImports();
-
-  return GetDataFromImage(buffer, buffer_size, time_date_stamp, image_size,
-                          image_name, section_path, section_basename);
-}
-
 }  // namespace third_party_dlls

chrome_elf/third_party_dlls/hook.h

@@ -5,10 +5,6 @@
 #ifndef CHROME_ELF_THIRD_PARTY_DLLS_HOOK_H_
 #define CHROME_ELF_THIRD_PARTY_DLLS_HOOK_H_
 
-#include <windows.h>
-
-#include <string>
-
 namespace third_party_dlls {
 
 // "static_cast<int>(HookStatus::value)" to access underlying value.
@@ -25,15 +21,6 @@ enum class HookStatus {
 // - Ensure the rest of third_party_dlls is initialized before hooking.
 HookStatus ApplyHook();
 
-// Testing-only access to private GetDataFromImage() function.
-bool GetDataFromImageForTesting(PVOID buffer,
-                                DWORD buffer_size,
-                                DWORD* time_date_stamp,
-                                DWORD* image_size,
-                                std::string* image_name,
-                                std::string* section_path,
-                                std::string* section_basename);
-
 }  // namespace third_party_dlls
 
 #endif  // CHROME_ELF_THIRD_PARTY_DLLS_HOOK_H_

chrome_elf/third_party_dlls/main_unittest_dll_1.cc

-// Copyright 2018 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 <windows.h>
-
-BOOL APIENTRY DllMain(HMODULE module, DWORD reason, LPVOID reserved) {
-  return TRUE;
-}

chrome_elf/third_party_dlls/main_unittest_dll_2.cc

-// Copyright 2018 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 <windows.h>
-
-extern "C" {
-// Have a dummy export so that the module gets an export table entry.
-void DummyExport() {}
-}  // extern "C"
-
-BOOL APIENTRY DllMain(HMODULE module, DWORD reason, LPVOID reserved) {
-  return TRUE;
-}

chrome_elf/third_party_dlls/main_unittest_dll_2.def

-; Copyright 2018 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.
-
-LIBRARY  "main_unittest_dll_2.dll"
-
-EXPORTS
-  DummyExport

chrome_elf/third_party_dlls/main_unittest_exe.cc

@@ -3,41 +3,10 @@
 // found in the LICENSE file.
 #include <windows.h>
 
-#include "chrome_elf/third_party_dlls/main_unittest_exe.h"
-
 #include <stdlib.h>
 
-#include <shellapi.h>
-
-#include "base/files/file.h"
-#include "base/scoped_native_library.h"
-#include "base/strings/utf_string_conversions.h"
 #include "chrome/install_static/product_install_details.h"
-#include "chrome_elf/third_party_dlls/logging_api.h"
 #include "chrome_elf/third_party_dlls/main.h"
-#include "chrome_elf/third_party_dlls/packed_list_file.h"
-
-using namespace third_party_dlls::main_unittest_exe;
-
-namespace {
-
-// Function object which invokes LocalFree on its parameter, which must be
-// a pointer.  To be used with std::unique_ptr and CommandLineToArgvW().
-struct LocalFreeDeleter {
-  inline void operator()(wchar_t** ptr) const { ::LocalFree(ptr); }
-};
-
-// Attempt to load a given DLL.
-ExitCode LoadDll(std::wstring name) {
-  base::FilePath dll_path(name);
-  base::ScopedNativeLibrary dll(dll_path);
-  if (!dll.is_valid())
-    return kDllLoadFailed;
-
-  return kDllLoadSuccess;
-}
-
-}  // namespace
 
 //------------------------------------------------------------------------------
 // PUBLIC
@@ -47,84 +16,18 @@ ExitCode LoadDll(std::wstring name) {
 // - Init third_party_dlls, which will apply a hook to NtMapViewOfSection.
 // - Attempt to load a specific DLL.
 //
-// Arguments:
-// #1: path to test blacklist file (mandatory).
-// #2: test identifier (mandatory).
-// #3: path to dll (test-identifier dependent).
-//
 // Returns:
 // - Negative values in case of unexpected error.
 // - 0 for successful DLL load.
 // - 1 for failed DLL load.
-int main() {
-  // NOTE: The arguments must be treated as unicode for these tests.
-  int argument_count = 0;
-  std::unique_ptr<wchar_t* [], LocalFreeDeleter> argv(
-      ::CommandLineToArgvW(::GetCommandLineW(), &argument_count));
-  if (!argv)
-    return kBadCommandLine;
-
+int main(int argc, char** argv) {
   if (third_party_dlls::IsThirdPartyInitialized())
-    return kThirdPartyAlreadyInitialized;
+    _exit(-1);
 
   install_static::InitializeProductDetailsForPrimaryModule();
 
-  // Get the required arguments, path to blacklist file and test id to run.
-  if (argument_count < 3)
-    return kMissingArgument;
-
-  const wchar_t* blacklist_path = argv[1];
-  if (!blacklist_path || ::wcslen(blacklist_path) == 0) {
-    return kBadBlacklistPath;
-  }
-
-  const wchar_t* arg2 = argv[2];
-  int test_id = ::_wtoi(arg2);
-  if (!test_id)
-    return kUnsupportedTestId;
-
-  // Override blacklist path before initializing.
-  third_party_dlls::OverrideFilePathForTesting(blacklist_path);
-
   if (!third_party_dlls::Init())
-    return kThirdPartyInitFailure;
-
-  // Switch on test id.
-  switch (test_id) {
-    // Just initialization.
-    case kTestOnlyInitialization:
-      break;
-    // Single DLL load.
-    case kTestSingleDllLoad: {
-      if (argument_count < 4)
-        return kMissingArgument;
-      const wchar_t* dll_name = argv[3];
-      if (!dll_name || ::wcslen(dll_name) == 0)
-        return kBadArgument;
-      ExitCode code = LoadDll(dll_name);
-
-      // Get logging.  Ensure the log is as expected.
-      uint32_t bytes = 0;
-      DrainLog(nullptr, 0, &bytes);
-      if (!bytes)
-        return kEmptyLog;
-      auto buffer = std::unique_ptr<uint8_t[]>(new uint8_t[bytes]);
-      bytes = DrainLog(&buffer[0], bytes, nullptr);
-      third_party_dlls::LogEntry* entry =
-          reinterpret_cast<third_party_dlls::LogEntry*>(&buffer[0]);
-      if ((code == kDllLoadFailed &&
-           entry->type != third_party_dlls::kBlocked) ||
-          (code == kDllLoadSuccess &&
-           entry->type != third_party_dlls::kAllowed)) {
-        return kUnexpectedLog;
-      }
-
-      return code;
-    }
-    // Unsupported argument.
-    default:
-      return kUnsupportedTestId;
-  }
+    _exit(-2);
 
   return 0;
 }

chrome_elf/third_party_dlls/main_unittest_exe.h

-// Copyright 2018 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_ELF_THIRD_PARTY_DLLS_MAIN_UNITTEST_EXE_H_
-#define CHROME_ELF_THIRD_PARTY_DLLS_MAIN_UNITTEST_EXE_H_
-
-namespace third_party_dlls {
-namespace main_unittest_exe {
-
-enum ExitCode {
-  kDllLoadSuccess = 0,
-  kDllLoadFailed = 1,
-  // Unexpected failures are negative ints:
-  kBadCommandLine = -1,
-  kThirdPartyAlreadyInitialized = -2,
-  kThirdPartyInitFailure = -3,
-  kMissingArgument = -4,
-  kBadBlacklistPath = -5,
-  kBadArgument = -6,
-  kUnsupportedTestId = -7,
-  kEmptyLog = -8,
-  kUnexpectedLog = -9,
-};
-
-enum TestId {
-  kTestOnlyInitialization = 1,
-  kTestSingleDllLoad = 2,
-};
-
-}  // namespace main_unittest_exe
-}  // namespace third_party_dlls
-
-#endif  // CHROME_ELF_THIRD_PARTY_DLLS_MAIN_UNITTEST_EXE_H_

chrome_elf/third_party_dlls/packed_list_file.cc

@@ -13,6 +13,7 @@
 #include <limits>
 
 #include "chrome/install_static/user_data_dir.h"
+#include "chrome_elf/sha1/sha1.h"
 #include "chrome_elf/third_party_dlls/packed_list_format.h"
 
 namespace third_party_dlls {
@@ -56,11 +57,6 @@ FileStatus ReadInArray(HANDLE file,
   if (!::ReadFile(file, &metadata, sizeof(PackedListMetadata), &bytes_read,
                   FALSE) ||
       bytes_read != sizeof(PackedListMetadata)) {
-    // If |bytes_read| is actually 0, then the file was empty.
-    // A decision was made to return kArraySizeZero here, instead of a
-    // full failure.
-    if (!bytes_read)
-      return FileStatus::kArraySizeZero;
     return FileStatus::kMetadataReadFail;
   }
 
@@ -168,6 +164,15 @@ FileStatus InitInternal() {
 // Public defines & functions
 //------------------------------------------------------------------------------
 
+std::string GetFingerprintHash(DWORD image_size, DWORD time_data_stamp) {
+  // Max hex 32-bit value is 8 characters long.  2*8+1.
+  char buffer[17] = {};
+  ::snprintf(buffer, sizeof(buffer), "%08lX%lx", time_data_stamp, image_size);
+
+  std::string shell(buffer);
+  return elf_sha1::SHA1HashString(shell);
+}
+
 bool IsModuleListed(const std::string& basename_hash,
                     const std::string& fingerprint_hash) {
   assert(g_initialized);

chrome_elf/third_party_dlls/packed_list_file.h

@@ -27,6 +27,10 @@ enum class FileStatus {
   COUNT
 };
 
+// Utility function that takes required PE fingerprint data and returns a SHA-1
+// fingerprint hash.  To be used with IsModuleListed() and logging APIs.
+std::string GetFingerprintHash(DWORD image_size, DWORD time_data_stamp);
+
 // 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,

chrome_elf/third_party_dlls/packed_list_file_unittest.cc

@@ -169,15 +169,13 @@ TEST_F(ThirdPartyFileTest, Success) {
   // Test matching.
   for (const auto& test_module : GetTestArray()) {
     fingerprint_hash =
-        GetFingerprintString(test_module.imagesize, test_module.timedatestamp);
-    fingerprint_hash = elf_sha1::SHA1HashString(fingerprint_hash);
+        GetFingerprintHash(test_module.imagesize, test_module.timedatestamp);
     name_hash = elf_sha1::SHA1HashString(test_module.basename);
     EXPECT_TRUE(IsModuleListed(name_hash, fingerprint_hash));
   }
 
   // Test a failure to match.
-  fingerprint_hash = GetFingerprintString(1337, 0x12345678);
-  fingerprint_hash = elf_sha1::SHA1HashString(fingerprint_hash);
+  fingerprint_hash = GetFingerprintHash(1337, 0x12345678);
   name_hash = elf_sha1::SHA1HashString("booya.dll");
   EXPECT_FALSE(IsModuleListed(name_hash, fingerprint_hash));
 }
@@ -186,8 +184,7 @@ TEST_F(ThirdPartyFileTest, Success) {
 TEST_F(ThirdPartyFileTest, NoFiles) {
   ASSERT_EQ(InitFromFile(), FileStatus::kSuccess);
 
-  std::string fingerprint_hash = GetFingerprintString(1337, 0x12345678);
-  fingerprint_hash = elf_sha1::SHA1HashString(fingerprint_hash);
+  std::string fingerprint_hash = GetFingerprintHash(1337, 0x12345678);
   std::string name_hash = elf_sha1::SHA1HashString("booya.dll");
   EXPECT_FALSE(IsModuleListed(name_hash, fingerprint_hash));
 }

chrome_elf/third_party_dlls/packed_list_format.cc

@@ -20,13 +20,4 @@ const wchar_t kFileSubdir[] =
 // Packed module data cache file.
 const wchar_t kBlFileName[] = L"\\bldata";
 
-std::string GetFingerprintString(uint32_t image_size,
-                                 uint32_t time_data_stamp) {
-  // Max hex 32-bit value is 8 characters long.  2*8+1.
-  char buffer[17] = {};
-  ::snprintf(buffer, sizeof(buffer), "%08X%x", time_data_stamp, image_size);
-
-  return std::string(buffer);
-}
-
 }  // namespace third_party_dlls

chrome_elf/third_party_dlls/packed_list_format.h

@@ -10,8 +10,6 @@
 #ifndef CHROME_ELF_THIRD_PARTY_DLLS_PACKED_LIST_FORMAT_H_
 #define CHROME_ELF_THIRD_PARTY_DLLS_PACKED_LIST_FORMAT_H_
 
-#include <string>
-
 #include <stdint.h>
 
 #include "chrome_elf/sha1/sha1.h"
@@ -54,22 +52,15 @@ struct PackedListMetadata {
 struct PackedListModule {
   // SHA1 of lowercase, UTF-8 basename (no path).
   uint8_t basename_hash[elf_sha1::kSHA1Length];
-  // Code ID. Get the formatted string via GetFingerprintString(), then SHA1.
+  // Code ID. This is equivalent to the string generated by formatting
+  // the FileHeader.TimeDateStamp and OptionalHeader.SizeOfImage with the
+  // formatting string %08X%x. Then SHA1 the string.
   uint8_t code_id_hash[elf_sha1::kSHA1Length];
   // A timestamp used for tracking "last attempted load".  Used to manage
   // lifetime of entries in the local caches.
   uint32_t time_date_stamp;
 };
 
-// Centralized utility function that takes required PE fingerprint data and
-// returns a formatted fingerprint string. The caller should SHA1 hash the
-// returned string.
-// - This string is generated by formatting the FileHeader.TimeDateStamp
-//   and OptionalHeader.SizeOfImage with the formatting string %08X%x.
-// - To be used for PackedListModule.code_id_hash, IsModuleListed() and logging
-//   APIs.
-std::string GetFingerprintString(uint32_t image_size, uint32_t time_data_stamp);
-
 // These structs are directly written to a storage type. Therefore the padding
 // should be consistent across compilations.
 static_assert(sizeof(PackedListMetadata) == 8,