use TpmManagerClient to get tpm version info.

We are deprecating tpm-related APIs by cryptohome.

To test this CL, it is attempted to filter out all the affected test
manually:
1. For browser test, device_status_collector_browsertest.cc is confirmed
working fine.
2. For unit test, the affected tests are searched by grepping
`DeviceStatusCollector` and as a result it is confirmed that with the
following filters all the tests pass:
*StatusUploader*
*DeviceCloudPolicy*
*AffiliatedSessionService*
*ManagementUIHandler*

BUG=b:172748724
TEST=build ok.
TEST=See above for the test items.

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

chrome/browser/chromeos/policy/status_collector/device_status_collector.cc

@@ -69,6 +69,8 @@
 #include "chromeos/dbus/cryptohome/tpm_util.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power_manager/idle.pb.h"
+#include "chromeos/dbus/tpm_manager/tpm_manager.pb.h"
+#include "chromeos/dbus/tpm_manager/tpm_manager_client.h"
 #include "chromeos/dbus/update_engine_client.h"
 #include "chromeos/dbus/util/version_loader.h"
 #include "chromeos/disks/disk_mount_manager.h"
@@ -1484,8 +1486,10 @@ DeviceStatusCollector::DeviceStatusCollector(
       base::BindOnce(&ReadFirmwareVersion),
       base::BindOnce(&DeviceStatusCollector::OnOSFirmware,
                      weak_factory_.GetWeakPtr()));
-  chromeos::tpm_util::GetTpmVersion(base::BindOnce(
-      &DeviceStatusCollector::OnTpmVersion, weak_factory_.GetWeakPtr()));
+  chromeos::TpmManagerClient::Get()->GetVersionInfo(
+      ::tpm_manager::GetVersionInfoRequest(),
+      base::BindOnce(&DeviceStatusCollector::OnGetTpmVersion,
+                     weak_factory_.GetWeakPtr()));
 
   pref_change_registrar_ = std::make_unique<PrefChangeRegistrar>();
   pref_change_registrar_->Init(pref_service_);
@@ -2052,12 +2056,12 @@ bool DeviceStatusCollector::GetVersionInfo(
 
   em::TpmVersionInfo* const tpm_version_info =
       status->mutable_tpm_version_info();
-  tpm_version_info->set_family(tpm_version_info_.family);
-  tpm_version_info->set_spec_level(tpm_version_info_.spec_level);
-  tpm_version_info->set_manufacturer(tpm_version_info_.manufacturer);
-  tpm_version_info->set_tpm_model(tpm_version_info_.tpm_model);
-  tpm_version_info->set_firmware_version(tpm_version_info_.firmware_version);
-  tpm_version_info->set_vendor_specific(tpm_version_info_.vendor_specific);
+  tpm_version_info->set_family(tpm_version_reply_.family());
+  tpm_version_info->set_spec_level(tpm_version_reply_.spec_level());
+  tpm_version_info->set_manufacturer(tpm_version_reply_.manufacturer());
+  tpm_version_info->set_tpm_model(tpm_version_reply_.tpm_model());
+  tpm_version_info->set_firmware_version(tpm_version_reply_.firmware_version());
+  tpm_version_info->set_vendor_specific(tpm_version_reply_.vendor_specific());
 
   return true;
 }
@@ -2708,9 +2712,12 @@ void DeviceStatusCollector::OnOSFirmware(
   firmware_fetch_error_ = version.second;
 }
 
-void DeviceStatusCollector::OnTpmVersion(
-    const chromeos::CryptohomeClient::TpmVersionInfo& tpm_version_info) {
-  tpm_version_info_ = tpm_version_info;
+void DeviceStatusCollector::OnGetTpmVersion(
+    const ::tpm_manager::GetVersionInfoReply& reply) {
+  if (reply.status() != ::tpm_manager::STATUS_SUCCESS) {
+    LOG(WARNING) << "Failed to get tpm version; status: " << reply.status();
+  }
+  tpm_version_reply_ = reply;
 }
 
 }  // namespace policy

