cryptohome: Call UnmountEx instead of deprecated Unmount.

This calls the modern UnmountEx method in cryptohome instead of the
deprecated Unmount method.

Bug: 811551
Change-Id: I262b3b29eaf2dba969c95f9d0a7cfc70468f4f19
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1464453Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Greg Kerr <kerrnel@chromium.org>
Auto-Submit: Greg Kerr <kerrnel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638370}

chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc

@@ -159,6 +159,10 @@ class TestCryptohomeClient : public ::chromeos::FakeCryptohomeClient {
     remove_ex_should_succeed_ = should_succeed;
   }
 
+  void set_unmount_ex_should_succeed(bool should_succeed) {
+    unmount_ex_should_succeed_ = should_succeed;
+  }
+
   void MountEx(const cryptohome::AccountIdentifier& cryptohome_id,
                const cryptohome::AuthorizationRequest& auth,
                const cryptohome::MountRequest& request,
@@ -230,6 +234,16 @@ class TestCryptohomeClient : public ::chromeos::FakeCryptohomeClient {
         FROM_HERE, base::BindOnce(std::move(callback), reply));
   }
 
+  void UnmountEx(const cryptohome::UnmountRequest& account,
+                 DBusMethodCallback<cryptohome::BaseReply> callback) override {
+    cryptohome::BaseReply reply;
+    if (!unmount_ex_should_succeed_)
+      reply.set_error(cryptohome::CRYPTOHOME_ERROR_REMOVE_FAILED);
+
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::BindOnce(std::move(callback), reply));
+  }
+
  private:
   cryptohome::AccountIdentifier expected_id_;
   std::string expected_authorization_secret_;
@@ -237,6 +251,7 @@ class TestCryptohomeClient : public ::chromeos::FakeCryptohomeClient {
   bool migrate_key_should_succeed_ = false;
   bool mount_guest_should_succeed_ = false;
   bool remove_ex_should_succeed_ = false;
+  bool unmount_ex_should_succeed_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(TestCryptohomeClient);
 };
@@ -582,7 +597,7 @@ TEST_F(CryptohomeAuthenticatorTest, ResolveOwnerNeededFailedMount) {
   EXPECT_TRUE(LoginState::Get()->IsInSafeMode());
 
   // Unset global objects used by this test.
-  fake_cryptohome_client_->set_unmount_result(true);
+  fake_cryptohome_client_->set_unmount_ex_should_succeed(true);
   LoginState::Shutdown();
 }
 
@@ -631,7 +646,7 @@ TEST_F(CryptohomeAuthenticatorTest, ResolveOwnerNeededSuccess) {
   EXPECT_TRUE(LoginState::Get()->IsInSafeMode());
 
   // Unset global objects used by this test.
-  fake_cryptohome_client_->set_unmount_result(true);
+  fake_cryptohome_client_->set_unmount_ex_should_succeed(true);
   LoginState::Shutdown();
 }
 

chrome/browser/chromeos/policy/pre_signin_policy_fetcher.cc

