Remove default sandbox type for services

Deletes content::GetServiceSandboxType<>() (see content/public/browser/service_process_host.h) so that ::Launch cannot compile if a sandbox is not specified by a specialized template. All existing services without a specified sandbox are launched as kUtility, so these now have that specified. No functional changes. Template for recording::mojom::RecordingService moved to chromeos and includes adjusted. Bug: 1210301 Change-Id: I1ae3f7e70246d9957940e71dc6b3e21ce6861d99 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2901584 Commit-Queue: Alex Gough <ajgo@chromium.org> Reviewed-by: Tony Yeoman <tby@chromium.org> Reviewed-by: Thanh Nguyen <thanhdng@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Reviewed-by: Glenn Hartmann <hartmanng@chromium.org> Reviewed-by: Stephen Nusko <nuskos@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Nasko Oskov <nasko@chromium.org> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Reviewed-by: Ken Rockot <rockot@google.com> Cr-Commit-Position: refs/heads/master@{#886572}
FairyWorld · May 26, 2021 · 4aac4e9 · 4aac4e9
1 parent 81540a3
commit 4aac4e9
Show file tree

Hide file tree

Showing 40 changed files with 342 additions and 70 deletions.
diff --git a/chrome/browser/chromeos/service_sandbox_type.h b/chrome/browser/chromeos/service_sandbox_type.h
@@ -5,14 +5,29 @@
 #ifndef CHROME_BROWSER_CHROMEOS_SERVICE_SANDBOX_TYPE_H_
 #define CHROME_BROWSER_CHROMEOS_SERVICE_SANDBOX_TYPE_H_
 
+#include "build/chromeos_buildflags.h"
 #include "chromeos/assistant/buildflags.h"
 #include "content/public/browser/service_process_host.h"
 #include "sandbox/policy/sandbox_type.h"
 
-// This file maps service classes to sandbox types.  Services which
-// require a non-utility sandbox can be added here.  See
+// This file maps service classes to sandbox types. See
 // ServiceProcessHost::Launch() for how these templates are consumed.
 
+// chromeos::assistant::mojom::AssistantAudioDecoderFactory
+namespace chromeos {
+namespace assistant {
+namespace mojom {
+class AssistantAudioDecoderFactory;
+}  // namespace mojom
+}  // namespace assistant
+}  // namespace chromeos
+
+template <>
+inline sandbox::policy::SandboxType content::GetServiceSandboxType<
+    chromeos::assistant::mojom::AssistantAudioDecoderFactory>() {
+  return sandbox::policy::SandboxType::kUtility;
+}
+
 // chromeos::ime::mojom::ImeService
 namespace chromeos {
 namespace ime {
@@ -59,4 +74,22 @@ inline sandbox::policy::SandboxType content::GetServiceSandboxType<
 }
 #endif  // BUILDFLAG(ENABLE_LIBASSISTANT_SANDBOX)
 
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+// recording::mojom::RecordingService
+namespace recording {
+namespace mojom {
+class RecordingService;
+}  // namespace mojom
+}  // namespace recording
+
+// This is needed to prevent the service from crashing on a sandbox seccomp-bpf
+// failure when the audio capturer tries to open a stream.
+// TODO(https://crbug.com/1147991): Explore alternatives if any.
+template <>
+inline sandbox::policy::SandboxType
+content::GetServiceSandboxType<recording::mojom::RecordingService>() {
+  return sandbox::policy::SandboxType::kVideoCapture;
+}
+#endif  // BUILDFLAG(IS_CHROMEOS_ASH)
+
 #endif  // CHROME_BROWSER_CHROMEOS_SERVICE_SANDBOX_TYPE_H_
diff --git a/chrome/browser/file_util_service.cc b/chrome/browser/file_util_service.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/file_util_service.h"
 
+#include "chrome/browser/service_sandbox_type.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/service_process_host.h"

diff --git a/chrome/browser/service_sandbox_type.h b/chrome/browser/service_sandbox_type.h
@@ -6,7 +6,6 @@
 #define CHROME_BROWSER_SERVICE_SANDBOX_TYPE_H_
 
 #include "build/build_config.h"
-#include "build/chromeos_buildflags.h"
 #include "content/public/browser/service_process_host.h"
 #include "media/base/media_switches.h"
 #include "printing/buildflags/buildflags.h"
@@ -18,10 +17,21 @@
 #include "chrome/browser/printing/print_backend_service_manager.h"
 #endif
 
-// This file maps service classes to sandbox types.  Services which
-// require a non-utility sandbox can be added here.  See
+// This file maps service classes to sandbox types. See
 // ServiceProcessHost::Launch() for how these templates are consumed.
 
+// chrome::mojom::FileUtilService
+namespace chrome {
+namespace mojom {
+class FileUtilService;
+}
+}  // namespace chrome
+template <>
+inline sandbox::policy::SandboxType
+content::GetServiceSandboxType<chrome::mojom::FileUtilService>() {
+  return sandbox::policy::SandboxType::kUtility;
+}
+
 // chrome::mojom::RemovableStorageWriter
 namespace chrome {
 namespace mojom {
@@ -82,6 +92,21 @@ content::GetServiceSandboxType<chrome::mojom::ProfileImport>() {
   return sandbox::policy::SandboxType::kNoSandbox;
 }
 
+// mac_notifications::mojom::MacNotificationProvider
+#if defined(OS_MAC)
+namespace mac_notifications {
+namespace mojom {
+class MacNotificationProvider;
+}  // namespace mojom
+}  // namespace mac_notifications
+
+template <>
+inline sandbox::policy::SandboxType content::GetServiceSandboxType<
+    mac_notifications::mojom::MacNotificationProvider>() {
+  return sandbox::policy::SandboxType::kNoSandbox;
+}
+#endif  // defined(OS_MAC)
+
 // media::mojom::SpeechRecognitionService
 #if !defined(OS_ANDROID)
 namespace media {
@@ -98,7 +123,6 @@ content::GetServiceSandboxType<media::mojom::SpeechRecognitionService>() {
 #endif  // !defined(OS_ANDROID)
 
 // mirroring::mojom::MirroringService
-#if defined(OS_MAC)
 namespace mirroring {
 namespace mojom {
 class MirroringService;
@@ -107,12 +131,15 @@ class MirroringService;
 template <>
 inline sandbox::policy::SandboxType
 content::GetServiceSandboxType<mirroring::mojom::MirroringService>() {
+#if defined(OS_MAC)
   return sandbox::policy::SandboxType::kMirroring;
-}
+#else
+  return sandbox::policy::SandboxType::kUtility;
 #endif  // OS_MAC
+}
 
 // printing::mojom::PrintingService
-#if defined(OS_WIN) && BUILDFLAG(ENABLE_PRINT_PREVIEW)
+#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
 namespace printing {
 namespace mojom {
 class PrintingService;
@@ -122,9 +149,13 @@ class PrintingService;
 template <>
 inline sandbox::policy::SandboxType
 content::GetServiceSandboxType<printing::mojom::PrintingService>() {
+#if defined(OS_WIN)
   return sandbox::policy::SandboxType::kPdfConversion;
+#else
+  return sandbox::policy::SandboxType::kUtility;
+#endif
 }
-#endif  // defined(OS_WIN) && BUILDFLAG(ENABLE_PRINT_PREVIEW)
+#endif  // BUILDFLAG(ENABLE_PRINT_PREVIEW)
 
 // printing::mojom::PrintBackendService
 #if (defined(OS_WIN) || defined(OS_MAC) || defined(OS_LINUX) || \
@@ -149,7 +180,7 @@ content::GetServiceSandboxType<printing::mojom::PrintBackendService>() {
         // BUILDFLAG(ENABLE_PRINTING)
 
 // proxy_resolver::mojom::ProxyResolverFactory
-#if defined(OS_WIN)
+#if !defined(OS_ANDROID)
 namespace proxy_resolver {
 namespace mojom {
 class ProxyResolverFactory;
@@ -159,9 +190,13 @@ class ProxyResolverFactory;
 template <>
 inline sandbox::policy::SandboxType
 content::GetServiceSandboxType<proxy_resolver::mojom::ProxyResolverFactory>() {
+#if defined(OS_WIN)
   return sandbox::policy::SandboxType::kProxyResolver;
+#else  // (!OS_WIN && !OS_ANDROID)
+  return sandbox::policy::SandboxType::kUtility;
+#endif
 }
-#endif  // defined(OS_WIN)
+#endif  // !defined(OS_ANDROID)
 
 // quarantine::mojom::Quarantine
 #if defined(OS_WIN)
@@ -193,37 +228,4 @@ content::GetServiceSandboxType<sharing::mojom::Sharing>() {
 }
 #endif  // !defined(OS_MAC)
 
-#if BUILDFLAG(IS_CHROMEOS_ASH)
-// recording::mojom::RecordingService
-namespace recording {
-namespace mojom {
-class RecordingService;
-}  // namespace mojom
-}  // namespace recording
-
-// This is needed to prevent the service from crashing on a sandbox seccomp-bpf
-// failure when the audio capturer tries to open a stream.
-// TODO(https://crbug.com/1147991): Explore alternatives if any.
-template <>
-inline sandbox::policy::SandboxType
-content::GetServiceSandboxType<recording::mojom::RecordingService>() {
-  return sandbox::policy::SandboxType::kVideoCapture;
-}
-#endif  // BUILDFLAG(IS_CHROMEOS_ASH)
-
-// mac_notifications::mojom::MacNotificationProvider
-#if defined(OS_MAC)
-namespace mac_notifications {
-namespace mojom {
-class MacNotificationProvider;
-}  // namespace mojom
-}  // namespace mac_notifications
-
-template <>
-inline sandbox::policy::SandboxType content::GetServiceSandboxType<
-    mac_notifications::mojom::MacNotificationProvider>() {
-  return sandbox::policy::SandboxType::kNoSandbox;
-}
-#endif  // defined(OS_MAC)
-
 #endif  // CHROME_BROWSER_SERVICE_SANDBOX_TYPE_H_
diff --git a/chrome/browser/ui/ash/assistant/assistant_client_impl.cc b/chrome/browser/ui/ash/assistant/assistant_client_impl.cc
@@ -12,6 +12,7 @@
 #include "ash/public/cpp/network_config_service.h"
 #include "chrome/browser/ash/assistant/assistant_util.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/chromeos/service_sandbox_type.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/ui/ash/assistant/assistant_context_util.h"
@@ -33,7 +34,6 @@
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
 #if BUILDFLAG(ENABLE_LIBASSISTANT_SANDBOX)
-#include "chrome/browser/chromeos/service_sandbox_type.h"
 #include "chromeos/services/libassistant/public/mojom/service.mojom.h"
 #endif  // BUILDFLAG(ENABLE_LIBASSISTANT_SANDBOX)
 

diff --git a/chrome/browser/ui/ash/chrome_capture_mode_delegate.cc b/chrome/browser/ui/ash/chrome_capture_mode_delegate.cc
@@ -14,10 +14,10 @@
 #include "chrome/browser/apps/app_service/launch_utils.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_content_manager.h"
+#include "chrome/browser/chromeos/service_sandbox_type.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/platform_util.h"
 #include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/service_sandbox_type.h"
 #include "chrome/browser/ui/ash/screenshot_area.h"
 #include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
 #include "chrome/browser/web_applications/components/web_app_id_constants.h"

diff --git a/chrome/services/BUILD.gn b/chrome/services/BUILD.gn
@@ -0,0 +1,11 @@
+# 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.
+
+source_set("sandbox_types") {
+  sources = [ "service_sandbox_type.h" ]
+  deps = [
+    "//content/public/browser",
+    "//sandbox/policy",
+  ]
+}
diff --git a/chrome/services/DEPS b/chrome/services/DEPS
@@ -6,5 +6,8 @@ include_rules = [
   # Services have to list which other services they depend on.
   "-chrome/services",
   "-services",
+  "+chrome/services/service_sandbox_type.h",
+  "+content/public/browser/service_process_host.h",
   "+services/service_manager/public",  # Every service talks to Service Manager.
+  "+sandbox/policy/sandbox_type.h",  # Services specify their SandboxType.
 ]
diff --git a/chrome/services/OWNERS b/chrome/services/OWNERS
@@ -1 +1,5 @@
 rockot@google.com
+
+# Service sandbox specialization must be reviewed by SECURITY_OWNERS
+per-file service_sandbox_type.h=set noparent
+per-file service_sandbox_type.h=file://ipc/SECURITY_OWNERS
diff --git a/chrome/services/file_util/public/cpp/DEPS b/chrome/services/file_util/public/cpp/DEPS
@@ -1,4 +1,5 @@
 include_rules = [
+  "+chrome/browser/service_sandbox_type.h",
   "+components/safe_browsing/buildflags.h",
   "+components/safe_browsing/core/proto",
   "+content/public/browser",

diff --git a/chrome/services/file_util/public/cpp/zip_file_creator_browsertest.cc b/chrome/services/file_util/public/cpp/zip_file_creator_browsertest.cc
@@ -14,6 +14,7 @@
 #include "base/rand_util.h"
 #include "base/run_loop.h"
 #include "base/threading/thread_restrictions.h"
+#include "chrome/browser/service_sandbox_type.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "content/public/browser/service_process_host.h"
 #include "content/public/test/browser_test.h"

diff --git a/chrome/services/ipp_parser/public/cpp/browser/BUILD.gn b/chrome/services/ipp_parser/public/cpp/browser/BUILD.gn
@@ -15,6 +15,7 @@ if (use_cups) {
     deps = [
       "//base",
       "//chrome:strings",
+      "//chrome/services:sandbox_types",
       "//chrome/services/ipp_parser/public/mojom",
       "//content/public/browser",
     ]

diff --git a/chrome/services/ipp_parser/public/cpp/browser/ipp_parser_launcher.cc b/chrome/services/ipp_parser/public/cpp/browser/ipp_parser_launcher.cc
@@ -7,6 +7,7 @@
 #include "base/no_destructor.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/services/ipp_parser/public/mojom/ipp_parser.mojom.h"
+#include "chrome/services/service_sandbox_type.h"
 #include "content/public/browser/service_process_host.h"
 
 namespace ipp_parser {

diff --git a/chrome/services/media_gallery_util/public/cpp/BUILD.gn b/chrome/services/media_gallery_util/public/cpp/BUILD.gn
@@ -17,6 +17,7 @@ source_set("cpp") {
   deps = [
     "//chrome/app:generated_resources",
     "//chrome/common",
+    "//chrome/services:sandbox_types",
     "//content/public/browser",
     "//media:media_buildflags",
     "//net:buildflags",

diff --git a/chrome/services/media_gallery_util/public/cpp/media_parser_provider.cc b/chrome/services/media_gallery_util/public/cpp/media_parser_provider.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "chrome/grit/generated_resources.h"
+#include "chrome/services/service_sandbox_type.h"
 #include "content/public/browser/service_process_host.h"
 #include "media/media_buildflags.h"
 #include "third_party/libyuv/include/libyuv.h"

diff --git a/chrome/services/qrcode_generator/public/cpp/BUILD.gn b/chrome/services/qrcode_generator/public/cpp/BUILD.gn
@@ -11,6 +11,7 @@ source_set("cpp") {
   deps = [
     "//base",
     "//chrome:strings",
+    "//chrome/services:sandbox_types",
     "//chrome/services/qrcode_generator/public/mojom",
     "//content/public/browser",
     "//skia",

diff --git a/chrome/services/qrcode_generator/public/cpp/qrcode_generator_service.cc b/chrome/services/qrcode_generator/public/cpp/qrcode_generator_service.cc
@@ -7,6 +7,7 @@
 #include "base/no_destructor.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/services/qrcode_generator/public/mojom/qrcode_generator.mojom.h"
+#include "chrome/services/service_sandbox_type.h"
 #include "content/public/browser/service_process_host.h"
 
 namespace qrcode_generator {

diff --git a/chrome/services/service_sandbox_type.h b/chrome/services/service_sandbox_type.h
@@ -0,0 +1,50 @@
+// 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.
+
+#ifndef CHROME_SERVICES_SERVICE_SANDBOX_TYPE_H_
+#define CHROME_SERVICES_SERVICE_SANDBOX_TYPE_H_
+
+#include "content/public/browser/service_process_host.h"
+#include "sandbox/policy/sandbox_type.h"
+
+// This file maps service classes to sandbox types. See
+// ServiceProcessHost::Launch() for how these templates are consumed.
+
+// chrome::mojom::MediaParserFactory
+namespace chrome {
+namespace mojom {
+class MediaParserFactory;
+}
+}  // namespace chrome
+template <>
+inline sandbox::policy::SandboxType
+content::GetServiceSandboxType<chrome::mojom::MediaParserFactory>() {
+  return sandbox::policy::SandboxType::kUtility;
+}
+
+// ipp_parser::mojom::IppParser
+namespace ipp_parser {
+namespace mojom {
+class IppParser;
+}
+}  // namespace ipp_parser
+template <>
+inline sandbox::policy::SandboxType
+content::GetServiceSandboxType<ipp_parser::mojom::IppParser>() {
+  return sandbox::policy::SandboxType::kUtility;
+}
+
+// qrcode_generator::mojom::QRCodeGeneratorService
+namespace qrcode_generator {
+namespace mojom {
+class QRCodeGeneratorService;
+}
+}  // namespace qrcode_generator
+template <>
+inline sandbox::policy::SandboxType content::GetServiceSandboxType<
+    qrcode_generator::mojom::QRCodeGeneratorService>() {
+  return sandbox::policy::SandboxType::kUtility;
+}
+
+#endif  // CHROME_SERVICES_SERVICE_SANDBOX_TYPE_H_
diff --git a/chromeos/components/local_search_service/BUILD.gn b/chromeos/components/local_search_service/BUILD.gn
@@ -49,6 +49,7 @@ source_set("local_search_service_provider") {
     "local_search_service_provider_for_testing.h",
     "oop_local_search_service_provider.cc",
     "oop_local_search_service_provider.h",
+    "service_sandbox_type.h",
   ]
 
   deps = [