Refactor OwnerKeyUtils and OwnerManager to leverage base::RSAPrivateKey

BUG=chromium-os:4485
TEST=unit tests

Review URL: http://codereview.chromium.org/3151008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56028 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 173 insertions, 322 deletions

diff --git a/chrome/browser/chromeos/login/owner_key_utils.cc b/chrome/browser/chromeos/login/owner_key_utils.cc
index 7831d99..2f4088b 100644
--- a/chrome/browser/chromeos/login/owner_key_utils.cc
+++ b/chrome/browser/chromeos/login/owner_key_utils.cc
@@ -12,6 +12,7 @@
 
 #include <limits>
 
+#include "base/crypto/rsa_private_key.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
@@ -20,6 +21,8 @@
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
 
+using base::RSAPrivateKey;
+
 namespace chromeos {
 
 ///////////////////////////////////////////////////////////////////////////
@@ -40,18 +43,16 @@ class OwnerKeyUtilsImpl : public OwnerKeyUtils {
   OwnerKeyUtilsImpl();
   virtual ~OwnerKeyUtilsImpl();
 
-  bool GenerateKeyPair(SECKEYPrivateKey** private_key_out,
-                               SECKEYPublicKey** public_key_out);
-
-  bool ExportPublicKeyViaDbus(SECKEYPublicKey* key);
+  RSAPrivateKey* GenerateKeyPair();
 
-  bool ExportPublicKeyToFile(SECKEYPublicKey* key, const FilePath& key_file);
+  bool ExportPublicKeyViaDbus(RSAPrivateKey* pair);
 
-  SECKEYPublicKey* ImportPublicKey(const FilePath& key_file);
+  bool ExportPublicKeyToFile(RSAPrivateKey* pair, const FilePath& key_file);
 
-  SECKEYPrivateKey* FindPrivateKey(SECKEYPublicKey* key);
+  bool ImportPublicKey(const FilePath& key_file,
+                       std::vector<uint8>* output);
 
-  void DestroyKeys(SECKEYPrivateKey* private_key, SECKEYPublicKey* public_key);
+  RSAPrivateKey* FindPrivateKey(const std::vector<uint8>& key);
 
   FilePath GetOwnerKeyFilePath();
 
@@ -75,9 +76,7 @@ class OwnerKeyUtilsImpl : public OwnerKeyUtils {
   static const char kOwnerKeyFile[];
 
   // Key generation parameters.
-  static const uint32 kKeyGenMechanism;  // used by PK11_GenerateKeyPair()
-  static const unsigned long kExponent;
-  static const int kKeySizeInBits;
+  static const uint16 kKeySizeInBits;
 
   DISALLOW_COPY_AND_ASSIGN(OwnerKeyUtilsImpl);
 };
@@ -95,136 +94,67 @@ const char OwnerKeyUtilsImpl::kOwnerKeyFile[] = "/var/lib/whitelist/owner.key";
 
 // We're generating and using 2048-bit RSA keys.
 // static
-const uint32 OwnerKeyUtilsImpl::kKeyGenMechanism = CKM_RSA_PKCS_KEY_PAIR_GEN;
-// static
-const unsigned long OwnerKeyUtilsImpl::kExponent = 65537UL;
-// static
-const int OwnerKeyUtilsImpl::kKeySizeInBits = 2048;
-
-OwnerKeyUtilsImpl::OwnerKeyUtilsImpl(){}
-
-OwnerKeyUtilsImpl::~OwnerKeyUtilsImpl() {}
-
-bool OwnerKeyUtilsImpl::GenerateKeyPair(SECKEYPrivateKey** private_key_out,
-                                        SECKEYPublicKey** public_key_out) {
-  DCHECK(private_key_out);
-  DCHECK(public_key_out);
-
-  *private_key_out = NULL;
-  *public_key_out = NULL;
-
-  // Temporary structures used for generating the result
-  // in the right format.
-  PK11SlotInfo* slot = NULL;
-  PK11RSAGenParams rsa_key_gen_params;
-  void* key_gen_params;
-
-  bool is_success = true;  // Set to false as soon as a step fails.
-
-  rsa_key_gen_params.keySizeInBits = kKeySizeInBits;
-  rsa_key_gen_params.pe = kExponent;
-  key_gen_params = &rsa_key_gen_params;
+const uint16 OwnerKeyUtilsImpl::kKeySizeInBits = 2048;
 
+OwnerKeyUtilsImpl::OwnerKeyUtilsImpl() {
   // Ensure NSS is initialized.
   base::EnsureNSSInit();
+}
 
-  slot = base::GetDefaultNSSKeySlot();
-  if (!slot) {
-    LOG(ERROR) << "Couldn't get Internal key slot!";
-    is_success = false;
-    goto failure;
-  }
-
-  // Need to make sure that the token was initialized.
-  // Assume a null password.
-  if (SECSuccess != PK11_Authenticate(slot, PR_TRUE, NULL)) {
-    LOG(ERROR) << "Couldn't initialze PK11 token!";
-    is_success = false;
-    goto failure;
-  }
-
-  LOG(INFO) << "Creating key pair...";
-  {
-    base::AutoNSSWriteLock lock;
-    *private_key_out = PK11_GenerateKeyPair(slot,
-                                            kKeyGenMechanism,
-                                            key_gen_params,
-                                            public_key_out,
-                                            PR_TRUE,  // isPermanent?
-                                            PR_TRUE,  // isSensitive?
-                                            NULL);
-  }
-  LOG(INFO) << "done.";
-
-  if (!*private_key_out) {
-    LOG(INFO) << "Generation of Keypair failed!";
-    is_success = false;
-    goto failure;
-  }
-
- failure:
-  if (!is_success) {
-    LOG(ERROR) << "Owner key generation failed! (NSS error code "
-               << PR_GetError() << ")";
-    DestroyKeys(*private_key_out, *public_key_out);
-    *private_key_out = NULL;
-    *public_key_out = NULL;
-  } else {
-    LOG(INFO) << "Owner key generation succeeded!";
-  }
-  if (slot != NULL) {
-    PK11_FreeSlot(slot);
-  }
+OwnerKeyUtilsImpl::~OwnerKeyUtilsImpl() {}
 
-  return is_success;
+RSAPrivateKey* OwnerKeyUtilsImpl::GenerateKeyPair() {
+  return RSAPrivateKey::CreateSensitive(kKeySizeInBits);
 }
 
-bool OwnerKeyUtilsImpl::ExportPublicKeyViaDbus(SECKEYPublicKey* key) {
-  DCHECK(key);
+bool OwnerKeyUtilsImpl::ExportPublicKeyViaDbus(RSAPrivateKey* pair) {
+  DCHECK(pair);
   bool ok = false;
 
-  std::string to_export;
-  if (!EncodePublicKey(key, &to_export)) {
+  std::vector<uint8> to_export;
+  if (pair->ExportPublicKey(&to_export)) {
     LOG(ERROR) << "Formatting key for export failed!";
-    return ok;
+    return false;
   }
 
   // TODO(cmasone): send the data over dbus.
   return ok;
 }
 
-bool OwnerKeyUtilsImpl::ExportPublicKeyToFile(SECKEYPublicKey* key,
+bool OwnerKeyUtilsImpl::ExportPublicKeyToFile(RSAPrivateKey* pair,
                                               const FilePath& key_file) {
-  DCHECK(key);
+  DCHECK(pair);
   bool ok = false;
   int safe_file_size = 0;
 
-  std::string to_export;
-  if (!EncodePublicKey(key, &to_export)) {
+  std::vector<uint8> to_export;
+  if (!pair->ExportPublicKey(&to_export)) {
     LOG(ERROR) << "Formatting key for export failed!";
-    return ok;
+    return false;
   }
 
-  if (to_export.length() > static_cast<uint>(INT_MAX)) {
-    LOG(ERROR) << "key is too big! " << to_export.length();
+  if (to_export.size() > static_cast<uint>(INT_MAX)) {
+    LOG(ERROR) << "key is too big! " << to_export.size();
   } else {
-    safe_file_size = static_cast<int>(to_export.length());
+    safe_file_size = static_cast<int>(to_export.size());
 
-    ok = (safe_file_size == file_util::WriteFile(key_file,
-                                                 to_export.c_str(),
-                                                 safe_file_size));
+    ok = (safe_file_size ==
+          file_util::WriteFile(key_file,
+                               reinterpret_cast<char*>(&to_export.front()),
+                               safe_file_size));
   }
   return ok;
 }
 
-SECKEYPublicKey* OwnerKeyUtilsImpl::ImportPublicKey(const FilePath& key_file) {
+bool OwnerKeyUtilsImpl::ImportPublicKey(const FilePath& key_file,
+                                        std::vector<uint8>* output) {
   SECItem key_der;
   key_der.data = NULL;
   key_der.len = 0;
 
   if (!ReadDERFromFile(key_file, &key_der)) {
     PLOG(ERROR) << "Could not read in key from " << key_file.value() << ":";
-    return NULL;
+    return false;
   }
 
   // See the comment in ExportPublicKey() for why I wrote a
@@ -234,13 +164,12 @@ SECKEYPublicKey* OwnerKeyUtilsImpl::ImportPublicKey(const FilePath& key_file) {
   if (!spki) {
     LOG(ERROR) << "Could not decode key info: " << PR_GetError();
     SECITEM_FreeItem(&key_der, PR_FALSE);
-    return NULL;
+    return false;
   }
 
-  SECKEYPublicKey *public_key = SECKEY_ExtractPublicKey(spki);
-  SECKEY_DestroySubjectPublicKeyInfo(spki);
-  SECITEM_FreeItem(&key_der, PR_FALSE);
-  return public_key;
+  output->assign(reinterpret_cast<uint8*>(key_der.data),
+                 reinterpret_cast<uint8*>(key_der.data + key_der.len));
+  return key_der.len == output->size();
 }
 
 // static
@@ -287,63 +216,9 @@ bool OwnerKeyUtilsImpl::ReadDERFromFile(const FilePath& key_file,
   return true;
 }
 
-bool OwnerKeyUtilsImpl::EncodePublicKey(SECKEYPublicKey* key,
-                                        std::string* out) {
-  SECItem* der;
-
-  // Instead of exporting/importing the key directly, I'm actually
-  // going to use a SubjectPublicKeyInfo.  The reason is because NSS
-  // exports functions that encode/decode these kinds of structures, while
-  // it does not export the ones that deal directly with public keys.
-  der = SECKEY_EncodeDERSubjectPublicKeyInfo(key);
-  if (!der) {
-    LOG(ERROR) << "Could not encode public key for export!";
-    return false;
-  }
-
-  out->assign(reinterpret_cast<char*>(der->data), der->len);
-
-  SECITEM_FreeItem(der, PR_TRUE);
-  return true;
-}
-
-SECKEYPrivateKey* OwnerKeyUtilsImpl::FindPrivateKey(SECKEYPublicKey* key) {
-  DCHECK(key);
-
-  PK11SlotInfo* slot = NULL;
-  SECItem* ck_id = NULL;
-  SECKEYPrivateKey* found = NULL;
-
-  slot = base::GetDefaultNSSKeySlot();
-  if (!slot)
-    goto cleanup;
-
-  ck_id = PK11_MakeIDFromPubKey(&(key->u.rsa.modulus));
-  if (!ck_id)
-    goto cleanup;
-
-  found = PK11_FindKeyByKeyID(slot, ck_id, NULL);
-
- cleanup:
-  if (slot)
-    PK11_FreeSlot(slot);
-  if (ck_id)
-    SECITEM_FreeItem(ck_id, PR_TRUE);
-
-  return found;
-}
-
-void OwnerKeyUtilsImpl::DestroyKeys(SECKEYPrivateKey* private_key,
-                                    SECKEYPublicKey* public_key) {
-  base::AutoNSSWriteLock lock;
-  if (private_key) {
-    PK11_DestroyTokenObject(private_key->pkcs11Slot, private_key->pkcs11ID);
-    SECKEY_DestroyPrivateKey(private_key);
-  }
-  if (public_key) {
-    PK11_DestroyTokenObject(public_key->pkcs11Slot, public_key->pkcs11ID);
-    SECKEY_DestroyPublicKey(public_key);
-  }
+RSAPrivateKey* OwnerKeyUtilsImpl::FindPrivateKey(
+    const std::vector<uint8>& key) {
+  return RSAPrivateKey::FindFromPublicKeyInfo(key);
 }
 
 FilePath OwnerKeyUtilsImpl::GetOwnerKeyFilePath() {
diff --git a/chrome/browser/chromeos/login/owner_key_utils.h b/chrome/browser/chromeos/login/owner_key_utils.h
index 20becab..38d9cbb 100644
--- a/chrome/browser/chromeos/login/owner_key_utils.h
+++ b/chrome/browser/chromeos/login/owner_key_utils.h
@@ -6,19 +6,16 @@
 #define CHROME_BROWSER_CHROMEOS_LOGIN_OWNER_KEY_UTILS_H_
 #pragma once
 
-#include "base/basictypes.h"
-
-// Forward declarations of NSS data structures.
-struct SECKEYPrivateKeyStr;
-struct SECKEYPublicKeyStr;
-struct SECItemStr;
+#include <vector>
 
-typedef struct SECKEYPrivateKeyStr SECKEYPrivateKey;
-typedef struct SECKEYPublicKeyStr SECKEYPublicKey;
-typedef struct SECItemStr SECItem;
+#include "base/basictypes.h"
 
 class FilePath;
 
+namespace base {
+class RSAPrivateKey;
+}
+
 namespace chromeos {
 
 class OwnerKeyUtils {
@@ -45,43 +42,32 @@ class OwnerKeyUtils {
 
   // Generate a public/private RSA keypair and store them in the NSS database.
   // The keys will be kKeySizeInBits in length (Recommend >= 2048 bits).
+  // The caller takes ownership.
   //
-  //  Returns false on error.
-  //
-  // The caller takes ownership of both objects, which are allocated by libnss.
-  // To free them, call
-  //    SECKEY_DestroyPrivateKey(*private_key_out);
-  //    SECKEY_DestroyPublicKey(*public_key_out);
-  virtual bool GenerateKeyPair(SECKEYPrivateKey** private_key_out,
-                               SECKEYPublicKey** public_key_out) = 0;
-
-  // DER encodes |key| and exports it via DBus.
+  //  Returns NULL on error.
+  virtual base::RSAPrivateKey* GenerateKeyPair() = 0;
+
+  // DER encodes public half of |pair| and exports it via DBus.
   // The data sent is a DER-encoded X509 SubjectPublicKeyInfo object.
   // Returns false on error.
-  virtual bool ExportPublicKeyViaDbus(SECKEYPublicKey* key) = 0;
+  virtual bool ExportPublicKeyViaDbus(base::RSAPrivateKey* pair) = 0;
 
-  // DER encodes |key| and writes it out to |key_file|.
+  // DER encodes public half of |pair| and writes it out to |key_file|.
   // The blob on disk is a DER-encoded X509 SubjectPublicKeyInfo object.
   // Returns false on error.
-  virtual bool ExportPublicKeyToFile(SECKEYPublicKey* key,
+  virtual bool ExportPublicKeyToFile(base::RSAPrivateKey* pair,
                                      const FilePath& key_file) = 0;
 
   // Assumes that the file at |key_file| exists.
-  // Caller takes ownership of returned object; returns NULL on error.
-  // To free, call SECKEY_DestroyPublicKey.
-  virtual SECKEYPublicKey* ImportPublicKey(const FilePath& key_file) = 0;
-
+  // Upon success, returns true and populates |output|.  False on failure.
+  virtual bool ImportPublicKey(const FilePath& key_file,
+                               std::vector<uint8>* output) = 0;
 
   // Looks for the private key associated with |key| in the default slot,
   // and returns it if it can be found.  Returns NULL otherwise.
-  // To free, call SECKEY_DestroyPrivateKey.
-  virtual SECKEYPrivateKey* FindPrivateKey(SECKEYPublicKey* key) = 0;
-
-  // If something's gone wrong with key generation or key exporting, the
-  // caller may wish to nuke some keys.  This will destroy key objects in
-  // memory and ALSO remove them from the NSS database.
-  virtual void DestroyKeys(SECKEYPrivateKey* private_key,
-                           SECKEYPublicKey* public_key) = 0;
+  // Caller takes ownership.
+  virtual base::RSAPrivateKey* FindPrivateKey(
+      const std::vector<uint8>& key) = 0;
 
   virtual FilePath GetOwnerKeyFilePath() = 0;
 
diff --git a/chrome/browser/chromeos/login/owner_key_utils_unittest.cc b/chrome/browser/chromeos/login/owner_key_utils_unittest.cc
index 69f0584..105c560 100644
--- a/chrome/browser/chromeos/login/owner_key_utils_unittest.cc
+++ b/chrome/browser/chromeos/login/owner_key_utils_unittest.cc
@@ -11,7 +11,9 @@
 #include <stdlib.h>
 
 #include <string>
+#include <vector>
 
+#include "base/crypto/rsa_private_key.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
@@ -26,10 +28,7 @@ namespace chromeos {
 
 class OwnerKeyUtilsTest : public ::testing::Test {
  public:
-  OwnerKeyUtilsTest()
-      : private_key_(NULL),
-        public_key_(NULL),
-        utils_(OwnerKeyUtils::Create()) {
+  OwnerKeyUtilsTest() : utils_(OwnerKeyUtils::Create()) {
 
   }
   virtual ~OwnerKeyUtilsTest() {}
@@ -38,59 +37,33 @@ class OwnerKeyUtilsTest : public ::testing::Test {
     base::OpenPersistentNSSDB();
   }
 
-  virtual void TearDown() {
-    if (private_key_) {
-      PK11_DestroyTokenObject(private_key_->pkcs11Slot, private_key_->pkcs11ID);
-      SECKEY_DestroyPrivateKey(private_key_);
-    }
-    if (public_key_) {
-      PK11_DestroyTokenObject(public_key_->pkcs11Slot, public_key_->pkcs11ID);
-      SECKEY_DestroyPublicKey(public_key_);
-    }
-  }
-
-  SECKEYPrivateKey* private_key_;
-  SECKEYPublicKey* public_key_;
   scoped_ptr<OwnerKeyUtils> utils_;
 };
 
-TEST_F(OwnerKeyUtilsTest, KeyGenerate) {
-  EXPECT_TRUE(utils_->GenerateKeyPair(&private_key_, &public_key_));
-  EXPECT_TRUE(private_key_ != NULL);
-  ASSERT_TRUE(public_key_ != NULL);
-  EXPECT_EQ(public_key_->keyType, rsaKey);
-}
-
 TEST_F(OwnerKeyUtilsTest, ExportImportPublicKey) {
-  EXPECT_TRUE(utils_->GenerateKeyPair(&private_key_, &public_key_));
+  scoped_ptr<base::RSAPrivateKey> pair(utils_->GenerateKeyPair());
+  ASSERT_NE(pair.get(), reinterpret_cast<base::RSAPrivateKey*>(NULL));
 
+  // Export public key to file.
   ScopedTempDir tmpdir;
   FilePath tmpfile;
   ASSERT_TRUE(tmpdir.CreateUniqueTempDir());
   ASSERT_TRUE(file_util::CreateTemporaryFileInDir(tmpdir.path(), &tmpfile));
-
-  EXPECT_TRUE(utils_->ExportPublicKeyToFile(public_key_, tmpfile));
-
-  // Now, verify that we can look up the private key, given the public
-  // key we exported.  Then we'll make sure it's the same as |private_key_|
-  SECKEYPublicKey* from_disk = NULL;
-  SECKEYPrivateKey* found = NULL;
-
-  from_disk = utils_->ImportPublicKey(tmpfile);
-  ASSERT_TRUE(from_disk != NULL);
-
-  found = utils_->FindPrivateKey(from_disk);
-  EXPECT_TRUE(found != NULL);
-  if (NULL == found)
-    goto cleanup;
-
-  EXPECT_EQ(private_key_->pkcs11ID, found->pkcs11ID);
-
- cleanup:
-  if (from_disk)
-    SECKEY_DestroyPublicKey(from_disk);
-  if (found)
-    SECKEY_DestroyPrivateKey(found);
+  ASSERT_TRUE(utils_->ExportPublicKeyToFile(pair.get(), tmpfile));
+
+  // Export public key, so that we can compare it to the one we get off disk.
+  std::vector<uint8> public_key;
+  ASSERT_TRUE(pair->ExportPublicKey(&public_key));
+  std::vector<uint8> from_disk;
+  ASSERT_TRUE(utils_->ImportPublicKey(tmpfile, &from_disk));
+
+  std::vector<uint8>::iterator pubkey_it;
+  std::vector<uint8>::iterator disk_it;
+  for (pubkey_it = public_key.begin(), disk_it = from_disk.begin();
+       pubkey_it < public_key.end();
+       pubkey_it++, disk_it++) {
+    EXPECT_EQ(*pubkey_it, *disk_it);
+  }
 }
 
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/login/owner_manager.cc b/chrome/browser/chromeos/login/owner_manager.cc
index 546952a3..14c0bc1 100644
--- a/chrome/browser/chromeos/login/owner_manager.cc
+++ b/chrome/browser/chromeos/login/owner_manager.cc
@@ -83,74 +83,74 @@ bool OwnerManager::StartVerifyAttempt(const std::string& data,
 
 void OwnerManager::LoadOwnerKey() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-  public_key_ = utils_->ImportPublicKey(utils_->GetOwnerKeyFilePath());
+  NotificationType result = NotificationType::OWNER_KEY_FETCH_ATTEMPT_FAILED;
+  if (utils_->ImportPublicKey(utils_->GetOwnerKeyFilePath(), &public_key_))
+    result = NotificationType::OWNER_KEY_FETCH_ATTEMPT_SUCCEEDED;
 
   // Whether we loaded the public key or not, send a notification indicating
-  // that we're done with this attempt.  We send along the key if we
-  // got it, NULL if not.
+  // that we're done with this attempt.
   ChromeThread::PostTask(
       ChromeThread::UI, FROM_HERE,
       NewRunnableMethod(this,
                         &OwnerManager::SendNotification,
-                        NotificationType::OWNER_KEY_FETCH_ATTEMPT_COMPLETE,
-                        Details<SECKEYPublicKey*>(&public_key_)));
+                        result,
+                        NotificationService::NoDetails()));
 }
 
 void OwnerManager::GenerateKeysAndExportPublic() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
-  public_key_ = NULL;
-  private_key_ = NULL;
+  NotificationType result = NotificationType::OWNER_KEY_FETCH_ATTEMPT_FAILED;
+  private_key_.reset(utils_->GenerateKeyPair());
 
-  if (utils_->GenerateKeyPair(&private_key_, &public_key_)) {
+  if (private_key_.get()) {
+    result = NotificationType::OWNER_KEY_FETCH_ATTEMPT_SUCCEEDED;
     // If we generated the keys successfully, export them.
     ChromeThread::PostTask(
         ChromeThread::UI, FROM_HERE,
         NewRunnableMethod(this, &OwnerManager::ExportKey));
   } else {
-    // If we didn't generate the key, send along NULL with the notification
-    // that we're done with this attempt.
+    // If we didn't generate the key, send along a notification of failure.
     ChromeThread::PostTask(
         ChromeThread::UI, FROM_HERE,
         NewRunnableMethod(this,
                           &OwnerManager::SendNotification,
-                          NotificationType::OWNER_KEY_FETCH_ATTEMPT_COMPLETE,
-                          Details<SECKEYPublicKey*>(&public_key_)));
+                          result,
+                          NotificationService::NoDetails()));
   }
 }
 
 void OwnerManager::ExportKey() {
-  if (!utils_->ExportPublicKeyViaDbus(public_key_)) {
-    utils_->DestroyKeys(private_key_, public_key_);
-    private_key_ = NULL;
-    public_key_ = NULL;
+  NotificationType result = NotificationType::OWNER_KEY_FETCH_ATTEMPT_SUCCEEDED;
+  if (!utils_->ExportPublicKeyViaDbus(private_key_.get())) {
+    private_key_.reset(NULL);
+    result = NotificationType::OWNER_KEY_FETCH_ATTEMPT_FAILED;
   }
 
   // Whether we generated the keys or not, send a notification indicating
-  // that we're done with this attempt.  We send along the public key if we
-  // got it, NULL if not.
+  // that we're done with this attempt.
   ChromeThread::PostTask(
       ChromeThread::UI, FROM_HERE,
       NewRunnableMethod(this,
                         &OwnerManager::SendNotification,
-                        NotificationType::OWNER_KEY_FETCH_ATTEMPT_COMPLETE,
-                        Details<SECKEYPublicKey*>(&public_key_)));
+                        result,
+                        NotificationService::NoDetails()));
 }
 
 bool OwnerManager::EnsurePublicKey() {
-  if (!public_key_)
+  if (public_key_.empty())
     LoadOwnerKey();
 
-  return public_key_ != NULL;
+  return !public_key_.empty();
 }
 
 bool OwnerManager::EnsurePrivateKey() {
   if (!EnsurePublicKey())
     return false;
 
-  if (!private_key_)
-    private_key_ = utils_->FindPrivateKey(public_key_);
+  if (!private_key_.get())
+    private_key_.reset(utils_->FindPrivateKey(public_key_));
 
-  return private_key_ != NULL;
+  return private_key_.get() != NULL;
 }
 
 void OwnerManager::Sign(const ChromeThread::ID thread_id,
diff --git a/chrome/browser/chromeos/login/owner_manager.h b/chrome/browser/chromeos/login/owner_manager.h
index 6ce7fc9..e9ba9b8 100644
--- a/chrome/browser/chromeos/login/owner_manager.h
+++ b/chrome/browser/chromeos/login/owner_manager.h
@@ -6,7 +6,10 @@
 #define CHROME_BROWSER_CHROMEOS_LOGIN_OWNER_MANAGER_H_
 #pragma once
 
+#include <vector>
+
 #include "base/basictypes.h"
+#include "base/crypto/rsa_private_key.h"
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "chrome/browser/chromeos/login/owner_key_utils.h"
@@ -140,8 +143,8 @@ class OwnerManager : public base::RefCountedThreadSafe<OwnerManager> {
     d->OnKeyOpComplete(return_code, payload);
   }
 
-  SECKEYPrivateKey* private_key_;
-  SECKEYPublicKey* public_key_;
+  scoped_ptr<base::RSAPrivateKey> private_key_;
+  std::vector<uint8> public_key_;
 
   scoped_ptr<OwnerKeyUtils> utils_;
 
diff --git a/chrome/browser/chromeos/login/owner_manager_unittest.cc b/chrome/browser/chromeos/login/owner_manager_unittest.cc
index 1db81e9..3308399 100644
--- a/chrome/browser/chromeos/login/owner_manager_unittest.cc
+++ b/chrome/browser/chromeos/login/owner_manager_unittest.cc
@@ -12,6 +12,7 @@
 
 #include <string>
 
+#include "base/crypto/rsa_private_key.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/logging.h"
@@ -25,6 +26,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ::base::RSAPrivateKey;
 using ::testing::DoAll;
 using ::testing::Invoke;
 using ::testing::Return;
@@ -39,16 +41,16 @@ class MockKeyUtils : public OwnerKeyUtils {
  public:
   MockKeyUtils() {}
   ~MockKeyUtils() {}
-  MOCK_METHOD2(GenerateKeyPair, bool(SECKEYPrivateKey** private_key_out,
-                                     SECKEYPublicKey** public_key_out));
-  MOCK_METHOD1(ExportPublicKeyViaDbus, bool(SECKEYPublicKey* key));
-  MOCK_METHOD2(ExportPublicKeyToFile, bool(SECKEYPublicKey* key,
+  MOCK_METHOD0(GenerateKeyPair, RSAPrivateKey*());
+  MOCK_METHOD1(ExportPublicKeyViaDbus, bool(RSAPrivateKey* pair));
+  MOCK_METHOD2(ExportPublicKeyToFile, bool(RSAPrivateKey* pair,
                                            const FilePath& key_file));
-  MOCK_METHOD1(ImportPublicKey, SECKEYPublicKey*(const FilePath& key_file));
-  MOCK_METHOD1(FindPrivateKey, SECKEYPrivateKey*(SECKEYPublicKey* key));
-  MOCK_METHOD2(DestroyKeys, void(SECKEYPrivateKey* private_key,
-                                 SECKEYPublicKey* public_key));
+  MOCK_METHOD2(ImportPublicKey, bool(const FilePath& key_file,
+                                     std::vector<uint8>* output));
+  MOCK_METHOD1(FindPrivateKey, RSAPrivateKey*(const std::vector<uint8>& key));
   MOCK_METHOD0(GetOwnerKeyFilePath, FilePath());
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockKeyUtils);
 };
 
 class MockInjector : public OwnerKeyUtils::Factory {
@@ -74,6 +76,7 @@ class MockInjector : public OwnerKeyUtils::Factory {
  private:
   MockKeyUtils* transient_;
   bool delete_transient_;
+  DISALLOW_COPY_AND_ASSIGN(MockInjector);
 };
 
 class KeyUser : public OwnerManager::Delegate {
@@ -91,14 +94,16 @@ class KeyUser : public OwnerManager::Delegate {
   }
 
   const OwnerManager::KeyOpCode expected_;
+ private:
+  DISALLOW_COPY_AND_ASSIGN(KeyUser);
 };
 
-static bool Win(SECKEYPublicKey* key) {
+static bool Win(RSAPrivateKey* key) {
   MessageLoop::current()->Quit();
   return true;
 }
 
-static bool Fail(SECKEYPublicKey* key) {
+static bool Fail(RSAPrivateKey* key) {
   MessageLoop::current()->Quit();
   return false;
 }
@@ -112,20 +117,26 @@ class OwnerManagerTest : public ::testing::Test,
       : message_loop_(MessageLoop::TYPE_UI),
         ui_thread_(ChromeThread::UI, &message_loop_),
         file_thread_(ChromeThread::FILE),
-        fake_public_key_(reinterpret_cast<SECKEYPublicKey*>(7)),
-        fake_private_key_(reinterpret_cast<SECKEYPrivateKey*>(7)),
         success_expected_(false),
         quit_on_observe_(true),
         mock_(new MockKeyUtils),
         injector_(mock_) /* injector_ takes ownership of mock_ */ {
     registrar_.Add(
         this,
-        NotificationType::OWNER_KEY_FETCH_ATTEMPT_COMPLETE,
+        NotificationType::OWNER_KEY_FETCH_ATTEMPT_FAILED,
+        NotificationService::AllSources());
+    registrar_.Add(
+        this,
+        NotificationType::OWNER_KEY_FETCH_ATTEMPT_SUCCEEDED,
         NotificationService::AllSources());
   }
   virtual ~OwnerManagerTest() {}
 
   virtual void SetUp() {
+    base::OpenPersistentNSSDB();  // TODO(cmasone): use test DB instead
+    fake_private_key_.reset(RSAPrivateKey::Create(256));
+    ASSERT_TRUE(fake_private_key_->ExportPublicKey(&fake_public_key_));
+
     // Mimic ownership.
     ASSERT_TRUE(tmpdir_.CreateUniqueTempDir());
     ASSERT_TRUE(file_util::CreateTemporaryFileInDir(tmpdir_.path(), &tmpfile_));
@@ -144,16 +155,19 @@ class OwnerManagerTest : public ::testing::Test,
 
   void InjectKeys(OwnerManager* manager) {
     manager->public_key_ = fake_public_key_;
-    manager->private_key_ = fake_private_key_;
+    manager->private_key_.reset(fake_private_key_.release());
   }
 
   // NotificationObserver implementation.
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details) {
-    if (type == NotificationType::OWNER_KEY_FETCH_ATTEMPT_COMPLETE) {
-      EXPECT_EQ(success_expected_,
-                NULL != *Details<SECKEYPublicKey*>(details).ptr());
+    if (type == NotificationType::OWNER_KEY_FETCH_ATTEMPT_SUCCEEDED) {
+      EXPECT_TRUE(success_expected_);
+      if (quit_on_observe_)
+        MessageLoop::current()->Quit();
+    } else if (type == NotificationType::OWNER_KEY_FETCH_ATTEMPT_FAILED) {
+      EXPECT_FALSE(success_expected_);
       if (quit_on_observe_)
         MessageLoop::current()->Quit();
     }
@@ -162,6 +176,7 @@ class OwnerManagerTest : public ::testing::Test,
   void ExpectKeyFetchSuccess(bool should_succeed) {
     success_expected_ = should_succeed;
   }
+
   void SetQuitOnKeyFetch(bool should_quit) { quit_on_observe_ = should_quit; }
 
   ScopedTempDir tmpdir_;
@@ -171,8 +186,8 @@ class OwnerManagerTest : public ::testing::Test,
   ChromeThread ui_thread_;
   ChromeThread file_thread_;
 
-  SECKEYPublicKey* fake_public_key_;
-  SECKEYPrivateKey* fake_private_key_;
+  std::vector<uint8> fake_public_key_;
+  scoped_ptr<RSAPrivateKey> fake_private_key_;
 
   NotificationRegistrar registrar_;
   bool success_expected_;
@@ -193,12 +208,10 @@ TEST_F(OwnerManagerTest, LoadKeyUnowned) {
 }
 
 TEST_F(OwnerManagerTest, LoadOwnerKeyFail) {
-  SECKEYPublicKey* to_return = NULL;
-
   EXPECT_CALL(*mock_, GetOwnerKeyFilePath())
       .WillRepeatedly(Return(tmpfile_));
-  EXPECT_CALL(*mock_, ImportPublicKey(tmpfile_))
-      .WillOnce(Return(to_return))
+  EXPECT_CALL(*mock_, ImportPublicKey(tmpfile_, _))
+      .WillOnce(Return(false))
       .RetiresOnSaturation();
 
   scoped_refptr<OwnerManager> manager(new OwnerManager);
@@ -213,8 +226,9 @@ TEST_F(OwnerManagerTest, LoadOwnerKey) {
 
   EXPECT_CALL(*mock_, GetOwnerKeyFilePath())
       .WillRepeatedly(Return(tmpfile_));
-  EXPECT_CALL(*mock_, ImportPublicKey(tmpfile_))
-      .WillOnce(Return(fake_public_key_))
+  EXPECT_CALL(*mock_, ImportPublicKey(tmpfile_, _))
+      .WillOnce(DoAll(SetArgumentPointee<1>(fake_public_key_),
+                      Return(true)))
       .RetiresOnSaturation();
 
   scoped_refptr<OwnerManager> manager(new OwnerManager);
@@ -236,8 +250,8 @@ TEST_F(OwnerManagerTest, TakeOwnershipAlreadyOwned) {
 TEST_F(OwnerManagerTest, KeyGenerationFail) {
   StartUnowned();
 
-  EXPECT_CALL(*mock_, GenerateKeyPair(_, _))
-      .WillOnce(Return(false))
+  EXPECT_CALL(*mock_, GenerateKeyPair())
+      .WillOnce(Return(reinterpret_cast<RSAPrivateKey*>(NULL)))
       .RetiresOnSaturation();
   EXPECT_CALL(*mock_, GetOwnerKeyFilePath())
       .WillRepeatedly(Return(tmpfile_));
@@ -251,14 +265,11 @@ TEST_F(OwnerManagerTest, KeyGenerationFail) {
 TEST_F(OwnerManagerTest, KeyExportFail) {
   StartUnowned();
 
-  EXPECT_CALL(*mock_, GenerateKeyPair(_, _))
-      .WillOnce(Return(true))
-      .RetiresOnSaturation();
-  EXPECT_CALL(*mock_, ExportPublicKeyViaDbus(_))
+  EXPECT_CALL(*mock_, ExportPublicKeyViaDbus(fake_private_key_.get()))
       .WillOnce(Invoke(Fail))
       .RetiresOnSaturation();
-  EXPECT_CALL(*mock_, DestroyKeys(_, _))
-      .Times(1)
+  EXPECT_CALL(*mock_, GenerateKeyPair())
+      .WillOnce(Return(fake_private_key_.release()))
       .RetiresOnSaturation();
   EXPECT_CALL(*mock_, GetOwnerKeyFilePath())
       .WillRepeatedly(Return(tmpfile_));
@@ -273,13 +284,12 @@ TEST_F(OwnerManagerTest, TakeOwnership) {
   StartUnowned();
   ExpectKeyFetchSuccess(true);
 
-  EXPECT_CALL(*mock_, GenerateKeyPair(_, _))
-      .WillOnce(DoAll(SetArgumentPointee<1>(fake_public_key_),
-                      Return(true)))
-      .RetiresOnSaturation();
-  EXPECT_CALL(*mock_, ExportPublicKeyViaDbus(_))
+  EXPECT_CALL(*mock_, ExportPublicKeyViaDbus(fake_private_key_.get()))
       .WillOnce(Invoke(Win))
       .RetiresOnSaturation();
+  EXPECT_CALL(*mock_, GenerateKeyPair())
+      .WillOnce(Return(fake_private_key_.release()))
+      .RetiresOnSaturation();
   EXPECT_CALL(*mock_, GetOwnerKeyFilePath())
       .WillRepeatedly(Return(tmpfile_));
 
@@ -317,12 +327,11 @@ TEST_F(OwnerManagerTest, AlreadyHaveKeysVerify) {
 
 TEST_F(OwnerManagerTest, GetKeyFailDuringVerify) {
   ExpectKeyFetchSuccess(false);
-  SECKEYPublicKey* to_return = NULL;
 
   EXPECT_CALL(*mock_, GetOwnerKeyFilePath())
       .WillRepeatedly(Return(tmpfile_));
-  EXPECT_CALL(*mock_, ImportPublicKey(tmpfile_))
-      .WillOnce(Return(to_return))
+  EXPECT_CALL(*mock_, ImportPublicKey(tmpfile_, _))
+      .WillOnce(Return(false))
       .RetiresOnSaturation();
 
   scoped_refptr<OwnerManager> manager(new OwnerManager);
@@ -338,8 +347,9 @@ TEST_F(OwnerManagerTest, GetKeyAndVerify) {
 
   EXPECT_CALL(*mock_, GetOwnerKeyFilePath())
       .WillRepeatedly(Return(tmpfile_));
-  EXPECT_CALL(*mock_, ImportPublicKey(tmpfile_))
-      .WillOnce(Return(fake_public_key_))
+  EXPECT_CALL(*mock_, ImportPublicKey(tmpfile_, _))
+      .WillOnce(DoAll(SetArgumentPointee<1>(fake_public_key_),
+                      Return(true)))
       .RetiresOnSaturation();
 
   scoped_refptr<OwnerManager> manager(new OwnerManager);
diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h
index f07cb59..71589ba 100644
--- a/chrome/common/notification_type.h
+++ b/chrome/common/notification_type.h
@@ -1081,8 +1081,12 @@ class NotificationType {
     NETWORK_STATE_CHANGED,
 
     // Sent when an attempt to acquire the public key of the owner of a chromium
-    // os device has completed.  Details are a boolean value indicating success.
-    OWNER_KEY_FETCH_ATTEMPT_COMPLETE,
+    // os device has succeeded.
+    OWNER_KEY_FETCH_ATTEMPT_SUCCEEDED,
+
+    // Sent when an attempt to acquire the public key of the owner of a chromium
+    // os device has failed.
+    OWNER_KEY_FETCH_ATTEMPT_FAILED,
 #endif
 
     // Sent before the repost form warning is brought up.