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

Unified Diff: content/browser/zygote_host/zygote_host_impl_linux.cc

Issue 1976403002: Fix logic for checking chrome-sandbox setuid binary (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: rickyz feedback Created 4 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 | « content/browser/zygote_host/zygote_host_impl_linux.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..38b8cf6b3b584322b5b24022216c43e2928f0c5f 100644
--- a/content/browser/zygote_host/zygote_host_impl_linux.cc
+++ b/content/browser/zygote_host/zygote_host_impl_linux.cc
@@ -4,26 +4,59 @@
#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) {
+ // Allocate an extra byte of buffer space so we can check that we received
+ // exactly |expect_len| bytes, and the message wasn't just truncated to fit.
+ char buf[expect_len + 1];
+ 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 +69,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 +125,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 +133,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)
« no previous file with comments | « content/browser/zygote_host/zygote_host_impl_linux.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698