Get rid of kZygoteIdFd from content

Move handling of the dummy file descriptor into SetuidSandboxClient.

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267743 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 64 insertions, 45 deletions

diff --git a/sandbox/linux/suid/client/setuid_sandbox_client.cc b/sandbox/linux/suid/client/setuid_sandbox_client.cc
index 3300cb4..fc03cdd 100644
--- a/sandbox/linux/suid/client/setuid_sandbox_client.cc
+++ b/sandbox/linux/suid/client/setuid_sandbox_client.cc
@@ -5,6 +5,8 @@
 #include "sandbox/linux/suid/client/setuid_sandbox_client.h"
 
 #include <fcntl.h>
+#include <stdlib.h>
+#include <sys/socket.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -136,7 +138,7 @@ namespace sandbox {
 
 SetuidSandboxClient* SetuidSandboxClient::Create() {
   base::Environment* environment(base::Environment::Create());
-  SetuidSandboxClient* sandbox_client(new(SetuidSandboxClient));
+  SetuidSandboxClient* sandbox_client(new SetuidSandboxClient);
 
   CHECK(environment);
   sandbox_client->env_ = environment;
@@ -152,6 +154,21 @@ SetuidSandboxClient::~SetuidSandboxClient() {
   delete env_;
 }
 
+void SetuidSandboxClient::CloseDummyFile() {
+  // When we're launched through the setuid sandbox, SetupLaunchOptions
+  // arranges for kZygoteIdFd to be a dummy file descriptor to satisfy an
+  // ancient setuid sandbox ABI requirement. However, the descriptor is no
+  // longer needed, so we can simply close it right away now.
+  CHECK(IsSuidSandboxChild());
+
+  // Sanity check that kZygoteIdFd refers to a pipe.
+  struct stat st;
+  PCHECK(0 == fstat(kZygoteIdFd, &st));
+  CHECK(S_ISFIFO(st.st_mode));
+
+  PCHECK(0 == IGNORE_EINTR(close(kZygoteIdFd)));
+}
+
 bool SetuidSandboxClient::ChrootMe() {
   int ipc_fd = GetIPCDescriptor(env_);
 
@@ -226,12 +243,6 @@ bool SetuidSandboxClient::IsDisabledViaEnvironment() {
   return false;
 }
 
-int SetuidSandboxClient::GetUniqueToChildFileDescriptor() {
-  // The setuid binary is hard-wired to close this in the helper process it
-  // creates.
-  return kZygoteIdFd;
-}
-
 base::FilePath SetuidSandboxClient::GetSandboxBinaryPath() {
   base::FilePath sandbox_binary;
   base::FilePath exe_dir;
@@ -256,8 +267,7 @@ base::FilePath SetuidSandboxClient::GetSandboxBinaryPath() {
   return sandbox_binary;
 }
 
-void SetuidSandboxClient::PrependWrapper(base::CommandLine* cmd_line,
-                                         base::LaunchOptions* options) {
+void SetuidSandboxClient::PrependWrapper(base::CommandLine* cmd_line) {
   std::string sandbox_binary(GetSandboxBinaryPath().value());
   struct stat st;
   if (sandbox_binary.empty() || stat(sandbox_binary.c_str(), &st) != 0) {
@@ -275,15 +285,30 @@ void SetuidSandboxClient::PrependWrapper(base::CommandLine* cmd_line,
                << sandbox_binary << " is owned by root and has mode 4755.";
   }
 
-  if (cmd_line) {
-    cmd_line->PrependWrapper(sandbox_binary);
-  }
+  cmd_line->PrependWrapper(sandbox_binary);
+}
 
-  if (options) {
-    // Launching a setuid binary requires PR_SET_NO_NEW_PRIVS to not be used.
-    options->allow_new_privs = true;
-    UnsetExpectedEnvironmentVariables(&options->environ);
-  }
+void SetuidSandboxClient::SetupLaunchOptions(
+    base::LaunchOptions* options,
+    base::FileHandleMappingVector* fds_to_remap,
+    base::ScopedFD* dummy_fd) {
+  DCHECK(options);
+  DCHECK(fds_to_remap);
+
+  // Launching a setuid binary requires PR_SET_NO_NEW_PRIVS to not be used.
+  options->allow_new_privs = true;
+  UnsetExpectedEnvironmentVariables(&options->environ);
+
+  // Set dummy_fd to the reading end of a closed pipe.
+  int pipe_fds[2];
+  PCHECK(0 == pipe(pipe_fds));
+  PCHECK(0 == IGNORE_EINTR(close(pipe_fds[1])));
+  dummy_fd->reset(pipe_fds[0]);
+
+  // We no longer need a dummy socket for discovering the child's PID,
+  // but the sandbox is still hard-coded to expect a file descriptor at
+  // kZygoteIdFd. Fixing this requires a sandbox API change. :(
+  fds_to_remap->push_back(std::make_pair(dummy_fd->get(), kZygoteIdFd));
 }
 
 void SetuidSandboxClient::SetupLaunchEnvironment() {
diff --git a/sandbox/linux/suid/client/setuid_sandbox_client.h b/sandbox/linux/suid/client/setuid_sandbox_client.h
index 332c63b..2bbad7a 100644
--- a/sandbox/linux/suid/client/setuid_sandbox_client.h
+++ b/sandbox/linux/suid/client/setuid_sandbox_client.h
@@ -8,14 +8,10 @@
 #include "base/basictypes.h"
 #include "base/callback_forward.h"
 #include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
+#include "base/process/launch.h"
 #include "sandbox/linux/sandbox_export.h"
 
-namespace base {
-class CommandLine;
-class Environment;
-struct LaunchOptions;
-}
-
 namespace sandbox {
 
 // Helper class to use the setuid sandbox. This class is to be used both
@@ -28,23 +24,21 @@ namespace sandbox {
 // 1. A calls SetupLaunchEnvironment()
 // 2. A sets up a CommandLine and then amends it with
 //    PrependWrapper() (or manually, by relying on GetSandboxBinaryPath()).
-// 3. A makes sure that GetUniqueToChildFileDescriptor() is an existing file
-//    descriptor that can be closed by the helper process created by the
-//    setuid sandbox. (This is the right file descriptor to use for magic
-//    "must-be-unique" sockets that are use to identify processes across
-//    pid namespaces.)
+// 3. A uses SetupLaunchOptions() to arrange for a dummy descriptor for the
+//    setuid sandbox ABI.
 // 4. A launches B with base::LaunchProcess, using the amended CommandLine.
-// 5. B performs various initializations that require access to the file
+// 5. B uses CloseDummyFile() to close the dummy file descriptor.
+// 6. B performs various initializations that require access to the file
 //    system.
-// 5.b (optional) B uses sandbox::Credentials::HasOpenDirectory() to verify
+// 6.b (optional) B uses sandbox::Credentials::HasOpenDirectory() to verify
 //    that no directory is kept open (which would allow bypassing the setuid
 //    sandbox).
-// 6. B should be prepared to assume the role of init(1). In particular, B
+// 7. B should be prepared to assume the role of init(1). In particular, B
 //    cannot receive any signal from any other process, excluding SIGKILL.
 //    If B dies, all the processes in the namespace will die.
 //    B can fork() and the parent can assume the role of init(1), by using
 //    CreateInitProcessReaper().
-// 7. B requests being chroot-ed through ChrootMe() and
+// 8. B requests being chroot-ed through ChrootMe() and
 //    requests other sandboxing status via the status functions.
 class SANDBOX_EXPORT SetuidSandboxClient {
  public:
@@ -52,6 +46,8 @@ class SANDBOX_EXPORT SetuidSandboxClient {
   static class SetuidSandboxClient* Create();
   ~SetuidSandboxClient();
 
+  // Close the dummy file descriptor leftover from the sandbox ABI.
+  void CloseDummyFile();
   // Ask the setuid helper over the setuid sandbox IPC channel to chroot() us
   // to an empty directory.
   // Will only work if we have been launched through the setuid helper.
@@ -76,22 +72,21 @@ class SANDBOX_EXPORT SetuidSandboxClient {
   // The setuid sandbox may still be disabled via the environment.
   // This is tracked in crbug.com/245376.
   bool IsDisabledViaEnvironment();
-  // When using the setuid sandbox, an extra helper process is created.
-  // Unfortunately, this helper process is hard-wired to close a specific file
-  // descriptor.
-  // The caller must make sure that GetUniqueToChildFileDescriptor() is an
-  // existing file descriptor that can be closed by the helper process. It's ok
-  // to make it a dummy, useless file descriptor if needed.
-  int GetUniqueToChildFileDescriptor();
   // Get the sandbox binary path. This method knows about the
   // CHROME_DEVEL_SANDBOX environment variable used for user-managed builds. If
   // the sandbox binary cannot be found, it will return an empty FilePath.
   base::FilePath GetSandboxBinaryPath();
-  // Modify |cmd_line| and |options| to launch via the setuid sandbox. Crash if
-  // the setuid sandbox binary cannot be found. Either can be NULL if the caller
-  // needs additional control.
-  void PrependWrapper(base::CommandLine* cmd_line,
-                      base::LaunchOptions* options);
+  // Modify |cmd_line| to launch via the setuid sandbox. Crash if the setuid
+  // sandbox binary cannot be found.  |cmd_line| must not be NULL.
+  void PrependWrapper(base::CommandLine* cmd_line);
+  // Set-up the launch options for launching via the setuid sandbox.  Caller is
+  // responsible for keeping |dummy_fd| alive until LaunchProcess() completes.
+  // |options| and |fds_to_remap| must not be NULL.
+  // (Keeping |dummy_fd| alive is an unfortunate historical artifact of the
+  // chrome-sandbox ABI.)
+  void SetupLaunchOptions(base::LaunchOptions* options,
+                          base::FileHandleMappingVector* fds_to_remap,
+                          base::ScopedFD* dummy_fd);
   // Set-up the environment. This should be done prior to launching the setuid
   // helper.
   void SetupLaunchEnvironment();
@@ -106,4 +101,3 @@ class SANDBOX_EXPORT SetuidSandboxClient {
 }  // namespace sandbox
 
 #endif  // SANDBOX_LINUX_SUID_SETUID_SANDBOX_CLIENT_H_
-