diff options
-rw-r--r-- | chrome/browser/certificate_manager_model.cc | 3 | ||||
-rw-r--r-- | chromeos/cert_loader.cc | 50 | ||||
-rw-r--r-- | chromeos/cert_loader.h | 24 | ||||
-rw-r--r-- | chromeos/cert_loader_unittest.cc | 2 | ||||
-rw-r--r-- | chromeos/network/client_cert_resolver_unittest.cc | 2 | ||||
-rw-r--r-- | chromeos/network/network_cert_migrator_unittest.cc | 2 | ||||
-rw-r--r-- | chromeos/network/network_connection_handler_unittest.cc | 2 | ||||
-rw-r--r-- | chromeos/network/onc/onc_certificate_importer_impl.cc | 3 | ||||
-rw-r--r-- | net/cert/nss_cert_database.cc | 56 | ||||
-rw-r--r-- | net/cert/nss_cert_database.h | 29 | ||||
-rw-r--r-- | net/cert/nss_cert_database_chromeos.cc | 48 | ||||
-rw-r--r-- | net/cert/nss_cert_database_chromeos.h | 13 | ||||
-rw-r--r-- | net/cert/nss_cert_database_chromeos_unittest.cc | 65 | ||||
-rw-r--r-- | net/cert/nss_cert_database_unittest.cc | 30 | ||||
-rw-r--r-- | net/cert/nss_profile_filter_chromeos.cc | 24 | ||||
-rw-r--r-- | net/cert/nss_profile_filter_chromeos.h | 23 | ||||
-rw-r--r-- | net/cert/nss_profile_filter_chromeos_unittest.cc | 10 |
17 files changed, 279 insertions, 107 deletions
diff --git a/chrome/browser/certificate_manager_model.cc b/chrome/browser/certificate_manager_model.cc index b8b43fa..96f6f3f 100644 --- a/chrome/browser/certificate_manager_model.cc +++ b/chrome/browser/certificate_manager_model.cc @@ -91,7 +91,8 @@ void CertificateManagerModel::Refresh() { void CertificateManagerModel::RefreshSlotsUnlocked() { DVLOG(1) << "refresh listing certs..."; - cert_db_->ListCerts(&cert_list_); + // TODO(tbarzic): Use async |ListCerts|. + cert_db_->ListCertsSync(&cert_list_); observer_->CertificatesRefreshed(); DVLOG(1) << "refresh finished"; } diff --git a/chromeos/cert_loader.cc b/chromeos/cert_loader.cc index 23fb3c7..7dc0b55 100644 --- a/chromeos/cert_loader.cc +++ b/chromeos/cert_loader.cc @@ -8,7 +8,7 @@ #include "base/bind.h" #include "base/location.h" -#include "base/sequenced_task_runner.h" +#include "base/message_loop/message_loop_proxy.h" #include "base/strings/string_number_conversions.h" #include "base/task_runner_util.h" #include "base/threading/worker_pool.h" @@ -19,16 +19,6 @@ namespace chromeos { -namespace { - -// Loads certificates from |cert_database| into |cert_list|. -void LoadNSSCertificates(net::NSSCertDatabase* cert_database, - net::CertificateList* cert_list) { - cert_database->ListCerts(cert_list); -} - -} // namespace - static CertLoader* g_cert_loader = NULL; // static @@ -61,6 +51,7 @@ CertLoader::CertLoader() certificates_update_running_(false), database_(NULL), force_hardware_backed_for_test_(false), + cert_list_(new net::CertificateList), weak_factory_(this) { } @@ -84,11 +75,6 @@ void CertLoader::StartWithNSSDB(net::NSSCertDatabase* database) { LoadCertificates(); } -void CertLoader::SetSlowTaskRunnerForTest( - const scoped_refptr<base::TaskRunner>& task_runner) { - slow_task_runner_for_test_ = task_runner; -} - void CertLoader::AddObserver(CertLoader::Observer* observer) { observers_.AddObserver(observer); } @@ -157,37 +143,21 @@ void CertLoader::LoadCertificates() { return; } - net::CertificateList* cert_list = new net::CertificateList; certificates_update_running_ = true; certificates_update_required_ = false; - base::TaskRunner* task_runner = slow_task_runner_for_test_.get(); - if (!task_runner) - task_runner = base::WorkerPool::GetTaskRunner(true /* task is slow */); - task_runner->PostTaskAndReply( - FROM_HERE, - base::Bind(LoadNSSCertificates, - // 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(), - base::Owned(cert_list))); -} - -void CertLoader::UpdateCertificates(net::CertificateList* cert_list) { + database_->ListCerts( + base::Bind(&CertLoader::UpdateCertificates, weak_factory_.GetWeakPtr())); +} + +void CertLoader::UpdateCertificates( + scoped_ptr<net::CertificateList> cert_list) { CHECK(thread_checker_.CalledOnValidThread()); DCHECK(certificates_update_running_); VLOG(1) << "UpdateCertificates: " << cert_list->size(); // Ignore any existing certificates. - cert_list_.swap(*cert_list); + cert_list_ = cert_list.Pass(); bool initial_load = !certificates_loaded_; certificates_loaded_ = true; @@ -200,7 +170,7 @@ void CertLoader::UpdateCertificates(net::CertificateList* cert_list) { void CertLoader::NotifyCertificatesLoaded(bool initial_load) { FOR_EACH_OBSERVER(Observer, observers_, - OnCertificatesLoaded(cert_list_, initial_load)); + OnCertificatesLoaded(*cert_list_, initial_load)); } void CertLoader::OnCACertChanged(const net::X509Certificate* cert) { diff --git a/chromeos/cert_loader.h b/chromeos/cert_loader.h index 94f6a41..848a2c8 100644 --- a/chromeos/cert_loader.h +++ b/chromeos/cert_loader.h @@ -17,10 +17,6 @@ #include "chromeos/chromeos_export.h" #include "net/cert/cert_database.h" -namespace base { -class TaskRunner; -} - namespace net { class NSSCertDatabase; class X509Certificate; @@ -64,14 +60,12 @@ 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. + // The CertLoader will _not_ take the ownership of the database, but it + // expects it to stay alive at least until the shutdown starts on the main + // thread. This assumes that |StartWithNSSDB| and other methods directly + // using |database_| are not called during shutdown. 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( - const scoped_refptr<base::TaskRunner>& task_runner); - void AddObserver(CertLoader::Observer* observer); void RemoveObserver(CertLoader::Observer* observer); @@ -89,7 +83,7 @@ class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer { bool certificates_loaded() const { return certificates_loaded_; } // This will be empty until certificates_loaded() is true. - const net::CertificateList& cert_list() const { return cert_list_; } + const net::CertificateList& cert_list() const { return *cert_list_; } void force_hardware_backed_for_test() { force_hardware_backed_for_test_ = true; @@ -104,7 +98,7 @@ class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer { void LoadCertificates(); // Called if a certificate load task is finished. - void UpdateCertificates(net::CertificateList* cert_list); + void UpdateCertificates(scoped_ptr<net::CertificateList> cert_list); void NotifyCertificatesLoaded(bool initial_load); @@ -128,14 +122,10 @@ class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer { bool force_hardware_backed_for_test_; // Cached Certificates loaded from the database. - net::CertificateList cert_list_; + scoped_ptr<net::CertificateList> cert_list_; base::ThreadChecker thread_checker_; - // 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_; DISALLOW_COPY_AND_ASSIGN(CertLoader); diff --git a/chromeos/cert_loader_unittest.cc b/chromeos/cert_loader_unittest.cc index 2f28861..57def93 100644 --- a/chromeos/cert_loader_unittest.cc +++ b/chromeos/cert_loader_unittest.cc @@ -57,7 +57,6 @@ class CertLoaderTest : public testing::Test, CertLoader::Initialize(); cert_loader_ = CertLoader::Get(); cert_loader_->AddObserver(this); - cert_loader_->SetSlowTaskRunnerForTest(message_loop_.message_loop_proxy()); } virtual void TearDown() { @@ -108,6 +107,7 @@ class CertLoaderTest : public testing::Test, database->reset(new net::NSSCertDatabaseChromeOS( crypto::GetPublicSlotForChromeOSUser(user->username_hash()), private_slot.Pass())); + (*database)->SetSlowTaskRunnerForTest(message_loop_.message_loop_proxy()); } int GetDbPrivateSlotId(net::NSSCertDatabase* db) { diff --git a/chromeos/network/client_cert_resolver_unittest.cc b/chromeos/network/client_cert_resolver_unittest.cc index d3d7b59..b3f51ca 100644 --- a/chromeos/network/client_cert_resolver_unittest.cc +++ b/chromeos/network/client_cert_resolver_unittest.cc @@ -64,6 +64,7 @@ class ClientCertResolverTest : public testing::Test { crypto::GetPrivateSlotForChromeOSUser( user_.username_hash(), base::Callback<void(crypto::ScopedPK11Slot)>()))); + test_nssdb_->SetSlowTaskRunnerForTest(message_loop_.message_loop_proxy()); DBusThreadManager::InitializeWithStub(); service_test_ = @@ -78,7 +79,6 @@ class ClientCertResolverTest : public testing::Test { CertLoader::Initialize(); 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()); } diff --git a/chromeos/network/network_cert_migrator_unittest.cc b/chromeos/network/network_cert_migrator_unittest.cc index 3f94607..0aa48e4 100644 --- a/chromeos/network/network_cert_migrator_unittest.cc +++ b/chromeos/network/network_cert_migrator_unittest.cc @@ -52,6 +52,7 @@ class NetworkCertMigratorTest : public testing::Test { crypto::GetPrivateSlotForChromeOSUser( user_.username_hash(), base::Callback<void(crypto::ScopedPK11Slot)>()))); + test_nssdb_->SetSlowTaskRunnerForTest(message_loop_.message_loop_proxy()); DBusThreadManager::InitializeWithStub(); service_test_ = @@ -62,7 +63,6 @@ class NetworkCertMigratorTest : public testing::Test { CertLoader::Initialize(); CertLoader* cert_loader_ = CertLoader::Get(); - cert_loader_->SetSlowTaskRunnerForTest(message_loop_.message_loop_proxy()); cert_loader_->StartWithNSSDB(test_nssdb_.get()); } diff --git a/chromeos/network/network_connection_handler_unittest.cc b/chromeos/network/network_connection_handler_unittest.cc index ddfa807..8725980 100644 --- a/chromeos/network/network_connection_handler_unittest.cc +++ b/chromeos/network/network_connection_handler_unittest.cc @@ -60,12 +60,12 @@ class NetworkConnectionHandlerTest : public testing::Test { crypto::GetPrivateSlotForChromeOSUser( user_.username_hash(), base::Callback<void(crypto::ScopedPK11Slot)>()))); + test_nssdb_->SetSlowTaskRunnerForTest(message_loop_.message_loop_proxy()); 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. diff --git a/chromeos/network/onc/onc_certificate_importer_impl.cc b/chromeos/network/onc/onc_certificate_importer_impl.cc index 4e46626..636ecaa 100644 --- a/chromeos/network/onc/onc_certificate_importer_impl.cc +++ b/chromeos/network/onc/onc_certificate_importer_impl.cc @@ -78,7 +78,8 @@ bool CertificateImporterImpl::ParseAndStoreCertificates( void CertificateImporterImpl::ListCertsWithNickname(const std::string& label, net::CertificateList* result) { net::CertificateList all_certs; - net::NSSCertDatabase::GetInstance()->ListCerts(&all_certs); + // TODO(tbarzic): Use async |ListCerts|. + net::NSSCertDatabase::GetInstance()->ListCertsSync(&all_certs); result->clear(); for (net::CertificateList::iterator iter = all_certs.begin(); iter != all_certs.end(); ++iter) { diff --git a/net/cert/nss_cert_database.cc b/net/cert/nss_cert_database.cc index 935b271..ed861b2 100644 --- a/net/cert/nss_cert_database.cc +++ b/net/cert/nss_cert_database.cc @@ -10,10 +10,14 @@ #include <pk11pub.h> #include <secmod.h> +#include "base/bind.h" +#include "base/callback.h" #include "base/lazy_instance.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/observer_list_threadsafe.h" +#include "base/task_runner.h" +#include "base/threading/worker_pool.h" #include "crypto/nss_util.h" #include "crypto/nss_util_internal.h" #include "crypto/scoped_nss_types.h" @@ -42,7 +46,6 @@ base::LazyInstance<NSSCertDatabase>::Leaky } // namespace - NSSCertDatabase::ImportCertFailure::ImportCertFailure( const scoped_refptr<X509Certificate>& cert, int err) @@ -71,18 +74,21 @@ NSSCertDatabase::NSSCertDatabase() NSSCertDatabase::~NSSCertDatabase() {} -void NSSCertDatabase::ListCerts(CertificateList* certs) { - certs->clear(); +void NSSCertDatabase::ListCertsSync(CertificateList* certs) { + ListCertsImpl(certs); +} - CERTCertList* cert_list = PK11_ListCerts(PK11CertListUnique, NULL); - CERTCertListNode* node; - for (node = CERT_LIST_HEAD(cert_list); - !CERT_LIST_END(node, cert_list); - node = CERT_LIST_NEXT(node)) { - certs->push_back(X509Certificate::CreateFromHandle( - node->cert, X509Certificate::OSCertHandles())); - } - CERT_DestroyCertList(cert_list); +void NSSCertDatabase::ListCerts( + const base::Callback<void(scoped_ptr<CertificateList> certs)>& callback) { + scoped_ptr<CertificateList> certs(new CertificateList()); + + // base::Pased will NULL out |certs|, so cache the underlying pointer here. + CertificateList* raw_certs = certs.get(); + GetSlowTaskRunner()->PostTaskAndReply( + FROM_HERE, + base::Bind(&NSSCertDatabase::ListCertsImpl, + base::Unretained(raw_certs)), + base::Bind(callback, base::Passed(&certs))); } crypto::ScopedPK11Slot NSSCertDatabase::GetPublicSlot() const { @@ -350,6 +356,32 @@ void NSSCertDatabase::RemoveObserver(Observer* observer) { observer_list_->RemoveObserver(observer); } +void NSSCertDatabase::SetSlowTaskRunnerForTest( + const scoped_refptr<base::TaskRunner>& task_runner) { + slow_task_runner_for_test_ = task_runner; +} + +// static +void NSSCertDatabase::ListCertsImpl(CertificateList* certs) { + certs->clear(); + + CERTCertList* cert_list = PK11_ListCerts(PK11CertListUnique, NULL); + CERTCertListNode* node; + for (node = CERT_LIST_HEAD(cert_list); + !CERT_LIST_END(node, cert_list); + node = CERT_LIST_NEXT(node)) { + certs->push_back(X509Certificate::CreateFromHandle( + node->cert, X509Certificate::OSCertHandles())); + } + CERT_DestroyCertList(cert_list); +} + +scoped_refptr<base::TaskRunner> NSSCertDatabase::GetSlowTaskRunner() const { + if (slow_task_runner_for_test_) + return slow_task_runner_for_test_; + return base::WorkerPool::GetTaskRunner(true /*task is slow*/); +} + void NSSCertDatabase::NotifyObserversOfCertAdded(const X509Certificate* cert) { observer_list_->Notify(&Observer::OnCertAdded, make_scoped_refptr(cert)); } diff --git a/net/cert/nss_cert_database.h b/net/cert/nss_cert_database.h index 0707b8d..94f1f20 100644 --- a/net/cert/nss_cert_database.h +++ b/net/cert/nss_cert_database.h @@ -9,6 +9,7 @@ #include <vector> #include "base/basictypes.h" +#include "base/callback_forward.h" #include "base/memory/ref_counted.h" #include "base/strings/string16.h" #include "crypto/scoped_nss_types.h" @@ -18,6 +19,7 @@ namespace base { template <typename T> struct DefaultLazyInstanceTraits; +class TaskRunner; } template <class ObserverType> class ObserverListThreadSafe; @@ -91,12 +93,21 @@ class NET_EXPORT NSSCertDatabase { DISTRUSTED_OBJ_SIGN = 1 << 5, }; + typedef base::Callback<void(scoped_ptr<CertificateList> certs)> + ListCertsCallback; + // DEPRECATED: See http://crbug.com/329735. static NSSCertDatabase* GetInstance(); // Get a list of unique certificates in the certificate database (one // instance of all certificates). - virtual void ListCerts(CertificateList* certs); + // DEPRECATED by |ListCerts|. See http://crbug.com/340460. + virtual void ListCertsSync(CertificateList* certs); + + // Asynchronously get a list of unique certificates in the certificate + // database (one instance of all certificates). Note that the callback may be + // run even after the database is deleted. + virtual void ListCerts(const ListCertsCallback& callback); // Get the default slot for public key data. virtual crypto::ScopedPK11Slot GetPublicSlot() const; @@ -209,10 +220,23 @@ class NET_EXPORT NSSCertDatabase { // on the same thread on which AddObserver() was called. void RemoveObserver(Observer* observer); + // Overrides task runner that's used for running slow tasks. + void SetSlowTaskRunnerForTest( + const scoped_refptr<base::TaskRunner>& task_runner); + protected: NSSCertDatabase(); virtual ~NSSCertDatabase(); + // Certificate listing implementation used by |ListCerts| and |ListCertsSync|. + // Static so it may safely be used on the worker thread. + static void ListCertsImpl(CertificateList* certs); + + // Gets task runner that should be used for slow tasks like certificate + // listing. Defaults to a base::WorkerPool runner, but may be overriden + // in tests (see SetSlowTaskRunnerForTest). + scoped_refptr<base::TaskRunner> GetSlowTaskRunner() const; + private: friend struct base::DefaultLazyInstanceTraits<NSSCertDatabase>; @@ -221,6 +245,9 @@ class NET_EXPORT NSSCertDatabase { void NotifyObserversOfCertRemoved(const X509Certificate* cert); void NotifyObserversOfCACertChanged(const X509Certificate* cert); + // Task runner that should be used in tests if set. + scoped_refptr<base::TaskRunner> slow_task_runner_for_test_; + const scoped_refptr<ObserverListThreadSafe<Observer> > observer_list_; DISALLOW_COPY_AND_ASSIGN(NSSCertDatabase); diff --git a/net/cert/nss_cert_database_chromeos.cc b/net/cert/nss_cert_database_chromeos.cc index 936287c..79e5781 100644 --- a/net/cert/nss_cert_database_chromeos.cc +++ b/net/cert/nss_cert_database_chromeos.cc @@ -7,6 +7,12 @@ #include <cert.h> #include <pk11pub.h> +#include <algorithm> + +#include "base/bind.h" +#include "base/callback.h" +#include "base/location.h" +#include "base/task_runner.h" #include "net/base/crypto_module.h" #include "net/cert/x509_certificate.h" @@ -22,18 +28,22 @@ NSSCertDatabaseChromeOS::NSSCertDatabaseChromeOS( NSSCertDatabaseChromeOS::~NSSCertDatabaseChromeOS() {} -void NSSCertDatabaseChromeOS::ListCerts(CertificateList* certs) { - NSSCertDatabase::ListCerts(certs); +void NSSCertDatabaseChromeOS::ListCertsSync(CertificateList* certs) { + ListCertsImpl(profile_filter_, certs); +} - size_t pre_size = certs->size(); - certs->erase(std::remove_if( - certs->begin(), - certs->end(), - NSSProfileFilterChromeOS::CertNotAllowedForProfilePredicate( - profile_filter_)), - certs->end()); - DVLOG(1) << "filtered " << pre_size - certs->size() << " of " << pre_size - << " certs"; +void NSSCertDatabaseChromeOS::ListCerts( + const base::Callback<void(scoped_ptr<CertificateList> certs)>& callback) { + scoped_ptr<CertificateList> certs(new CertificateList()); + + // base::Pased will NULL out |certs|, so cache the underlying pointer here. + CertificateList* raw_certs = certs.get(); + GetSlowTaskRunner()->PostTaskAndReply( + FROM_HERE, + base::Bind(&NSSCertDatabaseChromeOS::ListCertsImpl, + profile_filter_, + base::Unretained(raw_certs)), + base::Bind(callback, base::Passed(&certs))); } crypto::ScopedPK11Slot NSSCertDatabaseChromeOS::GetPublicSlot() const { @@ -62,4 +72,20 @@ void NSSCertDatabaseChromeOS::ListModules(CryptoModuleList* modules, << " modules"; } +void NSSCertDatabaseChromeOS::ListCertsImpl( + const NSSProfileFilterChromeOS& profile_filter, + CertificateList* certs) { + NSSCertDatabase::ListCertsImpl(certs); + + size_t pre_size = certs->size(); + certs->erase(std::remove_if( + certs->begin(), + certs->end(), + NSSProfileFilterChromeOS::CertNotAllowedForProfilePredicate( + profile_filter)), + certs->end()); + DVLOG(1) << "filtered " << pre_size - certs->size() << " of " << pre_size + << " certs"; +} + } // namespace net diff --git a/net/cert/nss_cert_database_chromeos.h b/net/cert/nss_cert_database_chromeos.h index 63eab07..07a1e67 100644 --- a/net/cert/nss_cert_database_chromeos.h +++ b/net/cert/nss_cert_database_chromeos.h @@ -6,7 +6,9 @@ #define NET_CERT_NSS_CERT_DATABASE_CHROMEOS_ #include "base/callback.h" +#include "base/memory/weak_ptr.h" #include "crypto/scoped_nss_types.h" +#include "net/base/net_export.h" #include "net/cert/nss_cert_database.h" #include "net/cert/nss_profile_filter_chromeos.h" @@ -19,7 +21,9 @@ class NET_EXPORT NSSCertDatabaseChromeOS : public NSSCertDatabase { virtual ~NSSCertDatabaseChromeOS(); // NSSCertDatabase implementation. - virtual void ListCerts(CertificateList* certs) OVERRIDE; + virtual void ListCertsSync(CertificateList* certs) OVERRIDE; + virtual void ListCerts(const NSSCertDatabase::ListCertsCallback& callback) + OVERRIDE; virtual crypto::ScopedPK11Slot GetPublicSlot() const OVERRIDE; virtual crypto::ScopedPK11Slot GetPrivateSlot() const OVERRIDE; virtual void ListModules(CryptoModuleList* modules, bool need_rw) const @@ -30,6 +34,13 @@ class NET_EXPORT NSSCertDatabaseChromeOS : public NSSCertDatabase { // TODO(mattm): handle trust setting correctly for certs in read-only slots. private: + // Certificate listing implementation used by |ListCerts| and |ListCertsSync|. + // The certificate list normally returned by NSSCertDatabase::ListCertsImpl + // is additionally filtered by |profile_filter|. + // Static so it may safely be used on the worker thread. + static void ListCertsImpl(const NSSProfileFilterChromeOS& profile_filter, + CertificateList* certs); + crypto::ScopedPK11Slot public_slot_; crypto::ScopedPK11Slot private_slot_; NSSProfileFilterChromeOS profile_filter_; diff --git a/net/cert/nss_cert_database_chromeos_unittest.cc b/net/cert/nss_cert_database_chromeos_unittest.cc index 465d25d..324575d 100644 --- a/net/cert/nss_cert_database_chromeos_unittest.cc +++ b/net/cert/nss_cert_database_chromeos_unittest.cc @@ -6,6 +6,7 @@ #include "base/bind.h" #include "base/callback.h" +#include "base/message_loop/message_loop_proxy.h" #include "base/run_loop.h" #include "crypto/nss_util.h" #include "crypto/nss_util_internal.h" @@ -30,6 +31,14 @@ bool IsCertInCertificateList(const X509Certificate* cert, return false; } +void SwapCertLists(CertificateList* destination, + scoped_ptr<CertificateList> source) { + ASSERT_TRUE(destination); + ASSERT_TRUE(source); + + destination->swap(*source); +} + } // namespace class NSSCertDatabaseChromeOSTest : public testing::Test, @@ -51,11 +60,13 @@ class NSSCertDatabaseChromeOSTest : public testing::Test, crypto::GetPrivateSlotForChromeOSUser( user_1_.username_hash(), base::Callback<void(crypto::ScopedPK11Slot)>()))); + db_1_->SetSlowTaskRunnerForTest(base::MessageLoopProxy::current()); db_2_.reset(new NSSCertDatabaseChromeOS( crypto::GetPublicSlotForChromeOSUser(user_2_.username_hash()), crypto::GetPrivateSlotForChromeOSUser( user_2_.username_hash(), base::Callback<void(crypto::ScopedPK11Slot)>()))); + db_2_->SetSlowTaskRunnerForTest(base::MessageLoopProxy::current()); // Add observer to CertDatabase for checking that notifications from // NSSCertDatabaseChromeOS are proxied to the CertDatabase. @@ -150,8 +161,8 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportCACerts) { // Get cert list for each user. CertificateList user_1_certlist; CertificateList user_2_certlist; - db_1_->ListCerts(&user_1_certlist); - db_2_->ListCerts(&user_2_certlist); + db_1_->ListCertsSync(&user_1_certlist); + db_2_->ListCertsSync(&user_2_certlist); // Check that the imported certs only shows up in the list for the user that // imported them. @@ -170,6 +181,22 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportCACerts) { // EXPECT_EQ(certs_1[0]->os_cert_handle(), added_ca_[0]); // EXPECT_EQ(certs_2[0]->os_cert_handle(), added_ca_[1]); EXPECT_EQ(0U, added_.size()); + + // Tests that the new certs are loaded by async ListCerts method. + CertificateList user_1_certlist_async; + CertificateList user_2_certlist_async; + db_1_->ListCerts( + base::Bind(&SwapCertLists, base::Unretained(&user_1_certlist_async))); + db_2_->ListCerts( + base::Bind(&SwapCertLists, base::Unretained(&user_2_certlist_async))); + + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(IsCertInCertificateList(certs_1[0], user_1_certlist_async)); + EXPECT_FALSE(IsCertInCertificateList(certs_1[0], user_2_certlist_async)); + + EXPECT_TRUE(IsCertInCertificateList(certs_2[0], user_2_certlist_async)); + EXPECT_FALSE(IsCertInCertificateList(certs_2[0], user_1_certlist_async)); } // Test that ImportServerCerts imports the cert to the correct slot, and that @@ -200,8 +227,8 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportServerCert) { // Get cert list for each user. CertificateList user_1_certlist; CertificateList user_2_certlist; - db_1_->ListCerts(&user_1_certlist); - db_2_->ListCerts(&user_2_certlist); + db_1_->ListCertsSync(&user_1_certlist); + db_2_->ListCertsSync(&user_2_certlist); // Check that the imported certs only shows up in the list for the user that // imported them. @@ -217,6 +244,36 @@ TEST_F(NSSCertDatabaseChromeOSTest, ImportServerCert) { // fire. Is that correct? EXPECT_EQ(0U, added_ca_.size()); EXPECT_EQ(0U, added_.size()); + + // Tests that the new certs are loaded by async ListCerts method. + CertificateList user_1_certlist_async; + CertificateList user_2_certlist_async; + db_1_->ListCerts( + base::Bind(&SwapCertLists, base::Unretained(&user_1_certlist_async))); + db_2_->ListCerts( + base::Bind(&SwapCertLists, base::Unretained(&user_2_certlist_async))); + + base::RunLoop().RunUntilIdle(); + + EXPECT_TRUE(IsCertInCertificateList(certs_1[0], user_1_certlist_async)); + EXPECT_FALSE(IsCertInCertificateList(certs_1[0], user_2_certlist_async)); + + EXPECT_TRUE(IsCertInCertificateList(certs_2[0], user_2_certlist_async)); + EXPECT_FALSE(IsCertInCertificateList(certs_2[0], user_1_certlist_async)); +} + +// Tests that There is no crash if the database is deleted while ListCerts +// is being processed on the worker pool. +TEST_F(NSSCertDatabaseChromeOSTest, NoCrashIfShutdownBeforeDoneOnWorkerPool) { + CertificateList certlist; + db_1_->ListCerts(base::Bind(&SwapCertLists, base::Unretained(&certlist))); + EXPECT_EQ(0U, certlist.size()); + + db_1_.reset(); + + base::RunLoop().RunUntilIdle(); + + EXPECT_LT(0U, certlist.size()); } } // namespace net diff --git a/net/cert/nss_cert_database_unittest.cc b/net/cert/nss_cert_database_unittest.cc index 39e1a33..342e0b9 100644 --- a/net/cert/nss_cert_database_unittest.cc +++ b/net/cert/nss_cert_database_unittest.cc @@ -8,11 +8,14 @@ #include <algorithm> +#include "base/bind.h" #include "base/file_util.h" #include "base/files/file_path.h" #include "base/lazy_instance.h" #include "base/message_loop/message_loop.h" +#include "base/message_loop/message_loop_proxy.h" #include "base/path_service.h" +#include "base/run_loop.h" #include "base/strings/string16.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" @@ -41,6 +44,16 @@ using base::ASCIIToUTF16; namespace net { +namespace { + +void SwapCertList(CertificateList* destination, + scoped_ptr<CertificateList> source) { + ASSERT_TRUE(destination); + destination->swap(*source); +} + +} // namespace + class CertDatabaseNSSTest : public testing::Test { public: virtual void SetUp() { @@ -127,11 +140,26 @@ class CertDatabaseNSSTest : public testing::Test { crypto::ScopedTestNSSDB test_nssdb_; }; +TEST_F(CertDatabaseNSSTest, ListCertsSync) { + // This test isn't terribly useful, though it will at least let valgrind test + // for leaks. + CertificateList certs; + cert_db_->ListCertsSync(&certs); + // The test DB is empty, but let's assume there will always be something in + // the other slots. + EXPECT_LT(0U, certs.size()); +} + TEST_F(CertDatabaseNSSTest, ListCerts) { // This test isn't terribly useful, though it will at least let valgrind test // for leaks. CertificateList certs; - cert_db_->ListCerts(&certs); + cert_db_->SetSlowTaskRunnerForTest(base::MessageLoopProxy::current()); + cert_db_->ListCerts(base::Bind(&SwapCertList, base::Unretained(&certs))); + EXPECT_EQ(0U, certs.size()); + + base::RunLoop().RunUntilIdle(); + // The test DB is empty, but let's assume there will always be something in // the other slots. EXPECT_LT(0U, certs.size()); diff --git a/net/cert/nss_profile_filter_chromeos.cc b/net/cert/nss_profile_filter_chromeos.cc index 4871817..906780f 100644 --- a/net/cert/nss_profile_filter_chromeos.cc +++ b/net/cert/nss_profile_filter_chromeos.cc @@ -4,9 +4,8 @@ #include "net/cert/nss_profile_filter_chromeos.h" -#include "base/bind.h" -#include "base/callback.h" #include "base/strings/stringprintf.h" +#include "net/cert/x509_certificate.h" namespace net { @@ -35,8 +34,29 @@ std::string CertSlotsString(const scoped_refptr<X509Certificate>& cert) { NSSProfileFilterChromeOS::NSSProfileFilterChromeOS() {} +NSSProfileFilterChromeOS::NSSProfileFilterChromeOS( + const NSSProfileFilterChromeOS& other) { + public_slot_.reset(other.public_slot_ ? + PK11_ReferenceSlot(other.public_slot_.get()) : + NULL); + private_slot_.reset(other.private_slot_ ? + PK11_ReferenceSlot(other.private_slot_.get()) : + NULL); +} + NSSProfileFilterChromeOS::~NSSProfileFilterChromeOS() {} +NSSProfileFilterChromeOS& NSSProfileFilterChromeOS::operator=( + const NSSProfileFilterChromeOS& other) { + public_slot_.reset(other.public_slot_ ? + PK11_ReferenceSlot(other.public_slot_.get()) : + NULL); + private_slot_.reset(other.private_slot_ ? + PK11_ReferenceSlot(other.private_slot_.get()) : + NULL); + return *this; +} + void NSSProfileFilterChromeOS::Init(crypto::ScopedPK11Slot public_slot, crypto::ScopedPK11Slot private_slot) { public_slot_ = public_slot.Pass(); diff --git a/net/cert/nss_profile_filter_chromeos.h b/net/cert/nss_profile_filter_chromeos.h index d5ff818..ae310c6 100644 --- a/net/cert/nss_profile_filter_chromeos.h +++ b/net/cert/nss_profile_filter_chromeos.h @@ -2,26 +2,27 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef NET_CERT_NSS_PROFILE_FILTER_CHROMEOS -#define NET_CERT_NSS_PROFILE_FILTER_CHROMEOS +#ifndef NET_CERT_NSS_PROFILE_FILTER_CHROMEOS_H_ +#define NET_CERT_NSS_PROFILE_FILTER_CHROMEOS_H_ -#include "base/callback_forward.h" +#include "base/memory/scoped_ptr.h" #include "crypto/scoped_nss_types.h" #include "net/base/crypto_module.h" -#include "net/cert/x509_certificate.h" - -namespace content { -class ResourceContext; -} // namespace content +#include "net/base/net_export.h" namespace net { +class X509Certificate; + class NET_EXPORT NSSProfileFilterChromeOS { public: NSSProfileFilterChromeOS(); + NSSProfileFilterChromeOS(const NSSProfileFilterChromeOS& other); ~NSSProfileFilterChromeOS(); - // Initialize with slot handles. + NSSProfileFilterChromeOS& operator=(const NSSProfileFilterChromeOS& other); + + // Initializes the filter with slot handles. void Init(crypto::ScopedPK11Slot public_slot, crypto::ScopedPK11Slot private_slot); @@ -51,10 +52,8 @@ class NET_EXPORT NSSProfileFilterChromeOS { private: crypto::ScopedPK11Slot public_slot_; crypto::ScopedPK11Slot private_slot_; - - DISALLOW_COPY_AND_ASSIGN(NSSProfileFilterChromeOS); }; } // namespace net -#endif // NET_CERT_NSS_PROFILE_FILTER_CHROMEOS +#endif // NET_CERT_NSS_PROFILE_FILTER_CHROMEOS_H_ diff --git a/net/cert/nss_profile_filter_chromeos_unittest.cc b/net/cert/nss_profile_filter_chromeos_unittest.cc index 8f38061..1f6260d 100644 --- a/net/cert/nss_profile_filter_chromeos_unittest.cc +++ b/net/cert/nss_profile_filter_chromeos_unittest.cc @@ -76,6 +76,8 @@ class NSSProfileFilterChromeOSTest : public testing::Test { crypto::GetPublicSlotForChromeOSUser(user_1_.username_hash()), private_slot_1.Pass()); + profile_filter_1_copy_ = profile_filter_1_; + crypto::ScopedPK11Slot private_slot_2(crypto::GetPrivateSlotForChromeOSUser( user_2_.username_hash(), base::Callback<void(crypto::ScopedPK11Slot)>())); @@ -97,12 +99,14 @@ class NSSProfileFilterChromeOSTest : public testing::Test { NSSProfileFilterChromeOS no_slots_profile_filter_; NSSProfileFilterChromeOS profile_filter_1_; NSSProfileFilterChromeOS profile_filter_2_; + NSSProfileFilterChromeOS profile_filter_1_copy_; }; TEST_F(NSSProfileFilterChromeOSTest, TempCertAllowed) { EXPECT_EQ(NULL, certs_[0]->os_cert_handle()->slot); EXPECT_TRUE(no_slots_profile_filter_.IsCertAllowed(certs_[0])); EXPECT_TRUE(profile_filter_1_.IsCertAllowed(certs_[0])); + EXPECT_TRUE(profile_filter_1_copy_.IsCertAllowed(certs_[0])); EXPECT_TRUE(profile_filter_2_.IsCertAllowed(certs_[0])); } @@ -111,6 +115,7 @@ TEST_F(NSSProfileFilterChromeOSTest, InternalSlotAllowed) { ASSERT_TRUE(internal_slot.get()); EXPECT_TRUE(no_slots_profile_filter_.IsModuleAllowed(internal_slot.get())); EXPECT_TRUE(profile_filter_1_.IsModuleAllowed(internal_slot.get())); + EXPECT_TRUE(profile_filter_1_copy_.IsModuleAllowed(internal_slot.get())); EXPECT_TRUE(profile_filter_2_.IsModuleAllowed(internal_slot.get())); crypto::ScopedPK11Slot internal_key_slot(PK11_GetInternalKeySlot()); @@ -118,6 +123,7 @@ TEST_F(NSSProfileFilterChromeOSTest, InternalSlotAllowed) { EXPECT_TRUE( no_slots_profile_filter_.IsModuleAllowed(internal_key_slot.get())); EXPECT_TRUE(profile_filter_1_.IsModuleAllowed(internal_key_slot.get())); + EXPECT_TRUE(profile_filter_1_copy_.IsModuleAllowed(internal_key_slot.get())); EXPECT_TRUE(profile_filter_2_.IsModuleAllowed(internal_key_slot.get())); } @@ -126,12 +132,14 @@ TEST_F(NSSProfileFilterChromeOSTest, RootCertsAllowed) { ASSERT_TRUE(root_certs_slot.get()); EXPECT_TRUE(no_slots_profile_filter_.IsModuleAllowed(root_certs_slot.get())); EXPECT_TRUE(profile_filter_1_.IsModuleAllowed(root_certs_slot.get())); + EXPECT_TRUE(profile_filter_1_copy_.IsModuleAllowed(root_certs_slot.get())); EXPECT_TRUE(profile_filter_2_.IsModuleAllowed(root_certs_slot.get())); CertificateList root_certs(ListCertsInSlot(root_certs_slot.get())); ASSERT_FALSE(root_certs.empty()); EXPECT_TRUE(no_slots_profile_filter_.IsCertAllowed(root_certs[0])); EXPECT_TRUE(profile_filter_1_.IsCertAllowed(root_certs[0])); + EXPECT_TRUE(profile_filter_1_copy_.IsCertAllowed(root_certs[0])); EXPECT_TRUE(profile_filter_2_.IsCertAllowed(root_certs[0])); } @@ -167,7 +175,9 @@ TEST_F(NSSProfileFilterChromeOSTest, SoftwareSlots) { EXPECT_FALSE(no_slots_profile_filter_.IsCertAllowed(cert_2)); EXPECT_TRUE(profile_filter_1_.IsCertAllowed(cert_1)); + EXPECT_TRUE(profile_filter_1_copy_.IsCertAllowed(cert_1)); EXPECT_FALSE(profile_filter_1_.IsCertAllowed(cert_2)); + EXPECT_FALSE(profile_filter_1_copy_.IsCertAllowed(cert_2)); EXPECT_FALSE(profile_filter_2_.IsCertAllowed(cert_1)); EXPECT_TRUE(profile_filter_2_.IsCertAllowed(cert_2)); |