Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(686)

Unified Diff: sandbox/linux/suid/sandbox.c

Issue 10389214: Revert setuid sandbox as a "init process" changes (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 8 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « sandbox/linux/suid/init_process.c ('k') | sandbox/sandbox.gyp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sandbox/linux/suid/sandbox.c
diff --git a/sandbox/linux/suid/sandbox.c b/sandbox/linux/suid/sandbox.c
index 41a68c73119b4af53157bd07c4a7c30b8ee92de5..ef8049db34769a787d2f88bda4e4a4ab88ddc08b 100644
--- a/sandbox/linux/suid/sandbox.c
+++ b/sandbox/linux/suid/sandbox.c
@@ -1,4 +1,4 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -28,7 +28,6 @@
#include <sys/vfs.h>
#include <unistd.h>
-#include "init_process.h"
#include "linux_util.h"
#include "process_util.h"
#include "suid_unsafe_environment_variables.h"
@@ -72,47 +71,11 @@ static void FatalError(const char *msg, ...) {
#define SAFE_DIR "/proc/self/fdinfo"
#define SAFE_DIR2 "/proc/self/fd"
-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;
-}
-
-static int SpawnChrootHelper() {
+static bool SpawnChrootHelper() {
int sv[2];
if (socketpair(AF_UNIX, SOCK_STREAM, 0, sv) == -1) {
perror("socketpair");
- return -1;
+ return false;
}
char *safedir = NULL;
@@ -124,7 +87,7 @@ static int SpawnChrootHelper() {
safedir = SAFE_DIR2;
else {
fprintf(stderr, "Could not find %s\n", SAFE_DIR2);
- return -1;
+ return false;
}
const pid_t pid = syscall(
@@ -134,7 +97,7 @@ static int SpawnChrootHelper() {
perror("clone");
close(sv[0]);
close(sv[1]);
- return -1;
+ return false;
}
if (pid == 0) {
@@ -162,7 +125,6 @@ static int SpawnChrootHelper() {
FatalError("read");
// do chrooting
- errno = 0;
if (msg != kMsgChrootMe)
FatalError("Unknown message from sandboxed process");
@@ -195,7 +157,7 @@ static int SpawnChrootHelper() {
if (close(sv[0])) {
close(sv[1]);
perror("close");
- return -1;
+ return false;
}
// In the parent process, we install an environment variable containing the
@@ -204,14 +166,13 @@ static int SpawnChrootHelper() {
int printed = snprintf(desc_str, sizeof(desc_str), "%u", sv[1]);
if (printed < 0 || printed >= (int)sizeof(desc_str)) {
fprintf(stderr, "Failed to snprintf\n");
- close(sv[1]);
- return -1;
+ return false;
}
if (setenv(kSandboxDescriptorEnvironmentVarName, desc_str, 1)) {
perror("setenv");
close(sv[1]);
- return -1;
+ return false;
}
// We also install an environment variable containing the pid of the child
@@ -219,51 +180,15 @@ static int SpawnChrootHelper() {
printed = snprintf(helper_pid_str, sizeof(helper_pid_str), "%u", pid);
if (printed < 0 || printed >= (int)sizeof(helper_pid_str)) {
fprintf(stderr, "Failed to snprintf\n");
- close(sv[1]);
- return -1;
+ return false;
}
if (setenv(kSandboxHelperPidEnvironmentVarName, helper_pid_str, 1)) {
perror("setenv");
close(sv[1]);
- return -1;
- }
-
- return sv[1];
-}
-
-static bool JailMe() {
- int fd = SpawnChrootHelper();
- if (fd < 0) {
- return false;
- }
- if (!DropRoot()) {
- close(fd);
- return false;
- }
- ssize_t bytes;
- char ch = kMsgChrootMe;
- do {
- errno = 0;
- bytes = write(fd, &ch, 1);
- } while (bytes == -1 && errno == EINTR);
- if (bytes != 1) {
- perror("write");
- close(fd);
- return false;
- }
- do {
- errno = 0;
- bytes = read(fd, &ch, 1);
- } while (bytes == -1 && errno == EINTR);
- close(fd);
- if (bytes != 1) {
- perror("read");
- return false;
- }
- if (ch != kMsgChrootSuccessful) {
return false;
}
+
return true;
}
@@ -283,51 +208,6 @@ static bool MoveToNewNamespaces() {
_exit(0);
if (pid == 0) {
- if (syscall(__NR_getpid) == 1) {
- int fds[2];
- char ch = 0;
- if (pipe(fds)) {
- perror("Failed to create pipe");
- _exit(1);
- }
- pid = fork();
- if (pid > 0) {
- // The very first process in the new namespace takes on the
- // role of the traditional "init" process. It must reap exit
- // codes of daemon processes until the namespace is completely
- // empty.
- // We have to be careful that this "init" process doesn't
- // provide a new attack surface. So, we also move it into
- // a separate chroot and we drop all privileges. It does
- // still need to access "/proc" and "/dev/null", though. So,
- // we have to provide it with a file handles to these resources.
- // These file handle are not accessible by any other processes in
- // the sandbox and thus safe.
- close(fds[0]);
- int proc_fd = open("/proc", O_RDONLY | O_DIRECTORY);
- int null_fd = open("/dev/null", O_RDWR);
- if (!JailMe()) {
- FatalError("Could not remove privileges from "
- "new \"init\" process");
- }
- SystemInitProcess(fds[1], pid, proc_fd, null_fd);
- } else if (pid != 0) {
- perror("Failed to fork");
- _exit(1);
- }
- // Wait for the "init" process to complete initialization.
- close(fds[1]);
- errno = 0;
- while (read(fds[0], &ch, 1) < 0 && errno == EINTR) {
- }
- close(fds[0]);
- if (ch != ' ') {
- // We'll likely never get here. If the "init" process fails, it's
- // death typically takes everyone of its children with it.
- FatalError("Failed to set up new \"init\" process inside sandbox");
- }
- }
-
if (kCloneExtraFlags[i] & CLONE_NEWPID) {
setenv("SBX_PID_NS", "", 1 /* overwrite */);
} else {
@@ -353,6 +233,42 @@ static bool MoveToNewNamespaces() {
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;
+}
+
static bool SetupChildEnvironment() {
unsigned i;
@@ -447,7 +363,7 @@ int main(int argc, char **argv) {
if (!MoveToNewNamespaces())
return 1;
- if (SpawnChrootHelper() < 0)
+ if (!SpawnChrootHelper())
return 1;
if (!DropRoot())
return 1;
« no previous file with comments | « sandbox/linux/suid/init_process.c ('k') | sandbox/sandbox.gyp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698