Finch Blacklist is now added to the Hardcoded blacklist.

The finch blacklist is collected from finch in chrome_elf_init_win.cc,
stored in a registry, and added to the blacklist in blacklist.cc.



BUG=

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275086 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 188 insertions, 26 deletions

diff --git a/chrome/browser/chrome_elf_init_unittest_win.cc b/chrome/browser/chrome_elf_init_unittest_win.cc
index 441f736..f98ee08 100644
--- a/chrome/browser/chrome_elf_init_unittest_win.cc
+++ b/chrome/browser/chrome_elf_init_unittest_win.cc
@@ -14,9 +14,14 @@
 #include "chrome/common/chrome_version_info.h"
 #include "chrome_elf/chrome_elf_constants.h"
 #include "components/variations/entropy_provider.h"
+#include "components/variations/variations_associated_data.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "version.h"  // NOLINT
 
+namespace {
+
+const char kBrowserBlacklistTrialEnabledGroupName[] = "Enabled";
+
 class ChromeBlacklistTrialTest : public testing::Test {
  protected:
   ChromeBlacklistTrialTest() {}
@@ -76,7 +81,6 @@ TEST_F(ChromeBlacklistTrialTest, DefaultRun) {
   ASSERT_EQ(version, GetBlacklistVersion());
 }
 
-
 // Ensure that the blacklist is disabled for any users in the
 // "BlacklistDisabled" finch group.
 TEST_F(ChromeBlacklistTrialTest, BlacklistDisabledRun) {
@@ -172,3 +176,43 @@ TEST_F(ChromeBlacklistTrialTest, VersionChanged) {
   base::string16 expected_version(base::UTF8ToUTF16(version_info.Version()));
   ASSERT_EQ(expected_version, GetBlacklistVersion());
 }
+
+TEST_F(ChromeBlacklistTrialTest, AddFinchBlacklistToRegistry) {
+  // Create the field trial with the blacklist enabled group.
+  base::FieldTrialList field_trial_list(
+      new metrics::SHA1EntropyProvider("test"));
+
+  scoped_refptr<base::FieldTrial> trial(base::FieldTrialList::CreateFieldTrial(
+      kBrowserBlacklistTrialName, kBrowserBlacklistTrialEnabledGroupName));
+
+  // Set up the trial with the desired parameters.
+  std::map<std::string, std::string> desired_params;
+  desired_params["TestDllName1"] = "TestDll1.dll";
+  desired_params["TestDllName2"] = "TestDll2.dll";
+
+  chrome_variations::AssociateVariationParams(
+      kBrowserBlacklistTrialName,
+      kBrowserBlacklistTrialEnabledGroupName,
+      desired_params);
+
+  // This should add the dlls in those parameters to the registry.
+  AddFinchBlacklistToRegistry();
+
+  // Check that all the values in desired_params were added to the registry.
+  base::win::RegKey finch_blacklist_registry_key(
+      HKEY_CURRENT_USER,
+      blacklist::kRegistryFinchListPath,
+      KEY_QUERY_VALUE | KEY_SET_VALUE);
+
+  ASSERT_EQ(desired_params.size(),
+            finch_blacklist_registry_key.GetValueCount());
+
+  for (std::map<std::string, std::string>::iterator it = desired_params.begin();
+       it != desired_params.end();
+       ++it) {
+    std::wstring name = base::UTF8ToWide(it->first);
+    ASSERT_TRUE(finch_blacklist_registry_key.HasValue(name.c_str()));
+  }
+}
+
+}  // namespace
diff --git a/chrome/browser/chrome_elf_init_win.cc b/chrome/browser/chrome_elf_init_win.cc
index 042f3d1..e09d2c9 100644
--- a/chrome/browser/chrome_elf_init_win.cc
+++ b/chrome/browser/chrome_elf_init_win.cc
@@ -12,6 +12,7 @@
 #include "chrome_elf/blacklist/blacklist.h"
 #include "chrome_elf/chrome_elf_constants.h"
 #include "chrome_elf/dll_hash/dll_hash.h"
+#include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
 #include "version.h"  // NOLINT
 
@@ -87,6 +88,7 @@ void InitializeChromeElf() {
     base::win::RegKey blacklist_registry_key(HKEY_CURRENT_USER);
     blacklist_registry_key.DeleteKey(blacklist::kRegistryBeaconPath);
   } else {
+    AddFinchBlacklistToRegistry();
     BrowserBlacklistBeaconSetup();
   }
 
@@ -103,6 +105,27 @@ void InitializeChromeElf() {
       base::TimeDelta::FromSeconds(kBlacklistReportingDelaySec));
 }
 
+void AddFinchBlacklistToRegistry() {
+  base::win::RegKey finch_blacklist_registry_key(
+      HKEY_CURRENT_USER, blacklist::kRegistryFinchListPath, KEY_SET_VALUE);
+
+  // No point in trying to continue if the registry key isn't valid.
+  if (!finch_blacklist_registry_key.Valid())
+    return;
+
+  std::map<std::string, std::string> params;
+  chrome_variations::GetVariationParams(kBrowserBlacklistTrialName, &params);
+
+  for (std::map<std::string, std::string>::iterator it = params.begin();
+       it != params.end();
+       ++it) {
+    std::wstring name = base::UTF8ToWide(it->first);
+    std::wstring val = base::UTF8ToWide(it->second);
+
+    finch_blacklist_registry_key.WriteValue(name.c_str(), val.c_str());
+  }
+}
+
 void BrowserBlacklistBeaconSetup() {
   base::win::RegKey blacklist_registry_key(HKEY_CURRENT_USER,
                                            blacklist::kRegistryBeaconPath,
diff --git a/chrome/browser/chrome_elf_init_win.h b/chrome/browser/chrome_elf_init_win.h
index d878709..84d3da4 100644
--- a/chrome/browser/chrome_elf_init_win.h
+++ b/chrome/browser/chrome_elf_init_win.h
@@ -13,6 +13,9 @@ extern const char kBrowserBlacklistTrialDisabledGroupName[];
 // only affect future runs since Chrome Elf is already setup by this point).
 void InitializeChromeElf();
 
+// Add the blacklist from the finch configs in the registry.
+void AddFinchBlacklistToRegistry();
+
 // Set the required state for an enabled browser blacklist.
 void BrowserBlacklistBeaconSetup();
 
diff --git a/chrome_elf/blacklist/blacklist.cc b/chrome_elf/blacklist/blacklist.cc
index a168c9a..c42f682 100644
--- a/chrome_elf/blacklist/blacklist.cc
+++ b/chrome_elf/blacklist/blacklist.cc
@@ -7,6 +7,8 @@
 #include <assert.h>
 #include <string.h>
 
+#include <vector>
+
 #include "base/basictypes.h"
 #include "chrome_elf/blacklist/blacklist_interceptions.h"
 #include "chrome_elf/chrome_elf_constants.h"
@@ -342,7 +344,49 @@ bool Initialize(bool force) {
 
   RecordSuccessfulThunkSetup(&key);
 
+  AddDllsFromRegistryToBlacklist();
+
   return NT_SUCCESS(ret) && page_executable;
 }
 
+bool AddDllsFromRegistryToBlacklist() {
+  HKEY key = NULL;
+  LONG result = ::RegOpenKeyEx(HKEY_CURRENT_USER,
+                               kRegistryFinchListPath,
+                               0,
+                               KEY_QUERY_VALUE | KEY_SET_VALUE,
+                               &key);
+
+  if (result != ERROR_SUCCESS)
+    return false;
+
+  // We add dlls from the registry to the blacklist, and then clear registry.
+  DWORD value_len;
+  DWORD name_len = MAX_PATH;
+  std::vector<wchar_t> name_buffer(name_len);
+  for (int i = 0; result == ERROR_SUCCESS; ++i) {
+    name_len = MAX_PATH;
+    value_len = 0;
+    result = ::RegEnumValue(
+        key, i, &name_buffer[0], &name_len, NULL, NULL, NULL, &value_len);
+    name_len = name_len + 1;
+    value_len = value_len + 1;
+    std::vector<wchar_t> value_buffer(value_len);
+    result = ::RegEnumValue(key, i, &name_buffer[0], &name_len, NULL, NULL,
+                            reinterpret_cast<BYTE*>(&value_buffer[0]),
+                            &value_len);
+    value_buffer[value_len - 1] = L'\0';
+
+    if (result == ERROR_SUCCESS) {
+      AddDllToBlacklist(&value_buffer[0]);
+    }
+  }
+
+  // Delete the finch registry key to clear the values.
+  result = ::RegDeleteKey(key, L"");
+
+  ::RegCloseKey(key);
+  return result == ERROR_SUCCESS;
+}
+
 }  // namespace blacklist
diff --git a/chrome_elf/blacklist/blacklist.h b/chrome_elf/blacklist/blacklist.h
index 9877c29..7b8d7a6 100644
--- a/chrome_elf/blacklist/blacklist.h
+++ b/chrome_elf/blacklist/blacklist.h
@@ -60,6 +60,10 @@ extern "C" bool RemoveDllFromBlacklist(const wchar_t* dll_name);
 // is only exposed in tests (and should stay that way).
 extern "C" void SuccessfullyBlocked(const wchar_t** blocked_dlls, int* size);
 
+// Add the dlls, originally passed in through finch, from the registry to the
+// blacklist so that they will be blocked identically to those hard coded in.
+extern "C" bool AddDllsFromRegistryToBlacklist();
+
 // Record that the dll at the given index was blocked.
 void BlockedDll(size_t blocked_index);
 
diff --git a/chrome_elf/blacklist/test/blacklist_test.cc b/chrome_elf/blacklist/test/blacklist_test.cc
index 05aeb14..1f91956 100644
--- a/chrome_elf/blacklist/test/blacklist_test.cc
+++ b/chrome_elf/blacklist/test/blacklist_test.cc
@@ -32,6 +32,7 @@ extern "C" {
 // When modifying the blacklist in the test process, use the exported test dll
 // functions on the test blacklist dll, not the ones linked into the test
 // executable itself.
+__declspec(dllimport) bool TestDll_AddDllsFromRegistryToBlacklist();
 __declspec(dllimport) bool TestDll_AddDllToBlacklist(const wchar_t* dll_name);
 __declspec(dllimport) bool TestDll_IsBlacklistInitialized();
 __declspec(dllimport) bool TestDll_RemoveDllFromBlacklist(
@@ -54,6 +55,13 @@ class BlacklistTest : public testing::Test {
   }
 };
 
+namespace {
+
+struct TestData {
+  const wchar_t* dll_name;
+  const wchar_t* dll_beacon;
+} test_data[] = {{kTestDllName2, kDll2Beacon}, {kTestDllName3, kDll3Beacon}};
+
 TEST_F(BlacklistTest, Beacon) {
   registry_util::RegistryOverrideManager override_manager;
   override_manager.OverrideRegistry(HKEY_CURRENT_USER, L"beacon_test");
@@ -138,34 +146,14 @@ TEST_F(BlacklistTest, SuccessfullyBlocked) {
   }
 }
 
-TEST_F(BlacklistTest, LoadBlacklistedLibrary) {
+void CheckBlacklistedDllsNotLoaded() {
   base::FilePath current_dir;
   ASSERT_TRUE(PathService::Get(base::DIR_EXE, &current_dir));
 
-  // Ensure that the blacklist is loaded.
-  ASSERT_TRUE(TestDll_IsBlacklistInitialized());
-
-  // Test that an un-blacklisted DLL can load correctly.
-  base::ScopedNativeLibrary dll1(current_dir.Append(kTestDllName1));
-  EXPECT_TRUE(dll1.is_valid());
-  dll1.Reset(NULL);
-
-  int num_blocked_dlls = 0;
-  TestDll_SuccessfullyBlocked(NULL, &num_blocked_dlls);
-  EXPECT_EQ(0, num_blocked_dlls);
-
-  struct TestData {
-    const wchar_t* dll_name;
-    const wchar_t* dll_beacon;
-  } test_data[] = {
-    { kTestDllName2, kDll2Beacon },
-    { kTestDllName3, kDll3Beacon }
-  };
-  for (int i = 0 ; i < arraysize(test_data); ++i) {
-    // Add the DLL to the blacklist, ensure that it is not loaded both by
-    // inspecting the handle returned by LoadLibrary and by looking for an
-    // environment variable that is set when the DLL's entry point is called.
-    EXPECT_TRUE(TestDll_AddDllToBlacklist(test_data[i].dll_name));
+  for (int i = 0; i < arraysize(test_data); ++i) {
+    // Ensure that the dll has not been loaded both by inspecting the handle
+    // returned by LoadLibrary and by looking for an environment variable that
+    // is set when the DLL's entry point is called.
     base::ScopedNativeLibrary dll_blacklisted(
         current_dir.Append(test_data[i].dll_name));
     EXPECT_FALSE(dll_blacklisted.is_valid());
@@ -203,7 +191,57 @@ TEST_F(BlacklistTest, LoadBlacklistedLibrary) {
 
     // The blocked dll was removed, so we shouldn't get anything returned
     // here.
+    int num_blocked_dlls = 0;
     TestDll_SuccessfullyBlocked(NULL, &num_blocked_dlls);
     EXPECT_EQ(0, num_blocked_dlls);
   }
 }
+
+TEST_F(BlacklistTest, LoadBlacklistedLibrary) {
+  base::FilePath current_dir;
+  ASSERT_TRUE(PathService::Get(base::DIR_EXE, &current_dir));
+
+  // Ensure that the blacklist is loaded.
+  ASSERT_TRUE(TestDll_IsBlacklistInitialized());
+
+  // Test that an un-blacklisted DLL can load correctly.
+  base::ScopedNativeLibrary dll1(current_dir.Append(kTestDllName1));
+  EXPECT_TRUE(dll1.is_valid());
+  dll1.Reset(NULL);
+
+  int num_blocked_dlls = 0;
+  TestDll_SuccessfullyBlocked(NULL, &num_blocked_dlls);
+  EXPECT_EQ(0, num_blocked_dlls);
+
+  // Add all DLLs to the blacklist then check they are blocked.
+  for (int i = 0; i < arraysize(test_data); ++i) {
+    EXPECT_TRUE(TestDll_AddDllToBlacklist(test_data[i].dll_name));
+  }
+  CheckBlacklistedDllsNotLoaded();
+}
+
+TEST_F(BlacklistTest, AddDllsFromRegistryToBlacklist) {
+  // Ensure that the blacklist is loaded.
+  ASSERT_TRUE(TestDll_IsBlacklistInitialized());
+
+  // Delete the finch registry key to clear its values.
+  base::win::RegKey key(HKEY_CURRENT_USER,
+                        blacklist::kRegistryFinchListPath,
+                        KEY_QUERY_VALUE | KEY_SET_VALUE);
+  key.DeleteKey(L"");
+
+  // Add the test dlls to the registry (with their name as both key and value).
+  base::win::RegKey finch_blacklist_registry_key(
+      HKEY_CURRENT_USER,
+      blacklist::kRegistryFinchListPath,
+      KEY_QUERY_VALUE | KEY_SET_VALUE);
+  for (int i = 0; i < arraysize(test_data); ++i) {
+    finch_blacklist_registry_key.WriteValue(test_data[i].dll_name,
+                                            test_data[i].dll_name);
+  }
+
+  EXPECT_TRUE(TestDll_AddDllsFromRegistryToBlacklist());
+  CheckBlacklistedDllsNotLoaded();
+}
+
+}  // namespace
diff --git a/chrome_elf/blacklist/test/blacklist_test_main_dll.def b/chrome_elf/blacklist/test/blacklist_test_main_dll.def
index 920dc6c..77cc500 100644
--- a/chrome_elf/blacklist/test/blacklist_test_main_dll.def
+++ b/chrome_elf/blacklist/test/blacklist_test_main_dll.def
@@ -5,6 +5,7 @@
 LIBRARY  "blacklist_test_main_dll.dll"
 
 EXPORTS
+  TestDll_AddDllsFromRegistryToBlacklist=AddDllsFromRegistryToBlacklist
   TestDll_AddDllToBlacklist=AddDllToBlacklist
   TestDll_IsBlacklistInitialized=IsBlacklistInitialized
   TestDll_SuccessfullyBlocked=SuccessfullyBlocked
diff --git a/chrome_elf/chrome_elf_constants.cc b/chrome_elf/chrome_elf_constants.cc
index 8879da7..1dbc749 100644
--- a/chrome_elf/chrome_elf_constants.cc
+++ b/chrome_elf/chrome_elf_constants.cc
@@ -17,6 +17,8 @@ const wchar_t kUserDataDirName[] = L"User Data";
 namespace blacklist {
 
 const wchar_t kRegistryBeaconPath[] = L"SOFTWARE\\Google\\Chrome\\BLBeacon";
+const wchar_t kRegistryFinchListPath[] =
+    L"SOFTWARE\\Google\\Chrome\\BLFinchList";
 const wchar_t kBeaconVersion[] = L"version";
 const wchar_t kBeaconState[] = L"state";
 
diff --git a/chrome_elf/chrome_elf_constants.h b/chrome_elf/chrome_elf_constants.h
index 2941815..4ff45b2 100644
--- a/chrome_elf/chrome_elf_constants.h
+++ b/chrome_elf/chrome_elf_constants.h
@@ -19,6 +19,9 @@ namespace blacklist {
 // The registry path of the blacklist beacon.
 extern const wchar_t kRegistryBeaconPath[];
 
+// The registry path of the finch blacklist dlls.
+extern const wchar_t kRegistryFinchListPath[];
+
 // The properties for the blacklist beacon.
 extern const wchar_t kBeaconVersion[];
 extern const wchar_t kBeaconState[];