Avoid treating unsandboxed processes as brokers when initializing sandbox_info.

In the past, we've checked for the presence of the kNoSandbox switch
when initializing sandbox_info, and if a child process was unsandboxed
we would avoid initializing sandbox_info at all. However, the kNoSandbox
switch is only meant to be used as a browser level flag for testing
purposes only. There exist other ways to denote an unsandboxed process
too, as in the case of the unsandboxed utility process which uses the
switch service-sandbox-type=none instead of --no-sandbox. Similarly,
there's also an unsandboxed GPU process which makes use of the
--disable-gpu-sandbox switch instead of the --no-sandbox switch. This
has resulted in having unsandboxed processes initializing their
sandbox_info as if they were brokers, and as a consequence, having the
same process mitigations applied to them. This last behavior is
certainly an unintended side-effect that could manifest as a bug
eventually.

In this CL, we're updating the way in which we check whether a process
is unsandboxed to ensure that no unsandboxed processes have their
sandbox_info initialized as brokers, and to prevent applying the same
process mitigations accidentally.

Bug: 1066258

Change-Id: Ifdc14b0e29f9505edeec93417bd9cc7ab73e94c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2208189
Auto-Submit: Andres Pico <anpico@microsoft.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Alex Gough <ajgo@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772048}

chrome/app/main_dll_loader_win.cc

@@ -42,7 +42,7 @@
 #include "content/public/app/sandbox_helper_win.h"
 #include "content/public/common/content_switches.h"
 #include "sandbox/win/src/sandbox.h"
-#include "services/service_manager/sandbox/switches.h"
+#include "services/service_manager/sandbox/sandbox_type.h"
 
 namespace {
 // The entry point signature of chrome.dll.
@@ -131,8 +131,11 @@ int MainDllLoader::Launch(HINSTANCE instance,
   // Initialize the sandbox services.
   sandbox::SandboxInterfaceInfo sandbox_info = {0};
   const bool is_browser = process_type_.empty();
+  // IsUnsandboxedSandboxType() can't be used here because its result can be
+  // gated behind a feature flag, which are not yet initialized.
   const bool is_sandboxed =
-      !cmd_line.HasSwitch(service_manager::switches::kNoSandbox);
+      service_manager::SandboxTypeFromCommandLine(cmd_line) !=
+      service_manager::SandboxType::kNoSandbox;
   if (is_browser || is_sandboxed) {
     // For child processes that are running as --no-sandbox, don't initialize
     // the sandbox info, otherwise they'll be treated as brokers (as if they

services/service_manager/sandbox/switches.cc

@@ -80,6 +80,7 @@ const char kGpuSandboxAllowSysVShm[] = "gpu-sandbox-allow-sysv-shm";
 const char kGpuSandboxFailuresFatal[] = "gpu-sandbox-failures-fatal";
 
 // Disables the sandbox for all process types that are normally sandboxed.
+// Meant to be used as a browser-level switch for testing purposes only.
 const char kNoSandbox[] = "no-sandbox";
 
 #if defined(OS_LINUX)