summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/certificate_manager_model.cc3
-rw-r--r--chromeos/cert_loader.cc50
-rw-r--r--chromeos/cert_loader.h24
-rw-r--r--chromeos/cert_loader_unittest.cc2
-rw-r--r--chromeos/network/client_cert_resolver_unittest.cc2
-rw-r--r--chromeos/network/network_cert_migrator_unittest.cc2
-rw-r--r--chromeos/network/network_connection_handler_unittest.cc2
-rw-r--r--chromeos/network/onc/onc_certificate_importer_impl.cc3
-rw-r--r--net/cert/nss_cert_database.cc56
-rw-r--r--net/cert/nss_cert_database.h29
-rw-r--r--net/cert/nss_cert_database_chromeos.cc48
-rw-r--r--net/cert/nss_cert_database_chromeos.h13
-rw-r--r--net/cert/nss_cert_database_chromeos_unittest.cc65
-rw-r--r--net/cert/nss_cert_database_unittest.cc30
-rw-r--r--net/cert/nss_profile_filter_chromeos.cc24
-rw-r--r--net/cert/nss_profile_filter_chromeos.h23
-rw-r--r--net/cert/nss_profile_filter_chromeos_unittest.cc10
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));