Don't implement RsaMethodSignRaw on Windows.

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 172 matching lines @@
 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;
 }
 
-// Signs |in| using |rsa| with PKCS #1 padding. If |hash_nid| is NID_md5_sha1,
-// |in| is a TLS MD5/SHA-1 concatenation and should be signed as-is. Otherwise
-// |in| is a standard hash function and should be prefixed with the
-// corresponding DigestInfo before signing. The signature is written to |out|
-// and its length written to |*out_len|. This function returns true on success
-// and false on failure.
-bool RsaSignPKCS1(const RSA* rsa,
-                  int hash_nid,
+int RsaMethodSign(int hash_nid,
                   const uint8_t* in,
-                  size_t in_len,
+                  unsigned in_len,
                   uint8_t* out,
-                  size_t max_out,
-                  size_t* out_len) {
+                  unsigned* out_len,
+                  const RSA* rsa) {
+  // TODO(davidben): Switch BoringSSL's sign hook to using size_t rather than
+  // unsigned.
   const KeyExData* ex_data = RsaGetExData(rsa);
   if (!ex_data) {
     NOTREACHED();
     OPENSSL_PUT_ERROR(RSA, RSA_sign, ERR_R_INTERNAL_ERROR);
-    return false;
+    return 0;
   }
 
   if (ex_data->key->dwKeySpec == CERT_NCRYPT_KEY_SPEC) {
     BCRYPT_PKCS1_PADDING_INFO rsa_padding_info;
     switch (hash_nid) {
       case NID_md5_sha1:
         rsa_padding_info.pszAlgId = nullptr;
         break;
       case NID_sha1:
         rsa_padding_info.pszAlgId = BCRYPT_SHA1_ALGORITHM;
         break;
       case NID_sha256:
         rsa_padding_info.pszAlgId = BCRYPT_SHA256_ALGORITHM;
         break;
       case NID_sha384:
         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 false;
+        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, max_out, &signature_len, BCRYPT_PAD_PKCS1);
+        in_len, out, RSA_size(rsa), &signature_len, BCRYPT_PAD_PKCS1);

[Ryan Sleevi, 2014/12/03 14:10:39]

Is this entirely safe? Does RSA_size handle any space needed for leading zeros
when encoding signatures? Just wanting to confirm that there's not a need to
allocate RSA_size(...)+1 for any BN encoding issues.

[davidben, 2014/12/03 16:01:38]

RSA_size is the number of bytes needed to represent the modulus, so that
includes leading zeros and all. Changing RSA_METHOD to still pass in a max_out
parameter to the size hook might not be a bad plan so we don't have to have the
bounds implicit...

The reason sign doesn't have a max_out while sign_raw does is because sign_raw
mirrors RSA_sign_raw which is new API and has a max_out parameter. sigh mirrors
RSA_sign which is existing API and doesn't. The caller is responsible for
ensuring there are at least RSA_size(rsa) bytes of space, so max_out is
implicitly always RSA_size(rsa).

https://boringssl.googlesource.com/boringssl/+/master/include/openssl/rsa.h#180

     if (FAILED(ncrypt_status) || signature_len == 0) {
       OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
-      return false;
+      return 0;
     }
     *out_len = signature_len;
-    return true;
+    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;
       break;
     case NID_sha256:
       hash_alg = CALG_SHA_256;
       break;
     case NID_sha384:
       hash_alg = CALG_SHA_384;
       break;
     case NID_sha512:
       hash_alg = CALG_SHA_512;
       break;
     default:
       OPENSSL_PUT_ERROR(RSA, RSA_sign, RSA_R_UNKNOWN_ALGORITHM_TYPE);
-      return false;
+      return 0;
   }
 
   HCRYPTHASH hash;
   if (!CryptCreateHash(ex_data->key->hCryptProv, hash_alg, 0, 0, &hash)) {
     PLOG(ERROR) << "CreateCreateHash failed";
     OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
-    return false;
+    return 0;
   }
   DWORD hash_len;
   DWORD arg_len = sizeof(hash_len);
   if (!CryptGetHashParam(hash, HP_HASHSIZE, reinterpret_cast<BYTE*>(&hash_len),
                          &arg_len, 0)) {
     PLOG(ERROR) << "CryptGetHashParam HP_HASHSIZE failed";
     OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
-    return false;
+    return 0;
   }
   if (hash_len != in_len) {
     OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
-    return false;
+    return 0;
   }
   if (!CryptSetHashParam(hash, HP_HASHVAL, const_cast<BYTE*>(in), 0)) {
     PLOG(ERROR) << "CryptSetHashParam HP_HASHVAL failed";
     OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
-    return false;
+    return 0;
   }
-  DWORD signature_len = max_out;
+  DWORD signature_len = RSA_size(rsa);
   if (!CryptSignHash(hash, ex_data->key->dwKeySpec, nullptr, 0, out,
                      &signature_len)) {
     PLOG(ERROR) << "CryptSignHash failed";
     OpenSSLPutNetError(FROM_HERE, ERR_SSL_CLIENT_AUTH_SIGNATURE_FAILED);
-    return false;
+    return 0;
   }
 
   /* CryptoAPI signs in little-endian, so reverse it. */
   std::reverse(out, out + signature_len);
   *out_len = signature_len;
-  return true;
-}
-
-int RsaMethodSign(int hash_nid,
-                  const uint8_t* in,
-                  unsigned in_len,
-                  uint8_t* out,
-                  unsigned* out_len,
-                  const RSA* rsa) {
-  // TOD(davidben): Switch BoringSSL's sign hook to using size_t rather than
-  // unsigned.
-  size_t len;
-  if (!RsaSignPKCS1(rsa, hash_nid, in, in_len, out, RSA_size(rsa), &len))
-    return 0;
-  *out_len = len;
   return 1;
 }
 
 int RsaMethodEncrypt(RSA* rsa,
                      size_t* out_len,
                      uint8_t* out,
                      size_t max_out,
                      const uint8_t* in,
                      size_t in_len,
                      int padding) {
   NOTIMPLEMENTED();
   OPENSSL_PUT_ERROR(RSA, encrypt, RSA_R_UNKNOWN_ALGORITHM_TYPE);
   return 0;
 }
 
 int RsaMethodSignRaw(RSA* rsa,
                      size_t* out_len,
                      uint8_t* out,
                      size_t max_out,
                      const uint8_t* in,
                      size_t in_len,
                      int padding) {
-  DCHECK_EQ(RSA_PKCS1_PADDING, padding);
-  if (padding != RSA_PKCS1_PADDING) {
-    OPENSSL_PUT_ERROR(RSA, sign_raw, RSA_R_UNKNOWN_PADDING_TYPE);
-    return 0;
-  }
-
-  // BoringSSL calls only sign_raw, not sign, in pre-TLS-1.2 MD5/SHA1
-  // signatures. This hook is implemented only for that case.
-  //
-  // TODO(davidben): Make client auth in BoringSSL call RSA_sign with
-  // NID_md5_sha1. https://crbug.com/437023
-  if (in_len != MD5_DIGEST_LENGTH + SHA_DIGEST_LENGTH) {
-    OPENSSL_PUT_ERROR(RSA, sign_raw, RSA_R_INVALID_MESSAGE_LENGTH);
-    return 0;
-  }
-  if (!RsaSignPKCS1(rsa, NID_md5_sha1, in, in_len, out, max_out, out_len))
-    return 0;
-  return 1;
+  NOTIMPLEMENTED();
+  OPENSSL_PUT_ERROR(RSA, encrypt, RSA_R_UNKNOWN_ALGORITHM_TYPE);
+  return 0;
 }
 
 int RsaMethodDecrypt(RSA* rsa,
                      size_t* out_len,
                      uint8_t* out,
                      size_t max_out,
                      const uint8_t* in,
                      size_t in_len,
                      int padding) {
   NOTIMPLEMENTED();
@@ skipping 332 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