[Windows Sandbox] MITIGATION_EXTENSION_POINT_DISABLE support for children.

sandbox/win/src/process_mitigations_test.cc

Index: sandbox/win/src/process_mitigations_test.cc
diff --git a/sandbox/win/src/process_mitigations_test.cc b/sandbox/win/src/process_mitigations_test.cc
index b7c608eafb2380339d9f3e9325e29faaa0953f35..88882487b23040b8ff683f78e588c10bfbb3e40b 100644
--- a/sandbox/win/src/process_mitigations_test.cc
+++ b/sandbox/win/src/process_mitigations_test.cc
@@ -2,20 +2,29 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "sandbox/win/src/process_mitigations.h"
+
+#include <psapi.h>
+
+#include <chrono>
+#include <thread>

[dcheng, 2016/04/03 23:30:01]

Btw, I hate to the bearer of bad news but both <chrono> and <thread> are
currently banned: http://chromium-cpp.appspot.com/#library-blacklist

Sorry =/

[Will Harris, 2016/04/04 00:16:41]

yes I noticed that too, suggested using base::PlatformThread::Sleep :)

what are you doing on a Sunday reviewing CLs?!

[penny, 2016/04/11 22:11:58]

Done.

+
+#include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/memory/free_deleter.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
 #include "base/process/launch.h"
-#include "base/strings/stringprintf.h"
+#include "base/win/registry.h"
 #include "base/win/scoped_handle.h"
+#include "base/win/startup_information.h"
+#include "base/win/win_util.h"
 #include "base/win/windows_version.h"
 #include "sandbox/win/src/nt_internals.h"
-#include "sandbox/win/src/process_mitigations.h"
 #include "sandbox/win/src/sandbox.h"
 #include "sandbox/win/src/sandbox_factory.h"
 #include "sandbox/win/src/target_services.h"
-#include "sandbox/win/src/win_utils.h"
 #include "sandbox/win/tests/common/controller.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -33,6 +42,10 @@ GetProcessMitigationPolicyFunction get_process_mitigation_policy;
 typedef decltype(AddFontMemResourceEx)* AddFontMemResourceExFunction;
 typedef decltype(RemoveFontMemResourceEx)* RemoveFontMemResourceExFunction;
 
