SystemTokenCertDBInitializer always use TpmManagerClient to check TPM.

we are migrating tpm status calls to TpmManagerClient.

BUG=b:172748724
TEST=unit_tests.

Change-Id: Ib5cabe3deacf4ea26f1663086b6cf9a9fcf001f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2581705Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Commit-Queue: Leo Lai <cylai@google.com>
Cr-Commit-Position: refs/heads/master@{#836482}

chrome/browser/chromeos/login/webview_login_browsertest.cc

@@ -61,8 +61,8 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "chromeos/constants/chromeos_switches.h"
-#include "chromeos/dbus/cryptohome/fake_cryptohome_client.h"
 #include "chromeos/dbus/session_manager/fake_session_manager_client.h"
+#include "chromeos/dbus/tpm_manager/fake_tpm_manager_client.h"
 #include "chromeos/dbus/tpm_manager/tpm_manager_client.h"
 #include "chromeos/tpm/tpm_token_loader.h"
 #include "components/content_settings/core/common/pref_names.h"
@@ -1161,9 +1161,15 @@ IN_PROC_BROWSER_TEST_F(WebviewClientCertsLoginTest,
 class WebviewClientCertsTokenLoadingLoginTest
     : public WebviewClientCertsLoginTestBase {
  public:
-  WebviewClientCertsTokenLoadingLoginTest()
-      : cryptohome_client_(new FakeCryptohomeClient) {
-    cryptohome_client_->set_tpm_is_ready(false);
+  WebviewClientCertsTokenLoadingLoginTest() {
+    // At very early stage, the system slot is being initialized becuase fake
+    // tpm manager tells the TPM is owned by default. So, it has to be overriden
+    // here instead of in the test body or `SetUpOnMainThread()`.
+    TpmManagerClient::InitializeFake();
+    TpmManagerClient::Get()
+        ->GetTestInterface()
+        ->mutable_nonsensitive_status_reply()
+        ->set_is_owned(false);
   }
 
   WebviewClientCertsTokenLoadingLoginTest(
@@ -1171,8 +1177,6 @@ class WebviewClientCertsTokenLoadingLoginTest
   WebviewClientCertsTokenLoadingLoginTest& operator=(
       const WebviewClientCertsTokenLoadingLoginTest&) = delete;
 
-  FakeCryptohomeClient* cryptohome_client() { return cryptohome_client_; }
-
   // Prepares a testing system slot (without injecting it as an already
   // initialized yet) and imports a client certificate into it.
   void PrepareSystemSlot() {
@@ -1229,9 +1233,6 @@ class WebviewClientCertsTokenLoadingLoginTest
     test_system_slot_nss_db_.reset();
   }
 
-  // Owned by the CryptohomeClient singleton.
-  FakeCryptohomeClient* cryptohome_client_;
-
   std::unique_ptr<crypto::ScopedTestNSSDB> test_system_slot_nss_db_;
 };
 
@@ -1278,7 +1279,10 @@ IN_PROC_BROWSER_TEST_F(WebviewClientCertsTokenLoadingLoginTest,
 
   // Report the TPM as ready, triggering the system token initialization by
   // SystemTokenCertDBInitializer.
-  cryptohome_client()->set_tpm_is_ready(true);
+  TpmManagerClient::Get()
+      ->GetTestInterface()
+      ->mutable_nonsensitive_status_reply()
+      ->set_is_owned(true);
   TpmManagerClient::Get()->GetTestInterface()->EmitOwnershipTakenSignal();
 
   const std::string https_reply_content =

chrome/browser/chromeos/system_token_cert_db_initializer.cc

@@ -207,16 +207,10 @@ void SystemTokenCertDBInitializer::OnCryptohomeAvailable(bool available) {
 }
 
 void SystemTokenCertDBInitializer::CheckTpm() {
-  if (IsSystemSlotSoftwareFallbackEnabled()) {
-    TpmManagerClient::Get()->GetTpmNonsensitiveStatus(
-        ::tpm_manager::GetTpmNonsensitiveStatusRequest(),
-        base::BindOnce(&SystemTokenCertDBInitializer::OnGetTpmStatus,
-                       weak_ptr_factory_.GetWeakPtr()));
-  } else {
-    CryptohomeClient::Get()->TpmIsReady(
-        base::BindOnce(&SystemTokenCertDBInitializer::OnGotTpmIsReady,
-                       weak_ptr_factory_.GetWeakPtr()));
-  }
+  TpmManagerClient::Get()->GetTpmNonsensitiveStatus(
+      ::tpm_manager::GetTpmNonsensitiveStatusRequest(),
+      base::BindOnce(&SystemTokenCertDBInitializer::OnGetTpmNonsensitiveStatus,
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 void SystemTokenCertDBInitializer::RetryCheckTpmLater() {
@@ -228,7 +222,7 @@ void SystemTokenCertDBInitializer::RetryCheckTpmLater() {
   tpm_request_delay_ = GetNextRequestDelay(tpm_request_delay_);
 }
 
-void SystemTokenCertDBInitializer::OnGetTpmStatus(
+void SystemTokenCertDBInitializer::OnGetTpmNonsensitiveStatus(
     const ::tpm_manager::GetTpmNonsensitiveStatusReply& reply) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
@@ -238,39 +232,35 @@ void SystemTokenCertDBInitializer::OnGetTpmStatus(
     return;
   }
 
-  // When the software fallback flag is set and the TPM is disabled, we skip the
-  // TpmIsReady() call. Otherwise, because the TPM won't be ready and will never
-  // be signaled as such, we won't proceed to the database initialization.
-  if (!reply.is_enabled()) {
+  // There are 2 cases we start initializing the database at this point: 1. TPM
+  // is ready, i.e., owned, or 2. TPM is disabled but software fallback is
+  // allowed. Note that we don't fall back to software solution as long as TPM
+  // is enabled.
+  if (reply.is_owned() ||
+      (!reply.is_enabled() && IsSystemSlotSoftwareFallbackEnabled())) {
+    VLOG_IF(1, !reply.is_owned())
+        << "Initializing database when TPM is not owned.";
     MaybeStartInitializingDatabase();
     return;
   }
 
-  CryptohomeClient::Get()->TpmIsReady(
-      base::BindOnce(&SystemTokenCertDBInitializer::OnGotTpmIsReady,
-                     weak_ptr_factory_.GetWeakPtr()));
-}
-
-void SystemTokenCertDBInitializer::OnGotTpmIsReady(
-    base::Optional<bool> tpm_is_ready) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  if (!tpm_is_ready.has_value() || !tpm_is_ready.value()) {
+  // If the TPM is enabled but not owned yet, request taking TPM initialization;
+  // when it's done, the ownership taken signal triggers database
+  // initialization.
+  if (reply.is_enabled() && !reply.is_owned()) {
     VLOG(1) << "SystemTokenCertDBInitializer: TPM is not ready - not loading "
                "system token.";
     if (ShallAttemptTpmOwnership()) {
-      // Signal to cryptohome that it can attempt TPM ownership, if it
-      // haven't done that yet. The previous signal from EULA dialogue could
-      // have been lost if initialization was interrupted.
-      // We don't care about the result, and don't block waiting for it.
-      LOG(WARNING) << "Request attempting TPM ownership.";
+      // Requests tpm manager to initialize TPM, if it haven't done that yet.
+      // The previous request from EULA dialogue could have been lost if
+      // initialization was interrupted. We don't care about the result, and
+      // don't block waiting for it.
+      LOG(WARNING) << "Request taking TPM ownership.";
       TpmManagerClient::Get()->TakeOwnership(
           ::tpm_manager::TakeOwnershipRequest(), base::DoNothing());
     }
-
     return;
   }
-  MaybeStartInitializingDatabase();
 }
 
 void SystemTokenCertDBInitializer::MaybeStartInitializingDatabase() {

chrome/browser/chromeos/system_token_cert_db_initializer.h

@@ -95,19 +95,14 @@ class SystemTokenCertDBInitializer : public TpmManagerClient::Observer {
   // schedules the initialization step retry attempt after a timeout.
   void RetryCheckTpmLater();
 
-  // This is a callback for the GetTpmNonsensitiveStatus() query. It is only
-  // called when the build flag system_slot_software_fallback is enabled. If the
-  // build flag is enabled and TPM is disabled, we skip the cryptohome
-  // TpmIsReady() check during initialization, otherwise we continue the normal
-  // flow with TpmIsReady() and its callback.
-  void OnGetTpmStatus(
+  // This is a callback for the GetTpmNonsensitiveStatus() query. 2 main
+  // operations are performed:
+  // 1. Initializes the database if TPM is owned or software fallback is
+  // enabled.
+  // 2. Triggers TPM ownership process if necessary.
+  void OnGetTpmNonsensitiveStatus(
       const ::tpm_manager::GetTpmNonsensitiveStatusReply& reply);
 
-  // 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 - that event is observed by `OnOwnershipTakenSignal()`.
-  void OnGotTpmIsReady(base::Optional<bool> tpm_is_ready);
-
   // Starts loading the system slot and initializing the corresponding NSS cert
   // database, unless it was already started before.
   void MaybeStartInitializingDatabase();

chromeos/dbus/tpm_manager/tpm_manager_client.cc

@@ -219,7 +219,10 @@ void TpmManagerClient::Initialize(dbus::Bus* bus) {
 
 // static
 void TpmManagerClient::InitializeFake() {
-  new FakeTpmManagerClient();
+  // Do not create a new instance if it was initialized early in a browser test
+  // (for early setup calls dependent on TpmManagerClient).
+  if (!FakeTpmManagerClient::Get())
+    new FakeTpmManagerClient();
 }
 
 // static