Re-land "Switch SessionManagerClient::RestartJob to use RestartJobWithAuth""

This reverts commit 2045be86fa3510f263a5780903dacb96859ec890.

The session manager is once again providing a RestartJobWithAuth
method to allow callers to directly authorize their request to
restart the browser by providing a unix domain socket ready to be
SO_PEERCREDified in lieu of a pid.

This has the advantage of working inside a PID namespace, while
normal dbus mechanisms do not.

BUG=448821
TEST=signing in as guest works.
R=stevenjb@chromium.org

Review URL: https://codereview.chromium.org/1122113002

Cr-Commit-Position: refs/heads/master@{#328210}

1 files changed, 80 insertions, 11 deletions

diff --git a/chromeos/dbus/session_manager_client.cc b/chromeos/dbus/session_manager_client.cc
index ff5f465..0854875 100644
--- a/chromeos/dbus/session_manager_client.cc
+++ b/chromeos/dbus/session_manager_client.cc
@@ -4,7 +4,10 @@
 
 #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"
@@ -58,6 +61,23 @@ void StoreFile(const base::FilePath& path, const std::string& data) {
   }
 }
 
+// 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.
@@ -96,16 +116,38 @@ class SessionManagerClientImpl : public SessionManagerClient {
   }
 
   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);
+
+    // The session_manager provides a new method to replace RestartJob, called
+    // RestartJobWithAuth, that is able to be used correctly within a PID
+    // namespace. To use it, the caller must create a unix domain socket pair
+    // and pass one end over dbus while holding the local end open for the
+    // duration of the call.
+    // Here, we call CreateValidCredConduit() to create the socket pair,
+    // and then pass both ends along to CallRestartJobWithValidFd(), which
+    // takes care of them from there.
+    // NB: PostTaskAndReply ensures that the second callback (which owns the
+    //     ScopedFileDescriptor objects) outlives the first, so passing the
+    //     bare pointers to CreateValidCredConduit is safe...
+    //     -- BUT --
+    //     you have to grab pointers to the contents of {local,remote}_auth_fd
+    //     _before_ they're acted on by base::Passed() below. Passing ownership
+    //     of the ScopedFileDescriptor objects to the callback actually nulls
+    //     out the storage inside the local instances. Since there are
+    //     no guarantees about the order of evaluation of arguments in a
+    //     function call, merely having them appear earlier among the args
+    //     to PostTaskAndReply() is not enough. Relying on this crashed on
+    //     some platforms.
+    base::Closure create_credentials_conduit_closure = base::Bind(
+        &CreateValidCredConduit, local_auth_fd.get(), remote_auth_fd.get());
+
+    base::WorkerPool::PostTaskAndReply(
+        FROM_HERE, create_credentials_conduit_closure,
+        base::Bind(&SessionManagerClientImpl::CallRestartJobWithValidFd,
+                   weak_ptr_factory_.GetWeakPtr(), base::Passed(&local_auth_fd),
+                   base::Passed(&remote_auth_fd), command_line),
+        false);
   }
 
   void StartSession(const std::string& user_email) override {
@@ -379,8 +421,35 @@ class SessionManagerClientImpl : public SessionManagerClient {
             callback));
   }
 
+  // Calls RestartJobWithAuth to tell the session manager to restart the
+  // browser using the contents of command_line, authorizing the call
+  // using credentials acquired via remote_auth_fd.
+  // Ownership of local_auth_fd is held for the duration of the dbus call.
+  void CallRestartJobWithValidFd(dbus::ScopedFileDescriptor local_auth_fd,
+                                 dbus::ScopedFileDescriptor remote_auth_fd,
+                                 const std::string& command_line) {
+    dbus::MethodCall method_call(
+        login_manager::kSessionManagerInterface,
+        login_manager::kSessionManagerRestartJobWithAuth);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendFileDescriptor(*remote_auth_fd);
+    writer.AppendString(command_line);
+
+    // Ownership of local_auth_fd is passed to the callback that is to be
+    // called on completion of this method call. This keeps the browser end
+    // of the socket-pair alive for the duration of the RPC.
+    session_manager_proxy_->CallMethod(
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&SessionManagerClientImpl::OnRestartJob,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   base::Passed(&local_auth_fd)));
+  }
+
   // Called when kSessionManagerRestartJob method is complete.
-  void OnRestartJob(dbus::Response* response) {
+  // Now that the call is complete, local_auth_fd can be closed and discarded,
+  // which will happen automatically when it goes out of scope.
+  void OnRestartJob(dbus::ScopedFileDescriptor local_auth_fd,
+                    dbus::Response* response) {
     LOG_IF(ERROR, !response)
         << "Failed to call "
         << login_manager::kSessionManagerRestartJob;