Linux: SUID sandbox support

  * Make processes dumpable when they crash.
  * Find crashing processes by searching for a socket inode, rather
    than relying on SCM_CREDENTIALS. The kernel doesn't translate PIDs
    between PID namespaces with SCM_CREDENTIALS, so we can't use the
    PID there.
  * Use a command line flag to the renderer to enable crash dumping.
    Previously it tried to access the user's home directory for this
    information.
  * Search for a sandbox helper binary and, if found, use it.
  * Include the source for a sandbox helper binary. It's currently not
    built by default.

http://codereview.chromium.org/149230
R=evan,markus
BUG=8081


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

7 files changed, 209 insertions, 22 deletions

diff --git a/chrome/app/breakpad_linux.cc b/chrome/app/breakpad_linux.cc
index 598ad28..8d8492e 100644
--- a/chrome/app/breakpad_linux.cc
+++ b/chrome/app/breakpad_linux.cc
@@ -479,16 +479,12 @@ RendererCrashHandler(const void* crash_context, size_t crash_context_size,
              void* context) {
   const int fd = (int) context;
   int fds[2];
-  pipe(fds);
+  socketpair(AF_UNIX, SOCK_STREAM, 0, fds);
 
   // The length of the control message:
-  static const unsigned kControlMsgSize =
-      CMSG_SPACE(sizeof(int)) + CMSG_SPACE(sizeof(struct ucred));
+  static const unsigned kControlMsgSize = CMSG_SPACE(sizeof(int));
 
-  union {
-    struct kernel_msghdr msg;
-    struct msghdr sys_msg;
-  };
+  struct kernel_msghdr msg;
   my_memset(&msg, 0, sizeof(struct kernel_msghdr));
   struct kernel_iovec iov[3];
   iov[0].iov_base = const_cast<void*>(crash_context);
@@ -510,14 +506,6 @@ RendererCrashHandler(const void* crash_context, size_t crash_context_size,
   hdr->cmsg_type = SCM_RIGHTS;
   hdr->cmsg_len = CMSG_LEN(sizeof(int));
   *((int*) CMSG_DATA(hdr)) = fds[1];
-  hdr = CMSG_NXTHDR(&sys_msg, hdr);
-  hdr->cmsg_level = SOL_SOCKET;
-  hdr->cmsg_type = SCM_CREDENTIALS;
-  hdr->cmsg_len = CMSG_LEN(sizeof(struct ucred));
-  struct ucred *cred = reinterpret_cast<struct ucred*>(CMSG_DATA(hdr));
-  cred->uid = getuid();
-  cred->gid = getgid();
-  cred->pid = getpid();
 
   HANDLE_EINTR(sys_sendmsg(fd, &msg, 0));
   sys_close(fds[1]);
@@ -538,17 +526,21 @@ void EnableRendererCrashDumping() {
 }
 
 void InitCrashReporter() {
-  if (!GoogleUpdateSettings::GetCollectStatsConsent())
-    return;
-
   // Determine the process type and take appropriate action.
   const CommandLine& parsed_command_line = *CommandLine::ForCurrentProcess();
   const std::wstring process_type =
       parsed_command_line.GetSwitchValue(switches::kProcessType);
   if (process_type.empty()) {
+    if (!GoogleUpdateSettings::GetCollectStatsConsent())
+      return;
     EnableCrashDumping();
   } else if (process_type == switches::kRendererProcess ||
              process_type == switches::kZygoteProcess) {
+    // We might be chrooted in a zygote or renderer process so we cannot call
+    // GetCollectStatsConsent because that needs access the the user's home
+    // dir. Instead, we set a command line flag for these processes.
+    if (!parsed_command_line.HasSwitch(switches::kRendererCrashDump))
+      return;
     EnableRendererCrashDumping();
   }
 }
diff --git a/chrome/browser/renderer_host/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc
index 0736bd7..131d2a9 100644
--- a/chrome/browser/renderer_host/browser_render_process_host.cc
+++ b/chrome/browser/renderer_host/browser_render_process_host.cc
@@ -50,6 +50,7 @@
 #include "chrome/common/render_messages.h"
 #include "chrome/common/result_codes.h"
 #include "chrome/renderer/render_process.h"
+#include "chrome/installer/util/google_update_settings.h"
 #include "grit/generated_resources.h"
 
 #if defined(OS_LINUX)
@@ -389,6 +390,11 @@ bool BrowserRenderProcessHost::Init() {
   }
 #endif  // OS_POSIX
 
+#if defined(OS_LINUX)
+  if (GoogleUpdateSettings::GetCollectStatsConsent())
+    cmd_line.AppendSwitch(switches::kRendererCrashDump);
+#endif
+
   cmd_line.AppendSwitchWithValue(switches::kProcessType,
                                  switches::kRendererProcess);
 
diff --git a/chrome/browser/renderer_host/render_crash_handler_host_linux.cc b/chrome/browser/renderer_host/render_crash_handler_host_linux.cc
index af94386..c3facf8 100644
--- a/chrome/browser/renderer_host/render_crash_handler_host_linux.cc
+++ b/chrome/browser/renderer_host/render_crash_handler_host_linux.cc
@@ -4,11 +4,14 @@
 
 #include "chrome/browser/renderer_host/render_crash_handler_host_linux.h"
 
+#include <dirent.h>
 #include <stdint.h>
-
-#include <unistd.h>
-#include <sys/uio.h>
 #include <sys/socket.h>
+#include <sys/types.h>
+#include <sys/uio.h>
+#include <unistd.h>
+
+#include <vector>
 
 #include "base/eintr_wrapper.h"
 #include "base/format_macros.h"
@@ -22,6 +25,122 @@
 #include "chrome/app/breakpad_linux.h"
 #include "chrome/browser/chrome_thread.h"
 
+// expected prefix of the target of the /proc/self/fd/%d link for a socket
+static const char kSocketLinkPrefix[] = "socket:[";
+
+// Parse a symlink in /proc/pid/fd/$x and return the inode number of the
+// socket.
+//   inode_out: (output) set to the inode number on success
+//   path: e.g. /proc/1234/fd/5 (must be a UNIX domain socket descriptor)
+//   log: if true, log messages about failure details
+static bool ProcPathGetInode(uint64_t* inode_out, const char* path,
+                             bool log = false) {
+  char buf[256];
+  const ssize_t n = readlink(path, buf, sizeof(buf) - 1);
+  if (n == -1) {
+    if (log) {
+      LOG(WARNING) << "Failed to read the inode number for a socket from /proc"
+                      "(" << errno << ")";
+    }
+    return false;
+  }
+  buf[n] = 0;
+
+  if (memcmp(kSocketLinkPrefix, buf, sizeof(kSocketLinkPrefix) - 1)) {
+    if (log) {
+      LOG(WARNING) << "The descriptor passed from the crashing process wasn't a"
+                      " UNIX domain socket.";
+    }
+    return false;
+  }
+
+  char *endptr;
+  const unsigned long long int inode_ul =
+      strtoull(buf + sizeof(kSocketLinkPrefix) - 1, &endptr, 10);
+  if (*endptr != ']')
+    return false;
+
+  if (inode_ul == ULLONG_MAX) {
+    if (log) {
+      LOG(WARNING) << "Failed to parse a socket's inode number: the number was "
+                      "too large. Please report this bug: " << buf;
+    }
+    return false;
+  }
+
+  *inode_out = inode_ul;
+  return true;
+}
+
+// Return the inode number for the UNIX domain socket |fd|.
+static bool FileDescriptorGetInode(uint64_t* inode_out, int fd) {
+  char path[256];
+  if (snprintf(path, sizeof(path), "/proc/self/fd/%d", fd) < 0)
+    return false;
+
+  return ProcPathGetInode(inode_out, path, true);
+}
+
+// Find the process which holds the given socket, named by inode number. If
+// multiple processes hold the socket, this function returns false.
+static bool FindProcessHoldingSocket(pid_t* pid_out, uint64_t socket_inode) {
+  bool already_found = false;
+
+  DIR* proc = opendir("/proc");
+  if (!proc) {
+    LOG(WARNING) << "Cannot open /proc";
+    return false;
+  }
+
+  std::vector<pid_t> pids;
+
+  struct dirent* dent;
+  while ((dent = readdir(proc))) {
+    char *endptr;
+    const unsigned long int pid_ul = strtoul(dent->d_name, &endptr, 10);
+    if (pid_ul == ULONG_MAX || *endptr)
+      continue;
+    pids.push_back(pid_ul);
+  }
+  closedir(proc);
+
+  for (std::vector<pid_t>::const_iterator
+       i = pids.begin(); i != pids.end(); ++i) {
+    const pid_t current_pid = *i;
+    char buf[256];
+    if (snprintf(buf, sizeof(buf), "/proc/%d/fd", current_pid) < 0)
+      continue;
+    DIR* fd = opendir(buf);
+    if (!fd)
+      continue;
+
+    while ((dent = readdir(fd))) {
+      if (snprintf(buf, sizeof(buf), "/proc/%d/fd/%s", current_pid,
+                   dent->d_name) < 0) {
+        continue;
+      }
+
+      uint64_t fd_inode;
+      if (ProcPathGetInode(&fd_inode, buf)) {
+        if (fd_inode == socket_inode) {
+          if (already_found) {
+            closedir(fd);
+            return false;
+          }
+
+          already_found = true;
+          *pid_out = current_pid;
+          break;
+        }
+      }
+    }
+
+    closedir(fd);
+  }
+
+  return already_found;
+}
+
 // Since RenderCrashHandlerHostLinux is a singleton, it's only destroyed at the
 // end of the processes lifetime, which is greater in span then the lifetime of
 // the IO message loop.
@@ -161,6 +280,26 @@ void RenderCrashHandlerHostLinux::OnFileCanReadWithoutBlocking(int fd) {
     return;
   }
 
+  // Kernel bug workaround (broken in 2.6.30 at least):
+  // The kernel doesn't translate PIDs in SCM_CREDENTIALS across PID
+  // namespaces. Thus |crashing_pid| might be garbage from our point of view.
+  // In the future we can remove this workaround, but we have to wait a couple
+  // of years to be sure that it's worked its way out into the world.
+
+  uint64_t inode_number;
+  if (!FileDescriptorGetInode(&inode_number, signal_fd)) {
+    LOG(WARNING) << "Failed to get inode number for passed socket";
+    HANDLE_EINTR(close(signal_fd));
+    return;
+  }
+
+  if (!FindProcessHoldingSocket(&crashing_pid, inode_number - 1)) {
+    LOG(WARNING) << "Failed to find process holding other end of crash reply "
+                    "socket";
+    HANDLE_EINTR(close(signal_fd));
+    return;
+  }
+
   const uint64 rand = base::RandUint64();
   const std::string minidump_filename =
       StringPrintf("/tmp/chromium-renderer-minidump-%016" PRIx64 ".dmp", rand);
diff --git a/chrome/browser/zygote_host_linux.cc b/chrome/browser/zygote_host_linux.cc
index afa55da..905b98f 100644
--- a/chrome/browser/zygote_host_linux.cc
+++ b/chrome/browser/zygote_host_linux.cc
@@ -7,6 +7,7 @@
 #include <unistd.h>
 #include <sys/types.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 
 #include "base/command_line.h"
 #include "base/eintr_wrapper.h"
@@ -14,11 +15,18 @@
 #include "base/path_service.h"
 #include "base/pickle.h"
 #include "base/process_util.h"
+#include "base/string_util.h"
 #include "base/unix_domain_socket_posix.h"
 
 #include "chrome/browser/renderer_host/render_sandbox_host_linux.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 
+// Previously we just looked for the binary next to the Chromium binary. But
+// this breaks people who do a build-all.
+// NOTE packagers: change this.
+static const char kSandboxBinary[] = "/opt/google/chrome/chrome-sandbox";
+
 ZygoteHost::ZygoteHost() {
   std::wstring chrome_path;
   CHECK(PathService::Get(base::FILE_EXE, &chrome_path));
@@ -39,6 +47,27 @@ ZygoteHost::ZygoteHost() {
     cmd_line.PrependWrapper(prefix);
   }
 
+  const std::string kSandboxPath =
+    WideToASCII(std::wstring(L"/var/run/") +
+                chrome::kBrowserProcessExecutableName +
+                L"-sandbox");
+
+  struct stat st;
+  if (stat(kSandboxBinary, &st) == 0) {
+    if (access(kSandboxBinary, X_OK) == 0 &&
+        (st.st_mode & S_ISUID) &&
+        (st.st_mode & S_IXOTH) &&
+        access(kSandboxPath.c_str(), F_OK) == 0) {
+      cmd_line.PrependWrapper(kSandboxBinary);
+    } else {
+      LOG(FATAL) << "The SUID sandbox helper binary was found, but is not "
+                    "configured correctly. Rather than run without sandboxing "
+                    "I'm aborting now. You need to make sure that "
+                 << kSandboxBinary << " is mode 4755 and that "
+                 << kSandboxPath << " exists";
+    }
+  }
+
   // Start up the sandbox host process and get the file descriptor for the
   // renderers to talk to it.
   const int sfd = Singleton<RenderSandboxHostLinux>()->GetRendererSocket();
diff --git a/chrome/browser/zygote_main_linux.cc b/chrome/browser/zygote_main_linux.cc
index 3f9b570..8e4fb55 100644
--- a/chrome/browser/zygote_main_linux.cc
+++ b/chrome/browser/zygote_main_linux.cc
@@ -13,6 +13,7 @@
 #include "base/eintr_wrapper.h"
 #include "base/global_descriptors_posix.h"
 #include "base/pickle.h"
+#include "base/rand_util.h"
 #include "base/unix_domain_socket_posix.h"
 
 #include "chrome/browser/zygote_host_linux.h"
@@ -207,6 +208,10 @@ static bool MaybeEnterChroot() {
       return false;
     const int fd = fd_long;
 
+    // Before entering the sandbox, "prime" any systems that need to open
+    // files and cache the results or the descriptors.
+    base::RandUint64();
+
     static const char kChrootMe = 'C';
     static const char kChrootMeSuccess = 'O';
 
@@ -221,9 +226,18 @@ static bool MaybeEnterChroot() {
     if (chdir("/") == -1)
       return false;
 
-    static const int kMagicSandboxIPCDescriptor = 4;
+    static const int kMagicSandboxIPCDescriptor = 5;
     SkiaFontConfigUseIPCImplementation(kMagicSandboxIPCDescriptor);
 
+    if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0)) {
+      LOG(ERROR) << "CRITICAL: The SUID sandbox is being used, but the chrome "
+                    "binary is also marked as readable. This means that the "
+                    "process starts up dumpable. That means that there's a "
+                    "window where another renderer process can ptrace this "
+                    "process and sequestrate it. This is a packaging error. "
+                    "Please report it as such.";
+    }
+
     prctl(PR_SET_DUMPABLE, 0, 0, 0, 0);
     if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0))
       return false;
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 453426d..83fcc82 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -532,6 +532,11 @@ const wchar_t kEnableMonitorProfile[] = L"enable-monitor-profile";
 // still experimental.
 const wchar_t kEnableXSSAuditor[] = L"enable-xss-auditor";
 
+// A flag, generated internally by Chrome for renderer command lines (Linux
+// only). It tells the renderer to enable crash dumping since it cannot access
+// the user's home directory to find out for itself.
+const wchar_t kRendererCrashDump[] = L"renderer-crash-dumping";
+
 // Enables the new Tabstrip on Windows.
 const wchar_t kEnableTabtastic2[] = L"enable-tabtastic2";
 
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 9ba2999..d4e65af 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -203,6 +203,8 @@ extern const wchar_t kEnableMonitorProfile[];
 
 extern const wchar_t kEnableXSSAuditor[];
 
+extern const wchar_t kRendererCrashDump[];
+
 extern const wchar_t kEnableTabtastic2[];
 
 }  // namespace switches