diff options
author | agl@chromium.org <agl@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-08 01:15:14 +0000 |
---|---|---|
committer | agl@chromium.org <agl@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-07-08 01:15:14 +0000 |
commit | 4378a822c0f819edb40d6903a9fa363d7c72c84d (patch) | |
tree | a67ad84d03f67605dd636d1ad913d487db0e348f | |
parent | 0e0b9771cc4fe496403a49126ec7cfa6c422a6d0 (diff) | |
download | chromium_src-4378a822c0f819edb40d6903a9fa363d7c72c84d.zip chromium_src-4378a822c0f819edb40d6903a9fa363d7c72c84d.tar.gz chromium_src-4378a822c0f819edb40d6903a9fa363d7c72c84d.tar.bz2 |
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
-rw-r--r-- | breakpad/linux/exception_handler.cc | 3 | ||||
-rw-r--r-- | build/all.gyp | 3 | ||||
-rw-r--r-- | chrome/app/breakpad_linux.cc | 28 | ||||
-rw-r--r-- | chrome/browser/renderer_host/browser_render_process_host.cc | 6 | ||||
-rw-r--r-- | chrome/browser/renderer_host/render_crash_handler_host_linux.cc | 145 | ||||
-rw-r--r-- | chrome/browser/zygote_host_linux.cc | 29 | ||||
-rw-r--r-- | chrome/browser/zygote_main_linux.cc | 16 | ||||
-rw-r--r-- | chrome/common/chrome_switches.cc | 5 | ||||
-rw-r--r-- | chrome/common/chrome_switches.h | 2 | ||||
-rw-r--r-- | sandbox/linux/suid/sandbox.cc | 224 | ||||
-rw-r--r-- | sandbox/sandbox.gyp | 11 |
11 files changed, 449 insertions, 23 deletions
diff --git a/breakpad/linux/exception_handler.cc b/breakpad/linux/exception_handler.cc index a1c8379..4a4955e 100644 --- a/breakpad/linux/exception_handler.cc +++ b/breakpad/linux/exception_handler.cc @@ -258,6 +258,9 @@ bool ExceptionHandler::HandleSignal(int sig, siginfo_t* info, void* uc) { if (filter_ && !filter_(callback_context_)) return false; + // Allow ourselves to be dumped. + sys_prctl(PR_SET_DUMPABLE, 1); + CrashContext context; memcpy(&context.siginfo, info, sizeof(siginfo_t)); memcpy(&context.context, uc, sizeof(struct ucontext)); diff --git a/build/all.gyp b/build/all.gyp index 63b8c23..3ef6f6d 100644 --- a/build/all.gyp +++ b/build/all.gyp @@ -48,10 +48,11 @@ }], ['OS=="linux"', { 'dependencies': [ + '../breakpad/breakpad.gyp:*', + '../sandbox/sandbox.gyp:*', '../third_party/harfbuzz/harfbuzz.gyp:*', '../tools/gtk_clipboard_dump/gtk_clipboard_dump.gyp:*', '../tools/xdisplaycheck/xdisplaycheck.gyp:*', - '../breakpad/breakpad.gyp:*', ], }], ['OS=="win"', { 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 diff --git a/sandbox/linux/suid/sandbox.cc b/sandbox/linux/suid/sandbox.cc new file mode 100644 index 0000000..1ea2af3 --- /dev/null +++ b/sandbox/linux/suid/sandbox.cc @@ -0,0 +1,224 @@ +// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include <asm/unistd.h> +#include <errno.h> +#include <sched.h> +#include <signal.h> +#include <stdarg.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/prctl.h> +#include <sys/resource.h> +#include <sys/socket.h> +#include <sys/stat.h> +#include <sys/time.h> +#include <sys/types.h> +#include <unistd.h> + +#if !defined(CLONE_NEWPID) +#define CLONE_NEWPID 0x20000000 +#endif + +static const char kSandboxPath[] = "/var/run/chrome-sandbox"; +static const char kChromeBinary[] = "/opt/google/chrome/chrome"; +static const char kSandboxDescriptorEnvironmentVarName[] = "SBX_D"; + +// These are the magic byte values which the sandboxed process uses to request +// that it be chrooted. +static const char kMsgChrootMe = 'C'; +static const char kMsgChrootSuccessful = 'O'; + +static void FatalError(const char *msg, ...) + __attribute__((noreturn, format(printf,1,2))); + +static void FatalError(const char *msg, ...) { + va_list ap; + va_start(ap, msg); + + vfprintf(stderr, msg, ap); + fprintf(stderr, ": %s\n", strerror(errno)); + fflush(stderr); + exit(1); +} + +static int CloneChrootHelperProcess() { + int sv[2]; + if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv) == -1) { + perror("socketpair"); + return -1; + } + + const pid_t pid = syscall( + __NR_clone, CLONE_FS | SIGCHLD, 0, 0, 0); + + if (pid == -1) { + perror("clone"); + close(sv[0]); + close(sv[1]); + return -1; + } + + if (pid == 0) { + // We share our files structure with an untrusted process. As a security in + // depth measure, we make sure that we can't open anything by mistake. + // TODO: drop CAP_SYS_RESOURCE + + const struct rlimit nofile = {0, 0}; + if (setrlimit(RLIMIT_NOFILE, &nofile)) + FatalError("Setting RLIMIT_NOFILE"); + + if (close(sv[1])) + FatalError("close"); + + char msg; + ssize_t bytes; + do { + bytes = read(sv[0], &msg, 1); + } while (bytes == -1 && errno == EINTR); + + if (bytes == 0) + _exit(0); + if (bytes != 1) + FatalError("read"); + + if (msg != kMsgChrootMe) + FatalError("Unknown message from sandboxed process"); + + if (chdir(kSandboxPath)) + FatalError("Cannot chdir into %s", kSandboxPath); + + struct stat st; + if (stat(".", &st)) + FatalError("stat"); + + if (st.st_uid || st.st_gid || st.st_mode & S_IWOTH) + FatalError("Bad permissions on chroot directory (%s)", kSandboxPath); + + if (chroot(".")) + FatalError("Cannot chroot into %s", kSandboxPath); + + if (chdir("/")) + FatalError("Cannot chdir to / after chroot"); + + const char reply = kMsgChrootSuccessful; + do { + bytes = write(sv[0], &reply, 1); + } while (bytes == -1 && errno == EINTR); + + if (bytes != 1) + FatalError("Writing reply"); + + _exit(0); + } + + if (close(sv[0])) { + close(sv[1]); + perror("close"); + return false; + } + + return sv[1]; +} + +static bool SpawnChrootHelper() { + const int chroot_signal_fd = CloneChrootHelperProcess(); + + if (chroot_signal_fd == -1) + return false; + + // In the parent process, we install an environment variable containing the + // number of the file descriptor. + char desc_str[64]; + snprintf(desc_str, sizeof(desc_str), "%d", chroot_signal_fd); + + if (setenv(kSandboxDescriptorEnvironmentVarName, desc_str, 1)) { + perror("setenv"); + close(chroot_signal_fd); + return false; + } + + return true; +} + +static bool MoveToNewPIDNamespace() { + const pid_t pid = syscall( + __NR_clone, CLONE_NEWPID | SIGCHLD, 0, 0, 0); + + if (pid == -1) { + if (errno == EINVAL) { + // System doesn't support NEWPID. We carry on anyway. + return true; + } + + perror("Failed to move to new PID namespace"); + return false; + } + + if (pid) + _exit(0); + + return true; +} + +static bool DropRoot() { + if (prctl(PR_SET_DUMPABLE, 0, 0, 0, 0)) { + perror("prctl(PR_SET_DUMPABLE)"); + return false; + } + + if (prctl(PR_GET_DUMPABLE, 0, 0, 0, 0)) { + perror("Still dumpable after prctl(PR_SET_DUMPABLE)"); + return false; + } + + gid_t rgid, egid, sgid; + if (getresgid(&rgid, &egid, &sgid)) { + perror("getresgid"); + return false; + } + + if (setresgid(rgid, rgid, rgid)) { + perror("setresgid"); + return false; + } + + uid_t ruid, euid, suid; + if (getresuid(&ruid, &euid, &suid)) { + perror("getresuid"); + return false; + } + + if (setresuid(ruid, ruid, ruid)) { + perror("setresuid"); + return false; + } + + return true; +} + +int main(int argc, char **argv) { + if (argc == 1) { + fprintf(stderr, "Usage: %s <renderer process> <args...>\n", argv[0]); + return 1; + } + + if (strcmp(argv[1], kChromeBinary)) { + fprintf(stderr, "This wrapper can only run %s!\n", kChromeBinary); + return 1; + } + + if (!MoveToNewPIDNamespace()) + return 1; + if (!SpawnChrootHelper()) + return 1; + if (!DropRoot()) + return 1; + + execv(argv[1], &argv[1]); + FatalError("execv failed"); + + return 1; +} diff --git a/sandbox/sandbox.gyp b/sandbox/sandbox.gyp index b9506bb..cfb6600 100644 --- a/sandbox/sandbox.gyp +++ b/sandbox/sandbox.gyp @@ -7,6 +7,17 @@ '../build/common.gypi', ], 'conditions': [ + [ 'OS=="linux"', { + 'targets': [ + { + 'target_name': 'chrome-sandbox', + 'type': 'executable', + 'sources': [ + 'linux/suid/sandbox.cc', + ], + } + ], + }], [ 'OS=="win"', { 'targets': [ { |