feat: support mixed-sandbox mode on linux (#15870)

atom/browser/web_contents_preferences.cc

@@ -245,10 +245,12 @@ void WebContentsPreferences::AppendCommandLineSwitches(
   // If the `sandbox` option was passed to the BrowserWindow's webPreferences,
   // pass `--enable-sandbox` to the renderer so it won't have any node.js
   // integration.
-  if (IsEnabled(options::kSandbox))
+  if (IsEnabled(options::kSandbox)) {
     command_line->AppendSwitch(switches::kEnableSandbox);
-  else if (!command_line->HasSwitch(switches::kEnableSandbox))
+  } else if (!command_line->HasSwitch(switches::kEnableSandbox)) {
     command_line->AppendSwitch(service_manager::switches::kNoSandbox);
+    command_line->AppendSwitch(::switches::kNoZygote);
+  }
 
   // Check if nativeWindowOpen is enabled.
   if (IsEnabled(options::kNativeWindowOpen))

patches/common/chromium/.patches

@@ -73,3 +73,4 @@ cross_site_document_resource_handler.patch
 content_allow_embedder_to_prevent_locking_scheme_registry.patch
 fix_trackpad_scrolling.patch
 mac_fix_form_control_rendering_on_10_14_mojave.patch
+support_mixed_sandbox_with_zygote.patch

patches/common/chromium/support_mixed_sandbox_with_zygote.patch

@@ -0,0 +1,80 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Apthorp <nornagon@nornagon.net>
+Date: Wed, 28 Nov 2018 13:20:27 -0800
+Subject: support mixed-sandbox with zygote
+
+On Linux, Chromium launches all new renderer processes via a "zygote"
+process which has the sandbox pre-initialized (see
+//docs/linux_zygote.md). In order to support mixed-sandbox mode, in
+which some renderers are launched with the sandbox engaged and others
+without it, we need the option to launch non-sandboxed renderers without
+going through the zygote.
+
+Chromium already supports a `--no-zygote` flag, but it turns off the
+zygote completely, and thus also disables sandboxing. This patch allows
+the `--no-zygote` flag to affect renderer processes on a case-by-case
+basis, checking immediately prior to launch whether to go through the
+zygote or not based on the command-line of the to-be-launched renderer.
+
+This patch could conceivably be upstreamed, as it does not affect
+production Chromium (which does not use the `--no-zygote` flag).
+However, the patch would need to be reviewed by the security team, as it
+does touch a security-sensitive class.
+
+diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
+index 05e0ee79e5ad..9a4522f59e6f 100644
+--- a/content/browser/renderer_host/render_process_host_impl.cc
++++ b/content/browser/renderer_host/render_process_host_impl.cc
+@@ -466,6 +466,10 @@ class RendererSandboxedProcessLauncherDelegate
+     : public SandboxedProcessLauncherDelegate {
+  public:
+   RendererSandboxedProcessLauncherDelegate() {}
++#if BUILDFLAG(USE_ZYGOTE_HANDLE)
++  RendererSandboxedProcessLauncherDelegate(bool use_zygote):
++    use_zygote_(use_zygote) {}
++#endif
+ 
+   ~RendererSandboxedProcessLauncherDelegate() override {}
+ 
+@@ -485,6 +489,9 @@ class RendererSandboxedProcessLauncherDelegate
+ 
+ #if BUILDFLAG(USE_ZYGOTE_HANDLE)
+   service_manager::ZygoteHandle GetZygote() override {
++    if (!use_zygote_) {
++      return nullptr;
++    }
+     const base::CommandLine& browser_command_line =
+         *base::CommandLine::ForCurrentProcess();
+     base::CommandLine::StringType renderer_prefix =
+@@ -498,6 +505,11 @@ class RendererSandboxedProcessLauncherDelegate
+   service_manager::SandboxType GetSandboxType() override {
+     return service_manager::SANDBOX_TYPE_RENDERER;
+   }
++
++ private:
++#if BUILDFLAG(USE_ZYGOTE_HANDLE)
++  bool use_zygote_ = true;
++#endif
+ };
+ 
+ const char kSessionStorageHolderKey[] = "kSessionStorageHolderKey";
+@@ -1731,11 +1743,18 @@ bool RenderProcessHostImpl::Init() {
+       cmd_line->PrependWrapper(renderer_prefix);
+     AppendRendererCommandLine(cmd_line.get());
+ 
++#if BUILDFLAG(USE_ZYGOTE_HANDLE)
++    bool use_zygote = !cmd_line->HasSwitch(switches::kNoZygote);
++    auto delegate = std::make_unique<RendererSandboxedProcessLauncherDelegate>(use_zygote);
++#else
++    auto delegate = std::make_unique<RendererSandboxedProcessLauncherDelegate>();
++#endif
++
+     // Spawn the child process asynchronously to avoid blocking the UI thread.
+     // As long as there's no renderer prefix, we can use the zygote process
+     // at this stage.
+     child_process_launcher_ = std::make_unique<ChildProcessLauncher>(
+-        std::make_unique<RendererSandboxedProcessLauncherDelegate>(),
++        std::move(delegate),
+         std::move(cmd_line), GetID(), this, std::move(mojo_invitation_),
+         base::BindRepeating(&RenderProcessHostImpl::OnMojoError, id_));
+     channel_->Pause();

spec/api-app-spec.js

@@ -914,13 +914,21 @@ describe('app module', () => {
     const socketPath = process.platform === 'win32' ? '\\\\.\\pipe\\electron-mixed-sandbox' : '/tmp/electron-mixed-sandbox'
 
     beforeEach(function (done) {
-      // XXX(alexeykuzmin): Calling `.skip()` inside a `before` hook
+      if (process.platform === 'linux' && (process.arch === 'arm64' || process.arch === 'arm')) {
-      // doesn't affect nested `describe`s.
+        // Our ARM tests are run on VSTS rather than CircleCI, and the Docker
-      // FIXME Get these specs running on Linux
+        // setup on VSTS disallows syscalls that Chrome requires for setting up
-      if (process.platform === 'linux') {
+        // sandboxing.
+        // See:
+        // - https://docs.docker.com/engine/security/seccomp/#significant-syscalls-blocked-by-the-default-profile
+        // - https://chromium.googlesource.com/chromium/src/+/70.0.3538.124/sandbox/linux/services/credentials.cc#292
+        // - https://github.com/docker/docker-ce/blob/ba7dfc59ccfe97c79ee0d1379894b35417b40bca/components/engine/profiles/seccomp/seccomp_default.go#L497
+        // - https://blog.jessfraz.com/post/how-to-use-new-docker-seccomp-profiles/
+        //
+        // Adding `--cap-add SYS_ADMIN` or `--security-opt seccomp=unconfined`
+        // to the Docker invocation allows the syscalls that Chrome needs, but
+        // are probably more permissive than we'd like.
         this.skip()
       }
-
       fs.unlink(socketPath, () => {
         server = net.createServer()
         server.listen(socketPath)