Work around a bug in the NCryptSignHash implementation of some smartcards.

net/ssl/openssl_platform_key_win.cc

Index: net/ssl/openssl_platform_key_win.cc
diff --git a/net/ssl/openssl_platform_key_win.cc b/net/ssl/openssl_platform_key_win.cc
index ead5957000800baab85ab59e331c254c4cc21071..fa5b6f5b1455ef626ad39cc4991cca4dd5f21fbf 100644
--- a/net/ssl/openssl_platform_key_win.cc
+++ b/net/ssl/openssl_platform_key_win.cc
@@ -167,6 +167,50 @@ class BoringSSLEngine {
 base::LazyInstance<BoringSSLEngine>::Leaky global_boringssl_engine =
     LAZY_INSTANCE_INITIALIZER;
 
+// Signs |in| with |key|, writing the output to |out| and the size to |out_len|.
+// Although the buffer is preallocated, this calls NCryptSignHash twice. Some
+// smartcards are buggy and assume the two-call pattern. See
+// https://crbug.com/470204. Returns true on success and false on error.
+bool DoNCryptSignHash(NCRYPT_KEY_HANDLE key,
+                      void* padding,
+                      const uint8_t* in,
+                      size_t in_len,
+                      uint8_t* out,
+                      size_t max_out,
+                      size_t* out_len,

[Ryan Sleevi, 2015/03/27 07:00:38]

Why is this a size_t? Why not consistent type and just use DWORD?

[davidben, 2015/03/27 18:59:23]

I figured I'd push the native types -> Windows boundary down a level. Switched
these all to Windows types at this level.

+                      DWORD flags) {
+  // Determine the output length.
+  DWORD signature_len;
+  SECURITY_STATUS ncrypt_status = g_cng_functions.Get().ncrypt_sign_hash()(
+      key, padding, const_cast<PBYTE>(in), in_len, nullptr, 0, &signature_len,
+      flags);
+  if (FAILED(ncrypt_status)) {
+    LOG(ERROR) << "NCryptSignHash failed: " << ncrypt_status;
+    return false;
+  }
+  // Check |max_out| externally rather than trust the smartcard.
+  if (signature_len == 0 || signature_len > max_out) {
+    LOG(ERROR) << "Bad signature length.";
+    return false;
+  }
+  // It is important that |signature_len| already be initialized with the
+  // correct size. Some smartcards are buggy and do not write to it on the
+  // second call.
+  ncrypt_status = g_cng_functions.Get().ncrypt_sign_hash()(
+      key, padding, const_cast<PBYTE>(in), in_len, out, signature_len,
+      &signature_len, flags);
+  if (FAILED(ncrypt_status)) {
+    LOG(ERROR) << "NCryptSignHash failed: " << ncrypt_status;
+    return false;
+  }
+  if (signature_len == 0) {
+    LOG(ERROR) << "Bad signature length.";
+    return false;
+  }
+  *out_len = signature_len;
+  return true;
+}
+
 // Custom RSA_METHOD that uses the platform APIs for signing.
 
 const KeyExData* RsaGetExData(const RSA* rsa) {
@@ -221,15 +265,14 @@ int RsaMethodSign(int hash_nid,
         return 0;
     }
 
-    DWORD signature_len;
-    SECURITY_STATUS ncrypt_status = g_cng_functions.Get().ncrypt_sign_hash()(
-        ex_data->key->hNCryptKey, &rsa_padding_info, const_cast<PBYTE>(in),
-        in_len, out, RSA_size(rsa), &signature_len, BCRYPT_PAD_PKCS1);
-    if (FAILED(ncrypt_status) || signature_len == 0) {
+    size_t out_len_size_t;

[Ryan Sleevi, 2015/03/27 07:00:37]

Reverse Hungarian Notation? :D

why not signature_len?

whatever it is, suffix size_t = embiggening my emsaddening.

[davidben, 2015/03/27 18:59:23]

Done.

+    if (!DoNCryptSignHash(ex_data->key->hNCryptKey, &rsa_padding_info, in,
+                          in_len, out, RSA_size(rsa), &out_len_size_t,
+                          BCRYPT_PAD_PKCS1)) {
       OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
       return 0;
     }
-    *out_len = signature_len;
+    *out_len = static_cast<unsigned>(out_len_size_t);
     return 1;
   }
 
@@ -438,11 +481,14 @@ int EcdsaMethodSign(const uint8_t* digest,
   }
   std::vector<uint8_t> raw_sig(order_len * 2);
 
-  DWORD signature_len;
-  SECURITY_STATUS ncrypt_status = g_cng_functions.Get().ncrypt_sign_hash()(
-      ex_data->key->hNCryptKey, nullptr, const_cast<PBYTE>(digest), digest_len,
-      &raw_sig[0], raw_sig.size(), &signature_len, 0);
-  if (FAILED(ncrypt_status) || signature_len != raw_sig.size()) {
+  size_t raw_sig_len;
+  if (!DoNCryptSignHash(ex_data->key->hNCryptKey, nullptr, digest, digest_len,
+                        &raw_sig[0], raw_sig.size(), &raw_sig_len, 0)) {
+    OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
+    return 0;
+  }
+  if (raw_sig_len != raw_sig.size()) {
+    LOG(ERROR) << "Bad signature length";
     OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
     return 0;
   }