Switch SessionManagerClient::RestartJob to use RestartJobWithAuth

chromeos/dbus/session_manager_client.cc

 // Copyright (c) 2012 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/dbus/session_manager_client.h"
 
+#include <sys/socket.h>
+
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/path_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/task_runner_util.h"
 #include "base/threading/worker_pool.h"
 #include "chromeos/chromeos_paths.h"
 #include "chromeos/dbus/blocking_method_caller.h"
@@ skipping 33 matching lines @@
 // Helper to write a file in a background thread.
 void StoreFile(const base::FilePath& path, const std::string& data) {
   const int size = static_cast<int>(data.size());
   if (path.empty() ||
       !base::CreateDirectory(path.DirName()) ||
       base::WriteFile(path, data.data(), size) != size) {
     LOG(WARNING) << "Failed to write to " << path.value();
   }
 }
 
+// Creates a pair of file descriptors that form a conduit for trustworthy
+// transfer of credentials between Chrome and the session_manager
+void CreateValidCredConduit(dbus::FileDescriptor* local_auth_fd,
+                            dbus::FileDescriptor* remote_auth_fd) {
+  int sockets[2] = {-1, -1};
+  if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) < 0) {
+    PLOG(ERROR) << "Failed to create a unix domain socketpair";
+    return;
+  }
+
+  local_auth_fd->PutValue(sockets[0]);
+  local_auth_fd->CheckValidity();
+
+  remote_auth_fd->PutValue(sockets[1]);
+  remote_auth_fd->CheckValidity();
+}
+
 }  // namespace
 
 // The SessionManagerClient implementation used in production.
 class SessionManagerClientImpl : public SessionManagerClient {
  public:
   SessionManagerClientImpl()
       : session_manager_proxy_(NULL),
         screen_is_locked_(false),
         weak_ptr_factory_(this) {}
 
@@ skipping 18 matching lines @@
 
   bool IsScreenLocked() const override { return screen_is_locked_; }
 
   void EmitLoginPromptVisible() override {
     SimpleMethodCallToSessionManager(
         login_manager::kSessionManagerEmitLoginPromptVisible);
     FOR_EACH_OBSERVER(Observer, observers_, EmitLoginPromptVisibleCalled());
   }
 
   void RestartJob(int pid, const std::string& command_line) override {
-    dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
-                                 login_manager::kSessionManagerRestartJob);
-    dbus::MessageWriter writer(&method_call);
-    writer.AppendInt32(pid);
-    writer.AppendString(command_line);
-    session_manager_proxy_->CallMethod(
-        &method_call,
-        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(&SessionManagerClientImpl::OnRestartJob,
-                   weak_ptr_factory_.GetWeakPtr()));
+    dbus::ScopedFileDescriptor local_auth_fd(new dbus::FileDescriptor());
+    dbus::ScopedFileDescriptor remote_auth_fd(new dbus::FileDescriptor());
+
+    base::Closure pipe_creator = base::Bind(
+        &CreateValidCredConduit, local_auth_fd.get(), remote_auth_fd.get());
+    dbus::ObjectProxy::ResponseCallback on_restart_job = base::Bind(
+        &SessionManagerClientImpl::OnRestartJob, weak_ptr_factory_.GetWeakPtr(),
+        base::Passed(&local_auth_fd));
+    base::Closure call_performer =
+        base::Bind(&SessionManagerClientImpl::CallRestartJobWithValidFd,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   base::Passed(&remote_auth_fd), command_line, on_restart_job);
+
+    base::WorkerPool::PostTaskAndReply(FROM_HERE, pipe_creator, call_performer,
+                                       false);

[stevenjb, 2015/04/28 01:25:26]

This is all pretty confusing. I'm pretty sure I followed it through and it looks
OK, but a couple of suggestions:

a) I'm not sure this will actually clarify things, but maybe pass local_auth_fd
to CallRestartJobWithValidFd instead of binding it to on_restart_job here? It
means you have to bind and pass local_auth_fd to the callback in the
session_manager_proxy_->CallMethod() call, but it might make ownership of it a
little easier to follow?

b) Either way, add some comments about ownership of the file descriptors, it's
challenging enough to follow when you're used to these patterns.

   }
 
   void StartSession(const std::string& user_email) override {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  login_manager::kSessionManagerStartSession);
     dbus::MessageWriter writer(&method_call);
     writer.AppendString(user_email);
     writer.AppendString("");  // Unique ID is deprecated
     session_manager_proxy_->CallMethod(
         &method_call,
@@ skipping 253 matching lines @@
     session_manager_proxy_->CallMethod(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::Bind(
             &SessionManagerClientImpl::OnStorePolicy,
             weak_ptr_factory_.GetWeakPtr(),
             method_name,
             callback));
   }
 
+  void CallRestartJobWithValidFd(dbus::ScopedFileDescriptor remote_auth_fd,
+                                 const std::string& command_line,
+                                 dbus::ObjectProxy::ResponseCallback callback) {
+    dbus::MethodCall method_call(
+        login_manager::kSessionManagerInterface,
+        login_manager::kSessionManagerRestartJobWithAuth);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendFileDescriptor(*remote_auth_fd);
+    writer.AppendString(command_line);
+    session_manager_proxy_->CallMethod(
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT, callback);
+  }
+
   // Called when kSessionManagerRestartJob method is complete.
-  void OnRestartJob(dbus::Response* response) {
+  void OnRestartJob(dbus::ScopedFileDescriptor local_auth_fd,
+                    dbus::Response* response) {
     LOG_IF(ERROR, !response)
         << "Failed to call "
         << login_manager::kSessionManagerRestartJob;
   }
 
   // Called when kSessionManagerStartSession method is complete.
   void OnStartSession(dbus::Response* response) {
     LOG_IF(ERROR, !response)
         << "Failed to call "
         << login_manager::kSessionManagerStartSession;
@@ skipping 350 matching lines @@
 
 SessionManagerClient* SessionManagerClient::Create(
     DBusClientImplementationType type) {
   if (type == REAL_DBUS_CLIENT_IMPLEMENTATION)
     return new SessionManagerClientImpl();
   DCHECK_EQ(STUB_DBUS_CLIENT_IMPLEMENTATION, type);
   return new SessionManagerClientStubImpl();
 }
 
 }  // namespace chromeos