+// WinProc event name for synchronization.
+const wchar_t* winproc_event = L"ChromeExtensionTestEvent";
+DWORD event_max_wait_ms = 3 * 1000;
+
 #if !defined(_WIN64)
 bool CheckWin8DepPolicy() {
   PROCESS_MITIGATION_DEP_POLICY policy = {};
@@ -76,7 +89,7 @@ bool CheckWin8Win32CallPolicy() {
   return policy.DisallowWin32kSystemCalls;
 }
 
-bool CheckWin8DllExtensionPolicy() {
+bool CheckWin8ExtensionPointPolicy() {
   PROCESS_MITIGATION_EXTENSION_POINT_DISABLE_POLICY policy = {};
   if (!get_process_mitigation_policy(::GetCurrentProcess(),
                                      ProcessExtensionPointDisablePolicy,
@@ -106,6 +119,245 @@ bool CheckWin10ImageLoadNoRemotePolicy() {
   return policy.NoRemoteImages;
 }
 
+// Spawn our Windows process (with or without mitigation enabled).

[Will Harris, 2016/04/04 00:15:08]

style nit, feel free to ignore, but prefer not to use "we" and "our" in chromium
code but active sense. e.g.

Spawn Windows process (with or without mitigation enabled).

but I can't find a citation for this (I just remember being told it) so up to
you to decide to ignore me or not.

[penny, 2016/04/11 22:11:59]

Done.

+void SpawnWinProc(PROCESS_INFORMATION* pi, bool success_test, HANDLE* event) {
+  base::win::StartupInformation startup_info;
+  DWORD creation_flags = 0;
+
+  if (!success_test) {
+    DWORD64 flags =
+        PROCESS_CREATION_MITIGATION_POLICY_EXTENSION_POINT_DISABLE_ALWAYS_ON;
+    // This test only runs on >= Win8, so I don't have to worry about
+    // illegal 64-bit flags on 32-bit <= Win7.
+    size_t flags_size = sizeof(flags);
+
+    EXPECT_TRUE(startup_info.InitializeProcThreadAttributeList(1));
+    EXPECT_TRUE(startup_info.UpdateProcThreadAttribute(
+        PROC_THREAD_ATTRIBUTE_MITIGATION_POLICY, &flags, flags_size));
+    creation_flags = EXTENDED_STARTUPINFO_PRESENT;
+  }
+
+  std::wstring cmd_line = L"sbox_integration_test_win_proc.exe ";
+  scoped_ptr<wchar_t, base::FreeDeleter> cmd_writeable(
+      ::wcsdup(cmd_line.c_str()));
+
+  EXPECT_TRUE(::CreateProcessW(NULL, cmd_writeable.get(), NULL, NULL, FALSE,
+                               creation_flags, NULL, NULL,
+                               startup_info.startup_info(), pi));
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(*event, event_max_wait_ms));
+
+  return;
+}
+
+//------------------------------------------------------------------------------
+// 1. Spawn our Windows process (with or without mitigation enabled).
+// 2. Load our hook Dll locally.
+// 3. Start the hook (for our WinProc or globally).
+// 4. Send a keystroke event.
+// 5. Ask the hook Dll if it received a hook callback.
+// 6. Cleanup the hooking.
+// 7. Signal our Windows process to shutdown.
+//
+// Do NOT use any ASSERTs in this function.  Cleanup required.
+//------------------------------------------------------------------------------
+
+typedef BOOL (*WasHookCalledFunction)();
+
+void TestWin8ExtensionPointHookWrapper(bool is_success_test, bool global_hook) {
+  HMODULE dll = NULL;

[Will Harris, 2016/04/04 00:15:09]

chromium style is to declare variables just before they are used. yes, it's
counter to how I always did it before, but that's life.

[penny, 2016/04/11 22:11:58]

Done.

+  HHOOK hook = NULL;
+  HOOKPROC hook_proc = NULL;
+  WasHookCalledFunction WasHookCalled = NULL;
+  PROCESS_INFORMATION proc_info = {};
+
+  // 1. Spawn WinProc.
+  HANDLE event = ::CreateEventW(NULL, FALSE, FALSE, winproc_event);
+  EXPECT_TRUE(event != NULL && event != INVALID_HANDLE_VALUE);
+  SpawnWinProc(&proc_info, is_success_test, &event);
+
+  // 2. Load the hook DLL.
+  dll = ::LoadLibraryW(L"sbox_integration_test_hook_dll.dll");

[Will Harris, 2016/04/04 00:15:08]

prefer ScopedNativeLibrary here rather than LoadLibraryW

[penny, 2016/04/11 22:11:58]

Done.

+  EXPECT_TRUE(dll);
+
+  hook_proc = reinterpret_cast<HOOKPROC>(::GetProcAddress(dll, "HookProc"));
+  WasHookCalled = reinterpret_cast<WasHookCalledFunction>(
+      ::GetProcAddress(dll, "WasHookCalled"));

[Will Harris, 2016/04/04 00:15:08]

you could #define WasHookCalled in a .h file for your DLL then use decltype
here... but could be overkill.

[penny, 2016/04/11 22:11:59]

Acknowledged.  I've created a new common.h file with literal strings and these
function definitions.  If it was only one function, it wouldn't be worth it, but
with other shared names, etc.

+  EXPECT_TRUE(hook_proc && WasHookCalled);
+
+  // 3. Try installing the hook (either on our remote target thread,
+  //    or globally).
+  DWORD target = 0;
+  if (!global_hook)
+    target = proc_info.dwThreadId;
+  hook = ::SetWindowsHookExW(WH_KEYBOARD, hook_proc, dll, target);
+  EXPECT_TRUE(hook);
+
+  // 4. Inject a keyboard event.
+
+  // Note: that PostThreadMessage and SendMessage APIs will not deliver
+  // a keystroke in such a way that triggers a "legitimate" hook.
+  // Have to use targetless SendInput or keybd_event.  The latter is
+  // less code and easier to work with.
+  keybd_event(VkKeyScan(L'A'), 0, 0, 0);
+  std::this_thread::sleep_for(std::chrono::milliseconds(500));

[Will Harris, 2016/04/04 00:15:09]

base::PlatformThread::Sleep() and base::TimeDelta

[penny, 2016/04/11 22:11:58]

Done.

+  keybd_event(VkKeyScan(L'A'), 0, KEYEVENTF_KEYUP, 0);
+
+  // Give it a chance...
+  std::this_thread::sleep_for(std::chrono::seconds(1));
+
+  // 5. Did our hook get hit?  Did we expect it to?
+  if (global_hook)
+    EXPECT_EQ((is_success_test ? TRUE : FALSE), WasHookCalled());

[Will Harris, 2016/04/04 00:15:08]

true and false not TRUE and FALSE

[penny, 2016/04/11 22:11:59]

Done.

The function WasHookCalled() was actually defined with BOOL return type.  But
I've now changed the hook DLL function to return bool.

+  else
+    // ***IMPORTANT: when targeting a specific thread id, the
+    // PROCESS_CREATION_MITIGATION_POLICY_EXTENSION_POINT_DISABLE
+    // mitigation does NOT disable the hook API.  It ONLY
+    // stops global hooks from running in your process.  Hence,
+    // we expect the hook to have hit (TRUE) even in the "failure"
+    // case for a non-global/targetted hook.
+    EXPECT_EQ((is_success_test ? TRUE : TRUE), WasHookCalled());
+
+  // 6. Rip it all down.
+  if (hook)
+    EXPECT_TRUE(::UnhookWindowsHookEx(hook));
+  if (dll)
+    EXPECT_TRUE(::FreeLibrary(dll));

[Will Harris, 2016/04/04 00:15:08]

this can go if you are using ScopedNativeLibrary

[penny, 2016/04/11 22:11:59]

Done.

+
+  // 7. Trigger shutdown of WinProc.
+  if (::PostThreadMessageW(proc_info.dwThreadId, WM_QUIT, 0, 0)) {
+    // Note: The combination/perfect-storm of a Global Hook, in a
+    // WinProc that has the EXTENSION_POINT_DISABLE mitigation ON, and the
+    // use of the SendInput or keybd_event API to inject a keystroke,
+    // results in the target becoming unresponsive.  If any one of these states
+    // are changed, the problem does not occur.
+    // This means the WM_QUIT message is not handled and the call to
+    // WaitForSingleObject times out.  Therefore not checking the return val.
+    ::WaitForSingleObject(event, event_max_wait_ms);
+    EXPECT_TRUE(::CloseHandle(event));
+  } else {
+    // Make sure we don't leave a stray.
+    ::TerminateProcess(proc_info.hProcess, 0);
+    EXPECT_TRUE(false);
+  }
+  EXPECT_TRUE(::CloseHandle(proc_info.hThread));
+  EXPECT_TRUE(::CloseHandle(proc_info.hProcess));
+}
+
+//------------------------------------------------------------------------------
+// 1. Set up our AppInit Dll in registry settings. (Enable)
+// 2. Spawn our Windows process (with or without mitigation enabled).
+// 3. Check if our AppInit Dll is loaded in our Windows process or not.
+// 4. Signal our Windows process to shutdown.
+// 5. Restore original reg settings.
+//
+// Do NOT use any ASSERTs in this function.  Cleanup required.
+//------------------------------------------------------------------------------
+void TestWin8ExtensionPointAppInitWrapper(bool is_success_test) {
+  PROCESS_INFORMATION proc_info = {};
+  wchar_t* hook_dll = L"sbox_integration_test_hook_dll.dll";

[Will Harris, 2016/04/04 00:15:08]

const

[penny, 2016/04/11 22:11:58]

Done.

+
+  // Get path of current executable.
+  wchar_t path[MAX_PATH];

[Will Harris, 2016/04/04 00:15:08]

not sure exactly what is happening here, you are getting the current module name
then changing it to the dll path?

Can you explain what is being done here - I suspect it could be done using
base::FilePath operations rather than having to manually parse the paths using
wcsrchr and wcsncat and messing with the raw strings?

[penny, 2016/04/11 22:11:59]

I actually don't think FilePath helps at all.  I've added some more comments
though, and am using a vector instead of a pointer that requires deleting later.

I need to get the absolute path to the currently running module, as that's where
the right version of the appinit DLL will be.  Then I need to convert that full
path into short-name format for registry compat.

[Will Harris, 2016/04/12 19:39:12]

I understand you need to call GetShortPathName as this isn't provided by
FilePath, but I'm still uncomfortable with you looking for separatators and
doing a wcsncat. This is exactly what base::FilePath is there for, can you just
place the result of GetModuleFileName into a base::FilePath then use DirName()
and Append() then pass result into GetShortPathName?

[penny, 2016/04/13 22:52:27]

Done.

I'm not really convinced this is much nicer, but not emotional about it - so
happy to leave it like this.  I'm slightly biased against FilePath, as I find
the APIs un-intuitive and generally clumsy when trying to interact with windows
system apis.  For some reason it takes me way too long to figure out which
FilePath apis to use, and what they actually return given the under-the-hood
"thinking" it does (not entirely well imho).  Then the whole "const" thing.  Oh
well.  I've done as you asked.  :)

OMG it happened again.  Append gets me again.  You have to use the returned
FilePath, it does not just append to the existing object.  <sigh>  Finally got
it working again.

+  EXPECT_TRUE(::GetModuleFileNameW(NULL, path, MAX_PATH));
+  // We just want the directory.
+  wchar_t* last_separator = ::wcsrchr(path, L'\\');
+  EXPECT_TRUE(last_separator);
+  last_separator++;
+  *last_separator = L'\0';
+  ::wcsncat(path, hook_dll, (MAX_PATH - ::wcslen(path)));
+  // Prep short-name path to our hook dll, for registry.
+  DWORD length = ::GetShortPathNameW(path, NULL, 0);
+  wchar_t* short_name = new wchar_t[length];

[Will Harris, 2016/04/04 00:15:08]

use a scoped_ptr with a base::FreeDeleter rather than a new

or, since this is a test you could just declare this as a stack buffer

wchar_t short_name[MAX_PATH];

but we typically try and avoid managing lifetime of memory manually.

[penny, 2016/04/11 22:11:59]

Done.

+  EXPECT_TRUE(::GetShortPathNameW(path, short_name, length));
+
+  // 1. Reg setup.
+  wchar_t* app_init_reg_path =
+      L"SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Windows";
+  wchar_t* dlls_value_name = L"AppInit_DLLs";
+  wchar_t* enabled_value_name = L"LoadAppInit_DLLs";
+  wchar_t* signing_value_name = L"RequireSignedAppInit_DLLs";
+  std::wstring orig_dlls;
+  std::wstring new_dlls;
+  DWORD orig_enabled_value = 0;
+  DWORD orig_signing_value = 0;
+  base::win::RegKey app_init_key(HKEY_LOCAL_MACHINE, app_init_reg_path,
+                                 KEY_QUERY_VALUE | KEY_SET_VALUE);
+  // Backup the existing settings.
+  EXPECT_TRUE(app_init_key.Valid());
+  EXPECT_TRUE(app_init_key.HasValue(dlls_value_name) &&
+              app_init_key.HasValue(enabled_value_name));
+  EXPECT_EQ(ERROR_SUCCESS, app_init_key.ReadValue(dlls_value_name, &orig_dlls));
+  EXPECT_EQ(ERROR_SUCCESS,
+            app_init_key.ReadValueDW(enabled_value_name, &orig_enabled_value));
+  if (app_init_key.HasValue(signing_value_name))
+    EXPECT_EQ(ERROR_SUCCESS, app_init_key.ReadValueDW(signing_value_name,
+                                                      &orig_signing_value));
+
+  // Set the new settings we want (obviously requires local admin privileges).
+  new_dlls = orig_dlls;
+  if (0 != orig_dlls.compare(L""))
+    new_dlls.append(L",");
+  new_dlls.append(short_name);
+  delete[] short_name;
+
+  EXPECT_EQ(ERROR_SUCCESS,
+            app_init_key.WriteValue(dlls_value_name, new_dlls.c_str()));
+  if (app_init_key.HasValue(signing_value_name))
+    EXPECT_EQ(ERROR_SUCCESS, app_init_key.WriteValue(signing_value_name,
+                                                     static_cast<DWORD>(0)));
+  EXPECT_EQ(ERROR_SUCCESS,
+            app_init_key.WriteValue(enabled_value_name, static_cast<DWORD>(1)));

[Will Harris, 2016/04/04 00:15:09]

can you just use 1U and avoid the cast here and above?

[penny, 2016/04/11 22:11:59]

So the EXPECT_EQ macro (and the function call) is actually why the cast is
needed.  If I don't cast the '0' value to DWORD, it complains that it can't tell
which overload of the WriteValue() function is being used.  Doesn't matter
whether the 'U' is added or not.

The '1' value can definitely be not cast, but I'd rather have it match the call
above for consistency!

+
+  // 2. Spawn WinProc.
+  HANDLE event = ::CreateEventW(NULL, FALSE, FALSE, winproc_event);

[Will Harris, 2016/04/04 00:15:09]

ScopedHandle

[penny, 2016/04/11 22:11:58]

Done.

+  EXPECT_TRUE(event != NULL && event != INVALID_HANDLE_VALUE);
+  SpawnWinProc(&proc_info, is_success_test, &event);
+
+  // 3. Check loaded modules in WinProc to see if AppInit dll is loaded.
+  std::vector<HMODULE>(modules);
+  EXPECT_TRUE(
+      base::win::GetLoadedModulesSnapshot(proc_info.hProcess, &modules));
+  BOOL dll_loaded = FALSE;
+
+  for (auto module : modules) {
+    wchar_t name[MAX_PATH];
+    EXPECT_TRUE(
+        ::GetModuleFileNameExW(proc_info.hProcess, module, name, MAX_PATH));
+
+    // Compare to our dll name!
+    if (::wcsstr(name, hook_dll)) {
+      // Found it.
+      dll_loaded = TRUE;
+      break;
+    }
+  }
+
+  // Did we pass the test as expected?
+  EXPECT_EQ((is_success_test ? TRUE : FALSE), dll_loaded);
+
+  // 4. Trigger shutdown of WinProc.
+  if (::PostThreadMessageW(proc_info.dwThreadId, WM_QUIT, 0, 0)) {
+    ::WaitForSingleObject(event, event_max_wait_ms);
+    EXPECT_TRUE(::CloseHandle(event));
+  } else {
+    // Make sure we don't leave a stray.
+    ::TerminateProcess(proc_info.hProcess, 0);
+    EXPECT_TRUE(false);
+  }
+  EXPECT_TRUE(::CloseHandle(proc_info.hThread));
+  EXPECT_TRUE(::CloseHandle(proc_info.hProcess));
+
+  // 5. Reg Restore
+  EXPECT_EQ(ERROR_SUCCESS,
+            app_init_key.WriteValue(enabled_value_name, orig_enabled_value));
+  if (app_init_key.HasValue(signing_value_name))
+    EXPECT_EQ(ERROR_SUCCESS,
+              app_init_key.WriteValue(signing_value_name, orig_signing_value));
+  EXPECT_EQ(ERROR_SUCCESS,
+            app_init_key.WriteValue(dlls_value_name, orig_dlls.c_str()));
+  app_init_key.Close();
+}
+
 void TestWin10ImageLoadRemote(bool is_success_test) {
   // ***Insert your manual testing share UNC path here!
   // E.g.: \\\\hostname\\sharename\\calc.exe
@@ -236,7 +488,6 @@ SBOX_TESTS_COMMAND int TestChildProcess(int argc, wchar_t** argv) {
 //------------------------------------------------------------------------------
 // Win8 Checks:
 // MITIGATION_DEP(_NO_ATL_THUNK)
-// MITIGATION_EXTENSION_DLL_DISABLE
 // MITIGATION_RELOCATE_IMAGE(_REQUIRED) - ASLR, release only
 // MITIGATION_STRICT_HANDLE_CHECKS
 // >= Win8
@@ -263,9 +514,6 @@ SBOX_TESTS_COMMAND int CheckWin8(int argc, wchar_t **argv) {
   if (!CheckWin8StrictHandlePolicy())
     return SBOX_TEST_THIRD_ERROR;
 
-  if (!CheckWin8DllExtensionPolicy())
-    return SBOX_TEST_FIFTH_ERROR;
-
   return SBOX_TEST_SUCCEEDED;
 }
 
@@ -276,9 +524,8 @@ TEST(ProcessMitigationsTest, CheckWin8) {
   TestRunner runner;
   sandbox::TargetPolicy* policy = runner.GetPolicy();
 
-  sandbox::MitigationFlags mitigations = MITIGATION_DEP |
-                                         MITIGATION_DEP_NO_ATL_THUNK |
-                                         MITIGATION_EXTENSION_DLL_DISABLE;
+  sandbox::MitigationFlags mitigations =
+      MITIGATION_DEP | MITIGATION_DEP_NO_ATL_THUNK;
 #if defined(NDEBUG)  // ASLR cannot be forced in debug builds.
   mitigations |= MITIGATION_RELOCATE_IMAGE |
                  MITIGATION_RELOCATE_IMAGE_REQUIRED;
@@ -413,6 +660,162 @@ TEST(ProcessMitigationsTest, CheckWin8Win32KLockDownSuccess) {
 }
 
 //------------------------------------------------------------------------------
+// Disable extension points (MITIGATION_EXTENSION_POINT_DISABLE).
+// >= Win8
+//------------------------------------------------------------------------------
+
+wchar_t* extension_point_mutex = L"ChromeExtensionTestMutex";
+DWORD mutex_max_wait_ms = 10 * 1000;

[Will Harris, 2016/04/04 00:15:08]

TestTimeouts::action_timeout() for defaults

[penny, 2016/04/11 22:11:58]

So TestTimeouts::action_timeout() returns a base::TimeDelta.  But I'm
interacting with ::WaitForSingleObject(), which takes a DWORD number of
milliseconds to wait for the object.

I'm not sure I can use our TimeDelta objects when interacting with a system API.
 I just need to pass in a number of milliseconds.  (?)

+
+SBOX_TESTS_COMMAND int CheckWin8ExtensionPointSetting(int argc,
+                                                      wchar_t** argv) {
+  get_process_mitigation_policy =
+      reinterpret_cast<GetProcessMitigationPolicyFunction>(::GetProcAddress(
+          ::GetModuleHandleW(L"kernel32.dll"), "GetProcessMitigationPolicy"));
+  if (!get_process_mitigation_policy)
+    return SBOX_TEST_NOT_FOUND;
+
+  if (!CheckWin8ExtensionPointPolicy())
+    return SBOX_TEST_FIRST_ERROR;
+  return SBOX_TEST_SUCCEEDED;
+}
+
+// This test validates that setting the MITIGATION_EXTENSION_POINT_DISABLE
+// mitigation enables the setting on a process.
+TEST(ProcessMitigationsTest, CheckWin8ExtensionPointPolicySuccess) {
+  if (base::win::GetVersion() < base::win::VERSION_WIN8)
+    return;
+
+  TestRunner runner;
+  sandbox::TargetPolicy* policy = runner.GetPolicy();
+
+  EXPECT_EQ(policy->SetProcessMitigations(MITIGATION_EXTENSION_POINT_DISABLE),
+            SBOX_ALL_OK);
+  EXPECT_EQ(SBOX_TEST_SUCCEEDED,
+            runner.RunTest(L"CheckWin8ExtensionPointSetting"));
+}
+
+// This test validates that we CAN add a "legitimate" global hook on the
+// sandboxed
+// proc/thread if the MITIGATION_EXTENSION_POINT_DISABLE mitigation is not set.
+//
+// MANUAL testing only.
+TEST(ProcessMitigationsTest,
+     DISABLED_CheckWin8ExtensionPoint_GlobalHook_Success) {
+  if (base::win::GetVersion() < base::win::VERSION_WIN8)
+    return;
+
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, extension_point_mutex);
+  EXPECT_TRUE(mutex != NULL && mutex != INVALID_HANDLE_VALUE);
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, mutex_max_wait_ms));
+
+  // (is_success_test, global_hook)
+  TestWin8ExtensionPointHookWrapper(true, true);
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
+}
+
+// This test validates that setting the MITIGATION_EXTENSION_POINT_DISABLE
+// mitigation prevents a global hook in our WinProc.
+//
+// MANUAL testing only.
+TEST(ProcessMitigationsTest,
+     DISABLED_CheckWin8ExtensionPoint_GlobalHook_Failure) {
+  if (base::win::GetVersion() < base::win::VERSION_WIN8)
+    return;
+
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, extension_point_mutex);
+  EXPECT_TRUE(mutex != NULL && mutex != INVALID_HANDLE_VALUE);
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, mutex_max_wait_ms));
+
+  // (is_success_test, global_hook)
+  TestWin8ExtensionPointHookWrapper(false, true);
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
+}
+
+// This test validates that we CAN add a "legitimate" hook on the sandboxed
+// proc/thread if the MITIGATION_EXTENSION_POINT_DISABLE mitigation is not set.
+//
+// MANUAL testing only.
+TEST(ProcessMitigationsTest, DISABLED_CheckWin8ExtensionPoint_Hook_Success) {
+  if (base::win::GetVersion() < base::win::VERSION_WIN8)
+    return;
+
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, extension_point_mutex);
+  EXPECT_TRUE(mutex != NULL && mutex != INVALID_HANDLE_VALUE);
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, mutex_max_wait_ms));
+
+  // (is_success_test, global_hook)
+  TestWin8ExtensionPointHookWrapper(true, false);
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
+}
+
+// *** Important: MITIGATION_EXTENSION_POINT_DISABLE does NOT prevent
+// hooks targetted at a specific thread id.  It only prevents
+// global hooks.  So this test does NOT actually expect the hook
+// to fail (see TestWin8ExtensionPointHookWrapper function) even
+// with the mitigation on.
+//
+// MANUAL testing only.
+TEST(ProcessMitigationsTest, DISABLED_CheckWin8ExtensionPoint_Hook_Failure) {
+  if (base::win::GetVersion() < base::win::VERSION_WIN8)
+    return;
+
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, extension_point_mutex);
+  EXPECT_TRUE(mutex != NULL && mutex != INVALID_HANDLE_VALUE);
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, mutex_max_wait_ms));
+
+  // (is_success_test, global_hook)
+  TestWin8ExtensionPointHookWrapper(false, false);
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
+}
+
+// This test validates that we CAN add an AppInit Dll to a target
+// WinProc if the MITIGATION_EXTENSION_POINT_DISABLE mitigation is not set.
+//
+// MANUAL testing only.
+// Must run this test as admin/elevated.
+TEST(ProcessMitigationsTest, DISABLED_CheckWin8ExtensionPoint_AppInit_Success) {
+  if (base::win::GetVersion() < base::win::VERSION_WIN8)
+    return;
+
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, extension_point_mutex);
+  EXPECT_TRUE(mutex != NULL && mutex != INVALID_HANDLE_VALUE);
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, mutex_max_wait_ms));
+
+  TestWin8ExtensionPointAppInitWrapper(true);
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
+}
+
+// This test validates that setting the MITIGATION_EXTENSION_POINT_DISABLE
+// mitigation prevents the loading of any AppInit Dll into our WinProc.
+//
+// MANUAL testing only.
+// Must run this test as admin/elevated.
+TEST(ProcessMitigationsTest, DISABLED_CheckWin8ExtensionPoint_AppInit_Failure) {
+  if (base::win::GetVersion() < base::win::VERSION_WIN8)
+    return;
+
+  HANDLE mutex = ::CreateMutexW(NULL, FALSE, extension_point_mutex);
+  EXPECT_TRUE(mutex != NULL && mutex != INVALID_HANDLE_VALUE);
+  EXPECT_EQ(WAIT_OBJECT_0, ::WaitForSingleObject(mutex, mutex_max_wait_ms));
+
+  TestWin8ExtensionPointAppInitWrapper(false);
+
+  EXPECT_TRUE(::ReleaseMutex(mutex));
+  EXPECT_TRUE(::CloseHandle(mutex));
+}
+
+//------------------------------------------------------------------------------
 // Disable non-system font loads (MITIGATION_NONSYSTEM_FONT_DISABLE)
 // >= Win10
 //------------------------------------------------------------------------------
@@ -577,7 +980,7 @@ TEST(ProcessMitigationsTest, CheckWin10ImageLoadNoRemotePolicySuccess) {
 // a remote UNC device, if the MITIGATION_IMAGE_LOAD_NO_REMOTE
 // mitigation is NOT set.
 //
-// DISABLED for automated testing bots.  Enable for manual testing.
+// MANUAL testing only.
 TEST(ProcessMitigationsTest, DISABLED_CheckWin10ImageLoadNoRemoteSuccess) {
   if (base::win::GetVersion() < base::win::VERSION_WIN10_TH2)
     return;
@@ -589,7 +992,7 @@ TEST(ProcessMitigationsTest, DISABLED_CheckWin10ImageLoadNoRemoteSuccess) {
 // mitigation prevents creating a new process from a remote
 // UNC device.
 //
-// DISABLED for automated testing bots.  Enable for manual testing.
+// MANUAL testing only.
 TEST(ProcessMitigationsTest, DISABLED_CheckWin10ImageLoadNoRemoteFailure) {
   if (base::win::GetVersion() < base::win::VERSION_WIN10_TH2)
     return;