Specify Audio and Network sandboxes in mojo

Adds ServiceSandbox attribute to AudioService and NetworkService
interfaces and deletes the final service_sandbox_type.h file. All
service sandboxes are now specified this way.

These two services are different, however. Their sandboxes can be
disabled (to kNoSandbox) by policy or feature flags. This means
the service launching machinery must know how to do this. In this
CL we move this logic into the ServiceProcessHost which in turn
asks the ContentBrowserClient if the sandboxes should be enabled.
As this is only necessary for two sandbox types we have not
generalised the CBC interface.

We also delete `sandbox_type` from service process host options
as it cannot be specified as an option and must be specified using
the ServiceSandbox attribute on the launched mojom. We migrate
the type used within ServiceProcessHost to sandbox::mojom::Sandbox.
A future CL will replace sandbox::policy::mojom::SandboxType with
the mojom type throughout.

Bug: 1210301
Change-Id: I4de13359487158a52da8ad414c8beb9cd93033bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3209992
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#930365}

content/browser/BUILD.gn

@@ -1737,7 +1737,6 @@ source_set("browser") {
     "screenlock_monitor/screenlock_monitor_source.cc",
     "screenlock_monitor/screenlock_monitor_source.h",
     "service_process_host_impl.cc",
-    "service_sandbox_type.h",
     "service_worker/embedded_worker_instance.cc",
     "service_worker/embedded_worker_instance.h",
     "service_worker/embedded_worker_status.h",

content/browser/OWNERS

@@ -33,9 +33,6 @@ per-file sandbox_host_linux.*=file://sandbox/linux/OWNERS
 per-file sandbox_ipc_linux.*=file://sandbox/linux/OWNERS
 per-file utility_process_sandbox_browsertest.cc=file://sandbox/linux/OWNERS
 
-# Service sandbox mappings require security review
-per-file service_sandbox_type.h=file://ipc/SECURITY_OWNERS
-
 # Utility sandbox delegate requires security review.
 per-file utility_sandbox_delegate.*=set noparent
 per-file utility_sandbox_delegate.*=file://ipc/SECURITY_OWNERS

content/browser/audio/audio_service.cc

@@ -12,7 +12,6 @@
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "content/browser/browser_main_loop.h"
-#include "content/browser/service_sandbox_type.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
@@ -24,6 +23,7 @@
 #include "media/base/media_switches.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "services/audio/public/cpp/audio_system_to_service_adapter.h"
+#include "services/audio/public/mojom/audio_service.mojom.h"
 #include "services/audio/service.h"
 #include "services/audio/service_factory.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"

content/browser/network_service_instance_impl.cc

@@ -31,7 +31,6 @@
 #include "build/chromeos_buildflags.h"
 #include "content/browser/browser_main_loop.h"
 #include "content/browser/network_service_client.h"
-#include "content/browser/service_sandbox_type.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"

content/browser/service_process_host_impl.cc

@@ -16,7 +16,9 @@
 #include "content/common/child_process.mojom.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/service_process_host.h"
+#include "content/public/common/content_client.h"
 #include "mojo/public/cpp/bindings/generic_pending_receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
 
@@ -24,6 +26,15 @@ namespace content {
 
 namespace {
 
+// Changes to this function should be reviewed by a security person.
+bool ShouldEnableSandbox(sandbox::mojom::Sandbox sandbox) {
+  if (sandbox == sandbox::mojom::Sandbox::kAudio)
+    return GetContentClient()->browser()->ShouldSandboxAudioService();
+  if (sandbox == sandbox::mojom::Sandbox::kNetwork)
+    return GetContentClient()->browser()->ShouldSandboxNetworkService();
+  return true;
+}
+
 // Internal helper to track running service processes.
 class ServiceProcessTracker {
  public:
@@ -162,14 +173,17 @@ class UtilityProcessClient : public UtilityProcessHost::Client {
 // TODO(crbug.com/977637): Once UtilityProcessHost is used only by service
 // processes, its logic can be inlined here.
 void LaunchServiceProcess(mojo::GenericPendingReceiver receiver,
-                          ServiceProcessHost::Options options) {
+                          ServiceProcessHost::Options options,
+                          sandbox::mojom::Sandbox sandbox) {
   UtilityProcessHost* host = new UtilityProcessHost(
       std::make_unique<UtilityProcessClient>(*receiver.interface_name()));
   host->SetName(!options.display_name.empty()
                     ? options.display_name
                     : base::UTF8ToUTF16(*receiver.interface_name()));
   host->SetMetricsName(*receiver.interface_name());
-  host->SetSandboxType(options.sandbox_type);
+  if (!ShouldEnableSandbox(sandbox))
+    sandbox = sandbox::mojom::Sandbox::kNoSandbox;
+  host->SetSandboxType(sandbox::policy::MapToSandboxType(sandbox));
   host->SetExtraCommandLineSwitches(std::move(options.extra_switches));
   if (options.child_flags)
     host->set_child_flags(*options.child_flags);
@@ -196,14 +210,15 @@ void ServiceProcessHost::RemoveObserver(Observer* observer) {
 
 // static
 void ServiceProcessHost::Launch(mojo::GenericPendingReceiver receiver,
-                                Options options) {
+                                Options options,
+                                sandbox::mojom::Sandbox sandbox) {
   DCHECK(receiver.interface_name().has_value());
   if (GetUIThreadTaskRunner({})->BelongsToCurrentThread()) {
-    LaunchServiceProcess(std::move(receiver), std::move(options));
+    LaunchServiceProcess(std::move(receiver), std::move(options), sandbox);
   } else {
     GetUIThreadTaskRunner({})->PostTask(
         FROM_HERE, base::BindOnce(&LaunchServiceProcess, std::move(receiver),
-                                  std::move(options)));
+                                  std::move(options), sandbox));
   }
 }
 

content/browser/service_sandbox_type.h

@@ -1,45 +0,0 @@
-// Copyright 2020 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.
-
-#ifndef CONTENT_BROWSER_SERVICE_SANDBOX_TYPE_H_
-#define CONTENT_BROWSER_SERVICE_SANDBOX_TYPE_H_
-
-#include "content/browser/network_service_instance_impl.h"
-#include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/service_process_host.h"
-#include "content/public/common/content_client.h"
-#include "sandbox/policy/sandbox_type.h"
-
-// This file maps service classes to sandbox types. See
-// ServiceProcessHost::Launch() for how these templates are consumed.
-
-// audio::mojom::AudioService
-namespace audio {
-namespace mojom {
-class AudioService;
-}
-}  // namespace audio
-template <>
-inline sandbox::policy::SandboxType
-content::GetServiceSandboxType<audio::mojom::AudioService>() {
-  return GetContentClient()->browser()->ShouldSandboxAudioService()
-             ? sandbox::policy::SandboxType::kAudio
-             : sandbox::policy::SandboxType::kNoSandbox;
-}
-
-// network::mojom::NetworkService
-namespace network {
-namespace mojom {
-class NetworkService;
-}
-}  // namespace network
-template <>
-inline sandbox::policy::SandboxType
-content::GetServiceSandboxType<network::mojom::NetworkService>() {
-  return GetContentClient()->browser()->ShouldSandboxNetworkService()
-             ? sandbox::policy::SandboxType::kNetwork
-             : sandbox::policy::SandboxType::kNoSandbox;
-}
-
-#endif  // CONTENT_BROWSER_SERVICE_SANDBOX_TYPE_H_

content/public/browser/service_process_host.h

@@ -30,17 +30,16 @@ namespace content {
 
 // Sandbox type for ServiceProcessHost::Launch<remote>() is found by
 // template matching on |remote|. Consult security-dev@chromium.org and
-// add a [ServiceSandbox=type] mojom attribute, or an appropriate
-// |service_sandbox_type.h|.
+// add a [ServiceSandbox=type] mojom attribute.
 template <typename Interface>
-inline sandbox::policy::SandboxType GetServiceSandboxType() {
+inline sandbox::mojom::Sandbox GetServiceSandboxType() {
   using ProvidedSandboxType = decltype(Interface::kServiceSandbox);
   static_assert(
       std::is_same<ProvidedSandboxType, const sandbox::mojom::Sandbox>::value,
       "This interface does not declare a proper ServiceSandbox attribute. See "
       "//docs/mojo_and_services.md (Specifying a sandbox).");
 
-  return sandbox::policy::MapToSandboxType(Interface::kServiceSandbox);
+  return Interface::kServiceSandbox;
 }
 
 // ServiceProcessHost is used to launch new service processes given basic
@@ -87,8 +86,6 @@ class CONTENT_EXPORT ServiceProcessHost {
     // to |Launch()|.
     Options Pass();
 
-    sandbox::policy::SandboxType sandbox_type =
-        sandbox::policy::SandboxType::kUtility;
     std::u16string display_name;
     absl::optional<int> child_flags;
     std::vector<std::string> extra_switches;
@@ -130,9 +127,8 @@ class CONTENT_EXPORT ServiceProcessHost {
   template <typename Interface>
   static void Launch(mojo::PendingReceiver<Interface> receiver,
                      Options options = {}) {
-    options.sandbox_type = content::GetServiceSandboxType<Interface>();
     Launch(mojo::GenericPendingReceiver(std::move(receiver)),
-           std::move(options));
+           std::move(options), content::GetServiceSandboxType<Interface>());
   }
 
   // Same as above but creates a new |Interface| pipe on the caller's behalf and
@@ -141,9 +137,9 @@ class CONTENT_EXPORT ServiceProcessHost {
   // May be called from any thread.
   template <typename Interface>
   static mojo::Remote<Interface> Launch(Options options = {}) {
-    options.sandbox_type = content::GetServiceSandboxType<Interface>();
     mojo::Remote<Interface> remote;
-    Launch(remote.BindNewPipeAndPassReceiver(), std::move(options));
+    Launch(remote.BindNewPipeAndPassReceiver(), std::move(options),
+           content::GetServiceSandboxType<Interface>());
     return remote;
   }
 
@@ -163,7 +159,9 @@ class CONTENT_EXPORT ServiceProcessHost {
   // Launches a new service process and asks it to bind a receiver for the
   // service interface endpoint carried by |receiver|, which should be connected
   // to a Remote of the same interface type.
-  static void Launch(mojo::GenericPendingReceiver receiver, Options options);
+  static void Launch(mojo::GenericPendingReceiver receiver,
+                     Options options,
+                     sandbox::mojom::Sandbox sandbox);
 };
 
 // DEPRECATED. DO NOT USE THIS. This is a helper for any remaining service

docs/mojo_and_services.md

@@ -444,14 +444,9 @@ Valid values are those in
 that the sandbox is only applied if the interface is launched
 out-of-process using `content::ServiceProcessHost::Launch()`.
 
-Dynamic or feature based mapping to an underlying platform sandbox can be
-achieved using `sandbox::policy::MapToSandboxType()`. As a last resort, specify
-a service's sandbox by specialization of `GetServiceSandboxType()` in an
-appropriate `service_sandbox_type.h` such as
-[`//chrome/browser/service_sandbox_type.h`](https://cs.chromium.org/chromium/src/chrome/browser/service_sandbox_type.h)
-or
-[`//content/browser/service_sandbox_type.h`](https://cs.chromium.org/chromium/src/content/browser/service_sandbox_type.h).
-This must be included where `ServiceProcessHost::Launch()` is called.
+As a last resort, dynamic or feature based mapping to an underlying platform
+sandbox can be achieved but requires plumbing through ContentBrowserClient
+(e.g. `ShouldEnableNetworkServiceSandbox()`).
 
 ## Content-Layer Services Overview
 

sandbox/policy/mojom/sandbox.mojom

@@ -21,6 +21,9 @@ enum Sandbox {
   // For instance, it allows dynamic code and wider access to APIs on Windows.
   kUtility,
 
+  // The audio service process. May be disabled by policy.
+  kAudio,
+
   // Hosts the content decryption module. Allows pre-loading of CDM libraries.
   // - On Windows, when `CdmServiceBroker` is connected the CDM was not
   // sandboxed to allow CDM preloading.
@@ -28,6 +31,9 @@ enum Sandbox {
   // - On Linux/ChromeOS, the CDM is preloaded in the zygote sandbox.
   kCdm,
 
+  // The network service. May be disabled by policy.
+  kNetwork,
+
   // Runs with the same rights as the browser. Usually needed to improve
   // stability by hosting code that interacts with third party code in another
   // process.

sandbox/policy/sandbox_type.h

@@ -125,10 +125,14 @@ enum class SandboxType {
 inline constexpr sandbox::policy::SandboxType MapToSandboxType(
     sandbox::mojom::Sandbox mojo_sandbox) {
   switch (mojo_sandbox) {
+    case sandbox::mojom::Sandbox::kAudio:
+      return sandbox::policy::SandboxType::kAudio;
     case sandbox::mojom::Sandbox::kCdm:
       return sandbox::policy::SandboxType::kCdm;
     case sandbox::mojom::Sandbox::kGpu:
       return sandbox::policy::SandboxType::kGpu;
+    case sandbox::mojom::Sandbox::kNetwork:
+      return sandbox::policy::SandboxType::kNetwork;
     case sandbox::mojom::Sandbox::kNoSandbox:
       return sandbox::policy::SandboxType::kNoSandbox;
     case sandbox::mojom::Sandbox::kPrintCompositor:

services/audio/public/mojom/BUILD.gn

@@ -18,6 +18,7 @@ mojom("mojom") {
   public_deps = [
     "//media/mojo/mojom",
     "//mojo/public/mojom/base",
+    "//sandbox/policy/mojom",
   ]
 
   cpp_typemaps = [

services/audio/public/mojom/audio_service.mojom

@@ -5,6 +5,7 @@
 module audio.mojom;
 
 import "media/mojo/mojom/audio_stream_factory.mojom";
+import "sandbox/policy/mojom/sandbox.mojom";
 import "services/audio/public/mojom/debug_recording.mojom";
 import "services/audio/public/mojom/device_notifications.mojom";
 import "services/audio/public/mojom/log_factory_manager.mojom";
@@ -12,7 +13,9 @@ import "services/audio/public/mojom/system_info.mojom";
 import "services/audio/public/mojom/testing_api.mojom";
 
 // The main interface to the Audio service. This is a privileged interface and
-// must only be bound by trusted processes, e.g. a browser process.
+// must only be bound by trusted processes, e.g. a browser process. Note that
+// the sandbox can be disabled (to kNoSandbox) by policy.
+[ServiceSandbox=sandbox.mojom.Sandbox.kAudio]
 interface AudioService {
   // Binds a SystemInfo interface receiver.
   BindSystemInfo(pending_receiver<SystemInfo> receiver);
@@ -33,4 +36,3 @@ interface AudioService {
   // environments.
   BindTestingApi(pending_receiver<TestingApi> receiver);
 };
-

services/network/public/mojom/BUILD.gn

@@ -891,6 +891,7 @@ mojom("mojom") {
     ":url_loader_base",
     ":websocket_mojom",
     "//mojo/public/mojom/base",
+    "//sandbox/policy/mojom",
     "//services/proxy_resolver/public/mojom",
     "//url/mojom:url_mojom_gurl",
     "//url/mojom:url_mojom_origin",

services/network/public/mojom/network_service.mojom

@@ -13,6 +13,7 @@ import "mojo/public/mojom/base/string16.mojom";
 import "mojo/public/mojom/base/time.mojom";
 import "mojo/public/mojom/base/unguessable_token.mojom";
 import "mojo/public/mojom/base/values.mojom";
+import "sandbox/policy/mojom/sandbox.mojom";
 import "services/network/public/mojom/cookie_manager.mojom";
 import "services/network/public/mojom/host_resolver.mojom";
 import "services/network/public/mojom/http_raw_headers.mojom";
@@ -150,6 +151,9 @@ struct LoggingSettings {
 //
 // This is a trusted interface that only the browser process should have access
 // to. It must not be sent to any untrusted process like a renderer process.
+// The network sandbox may be disabled (to kNoSandbox) by policy, and may not
+// be available on all platforms.
+[ServiceSandbox=sandbox.mojom.Sandbox.kNetwork]
 interface NetworkService {
   // Sets the parameters and initializes the service.
   SetParams(NetworkServiceParams params);