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

net/ssl/openssl_platform_key_win.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/ssl/openssl_platform_key.h"
 
 #include <windows.h>
 #include <NCrypt.h>
 
 #include <string.h>
@@ skipping 149 matching lines @@
 
  private:
   const int rsa_index_;
   const int ec_key_index_;
   ENGINE* const engine_;
 };
 
 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) {
   return reinterpret_cast<const KeyExData*>(
       RSA_get_ex_data(rsa, global_boringssl_engine.Get().rsa_ex_index()));
 }
 
 size_t RsaMethodSize(const RSA* rsa) {
   const KeyExData* ex_data = RsaGetExData(rsa);
   return (ex_data->key_length + 7) / 8;
@@ skipping 34 matching lines @@
         rsa_padding_info.pszAlgId = BCRYPT_SHA384_ALGORITHM;
         break;
       case NID_sha512:
         rsa_padding_info.pszAlgId = BCRYPT_SHA512_ALGORITHM;
         break;
       default:
         OPENSSL_PUT_ERROR(RSA, RSA_sign, RSA_R_UNKNOWN_ALGORITHM_TYPE);
         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;
   }
 
   ALG_ID hash_alg;
   switch (hash_nid) {
     case NID_md5_sha1:
       hash_alg = CALG_SSL3_SHAMD5;
       break;
     case NID_sha1:
       hash_alg = CALG_SHA1;
@@ skipping 188 matching lines @@
 
   // An ECDSA signature is two integers, modulo the order of the group.
   size_t order_len = (ex_data->key_length + 7) / 8;
   if (order_len == 0) {
     NOTREACHED();
     OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
     return 0;
   }
   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;
+  }
 
   // Convert the RAW ECDSA signature to a DER-encoded ECDSA-Sig-Value.
   crypto::ScopedECDSA_SIG sig(ECDSA_SIG_new());
   if (!sig) {
     OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
     return 0;
   }
   sig->r = BN_bin2bn(&raw_sig[0], order_len, nullptr);
   sig->s = BN_bin2bn(&raw_sig[order_len], order_len, nullptr);
   if (!sig->r || !sig->s) {
@@ skipping 138 matching lines @@
     case EVP_PKEY_RSA:
       return CreateRSAWrapper(key.Pass(), key_length);
     case EVP_PKEY_EC:
       return CreateECDSAWrapper(key.Pass(), key_length);
     default:
       return nullptr;
   }
 }
 
 }  // namespace net