@@ -130,16 +130,18 @@ void PreSigninPolicyFetcher::OnCachedPolicyRetrieved(
 void PreSigninPolicyFetcher::OnPolicyKeyLoaded(
     RetrievePolicyResponseType retrieve_policy_response,
     const std::string& policy_blob) {
-  cryptohome_client_->Unmount(base::BindOnce(
-      &PreSigninPolicyFetcher::OnUnmountTemporaryUserHome,
-      weak_ptr_factory_.GetWeakPtr(), retrieve_policy_response, policy_blob));
+  cryptohome_client_->UnmountEx(
+      cryptohome::UnmountRequest(),
+      base::BindOnce(&PreSigninPolicyFetcher::OnUnmountTemporaryUserHome,
+                     weak_ptr_factory_.GetWeakPtr(), retrieve_policy_response,
+                     policy_blob));
 }
 
 void PreSigninPolicyFetcher::OnUnmountTemporaryUserHome(
     RetrievePolicyResponseType retrieve_policy_response,
     const std::string& policy_blob,
-    base::Optional<bool> unmount_success) {
-  if (!unmount_success.has_value() || !unmount_success.value()) {
+    base::Optional<cryptohome::BaseReply> reply) {
+  if (BaseReplyToMountError(reply) != cryptohome::MOUNT_ERROR_NONE) {
     // The temporary userhome mount could not be unmounted. Log an error and
     // continue, and hope that the unmount will be successful on the next mount
     // (temporary user homes are automatically unmounted by cryptohomed on every

chrome/browser/chromeos/policy/pre_signin_policy_fetcher.h

@@ -102,7 +102,7 @@ class PreSigninPolicyFetcher : public CloudPolicyClient::Observer {
   void OnUnmountTemporaryUserHome(
       RetrievePolicyResponseType retrieve_policy_response,
       const std::string& policy_blob,
-      base::Optional<bool> unmount_success);
+      base::Optional<cryptohome::BaseReply> reply);
 
   void OnCachedPolicyValidated(UserCloudPolicyValidator* validator);
 

chromeos/dbus/cryptohome_client.cc

@@ -134,10 +134,17 @@ class CryptohomeClientImpl : public CryptohomeClient {
   }
 
   // CryptohomeClient override.
-  void Unmount(DBusMethodCallback<bool> callback) override {
+  void UnmountEx(const cryptohome::UnmountRequest& request,
+                 DBusMethodCallback<cryptohome::BaseReply> callback) override {
     dbus::MethodCall method_call(cryptohome::kCryptohomeInterface,
-                                 cryptohome::kCryptohomeUnmount);
-    CallBoolMethod(&method_call, std::move(callback));
+                                 cryptohome::kCryptohomeUnmountEx);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendProtoAsArrayOfBytes(request);
+
+    proxy_->CallMethod(
+        &method_call, kTpmDBusTimeoutMs,
+        base::BindOnce(&CryptohomeClientImpl::OnBaseReplyMethod,
+                       weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
   }
 
   // CryptohomeClient override.

chromeos/dbus/cryptohome_client.h

@@ -39,6 +39,7 @@ class RemoveFirmwareManagementParametersRequest;
 class RemoveKeyRequest;
 class SetBootAttributeRequest;
 class SetFirmwareManagementParametersRequest;
+class UnmountRequest;
 class UpdateKeyRequest;
 
 }  // namespace cryptohome
@@ -147,8 +148,11 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) CryptohomeClient : public DBusClient {
   // Calls IsMounted method and returns true when the call succeeds.
   virtual void IsMounted(DBusMethodCallback<bool> callback) = 0;
 
-  // Calls Unmount method and returns true when the call succeeds.
-  virtual void Unmount(DBusMethodCallback<bool> callback) = 0;
+  // Calls UnmountEx method. |callback| is called after the method call
+  // succeeds.
+  virtual void UnmountEx(
+      const cryptohome::UnmountRequest& request,
+      DBusMethodCallback<cryptohome::BaseReply> callback) = 0;
 
   // Calls MigrateKeyEx method. |callback| is called after the method call
   // succeeds.

chromeos/dbus/fake_cryptohome_client.cc

@@ -85,9 +85,10 @@ void FakeCryptohomeClient::IsMounted(DBusMethodCallback<bool> callback) {
       FROM_HERE, base::BindOnce(std::move(callback), true));
 }
 
-void FakeCryptohomeClient::Unmount(DBusMethodCallback<bool> callback) {
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(std::move(callback), unmount_result_));
+void FakeCryptohomeClient::UnmountEx(
+    const cryptohome::UnmountRequest& request,
+    DBusMethodCallback<cryptohome::BaseReply> callback) {
+  ReturnProtobufMethodCallback(cryptohome::BaseReply(), std::move(callback));
 }
 
 void FakeCryptohomeClient::MigrateKeyEx(

chromeos/dbus/fake_cryptohome_client.h

@@ -36,7 +36,8 @@ class COMPONENT_EXPORT(CHROMEOS_DBUS) FakeCryptohomeClient
   void WaitForServiceToBeAvailable(
       WaitForServiceToBeAvailableCallback callback) override;
   void IsMounted(DBusMethodCallback<bool> callback) override;
-  void Unmount(DBusMethodCallback<bool> callback) override;
+  void UnmountEx(const cryptohome::UnmountRequest& request,
+                 DBusMethodCallback<cryptohome::BaseReply> callback) override;
   void MigrateKeyEx(
       const cryptohome::AccountIdentifier& account,
       const cryptohome::AuthorizationRequest& auth_request,

chromeos/login/auth/cryptohome_authenticator.cc

@@ -792,6 +792,14 @@ void CryptohomeAuthenticator::OnOldEncryptionDetected(
   }
 }
 
+// Callback invoked when UnmountEx returns.
+void CryptohomeAuthenticator::OnUnmountEx(
+    base::Optional<cryptohome::BaseReply> reply) {
+  if (BaseReplyToMountError(reply) != cryptohome::MOUNT_ERROR_NONE)
+    LOGIN_LOG(ERROR) << "Couldn't unmount user's homedir";
+  OnAuthFailure(AuthFailure(AuthFailure::OWNER_REQUIRED));
+}
+
 void CryptohomeAuthenticator::OnAuthFailure(const AuthFailure& error) {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
 
@@ -857,14 +865,6 @@ void CryptohomeAuthenticator::OnOwnershipChecked(bool is_owner) {
   Resolve();
 }
 
-void CryptohomeAuthenticator::OnUnmount(base::Optional<bool> success) {
-  if (!success.has_value() || !success.value()) {
-    // Maybe we should reboot immediately here?
-    LOGIN_LOG(ERROR) << "Couldn't unmount users home!";
-  }
-  OnAuthFailure(AuthFailure(AuthFailure::OWNER_REQUIRED));
-}
-
 void CryptohomeAuthenticator::Resolve() {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
   bool create_if_nonexistent = false;
@@ -984,8 +984,9 @@ void CryptohomeAuthenticator::Resolve() {
       break;
     case OWNER_REQUIRED: {
       current_state_->ResetCryptohomeStatus();
-      DBusThreadManager::Get()->GetCryptohomeClient()->Unmount(
-          base::BindOnce(&CryptohomeAuthenticator::OnUnmount, this));
+      DBusThreadManager::Get()->GetCryptohomeClient()->UnmountEx(
+          cryptohome::UnmountRequest(),
+          base::BindOnce(&CryptohomeAuthenticator::OnUnmountEx, this));
       break;
     }
     case FAILED_OLD_ENCRYPTION:

chromeos/login/auth/cryptohome_authenticator.h

@@ -27,6 +27,10 @@ namespace content {
 class BrowserContext;
 }
 
+namespace cryptohome {
+class BaseReply;
+}
+
 namespace chromeos {
 
 class AuthStatusConsumer;
@@ -158,6 +162,9 @@ class COMPONENT_EXPORT(CHROMEOS_LOGIN_AUTH) CryptohomeAuthenticator
   void RecoverEncryptedData(const std::string& old_password) override;
   void ResyncEncryptedData() override;
 
+  // Called after UnmountEx finishes.
+  void OnUnmountEx(base::Optional<cryptohome::BaseReply> reply);
+
   // AuthAttemptStateResolver overrides.
   // Attempts to make a decision and call back |consumer_| based on
   // the state we have gathered at the time of call.  If a decision