Create a Zygote which starts unsandboxed for utility processes.

This ensures that utility processes don't end up starting with a
different on disk binary than is currently running.

This works by adding a new "--no-zygote-sandbox" command line flag
which is set in ZygoteCommunication based on a construction flag. It's
used to launch the zygote without the setuid and namespace sandboxes;
which sounds scary, but is already the case today for utility processes
with specialized sandboxes (net, ime, audio, soda).

Within the ZygoteMain this functions as if "--no-sandbox" was specified
to the Zygote and skips its sandbox setup. The actual sandbox is later
applied by UtilityMain().

Bug: 22703,1049234
Change-Id: Ib61e7c71c6ee43e2ab518fa0ed109bea36677bff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2047932
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742875}

content/app/content_main_runner_impl.cc

@@ -287,6 +287,7 @@ void InitializeZygoteSandboxForBrowserProcess(
 
   // Tickle the zygote host so it forks now.
   service_manager::ZygoteHostImpl::GetInstance()->Init(parsed_command_line);
+  service_manager::CreateUnsandboxedZygote(base::BindOnce(LaunchZygoteHelper));
   service_manager::ZygoteHandle generic_zygote =
       service_manager::CreateGenericZygote(base::BindOnce(LaunchZygoteHelper));
 

content/browser/utility_process_host.cc

@@ -169,15 +169,23 @@ class UtilitySandboxedProcessLauncherDelegate
 
 #if BUILDFLAG(USE_ZYGOTE_HANDLE)
   service_manager::ZygoteHandle GetZygote() override {
-    if (service_manager::IsUnsandboxedSandboxType(sandbox_type_) ||
+    // If the sandbox has been disabled for a given type, don't use a zygote.
-        sandbox_type_ == service_manager::SandboxType::kNetwork ||
+    if (service_manager::IsUnsandboxedSandboxType(sandbox_type_))
+      return nullptr;
+
+    // Utility processes which need specialized sandboxes fork from the
+    // unsandboxed zygote and then apply their actual sandboxes in the forked
+    // process upon startup.
+    if (sandbox_type_ == service_manager::SandboxType::kNetwork ||
 #if defined(OS_CHROMEOS)
         sandbox_type_ == service_manager::SandboxType::kIme ||
 #endif  // OS_CHROMEOS
         sandbox_type_ == service_manager::SandboxType::kAudio ||
         sandbox_type_ == service_manager::SandboxType::kSoda) {
-      return nullptr;
+      return service_manager::GetUnsandboxedZygote();
     }
+
+    // All other types use the pre-sandboxed zygote.
     return service_manager::GetGenericZygote();
   }
 #endif  // BUILDFLAG(USE_ZYGOTE_HANDLE)

content/zygote/zygote_browsertest.cc

@@ -13,9 +13,12 @@
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "services/service_manager/embedder/switches.h"
+#include "services/service_manager/sandbox/linux/sandbox_linux.h"
 #include "services/service_manager/sandbox/switches.h"
 #include "services/service_manager/zygote/common/zygote_buildflags.h"
 #if BUILDFLAG(USE_ZYGOTE_HANDLE)
+#include "services/service_manager/zygote/common/zygote_handle.h"
+#include "services/service_manager/zygote/host/zygote_communication_linux.h"
 #include "services/service_manager/zygote/host/zygote_host_impl_linux.h"
 #endif
 
@@ -23,8 +26,8 @@ namespace content {
 
 class LinuxZygoteBrowserTest : public ContentBrowserTest {
  public:
-  LinuxZygoteBrowserTest() {}
+  LinuxZygoteBrowserTest() = default;
-  ~LinuxZygoteBrowserTest() override {}
+  ~LinuxZygoteBrowserTest() override = default;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(LinuxZygoteBrowserTest);
@@ -46,10 +49,30 @@ IN_PROC_BROWSER_TEST_F(LinuxZygoteBrowserTest, GetLocalTimeHasTimeZone) {
   EXPECT_TRUE(parts[2].empty());
 }
 
+#if BUILDFLAG(USE_ZYGOTE_HANDLE)
+IN_PROC_BROWSER_TEST_F(LinuxZygoteBrowserTest, ZygoteSandboxes) {
+  // We need zygotes and the standard sandbox config to run this test.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoZygote) ||
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          service_manager::switches::kNoSandbox)) {
+    return;
+  }
+
+  // Sanity check the sandbox flags we expect to be everywhere.
+  const int flags = service_manager::GetGenericZygote()->GetSandboxStatus();
+  constexpr int kExpectedFlags = service_manager::SandboxLinux::kPIDNS |
+                                 service_manager::SandboxLinux::kNetNS |
+                                 service_manager::SandboxLinux::kUserNS;
+  EXPECT_EQ(kExpectedFlags, flags & kExpectedFlags);
+
+  EXPECT_EQ(service_manager::GetUnsandboxedZygote()->GetSandboxStatus(), 0);
+}
+#endif
+
 class LinuxZygoteDisabledBrowserTest : public ContentBrowserTest {
  public:
-  LinuxZygoteDisabledBrowserTest() {}
+  LinuxZygoteDisabledBrowserTest() = default;
-  ~LinuxZygoteDisabledBrowserTest() override {}
+  ~LinuxZygoteDisabledBrowserTest() override = default;
 
  protected:
   void SetUpCommandLine(base::CommandLine* command_line) override {

services/service_manager/sandbox/switches.cc

@@ -78,6 +78,12 @@ const char kGpuSandboxFailuresFatal[] = "gpu-sandbox-failures-fatal";
 // Disables the sandbox for all process types that are normally sandboxed.
 const char kNoSandbox[] = "no-sandbox";
 
+#if defined(OS_LINUX)
+// Instructs the zygote to launch without a sandbox. Processes forked from this
+// type of zygote will apply their own custom sandboxes later.
+const char kNoZygoteSandbox[] = "no-zygote-sandbox";
+#endif
+
 #if defined(OS_WIN)
 // Allows third party modules to inject by disabling the BINARY_SIGNATURE
 // mitigation policy on Win10+. Also has other effects in ELF.

services/service_manager/sandbox/switches.h

@@ -49,6 +49,9 @@ SERVICE_MANAGER_SANDBOX_EXPORT extern const char kEnableAudioServiceSandbox[];
 SERVICE_MANAGER_SANDBOX_EXPORT extern const char kGpuSandboxAllowSysVShm[];
 SERVICE_MANAGER_SANDBOX_EXPORT extern const char kGpuSandboxFailuresFatal[];
 SERVICE_MANAGER_SANDBOX_EXPORT extern const char kNoSandbox[];
+#if defined(OS_LINUX)
+SERVICE_MANAGER_SANDBOX_EXPORT extern const char kNoZygoteSandbox[];
+#endif
 #if defined(OS_WIN)
 SERVICE_MANAGER_SANDBOX_EXPORT extern const char kAllowThirdPartyModules[];
 SERVICE_MANAGER_SANDBOX_EXPORT extern const char kAddGpuAppContainerCaps[];

services/service_manager/zygote/common/zygote_handle.h

@@ -26,22 +26,24 @@ using ZygoteHandle = ZygoteCommunication*;
 #error "Can not use zygote handles on this platform"
 #endif  // defined(OS_POSIX)
 
+using ZygoteLaunchCallback =
+    base::OnceCallback<pid_t(base::CommandLine*, base::ScopedFD*)>;
+
 // Allocates and initializes the global generic zygote process, and returns the
-// ZygoteHandle used to communicate with it. |launcher| is a callback that
+// ZygoteHandle used to communicate with it. |launch_cb| is a callback that
 // should actually launch the process, after adding additional command line
 // switches to the ones composed by this function. It returns the pid created,
 // and provides a control fd for it.
-
 COMPONENT_EXPORT(SERVICE_MANAGER_ZYGOTE)
-ZygoteHandle CreateGenericZygote(
+ZygoteHandle CreateGenericZygote(ZygoteLaunchCallback launch_cb);
-    base::OnceCallback<pid_t(base::CommandLine*, base::ScopedFD*)> launcher);
-
-// Returns a handle to a global generic zygote object. This function allows the
-// browser to launch and use a single zygote process until the performance
-// issues around launching multiple zygotes are resolved.
-// http://crbug.com/569191
 COMPONENT_EXPORT(SERVICE_MANAGER_ZYGOTE) ZygoteHandle GetGenericZygote();
 
+// Similar to the above but for creating an unsandboxed zygote from which
+// processes which need non-generic sandboxes can be derived.
+COMPONENT_EXPORT(SERVICE_MANAGER_ZYGOTE)
+ZygoteHandle CreateUnsandboxedZygote(ZygoteLaunchCallback launch_cb);
+COMPONENT_EXPORT(SERVICE_MANAGER_ZYGOTE) ZygoteHandle GetUnsandboxedZygote();
+
 }  // namespace service_manager
 
 #endif  // SERVICES_SERVICE_MANAGER_ZYGOTE_COMMON_ZYGOTE_HANDLE_H_

services/service_manager/zygote/host/zygote_communication_linux.cc

@@ -25,12 +25,9 @@
 
 namespace service_manager {
 
-ZygoteCommunication::ZygoteCommunication()
+ZygoteCommunication::ZygoteCommunication(ZygoteType type)
-    : control_fd_(),
+    : type_(type),
-      control_lock_(),
       pid_(),
-      list_of_running_zygote_children_(),
-      child_tracking_lock_(),
       sandbox_status_(0),
       have_read_sandbox_status_word_(false),
       init_(false) {}
@@ -230,6 +227,9 @@ void ZygoteCommunication::Init(
   base::CommandLine cmd_line(chrome_path);
   cmd_line.AppendSwitchASCII(switches::kProcessType, switches::kZygoteProcess);
 
+  if (type_ == ZygoteType::kUnsandboxed)
+    cmd_line.AppendSwitch(switches::kNoZygoteSandbox);
+
   const base::CommandLine& browser_command_line =
       *base::CommandLine::ForCurrentProcess();
   if (browser_command_line.HasSwitch(switches::kZygoteCmdPrefix)) {

services/service_manager/zygote/host/zygote_communication_linux.h

@@ -31,7 +31,8 @@ namespace service_manager {
 // https://chromium.googlesource.com/chromium/src/+/master/docs/linux/sandbox_ipc.md
 class COMPONENT_EXPORT(SERVICE_MANAGER_ZYGOTE) ZygoteCommunication {
  public:
-  ZygoteCommunication();
+  enum class ZygoteType { kSandboxed, kUnsandboxed };
+  explicit ZygoteCommunication(ZygoteType type);
   ~ZygoteCommunication();
 
   void Init(
@@ -81,6 +82,9 @@ class COMPONENT_EXPORT(SERVICE_MANAGER_ZYGOTE) ZygoteCommunication {
   // Get the sandbox status from the zygote.
   ssize_t ReadSandboxStatus();
 
+  // Indicates whether the Zygote starts unsandboxed or not.
+  const ZygoteType type_;
+
   base::ScopedFD control_fd_;  // the socket to the zygote.
   // A lock protecting all communication with the zygote. This lock must be
   // acquired before sending a command and released after the result has been

services/service_manager/zygote/host/zygote_handle_linux.cc

@@ -11,14 +11,15 @@ namespace {
 
 // Intentionally leaked.
 ZygoteHandle g_generic_zygote = nullptr;
+ZygoteHandle g_unsandboxed_zygote = nullptr;
 
 }  // namespace
 
-ZygoteHandle CreateGenericZygote(
+ZygoteHandle CreateGenericZygote(ZygoteLaunchCallback launch_cb) {
-    base::OnceCallback<pid_t(base::CommandLine*, base::ScopedFD*)> launcher) {
   CHECK(!g_generic_zygote);
-  g_generic_zygote = new ZygoteCommunication();
+  g_generic_zygote =
-  g_generic_zygote->Init(std::move(launcher));
+      new ZygoteCommunication(ZygoteCommunication::ZygoteType::kSandboxed);
+  g_generic_zygote->Init(std::move(launch_cb));
   return g_generic_zygote;
 }
 
@@ -27,4 +28,17 @@ ZygoteHandle GetGenericZygote() {
   return g_generic_zygote;
 }
 
+ZygoteHandle CreateUnsandboxedZygote(ZygoteLaunchCallback launch_cb) {
+  CHECK(!g_unsandboxed_zygote);
+  g_unsandboxed_zygote =
+      new ZygoteCommunication(ZygoteCommunication::ZygoteType::kUnsandboxed);
+  g_unsandboxed_zygote->Init(std::move(launch_cb));
+  return g_unsandboxed_zygote;
+}
+
+ZygoteHandle GetUnsandboxedZygote() {
+  CHECK(g_unsandboxed_zygote);
+  return g_unsandboxed_zygote;
+}
+
 }  // namespace service_manager

services/service_manager/zygote/host/zygote_host_impl_linux.cc

@@ -154,8 +154,11 @@ pid_t ZygoteHostImpl::LaunchZygote(
   options.fds_to_remap = std::move(additional_remapped_fds);
   options.fds_to_remap.emplace_back(fds[1], kZygoteSocketPairFd);
 
+  const bool is_sandboxed_zygote =
+      !cmd_line->HasSwitch(service_manager::switches::kNoZygoteSandbox);
+
   base::ScopedFD dummy_fd;
-  if (use_suid_sandbox_) {
+  if (is_sandboxed_zygote && use_suid_sandbox_) {
     std::unique_ptr<sandbox::SetuidSandboxHost> sandbox_host(
         sandbox::SetuidSandboxHost::Create());
     sandbox_host->PrependWrapper(cmd_line);
@@ -164,7 +167,7 @@ pid_t ZygoteHostImpl::LaunchZygote(
   }
 
   base::Process process =
-      use_namespace_sandbox_
+      (is_sandboxed_zygote && use_namespace_sandbox_)
           ? sandbox::NamespaceSandbox::LaunchProcess(*cmd_line, options)
           : base::LaunchProcess(*cmd_line, options);
   CHECK(process.IsValid()) << "Failed to launch zygote process";
@@ -175,7 +178,7 @@ pid_t ZygoteHostImpl::LaunchZygote(
 
   pid_t pid = process.Pid();
 
-  if (use_namespace_sandbox_ || use_suid_sandbox_) {
+  if (is_sandboxed_zygote && (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

services/service_manager/zygote/zygote_main_linux.cc

@@ -183,7 +183,9 @@ bool ZygoteMain(
   // Skip pre-initializing sandbox when sandbox is disabled for
   // https://crbug.com/444900.
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
-          service_manager::switches::kNoSandbox)) {
+          service_manager::switches::kNoSandbox) &&
+      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+          service_manager::switches::kNoZygoteSandbox)) {
     // This will pre-initialize the various sandboxes that need it.
     linux_sandbox->PreinitializeSandbox();
   }