Skip loading system token on startup if TPM is not ready

If the TPM is not ready, don't try to load the system token. It would
fail anyway and prevent the TPM initialization process from running on
user sign-in (because TPMTokenLoader only performs the initialization
process once). Also, delay system token initialization until cryptohome
is ready.

BUG=725346,655266
TEST=Manual test on Chrome OS:
(1) On a freshly resetted device (not enrolled, not owned), the system
    token is simply not loaded on the sign-in screen
(2) On an enrolled device, the system token is loaded on the sign-in
    screen. Device-wide EAP-TLS networks can connect.
In both cases, there should be no errors related to TPM init in
  /var/log/chrome/chrome.

Change-Id: If810287747f05361ffa3a0a06fe9f2c8988ea676
Reviewed-on: https://chromium-review.googlesource.com/512662Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#473937}

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

@@ -18,6 +18,7 @@
 #include "base/files/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/linux_util.h"
+#include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/weak_ptr.h"
@@ -112,6 +113,7 @@
 #include "chromeos/cryptohome/cryptohome_parameters.h"
 #include "chromeos/cryptohome/homedir_methods.h"
 #include "chromeos/cryptohome/system_salt_getter.h"
+#include "chromeos/dbus/cryptohome_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_policy_controller.h"
 #include "chromeos/dbus/services/console_service_provider.h"
@@ -409,6 +411,42 @@ class SystemTokenCertDBInitializer {
 
   // Entry point, called on UI thread.
   void Initialize() {
+    // Only start loading the system token once cryptohome is available and only
+    // if the TPM is ready (available && owned && not being owned).
+    DBusThreadManager::Get()
+        ->GetCryptohomeClient()
+        ->WaitForServiceToBeAvailable(
+            base::Bind(&SystemTokenCertDBInitializer::OnCryptohomeAvailable,
+                       weak_ptr_factory_.GetWeakPtr()));
+  }
+
+ private:
+  // Called once the cryptohome service is available.
+  void OnCryptohomeAvailable(bool available) {
+    if (!available) {
+      LOG(ERROR) << "SystemTokenCertDBInitializer: Failed to wait for "
+                    "cryptohome to become available.";
+      return;
+    }
+
+    VLOG(1) << "SystemTokenCertDBInitializer: Cryptohome available.";
+    DBusThreadManager::Get()->GetCryptohomeClient()->TpmIsReady(
+        base::Bind(&SystemTokenCertDBInitializer::OnGotTpmIsReady,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  // This is a callback for the cryptohome TpmIsReady query. Note that this is
+  // not a listener which would be called once TPM becomes ready if it was not
+  // ready on startup (e.g. after device enrollment), see crbug.com/725500.
+  void OnGotTpmIsReady(DBusMethodCallStatus call_status, bool tpm_is_ready) {
+    if (!tpm_is_ready) {
+      VLOG(1) << "SystemTokenCertDBInitializer: TPM is not ready - not loading "
+                 "system token.";
+      return;
+    }
+    VLOG(1)
+        << "SystemTokenCertDBInitializer: TPM is ready, loading system token.";
+    TPMTokenLoader::Get()->EnsureStarted();
     base::Callback<void(crypto::ScopedPK11Slot)> callback =
         base::BindRepeating(&SystemTokenCertDBInitializer::InitializeDatabase,
                             weak_ptr_factory_.GetWeakPtr());
@@ -417,7 +455,6 @@ class SystemTokenCertDBInitializer {
         base::BindOnce(&GetSystemSlotOnIOThread, callback));
   }
 
- private:
   // Initializes the global system token NSSCertDatabase with |system_slot|.
   // Also starts CertLoader with the system token database.
   void InitializeDatabase(crypto::ScopedPK11Slot system_slot) {
@@ -434,6 +471,8 @@ class SystemTokenCertDBInitializer {
     database->SetSystemSlot(std::move(system_slot_copy));
     system_token_cert_database_ = std::move(database);
 
+    VLOG(1) << "SystemTokenCertDBInitializer: Passing system token NSS "
+               "database to CertLoader.";
     CertLoader::Get()->SetSystemNSSDB(system_token_cert_database_.get());
   }
 
@@ -541,7 +580,6 @@ void ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun() {
           content::BrowserThread::IO));
 
   // Initialize NSS database for system token.
-  TPMTokenLoader::Get()->EnsureStarted();
   system_token_certdb_initializer_ =
       base::MakeUnique<internal::SystemTokenCertDBInitializer>();
   system_token_certdb_initializer_->Initialize();

chrome/browser/chromeos/login/session/user_session_manager.cc

@@ -252,8 +252,8 @@ base::FilePath GetRlzDisabledFlagPath() {
 }
 #endif
 
-// Callback to GetNSSCertDatabaseForProfile. It starts CertLoader using the
-// provided NSS database. It must be called for primary user only.
+// Callback to GetNSSCertDatabaseForProfile. It passes the user-specific NSS
+// database to CertLoader. It must be called for primary user only.
 void OnGetNSSCertDatabaseForUser(net::NSSCertDatabase* database) {
   if (!CertLoader::IsInitialized())
     return;

chromeos/cert_loader.h

@@ -69,20 +69,20 @@ class CHROMEOS_EXPORT CertLoader {
   static std::string GetPkcs11IdAndSlotForCert(const net::X509Certificate& cert,
                                                int* slot_id);
 
-  // Starts the CertLoader with the passed system NSS cert database.
-  // The CertLoader will _not_ take ownership of the database - see comment on
-  // SetUserNSSDB.
-  // CertLoader supports working with only one database or with both (system and
-  // user) databases.
+  // Sets the NSS cert database which CertLoader should use to access system
+  // slot certificates. The CertLoader will _not_ take ownership of the database
+  // - see comment on SetUserNSSDB. CertLoader supports working with only one
+  // database or with both (system and user) databases.
   void SetSystemNSSDB(net::NSSCertDatabase* system_slot_database);
 
-  // Starts the CertLoader with the passed user NSS cert database.
+  // Sets the NSS cert database which CertLoader should use to access user slot
+  // certificates. CertLoader understands the edge case that this database could
+  // also give access to system slot certificates (e.g. for affiliated users).
   // 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 SetUserNSSDB and other methods directly
-  // using |database_| are not called during shutdown.
-  // CertLoader supports working with only one database or with both (system and
-  // user) databases.
+  // thread. This assumes that SetUserNSSDB and other methods directly using
+  // |database_| are not called during shutdown. CertLoader supports working
+  // with only one database or with both (system and user) databases.
   void SetUserNSSDB(net::NSSCertDatabase* user_database);
 
   void AddObserver(CertLoader::Observer* observer);