Call crypto::InitializeTPMToken on the IO thread (Take 2)

chromeos/network/cert_loader.cc

 // Copyright (c) 2013 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/network/cert_loader.h"
 
 #include <algorithm>
 
 #include "base/chromeos/chromeos_version.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/observer_list.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 "chromeos/dbus/cryptohome_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "crypto/encryptor.h"
 #include "crypto/nss_util.h"
 #include "crypto/sha2.h"
 #include "crypto/symmetric_key.h"
 #include "net/cert/nss_cert_database.h"
@@ skipping 18 matching lines @@
   if (next_delay > max_delay)
     next_delay = max_delay;
   return next_delay;
 }
 
 void LoadNSSCertificates(net::CertificateList* cert_list) {
   if (base::chromeos::IsRunningOnChromeOS())
     net::NSSCertDatabase::GetInstance()->ListCerts(cert_list);
 }
 
+void CallOpenPersistentNSSDB() {
+  // Called from crypto_task_runner_.
+  VLOG(1) << "CallOpenPersistentNSSDB";
+
+  // Ensure we've opened the user's key/certificate database.
+  crypto::OpenPersistentNSSDB();
+  if (base::chromeos::IsRunningOnChromeOS())
+    crypto::EnableTPMTokenForNSS();
+}
+
 }  // namespace
 
 CertLoader::CertLoader()
     : certificates_requested_(false),
       certificates_loaded_(false),
       certificates_update_required_(false),
       certificates_update_running_(false),
       tpm_token_state_(TPM_STATE_UNKNOWN),
       tpm_request_delay_(
           base::TimeDelta::FromMilliseconds(kInitialRequestDelayMs)),
       initialize_token_factory_(this),
       update_certificates_factory_(this) {
+}
+
+void CertLoader::Init() {
   net::CertDatabase::GetInstance()->AddObserver(this);
   if (LoginState::IsInitialized())
     LoginState::Get()->AddObserver(this);
-  RequestCertificates();
+}
+
+void CertLoader::SetCryptoTaskRunner(
+    const scoped_refptr<base::SequencedTaskRunner>& crypto_task_runner) {
+  crypto_task_runner_ = crypto_task_runner;
+  MaybeRequestCertificates();
 }
 
 CertLoader::~CertLoader() {
   net::CertDatabase::GetInstance()->RemoveObserver(this);
   if (LoginState::IsInitialized())
     LoginState::Get()->RemoveObserver(this);
 }
 
 void CertLoader::AddObserver(CertLoader::Observer* observer) {
   observers_.AddObserver(observer);
 }
 
 void CertLoader::RemoveObserver(CertLoader::Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
 bool CertLoader::CertificatesLoading() const {
   return certificates_requested_ && !certificates_loaded_;
 }
 
 bool CertLoader::IsHardwareBacked() const {
   return !tpm_token_name_.empty();
 }
 
-void CertLoader::RequestCertificates() {
+void CertLoader::MaybeRequestCertificates() {
   CHECK(thread_checker_.CalledOnValidThread());
+  if (certificates_requested_ || !crypto_task_runner_.get())
+    return;
+
   const bool logged_in = LoginState::IsInitialized() ?
       LoginState::Get()->IsUserLoggedIn() : false;
   VLOG(1) << "RequestCertificates: " << logged_in;
-  if (certificates_requested_ || !logged_in)
+  if (!logged_in)
     return;
 
   certificates_requested_ = true;
 
-  // Ensure we've opened the user's key/certificate database.
-  crypto::OpenPersistentNSSDB();
-  if (base::chromeos::IsRunningOnChromeOS())
-    crypto::EnableTPMTokenForNSS();
-
-  // This is the entry point to the TPM token initialization process, which we
-  // should do at most once.
-  DCHECK(!initialize_token_factory_.HasWeakPtrs());
+  // This is the entry point to the TPM token initialization process,

[pneubeck (no reviews), 2013/07/25 09:08:43]

This comment should be moved to the first if, as certificates_requested_ is the
more reliable test for this claim ("...at most once...").
tpm_token_state_ is updated asynchronously and can't prevent re-requests.

This DCHECK verifies that we start in the right state, which is OK.

[stevenjb, 2013/07/25 17:02:27]

Done.

+  // which we should do at most once.
+  DCHECK_EQ(tpm_token_state_, TPM_STATE_UNKNOWN);
   InitializeTokenAndLoadCertificates();
 }
 
 void CertLoader::InitializeTokenAndLoadCertificates() {
   CHECK(thread_checker_.CalledOnValidThread());
-  VLOG(1) << "InitializeTokenAndLoadCertificates";
+  VLOG(1) << "InitializeTokenAndLoadCertificates: " << tpm_token_state_;
 
   switch (tpm_token_state_) {
     case TPM_STATE_UNKNOWN: {
+      crypto_task_runner_->PostTaskAndReply(
+          FROM_HERE,
+          base::Bind(&CallOpenPersistentNSSDB),
+          base::Bind(&CertLoader::OnPersistentNSSDBOpened,
+                     initialize_token_factory_.GetWeakPtr()));
+      return;
+    }
+    case TPM_DB_OPENED: {
       DBusThreadManager::Get()->GetCryptohomeClient()->TpmIsEnabled(
-      base::Bind(&CertLoader::OnTpmIsEnabled,
-                 initialize_token_factory_.GetWeakPtr()));
+          base::Bind(&CertLoader::OnTpmIsEnabled,
+                     initialize_token_factory_.GetWeakPtr()));
       return;
     }
     case TPM_DISABLED: {
       // TPM is disabled, so proceed with empty tpm token name.
       StartLoadCertificates();
       return;
     }
     case TPM_ENABLED: {
       DBusThreadManager::Get()->GetCryptohomeClient()->Pkcs11IsTpmTokenReady(
           base::Bind(&CertLoader::OnPkcs11IsTpmTokenReady,
                      initialize_token_factory_.GetWeakPtr()));
       return;
     }
     case TPM_TOKEN_READY: {
       // Retrieve token_name_ and user_pin_ here since they will never change
       // and CryptohomeClient calls are not thread safe.
       DBusThreadManager::Get()->GetCryptohomeClient()->Pkcs11GetTpmTokenInfo(
           base::Bind(&CertLoader::OnPkcs11GetTpmTokenInfo,
                      initialize_token_factory_.GetWeakPtr()));
       return;
     }
     case TPM_TOKEN_INFO_RECEIVED: {
-      InitializeNSSForTPMToken();
-      return;
+      if (base::chromeos::IsRunningOnChromeOS()) {
+        base::PostTaskAndReplyWithResult(
+            crypto_task_runner_.get(),
+            FROM_HERE,
+            base::Bind(&crypto::InitializeTPMToken,
+                       tpm_token_name_, tpm_user_pin_),
+            base::Bind(&CertLoader::OnTPMTokenInitialized,
+                       initialize_token_factory_.GetWeakPtr()));
+        return;
+      }
+      tpm_token_state_ = TPM_TOKEN_INITIALIZED;
+      // FALL_THROUGH_INTENDED
     }
-    case TPM_TOKEN_NSS_INITIALIZED: {
+    case TPM_TOKEN_INITIALIZED: {
       StartLoadCertificates();
       return;
     }
   }
 }
 
 void CertLoader::RetryTokenInitializationLater() {
+  CHECK(thread_checker_.CalledOnValidThread());
   LOG(WARNING) << "Re-Requesting Certificates later.";
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       base::Bind(&CertLoader::InitializeTokenAndLoadCertificates,
                  initialize_token_factory_.GetWeakPtr()),
       tpm_request_delay_);
   tpm_request_delay_ = GetNextRequestDelayMs(tpm_request_delay_);
 }
 
+void CertLoader::OnPersistentNSSDBOpened() {
+  VLOG(1) << "PersistentNSSDBOpened";
+  tpm_token_state_ = TPM_DB_OPENED;
+  InitializeTokenAndLoadCertificates();
+}
+
 // For background see this discussion on dev-tech-crypto.lists.mozilla.org:
 // http://web.archiveorange.com/archive/v/6JJW7E40sypfZGtbkzxX
 //
 // NOTE: This function relies on the convention that the same PKCS#11 ID
 // is shared between a certificate and its associated private and public
 // keys.  I tried to implement this with PK11_GetLowLevelKeyIDForCert(),
 // but that always returns NULL on Chrome OS for me.
 std::string CertLoader::GetPkcs11IdForCert(
     const net::X509Certificate& cert) const {
   if (!IsHardwareBacked())
@@ skipping 56 matching lines @@
   // TODO(stevenjb): The network code expects a slot ID, not a label. See
   // crbug.com/201101. For now, use a hard coded, well known slot instead.
   const char kHardcodedTpmSlot[] = "0";
   tpm_token_slot_ = kHardcodedTpmSlot;
   tpm_user_pin_ = user_pin;
   tpm_token_state_ = TPM_TOKEN_INFO_RECEIVED;
 
   InitializeTokenAndLoadCertificates();
 }
 
-void CertLoader::InitializeNSSForTPMToken() {
-  VLOG(1) << "InitializeNSSForTPMToken";
-
-  if (base::chromeos::IsRunningOnChromeOS() &&
-      !crypto::InitializeTPMToken(tpm_token_name_, tpm_user_pin_)) {
+void CertLoader::OnTPMTokenInitialized(bool success) {
+  VLOG(1) << "OnTPMTokenInitialized: " << success;
+  if (!success) {
     RetryTokenInitializationLater();
     return;
   }
-
-  tpm_token_state_ = TPM_TOKEN_NSS_INITIALIZED;
+  tpm_token_state_ = TPM_TOKEN_INITIALIZED;
   InitializeTokenAndLoadCertificates();
 }
 
 void CertLoader::StartLoadCertificates() {
-  VLOG(1) << "StartLoadCertificates";
+  CHECK(thread_checker_.CalledOnValidThread());
+  VLOG(1) << "StartLoadCertificates: " << certificates_update_running_;
 
   if (certificates_update_running_) {
     certificates_update_required_ = true;
     return;
   }
 
   net::CertificateList* cert_list = new net::CertificateList;
   certificates_update_running_ = true;
   certificates_update_required_ = false;
   base::WorkerPool::GetTaskRunner(true /* task_is_slow */)->
@@ skipping 34 matching lines @@
   StartLoadCertificates();
 }
 
 void CertLoader::OnCertRemoved(const net::X509Certificate* cert) {
   VLOG(1) << "OnCertRemoved";
   StartLoadCertificates();
 }
 
 void CertLoader::LoggedInStateChanged(LoginState::LoggedInState state) {
   VLOG(1) << "LoggedInStateChanged: " << state;
-  RequestCertificates();
+  MaybeRequestCertificates();
 }
 
 }  // namespace chromeos