Use user specific NSSDatabase in CertLoader.

chromeos/cert_loader.cc

Index: chromeos/cert_loader.cc
diff --git a/chromeos/cert_loader.cc b/chromeos/cert_loader.cc
index 70e49819d0840705d1a5c2b65e0f5e93612f2e5f..0eb179119f58ff207f3e88e4c5045cb12c5a6ba3 100644
--- a/chromeos/cert_loader.cc
+++ b/chromeos/cert_loader.cc
@@ -8,19 +8,61 @@
 
 #include "base/bind.h"
 #include "base/location.h"
+#include "base/message_loop/message_loop_proxy.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 "crypto/nss_util_internal.h"
+#include "net/cert/nss_cert_database_chromeos.h"
 #include "net/cert/x509_certificate.h"
 
 namespace chromeos {
 
 namespace {
 
+typedef base::Callback<void(scoped_ptr<net::NSSCertDatabaseChromeOS> db)>
+    DatabaseCreatedCallback;
+
+void RelayToMessageLoop(
+    const scoped_refptr<base::MessageLoopProxy>& message_loop_proxy,
+    const DatabaseCreatedCallback& callback,
+    scoped_ptr<net::NSSCertDatabaseChromeOS> database) {
+  message_loop_proxy->PostTask(FROM_HERE,
+                               base::Bind(callback, base::Passed(&database)));
+}
+
+void DidGetPrivateSlot(const DatabaseCreatedCallback& callback,
+                       const std::string& userhash,
+                       crypto::ScopedPK11Slot private_slot) {
+  scoped_ptr<net::NSSCertDatabaseChromeOS> database(
+      new net::NSSCertDatabaseChromeOS(
+          crypto::GetPublicSlotForChromeOSUser(userhash),
+          private_slot.Pass()));
+  callback.Run(database.Pass());
+}
+
+void CreateDBForUser(const std::string& userhash,
+                     const DatabaseCreatedCallback& callback) {
+  CHECK(crypto::GetPublicSlotForChromeOSUser(userhash))
+      << "Trying to create user's NSS db instance before the NSS has been "
+      << "initialized for the user";
+
+  base::Callback<void(crypto::ScopedPK11Slot private_slot)> on_private_slot =
+      base::Bind(&DidGetPrivateSlot, callback, userhash);
+
+  // Note that callback passed to |crypto::GetPrivateSlotForChromeOSUser| is
+  // called only if the function returns NULL.
+  crypto::ScopedPK11Slot private_slot(
+      crypto::GetPrivateSlotForChromeOSUser(userhash, on_private_slot));
+
+  if (private_slot)
+    on_private_slot.Run(private_slot.Pass());
+}
+
 // Loads certificates from |cert_database| into |cert_list|.
-void LoadNSSCertificates(net::NSSCertDatabase* cert_database,
+void LoadNSSCertificates(net::NSSCertDatabaseChromeOS* cert_database,
                          net::CertificateList* cert_list) {
   cert_database->ListCerts(cert_list);
 }
@@ -59,36 +101,13 @@ CertLoader::CertLoader()
       certificates_update_required_(false),
       certificates_update_running_(false),
       tpm_token_slot_id_(-1),
+      is_hardware_backed_(false),
+      hardware_backed_for_test_(false),
       weak_factory_(this) {
-  if (TPMTokenLoader::IsInitialized())
-    TPMTokenLoader::Get()->AddObserver(this);
-}
-
-void CertLoader::SetSlowTaskRunnerForTest(
-    const scoped_refptr<base::TaskRunner>& task_runner) {
-  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);
-}
-
-void CertLoader::RemoveObserver(CertLoader::Observer* observer) {
-  observers_.RemoveObserver(observer);
-}
-
-bool CertLoader::IsHardwareBacked() const {
-  return !tpm_token_name_.empty();
-}
-
-bool CertLoader::CertificatesLoading() const {
-  return certificates_requested_ && !certificates_loaded_;
 }
 
 // This is copied from chrome/common/net/x509_certificate_model_nss.cc.
@@ -120,13 +139,78 @@ std::string CertLoader::GetPkcs11IdForCert(const net::X509Certificate& cert) {
   return pkcs11_id;
 }
 
+void CertLoader::StartWithUser(const std::string& userhash) {
+  VLOG(1) << "Start CertLoader with user " << userhash;
+
+  CHECK(userhash_.empty());
+  CHECK(!userhash.empty());
+  userhash_ = userhash;
+  RequestCertificates();
+}
+
+void CertLoader::SetCryptoTaskRunner(
+    const scoped_refptr<base::SequencedTaskRunner>& crypto_task_runner) {
+  crypto_task_runner_ = crypto_task_runner;
+}
+
+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);
+}
+
+void CertLoader::RemoveObserver(CertLoader::Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+bool CertLoader::IsCertificateInPrivateSlot(
+    const net::X509Certificate& cert) const {
+  if (!certificates_loaded_)
+    return false;
+  if (!cert.os_cert_handle() || !cert.os_cert_handle()->slot)
+    return false;
+  int cert_slot = static_cast<int>(PK11_GetSlotID(cert.os_cert_handle()->slot));
+  return cert_slot == tpm_token_slot_id_;
+}
+
+bool CertLoader::CertificatesLoading() const {
+  return certificates_requested_ && !certificates_loaded_;
+}
+
 void CertLoader::RequestCertificates() {
   if (certificates_requested_)
     return;
   certificates_requested_ = true;
 
+  CHECK(crypto_task_runner_) << "Crypto task runner not set.";
   DCHECK(!certificates_loaded_ && !certificates_update_running_);
+
+  DatabaseCreatedCallback on_db_ready =
+      base::Bind(&RelayToMessageLoop,
+                 base::MessageLoopProxy::current(),
+                 base::Bind(&CertLoader::OnDatabaseReady,
+                            weak_factory_.GetWeakPtr()));
+  crypto_task_runner_->PostTask(FROM_HERE,
+      base::Bind(&CreateDBForUser, userhash_, on_db_ready));
+}
+
+void CertLoader::OnDatabaseReady(
+    scoped_ptr<net::NSSCertDatabaseChromeOS> database) {
+  database_ = database.Pass();
+  tpm_token_slot_id_ =
+      static_cast<int>(PK11_GetSlotID(database_->GetPrivateSlot().get()));
+
+  int software_slot_id =
+      static_cast<int>(PK11_GetSlotID(database_->GetPublicSlot().get()));
+  is_hardware_backed_ = hardware_backed_for_test_ ||
+      tpm_token_slot_id_ != software_slot_id;
+
+  // Start observing cert database for changes.
   net::CertDatabase::GetInstance()->AddObserver(this);
+
   LoadCertificates();
 }
 
@@ -149,7 +233,11 @@ 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.
+                 base::Owned(new net::NSSCertDatabaseChromeOS(

[mattm, 2014/01/23 01:45:55]

hmm.. I think it would be preferable to have the NSSCertDatabase expose
asynchronous methods.

[tbarzic, 2014/01/23 04:45:28]

I'm not sure that by itself would help here. Unless the NSSCertDatabase ensures
ListCerts is done on the right thread.

And if we want to avoid creating new NSSCertDatabase here, we'd have to post
ListCerts on IO thread (instead of worker thread). Is that OK?

[mattm, 2014/01/23 19:46:52]

Yeah, what I meant was NSSCertDatabase would internally handle safely doing the
posting to a worker thread to implement the asynchronous call. It could take the
task runner as a constructor arg so that tests don't need to use workers.
 
Keeping this on a workerthread would be best.

[tbarzic, 2014/01/23 19:52:03]

OK, then I'll keep creating new instance NSSCertDatabaseChromeOS instance for
now and make NSSCertDatabase use Worker thread in a follow up. Sounds good?

Or would you prefer to reverse the order of patches :)

+                     database_->GetPublicSlot(),
+                     database_->GetPrivateSlot())),
                  cert_list),
       base::Bind(&CertLoader::UpdateCertificates,
                  weak_factory_.GetWeakPtr(),
@@ -195,15 +283,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