Fix logic for checking chrome-sandbox setuid binary

content/browser/zygote_host/zygote_host_impl_linux.cc

Index: content/browser/zygote_host/zygote_host_impl_linux.cc
diff --git a/content/browser/zygote_host/zygote_host_impl_linux.cc b/content/browser/zygote_host/zygote_host_impl_linux.cc
index e798f96fd7af1beea4ef4d4791ed6b7e59f21c20..9849eb6165c5b8425d6436d2ac369884cd644014 100644
--- a/content/browser/zygote_host/zygote_host_impl_linux.cc
+++ b/content/browser/zygote_host/zygote_host_impl_linux.cc
@@ -4,26 +4,57 @@
 
 #include "content/browser/zygote_host/zygote_host_impl_linux.h"
 
+#include <sys/socket.h>
+
 #include "base/allocator/allocator_extension.h"
-#include "base/command_line.h"
 #include "base/files/file_enumerator.h"
+#include "base/posix/unix_domain_socket_linux.h"
 #include "base/process/kill.h"
 #include "base/process/memory.h"
 #include "base/strings/string_number_conversions.h"
-#include "content/public/browser/content_browser_client.h"
+#include "content/browser/renderer_host/render_sandbox_host_linux.h"
+#include "content/common/child_process_sandbox_support_impl_linux.h"
+#include "content/common/zygote_commands_linux.h"
 #include "content/public/common/content_switches.h"
 #include "sandbox/linux/services/credentials.h"
+#include "sandbox/linux/services/namespace_sandbox.h"
+#include "sandbox/linux/suid/client/setuid_sandbox_host.h"
 #include "sandbox/linux/suid/common/sandbox.h"
 
 namespace content {
 
+namespace {
+
+// Receive a fixed message on fd and return the sender's PID.
+// Returns true if the message received matches the expected message.
+bool ReceiveFixedMessage(int fd,
+                         const char* expect_msg,
+                         size_t expect_len,
+                         base::ProcessId* sender_pid) {
+  char buf[expect_len + 1];

[rickyz (no longer on Chrome), 2016/05/20 23:41:44]

Didn't originate in your change, but I wonder why there was ever a + 1 here.

[mdempsky, 2016/05/21 00:05:07]

With framed-message socket types like SOCK_DATAGRAM and SOCK_SEQPACKET, recvmsg
truncates the messages to fit the buffer. If we had allocated the buffer to be
exactly expect_len bytes, then we wouldn't be able to detect whether:

  1) the sender sent us exactly expect_len bytes, or
  2) the sender sent us more than expect_len bytes, but recvmsg truncated it to
expect_len bytes.

By allocating an extra byte, we're able to reliably distinguish those two cases.

Added a comment to explain.

+  std::vector<base::ScopedFD> fds_vec;
+
+  const ssize_t len = base::UnixDomainSocket::RecvMsgWithPid(
+      fd, buf, sizeof(buf), &fds_vec, sender_pid);
+  if (static_cast<size_t>(len) != expect_len)
+    return false;
+  if (memcmp(buf, expect_msg, expect_len) != 0)
+    return false;
+  if (!fds_vec.empty())
+    return false;
+  return true;
+}
+
+}  // namespace
+
 // static
 ZygoteHost* ZygoteHost::GetInstance() {
   return ZygoteHostImpl::GetInstance();
 }
 
 ZygoteHostImpl::ZygoteHostImpl()
-    : should_use_namespace_sandbox_(true),
+    : use_namespace_sandbox_(false),
+      use_suid_sandbox_(false),
       use_suid_sandbox_for_adj_oom_score_(false),
       sandbox_binary_(),
       zygote_pids_lock_(),
@@ -36,34 +67,50 @@ ZygoteHostImpl* ZygoteHostImpl::GetInstance() {
   return base::Singleton<ZygoteHostImpl>::get();
 }
 
