Reland "Don't use RSAPrivateKey in NSS integration code."

chrome/browser/chromeos/ownership/owner_settings_service_chromeos.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/ownership/owner_settings_service_chromeos.h"
 
+#include <keyhi.h>
+
 #include <algorithm>
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/prefs/pref_service.h"
 #include "base/threading/thread_checker.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/device_settings_provider.h"
 #include "chrome/browser/chromeos/settings/session_manager_operation.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/tpm/tpm_token_loader.h"
 #include "components/ownership/owner_key_util.h"
 #include "components/user_manager/user.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/common/content_switches.h"
+#include "crypto/nss_key_util.h"
 #include "crypto/nss_util.h"
 #include "crypto/nss_util_internal.h"
-#include "crypto/rsa_private_key.h"
 #include "crypto/scoped_nss_types.h"
 #include "crypto/signature_creator.h"
 
 namespace em = enterprise_management;
 
 using content::BrowserThread;
 using ownership::OwnerKeyUtil;
 using ownership::PrivateKey;
 using ownership::PublicKey;
 
@@ skipping 20 matching lines @@
     const std::string& username_hash,
     const base::Callback<void(const scoped_refptr<PublicKey>& public_key,
                               const scoped_refptr<PrivateKey>& private_key)>&
         callback) {
   crypto::EnsureNSSInit();
   crypto::ScopedPK11Slot public_slot =
       crypto::GetPublicSlotForChromeOSUser(username_hash);
   crypto::ScopedPK11Slot private_slot = crypto::GetPrivateSlotForChromeOSUser(
       username_hash, base::Callback<void(crypto::ScopedPK11Slot)>());
 
-  // If private slot is already available, this will check it. If not,
-  // we'll get called again later when the TPM Token is ready, and the
-  // slot will be available then.
-  scoped_refptr<PrivateKey> private_key(
-      new PrivateKey(owner_key_util->FindPrivateKeyInSlot(public_key->data(),
-                                                          private_slot.get())));
-  if (!private_key->key()) {
-    private_key = new PrivateKey(owner_key_util->FindPrivateKeyInSlot(
-        public_key->data(), public_slot.get()));
+  crypto::RSAPrivateKey* private_key = nullptr;
+  if (private_slot) {
+    // If private slot is already available, this will check it. If not, we'll
+    // get called again later when the TPM Token is ready, and the slot will be
+    // available then.
+    private_key = owner_key_util->FindPrivateKeyInSlot(public_key->data(),
+                                                       private_slot.get());
   }
-  BrowserThread::PostTask(BrowserThread::UI,
-                          FROM_HERE,
-                          base::Bind(callback, public_key, private_key));
+  if (!private_key && public_slot) {
+    private_key = owner_key_util->FindPrivateKeyInSlot(public_key->data(),
+                                                       public_slot.get());

[Ryan Sleevi, 2015/05/06 23:46:03]

This shouldn't have a private key. I have no clue what's going on here.
pneubeck@, please tell me you can make sense of this :)

[Dmitry Polukhin, 2015/05/07 10:59:16]

The code was added here https://codereview.chromium.org/660343002

[pneubeck (no reviews), 2015/05/07 12:19:16]

This was added by Chris Masone here https://codereview.chromium.org/660343002,
explicitly to find both legacy owner keys (stored in the software NSSDB) and new
owner keys (stored in Chaps/TPM).

Rational behind this was discussed in this thread:
https://code.google.com/p/chromium/issues/detail?id=396439

So, I think this is fine.

[davidben, 2015/05/07 20:51:44]

Unfortunately, I had to switch this back anyway because the tests depended on it
being called with null slot. I left a TODO and moved the null check to
OwnerKeyUtilImpl.

+  }
+  // Note: Whether or not |private_key| is nullptr, the callback receives a
+  // PrivateKey wrapper.
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(callback, public_key,
+                 make_scoped_refptr(new PrivateKey(private_key))));
 }
 
 void LoadPrivateKey(
     const scoped_refptr<OwnerKeyUtil>& owner_key_util,
     const std::string username_hash,
     const base::Callback<void(const scoped_refptr<PublicKey>& public_key,
                               const scoped_refptr<PrivateKey>& private_key)>&
         callback) {
   std::vector<uint8> public_key_data;
   scoped_refptr<PublicKey> public_key;
@@ skipping 19 matching lines @@
     LoadPrivateKeyByPublicKey(
         owner_key_util, public_key, username_hash, callback);
   }
 }
 
 bool DoesPrivateKeyExistAsyncHelper(
     const scoped_refptr<OwnerKeyUtil>& owner_key_util) {
   std::vector<uint8> public_key;
   if (!owner_key_util->ImportPublicKey(&public_key))
     return false;
-  scoped_ptr<crypto::RSAPrivateKey> key(
-      crypto::RSAPrivateKey::FindFromPublicKeyInfo(public_key));
-  bool is_owner = key.get() != NULL;
-  return is_owner;
+  crypto::ScopedSECKEYPrivateKey key =
+      crypto::FindNSSKeyFromPublicKeyInfo(public_key);
+  return key && SECKEY_GetPrivateKeyType(key.get()) == rsaKey;
 }
 
 // Checks whether NSS slots with private key are mounted or
 // not. Responds via |callback|.
 void DoesPrivateKeyExistAsync(
     const scoped_refptr<OwnerKeyUtil>& owner_key_util,
     const OwnerSettingsServiceChromeOS::IsOwnerCallback& callback) {
   if (!owner_key_util.get()) {
     callback.Run(false);
     return;
@@ skipping 662 matching lines @@
   std::vector<OnManagementSettingsSetCallback> callbacks;
   pending_management_settings_callbacks_.swap(callbacks);
   for (const auto& callback : callbacks) {
     if (!callback.is_null())
       callback.Run(success);
   }
   StorePendingChanges();
 }
 
 }  // namespace chromeos