chrome/browser/chromeos/policy/status_collector/device_status_collector.h

@@ -17,12 +17,12 @@
 #include "base/callback_list.h"
 #include "base/compiler_specific.h"
 #include "base/containers/circular_deque.h"
-#include "base/time/default_clock.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner.h"
 #include "base/task/cancelable_task_tracker.h"
+#include "base/time/default_clock.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/chromeos/policy/status_collector/app_info_generator.h"
@@ -30,6 +30,7 @@
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chromeos/dbus/cryptohome/cryptohome_client.h"
 #include "chromeos/dbus/power/power_manager_client.h"
+#include "chromeos/dbus/tpm_manager/tpm_manager.pb.h"
 #include "chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "components/prefs/pref_member.h"
@@ -256,8 +257,9 @@ class DeviceStatusCollector : public StatusCollector,
   // Callbacks from chromeos::VersionLoader.
   void OnOSVersion(const std::string& version);
   void OnOSFirmware(std::pair<const std::string&, const std::string&> version);
-  void OnTpmVersion(
-      const chromeos::CryptohomeClient::TpmVersionInfo& tpm_version_info);
+
+  // Callbacks from `chromeos::TpmManagerClient`.
+  void OnGetTpmVersion(const ::tpm_manager::GetVersionInfoReply& reply);
 
   void GetDeviceStatus(scoped_refptr<DeviceStatusCollectorState> state);
   void GetSessionStatus(scoped_refptr<DeviceStatusCollectorState> state);
@@ -379,7 +381,7 @@ class DeviceStatusCollector : public StatusCollector,
   std::string os_version_;
   std::string firmware_version_;
   std::string firmware_fetch_error_;
