Add [ServiceSandbox=type] attribute to mojom interfaces

See doc linked in bug 1210301.

This allows a mojom interface to specify the sandbox its service should
be launched in:-

[ServiceSandbox=sandbox.mojom.Sandbox.kService]
interface FakeService {
  Foo() => ();
}

This is achieved by:

* Allowing fully-qualified names as attribute values in .mojom
files. This was not allowed before so shouldn't change any existing
behavior.
* Adding the Sandbox attribute to the mojom cpp generator.
* constexpr kServiceSandbox members on the generated mojom classes.
* mapping mojom sandbox types to chrome sandbox types.
* Modifying content::ServiceProcessHost to fall back to asking mojo
which sandbox to use if a specialization of GetServiceSandboxType()
has not already been provided. If no kServiceSandbox exists
compilation still fails if no sandbox is specified for the
interface being ::Launch()ed.

Sandbox attributes are verified at C++ compilation time.

This makes it much easier to select an approved sandbox, and difficult
but still possible to select a build or platform varying sandbox, while
still requiring security review.

A following change will add a presubmit to prevent direct inclusion of
GetInterfaceSandbox specializations.

This also adopts this attribute for the TestService and for the
DataDecoderService.

tests: content_browsertests ServiceProcessHostBrowserTest.*
Bug: 1210301
Change-Id: Ie014724de603facae1edb6808733d4212ec20ee1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2912898
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#907309}

content/browser/browser_main_loop.cc

@@ -83,7 +83,6 @@
 #include "content/browser/scheduler/responsiveness/watcher.h"
 #include "content/browser/screenlock_monitor/screenlock_monitor.h"
 #include "content/browser/screenlock_monitor/screenlock_monitor_device_source.h"
-#include "content/browser/service_sandbox_type.h"
 #include "content/browser/sms/sms_provider.h"
 #include "content/browser/speech/speech_recognition_manager_impl.h"
 #include "content/browser/speech/tts_controller_impl.h"
@@ -134,6 +133,7 @@
 #include "ppapi/buildflags/buildflags.h"
 #include "services/audio/service.h"
 #include "services/data_decoder/public/cpp/service_provider.h"
+#include "services/data_decoder/public/mojom/data_decoder_service.mojom.h"
 #include "services/network/transitional_url_loader_factory_owner.h"
 #include "skia/ext/event_tracer_impl.h"
 #include "skia/ext/skia_memory_dump_provider.h"

content/browser/service_process_host_browsertest.cc

