Refactoring key generation and export util code to make mocking possible.
BUG=chromium-os:4485
TEST=unit tests
Review URL: http://codereview.chromium.org/3017020
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@53292 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/chromeos/login/owner_manager.cc b/chrome/browser/chromeos/login/owner_key_utils.cc
similarity index 70%
rename from chrome/browser/chromeos/login/owner_manager.cc
rename to chrome/browser/chromeos/login/owner_key_utils.cc
index 69adbc1..916d9c7 100644
--- a/chrome/browser/chromeos/login/owner_manager.cc
+++ b/chrome/browser/chromeos/login/owner_key_utils.cc
@@ -2,7 +2,7 @@
// 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/owner_manager.h"
+#include "chrome/browser/chromeos/login/owner_key_utils.h"
#include <keyhi.h> // SECKEY_CreateSubjectPublicKeyInfo()
#include <pk11pub.h>
@@ -21,19 +21,73 @@
#include "base/string_util.h"
// static
-const char OwnerManager::kOwnerKeyFile[] = "/var/lib/whitelist/owner.key";
+OwnerKeyUtils::Factory* OwnerKeyUtils::factory_ = NULL;
+
+class OwnerKeyUtilsImpl : public OwnerKeyUtils {
+ public:
+ OwnerKeyUtilsImpl();
+ virtual ~OwnerKeyUtilsImpl();
+
+ bool GenerateKeyPair(SECKEYPrivateKey** private_key_out,
+ SECKEYPublicKey** public_key_out);
+
+ bool ExportPublicKey(SECKEYPublicKey* key, const FilePath& key_file);
+
+ SECKEYPublicKey* ImportPublicKey(const FilePath& key_file);
+
+ private:
+ // Fills in fields of |key_der| with DER encoded data from a file at
+ // |key_file|. The caller must pass in a pointer to an actual SECItem
+ // struct for |key_der|. |key_der->data| should be initialized to NULL
+ // and |key_der->len| should be set to 0.
+ //
+ // Upon success, data is stored in key_der->data, and the caller takes
+ // ownership. Returns false on error.
+ //
+ // To free the data, call
+ // SECITEM_FreeItem(key_der, PR_FALSE);
+ static bool ReadDERFromFile(const FilePath& key_file, SECItem* key_der);
+
+ // The place outside the owner's encrypted home directory where her
+ // key will live.
+ static const char kOwnerKeyFile[];
+
+ // Key generation parameters.
+ static const uint32 kKeyGenMechanism; // used by PK11_GenerateKeyPair()
+ static const unsigned long kExponent;
+ static const int kKeySizeInBits;
+
+ DISALLOW_COPY_AND_ASSIGN(OwnerKeyUtilsImpl);
+};
+
+OwnerKeyUtils::OwnerKeyUtils() {}
+
+OwnerKeyUtils::~OwnerKeyUtils() {}
+
+OwnerKeyUtils* OwnerKeyUtils::Create() {
+ if (!factory_)
+ return new OwnerKeyUtilsImpl();
+ else
+ return factory_->CreateOwnerKeyUtils();
+}
+
+// static
+const char OwnerKeyUtilsImpl::kOwnerKeyFile[] = "/var/lib/whitelist/owner.key";
// We're generating and using 2048-bit RSA keys.
// static
-const uint32 OwnerManager::kKeyGenMechanism = CKM_RSA_PKCS_KEY_PAIR_GEN;
+const uint32 OwnerKeyUtilsImpl::kKeyGenMechanism = CKM_RSA_PKCS_KEY_PAIR_GEN;
// static
-const unsigned long OwnerManager::kExponent = 65537UL;
+const unsigned long OwnerKeyUtilsImpl::kExponent = 65537UL;
// static
-const int OwnerManager::kKeySizeInBits = 2048;
+const int OwnerKeyUtilsImpl::kKeySizeInBits = 2048;
-// static
-bool OwnerManager::GenerateKeyPair(SECKEYPrivateKey** private_key_out,
- SECKEYPublicKey** public_key_out) {
+OwnerKeyUtilsImpl::OwnerKeyUtilsImpl(){}
+
+OwnerKeyUtilsImpl::~OwnerKeyUtilsImpl() {}
+
+bool OwnerKeyUtilsImpl::GenerateKeyPair(SECKEYPrivateKey** private_key_out,
+ SECKEYPublicKey** public_key_out) {
DCHECK(private_key_out);
DCHECK(public_key_out);
@@ -115,9 +169,8 @@
return is_success;
}
-// static
-bool OwnerManager::ExportPublicKey(SECKEYPublicKey* key,
- const FilePath& key_file) {
+bool OwnerKeyUtilsImpl::ExportPublicKey(SECKEYPublicKey* key,
+ const FilePath& key_file) {
DCHECK(key);
SECItem* der;
bool ok = false;
@@ -147,8 +200,7 @@
return ok;
}
-// static
-SECKEYPublicKey* OwnerManager::ImportPublicKey(const FilePath& key_file) {
+SECKEYPublicKey* OwnerKeyUtilsImpl::ImportPublicKey(const FilePath& key_file) {
SECItem key_der;
if (!ReadDERFromFile(key_file, &key_der)) {
@@ -173,8 +225,8 @@
}
// static
-bool OwnerManager::ReadDERFromFile(const FilePath& key_file,
- SECItem* key_der) {
+bool OwnerKeyUtilsImpl::ReadDERFromFile(const FilePath& key_file,
+ SECItem* key_der) {
// I'd use NSS' SECU_ReadDERFromFile() here, but the SECU_* functions are
// considered internal to the NSS command line utils.
// This code is lifted, in spirit, from the implementation of that function.
diff --git a/chrome/browser/chromeos/login/owner_key_utils.h b/chrome/browser/chromeos/login/owner_key_utils.h
new file mode 100644
index 0000000..8900ed4
--- /dev/null
+++ b/chrome/browser/chromeos/login/owner_key_utils.h
@@ -0,0 +1,70 @@
+// Copyright (c) 2010 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_OWNER_KEY_UTILS_H_
+#define CHROME_BROWSER_CHROMEOS_LOGIN_OWNER_KEY_UTILS_H_
+
+#include "base/basictypes.h"
+
+// Forward declarations of NSS data structures.
+struct SECKEYPrivateKeyStr;
+struct SECKEYPublicKeyStr;
+struct SECItemStr;
+
+typedef struct SECKEYPrivateKeyStr SECKEYPrivateKey;
+typedef struct SECKEYPublicKeyStr SECKEYPublicKey;
+typedef struct SECItemStr SECItem;
+
+class FilePath;
+
+class OwnerKeyUtils {
+ public:
+ class Factory {
+ public:
+ virtual OwnerKeyUtils* CreateOwnerKeyUtils() = 0;
+ };
+
+ OwnerKeyUtils();
+ virtual ~OwnerKeyUtils();
+
+ // Sets the factory used by the static method Create to create an
+ // OwnerKeyUtils. OwnerKeyUtils does not take ownership of
+ // |factory|. A value of NULL results in an OwnerKeyUtils being
+ // created directly.
+#if defined(UNIT_TEST)
+ static void set_factory(Factory* factory) { factory_ = factory; }
+#endif
+
+ // Creates an OwnerKeyUtils, ownership returns to the caller. If there is no
+ // Factory (the default) this creates and returns a new OwnerKeyUtils.
+ static OwnerKeyUtils* Create();
+
+ // Generate a public/private RSA keypair and store them in the NSS database.
+ // The keys will be kKeySizeInBits in length (Recommend >= 2048 bits).
+ //
+ // 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 writes it out to |key_file|.
+ // The blob on disk is a DER-encoded X509 SubjectPublicKeyInfo object.
+ // Returns false on error.
+ virtual bool ExportPublicKey(SECKEYPublicKey* key,
+ 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;
+
+ private:
+ static Factory* factory_;
+};
+
+#endif // CHROME_BROWSER_CHROMEOS_LOGIN_OWNER_KEY_UTILS_H_
diff --git a/chrome/browser/chromeos/login/owner_manager_unittest.cc b/chrome/browser/chromeos/login/owner_key_utils_unittest.cc
similarity index 79%
rename from chrome/browser/chromeos/login/owner_manager_unittest.cc
rename to chrome/browser/chromeos/login/owner_key_utils_unittest.cc
index 1f429004..7a7e9e7 100644
--- a/chrome/browser/chromeos/login/owner_manager_unittest.cc
+++ b/chrome/browser/chromeos/login/owner_key_utils_unittest.cc
@@ -2,7 +2,7 @@
// 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/owner_manager.h"
+#include "chrome/browser/chromeos/login/owner_key_utils.h"
#include <cert.h>
#include <keyhi.h>
@@ -17,20 +17,22 @@
#include "base/logging.h"
#include "base/nss_util_internal.h"
#include "base/nss_util.h"
+#include "base/scoped_ptr.h"
#include "base/scoped_temp_dir.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/gmock/include/gmock/gmock.h"
namespace chromeos {
-class OwnerManagerTest : public ::testing::Test {
+class OwnerKeyUtilsTest : public ::testing::Test {
public:
- OwnerManagerTest()
+ OwnerKeyUtilsTest()
: private_key_(NULL),
- public_key_(NULL) {
+ public_key_(NULL),
+ utils_(OwnerKeyUtils::Create()) {
}
- virtual ~OwnerManagerTest() {}
+ virtual ~OwnerKeyUtilsTest() {}
virtual void SetUp() {
base::OpenPersistentNSSDB();
@@ -49,24 +51,25 @@
SECKEYPrivateKey* private_key_;
SECKEYPublicKey* public_key_;
+ scoped_ptr<OwnerKeyUtils> utils_;
};
-TEST_F(OwnerManagerTest, KeyGenerate) {
- EXPECT_TRUE(OwnerManager::GenerateKeyPair(&private_key_, &public_key_));
+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(OwnerManagerTest, ExportImportPublicKey) {
- EXPECT_TRUE(OwnerManager::GenerateKeyPair(&private_key_, &public_key_));
+TEST_F(OwnerKeyUtilsTest, ExportImportPublicKey) {
+ EXPECT_TRUE(utils_->GenerateKeyPair(&private_key_, &public_key_));
ScopedTempDir tmpdir;
FilePath tmpfile;
ASSERT_TRUE(tmpdir.CreateUniqueTempDir());
ASSERT_TRUE(file_util::CreateTemporaryFileInDir(tmpdir.path(), &tmpfile));
- EXPECT_TRUE(OwnerManager::ExportPublicKey(public_key_, tmpfile));
+ EXPECT_TRUE(utils_->ExportPublicKey(public_key_, tmpfile));
// Now, verify that we can look up the private key, given the public key
// we exported. We'll create
@@ -83,7 +86,7 @@
if (NULL == slot)
goto cleanup;
- from_disk = OwnerManager::ImportPublicKey(tmpfile);
+ from_disk = utils_->ImportPublicKey(tmpfile);
ASSERT_TRUE(from_disk != NULL);
ck_id = PK11_MakeIDFromPubKey(&(from_disk->u.rsa.modulus));
diff --git a/chrome/browser/chromeos/login/owner_manager.h b/chrome/browser/chromeos/login/owner_manager.h
deleted file mode 100644
index ed7a3d6..0000000
--- a/chrome/browser/chromeos/login/owner_manager.h
+++ /dev/null
@@ -1,83 +0,0 @@
-// Copyright (c) 2010 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.
-
-#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_OWNER_MANAGER_H_
-#define CHROME_BROWSER_CHROMEOS_LOGIN_OWNER_MANAGER_H_
-
-#include "base/basictypes.h"
-
-// Forward declarations of NSS data structures.
-struct CERTCertificateStr;
-struct CERTCertificateRequestStr;
-struct SECKEYPrivateKeyStr;
-struct SECKEYPublicKeyStr;
-struct SECItemStr;
-
-typedef struct CERTCertificateStr CERTCertificate;
-typedef struct CERTCertificateRequestStr CERTCertificateRequest;
-typedef struct SECKEYPrivateKeyStr SECKEYPrivateKey;
-typedef struct SECKEYPublicKeyStr SECKEYPublicKey;
-typedef struct SECItemStr SECItem;
-
-class FilePath;
-
-// This class allows the registration of an Owner of a Chromium OS device.
-// It handles generating the appropriate keys and storing them in the
-// appropriate locations.
-class OwnerManager {
- public:
- OwnerManager() {}
- virtual ~OwnerManager() {}
-
- bool OwnershipAlreadyTaken();
-
- bool TakeOwnership();
-
- // Generate a public/private RSA keypair and store them in the NSS database.
- // The keys will be kKeySizeInBits in length (Recommend >= 2048 bits).
- //
- // 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);
- static bool GenerateKeyPair(SECKEYPrivateKey** private_key_out,
- SECKEYPublicKey** public_key_out);
-
- // DER encodes |key| and writes it out to |key_file|.
- // The blob on disk is a DER-encoded X509 SubjectPublicKeyInfo object.
- // Returns false on error.
- static bool ExportPublicKey(SECKEYPublicKey* key,
- const FilePath& key_file);
-
- // Assumes that the file at |key_file| exists.
- // Caller takes ownership of returned object; returns NULL on error.
- // To free, call SECKEY_DestroyPublicKey.
- static SECKEYPublicKey* ImportPublicKey(const FilePath& key_file);
-
- private:
- // Fills in fields of |key_der| with DER encoded data from a file at
- // |key_file|. The caller must pass in a pointer to an actual SECItem
- // struct for |key_der|. |key_der->data| should be initialized to NULL
- // and |key_der->len| should be set to 0.
- //
- // Upon success, data is stored in key_der->data, and the caller takes
- // ownership. Returns false on error.
- //
- // To free the data, call
- // SECITEM_FreeItem(key_der, PR_FALSE);
- static bool ReadDERFromFile(const FilePath& key_file, SECItem* key_der);
-
- // The place outside the owner's encrypted home directory where her
- // key will live.
- static const char kOwnerKeyFile[];
-
- // Key generation parameters.
- static const uint32 kKeyGenMechanism; // used by PK11_GenerateKeyPair()
- static const unsigned long kExponent;
- static const int kKeySizeInBits;
-};
-
-#endif // CHROME_BROWSER_CHROMEOS_LOGIN_OWNER_MANAGER_H_
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index c25182b..f6198d06 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -476,8 +476,8 @@
'browser/chromeos/login/network_screen_delegate.h',
'browser/chromeos/login/new_user_view.cc',
'browser/chromeos/login/new_user_view.h',
- 'browser/chromeos/login/owner_manager.cc',
- 'browser/chromeos/login/owner_manager.h',
+ 'browser/chromeos/login/owner_key_utils.cc',
+ 'browser/chromeos/login/owner_key_utils.h',
'browser/chromeos/login/password_changed_view.cc',
'browser/chromeos/login/password_changed_view.h',
'browser/chromeos/login/registration_screen.cc',
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 7101011f..253b779 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -737,7 +737,7 @@
'browser/chromeos/login/cookie_fetcher_unittest.cc',
'browser/chromeos/login/google_authenticator_unittest.cc',
'browser/chromeos/login/mock_auth_response_handler.cc',
- 'browser/chromeos/login/owner_manager_unittest.cc',
+ 'browser/chromeos/login/owner_key_utils_unittest.cc',
'browser/chromeos/notifications/desktop_notifications_unittest.cc',
'browser/chromeos/offline/offline_load_page_unittest.cc',
'browser/chromeos/options/language_config_model_unittest.cc',
