NaCl Linux: use own setuid sandbox instance

NaCl now uses its own instance of the setuid sandbox. In particular, NaCl
is now running in its own PID namespace (which is a sub-space of the Zygote
PID namespace).
Moreover, the NaCl helper is responsible for getting chrooted, instead of
relying on a shared FS view (via CLONE_FS) with the Zygote.

This CL also ensures consistency between the setuid sandbox status as
reported in about:sandbox and NaCl's setuid sandbox status.

Before, the process tree looks like this:

__browser
____chrome-sandbox [X, fs_state1]
______init [pid_ns1, fs_state1]
________zygote [pid_ns1, fs_state1]
________nacl_helper [pid_ns1, fs_state1]

  -- "X" means same as parent.

After:

__browser
____chrome-sandbox [X , fs_state1]
______init [pid_ns1, fs_state1]
________zygote         [pid_ns1, fs_state1]
________chrome-sandbox [pid_ns1, fs_state2]
__________nacl_helper  [pid_ns2, fs_state2] (nacl_helper doubles as init(1) in pid_ns2).

The main change is to make nacl_fork_delegate_linux.cc launch nacl_helper via
chrome-sandbox instead trying to share the view of the file system with the Zygote
via CLONE_FS.  It uses SetuidSandboxClient to help with this.

Then change nacl_helper_linux.cc to tell (via IPC) chrome-sandbox to enable
the sandbox, and add some more sanity checks.

BUG=358733
R=mseaborn@chromium.org, piman@chromium.org

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

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

2 files changed, 84 insertions, 10 deletions

diff --git a/sandbox/linux/suid/client/setuid_sandbox_client.cc b/sandbox/linux/suid/client/setuid_sandbox_client.cc
index 8feec04..3300cb4 100644
--- a/sandbox/linux/suid/client/setuid_sandbox_client.cc
+++ b/sandbox/linux/suid/client/setuid_sandbox_client.cc
@@ -4,6 +4,7 @@
 
 #include "sandbox/linux/suid/client/setuid_sandbox_client.h"
 
+#include <fcntl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -13,10 +14,13 @@
 #include "base/environment.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
+#include "base/files/scoped_file.h"
 #include "base/logging.h"
+#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
 #include "base/posix/eintr_wrapper.h"
+#include "base/process/launch.h"
 #include "base/process/process_metrics.h"
 #include "base/strings/string_number_conversions.h"
 #include "sandbox/linux/services/init_process_reaper.h"
@@ -25,6 +29,11 @@
 
 namespace {
 
+bool IsFileSystemAccessDenied() {
+  base::ScopedFD self_exe(HANDLE_EINTR(open(base::kProcSelfExe, O_RDONLY)));
+  return !self_exe.is_valid();
+}
+
 // Set an environment variable that reflects the API version we expect from the
 // setuid sandbox. Old versions of the sandbox will ignore this.
 void SetSandboxAPIEnvironmentVariable(base::Environment* env) {
@@ -32,6 +41,26 @@ void SetSandboxAPIEnvironmentVariable(base::Environment* env) {
               base::IntToString(sandbox::kSUIDSandboxApiNumber));
 }
 
+// Unset environment variables that are expected to be set by the setuid
+// sandbox. This is to allow nesting of one instance of the SUID sandbox
+// inside another.
+void UnsetExpectedEnvironmentVariables(base::EnvironmentMap* env_map) {
+  DCHECK(env_map);
+  const base::NativeEnvironmentString environment_vars[] = {
+      sandbox::kSandboxDescriptorEnvironmentVarName,
+      sandbox::kSandboxHelperPidEnvironmentVarName,
+      sandbox::kSandboxEnvironmentApiProvides,
+      sandbox::kSandboxPIDNSEnvironmentVarName,
+      sandbox::kSandboxNETNSEnvironmentVarName,
+  };
+
+  for (size_t i = 0; i < arraysize(environment_vars); ++i) {
+    // Setting values in EnvironmentMap to an empty-string will make
+    // sure that they get unset from the environment via AlterEnvironment().
+    (*env_map)[environment_vars[i]] = base::NativeEnvironmentString();
+  }
+}
+
 // Wrapper around a shared C function.
 // Returns the "saved" environment variable name corresponding to |envvar|
 // in a new string or NULL.
@@ -157,6 +186,7 @@ bool SetuidSandboxClient::ChrootMe() {
 
   // We now consider ourselves "fully sandboxed" as far as the
   // setuid sandbox is concerned.
+  CHECK(IsFileSystemAccessDenied());
   sandboxed_ = true;
   return true;
 }
@@ -196,6 +226,12 @@ 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;
@@ -220,8 +256,8 @@ base::FilePath SetuidSandboxClient::GetSandboxBinaryPath() {
   return sandbox_binary;
 }
 
-void SetuidSandboxClient::PrependWrapper(base::CommandLine* cmd_line) {
-  DCHECK(cmd_line);
+void SetuidSandboxClient::PrependWrapper(base::CommandLine* cmd_line,
+                                         base::LaunchOptions* options) {
   std::string sandbox_binary(GetSandboxBinaryPath().value());
   struct stat st;
   if (sandbox_binary.empty() || stat(sandbox_binary.c_str(), &st) != 0) {
@@ -237,10 +273,17 @@ void SetuidSandboxClient::PrependWrapper(base::CommandLine* cmd_line) {
                   "configured correctly. Rather than run without sandboxing "
                   "I'm aborting now. You need to make sure that "
                << sandbox_binary << " is owned by root and has mode 4755.";
+  }
 
-  } else {
+  if (cmd_line) {
     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::SetupLaunchEnvironment() {
diff --git a/sandbox/linux/suid/client/setuid_sandbox_client.h b/sandbox/linux/suid/client/setuid_sandbox_client.h
index 20a9905..332c63b 100644
--- a/sandbox/linux/suid/client/setuid_sandbox_client.h
+++ b/sandbox/linux/suid/client/setuid_sandbox_client.h
@@ -13,6 +13,7 @@
 namespace base {
 class CommandLine;
 class Environment;
+struct LaunchOptions;
 }
 
 namespace sandbox {
@@ -20,11 +21,30 @@ namespace sandbox {
 // Helper class to use the setuid sandbox. This class is to be used both
 // before launching the setuid helper and after being executed through the
 // setuid helper.
+// This class is difficult to use. It has been created by refactoring very old
+// code scathered through the Chromium code base.
 //
-// A typical use would be:
-// 1. The browser calls SetupLaunchEnvironment()
-// 2. The browser launches a renderer through the setuid sandbox.
-// 3. The renderer requests being chroot-ed through ChrootMe() and
+// A typical use for "A" launching a sandboxed process "B" would be:
+// 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.)
+// 4. A launches B with base::LaunchProcess, using the amended CommandLine.
+// 5. B performs various initializations that require access to the file
+//    system.
+// 5.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
+//    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
 //    requests other sandboxing status via the status functions.
 class SANDBOX_EXPORT SetuidSandboxClient {
  public:
@@ -53,14 +73,25 @@ class SANDBOX_EXPORT SetuidSandboxClient {
   // Are we done and fully sandboxed ?
   bool IsSandboxed() const;
 
+  // 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| to launch via the setuid sandbox. Crash if the setuid
-  // sandbox binary cannot be found.
-  void PrependWrapper(base::CommandLine* cmd_line);
+  // 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);
   // Set-up the environment. This should be done prior to launching the setuid
   // helper.
   void SetupLaunchEnvironment();