-void ZygoteHostImpl::Init(const std::string& sandbox_cmd) {
-  sandbox_binary_ = sandbox_cmd;
-
-  const base::CommandLine& command_line =
-      *base::CommandLine::ForCurrentProcess();
-  if (command_line.HasSwitch(switches::kNoSandbox) ||
-      command_line.HasSwitch(switches::kDisableNamespaceSandbox) ||
-      !sandbox::Credentials::CanCreateProcessInNewUserNS()) {
-    should_use_namespace_sandbox_ = false;
+void ZygoteHostImpl::Init(const base::CommandLine& command_line) {
+  if (command_line.HasSwitch(switches::kNoSandbox)) {
+    return;
   }
 
-  const bool using_namespace_sandbox = ShouldUseNamespaceSandbox();
-  // A non empty sandbox_cmd means we want a SUID sandbox.
-  const bool using_suid_sandbox =
-      !sandbox_binary_.empty() && !using_namespace_sandbox;
+  {
+    std::unique_ptr<sandbox::SetuidSandboxHost> setuid_sandbox_host(
+        sandbox::SetuidSandboxHost::Create());
+    sandbox_binary_ = setuid_sandbox_host->GetSandboxBinaryPath().value();
+  }
 
-  // Use the SUID sandbox for adjusting OOM scores when we are using the setuid
-  // sandbox. This is needed beacuse the processes are non-dumpable, so
-  // /proc/pid/oom_score_adj can only be written by root.
-  use_suid_sandbox_for_adj_oom_score_ = using_suid_sandbox;
+  if (!command_line.HasSwitch(switches::kDisableNamespaceSandbox) &&
+      sandbox::Credentials::CanCreateProcessInNewUserNS()) {
+    use_namespace_sandbox_ = true;
 
 #if defined(OS_CHROMEOS)
-  // Chrome OS has a kernel patch that restricts oom_score_adj. See
-  // crbug.com/576409 for details.
-  if (!sandbox_binary_.empty()) {
-    use_suid_sandbox_for_adj_oom_score_ = true;
-  }
+    // Chrome OS has a kernel patch that restricts oom_score_adj. See
+    // crbug.com/576409 for details.
+    if (!sandbox_binary_.empty()) {
+      use_suid_sandbox_for_adj_oom_score_ = true;
+    } else {
+      LOG(ERROR) << "SUID sandbox binary is missing. Won't be able to adjust "
+                    "OOM scores.";
+    }
 #endif
+  } else if (!command_line.HasSwitch(switches::kDisableSetuidSandbox) &&
+             !sandbox_binary_.empty()) {
+    use_suid_sandbox_ = true;
+
+    // Use the SUID sandbox for adjusting OOM scores when we are using
+    // the setuid sandbox. This is needed beacuse the processes are
+    // non-dumpable, so /proc/pid/oom_score_adj can only be written by
+    // root.
+    use_suid_sandbox_for_adj_oom_score_ = use_suid_sandbox_;
+  } else {
+    LOG(FATAL)
+        << "No usable sandbox! Update your kernel or see "
+           "https://chromium.googlesource.com/chromium/src/+/master/"
+           "docs/linux_suid_sandbox_development.md for more information on "
+           "developing with the SUID sandbox. "
+           "If you want to live dangerously and need an immediate workaround, "
+           "you can try using --"
+        << switches::kNoSandbox << ".";
+  }
 }
 
 void ZygoteHostImpl::AddZygotePid(pid_t pid) {
@@ -76,10 +123,6 @@ bool ZygoteHostImpl::IsZygotePid(pid_t pid) {
   return zygote_pids_.find(pid) != zygote_pids_.end();
 }
 
-const std::string& ZygoteHostImpl::SandboxCommand() const {
-  return sandbox_binary_;
-}
-
 void ZygoteHostImpl::SetRendererSandboxStatus(int status) {
   renderer_sandbox_status_ = status;
 }
@@ -88,8 +131,78 @@ int ZygoteHostImpl::GetRendererSandboxStatus() const {
   return renderer_sandbox_status_;
 }
 
-bool ZygoteHostImpl::ShouldUseNamespaceSandbox() {
-  return should_use_namespace_sandbox_;
+pid_t ZygoteHostImpl::LaunchZygote(base::CommandLine* cmd_line,
+                                   base::ScopedFD* control_fd) {
+  int fds[2];
+  CHECK_EQ(0, socketpair(AF_UNIX, SOCK_SEQPACKET, 0, fds));
+  CHECK(base::UnixDomainSocket::EnableReceiveProcessId(fds[0]));
+
+  base::FileHandleMappingVector fds_to_map;
+  fds_to_map.push_back(std::make_pair(fds[1], kZygoteSocketPairFd));
+
+  // Start up the sandbox host process and get the file descriptor for the
+  // renderers to talk to it.
+  const int sfd = RenderSandboxHostLinux::GetInstance()->GetRendererSocket();
+  fds_to_map.push_back(std::make_pair(sfd, GetSandboxFD()));
+
+  base::LaunchOptions options;
+  base::ScopedFD dummy_fd;
+  if (use_suid_sandbox_) {
+    std::unique_ptr<sandbox::SetuidSandboxHost> sandbox_host(
+        sandbox::SetuidSandboxHost::Create());
+    sandbox_host->PrependWrapper(cmd_line);
+    sandbox_host->SetupLaunchOptions(&options, &fds_to_map, &dummy_fd);
+    sandbox_host->SetupLaunchEnvironment();
+  }
+
+  options.fds_to_remap = &fds_to_map;
+  base::Process process =
+      use_namespace_sandbox_
+          ? sandbox::NamespaceSandbox::LaunchProcess(*cmd_line, options)
+          : base::LaunchProcess(*cmd_line, options);
+  CHECK(process.IsValid()) << "Failed to launch zygote process";
+
+  dummy_fd.reset();
+  close(fds[1]);
+  control_fd->reset(fds[0]);
+
+  pid_t pid = process.Pid();
+
+  if (use_namespace_sandbox_ || use_suid_sandbox_) {
+    // The namespace and SUID sandbox will execute the zygote in a new
+    // PID namespace, and the main zygote process will then fork from
+    // there. Watch now our elaborate dance to find and validate the
+    // zygote's PID.
+
+    // First we receive a message from the zygote boot process.
+    base::ProcessId boot_pid;
+    CHECK(ReceiveFixedMessage(fds[0], kZygoteBootMessage,
+                              sizeof(kZygoteBootMessage), &boot_pid));
+
+    // Within the PID namespace, the zygote boot process thinks it's PID 1,
+    // but its real PID can never be 1. This gives us a reliable test that
+    // the kernel is translating the sender's PID to our namespace.
+    CHECK_GT(boot_pid, 1)
+        << "Received invalid process ID for zygote; kernel might be too old? "
+           "See crbug.com/357670 or try using --"
+        << switches::kNoSandbox << " to workaround.";
+
+    // Now receive the message that the zygote's ready to go, along with the
+    // main zygote process's ID.
+    pid_t real_pid;
+    CHECK(ReceiveFixedMessage(fds[0], kZygoteHelloMessage,
+                              sizeof(kZygoteHelloMessage), &real_pid));
+    CHECK_GT(real_pid, 1);
+
+    if (real_pid != pid) {
+      // Reap the sandbox.
+      base::EnsureProcessGetsReaped(pid);
+    }
+    pid = real_pid;
+  }
+
+  AddZygotePid(pid);
+  return pid;
 }
 
 #if !defined(OS_OPENBSD)