diff options
author | tbarzic@chromium.org <tbarzic@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-28 06:17:00 +0000 |
---|---|---|
committer | tbarzic@chromium.org <tbarzic@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-28 06:17:00 +0000 |
commit | 69295bad5da2d9ddc84f0ed196dc3efcce3e239c (patch) | |
tree | ea58a49fce76df7bcd43e083ca79003a4bd36e40 | |
parent | fb7b39f067140c5bd66f38f23fcdb95eb0f71a78 (diff) | |
download | chromium_src-69295bad5da2d9ddc84f0ed196dc3efcce3e239c.zip chromium_src-69295bad5da2d9ddc84f0ed196dc3efcce3e239c.tar.gz chromium_src-69295bad5da2d9ddc84f0ed196dc3efcce3e239c.tar.bz2 |
Use user specific NSSDatabase in CertLoader.
CertLoader is still global object, but it now loads only primary
user's certificates. Loading only primary user's certificates
is ok, since shill only uses primary user's network profile
(and currently only network stack is interested in certificates from CertLoader).
Added some tests for CertLoader and NetworkConnectionHandler.
BUG=315343
Review URL: https://codereview.chromium.org/135193007
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247414 0039d316-1c4b-4281-b951-d872f2087c98
28 files changed, 735 insertions, 206 deletions
diff --git a/chrome/browser/chromeos/login/fake_login_utils.cc b/chrome/browser/chromeos/login/fake_login_utils.cc index 2f7a0f1..5a5de7c 100644 --- a/chrome/browser/chromeos/login/fake_login_utils.cc +++ b/chrome/browser/chromeos/login/fake_login_utils.cc @@ -109,6 +109,10 @@ void FakeLoginUtils::InitRlzDelayed(Profile* user_profile) { NOTREACHED() << "Method not implemented."; } +void FakeLoginUtils::StartCertLoader(Profile* user_profile) { + NOTREACHED() << "Method not implemented."; +} + Profile* FakeLoginUtils::CreateProfile(const std::string& username_hash) { base::FilePath path; PathService::Get(chrome::DIR_USER_DATA, &path); diff --git a/chrome/browser/chromeos/login/fake_login_utils.h b/chrome/browser/chromeos/login/fake_login_utils.h index 23a834e..34cae03 100644 --- a/chrome/browser/chromeos/login/fake_login_utils.h +++ b/chrome/browser/chromeos/login/fake_login_utils.h @@ -34,6 +34,7 @@ class FakeLoginUtils : public LoginUtils { LoginStatusConsumer* consumer) OVERRIDE; virtual void RestoreAuthenticationSession(Profile* profile) OVERRIDE; virtual void InitRlzDelayed(Profile* user_profile) OVERRIDE; + virtual void StartCertLoader(Profile* user_profile) OVERRIDE; void SetExpectedCredentials(const std::string& username, const std::string& password); diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc index 57df965..cdf7261 100644 --- a/chrome/browser/chromeos/login/login_utils.cc +++ b/chrome/browser/chromeos/login/login_utils.cc @@ -51,6 +51,7 @@ #include "chrome/browser/first_run/first_run.h" #include "chrome/browser/google/google_util_chromeos.h" #include "chrome/browser/lifetime/application_lifetime.h" +#include "chrome/browser/net/nss_context.h" #include "chrome/browser/pref_service_flags_storage.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_manager.h" @@ -63,6 +64,7 @@ #include "chrome/common/chrome_switches.h" #include "chrome/common/logging_chrome.h" #include "chrome/common/pref_names.h" +#include "chromeos/cert_loader.h" #include "chromeos/chromeos_switches.h" #include "chromeos/cryptohome/cryptohome_util.h" #include "chromeos/dbus/cryptohome_client.h" @@ -81,6 +83,10 @@ using content::BrowserThread; +namespace net { +class NSSCertDatabase; +} + namespace chromeos { namespace { @@ -134,6 +140,7 @@ class LoginUtilsImpl LoginStatusConsumer* consumer) OVERRIDE; virtual void RestoreAuthenticationSession(Profile* profile) OVERRIDE; virtual void InitRlzDelayed(Profile* user_profile) OVERRIDE; + virtual void StartCertLoader(Profile* user_profile) OVERRIDE; // OAuth2LoginManager::Observer overrides. virtual void OnSessionRestoreStateChanged( @@ -197,6 +204,10 @@ class LoginUtilsImpl // Initializes RLZ. If |disabled| is true, RLZ pings are disabled. void InitRlz(Profile* user_profile, bool disabled); + // Starts CertLoader with the provided NSS database. It must be called at most + // once, and with the primary user's database. + void StartCertLoaderWithNSSDB(net::NSSCertDatabase* database); + // Attempts restarting the browser process and esures that this does // not happen while we are still fetching new OAuth refresh tokens. void AttemptRestart(Profile* profile); @@ -584,10 +595,12 @@ void LoginUtilsImpl::FinalizePrepareProfile(Profile* user_profile) { content::NotificationService::AllSources(), content::Details<Profile>(user_profile)); - // Initialize RLZ only for primary user. + // Initialize RLZ and CertLoader only for primary user. if (UserManager::Get()->GetPrimaryUser() == UserManager::Get()->GetUserByProfile(user_profile)) { InitRlzDelayed(user_profile); + if (CertLoader::IsInitialized()) + StartCertLoader(user_profile); } // TODO(altimofeev): This pointer should probably never be NULL, but it looks // like LoginUtilsImpl::OnProfileCreated() may be getting called before @@ -640,6 +653,16 @@ void LoginUtilsImpl::InitRlz(Profile* user_profile, bool disabled) { #endif } +void LoginUtilsImpl::StartCertLoader(Profile* user_profile) { + GetNSSCertDatabaseForProfile( + user_profile, + base::Bind(&LoginUtilsImpl::StartCertLoaderWithNSSDB, AsWeakPtr())); +} + +void LoginUtilsImpl::StartCertLoaderWithNSSDB(net::NSSCertDatabase* database) { + CertLoader::Get()->StartWithNSSDB(database); +} + void LoginUtilsImpl::CompleteOffTheRecordLogin(const GURL& start_url) { VLOG(1) << "Completing incognito login"; diff --git a/chrome/browser/chromeos/login/login_utils.h b/chrome/browser/chromeos/login/login_utils.h index 0e64383..52f2deb 100644 --- a/chrome/browser/chromeos/login/login_utils.h +++ b/chrome/browser/chromeos/login/login_utils.h @@ -103,6 +103,9 @@ class LoginUtils { // Initialize RLZ. virtual void InitRlzDelayed(Profile* user_profile) = 0; + + // Initiates process of starting CertLoader for the user_profile. + virtual void StartCertLoader(Profile* user_profile) = 0; }; } // namespace chromeos diff --git a/chrome/browser/chromeos/login/mock_login_utils.h b/chrome/browser/chromeos/login/mock_login_utils.h index 269ac49..8273065 100644 --- a/chrome/browser/chromeos/login/mock_login_utils.h +++ b/chrome/browser/chromeos/login/mock_login_utils.h @@ -46,6 +46,7 @@ class MockLoginUtils : public LoginUtils { MOCK_METHOD2(TransferDefaultAuthCache, void(Profile*, Profile*)); MOCK_METHOD0(StopBackgroundFetchers, void(void)); MOCK_METHOD1(InitRlzDelayed, void(Profile*)); + MOCK_METHOD1(StartCertLoader, void(Profile*)); void DelegateToFake(); FakeLoginUtils* GetFakeLoginUtils(); diff --git a/chrome/browser/chromeos/login/parallel_authenticator_unittest.cc b/chrome/browser/chromeos/login/parallel_authenticator_unittest.cc index 27a82202..0ad3bdd 100644 --- a/chrome/browser/chromeos/login/parallel_authenticator_unittest.cc +++ b/chrome/browser/chromeos/login/parallel_authenticator_unittest.cc @@ -321,10 +321,8 @@ TEST_F(ParallelAuthenticatorTest, ResolveOwnerNeededFailedMount) { SetAndResolveState(auth_.get(), state_.release())); EXPECT_TRUE(LoginState::Get()->IsInSafeMode()); - // Simulate TPM token ready event. The tpm token parameters are not - // actually used by the DeviceSettingsService, so it is OK to pass arbitrary - // values. - DeviceSettingsService::Get()->OnTPMTokenReady("pin", "token_name", 0); + // Simulate TPM token ready event. + DeviceSettingsService::Get()->OnTPMTokenReady(); // Flush all the pending operations. The operations should induce an owner // verification. diff --git a/chrome/browser/chromeos/login/test_login_utils.cc b/chrome/browser/chromeos/login/test_login_utils.cc index 750e773..a3f5934 100644 --- a/chrome/browser/chromeos/login/test_login_utils.cc +++ b/chrome/browser/chromeos/login/test_login_utils.cc @@ -41,4 +41,7 @@ scoped_refptr<Authenticator> TestLoginUtils::CreateAuthenticator( void TestLoginUtils::InitRlzDelayed(Profile* user_profile) { } +void TestLoginUtils::StartCertLoader(Profile* user_profile) { +} + } // namespace chromeos diff --git a/chrome/browser/chromeos/login/test_login_utils.h b/chrome/browser/chromeos/login/test_login_utils.h index 9f27300..84b49ea 100644 --- a/chrome/browser/chromeos/login/test_login_utils.h +++ b/chrome/browser/chromeos/login/test_login_utils.h @@ -46,6 +46,8 @@ class TestLoginUtils : public LoginUtils { virtual void InitRlzDelayed(Profile* user_profile) OVERRIDE; + virtual void StartCertLoader(Profile* user_profile) OVERRIDE; + private: std::string expected_username_; std::string expected_password_; diff --git a/chrome/browser/chromeos/options/cert_library.cc b/chrome/browser/chromeos/options/cert_library.cc index 8c9ac18..d905186 100644 --- a/chrome/browser/chromeos/options/cert_library.cc +++ b/chrome/browser/chromeos/options/cert_library.cc @@ -36,7 +36,8 @@ namespace { // Root CA certificates that are built into Chrome use this token name. const char kRootCertificateTokenName[] = "Builtin Object Token"; -base::string16 GetDisplayString(net::X509Certificate* cert, bool hardware_backed) { +base::string16 GetDisplayString(net::X509Certificate* cert, + bool hardware_backed) { std::string org; if (!cert->subject().organization_names.empty()) org = cert->subject().organization_names[0]; @@ -170,13 +171,8 @@ std::string CertLibrary::GetCertPkcs11IdAt(CertType type, int index) const { } bool CertLibrary::IsCertHardwareBackedAt(CertType type, int index) const { - if (!CertLoader::Get()->IsHardwareBacked()) - return false; net::X509Certificate* cert = GetCertificateAt(type, index); - std::string cert_token_name = - x509_certificate_model::GetTokenName(cert->os_cert_handle()); - return cert_token_name == - CertLoader::Get()->tpm_token_name(); + return CertLoader::Get()->IsCertificateHardwareBacked(cert); } int CertLibrary::GetCertIndexByPEM(CertType type, diff --git a/chrome/browser/chromeos/settings/device_settings_service.cc b/chrome/browser/chromeos/settings/device_settings_service.cc index 5aa7efe..f591948 100644 --- a/chrome/browser/chromeos/settings/device_settings_service.cc +++ b/chrome/browser/chromeos/settings/device_settings_service.cc @@ -235,9 +235,7 @@ void DeviceSettingsService::PropertyChangeComplete(bool success) { EnsureReload(false); } -void DeviceSettingsService::OnTPMTokenReady(const std::string& tpm_user_pin, - const std::string& tpm_token_name, - int tpm_token_slot_id) { +void DeviceSettingsService::OnTPMTokenReady() { waiting_for_tpm_token_ = false; // TPMTokenLoader initializes the TPM and NSS database which is necessary to diff --git a/chrome/browser/chromeos/settings/device_settings_service.h b/chrome/browser/chromeos/settings/device_settings_service.h index b70a753..051d831 100644 --- a/chrome/browser/chromeos/settings/device_settings_service.h +++ b/chrome/browser/chromeos/settings/device_settings_service.h @@ -197,9 +197,7 @@ class DeviceSettingsService : public SessionManagerClient::Observer, virtual void PropertyChangeComplete(bool success) OVERRIDE; // TPMTokenLoader::Observer: - virtual void OnTPMTokenReady(const std::string& tpm_user_pin, - const std::string& tpm_token_name, - int tpm_token_slot_id) OVERRIDE; + virtual void OnTPMTokenReady() OVERRIDE; private: // Enqueues a new operation. Takes ownership of |operation| and starts it diff --git a/chrome/browser/chromeos/settings/device_settings_service_unittest.cc b/chrome/browser/chromeos/settings/device_settings_service_unittest.cc index aaa3cea..746287c 100644 --- a/chrome/browser/chromeos/settings/device_settings_service_unittest.cc +++ b/chrome/browser/chromeos/settings/device_settings_service_unittest.cc @@ -349,7 +349,7 @@ TEST_F(DeviceSettingsServiceTest, OnTPMTokenReadyForNonOwner) { device_settings_service_.GetOwnershipStatus()); EXPECT_FALSE(is_owner_set_); - device_settings_service_.OnTPMTokenReady("tpm_pin", "tpm_token", 0); + device_settings_service_.OnTPMTokenReady(); FlushDeviceSettings(); EXPECT_FALSE(device_settings_service_.HasPrivateOwnerKey()); @@ -392,7 +392,7 @@ TEST_F(DeviceSettingsServiceTest, OnTPMTokenReadyForOwner) { owner_key_util_->SetPrivateKey(device_policy_.GetSigningKey()); device_settings_service_.SetUsername(device_policy_.policy_data().username()); - device_settings_service_.OnTPMTokenReady("tpm_pin", "tpm_token_name", 0); + device_settings_service_.OnTPMTokenReady(); FlushDeviceSettings(); EXPECT_TRUE(device_settings_service_.HasPrivateOwnerKey()); @@ -420,7 +420,7 @@ TEST_F(DeviceSettingsServiceTest, IsCurrentUserOwnerAsyncWithLoadedCerts) { device_settings_service_.SetUsername(device_policy_.policy_data().username()); ReloadDeviceSettings(); - device_settings_service_.OnTPMTokenReady("tpm_pin", "tpm_token_name", 0); + device_settings_service_.OnTPMTokenReady(); FlushDeviceSettings(); EXPECT_TRUE(device_settings_service_.HasPrivateOwnerKey()); diff --git a/chrome/browser/net/nss_context.cc b/chrome/browser/net/nss_context.cc new file mode 100644 index 0000000..1aad384 --- /dev/null +++ b/chrome/browser/net/nss_context.cc @@ -0,0 +1,57 @@ +// 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/net/nss_context.h" + +#include "base/message_loop/message_loop_proxy.h" +#include "chrome/browser/profiles/profile.h" +#include "content/public/browser/browser_thread.h" +#include "content/public/browser/resource_context.h" + +using content::BrowserThread; + +namespace { + +// Relays callback to the right message loop. +void DidGetCertDBOnIOThread( + scoped_refptr<base::MessageLoopProxy> response_message_loop, + const base::Callback<void(net::NSSCertDatabase*)>& callback, + net::NSSCertDatabase* cert_db) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + + response_message_loop->PostTask(FROM_HERE, base::Bind(callback, cert_db)); +} + +// Gets NSSCertDatabase for the resource context. +void GetCertDBOnIOThread( + content::ResourceContext* context, + scoped_refptr<base::MessageLoopProxy> response_message_loop, + const base::Callback<void(net::NSSCertDatabase*)>& callback) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); + + // Note that the callback will be used only if the cert database hasn't yet + // been initialized. + net::NSSCertDatabase* cert_db = GetNSSCertDatabaseForResourceContext( + context, + base::Bind(&DidGetCertDBOnIOThread, response_message_loop, callback)); + + if (cert_db) + DidGetCertDBOnIOThread(response_message_loop, callback, cert_db); +} + +} // namespace + +void GetNSSCertDatabaseForProfile( + Profile* profile, + const base::Callback<void(net::NSSCertDatabase*)>& callback) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + BrowserThread::PostTask(BrowserThread::IO, + FROM_HERE, + base::Bind(&GetCertDBOnIOThread, + profile->GetResourceContext(), + base::MessageLoopProxy::current(), + callback)); +} + diff --git a/chrome/browser/net/nss_context.h b/chrome/browser/net/nss_context.h index 53eab54..feb178c 100644 --- a/chrome/browser/net/nss_context.h +++ b/chrome/browser/net/nss_context.h @@ -11,6 +11,8 @@ #include "base/compiler_specific.h" #include "crypto/scoped_nss_types.h" +class Profile; + namespace net { class NSSCertDatabase; } @@ -44,4 +46,13 @@ net::NSSCertDatabase* GetNSSCertDatabaseForResourceContext( const base::Callback<void(net::NSSCertDatabase*)>& callback) WARN_UNUSED_RESULT; +// Gets a pointer to the NSSCertDatabase for the user associated with |context|. +// It's a wrapper around |GetNSSCertDatabaseForResourceContext| which makes +// sure it's called on IO thread (with |profile|'s resource context). The +// callback will be called on the originating message loop. +// It's accessing profile, so it should be called on the UI thread. +void GetNSSCertDatabaseForProfile( + Profile* profile, + const base::Callback<void(net::NSSCertDatabase*)>& callback); + #endif // CHROME_BROWSER_NET_NSS_CONTEXT_H_ diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 087e42d..0cb2356 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -1280,6 +1280,7 @@ 'browser/net/network_stats.h', 'browser/net/nss_context_chromeos.cc', 'browser/net/nss_context_linux.cc', + 'browser/net/nss_context.cc', 'browser/net/nss_context.h', 'browser/net/preconnect.cc', 'browser/net/preconnect.h', @@ -3114,6 +3115,7 @@ 'browser/certificate_manager_model.h', 'browser/net/nss_context_chromeos.cc', 'browser/net/nss_context_linux.cc', + 'browser/net/nss_context.cc', 'browser/net/nss_context.h', ], }], diff --git a/chromeos/cert_loader.cc b/chromeos/cert_loader.cc index 70e4981..23fb3c7 100644 --- a/chromeos/cert_loader.cc +++ b/chromeos/cert_loader.cc @@ -8,11 +8,13 @@ #include "base/bind.h" #include "base/location.h" +#include "base/sequenced_task_runner.h" #include "base/strings/string_number_conversions.h" #include "base/task_runner_util.h" #include "base/threading/worker_pool.h" #include "crypto/nss_util.h" #include "net/cert/nss_cert_database.h" +#include "net/cert/nss_cert_database_chromeos.h" #include "net/cert/x509_certificate.h" namespace chromeos { @@ -54,14 +56,32 @@ bool CertLoader::IsInitialized() { } CertLoader::CertLoader() - : certificates_requested_(false), - certificates_loaded_(false), + : certificates_loaded_(false), certificates_update_required_(false), certificates_update_running_(false), - tpm_token_slot_id_(-1), + database_(NULL), + force_hardware_backed_for_test_(false), weak_factory_(this) { - if (TPMTokenLoader::IsInitialized()) - TPMTokenLoader::Get()->AddObserver(this); +} + +CertLoader::~CertLoader() { + net::CertDatabase::GetInstance()->RemoveObserver(this); +} + +void CertLoader::StartWithNSSDB(net::NSSCertDatabase* database) { + CHECK(!database_); + database_ = database; + + // Start observing cert database for changes. + // Observing net::CertDatabase is preferred over observing |database_| + // directly, as |database_| observers receive only events generated directly + // by |database_|, so they may miss a few relevant ones. + // TODO(tbarzic): Once singleton NSSCertDatabase is removed, investigate if + // it would be OK to observe |database_| directly; or change NSSCertDatabase + // to send notification on all relevant changes. + net::CertDatabase::GetInstance()->AddObserver(this); + + LoadCertificates(); } void CertLoader::SetSlowTaskRunnerForTest( @@ -69,12 +89,6 @@ void CertLoader::SetSlowTaskRunnerForTest( slow_task_runner_for_test_ = task_runner; } -CertLoader::~CertLoader() { - net::CertDatabase::GetInstance()->RemoveObserver(this); - if (TPMTokenLoader::IsInitialized()) - TPMTokenLoader::Get()->RemoveObserver(this); -} - void CertLoader::AddObserver(CertLoader::Observer* observer) { observers_.AddObserver(observer); } @@ -83,12 +97,26 @@ void CertLoader::RemoveObserver(CertLoader::Observer* observer) { observers_.RemoveObserver(observer); } +int CertLoader::TPMTokenSlotID() const { + if (!database_) + return -1; + return static_cast<int>(PK11_GetSlotID(database_->GetPrivateSlot().get())); +} + bool CertLoader::IsHardwareBacked() const { - return !tpm_token_name_.empty(); + return force_hardware_backed_for_test_ || + (database_ && PK11_IsHW(database_->GetPrivateSlot().get())); +} + +bool CertLoader::IsCertificateHardwareBacked( + const net::X509Certificate* cert) const { + if (!database_) + return false; + return database_->IsHardwareBacked(cert); } bool CertLoader::CertificatesLoading() const { - return certificates_requested_ && !certificates_loaded_; + return database_ && !certificates_loaded_; } // This is copied from chrome/common/net/x509_certificate_model_nss.cc. @@ -120,16 +148,6 @@ std::string CertLoader::GetPkcs11IdForCert(const net::X509Certificate& cert) { return pkcs11_id; } -void CertLoader::RequestCertificates() { - if (certificates_requested_) - return; - certificates_requested_ = true; - - DCHECK(!certificates_loaded_ && !certificates_update_running_); - net::CertDatabase::GetInstance()->AddObserver(this); - LoadCertificates(); -} - void CertLoader::LoadCertificates() { CHECK(thread_checker_.CalledOnValidThread()); VLOG(1) << "LoadCertificates: " << certificates_update_running_; @@ -149,7 +167,14 @@ void CertLoader::LoadCertificates() { task_runner->PostTaskAndReply( FROM_HERE, base::Bind(LoadNSSCertificates, - net::NSSCertDatabase::GetInstance(), + // Create a copy of the database so it can be used on the + // worker pool. + // TODO(tbarzic): Make net::NSSCertDatabase::ListCerts async + // and change it to do the certificate listing on worker + // pool. + base::Owned(new net::NSSCertDatabaseChromeOS( + database_->GetPublicSlot(), + database_->GetPrivateSlot())), cert_list), base::Bind(&CertLoader::UpdateCertificates, weak_factory_.GetWeakPtr(), @@ -195,15 +220,4 @@ void CertLoader::OnCertRemoved(const net::X509Certificate* cert) { LoadCertificates(); } -void CertLoader::OnTPMTokenReady(const std::string& tpm_user_pin, - const std::string& tpm_token_name, - int tpm_token_slot_id) { - tpm_user_pin_ = tpm_user_pin; - tpm_token_name_ = tpm_token_name; - tpm_token_slot_id_ = tpm_token_slot_id; - - VLOG(1) << "TPM token ready."; - RequestCertificates(); -} - } // namespace chromeos diff --git a/chromeos/cert_loader.h b/chromeos/cert_loader.h index 40934c0..94f6a41 100644 --- a/chromeos/cert_loader.h +++ b/chromeos/cert_loader.h @@ -6,14 +6,15 @@ #define CHROMEOS_CERT_LOADER_H_ #include <string> +#include <vector> #include "base/basictypes.h" +#include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" #include "base/threading/thread_checker.h" #include "chromeos/chromeos_export.h" -#include "chromeos/tpm_token_loader.h" #include "net/cert/cert_database.h" namespace base { @@ -21,7 +22,9 @@ class TaskRunner; } namespace net { +class NSSCertDatabase; class X509Certificate; +typedef std::vector<scoped_refptr<X509Certificate> > CertificateList; } namespace chromeos { @@ -32,14 +35,7 @@ namespace chromeos { // When certificates have been loaded (after login completes and tpm token is // initialized), or the cert database changes, observers are called with // OnCertificatesLoaded(). -// TODO(tbarzic): Remove direct dependency on TPMTokenLoader. The reason -// TPMTokenLoader has to be observed is to make sure singleton NSS DB is -// initialized before certificate loading starts. CertLoader should use -// (primary) user specific NSS DB, whose loading already takes this into -// account (crypto::GetPrivateSlotForChromeOSUser waits until TPM token is -// ready). -class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer, - public TPMTokenLoader::Observer { +class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer { public: class Observer { public: @@ -67,6 +63,10 @@ class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer, static std::string GetPkcs11IdForCert(const net::X509Certificate& cert); + // Starts the CertLoader with the NSS cert database. + // The CertLoader will _not_ take the ownership of the database. + void StartWithNSSDB(net::NSSCertDatabase* database); + // Sets the task runner that any slow calls will be made from, e.g. calls // to the NSS database. If not set, uses base::WorkerPool. void SetSlowTaskRunnerForTest( @@ -75,9 +75,14 @@ class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer, void AddObserver(CertLoader::Observer* observer); void RemoveObserver(CertLoader::Observer* observer); - // Returns true if the TPM is available for hardware-backed certificates. + int TPMTokenSlotID() const; bool IsHardwareBacked() const; + // Whether the certificate is hardware backed. Returns false if the CertLoader + // was not yet started (both |CertificatesLoading()| and + // |certificates_loaded()| are false). + bool IsCertificateHardwareBacked(const net::X509Certificate* cert) const; + // Returns true when the certificate list has been requested but not loaded. bool CertificatesLoading() const; @@ -86,20 +91,16 @@ class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer, // This will be empty until certificates_loaded() is true. const net::CertificateList& cert_list() const { return cert_list_; } - // Getters for cached TPM token info. - std::string tpm_user_pin() const { return tpm_user_pin_; } - std::string tpm_token_name() const { return tpm_token_name_; } - int tpm_token_slot_id() const { return tpm_token_slot_id_; } + void force_hardware_backed_for_test() { + force_hardware_backed_for_test_ = true; + } private: CertLoader(); virtual ~CertLoader(); - // Starts certificate loading. - void RequestCertificates(); - // Trigger a certificate load. If a certificate loading task is already in - // progress, will start a reload once the current task finished. + // progress, will start a reload once the current task is finished. void LoadCertificates(); // Called if a certificate load task is finished. @@ -112,30 +113,27 @@ class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer, virtual void OnCertAdded(const net::X509Certificate* cert) OVERRIDE; virtual void OnCertRemoved(const net::X509Certificate* cert) OVERRIDE; - // chromeos::TPMTokenLoader::Observer - virtual void OnTPMTokenReady(const std::string& tpm_user_pin, - const std::string& tpm_token_name, - int tpm_token_slot_id) OVERRIDE; - ObserverList<Observer> observers_; // Flags describing current CertLoader state. - bool certificates_requested_; bool certificates_loaded_; bool certificates_update_required_; bool certificates_update_running_; - // Cached TPM token info. Set when the |OnTPMTokenReady| gets called. - std::string tpm_user_pin_; - std::string tpm_token_name_; - int tpm_token_slot_id_; + // The user-specific NSS certificate database from which the certificates + // should be loaded. + net::NSSCertDatabase* database_; + + // Set during tests if |IsHardwareBacked()| should always return true. + bool force_hardware_backed_for_test_; - // Cached Certificates. + // Cached Certificates loaded from the database. net::CertificateList cert_list_; base::ThreadChecker thread_checker_; - // TaskRunner for other slow tasks. May be set in tests. + // TaskRunner that, if set, replaces base::WorkerPool. Should only be set in + // tests. scoped_refptr<base::TaskRunner> slow_task_runner_for_test_; base::WeakPtrFactory<CertLoader> weak_factory_; diff --git a/chromeos/cert_loader_unittest.cc b/chromeos/cert_loader_unittest.cc new file mode 100644 index 0000000..2f28861 --- /dev/null +++ b/chromeos/cert_loader_unittest.cc @@ -0,0 +1,308 @@ +// 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 "chromeos/cert_loader.h" + +#include "base/bind.h" +#include "base/file_util.h" +#include "base/memory/scoped_ptr.h" +#include "base/message_loop/message_loop.h" +#include "base/run_loop.h" +#include "crypto/nss_util.h" +#include "crypto/nss_util_internal.h" +#include "net/base/net_errors.h" +#include "net/base/test_data_directory.h" +#include "net/cert/nss_cert_database_chromeos.h" +#include "net/cert/x509_certificate.h" +#include "net/test/cert_test_util.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace chromeos { +namespace { + +bool IsCertInCertificateList(const net::X509Certificate* cert, + const net::CertificateList& cert_list) { + for (net::CertificateList::const_iterator it = cert_list.begin(); + it != cert_list.end(); + ++it) { + if (net::X509Certificate::IsSameOSCert((*it)->os_cert_handle(), + cert->os_cert_handle())) { + return true; + } + } + return false; +} + +void FailOnPrivateSlotCallback(crypto::ScopedPK11Slot slot) { + EXPECT_FALSE(true) << "GetPrivateSlotForChromeOSUser callback called even " + << "though the private slot had been initialized."; +} + +class CertLoaderTest : public testing::Test, + public CertLoader::Observer { + public: + CertLoaderTest() : cert_loader_(NULL), + primary_user_("primary"), + certificates_loaded_events_count_(0U) { + } + + virtual ~CertLoaderTest() {} + + virtual void SetUp() OVERRIDE { + ASSERT_TRUE(primary_user_.constructed_successfully()); + ASSERT_TRUE( + crypto::GetPublicSlotForChromeOSUser(primary_user_.username_hash())); + + CertLoader::Initialize(); + cert_loader_ = CertLoader::Get(); + cert_loader_->AddObserver(this); + cert_loader_->SetSlowTaskRunnerForTest(message_loop_.message_loop_proxy()); + } + + virtual void TearDown() { + cert_loader_->RemoveObserver(this); + CertLoader::Shutdown(); + } + + protected: + void StartCertLoaderWithPrimaryUser() { + FinishUserInitAndGetDatabase(&primary_user_, &primary_db_); + cert_loader_->StartWithNSSDB(primary_db_.get()); + + base::RunLoop().RunUntilIdle(); + GetAndResetCertificatesLoadedEventsCount(); + } + + // CertLoader::Observer: + // The test keeps count of times the observer method was called. + virtual void OnCertificatesLoaded(const net::CertificateList& cert_list, + bool initial_load) OVERRIDE { + EXPECT_TRUE(certificates_loaded_events_count_ == 0 || !initial_load); + certificates_loaded_events_count_++; + } + + // Returns the number of |OnCertificatesLoaded| calls observed since the + // last call to this method equals |value|. + size_t GetAndResetCertificatesLoadedEventsCount() { + size_t result = certificates_loaded_events_count_; + certificates_loaded_events_count_ = 0; + return result; + } + + // Finishes initialization for the |user| and returns a user's NSS database + // instance. + void FinishUserInitAndGetDatabase( + crypto::ScopedTestNSSChromeOSUser* user, + scoped_ptr<net::NSSCertDatabaseChromeOS>* database) { + ASSERT_TRUE(user->constructed_successfully()); + + user->FinishInit(); + + crypto::ScopedPK11Slot private_slot( + crypto::GetPrivateSlotForChromeOSUser( + user->username_hash(), + base::Bind(&FailOnPrivateSlotCallback))); + ASSERT_TRUE(private_slot); + + database->reset(new net::NSSCertDatabaseChromeOS( + crypto::GetPublicSlotForChromeOSUser(user->username_hash()), + private_slot.Pass())); + } + + int GetDbPrivateSlotId(net::NSSCertDatabase* db) { + return static_cast<int>(PK11_GetSlotID(db->GetPrivateSlot().get())); + } + + void ImportCACert(const std::string& cert_file, + net::NSSCertDatabase* database, + net::CertificateList* imported_certs) { + ASSERT_TRUE(database); + ASSERT_TRUE(imported_certs); + + // Add a certificate to the user's db. + *imported_certs = net::CreateCertificateListFromFile( + net::GetTestCertsDirectory(), + cert_file, + net::X509Certificate::FORMAT_AUTO); + ASSERT_EQ(1U, imported_certs->size()); + + net::NSSCertDatabase::ImportCertFailureList failed; + ASSERT_TRUE(database->ImportCACerts(*imported_certs, + net::NSSCertDatabase::TRUST_DEFAULT, + &failed)); + ASSERT_TRUE(failed.empty()); + } + + void ImportClientCertAndKey(const std::string& pkcs12_file, + net::NSSCertDatabase* database, + net::CertificateList* imported_certs) { + ASSERT_TRUE(database); + ASSERT_TRUE(imported_certs); + + std::string pkcs12_data; + base::FilePath pkcs12_file_path = + net::GetTestCertsDirectory().Append(pkcs12_file); + ASSERT_TRUE(base::ReadFileToString(pkcs12_file_path, &pkcs12_data)); + + net::CertificateList client_cert_list; + scoped_refptr<net::CryptoModule> module(net::CryptoModule::CreateFromHandle( + database->GetPrivateSlot().get())); + ASSERT_EQ( + net::OK, + database->ImportFromPKCS12(module, pkcs12_data, base::string16(), false, + imported_certs)); + ASSERT_EQ(1U, imported_certs->size()); + } + + CertLoader* cert_loader_; + + // The user is primary as the one whose certificates CertLoader handles, it + // has nothing to do with crypto::InitializeNSSForChromeOSUser is_primary_user + // parameter (which is irrelevant for these tests). + crypto::ScopedTestNSSChromeOSUser primary_user_; + scoped_ptr<net::NSSCertDatabaseChromeOS> primary_db_; + + base::MessageLoop message_loop_; + + private: + size_t certificates_loaded_events_count_; +}; + +TEST_F(CertLoaderTest, Basic) { + EXPECT_FALSE(cert_loader_->CertificatesLoading()); + EXPECT_FALSE(cert_loader_->certificates_loaded()); + EXPECT_FALSE(cert_loader_->IsHardwareBacked()); + + FinishUserInitAndGetDatabase(&primary_user_, &primary_db_); + + cert_loader_->StartWithNSSDB(primary_db_.get()); + + EXPECT_FALSE(cert_loader_->certificates_loaded()); + EXPECT_TRUE(cert_loader_->CertificatesLoading()); + EXPECT_TRUE(cert_loader_->cert_list().empty()); + + ASSERT_EQ(0U, GetAndResetCertificatesLoadedEventsCount()); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(1U, GetAndResetCertificatesLoadedEventsCount()); + + EXPECT_TRUE(cert_loader_->certificates_loaded()); + EXPECT_FALSE(cert_loader_->CertificatesLoading()); + EXPECT_EQ(GetDbPrivateSlotId(primary_db_.get()), + cert_loader_->TPMTokenSlotID()); + + // Default CA cert roots should get loaded. + EXPECT_FALSE(cert_loader_->cert_list().empty()); +} + +TEST_F(CertLoaderTest, CertLoaderUpdatesCertListOnNewCert) { + StartCertLoaderWithPrimaryUser(); + + net::CertificateList certs; + ImportCACert("root_ca_cert.pem", primary_db_.get(), &certs); + + // Certs are loaded asynchronously, so the new cert should not yet be in the + // cert list. + EXPECT_FALSE(IsCertInCertificateList(certs[0], cert_loader_->cert_list())); + + ASSERT_EQ(0U, GetAndResetCertificatesLoadedEventsCount()); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(1U, GetAndResetCertificatesLoadedEventsCount()); + + // The certificate list should be updated now, as the message loop's been run. + EXPECT_TRUE(IsCertInCertificateList(certs[0], cert_loader_->cert_list())); +} + +TEST_F(CertLoaderTest, CertLoaderNoUpdateOnSecondaryDbChanges) { + crypto::ScopedTestNSSChromeOSUser secondary_user("secondary"); + scoped_ptr<net::NSSCertDatabaseChromeOS> secondary_db; + + StartCertLoaderWithPrimaryUser(); + FinishUserInitAndGetDatabase(&secondary_user, &secondary_db); + + net::CertificateList certs; + ImportCACert("root_ca_cert.pem", secondary_db.get(), &certs); + + base::RunLoop().RunUntilIdle(); + + EXPECT_FALSE(IsCertInCertificateList(certs[0], cert_loader_->cert_list())); +} + +TEST_F(CertLoaderTest, ClientLoaderUpdateOnNewClientCert) { + StartCertLoaderWithPrimaryUser(); + + net::CertificateList certs; + ImportClientCertAndKey("websocket_client_cert.p12", + primary_db_.get(), + &certs); + + ASSERT_EQ(0U, GetAndResetCertificatesLoadedEventsCount()); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(1U, GetAndResetCertificatesLoadedEventsCount()); + + EXPECT_TRUE(IsCertInCertificateList(certs[0], cert_loader_->cert_list())); +} + +TEST_F(CertLoaderTest, CertLoaderNoUpdateOnNewClientCertInSecondaryDb) { + crypto::ScopedTestNSSChromeOSUser secondary_user("secondary"); + scoped_ptr<net::NSSCertDatabaseChromeOS> secondary_db; + + StartCertLoaderWithPrimaryUser(); + FinishUserInitAndGetDatabase(&secondary_user, &secondary_db); + + net::CertificateList certs; + ImportClientCertAndKey("websocket_client_cert.p12", + secondary_db.get(), + &certs); + + base::RunLoop().RunUntilIdle(); + + EXPECT_FALSE(IsCertInCertificateList(certs[0], cert_loader_->cert_list())); +} + +TEST_F(CertLoaderTest, UpdatedOnCertRemoval) { + StartCertLoaderWithPrimaryUser(); + + net::CertificateList certs; + ImportClientCertAndKey("websocket_client_cert.p12", + primary_db_.get(), + &certs); + + base::RunLoop().RunUntilIdle(); + + ASSERT_EQ(1U, GetAndResetCertificatesLoadedEventsCount()); + ASSERT_TRUE(IsCertInCertificateList(certs[0], cert_loader_->cert_list())); + + primary_db_->DeleteCertAndKey(certs[0]); + + ASSERT_EQ(0U, GetAndResetCertificatesLoadedEventsCount()); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(1U, GetAndResetCertificatesLoadedEventsCount()); + + ASSERT_FALSE(IsCertInCertificateList(certs[0], cert_loader_->cert_list())); +} + +TEST_F(CertLoaderTest, UpdatedOnCACertTrustChange) { + StartCertLoaderWithPrimaryUser(); + + net::CertificateList certs; + ImportCACert("root_ca_cert.pem", primary_db_.get(), &certs); + + base::RunLoop().RunUntilIdle(); + ASSERT_EQ(1U, GetAndResetCertificatesLoadedEventsCount()); + ASSERT_TRUE(IsCertInCertificateList(certs[0], cert_loader_->cert_list())); + + // The value that should have been set by |ImportCACert|. + ASSERT_EQ(net::NSSCertDatabase::TRUST_DEFAULT, + primary_db_->GetCertTrust(certs[0], net::CA_CERT)); + ASSERT_TRUE(primary_db_->SetCertTrust( + certs[0], net::CA_CERT, net::NSSCertDatabase::TRUSTED_SSL)); + + // Cert trust change should trigger certificate reload in cert_loader_. + ASSERT_EQ(0U, GetAndResetCertificatesLoadedEventsCount()); + base::RunLoop().RunUntilIdle(); + EXPECT_EQ(1U, GetAndResetCertificatesLoadedEventsCount()); +} + +} // namespace +} // namespace chromeos diff --git a/chromeos/chromeos.gyp b/chromeos/chromeos.gyp index de1fa56..33cc695 100644 --- a/chromeos/chromeos.gyp +++ b/chromeos/chromeos.gyp @@ -471,6 +471,7 @@ 'app_mode/kiosk_oem_manifest_parser_unittest.cc', 'attestation/attestation_flow_unittest.cc', 'audio/cras_audio_handler_unittest.cc', + 'cert_loader_unittest.cc', 'cryptohome/system_salt_getter_unittest.cc', 'dbus/blocking_method_caller_unittest.cc', 'dbus/cros_disks_client_unittest.cc', diff --git a/chromeos/network/client_cert_resolver.cc b/chromeos/network/client_cert_resolver.cc index 018609f..52c6612a 100644 --- a/chromeos/network/client_cert_resolver.cc +++ b/chromeos/network/client_cert_resolver.cc @@ -27,6 +27,7 @@ #include "chromeos/network/managed_network_configuration_handler.h" #include "chromeos/network/network_state_handler.h" #include "chromeos/network/network_ui_data.h" +#include "chromeos/tpm_token_loader.h" #include "components/onc/onc_constants.h" #include "dbus/object_path.h" #include "net/cert/x509_certificate.h" @@ -436,8 +437,8 @@ void ClientCertResolver::ConfigureCertificates(NetworkCertMatches* matches) { base::DictionaryValue shill_properties; client_cert::SetShillProperties( it->cert_config_type, - base::IntToString(cert_loader->tpm_token_slot_id()), - cert_loader->tpm_user_pin(), + base::IntToString(cert_loader->TPMTokenSlotID()), + TPMTokenLoader::Get()->tpm_user_pin(), &it->pkcs11_id, &shill_properties); DBusThreadManager::Get()->GetShillServiceClient()-> diff --git a/chromeos/network/client_cert_resolver_unittest.cc b/chromeos/network/client_cert_resolver_unittest.cc index abcdfd4..d3d7b59 100644 --- a/chromeos/network/client_cert_resolver_unittest.cc +++ b/chromeos/network/client_cert_resolver_unittest.cc @@ -16,17 +16,17 @@ #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/shill_profile_client.h" #include "chromeos/dbus/shill_service_client.h" -#include "chromeos/login/login_state.h" #include "chromeos/network/managed_network_configuration_handler_impl.h" #include "chromeos/network/network_configuration_handler.h" #include "chromeos/network/network_profile_handler.h" #include "chromeos/network/network_state_handler.h" #include "chromeos/tpm_token_loader.h" #include "crypto/nss_util.h" +#include "crypto/nss_util_internal.h" #include "net/base/crypto_module.h" #include "net/base/net_errors.h" #include "net/base/test_data_directory.h" -#include "net/cert/nss_cert_database.h" +#include "net/cert/nss_cert_database_chromeos.h" #include "net/cert/x509_certificate.h" #include "net/test/cert_test_util.h" #include "testing/gtest/include/gtest/gtest.h" @@ -45,33 +45,42 @@ const char* kUserHash = "user_hash"; class ClientCertResolverTest : public testing::Test { public: - ClientCertResolverTest() {} + ClientCertResolverTest() : service_test_(NULL), + profile_test_(NULL), + user_(kUserHash) { + } virtual ~ClientCertResolverTest() {} virtual void SetUp() OVERRIDE { - ASSERT_TRUE(test_nssdb_.is_open()); - slot_ = net::NSSCertDatabase::GetInstance()->GetPublicModule(); - ASSERT_TRUE(slot_->os_module_handle()); - - LoginState::Initialize(); + // Initialize NSS db for the user. + ASSERT_TRUE(user_.constructed_successfully()); + user_.FinishInit(); + private_slot_ = crypto::GetPrivateSlotForChromeOSUser( + user_.username_hash(), + base::Callback<void(crypto::ScopedPK11Slot)>()); + ASSERT_TRUE(private_slot_.get()); + test_nssdb_.reset(new net::NSSCertDatabaseChromeOS( + crypto::GetPublicSlotForChromeOSUser(user_.username_hash()), + crypto::GetPrivateSlotForChromeOSUser( + user_.username_hash(), + base::Callback<void(crypto::ScopedPK11Slot)>()))); DBusThreadManager::InitializeWithStub(); service_test_ = DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface(); profile_test_ = DBusThreadManager::Get()->GetShillProfileClient()->GetTestInterface(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); service_test_->ClearServices(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); - TPMTokenLoader::Initialize(); - TPMTokenLoader* tpm_token_loader = TPMTokenLoader::Get(); - tpm_token_loader->InitializeTPMForTest(); - tpm_token_loader->SetCryptoTaskRunner(message_loop_.message_loop_proxy()); + TPMTokenLoader::InitializeForTest(); CertLoader::Initialize(); - CertLoader::Get()->SetSlowTaskRunnerForTest( - message_loop_.message_loop_proxy()); + CertLoader* cert_loader_ = CertLoader::Get(); + cert_loader_->SetSlowTaskRunnerForTest(message_loop_.message_loop_proxy()); + cert_loader_->force_hardware_backed_for_test(); + cert_loader_->StartWithNSSDB(test_nssdb_.get()); } virtual void TearDown() OVERRIDE { @@ -83,7 +92,6 @@ class ClientCertResolverTest : public testing::Test { CertLoader::Shutdown(); TPMTokenLoader::Shutdown(); DBusThreadManager::Shutdown(); - LoginState::Shutdown(); CleanupSlotContents(); } @@ -93,14 +101,13 @@ class ClientCertResolverTest : public testing::Test { // |test_pkcs11_id_|. void SetupTestCerts() { // Import a CA cert. - net::NSSCertDatabase* cert_db = net::NSSCertDatabase::GetInstance(); net::CertificateList ca_cert_list = net::CreateCertificateListFromFile(net::GetTestCertsDirectory(), "websocket_cacert.pem", net::X509Certificate::FORMAT_AUTO); ASSERT_TRUE(!ca_cert_list.empty()); net::NSSCertDatabase::ImportCertFailureList failures; - EXPECT_TRUE(cert_db->ImportCACerts( + EXPECT_TRUE(test_nssdb_->ImportCACerts( ca_cert_list, net::NSSCertDatabase::TRUST_DEFAULT, &failures)); ASSERT_TRUE(failures.empty()) << net::ErrorToString(failures[0].net_error); @@ -109,19 +116,18 @@ class ClientCertResolverTest : public testing::Test { ASSERT_TRUE(!test_ca_cert_pem_.empty()); // Import a client cert signed by that CA. - scoped_refptr<net::CryptoModule> crypt_module = cert_db->GetPrivateModule(); std::string pkcs12_data; ASSERT_TRUE(base::ReadFileToString( net::GetTestCertsDirectory().Append("websocket_client_cert.p12"), &pkcs12_data)); net::CertificateList client_cert_list; - ASSERT_EQ(net::OK, - cert_db->ImportFromPKCS12(crypt_module.get(), - pkcs12_data, - base::string16(), - false, - &client_cert_list)); + scoped_refptr<net::CryptoModule> module( + net::CryptoModule::CreateFromHandle(private_slot_.get())); + ASSERT_EQ( + net::OK, + test_nssdb_->ImportFromPKCS12( + module, pkcs12_data, base::string16(), false, &client_cert_list)); ASSERT_TRUE(!client_cert_list.empty()); test_pkcs11_id_ = CertLoader::GetPkcs11IdForCert(*client_cert_list[0]); ASSERT_TRUE(!test_pkcs11_id_.empty()); @@ -219,14 +225,14 @@ class ClientCertResolverTest : public testing::Test { private: void CleanupSlotContents() { - CERTCertList* cert_list = PK11_ListCertsInSlot(slot_->os_module_handle()); + CERTCertList* cert_list = PK11_ListCertsInSlot(private_slot_.get()); for (CERTCertListNode* node = CERT_LIST_HEAD(cert_list); !CERT_LIST_END(node, cert_list); node = CERT_LIST_NEXT(node)) { scoped_refptr<net::X509Certificate> cert( net::X509Certificate::CreateFromHandle( node->cert, net::X509Certificate::OSCertHandles())); - net::NSSCertDatabase::GetInstance()->DeleteCertAndKey(cert.get()); + test_nssdb_->DeleteCertAndKey(cert.get()); } CERT_DestroyCertList(cert_list); } @@ -236,8 +242,9 @@ class ClientCertResolverTest : public testing::Test { scoped_ptr<NetworkConfigurationHandler> network_config_handler_; scoped_ptr<ManagedNetworkConfigurationHandlerImpl> managed_config_handler_; scoped_ptr<ClientCertResolver> client_cert_resolver_; - scoped_refptr<net::CryptoModule> slot_; - crypto::ScopedTestNSSDB test_nssdb_; + crypto::ScopedTestNSSChromeOSUser user_; + scoped_ptr<net::NSSCertDatabaseChromeOS> test_nssdb_; + crypto::ScopedPK11Slot private_slot_; DISALLOW_COPY_AND_ASSIGN(ClientCertResolverTest); }; @@ -245,10 +252,10 @@ class ClientCertResolverTest : public testing::Test { TEST_F(ClientCertResolverTest, NoMatchingCertificates) { SetupNetworkHandlers(); SetupPolicy(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); SetupWifi(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); // Verify that no client certificate was configured. std::string pkcs11_id; @@ -260,10 +267,10 @@ TEST_F(ClientCertResolverTest, ResolveOnInitialization) { SetupTestCerts(); SetupNetworkHandlers(); SetupPolicy(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); SetupWifi(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); // Verify that the resolver positively matched the pattern in the policy with // the test client cert and configured the network. @@ -275,11 +282,11 @@ TEST_F(ClientCertResolverTest, ResolveOnInitialization) { TEST_F(ClientCertResolverTest, ResolveAfterPolicyApplication) { SetupTestCerts(); SetupNetworkHandlers(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); // The policy will trigger the creation of a new wifi service. SetupPolicy(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); // Verify that the resolver positively matched the pattern in the policy with // the test client cert and configured the network. diff --git a/chromeos/network/client_cert_util.cc b/chromeos/network/client_cert_util.cc index a1ec8a4..a7ece77 100644 --- a/chromeos/network/client_cert_util.cc +++ b/chromeos/network/client_cert_util.cc @@ -138,18 +138,17 @@ bool CertPrincipalMatches(const IssuerSubjectPattern& pattern, } scoped_refptr<net::X509Certificate> GetCertificateMatch( - const CertificatePattern& pattern) { + const CertificatePattern& pattern, + const net::CertificateList& all_certs) { typedef std::list<scoped_refptr<net::X509Certificate> > CertificateStlList; // Start with all the certs, and narrow it down from there. - net::CertificateList all_certs; CertificateStlList matching_certs; - net::NSSCertDatabase::GetInstance()->ListCerts(&all_certs); if (all_certs.empty()) return NULL; - for (net::CertificateList::iterator iter = all_certs.begin(); + for (net::CertificateList::const_iterator iter = all_certs.begin(); iter != all_certs.end(); ++iter) { matching_certs.push_back(*iter); } diff --git a/chromeos/network/client_cert_util.h b/chromeos/network/client_cert_util.h index 1972733..5b6839d 100644 --- a/chromeos/network/client_cert_util.h +++ b/chromeos/network/client_cert_util.h @@ -6,6 +6,7 @@ #define CHROMEOS_NETWORK_CLIENT_CERT_UTIL_H_ #include <string> +#include <vector> #include "base/memory/ref_counted.h" #include "chromeos/chromeos_export.h" @@ -17,6 +18,7 @@ class DictionaryValue; namespace net { struct CertPrincipal; class X509Certificate; +typedef std::vector<scoped_refptr<X509Certificate> > CertificateList; } namespace chromeos { @@ -43,7 +45,8 @@ bool CertPrincipalMatches(const IssuerSubjectPattern& pattern, // Fetches the matching certificate that has the latest valid start date. // Returns a NULL refptr if there is no such match. CHROMEOS_EXPORT scoped_refptr<net::X509Certificate> GetCertificateMatch( - const CertificatePattern& pattern); + const CertificatePattern& pattern, + const net::CertificateList& all_certs); // If not empty, sets the TPM properties in |properties|. If |pkcs11_id| is not // NULL, also sets the ClientCertID. |cert_config_type| determines which diff --git a/chromeos/network/network_cert_migrator_unittest.cc b/chromeos/network/network_cert_migrator_unittest.cc index 247bda5..3f94607 100644 --- a/chromeos/network/network_cert_migrator_unittest.cc +++ b/chromeos/network/network_cert_migrator_unittest.cc @@ -12,14 +12,14 @@ #include "chromeos/cert_loader.h" #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/shill_service_client.h" -#include "chromeos/login/login_state.h" #include "chromeos/network/network_state_handler.h" #include "chromeos/tpm_token_loader.h" #include "crypto/nss_util.h" +#include "crypto/nss_util_internal.h" #include "net/base/crypto_module.h" #include "net/base/net_errors.h" #include "net/base/test_data_directory.h" -#include "net/cert/nss_cert_database.h" +#include "net/cert/nss_cert_database_chromeos.h" #include "net/cert/x509_certificate.h" #include "net/test/cert_test_util.h" #include "testing/gtest/include/gtest/gtest.h" @@ -38,40 +38,39 @@ const char* kFakePEM = "pem"; class NetworkCertMigratorTest : public testing::Test { public: - NetworkCertMigratorTest() {} + NetworkCertMigratorTest() : service_test_(NULL), + user_("user_hash") { + } virtual ~NetworkCertMigratorTest() {} virtual void SetUp() OVERRIDE { - ASSERT_TRUE(test_nssdb_.is_open()); - slot_ = net::NSSCertDatabase::GetInstance()->GetPublicModule(); - ASSERT_TRUE(slot_->os_module_handle()); - - LoginState::Initialize(); + // Initialize NSS db for the user. + ASSERT_TRUE(user_.constructed_successfully()); + user_.FinishInit(); + test_nssdb_.reset(new net::NSSCertDatabaseChromeOS( + crypto::GetPublicSlotForChromeOSUser(user_.username_hash()), + crypto::GetPrivateSlotForChromeOSUser( + user_.username_hash(), + base::Callback<void(crypto::ScopedPK11Slot)>()))); DBusThreadManager::InitializeWithStub(); service_test_ = DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); service_test_->ClearServices(); - message_loop_.RunUntilIdle(); - - TPMTokenLoader::Initialize(); - TPMTokenLoader* tpm_token_loader = TPMTokenLoader::Get(); - tpm_token_loader->InitializeTPMForTest(); - tpm_token_loader->SetCryptoTaskRunner(message_loop_.message_loop_proxy()); + base::RunLoop().RunUntilIdle(); CertLoader::Initialize(); - CertLoader::Get()->SetSlowTaskRunnerForTest( - message_loop_.message_loop_proxy()); + CertLoader* cert_loader_ = CertLoader::Get(); + cert_loader_->SetSlowTaskRunnerForTest(message_loop_.message_loop_proxy()); + cert_loader_->StartWithNSSDB(test_nssdb_.get()); } virtual void TearDown() OVERRIDE { network_cert_migrator_.reset(); network_state_handler_.reset(); CertLoader::Shutdown(); - TPMTokenLoader::Shutdown(); DBusThreadManager::Shutdown(); - LoginState::Shutdown(); CleanupTestCert(); } @@ -91,11 +90,10 @@ class NetworkCertMigratorTest : public testing::Test { test_ca_cert_ = net::X509Certificate::CreateFromBytesWithNickname( der_encoded.data(), der_encoded.size(), kNSSNickname); - net::NSSCertDatabase* cert_database = net::NSSCertDatabase::GetInstance(); net::CertificateList cert_list; cert_list.push_back(test_ca_cert_); net::NSSCertDatabase::ImportCertFailureList failures; - EXPECT_TRUE(cert_database->ImportCACerts( + EXPECT_TRUE(test_nssdb_->ImportCACerts( cert_list, net::NSSCertDatabase::TRUST_DEFAULT, &failures)); ASSERT_TRUE(failures.empty()) << net::ErrorToString(failures[0].net_error); } @@ -181,14 +179,13 @@ class NetworkCertMigratorTest : public testing::Test { private: void CleanupTestCert() { - ASSERT_TRUE(net::NSSCertDatabase::GetInstance()->DeleteCertAndKey( - test_ca_cert_.get())); + ASSERT_TRUE(test_nssdb_->DeleteCertAndKey(test_ca_cert_.get())); } scoped_ptr<NetworkStateHandler> network_state_handler_; scoped_ptr<NetworkCertMigrator> network_cert_migrator_; - scoped_refptr<net::CryptoModule> slot_; - crypto::ScopedTestNSSDB test_nssdb_; + crypto::ScopedTestNSSChromeOSUser user_; + scoped_ptr<net::NSSCertDatabaseChromeOS> test_nssdb_; DISALLOW_COPY_AND_ASSIGN(NetworkCertMigratorTest); }; @@ -199,7 +196,7 @@ TEST_F(NetworkCertMigratorTest, MigrateNssOnInitialization) { SetupTestCACert(); SetupNetworkHandlers(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); std::string nss_nickname, ca_pem; GetEapCACertProperties(&nss_nickname, &ca_pem); EXPECT_TRUE(nss_nickname.empty()); @@ -209,12 +206,12 @@ TEST_F(NetworkCertMigratorTest, MigrateNssOnInitialization) { TEST_F(NetworkCertMigratorTest, MigrateNssOnNetworkAppearance) { SetupTestCACert(); SetupNetworkHandlers(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); // Add a new network for migration after the handlers are initialized. SetupWifiWithNss(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); std::string nss_nickname, ca_pem; GetEapCACertProperties(&nss_nickname, &ca_pem); EXPECT_TRUE(nss_nickname.empty()); @@ -231,7 +228,7 @@ TEST_F(NetworkCertMigratorTest, DoNotMigrateNssIfPemSet) { SetupTestCACert(); SetupNetworkHandlers(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); std::string nss_nickname, ca_pem; GetEapCACertProperties(&nss_nickname, &ca_pem); @@ -246,7 +243,7 @@ TEST_F(NetworkCertMigratorTest, MigrateOpenVpn) { SetupTestCACert(); SetupNetworkHandlers(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); std::string nss_nickname, ca_pem; GetVpnCACertProperties(true /* OpenVPN */, &nss_nickname, &ca_pem); EXPECT_TRUE(nss_nickname.empty()); @@ -260,12 +257,11 @@ TEST_F(NetworkCertMigratorTest, MigrateIpsecVpn) { SetupTestCACert(); SetupNetworkHandlers(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); std::string nss_nickname, ca_pem; GetVpnCACertProperties(false /* not OpenVPN */, &nss_nickname, &ca_pem); EXPECT_TRUE(nss_nickname.empty()); EXPECT_EQ(test_ca_cert_pem_, ca_pem); } - } // namespace chromeos diff --git a/chromeos/network/network_connection_handler.cc b/chromeos/network/network_connection_handler.cc index 32de721..79f90a8 100644 --- a/chromeos/network/network_connection_handler.cc +++ b/chromeos/network/network_connection_handler.cc @@ -9,6 +9,7 @@ #include "base/json/json_reader.h" #include "base/location.h" #include "base/strings/string_number_conversions.h" +#include "chromeos/cert_loader.h" #include "chromeos/chromeos_switches.h" #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/shill_manager_client.h" @@ -22,6 +23,7 @@ #include "chromeos/network/network_state_handler.h" #include "chromeos/network/network_ui_data.h" #include "chromeos/network/shill_property_util.h" +#include "chromeos/tpm_token_loader.h" #include "dbus/object_path.h" #include "net/cert/x509_certificate.h" #include "third_party/cros_system_api/dbus/service_constants.h" @@ -161,14 +163,16 @@ void NetworkConnectionHandler::Init( LoginState::Get()->AddObserver(this); logged_in_ = LoginState::Get()->IsUserLoggedIn(); } + if (CertLoader::IsInitialized()) { cert_loader_ = CertLoader::Get(); cert_loader_->AddObserver(this); certificates_loaded_ = cert_loader_->certificates_loaded(); } else { - // TODO(stevenjb): Require a mock or stub cert_loader in tests. + // TODO(tbarzic): Require a mock or stub cert_loader in tests. certificates_loaded_ = true; } + if (network_state_handler) { network_state_handler_ = network_state_handler; network_state_handler_->AddObserver(this, FROM_HERE); @@ -451,8 +455,8 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect( // previously configured client cert. client_cert::SetShillProperties( client_cert_type, - base::IntToString(cert_loader_->tpm_token_slot_id()), - cert_loader_->tpm_user_pin(), + base::IntToString(cert_loader_->TPMTokenSlotID()), + TPMTokenLoader::Get()->tpm_user_pin(), pkcs11_id.empty() ? NULL : &pkcs11_id, &config_properties); } @@ -632,7 +636,8 @@ std::string NetworkConnectionHandler::CertificateIsConfigured( return std::string(); // Find the matching certificate. scoped_refptr<net::X509Certificate> matching_cert = - client_cert::GetCertificateMatch(ui_data->certificate_pattern()); + client_cert::GetCertificateMatch(ui_data->certificate_pattern(), + cert_loader_->cert_list()); if (!matching_cert.get()) return std::string(); return CertLoader::GetPkcs11IdForCert(*matching_cert.get()); diff --git a/chromeos/network/network_connection_handler_unittest.cc b/chromeos/network/network_connection_handler_unittest.cc index 0e991ef..ddfa807 100644 --- a/chromeos/network/network_connection_handler_unittest.cc +++ b/chromeos/network/network_connection_handler_unittest.cc @@ -5,14 +5,27 @@ #include "chromeos/network/network_connection_handler.h" #include "base/bind.h" +#include "base/callback.h" +#include "base/file_util.h" #include "base/memory/scoped_ptr.h" #include "base/message_loop/message_loop.h" +#include "base/run_loop.h" +#include "base/strings/stringprintf.h" +#include "chromeos/cert_loader.h" #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/shill_manager_client.h" #include "chromeos/dbus/shill_service_client.h" #include "chromeos/network/network_configuration_handler.h" #include "chromeos/network/network_state_handler.h" #include "chromeos/network/onc/onc_utils.h" +#include "chromeos/tpm_token_loader.h" +#include "crypto/nss_util.h" +#include "crypto/nss_util_internal.h" +#include "net/base/net_errors.h" +#include "net/base/test_data_directory.h" +#include "net/cert/nss_cert_database_chromeos.h" +#include "net/cert/x509_certificate.h" +#include "net/test/cert_test_util.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/cros_system_api/dbus/service_constants.h" @@ -33,25 +46,41 @@ namespace chromeos { class NetworkConnectionHandlerTest : public testing::Test { public: - NetworkConnectionHandlerTest() { + NetworkConnectionHandlerTest() : user_("userhash") { } virtual ~NetworkConnectionHandlerTest() { } virtual void SetUp() OVERRIDE { + ASSERT_TRUE(user_.constructed_successfully()); + user_.FinishInit(); + + test_nssdb_.reset(new net::NSSCertDatabaseChromeOS( + crypto::GetPublicSlotForChromeOSUser(user_.username_hash()), + crypto::GetPrivateSlotForChromeOSUser( + user_.username_hash(), + base::Callback<void(crypto::ScopedPK11Slot)>()))); + + TPMTokenLoader::InitializeForTest(); + + CertLoader::Initialize(); + CertLoader* cert_loader = CertLoader::Get(); + cert_loader->SetSlowTaskRunnerForTest(message_loop_.message_loop_proxy()); + cert_loader->force_hardware_backed_for_test(); + // Initialize DBusThreadManager with a stub implementation. DBusThreadManager::InitializeWithStub(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); DBusThreadManager::Get()->GetShillServiceClient()->GetTestInterface() ->ClearServices(); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); LoginState::Initialize(); network_state_handler_.reset(NetworkStateHandler::InitializeForTest()); network_configuration_handler_.reset( NetworkConfigurationHandler::InitializeForTest( network_state_handler_.get())); + network_connection_handler_.reset(new NetworkConnectionHandler); - // TODO(stevenjb): Test integration with CertLoader using a stub or mock. network_connection_handler_->Init(network_state_handler_.get(), network_configuration_handler_.get()); } @@ -60,6 +89,8 @@ class NetworkConnectionHandlerTest : public testing::Test { network_connection_handler_.reset(); network_configuration_handler_.reset(); network_state_handler_.reset(); + CertLoader::Shutdown(); + TPMTokenLoader::Shutdown(); LoginState::Shutdown(); DBusThreadManager::Shutdown(); } @@ -76,7 +107,7 @@ class NetworkConnectionHandlerTest : public testing::Test { *json_dict, base::Bind(&ConfigureCallback), base::Bind(&ConfigureErrorCallback)); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); return true; } @@ -89,7 +120,7 @@ class NetworkConnectionHandlerTest : public testing::Test { base::Bind(&NetworkConnectionHandlerTest::ErrorCallback, base::Unretained(this)), check_error_state); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); } void Disconnect(const std::string& service_path) { @@ -99,7 +130,7 @@ class NetworkConnectionHandlerTest : public testing::Test { base::Unretained(this)), base::Bind(&NetworkConnectionHandlerTest::ErrorCallback, base::Unretained(this))); - message_loop_.RunUntilIdle(); + base::RunLoop().RunUntilIdle(); } void SuccessCallback() { @@ -128,9 +159,33 @@ class NetworkConnectionHandlerTest : public testing::Test { return result; } + void StartCertLoader() { + CertLoader::Get()->StartWithNSSDB(test_nssdb_.get()); + base::RunLoop().RunUntilIdle(); + } + + void ImportClientCertAndKey(const std::string& pkcs12_file, + net::NSSCertDatabase* nssdb, + net::CertificateList* loaded_certs) { + std::string pkcs12_data; + base::FilePath pkcs12_path = + net::GetTestCertsDirectory().Append(pkcs12_file); + ASSERT_TRUE(base::ReadFileToString(pkcs12_path, &pkcs12_data)); + + scoped_refptr<net::CryptoModule> module( + net::CryptoModule::CreateFromHandle(nssdb->GetPrivateSlot().get())); + ASSERT_EQ( + net::OK, + nssdb->ImportFromPKCS12(module, pkcs12_data, base::string16(), false, + loaded_certs)); + ASSERT_EQ(1U, loaded_certs->size()); + } + scoped_ptr<NetworkStateHandler> network_state_handler_; scoped_ptr<NetworkConfigurationHandler> network_configuration_handler_; scoped_ptr<NetworkConnectionHandler> network_connection_handler_; + crypto::ScopedTestNSSChromeOSUser user_; + scoped_ptr<net::NSSCertDatabaseChromeOS> test_nssdb_; base::MessageLoopForUI message_loop_; std::string result_; @@ -191,27 +246,70 @@ TEST_F(NetworkConnectionHandlerTest, NetworkConnectionHandlerConnectFailure) { namespace { -const char* kConfigRequiresCertificate = +const char* kConfigRequiresCertificateTemplate = "{ \"GUID\": \"wifi4\", \"Type\": \"wifi\", \"Connectable\": false," " \"Security\": \"802_1x\"," " \"UIData\": \"{" " \\\"certificate_type\\\": \\\"pattern\\\"," " \\\"certificate_pattern\\\": {" - " \\\"Subject\\\": { \\\"CommonName\\\": \\\"Foo\\\" }" + " \\\"Subject\\\": {\\\"CommonName\\\": \\\"%s\\\" }" " } }\" }"; } // namespace -// Handle certificates. TODO(stevenjb): Add certificate stubs to improve -// test coverage. -TEST_F(NetworkConnectionHandlerTest, - NetworkConnectionHandlerConnectCertificate) { - EXPECT_TRUE(Configure(kConfigRequiresCertificate)); +// Handle certificates. +TEST_F(NetworkConnectionHandlerTest, ConnectCertificateMissing) { + StartCertLoader(); + + EXPECT_TRUE(Configure( + base::StringPrintf(kConfigRequiresCertificateTemplate, "unknown"))); Connect("wifi4"); EXPECT_EQ(NetworkConnectionHandler::kErrorCertificateRequired, GetResultAndReset()); } +TEST_F(NetworkConnectionHandlerTest, ConnectWithCertificateSuccess) { + StartCertLoader(); + + net::CertificateList certs; + ImportClientCertAndKey("websocket_client_cert.p12", + test_nssdb_.get(), + &certs); + + EXPECT_TRUE(Configure( + base::StringPrintf(kConfigRequiresCertificateTemplate, + certs[0]->subject().common_name.c_str()))); + + Connect("wifi4"); + EXPECT_EQ(kSuccessResult, GetResultAndReset()); +} + +TEST_F(NetworkConnectionHandlerTest, + ConnectWithCertificateRequestedBeforeCertsAreLoaded) { + net::CertificateList certs; + ImportClientCertAndKey("websocket_client_cert.p12", + test_nssdb_.get(), + &certs); + + EXPECT_TRUE(Configure( + base::StringPrintf(kConfigRequiresCertificateTemplate, + certs[0]->subject().common_name.c_str()))); + + Connect("wifi4"); + + // Connect request came before the cert loader loaded certificates, so the + // connect request should have been throttled until the certificates are + // loaded. + EXPECT_EQ("", GetResultAndReset()); + + StartCertLoader(); + + // |StartCertLoader| should have triggered certificate loading. + // When the certificates got loaded, the connection request should have + // proceeded and eventually succeeded. + EXPECT_EQ(kSuccessResult, GetResultAndReset()); +} + TEST_F(NetworkConnectionHandlerTest, NetworkConnectionHandlerDisconnectSuccess) { EXPECT_TRUE(Configure(kConfigConnected)); diff --git a/chromeos/tpm_token_loader.cc b/chromeos/tpm_token_loader.cc index 6d1bd53..f48fb16 100644 --- a/chromeos/tpm_token_loader.cc +++ b/chromeos/tpm_token_loader.cc @@ -55,7 +55,13 @@ static TPMTokenLoader* g_tpm_token_loader = NULL; // static void TPMTokenLoader::Initialize() { CHECK(!g_tpm_token_loader); - g_tpm_token_loader = new TPMTokenLoader(); + g_tpm_token_loader = new TPMTokenLoader(false /*for_test*/); +} + +// static +void TPMTokenLoader::InitializeForTest() { + CHECK(!g_tpm_token_loader); + g_tpm_token_loader = new TPMTokenLoader(true /*for_test*/); } // static @@ -77,19 +83,20 @@ bool TPMTokenLoader::IsInitialized() { return g_tpm_token_loader; } -TPMTokenLoader::TPMTokenLoader() - : initialize_tpm_for_test_(false), +TPMTokenLoader::TPMTokenLoader(bool for_test) + : initialized_for_test_(for_test), tpm_token_state_(TPM_STATE_UNKNOWN), tpm_request_delay_( base::TimeDelta::FromMilliseconds(kInitialRequestDelayMs)), tpm_token_slot_id_(-1), weak_factory_(this) { - if (LoginState::IsInitialized()) + if (!initialized_for_test_ && LoginState::IsInitialized()) LoginState::Get()->AddObserver(this); -} -void TPMTokenLoader::InitializeTPMForTest() { - initialize_tpm_for_test_ = true; + if (initialized_for_test_) { + tpm_token_state_ = TPM_TOKEN_INITIALIZED; + tpm_user_pin_ = "111111"; + } } void TPMTokenLoader::SetCryptoTaskRunner( @@ -99,7 +106,7 @@ void TPMTokenLoader::SetCryptoTaskRunner( } TPMTokenLoader::~TPMTokenLoader() { - if (LoginState::IsInitialized()) + if (!initialized_for_test_ && LoginState::IsInitialized()) LoginState::Get()->RemoveObserver(this); } @@ -127,14 +134,14 @@ void TPMTokenLoader::MaybeStartTokenInitialization() { if (!LoginState::IsInitialized()) return; - bool request_certificates = LoginState::Get()->IsUserLoggedIn() || + bool start_initialization = LoginState::Get()->IsUserLoggedIn() || LoginState::Get()->IsInSafeMode(); - VLOG(1) << "RequestCertificates: " << request_certificates; - if (!request_certificates) + VLOG(1) << "StartTokenInitialization: " << start_initialization; + if (!start_initialization) return; - if (!initialize_tpm_for_test_ && !base::SysInfo::IsRunningOnChromeOS()) + if (!base::SysInfo::IsRunningOnChromeOS()) tpm_token_state_ = TPM_DISABLED; // Treat TPM as disabled for guest users since they do not store certs. @@ -277,8 +284,7 @@ void TPMTokenLoader::OnTPMTokenInitialized(bool success) { } void TPMTokenLoader::NotifyTPMTokenReady() { - FOR_EACH_OBSERVER(Observer, observers_, - OnTPMTokenReady(tpm_user_pin_, tpm_token_name_, tpm_token_slot_id_)); + FOR_EACH_OBSERVER(Observer, observers_, OnTPMTokenReady()); } void TPMTokenLoader::LoggedInStateChanged() { diff --git a/chromeos/tpm_token_loader.h b/chromeos/tpm_token_loader.h index 8500d4a..9ac7a61 100644 --- a/chromeos/tpm_token_loader.h +++ b/chromeos/tpm_token_loader.h @@ -33,12 +33,8 @@ class CHROMEOS_EXPORT TPMTokenLoader : public LoginState::Observer { class Observer { public: // Called when the TPM token initialization is done or the case where TPM - // should stay disabled is detected (e.g. on guest login). If TPM is - // disabled, |tpm_user_pin|, |tpm_token_name| and |tpm_token_slot_id| will - // not be set. - virtual void OnTPMTokenReady(const std::string& tpm_user_pin, - const std::string& tpm_token_name, - int tpm_token_slot_id) = 0; + // should stay disabled is detected (e.g. on guest login). + virtual void OnTPMTokenReady() = 0; protected: virtual ~Observer() {} @@ -48,6 +44,9 @@ class CHROMEOS_EXPORT TPMTokenLoader : public LoginState::Observer { // The global instance will immediately start observing |LoginState|. static void Initialize(); + // Sets the global. stubbed out, instance. To be used in tests. + static void InitializeForTest(); + // Destroys the global instance. static void Shutdown(); @@ -57,11 +56,6 @@ class CHROMEOS_EXPORT TPMTokenLoader : public LoginState::Observer { // Returns true if the global instance has been initialized. static bool IsInitialized(); - // By default, TPMTokenLoader tries to load the TPMToken only if running - // in a ChromeOS environment. Tests can call this function after Initialize() - // and before SetCryptoTaskRunner() to enable the TPM initialization. - void InitializeTPMForTest(); - // |crypto_task_runner| is the task runner that any synchronous crypto calls // should be made from, e.g. in Chrome this is the IO thread. Must be called // after the thread is started. When called, this will attempt to start TPM @@ -75,8 +69,10 @@ class CHROMEOS_EXPORT TPMTokenLoader : public LoginState::Observer { // Checks if the TPM token in ready to be used. bool IsTPMTokenReady() const; + std::string tpm_user_pin() const { return tpm_user_pin_; } + private: - TPMTokenLoader(); + explicit TPMTokenLoader(bool for_test); virtual ~TPMTokenLoader(); // Starts tpm token initialization if the user is logged in and the crypto @@ -106,7 +102,7 @@ class CHROMEOS_EXPORT TPMTokenLoader : public LoginState::Observer { // LoginState::Observer virtual void LoggedInStateChanged() OVERRIDE; - bool initialize_tpm_for_test_; + bool initialized_for_test_; ObserverList<Observer> observers_; |