@@ -18,17 +18,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
-// Provides sandbox for echo::mojom::EchoService.
-namespace echo {
-namespace mojom {
-class EchoService;
-}
-}  // namespace echo
-template <>
-inline sandbox::policy::SandboxType
-content::GetServiceSandboxType<echo::mojom::EchoService>() {
-  return sandbox::policy::SandboxType::kUtility;
-}
 
 namespace content {
 

content/browser/service_sandbox_type.h

@@ -43,18 +43,6 @@ content::GetServiceSandboxType<audio::mojom::AudioService>() {
              : sandbox::policy::SandboxType::kNoSandbox;
 }
 
-// data_decoder::mojom::DataDecoderService
-namespace data_decoder {
-namespace mojom {
-class DataDecoderService;
-}
-}  // namespace data_decoder
-template <>
-inline sandbox::policy::SandboxType
-content::GetServiceSandboxType<data_decoder::mojom::DataDecoderService>() {
-  return sandbox::policy::SandboxType::kService;
-}
-
 // device::mojom::XRDeviceService
 namespace device {
 namespace mojom {

content/public/browser/service_process_host.h

@@ -22,16 +22,26 @@
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/system/message_pipe.h"
+#include "sandbox/policy/mojom/sandbox.mojom.h"
 #include "sandbox/policy/sandbox_type.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace content {
 
 // Sandbox type for ServiceProcessHost::Launch<remote>() is found by
 // template matching on |remote|. Consult security-dev@chromium.org and
-// add to an appropriate |service_sandbox_type.h|.
+// add a [ServiceSandbox=type] mojom attribute, or an appropriate
+// |service_sandbox_type.h|.
 template <typename Interface>
-inline sandbox::policy::SandboxType GetServiceSandboxType() = delete;
+inline sandbox::policy::SandboxType 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);
+}
 
 // ServiceProcessHost is used to launch new service processes given basic
 // parameters like sandbox type, as well as a primordial Mojo interface to drive

docs/mojo_and_services.md

@@ -421,19 +421,37 @@ NOTE: To ensure the execution of the response callback, the
 and [this note from an earlier section](#sending-a-message)).
 ***
 
-### Using a non-standard sandbox
+### Specifying a sandbox
 
-Ideally services will run inside the service process sandbox unless
-they need access to operating system resources. For services that need
-a custom sandbox, a new sandbox type must be defined in consultation
-with security-dev@chromium.org.
+All services must specify a sandbox. Ideally services will run inside the
+`kService` process sandbox unless they need access to operating system
+resources. For services that need a custom sandbox, a new sandbox type must be
+defined in consultation with security-dev@chromium.org.
 
-All services must specify their sandbox by specialization of
-`GetServiceSandboxType()` in an appropriate `service_sandbox_type.h` such as
+The preferred way to define the sandbox for your interface is by specifying a
+`[ServiceSandbox=type]` attribute on your `interface {}` in its `.mojom` file:
+
+```
+import "sandbox/policy/mojom/sandbox.mojom"
+[ServiceSandbox=sandbox.mojom.Sandbox.kService]
+interface FakeService {
+  ...
+};
+```
+
+Valid values are those in
+[`//sandbox/policy/mojom/sandbox.mojom`](https://cs.chromium.org/chromium/src/sandbox/policy/mojom/sandbox.mojom). Note
+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)
-and included where `ServiceProcessHost::Launch()` is called.
+[`//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.
 
 ## Content-Layer Services Overview
 

mojo/public/tools/bindings/README.md

@@ -450,6 +450,13 @@ interesting attributes supported today.
   platform. Note that the `EnableIf` attribute can only be set once per
   definition.
 
+* **`[ServiceSandbox=value]`**:
+  The `ServiceSandbox` attribute is used in Chromium to tag which sandbox a
+  service hosting an implementation of interface will be launched in. This only
+  applies to `C++` bindings. `value` should match a constant defined in an
+  imported `sandbox.mojom.Sandbox` enum (for Chromium this is
+  `//sandbox/policy/mojom/sandbox.mojom`), such as `kService`.
+
 ## Generated Code For Target Languages
 
 When the bindings generator successfully processes an input Mojom file, it emits

mojo/public/tools/bindings/generators/cpp_templates/interface_declaration.tmpl

@@ -20,6 +20,10 @@ class {{export_attribute}} {{interface.name}}
   static constexpr base::Token Uuid_{ {{interface.uuid[0]}}ULL,
                                       {{interface.uuid[1]}}ULL };
 {%-  endif %}
+{%- if interface.service_sandbox %}
+{%- set sandbox_enum = "%s"|format(interface.service_sandbox|replace(".","::")) %}
+  static constexpr auto kServiceSandbox = {{ sandbox_enum }};
+{%- endif %}
   static constexpr uint32_t Version_ = {{interface.version}};
   static constexpr bool PassesAssociatedKinds_ = {% if interface|passes_associated_kinds %}true{% else %}false{% endif %};
   static constexpr bool HasSyncMethods_ = {% if interface|has_sync_methods %}true{% else %}false{% endif %};

mojo/public/tools/mojom/mojom/generate/module.py

@@ -294,6 +294,7 @@ def IsBackwardCompatible(self, rhs, checker):
 ATTRIBUTE_SYNC = 'Sync'
 ATTRIBUTE_UNLIMITED_SIZE = 'UnlimitedSize'
 ATTRIBUTE_UUID = 'Uuid'
+ATTRIBUTE_SERVICE_SANDBOX = 'ServiceSandbox'
 
 
 class NamedValue(object):
@@ -1206,6 +1207,12 @@ def buildOrdinalMethodMap(interface):
 
     return True
 
+  @property
+  def service_sandbox(self):
+    if not self.attributes:
+      return None
+    return self.attributes.get(ATTRIBUTE_SERVICE_SANDBOX, None)
+
   @property
   def stable(self):
     return self.attributes.get(ATTRIBUTE_STABLE, False) \

mojo/public/tools/mojom/mojom/parse/parser.py

@@ -140,11 +140,18 @@ def p_nonempty_attribute_list_2(self, p):
     p[0].Append(p[3])
 
   def p_attribute_1(self, p):
+    """attribute : NAME EQUALS identifier_wrapped"""
+    p[0] = ast.Attribute(p[1],
+                         p[3][1],
+                         filename=self.filename,
+                         lineno=p.lineno(1))
+
+  def p_attribute_2(self, p):
     """attribute : NAME EQUALS evaled_literal
                  | NAME EQUALS NAME"""
     p[0] = ast.Attribute(p[1], p[3], filename=self.filename, lineno=p.lineno(1))
 
-  def p_attribute_2(self, p):
+  def p_attribute_3(self, p):
     """attribute : NAME"""
     p[0] = ast.Attribute(p[1], True, filename=self.filename, lineno=p.lineno(1))
 

sandbox/policy/BUILD.gn

@@ -10,6 +10,7 @@ import("//chromeos/assistant/assistant.gni")
 import("//printing/buildflags/buildflags.gni")
 import("//testing/test.gni")
 
+# Most consumers of sandbox::policy should depend on this target.
 component("policy") {
   sources = [
     "export.h",
@@ -30,6 +31,7 @@ component("policy") {
     "//build:chromeos_buildflags",
     "//printing/buildflags",
     "//sandbox:common",
+    "//sandbox/policy/mojom",
   ]
   public_deps = []
   if (is_linux || is_chromeos) {

sandbox/policy/mojom/BUILD.gn

@@ -0,0 +1,10 @@
+# Copyright 2021 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.
+
+import("//mojo/public/tools/bindings/mojom.gni")
+
+mojom("mojom") {
+  generate_java = true
+  sources = [ "sandbox.mojom" ]
+}

sandbox/policy/mojom/OWNERS

@@ -0,0 +1,2 @@
+per-file *.mojom=set noparent
+per-file *.mojom=file://ipc/SECURITY_OWNERS

sandbox/policy/mojom/sandbox.mojom

@@ -0,0 +1,18 @@
+// Copyright 2021 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.
+
+module sandbox.mojom;
+
+// Sandbox type that can be specified as an attribute of mojo interfaces.
+// To specify the sandbox a service should be launched in, use the
+// [ServiceSandbox=type] attribute.
+// If your service does not need access to OS resources it should be
+// possible to host it in |kService|. These values are mapped to
+// //sandbox/policy/sandbox_type.h values.
+enum Sandbox {
+  // |kService| hosts 'computation only' services such as decoders that
+  // use limited operating system services. Prefer to use this sandbox
+  // if possible.
+  kService,
+};

sandbox/policy/sandbox_type.h

@@ -12,6 +12,7 @@
 #include "build/chromeos_buildflags.h"
 #include "printing/buildflags/buildflags.h"
 #include "sandbox/policy/export.h"
+#include "sandbox/policy/mojom/sandbox.mojom.h"
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
 #include "chromeos/assistant/buildflags.h"
@@ -119,6 +120,14 @@ enum class SandboxType {
   kMaxValue = kVideoCapture
 };
 
+inline constexpr sandbox::policy::SandboxType MapToSandboxType(
+    sandbox::mojom::Sandbox mojo_sandbox) {
+  switch (mojo_sandbox) {
+    case sandbox::mojom::Sandbox::kService:
+      return sandbox::policy::SandboxType::kService;
+  }
+}
+
 SANDBOX_POLICY_EXPORT bool IsUnsandboxedSandboxType(SandboxType sandbox_type);
 
 SANDBOX_POLICY_EXPORT void SetCommandLineFlagsForSandboxType(

services/data_decoder/public/mojom/BUILD.gn

@@ -19,6 +19,7 @@ mojom("mojom") {
     ":mojom_resource_snapshot_for_web_bundle",
     "//components/web_package/mojom",
     "//mojo/public/mojom/base",
+    "//sandbox/policy/mojom",
     "//skia/public/mojom",
     "//ui/gfx/geometry/mojom",
     "//url/mojom:url_mojom_gurl",

services/data_decoder/public/mojom/data_decoder_service.mojom

@@ -5,6 +5,7 @@
 module data_decoder.mojom;
 
 import "components/web_package/mojom/web_bundle_parser.mojom";
+import "sandbox/policy/mojom/sandbox.mojom";
 import "services/data_decoder/public/mojom/gzipper.mojom";
 import "services/data_decoder/public/mojom/image_decoder.mojom";
 import "services/data_decoder/public/mojom/json_parser.mojom";
@@ -15,6 +16,7 @@ import "services/data_decoder/public/mojom/xml_parser.mojom";
 import "services/data_decoder/public/mojom/ble_scan_parser.mojom";
 
 // The main interface to the Data Decoder service.
+[ServiceSandbox=sandbox.mojom.Sandbox.kService]
 interface DataDecoderService {
   // Binds an interface which can be used to decode compressed image data.
   BindImageDecoder(pending_receiver<ImageDecoder> receiver);

services/test/echo/public/mojom/BUILD.gn

@@ -7,5 +7,8 @@ import("//mojo/public/tools/bindings/mojom.gni")
 mojom("mojom") {
   generate_java = true
   sources = [ "echo.mojom" ]
-  public_deps = [ "//mojo/public/mojom/base" ]
+  public_deps = [
+    "//mojo/public/mojom/base",
+    "//sandbox/policy/mojom",
+  ]
 }

services/test/echo/public/mojom/echo.mojom

@@ -5,8 +5,10 @@
 module echo.mojom;
 
 import "mojo/public/mojom/base/shared_memory.mojom";
+import "sandbox/policy/mojom/sandbox.mojom";
 
 // Echos its input.
+[ServiceSandbox=sandbox.mojom.Sandbox.kService]
 interface EchoService {
   // Echos the passed-in string.
   EchoString(string input) => (string echoed_input);