Extract key metadata from the client certificate on Windows.

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 82b76ca9af8067a5d2b87bd940880beb47eff6da..30d1816ed236eec9d2ed124fdddd4e7c58846b6b 100644
--- a/net/ssl/openssl_platform_key_win.cc
+++ b/net/ssl/openssl_platform_key_win.cc
@@ -22,6 +22,7 @@
 #include <openssl/obj_mac.h>
 #include <openssl/rsa.h>
 #include <openssl/sha.h>
+#include <openssl/x509.h>
 
 #include "base/debug/debugger.h"
 #include "base/debug/stack_trace.h"
@@ -30,24 +31,20 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/win/windows_version.h"
+#include "crypto/openssl_util.h"
 #include "crypto/scoped_capi_types.h"
+#include "crypto/scoped_openssl_types.h"
 #include "crypto/wincrypt_shim.h"
 #include "net/base/net_errors.h"
 #include "net/cert/x509_certificate.h"
 #include "net/ssl/openssl_ssl_util.h"
+#include "net/ssl/scoped_openssl_types.h"

[davidben, 2015/03/18 21:42:32]

This will likely cause a minor merge conflict, but it's easily resolvable.

 
 namespace net {
 
 namespace {
 
 using NCryptFreeObjectFunc = SECURITY_STATUS(WINAPI*)(NCRYPT_HANDLE);
-using NCryptGetPropertyFunc =
-    SECURITY_STATUS(WINAPI*)(NCRYPT_HANDLE,  // hObject
-                             LPCWSTR,  // pszProperty
-                             PBYTE,  // pbOutput
-                             DWORD,  // cbOutput
-                             DWORD*,  // pcbResult
-                             DWORD);  // dwFlags
 using NCryptSignHashFunc =
     SECURITY_STATUS(WINAPI*)(NCRYPT_KEY_HANDLE,  // hKey
                              VOID*,  // pPaddingInfo
@@ -62,14 +59,11 @@ class CNGFunctions {
  public:
   CNGFunctions()
       : ncrypt_free_object_(nullptr),
-        ncrypt_get_property_(nullptr),
         ncrypt_sign_hash_(nullptr) {
     HMODULE ncrypt = GetModuleHandle(L"ncrypt.dll");
     if (ncrypt != nullptr) {
       ncrypt_free_object_ = reinterpret_cast<NCryptFreeObjectFunc>(
           GetProcAddress(ncrypt, "NCryptFreeObject"));
-      ncrypt_get_property_ = reinterpret_cast<NCryptGetPropertyFunc>(
-          GetProcAddress(ncrypt, "NCryptGetProperty"));
       ncrypt_sign_hash_ = reinterpret_cast<NCryptSignHashFunc>(
           GetProcAddress(ncrypt, "NCryptSignHash"));
     }
@@ -79,15 +73,10 @@ class CNGFunctions {
     return ncrypt_free_object_;
   }
 
-  NCryptGetPropertyFunc ncrypt_get_property() const {
-    return ncrypt_get_property_;
-  }
-
   NCryptSignHashFunc ncrypt_sign_hash() const { return ncrypt_sign_hash_; }
 
  private:
   NCryptFreeObjectFunc ncrypt_free_object_;
-  NCryptGetPropertyFunc ncrypt_get_property_;
   NCryptSignHashFunc ncrypt_sign_hash_;
 };
 
@@ -111,11 +100,11 @@ using ScopedCERT_KEY_CONTEXT =
 // KeyExData contains the data that is contained in the EX_DATA of the
 // RSA and ECDSA objects that are created to wrap Windows system keys.
 struct KeyExData {
-  KeyExData(ScopedCERT_KEY_CONTEXT key, DWORD key_length)
+  KeyExData(ScopedCERT_KEY_CONTEXT key, size_t key_length)
       : key(key.Pass()), key_length(key_length) {}
 
   ScopedCERT_KEY_CONTEXT key;
-  DWORD key_length;
+  size_t key_length;
 };
 
 // ExDataDup is called when one of the RSA or EC_KEY objects is
@@ -420,14 +409,7 @@ const KeyExData* EcKeyGetExData(const EC_KEY* ec_key) {
 
 size_t EcdsaMethodGroupOrderSize(const EC_KEY* ec_key) {
   const KeyExData* ex_data = EcKeyGetExData(ec_key);
-  // Windows doesn't distinguish the sizes of the curve's degree (which
-  // determines the size of a point on the curve) and the base point's order
-  // (which determines the size of a scalar). For P-256, P-384, and P-521, these
-  // two sizes are the same.
-  //
-  // See
-  // http://msdn.microsoft.com/en-us/library/windows/desktop/aa375520(v=vs.85).aspx
-  // which uses the same length for both.
+  // key_length is the size of the group order for EC keys.
   return (ex_data->key_length + 7) / 8;
 }
 
@@ -448,13 +430,14 @@ int EcdsaMethodSign(const uint8_t* digest,
     return 0;
   }
 
-  size_t degree = (ex_data->key_length + 7) / 8;
-  if (degree == 0) {

[davidben, 2015/03/18 21:42:32]

I'm not sure why I called it degree here. That was just wrong to begin with; the
values are taken modulo the order. (Though either way the sizes end up being the
same in practice.)

+  // 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(degree * 2);
+  std::vector<uint8_t> raw_sig(order_len * 2);
 
   DWORD signature_len;
   SECURITY_STATUS ncrypt_status = g_cng_functions.Get().ncrypt_sign_hash()(
@@ -471,8 +454,8 @@ int EcdsaMethodSign(const uint8_t* digest,
     OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
     return 0;
   }
-  sig->r = BN_bin2bn(&raw_sig[0], degree, nullptr);
-  sig->s = BN_bin2bn(&raw_sig[degree], degree, nullptr);
+  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) {
     OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
     return 0;
@@ -519,96 +502,33 @@ const ECDSA_METHOD win_ecdsa_method = {
     ECDSA_FLAG_OPAQUE,
 };
 
-// Determines the key type and length of |key|. The type is returned as an
-// OpenSSL EVP_PKEY type. The key length for RSA key is the size of the RSA
-// modulus in bits. For an ECDSA key, it is the number of bits to represent the
-// group order. It returns true on success and false on failure.
-bool GetKeyInfo(PCERT_KEY_CONTEXT key, int* out_type, DWORD* out_length) {
-  if (key->dwKeySpec == CERT_NCRYPT_KEY_SPEC) {
-    DWORD prop_len;
-    SECURITY_STATUS status = g_cng_functions.Get().ncrypt_get_property()(
-        key->hNCryptKey, NCRYPT_ALGORITHM_GROUP_PROPERTY, nullptr, 0, &prop_len,
-        0);
-    if (FAILED(status) || prop_len == 0 || prop_len % 2 != 0) {
-      LOG(ERROR) << "Could not query CNG key type: " << status;
-      return false;
-    }
-
-    std::vector<BYTE> prop_buf(prop_len);
-    status = g_cng_functions.Get().ncrypt_get_property()(
-        key->hNCryptKey, NCRYPT_ALGORITHM_GROUP_PROPERTY, &prop_buf[0],
-        prop_buf.size(), &prop_len, 0);
-    if (FAILED(status) || prop_len == 0 || prop_len % 2 != 0) {
-      LOG(ERROR) << "Could not query CNG key type: " << status;
-      return false;
-    }
-
-    int type;
-    const wchar_t* alg = reinterpret_cast<const wchar_t*>(&prop_buf[0]);
-    if (wcsncmp(NCRYPT_RSA_ALGORITHM_GROUP, alg, prop_len / 2) == 0) {
-      type = EVP_PKEY_RSA;
-    } else if (wcsncmp(NCRYPT_ECDSA_ALGORITHM_GROUP, alg, prop_len / 2) == 0 ||
-               wcsncmp(NCRYPT_ECDH_ALGORITHM_GROUP, alg, prop_len / 2) == 0) {
-      // Importing an ECDSA key via PKCS #12 seems to label it as ECDH rather
-      // than ECDSA, so also allow ECDH.
-      type = EVP_PKEY_EC;
-    } else {
-      LOG(ERROR) << "Unknown CNG key type: "
-                 << std::wstring(alg, wcsnlen(alg, prop_len / 2));
-      return false;
-    }
-
-    DWORD length;
-    prop_len;
-    status = g_cng_functions.Get().ncrypt_get_property()(
-        key->hNCryptKey, NCRYPT_LENGTH_PROPERTY,
-        reinterpret_cast<BYTE*>(&length), sizeof(DWORD), &prop_len, 0);
-    if (FAILED(status)) {
-      LOG(ERROR) << "Could not get CNG key length " << status;
-      return false;
-    }
-    DCHECK_EQ(sizeof(DWORD), prop_len);
-
-    *out_type = type;
-    *out_length = length;
-    return true;
-  }
-
-  crypto::ScopedHCRYPTKEY hcryptkey;
-  if (!CryptGetUserKey(key->hCryptProv, key->dwKeySpec, hcryptkey.receive())) {
-    PLOG(ERROR) << "Could not get CAPI key handle";
-    return false;
-  }
-
-  ALG_ID alg_id;
-  DWORD prop_len = sizeof(alg_id);
-  if (!CryptGetKeyParam(hcryptkey.get(), KP_ALGID,
-                        reinterpret_cast<BYTE*>(&alg_id), &prop_len, 0)) {
-    PLOG(ERROR) << "Could not query CAPI key type";
+// Determines the key type and length of |certificate|'s public key. The type is
+// returned as an OpenSSL EVP_PKEY type. The key length for RSA key is the size
+// of the RSA modulus in bits. For an ECDSA key, it is the number of bits to
+// represent the group order. It returns true on success and false on failure.
+bool GetKeyInfo(const X509Certificate* certificate,
+                int* out_type,
+                size_t* out_length) {
+  crypto::OpenSSLErrStackTracer tracker(FROM_HERE);
+
+  std::string der_encoded;
+  if (!X509Certificate::GetDEREncoded(certificate->os_cert_handle(),
+                                      &der_encoded))
     return false;
-  }
-
-  if (alg_id != CALG_RSA_SIGN && alg_id != CALG_RSA_KEYX) {
-    LOG(ERROR) << "Unknown CAPI key type: " << alg_id;
+  const uint8_t* bytes = reinterpret_cast<const uint8_t*>(der_encoded.data());
+  ScopedX509 x509(d2i_X509(NULL, &bytes, der_encoded.size()));
+  if (!x509)
     return false;
-  }
-
-  DWORD length;
-  prop_len = sizeof(DWORD);
-  if (!CryptGetKeyParam(hcryptkey.get(), KP_KEYLEN,
-                        reinterpret_cast<BYTE*>(&length), &prop_len, 0)) {
-    PLOG(ERROR) << "Could not get CAPI key length";
+  crypto::ScopedEVP_PKEY key(X509_get_pubkey(x509.get()));
+  if (!key)
     return false;
-  }
-  DCHECK_EQ(sizeof(DWORD), prop_len);
-
-  *out_type = EVP_PKEY_RSA;
-  *out_length = length;
+  *out_type = EVP_PKEY_id(key.get());
+  *out_length = EVP_PKEY_bits(key.get());
   return true;
 }
 
 crypto::ScopedEVP_PKEY CreateRSAWrapper(ScopedCERT_KEY_CONTEXT key,
-                                        DWORD key_length) {
+                                        size_t key_length) {
   crypto::ScopedRSA rsa(RSA_new_method(global_boringssl_engine.Get().engine()));
   if (!rsa)
     return nullptr;
@@ -623,7 +543,7 @@ crypto::ScopedEVP_PKEY CreateRSAWrapper(ScopedCERT_KEY_CONTEXT key,
 }
 
 crypto::ScopedEVP_PKEY CreateECDSAWrapper(ScopedCERT_KEY_CONTEXT key,
-                                          DWORD key_length) {
+                                          size_t key_length) {
   crypto::ScopedEC_KEY ec_key(
       EC_KEY_new_method(global_boringssl_engine.Get().engine()));
   if (!ec_key)
@@ -666,9 +586,12 @@ crypto::ScopedEVP_PKEY FetchClientCertPrivateKey(
   key->dwKeySpec = key_spec;
   key->hCryptProv = crypt_prov;
 
+  // Rather than query the private key for metadata, extract the public key from
+  // the certificate without using Windows APIs. CAPI and CNG do not
+  // consistently work depending on the system. See https://crbug.com/468345.
   int key_type;
-  DWORD key_length;
-  if (!GetKeyInfo(key.get(), &key_type, &key_length))
+  size_t key_length;
+  if (!GetKeyInfo(certificate, &key_type, &key_length))
     return nullptr;
 
   switch (key_type) {