diff options
author | mdempsky <mdempsky@chromium.org> | 2014-09-12 13:29:34 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2014-09-12 20:32:27 +0000 |
commit | 3f37ce216d768da8795a80860b00a4ad45735bd9 (patch) | |
tree | 2d47d36dcee3170a112518cb0abc4e9f9dd18c2e /sandbox/linux/suid | |
parent | b546ca9a2f80dcf22412d47102bb37d59c8d034d (diff) | |
download | chromium_src-3f37ce216d768da8795a80860b00a4ad45735bd9.zip chromium_src-3f37ce216d768da8795a80860b00a4ad45735bd9.tar.gz chromium_src-3f37ce216d768da8795a80860b00a4ad45735bd9.tar.bz2 |
Remove --find-inode-switch hack from chrome-sandbox
Chrome was modified back in May 2014 to use SCM_CREDENTIALS instead of
--find-inode-switch and we haven't any heard any negative feedback, so
it's time to remove --find-inode-switch entirely.
BUG=357670
Review URL: https://codereview.chromium.org/569533002
Cr-Commit-Position: refs/heads/master@{#294649}
Diffstat (limited to 'sandbox/linux/suid')
-rw-r--r-- | sandbox/linux/suid/common/sandbox.h | 1 | ||||
-rw-r--r-- | sandbox/linux/suid/linux_util.c | 140 | ||||
-rw-r--r-- | sandbox/linux/suid/linux_util.h | 21 | ||||
-rw-r--r-- | sandbox/linux/suid/sandbox.c | 31 |
4 files changed, 3 insertions, 190 deletions
diff --git a/sandbox/linux/suid/common/sandbox.h b/sandbox/linux/suid/common/sandbox.h index 9345287..99eb7b5 100644 --- a/sandbox/linux/suid/common/sandbox.h +++ b/sandbox/linux/suid/common/sandbox.h @@ -11,6 +11,7 @@ namespace sandbox { // These are command line switches that may be used by other programs // (e.g. Chrome) to construct a command line for the sandbox. +static const char kSuidSandboxGetApiSwitch[] = "--get-api"; static const char kAdjustOOMScoreSwitch[] = "--adjust-oom-score"; static const char kSandboxDescriptorEnvironmentVarName[] = "SBX_D"; diff --git a/sandbox/linux/suid/linux_util.c b/sandbox/linux/suid/linux_util.c deleted file mode 100644 index 9febe6d9..0000000 --- a/sandbox/linux/suid/linux_util.c +++ /dev/null @@ -1,140 +0,0 @@ -// 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. - -// The following is duplicated from base/linux_utils.cc. -// We shouldn't link against C++ code in a setuid binary. - -// Needed for O_DIRECTORY, must be defined before fcntl.h is included -// (and it can be included earlier than the explicit #include below -// in some versions of glibc). -#define _GNU_SOURCE - -#include "sandbox/linux/suid/linux_util.h" - -#include <dirent.h> -#include <errno.h> -#include <fcntl.h> -#include <limits.h> -#include <stdio.h> -#include <stdlib.h> -#include <string.h> -#include <sys/stat.h> -#include <sys/types.h> -#include <unistd.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) -static bool ProcPathGetInodeAt(ino_t* inode_out, - int base_dir_fd, - const char* path) { - // We also check that the path is relative. - if (!inode_out || !path || *path == '/') - return false; - char buf[256]; - const ssize_t n = readlinkat(base_dir_fd, path, buf, sizeof(buf) - 1); - if (n < 0) - return false; - buf[n] = 0; - - if (memcmp(kSocketLinkPrefix, buf, sizeof(kSocketLinkPrefix) - 1)) - return false; - - char* endptr = NULL; - errno = 0; - const unsigned long long int inode_ull = - strtoull(buf + sizeof(kSocketLinkPrefix) - 1, &endptr, 10); - if (inode_ull == ULLONG_MAX || !endptr || *endptr != ']' || errno != 0) - return false; - - *inode_out = inode_ull; - return true; -} - -static DIR* opendirat(int base_dir_fd, const char* name) { - // Also check that |name| is relative. - if (base_dir_fd < 0 || !name || *name == '/') - return NULL; - int new_dir_fd = openat(base_dir_fd, name, O_RDONLY | O_DIRECTORY); - if (new_dir_fd < 0) - return NULL; - - return fdopendir(new_dir_fd); -} - -bool FindProcessHoldingSocket(pid_t* pid_out, ino_t socket_inode) { - bool already_found = false; - - DIR* proc = opendir("/proc"); - if (!proc) - return false; - - const uid_t uid = getuid(); - struct dirent* dent; - while ((dent = readdir(proc))) { - char* endptr = NULL; - errno = 0; - const unsigned long int pid_ul = strtoul(dent->d_name, &endptr, 10); - if (pid_ul == ULONG_MAX || !endptr || *endptr || errno != 0) - continue; - - // We have this setuid code here because the zygote and its children have - // /proc/$pid/fd owned by root. While scanning through /proc, we add this - // extra check so users cannot accidentally gain information about other - // users' processes. To determine process ownership, we use the property - // that if user foo owns process N, then /proc/N is owned by foo. - int proc_pid_fd = -1; - { - char buf[256]; - struct stat statbuf; - snprintf(buf, sizeof(buf), "/proc/%lu", pid_ul); - proc_pid_fd = open(buf, O_RDONLY | O_DIRECTORY); - if (proc_pid_fd < 0) - continue; - if (fstat(proc_pid_fd, &statbuf) < 0 || uid != statbuf.st_uid) { - close(proc_pid_fd); - continue; - } - } - - DIR* fd = opendirat(proc_pid_fd, "fd"); - if (!fd) { - close(proc_pid_fd); - continue; - } - - while ((dent = readdir(fd))) { - char buf[256]; - int printed = snprintf(buf, sizeof(buf), "fd/%s", dent->d_name); - if (printed < 0 || printed >= (int)(sizeof(buf) - 1)) { - continue; - } - - ino_t fd_inode; - if (ProcPathGetInodeAt(&fd_inode, proc_pid_fd, buf)) { - if (fd_inode == socket_inode) { - if (already_found) { - closedir(fd); - close(proc_pid_fd); - closedir(proc); - return false; - } - - already_found = true; - *pid_out = pid_ul; - break; - } - } - } - closedir(fd); - close(proc_pid_fd); - } - closedir(proc); - - return already_found; -} diff --git a/sandbox/linux/suid/linux_util.h b/sandbox/linux/suid/linux_util.h deleted file mode 100644 index d064252..0000000 --- a/sandbox/linux/suid/linux_util.h +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) 2012 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. - -// The following is duplicated from base/linux_utils.h. -// We shouldn't link against C++ code in a setuid binary. - -#ifndef SANDBOX_LINUX_SUID_LINUX_UTIL_H_ -#define SANDBOX_LINUX_SUID_LINUX_UTIL_H_ - -#include <stdbool.h> -#include <sys/types.h> - -static const char kFindInodeSwitch[] = "--find-inode"; -static const char kSuidSandboxGetApiSwitch[] = "--get-api"; - -// Find the process which holds the given socket, named by inode number. If -// multiple processes hold the socket, this function returns false. -bool FindProcessHoldingSocket(pid_t* pid_out, ino_t socket_inode); - -#endif // SANDBOX_LINUX_SUID_LINUX_UTIL_H_ diff --git a/sandbox/linux/suid/sandbox.c b/sandbox/linux/suid/sandbox.c index 7410b71..3049ae5 100644 --- a/sandbox/linux/suid/sandbox.c +++ b/sandbox/linux/suid/sandbox.c @@ -30,7 +30,6 @@ #include <unistd.h> #include "sandbox/linux/suid/common/suid_unsafe_environment_variables.h" -#include "sandbox/linux/suid/linux_util.h" #include "sandbox/linux/suid/process_util.h" #if !defined(CLONE_NEWPID) @@ -433,34 +432,8 @@ int main(int argc, char** argv) { return 0; } - // In the SUID sandbox, if we succeed in calling MoveToNewNamespaces() - // below, then the zygote and all the renderers are in an alternate PID - // namespace and do not know their real PIDs. As such, they report the wrong - // PIDs to the task manager. - // - // To fix this, when the zygote spawns a new renderer, it gives the renderer - // a dummy socket, which has a unique inode number. Then it asks the sandbox - // host to find the PID of the process holding that fd by searching /proc. - // - // Since the zygote and renderers are all spawned by this setuid executable, - // their entries in /proc are owned by root and only readable by root. In - // order to search /proc for the fd we want, this setuid executable has to - // double as a helper and perform the search. The code block below does this - // when you call it with --find-inode INODE_NUMBER. - if (argc == 3 && (0 == strcmp(argv[1], kFindInodeSwitch))) { - pid_t pid; - char* endptr = NULL; - errno = 0; - ino_t inode = strtoull(argv[2], &endptr, 10); - if (inode == ULLONG_MAX || !endptr || *endptr || errno != 0) - return 1; - if (!FindProcessHoldingSocket(&pid, inode)) - return 1; - printf("%d\n", pid); - return 0; - } - // Likewise, we cannot adjust /proc/pid/oom_adj for sandboxed renderers - // because those files are owned by root. So we need another helper here. + // We cannot adjust /proc/pid/oom_adj for sandboxed renderers + // because those files are owned by root. So we need a helper here. if (argc == 4 && (0 == strcmp(argv[1], kAdjustOOMScoreSwitch))) { char* endptr = NULL; long score; |