[Chrome ELF] Early browser security support.

chrome_elf/chrome_elf_util.cc

Index: chrome_elf/chrome_elf_util.cc
diff --git a/chrome_elf/chrome_elf_util.cc b/chrome_elf/chrome_elf_util.cc
index a3fd727e9d2ebdf54d795bf3bd3b76e0d97bca86..b029c25c89057808ff794d65e10e350265374e24 100644
--- a/chrome_elf/chrome_elf_util.cc
+++ b/chrome_elf/chrome_elf_util.cc
@@ -5,7 +5,6 @@
 #include "chrome_elf/chrome_elf_util.h"
 
 #include <assert.h>
-#include <windows.h>
 #include <stddef.h>
 
 #include "base/macros.h"
@@ -222,3 +221,26 @@ bool IsNonBrowserProcess() {
   assert(g_process_type != ProcessType::UNINITIALIZED);
   return g_process_type == ProcessType::NON_BROWSER_PROCESS;
 }
+
+typedef decltype(SetProcessMitigationPolicy)* SetProcessMitigationPolicyFunc;
+
+void EarlyBrowserSecurity() {
+  // This function is called from within DllMain.
+  // Don't do anything naughty while we have the loader lock.
+

[Will Harris, 2016/01/30 01:19:05]

nit: remove line

[penny, 2016/02/01 21:31:52]

Done.

+  if (::IsWindows8OrGreater()) {

[Will Harris, 2016/01/30 01:19:05]

What does this function pull in? we would normally use base::win::Version even
though MS says not to. Any particular reason to use this instead?

[jschuh, 2016/02/01 20:28:07]

ELF doesn't depend on base. So, this is an easy way to version check without
depending on anything outside of kernel32. IIRC, it's implemented almost
entirely with macros in the header.

[penny, 2016/02/01 21:31:52]

Yes indeed.  I like these APIs.  Supported back to Win2k.  They're in kernel32,
are mostly macros, don't pull anything in, and are safe for using with loader
lock.  Since I don't have base in chrome_elf, went this route to keep the code
clean and simple.

+    SetProcessMitigationPolicyFunc set_process_mitigation_policy =
+        reinterpret_cast<SetProcessMitigationPolicyFunc>(::GetProcAddress(
+            ::GetModuleHandleW(L"kernel32.dll"), "SetProcessMitigationPolicy"));
+    if (set_process_mitigation_policy) {
+      // Disable extension DLLs in this process.
+      // (Legacy hooking.)
+      PROCESS_MITIGATION_EXTENSION_POINT_DISABLE_POLICY policy = {};
+      policy.DisableExtensionPoints = true;
+
+      set_process_mitigation_policy(ProcessExtensionPointDisablePolicy, &policy,
+                                    sizeof(policy));
+    }

[Will Harris, 2016/01/30 01:19:05]

my vote would be a DCHECK here, this would trip the bots.  Maybe a CHECK...?

[jschuh, 2016/02/01 20:28:07]

I don't think you can introduce that dependency either, but a DebugBreak() might
work.

[grt (UTC plus 2), 2016/02/01 20:43:06]

nit: prefer the __debugbreak() compiler intrinsic
(https://msdn.microsoft.com/en-us/library/f408b4et.aspx) over the function call,
IMO.

[penny, 2016/02/01 21:31:52]

Done.  I'm in the habit of using the intrinsic as well - added if the call to
set mitigation happens to fail.

**Are we ok with having a __debugbreak() - essentially an int 3 - inside
DllMain?  And in both debug and release?  Or only debug?

[grt (UTC plus 2), 2016/02/02 01:12:29]

As a rule of thumb: crash the process in a release build if Chrome should not
run at all in this scenario because it's such a gross security violation. I
haven't grokked this CL, but bear in mind that a crash during startup is
completely incomprehensible and undiagnosable for users. Is Chrome connected to
the crash service at this point?

+  }
+  return;
+}