Don't use RSAPrivateKey in NSS integration code.

chrome/browser/chromeos/login/easy_unlock/easy_unlock_tpm_key_manager.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 "chrome/browser/chromeos/login/easy_unlock/easy_unlock_tpm_key_manager.h"
 
 #include <cryptohi.h>
+#include <keyhi.h>
 
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/worker_pool.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
+#include "crypto/nss_key_util.h"
 #include "crypto/nss_util_internal.h"
-#include "crypto/rsa_private_key.h"
 #include "crypto/scoped_nss_types.h"
 
 namespace {
 
 // The modulus length for RSA keys used by easy sign-in.
 const int kKeyModulusLength = 2048;
 
 // Relays |GetSystemSlotOnIOThread| callback to |response_task_runner|.
 void RunCallbackOnThreadRunner(
     const scoped_refptr<base::SingleThreadTaskRunner>& response_task_runner,
@@ skipping 13 matching lines @@
 
   crypto::ScopedPK11Slot system_slot =
       crypto::GetSystemNSSKeySlot(callback_on_origin_thread);
   if (system_slot)
     callback_on_origin_thread.Run(system_slot.Pass());
 }
 
 // Checks if a private RSA key associated with |public_key| can be found in
 // |slot|.
 // Must be called on a worker thread.
-scoped_ptr<crypto::RSAPrivateKey> GetPrivateKeyOnWorkerThread(
+crypto::ScopedSECKEYPrivateKey GetPrivateKeyOnWorkerThread(
     PK11SlotInfo* slot,
     const std::string& public_key) {
   const uint8* public_key_uint8 =
       reinterpret_cast<const uint8*>(public_key.data());
   std::vector<uint8> public_key_vector(
       public_key_uint8, public_key_uint8 + public_key.size());
 
-  scoped_ptr<crypto::RSAPrivateKey> rsa_key(
-      crypto::RSAPrivateKey::FindFromPublicKeyInfo(public_key_vector));
-  if (!rsa_key || rsa_key->key()->pkcs11Slot != slot)
-    return scoped_ptr<crypto::RSAPrivateKey>();
+  crypto::ScopedSECKEYPrivateKey rsa_key(
+      crypto::FindNSSKeyFromPublicKeyInfoInSlot(public_key_vector, slot));
+  if (!rsa_key || SECKEY_GetPrivateKeyType(rsa_key.get()) != rsaKey)
+    return nullptr;
   return rsa_key.Pass();
 }
 
 // Signs |data| using a private key associated with |public_key| and stored in
 // |slot|. Once the data is signed, callback is run on |response_task_runner|.
 // In case of an error, the callback will be passed an empty string.
 void SignDataOnWorkerThread(
     crypto::ScopedPK11Slot slot,
     const std::string& public_key,
     const std::string& data,
     const scoped_refptr<base::SingleThreadTaskRunner>& response_task_runner,
     const base::Callback<void(const std::string&)>& callback) {
-  scoped_ptr<crypto::RSAPrivateKey> private_key(
+  crypto::ScopedSECKEYPrivateKey private_key(
       GetPrivateKeyOnWorkerThread(slot.get(), public_key));
   if (!private_key) {
     LOG(ERROR) << "Private key for signing data not found";
     response_task_runner->PostTask(FROM_HERE,
                                    base::Bind(callback, std::string()));
     return;
   }
 
   crypto::ScopedSECItem sign_result(SECITEM_AllocItem(NULL, NULL, 0));
   if (SEC_SignData(sign_result.get(),
                    reinterpret_cast<const unsigned char*>(data.data()),
-                   data.size(),
-                   private_key->key(),
+                   data.size(), private_key.get(),
                    SEC_OID_PKCS1_SHA256_WITH_RSA_ENCRYPTION) != SECSuccess) {
     LOG(ERROR) << "Failed to sign data";
     response_task_runner->PostTask(FROM_HERE,
                                    base::Bind(callback, std::string()));
     return;
   }
 
   std::string signature(reinterpret_cast<const char*>(sign_result->data),
                         sign_result->len);
   response_task_runner->PostTask(FROM_HERE, base::Bind(callback, signature));
 }
 
 // Creates a RSA key pair in |slot|. When done, it runs |callback| with the
 // created public key on |response_task_runner|.
 // If |public_key| is not empty, a key pair will be created only if the private
 // key associated with |public_key| does not exist in |slot|. Otherwise the
 // callback will be run with |public_key|.
 void CreateTpmKeyPairOnWorkerThread(
     crypto::ScopedPK11Slot slot,
     const std::string& public_key,
     const scoped_refptr<base::SingleThreadTaskRunner>& response_task_runner,
     const base::Callback<void(const std::string&)>& callback) {
   if (!public_key.empty() &&
       GetPrivateKeyOnWorkerThread(slot.get(), public_key)) {
     response_task_runner->PostTask(FROM_HERE, base::Bind(callback, public_key));
     return;
   }
 
-  scoped_ptr<crypto::RSAPrivateKey> rsa_key(
-      crypto::RSAPrivateKey::CreateSensitive(slot.get(), kKeyModulusLength));
-  if (!rsa_key) {
+  crypto::ScopedSECKEYPublicKey public_key_obj;
+  crypto::ScopedSECKEYPrivateKey private_key;

[Ryan Sleevi, 2015/04/27 19:11:26]

Why the different naming? public_key_obj vs private_key (no _obj); seems like
there should be consistency (and yes, I'm aware it's because public_key is used
on 117)

[davidben, 2015/04/27 19:53:44]

Done.

+  if (!crypto::GenerateRSAKeyPairNSS(slot.get(), kKeyModulusLength,
+                                     true /* permanent */, &public_key_obj,
+                                     &private_key)) {
     LOG(ERROR) << "Failed to create an RSA key.";
     response_task_runner->PostTask(FROM_HERE,
                                    base::Bind(callback, std::string()));
     return;
   }
 
-  std::vector<uint8> created_public_key;
-  if (!rsa_key->ExportPublicKey(&created_public_key)) {
+  crypto::ScopedSECItem created_public_key(

[Ryan Sleevi, 2015/04/27 19:11:26]

seems like this should be named like public_key_der ?

[davidben, 2015/04/27 19:53:44]

Done.

+      SECKEY_EncodeDERSubjectPublicKeyInfo(public_key_obj.get()));
+  if (!created_public_key) {
     LOG(ERROR) << "Failed to export public key.";
     response_task_runner->PostTask(FROM_HERE,
                                    base::Bind(callback, std::string()));
     return;
   }
 
   response_task_runner->PostTask(
-      FROM_HERE,
-      base::Bind(callback,
-                 std::string(created_public_key.begin(),
-                             created_public_key.end())));
+      FROM_HERE, base::Bind(callback, std::string(reinterpret_cast<const char*>(
+                                                      created_public_key->data),
+                                                  created_public_key->len)));
 }
 
 }  // namespace
 
 // static
 void EasyUnlockTpmKeyManager::RegisterLocalStatePrefs(
     PrefRegistrySimple* registry) {
   registry->RegisterDictionaryPref(prefs::kEasyUnlockLocalStateTpmKeys);
 }
 
@@ skipping 188 matching lines @@
   // If key creation failed, reset the state machine.
   create_tpm_key_state_ =
       public_key.empty() ? CREATE_TPM_KEY_NOT_STARTED : CREATE_TPM_KEY_DONE;
 }
 
 void EasyUnlockTpmKeyManager::OnDataSigned(
     const base::Callback<void(const std::string&)>& callback,
     const std::string& signature) {
   callback.Run(signature);
 }