-  chromeos::CryptohomeClient::TpmVersionInfo tpm_version_info_;
+  ::tpm_manager::GetVersionInfoReply tpm_version_reply_;
 
   struct ResourceUsage {
     // Sample of percentage-of-CPU-used.

chrome/browser/chromeos/policy/status_collector/device_status_collector_browsertest.cc

@@ -66,6 +66,7 @@
 #include "chromeos/dbus/shill/shill_ipconfig_client.h"
 #include "chromeos/dbus/shill/shill_profile_client.h"
 #include "chromeos/dbus/shill/shill_service_client.h"
+#include "chromeos/dbus/tpm_manager/tpm_manager_client.h"
 #include "chromeos/dbus/vm_applications/apps.pb.h"
 #include "chromeos/disks/disk_mount_manager.h"
 #include "chromeos/disks/mock_disk_mount_manager.h"
@@ -798,11 +799,13 @@ class DeviceStatusCollectorTest : public testing::Test {
     chromeos::CrasAudioHandler::InitializeForTesting();
     chromeos::CryptohomeClient::InitializeFake();
     chromeos::PowerManagerClient::InitializeFake();
+    chromeos::TpmManagerClient::InitializeFake();
     chromeos::LoginState::Initialize();
   }
 
   ~DeviceStatusCollectorTest() override {
     chromeos::LoginState::Shutdown();
+    chromeos::TpmManagerClient::Shutdown();
     chromeos::PowerManagerClient::Shutdown();
     chromeos::CryptohomeClient::Shutdown();
     chromeos::CrasAudioHandler::Shutdown();
@@ -1529,6 +1532,24 @@ TEST_F(DeviceStatusCollectorTest, VersionInfo) {
   EXPECT_TRUE(device_status_.has_firmware_version());
   EXPECT_TRUE(device_status_.has_tpm_version_info());
 
+  // Expect tpm version info is still set (with an empty one) regardless of
+  // D-Bus error.
+  chromeos::TpmManagerClient::Get()
+      ->GetTestInterface()
+      ->mutable_version_info_reply()
+      ->set_status(::tpm_manager::STATUS_DBUS_ERROR);
+  GetStatus();
+  EXPECT_TRUE(device_status_.has_browser_version());
+  EXPECT_TRUE(device_status_.has_channel());
+  EXPECT_TRUE(device_status_.has_os_version());
+  EXPECT_TRUE(device_status_.has_firmware_version());
+  EXPECT_TRUE(device_status_.has_tpm_version_info());
+  // Reset the version info reply just in case the rest of tests get affected.
+  chromeos::TpmManagerClient::Get()
+      ->GetTestInterface()
+      ->mutable_version_info_reply()
+      ->clear_status();
+
   // When the pref to collect this data is not enabled, expect that none of
   // the fields are present in the protobuf.
   scoped_testing_cros_settings_.device_settings()->SetBoolean(

chrome/browser/chromeos/policy/status_uploader_unittest.cc

@@ -17,8 +17,10 @@
 #include "chrome/browser/chromeos/policy/status_collector/device_status_collector.h"
 #include "chrome/browser/chromeos/settings/scoped_testing_cros_settings.h"
 #include "chrome/browser/chromeos/settings/stub_cros_settings_provider.h"
+#include "chromeos/dbus/cryptohome/cryptohome_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power/power_manager_client.h"
+#include "chromeos/dbus/tpm_manager/tpm_manager_client.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "components/policy/core/common/cloud/cloud_policy_client.h"
 #include "components/policy/core/common/cloud/mock_cloud_policy_client.h"
@@ -85,6 +87,7 @@ class StatusUploaderTest : public testing::Test {
 
     chromeos::CryptohomeClient::InitializeFake();
     chromeos::PowerManagerClient::InitializeFake();
+    chromeos::TpmManagerClient::InitializeFake();
     client_.SetDMToken("dm_token");
     collector_.reset(new MockDeviceStatusCollector(&prefs_));
 
@@ -95,6 +98,7 @@ class StatusUploaderTest : public testing::Test {
 
   void TearDown() override {
     content::RunAllTasksUntilIdle();
+    chromeos::TpmManagerClient::Shutdown();
     chromeos::PowerManagerClient::Shutdown();
     chromeos::CryptohomeClient::Shutdown();
     chromeos::DBusThreadManager::Shutdown();

chrome/browser/chromeos/settings/device_settings_test_helper.cc

@@ -12,8 +12,10 @@
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/cryptohome/cryptohome_parameters.h"
+#include "chromeos/dbus/cryptohome/cryptohome_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/power/fake_power_manager_client.h"
+#include "chromeos/dbus/tpm_manager/tpm_manager_client.h"
 #include "components/ownership/mock_owner_key_util.h"
 #include "components/policy/proto/chrome_device_policy.pb.h"
 #include "content/public/browser/browser_thread.h"
@@ -56,6 +58,7 @@ void DeviceSettingsTestBase::SetUp() {
   dbus_setter_ = DBusThreadManager::GetSetterForTesting();
   CryptohomeClient::InitializeFake();
   PowerManagerClient::InitializeFake();
+  TpmManagerClient::InitializeFake();
   OwnerSettingsServiceChromeOSFactory::SetDeviceSettingsServiceForTesting(
       device_settings_service_.get());
   OwnerSettingsServiceChromeOSFactory::GetInstance()->SetOwnerKeyUtilForTesting(
@@ -79,6 +82,7 @@ void DeviceSettingsTestBase::TearDown() {
   FlushDeviceSettings();
   device_settings_service_->UnsetSessionManager();
   device_settings_service_.reset();
+  TpmManagerClient::Shutdown();
   PowerManagerClient::Shutdown();
   CryptohomeClient::Shutdown();
   DBusThreadManager::Shutdown();

chromeos/dbus/tpm_manager/fake_tpm_manager_client.cc

@@ -4,9 +4,26 @@
 
 #include "chromeos/dbus/tpm_manager/fake_tpm_manager_client.h"
 
+#include <utility>
+
+#include "base/bind.h"
+#include "base/location.h"
 #include "base/notreached.h"
+#include "base/threading/thread_task_runner_handle.h"
 
 namespace chromeos {
+namespace {
+
+// Posts `callback` on the current thread's task runner, passing it the
+// `reply` message.
+template <class ReplyType>
+void PostProtoResponse(base::OnceCallback<void(const ReplyType&)> callback,
+                       const ReplyType& reply) {
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(std::move(callback), reply));
+}
+
+}  // namespace
 
 FakeTpmManagerClient::FakeTpmManagerClient() = default;
 
@@ -21,7 +38,7 @@ void FakeTpmManagerClient::GetTpmNonsensitiveStatus(
 void FakeTpmManagerClient::GetVersionInfo(
     const ::tpm_manager::GetVersionInfoRequest& request,
     GetVersionInfoCallback callback) {
-  NOTIMPLEMENTED();
+  PostProtoResponse(std::move(callback), version_info_reply_);
 }
 
 void FakeTpmManagerClient::GetDictionaryAttackInfo(
@@ -42,4 +59,13 @@ void FakeTpmManagerClient::ClearStoredOwnerPassword(
   NOTIMPLEMENTED();
 }
 
+TpmManagerClient::TestInterface* FakeTpmManagerClient::GetTestInterface() {
+  return this;
+}
+
+::tpm_manager::GetVersionInfoReply*
+FakeTpmManagerClient::mutable_version_info_reply() {
+  return &version_info_reply_;
+}
+
 }  // namespace chromeos

chromeos/dbus/tpm_manager/fake_tpm_manager_client.h

@@ -13,7 +13,8 @@
 namespace chromeos {
 
 class COMPONENT_EXPORT(CHROMEOS_DBUS_TPM_MANAGER) FakeTpmManagerClient
-    : public TpmManagerClient {
+    : public TpmManagerClient,
+      public TpmManagerClient::TestInterface {
  public:
   FakeTpmManagerClient();
   ~FakeTpmManagerClient() override;
@@ -38,6 +39,14 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_TPM_MANAGER) FakeTpmManagerClient
   void ClearStoredOwnerPassword(
       const ::tpm_manager::ClearStoredOwnerPasswordRequest& request,
       ClearStoredOwnerPasswordCallback callback) override;
+
+  TpmManagerClient::TestInterface* GetTestInterface() override;
+
+  // TpmManagerClient::TestInterface:
+  ::tpm_manager::GetVersionInfoReply* mutable_version_info_reply() override;
+
+ private:
+  ::tpm_manager::GetVersionInfoReply version_info_reply_;
 };
 
 }  // namespace chromeos

chromeos/dbus/tpm_manager/tpm_manager_client.cc

@@ -103,6 +103,8 @@ class TpmManagerClientImpl : public TpmManagerClient {
   }
 
  private:
+  TestInterface* GetTestInterface() override { return nullptr; }
+
   // Calls tpm_managerd's |method_name| method, passing in |request| as input
   // with |timeout_ms|. Once the (asynchronous) call finishes, |callback| is
   // called with the response proto.

chromeos/dbus/tpm_manager/tpm_manager_client.h

@@ -34,6 +34,15 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_TPM_MANAGER) TpmManagerClient {
   using ClearStoredOwnerPasswordCallback = base::OnceCallback<void(
       const ::tpm_manager::ClearStoredOwnerPasswordReply&)>;
 
+  // Interface with testing functionality. Accessed through GetTestInterface(),
+  // only implemented in the fake implementation.
+  class TestInterface {
+   public:
+    // Gets a mutable reply that is returned when `GetVersionInfo()` is called.
+    virtual ::tpm_manager::GetVersionInfoReply*
+    mutable_version_info_reply() = 0;
+  };
+
   // Not copyable or movable.
   TpmManagerClient(const TpmManagerClient&) = delete;
   TpmManagerClient& operator=(const TpmManagerClient&) = delete;
@@ -80,6 +89,9 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS_TPM_MANAGER) TpmManagerClient {
       const ::tpm_manager::ClearStoredOwnerPasswordRequest& request,
       ClearStoredOwnerPasswordCallback callback) = 0;
 
+  // Returns an interface for testing (fake only), or returns nullptr.
+  virtual TestInterface* GetTestInterface() = 0;
+
  protected:
   // Initialize/Shutdown should be used instead.
   TpmManagerClient();