Support SHA-256 on pre-Vista Windows clients

content/browser/browser_main_runner.cc

Index: content/browser/browser_main_runner.cc
diff --git a/content/browser/browser_main_runner.cc b/content/browser/browser_main_runner.cc
index a06f18af30dfe3d2a6c4342124c87e6507177daf..3faba5b3a0e71f75207f844806a8cec9d669e4e1 100644
--- a/content/browser/browser_main_runner.cc
+++ b/content/browser/browser_main_runner.cc
@@ -21,6 +21,8 @@
 
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
+#include "net/cert/sha256_legacy_support_win.h"
+#include "sandbox/win/src/sidestep/preamble_patcher.h"
 #include "ui/base/win/scoped_ole_initializer.h"
 #endif
 
@@ -28,6 +30,86 @@ bool g_exited_main_message_loop = false;
 
 namespace content {
 
+#if defined(OS_WIN)
+namespace {
+
+// Pointer to the original CryptVerifyCertificateSignatureEx function.
+net::sha256_interception::CryptVerifyCertificateSignatureExFunc
+    g_real_crypt_verify_signature_stub = NULL;
+
+// Stub function that is called whenever the Crypt32 function
+// CryptVerifyCertificateSignatureEx is called. It just defers to net to perform
+// the actual verification.
+BOOL WINAPI CryptVerifyCertificateSignatureExStub(
+    HCRYPTPROV_LEGACY provider,
+    DWORD encoding_type,
+    DWORD subject_type,
+    void* subject_data,
+    DWORD issuer_type,
+    void* issuer_data,
+    DWORD flags,
+    void* extra) {
+  return net::sha256_interception::CryptVerifyCertificateSignatureExHook(
+      g_real_crypt_verify_signature_stub, provider, encoding_type, subject_type,
+      subject_data, issuer_type, issuer_data, flags, extra);
+}
+
+// If necessary, install an interception
+void InstallSha256LegacyHooks() {
+#if defined(_WIN64)
+  // Interception on x64 is not supported.

[jam, 2014/09/12 04:13:08]

nit: move this to net::sha256_interception::IsNeeded() so that there's only one
place that knows when this is needed, instead of two?

[Ryan Sleevi, 2014/09/12 08:34:35]

The logic behind this is that the function would totally work for x64 too, but
sidestep can't handle it.

In other places in Chrome (e.g. NtDLL patching), we do implement x64 support,
but by manually constructing the sidestep.

Since its a limitation of the sandbox, not net, that's why I left it here.

+  return;
+#else
+  if (!net::sha256_interception::IsNeeded())
+    return;
+
+  net::sha256_interception::CryptVerifyCertificateSignatureExFunc
+      cert_verify_signature_ptr = reinterpret_cast<
+          net::sha256_interception::CryptVerifyCertificateSignatureExFunc>(
+              ::GetProcAddress(::GetModuleHandle(L"crypt32.dll"),
+                               "CryptVerifyCertificateSignatureEx"));
+  CHECK(cert_verify_signature_ptr);
+
+  DWORD old_protect = 0;
+  if (!::VirtualProtect(cert_verify_signature_ptr, 5, PAGE_EXECUTE_READWRITE,
+                        &old_protect)) {
+    return;
+  }
+
+  g_real_crypt_verify_signature_stub =
+      reinterpret_cast<
+          net::sha256_interception::CryptVerifyCertificateSignatureExFunc>(
+              VirtualAllocEx(::GetCurrentProcess(), NULL,
+                             sidestep::kMaxPreambleStubSize, MEM_COMMIT,
+                             PAGE_EXECUTE_READWRITE));
+  if (g_real_crypt_verify_signature_stub == NULL)
+    return;

[davidben, 2014/09/12 22:51:17]

If this return fires, we'll leave cert_verify_signature_ptr writable.

+
+  sidestep::SideStepError patch_result =
+      sidestep::PreamblePatcher::Patch(
+          cert_verify_signature_ptr, CryptVerifyCertificateSignatureExStub,
+          g_real_crypt_verify_signature_stub, sidestep::kMaxPreambleStubSize);
+  if (patch_result != sidestep::SIDESTEP_SUCCESS) {
+    CHECK(::VirtualFreeEx(::GetCurrentProcess(),
+                          g_real_crypt_verify_signature_stub, 0,
+                          MEM_RELEASE));
+    CHECK(::VirtualProtect(cert_verify_signature_ptr, 5, old_protect,
+                           &old_protect));
+    return;
+  }
+
+  DWORD dummy = 0;
+  CHECK(::VirtualProtect(cert_verify_signature_ptr, 5, old_protect, &dummy));
+  CHECK(::VirtualProtect(g_real_crypt_verify_signature_stub,
+                         sidestep::kMaxPreambleStubSize, old_protect,
+                         &old_protect));

[jam, 2014/09/12 04:13:08]

that's a lot of lines to patch the method. can you use
base::win::IATPatchFunction instead? it's much smaller. i.e. look at 
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/renderer/ch...

[Ryan Sleevi, 2014/09/12 08:34:35]

Nope. I worked with Carlos and Shrikant on this. Because of how and when its
called, it has to be sidestepped. In particular, we are intercepting an
in-module call (crypt32 CertGetCerificateChain to crypt32
CryptVerifyCertificateSignatureEx), so this has to be sidestepped, as the IAT is
entirely bypassed for that call.

+#endif  // _WIN64
+}
+
+}  // namespace
+
+#endif  // OS_WIN
+
 class BrowserMainRunnerImpl : public BrowserMainRunner {
  public:
   BrowserMainRunnerImpl()
@@ -64,6 +146,7 @@ class BrowserMainRunnerImpl : public BrowserMainRunner {
         // Win32 API here directly.
         ImmDisableTextFrameService(static_cast<DWORD>(-1));
       }
+      InstallSha256LegacyHooks();

[davidben, 2014/09/12 22:51:17]

Perhaps add tests the exercise the patched version? Like an end-to-end
CertVerifier test on XP that a SHA-256 chain is fine. Or perhaps even have tests
that unconditionally patch and run on all versions of Windows, though that might
be bothering with things we don't want to. (As far as I can tell, it should work
anyway on 32-bit?)

[Ryan Sleevi, 2014/09/23 21:59:58]

Considering that the XP bots we have are SP3, I don't think we'll actually end
up testing anything. Which is why I made the patched-function test
unconditional, but the patch-testing is no-opy.

Note that once https://codereview.chromium.org/515813002/ lands (which would be
after this, to be on the safeside), ALL of our SSL tests will exercise this
path, even if it won't be hit.

 #endif  // OS_WIN
 
       base::StatisticsRecorder::Initialize();