Restrict command processed by syscall_broker process.

As the syscall_broker process becomes more capable, it is important
not to expose all the new capabilities to old clients which do no
require them. Introduce a set of flags to track allowed commands.

Rename broker_common.{cc,h} to broker_command.{cc,h} since all the
contents are related to commands issued to the broker. Consolidate
client/host side checks in this new file.

Parameterize some tests so we cover both client/host denials.
Tidy, brace initialize some vectors for readability.


Cq-Include-Trybots: master.tryserver.chromium.android:android_arm64_dbg_recipe;master.tryserver.chromium.android:android_compile_x64_dbg;master.tryserver.chromium.android:android_compile_x86_dbg;master.tryserver.chromium.linux:linux_mojo
Change-Id: I1ca23543f54e3eb5445bd476c24cf4a1bed06c37
Reviewed-on: https://chromium-review.googlesource.com/801936
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521568}

content/gpu/gpu_sandbox_hook_linux.cc

@@ -20,6 +20,7 @@
 #include "content/public/common/content_switches.h"
 #include "media/gpu/features.h"
 #include "sandbox/linux/bpf_dsl/policy.h"
+#include "sandbox/linux/syscall_broker/broker_command.h"
 #include "sandbox/linux/syscall_broker/broker_file_permission.h"
 #include "sandbox/linux/syscall_broker/broker_process.h"
 #include "services/service_manager/embedder/set_process_title.h"
@@ -202,11 +203,10 @@ void AddStandardGpuWhiteList(std::vector<BrokerFilePermission>* permissions) {
 
 std::vector<BrokerFilePermission> FilePermissionsForGpu(
     const service_manager::SandboxSeccompBPF::Options& options) {
-  std::vector<BrokerFilePermission> permissions;
-
   // All GPU process policies need this file brokered out.
   static const char kDriRcPath[] = "/etc/drirc";
-  permissions.push_back(BrokerFilePermission::ReadOnly(kDriRcPath));
+  std::vector<BrokerFilePermission> permissions = {
+      BrokerFilePermission::ReadOnly(kDriRcPath)};
 
   if (IsChromeOS()) {
     if (UseV4L2Codec())
@@ -320,6 +320,18 @@ bool LoadLibrariesForGpu(
   return true;
 }
 
+sandbox::syscall_broker::BrokerCommandSet CommandSetForGPU(
+    const service_manager::SandboxLinux::Options& options) {
+  sandbox::syscall_broker::BrokerCommandSet command_set;
+  command_set.set(sandbox::syscall_broker::COMMAND_ACCESS);
+  command_set.set(sandbox::syscall_broker::COMMAND_OPEN);
+  if (IsChromeOS() && options.use_amd_specific_policies) {
+    command_set.set(sandbox::syscall_broker::COMMAND_READLINK);
+    command_set.set(sandbox::syscall_broker::COMMAND_STAT);
+  }
+  return command_set;
+}
+
 bool BrokerProcessPreSandboxHook(
     service_manager::BPFBasePolicy* policy,
     service_manager::SandboxLinux::Options options) {
@@ -334,10 +346,9 @@ bool BrokerProcessPreSandboxHook(
 
 bool GpuProcessPreSandboxHook(service_manager::BPFBasePolicy* policy,
                               service_manager::SandboxLinux::Options options) {
-  auto* instance = service_manager::SandboxLinux::GetInstance();
-  instance->StartBrokerProcess(policy, FilePermissionsForGpu(options),
-                               base::BindOnce(BrokerProcessPreSandboxHook),
-                               options);
+  service_manager::SandboxLinux::GetInstance()->StartBrokerProcess(
+      policy, CommandSetForGPU(options), FilePermissionsForGpu(options),
+      base::BindOnce(BrokerProcessPreSandboxHook), options);
 
   if (!LoadLibrariesForGpu(options))
     return false;

content/network/BUILD.gn

@@ -96,6 +96,10 @@ jumbo_source_set("network_sources") {
     "//services/service_manager/public/cpp",
     "//services/service_manager/public/interfaces",
   ]
+
+  if (is_linux) {
+    deps += [ "//sandbox/linux:sandbox_services" ]
+  }
 }
 
 service_manifest("manifest") {

content/network/OWNERS

@@ -4,6 +4,7 @@ kinuko@chromium.org
 mmenke@chromium.org
 rdsmith@chromium.org
 scottmg@chromium.org
+tsepez@chromium.org
 xunjieli@chromium.org
 yhirano@chromium.org
 yzshen@chromium.org

content/network/network_sandbox_hook_linux.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "content/network/network_sandbox_hook_linux.h"
+#include "sandbox/linux/syscall_broker/broker_command.h"
 
 #include "base/rand_util.h"
 #include "base/sys_info.h"
@@ -13,13 +14,19 @@ namespace content {
 
 bool NetworkPreSandboxHook(service_manager::BPFBasePolicy* policy,
                            service_manager::SandboxLinux::Options options) {
+  sandbox::syscall_broker::BrokerCommandSet command_set;
+  command_set.set(sandbox::syscall_broker::COMMAND_ACCESS);
+  command_set.set(sandbox::syscall_broker::COMMAND_OPEN);
+  command_set.set(sandbox::syscall_broker::COMMAND_READLINK);
+  command_set.set(sandbox::syscall_broker::COMMAND_RENAME);
+  command_set.set(sandbox::syscall_broker::COMMAND_STAT);
+
   // TODO(tsepez): FIX THIS.
-  std::vector<BrokerFilePermission> file_permissions;
-  file_permissions.push_back(
-      BrokerFilePermission::ReadWriteCreateRecursive("/"));
+  std::vector<BrokerFilePermission> file_permissions = {
+      BrokerFilePermission::ReadWriteCreateRecursive("/")};
 
   service_manager::SandboxLinux::GetInstance()->StartBrokerProcess(
-      policy, std::move(file_permissions),
+      policy, command_set, std::move(file_permissions),
       service_manager::SandboxLinux::PreSandboxHook(), options);
 
   return true;

sandbox/linux/BUILD.gn

@@ -358,7 +358,8 @@ component("sandbox_services") {
     "syscall_broker/broker_channel.h",
     "syscall_broker/broker_client.cc",
     "syscall_broker/broker_client.h",
-    "syscall_broker/broker_common.h",
+    "syscall_broker/broker_command.cc",
+    "syscall_broker/broker_command.h",
     "syscall_broker/broker_file_permission.cc",
     "syscall_broker/broker_file_permission.h",
     "syscall_broker/broker_host.cc",
@@ -406,7 +407,8 @@ component("sandbox_services") {
       "syscall_broker/broker_channel.h",
       "syscall_broker/broker_client.cc",
       "syscall_broker/broker_client.h",
-      "syscall_broker/broker_common.h",
+      "syscall_broker/broker_command.cc",
+      "syscall_broker/broker_command.h",
       "syscall_broker/broker_file_permission.cc",
       "syscall_broker/broker_file_permission.h",
       "syscall_broker/broker_host.cc",

sandbox/linux/integration_tests/seccomp_broker_process_unittest.cc

@@ -20,6 +20,7 @@
 #include "sandbox/linux/bpf_dsl/seccomp_macros.h"
 #include "sandbox/linux/seccomp-bpf/bpf_tests.h"
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
+#include "sandbox/linux/syscall_broker/broker_command.h"
 #include "sandbox/linux/syscall_broker/broker_file_permission.h"
 #include "sandbox/linux/syscall_broker/broker_process.h"
 #include "sandbox/linux/system_headers/linux_syscalls.h"
@@ -43,27 +44,27 @@ bool NoOpCallback() {
 class InitializedOpenBroker {
  public:
   InitializedOpenBroker() : initialized_(false) {
-    std::vector<syscall_broker::BrokerFilePermission> permissions;
-    permissions.push_back(
-        syscall_broker::BrokerFilePermission::ReadOnly("/proc/allowed"));
-    permissions.push_back(
-        syscall_broker::BrokerFilePermission::ReadOnly("/proc/cpuinfo"));
-
-    broker_process_.reset(
-        new syscall_broker::BrokerProcess(EPERM, permissions));
-    BPF_ASSERT(broker_process() != NULL);
+    syscall_broker::BrokerCommandSet command_set;
+    command_set.set(syscall_broker::COMMAND_OPEN);
+    command_set.set(syscall_broker::COMMAND_ACCESS);
+    std::vector<syscall_broker::BrokerFilePermission> permissions = {
+        syscall_broker::BrokerFilePermission::ReadOnly("/proc/allowed"),
+        syscall_broker::BrokerFilePermission::ReadOnly("/proc/cpuinfo")};
+    broker_process_ = std::make_unique<syscall_broker::BrokerProcess>(
+        EPERM, command_set, permissions);
     BPF_ASSERT(broker_process_->Init(base::Bind(&NoOpCallback)));
-
     initialized_ = true;
   }
-  bool initialized() { return initialized_; }
-  class syscall_broker::BrokerProcess* broker_process() {
+
+  bool initialized() const { return initialized_; }
+
+  syscall_broker::BrokerProcess* broker_process() const {
     return broker_process_.get();
   }
 
  private:
   bool initialized_;
-  std::unique_ptr<class syscall_broker::BrokerProcess> broker_process_;
+  std::unique_ptr<syscall_broker::BrokerProcess> broker_process_;
   DISALLOW_COPY_AND_ASSIGN(InitializedOpenBroker);
 };
 

sandbox/linux/syscall_broker/broker_client.cc

@@ -17,7 +17,7 @@
 #include "base/posix/unix_domain_socket.h"
 #include "build/build_config.h"
 #include "sandbox/linux/syscall_broker/broker_channel.h"
-#include "sandbox/linux/syscall_broker/broker_common.h"
+#include "sandbox/linux/syscall_broker/broker_command.h"
 #include "sandbox/linux/syscall_broker/broker_policy.h"
 
 #if defined(OS_ANDROID) && !defined(MSG_CMSG_CLOEXEC)
@@ -32,7 +32,7 @@ namespace syscall_broker {
 // as arguments, currently open() and access().
 // Will return -errno like a real system call.
 // This function needs to be async signal safe.
-int BrokerClient::PathAndFlagsSyscall(IPCCommand syscall_type,
+int BrokerClient::PathAndFlagsSyscall(BrokerCommand syscall_type,
                                       const char* pathname,
                                       int flags) const {
   int recvmsg_flags = 0;
@@ -56,13 +56,14 @@ int BrokerClient::PathAndFlagsSyscall(IPCCommand syscall_type,
   // IPC.
   if (fast_check_in_client_) {
     if (syscall_type == COMMAND_OPEN &&
-        !broker_policy_.GetFileNameIfAllowedToOpen(
-            pathname, flags, NULL /* file_to_open */,
-            NULL /* unlink_after_open */)) {
+        !CommandOpenIsSafe(allowed_command_set_, broker_policy_, pathname,
+                           flags, NULL /* file_to_open */,
+                           NULL /* unlink_after_open */)) {
       return -broker_policy_.denied_errno();
     }
     if (syscall_type == COMMAND_ACCESS &&
-        !broker_policy_.GetFileNameIfAllowedToAccess(pathname, flags, NULL)) {
+        !CommandAccessIsSafe(allowed_command_set_, broker_policy_, pathname,
+                             flags, NULL)) {
       return -broker_policy_.denied_errno();
     }
   }
@@ -122,15 +123,16 @@ int BrokerClient::PathAndFlagsSyscall(IPCCommand syscall_type,
 
 BrokerClient::BrokerClient(const BrokerPolicy& broker_policy,
                            BrokerChannel::EndPoint ipc_channel,
+                           const BrokerCommandSet& allowed_command_set,
                            bool fast_check_in_client,
                            bool quiet_failures_for_tests)
     : broker_policy_(broker_policy),
       ipc_channel_(std::move(ipc_channel)),
+      allowed_command_set_(allowed_command_set),
       fast_check_in_client_(fast_check_in_client),
       quiet_failures_for_tests_(quiet_failures_for_tests) {}
 
-BrokerClient::~BrokerClient() {
-}
+BrokerClient::~BrokerClient() {}
 
 int BrokerClient::Access(const char* pathname, int mode) const {
   return PathAndFlagsSyscall(COMMAND_ACCESS, pathname, mode);
@@ -148,7 +150,7 @@ int BrokerClient::Stat64(const char* pathname, struct stat64* sb) {
   return StatFamilySyscall(COMMAND_STAT64, pathname, sb, sizeof(*sb));
 }
 
-int BrokerClient::StatFamilySyscall(IPCCommand syscall_type,
+int BrokerClient::StatFamilySyscall(BrokerCommand syscall_type,
                                     const char* pathname,
                                     void* result_ptr,
                                     size_t expected_result_size) const {

sandbox/linux/syscall_broker/broker_client.h

@@ -11,7 +11,7 @@
 
 #include "base/macros.h"
 #include "sandbox/linux/syscall_broker/broker_channel.h"
-#include "sandbox/linux/syscall_broker/broker_common.h"
+#include "sandbox/linux/syscall_broker/broker_command.h"
 
 namespace sandbox {
 
@@ -36,6 +36,7 @@ class BrokerClient {
   // |quiet_failures_for_tests| to false unless you are writing tests.
   BrokerClient(const BrokerPolicy& policy,
                BrokerChannel::EndPoint ipc_channel,
+               const BrokerCommandSet& allowed_command_set,
                bool fast_check_in_client,
                bool quiet_failures_for_tests);
   ~BrokerClient();
@@ -72,17 +73,18 @@ class BrokerClient {
   int GetIPCDescriptor() const { return ipc_channel_.get(); }
 
  private:
-  int PathAndFlagsSyscall(IPCCommand syscall_type,
+  int PathAndFlagsSyscall(BrokerCommand syscall_type,
                           const char* pathname,
                           int flags) const;
 
-  int StatFamilySyscall(IPCCommand syscall_type,
+  int StatFamilySyscall(BrokerCommand syscall_type,
                         const char* pathname,
                         void* result_ptr,
                         size_t expected_result_size) const;
 
   const BrokerPolicy& broker_policy_;
   const BrokerChannel::EndPoint ipc_channel_;
+  const BrokerCommandSet allowed_command_set_;
   const bool fast_check_in_client_;  // Whether to forward a request that we
                                      // know will be denied to the broker. (Used
                                      // for tests).

sandbox/linux/syscall_broker/broker_command.cc

+// Copyright 2017 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.
+
+#include "sandbox/linux/syscall_broker/broker_command.h"
+#include "sandbox/linux/syscall_broker/broker_policy.h"
+
+namespace sandbox {
+namespace syscall_broker {
+
+bool CommandAccessIsSafe(const BrokerCommandSet& command_set,
+                         const BrokerPolicy& policy,
+                         const char* requested_filename,
+                         int requested_mode,
+                         const char** filename_to_use) {
+  return command_set.test(COMMAND_ACCESS) &&
+         policy.GetFileNameIfAllowedToAccess(requested_filename, requested_mode,
+                                             filename_to_use);
+}
+
+bool CommandOpenIsSafe(const BrokerCommandSet& command_set,
+                       const BrokerPolicy& policy,
+                       const char* requested_filename,
+                       int requested_flags,
+                       const char** filename_to_use,
+                       bool* unlink_after_open) {
+  return command_set.test(COMMAND_OPEN) &&
+         policy.GetFileNameIfAllowedToOpen(requested_filename, requested_flags,
+                                           filename_to_use, unlink_after_open);
+}
+
+bool CommandReadlinkIsSafe(const BrokerCommandSet& command_set,
+                           const BrokerPolicy& policy,
+                           const char* requested_filename,
+                           const char** filename_to_use) {
+  return command_set.test(COMMAND_READLINK) &&
+         policy.GetFileNameIfAllowedToOpen(requested_filename, O_RDONLY,
+                                           filename_to_use, nullptr);
+}
+
+bool CommandRenameIsSafe(const BrokerCommandSet& command_set,
+                         const BrokerPolicy& policy,
+                         const char* old_filename,
+                         const char* new_filename,
+                         const char** old_filename_to_use,
+                         const char** new_filename_to_use) {
+  return command_set.test(COMMAND_RENAME) &&
+         policy.GetFileNameIfAllowedToOpen(old_filename, O_RDWR,
+                                           old_filename_to_use, nullptr) &&
+         policy.GetFileNameIfAllowedToOpen(new_filename, O_RDWR,
+                                           new_filename_to_use, nullptr);
+}
+
+bool CommandStatIsSafe(const BrokerCommandSet& command_set,
+                       const BrokerPolicy& policy,
+                       const char* requested_filename,
+                       const char** filename_to_use) {
+  return command_set.test(COMMAND_STAT) &&
+         policy.GetFileNameIfAllowedToAccess(requested_filename, F_OK,
+                                             filename_to_use);
+}
+
+}  // namespace syscall_broker
+}  // namespace sandbox

sandbox/linux/syscall_broker/broker_common.h → sandbox/linux/syscall_broker/broker_command.h

@@ -2,16 +2,21 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef SANDBOX_LINUX_SYSCALL_BROKER_BROKER_COMMON_H_
-#define SANDBOX_LINUX_SYSCALL_BROKER_BROKER_COMMON_H_
+#ifndef SANDBOX_LINUX_SYSCALL_BROKER_BROKER_COMMAND_H_
+#define SANDBOX_LINUX_SYSCALL_BROKER_BROKER_COMMAND_H_
 
 #include <fcntl.h>
 #include <stddef.h>
+#include <stdint.h>
+
+#include <bitset>
 
 namespace sandbox {
 namespace syscall_broker {
 
-const size_t kMaxMessageLength = 4096;
+class BrokerPolicy;
+
+constexpr size_t kMaxMessageLength = 4096;
 
 // Some flags are local to the current process and cannot be sent over a Unix
 // socket. They need special treatment from the client.
@@ -25,19 +30,57 @@ const size_t kMaxMessageLength = 4096;
 // over unix sockets just fine, so a receiver that would (incorrectly) look at
 // O_CLOEXEC instead of FD_CLOEXEC may be tricked in thinking that the file
 // descriptor will or won't be closed on execve().
-const int kCurrentProcessOpenFlagsMask = O_CLOEXEC;
+constexpr int kCurrentProcessOpenFlagsMask = O_CLOEXEC;
 
-enum IPCCommand {
+enum BrokerCommand {
   COMMAND_INVALID = 0,
-  COMMAND_OPEN,
   COMMAND_ACCESS,
+  COMMAND_OPEN,
+  COMMAND_READLINK,
+  COMMAND_RENAME,
   COMMAND_STAT,
   COMMAND_STAT64,
-  COMMAND_RENAME,
-  COMMAND_READLINK,
+
+  // NOTE: update when adding new commands.
+  COMMAND_MAX = COMMAND_STAT64
 };
 
+using BrokerCommandSet = std::bitset<COMMAND_MAX + 1>;
+
+// Helper functions to perform the same permissions test on either side
+// (client or broker process) of a broker IPC command. The implementations
+// must be safe when called from an async signal handler.
+bool CommandAccessIsSafe(const BrokerCommandSet& command_set,
+                         const BrokerPolicy& policy,
+                         const char* requested_filename,
+                         int requested_mode,  // e.g. F_OK, R_OK, W_OK.
+                         const char** filename_to_use);
+
+bool CommandOpenIsSafe(const BrokerCommandSet& command_set,
+                       const BrokerPolicy& policy,
+                       const char* requested_filename,
+                       int requested_flags,  // e.g. O_RDONLY, O_RDWR.
+                       const char** filename_to_use,
+                       bool* unlink_after_open);
+
+bool CommandReadlinkIsSafe(const BrokerCommandSet& command_set,
+                           const BrokerPolicy& policy,
+                           const char* requested_filename,
+                           const char** filename_to_use);
+
+bool CommandRenameIsSafe(const BrokerCommandSet& command_set,
+                         const BrokerPolicy& policy,
+                         const char* old_filename,
+                         const char* new_filename,
+                         const char** old_filename_to_use,
+                         const char** new_filename_to_use);
+
+bool CommandStatIsSafe(const BrokerCommandSet& command_set,
+                       const BrokerPolicy& policy,
+                       const char* requested_filename,
+                       const char** filename_to_use);
+
 }  // namespace syscall_broker
 }  // namespace sandbox
 
-#endif  // SANDBOX_LINUX_SYSCALL_BROKER_BROKER_COMMON_H_
+#endif  // SANDBOX_LINUX_SYSCALL_BROKER_BROKER_COMMAND_H_

sandbox/linux/syscall_broker/broker_file_permission.cc

@@ -11,7 +11,7 @@
 #include <string>
 
 #include "base/logging.h"
-#include "sandbox/linux/syscall_broker/broker_common.h"
+#include "sandbox/linux/syscall_broker/broker_command.h"
 
 namespace sandbox {
 namespace syscall_broker {

sandbox/linux/syscall_broker/broker_host.h

@@ -7,6 +7,7 @@
 
 #include "base/macros.h"
 #include "sandbox/linux/syscall_broker/broker_channel.h"
+#include "sandbox/linux/syscall_broker/broker_command.h"
 
 namespace sandbox {
 
@@ -22,6 +23,7 @@ class BrokerHost {
   enum class RequestStatus { LOST_CLIENT = 0, SUCCESS, FAILURE };
 
   BrokerHost(const BrokerPolicy& broker_policy,
+             const BrokerCommandSet& allowed_command_set,
              BrokerChannel::EndPoint ipc_channel);
   ~BrokerHost();
 
@@ -29,6 +31,7 @@ class BrokerHost {
 
  private:
   const BrokerPolicy& broker_policy_;
+  const BrokerCommandSet allowed_command_set_;
   const BrokerChannel::EndPoint ipc_channel_;
 
   DISALLOW_COPY_AND_ASSIGN(BrokerHost);

sandbox/linux/syscall_broker/broker_policy.cc

@@ -13,7 +13,7 @@
 #include <vector>
 
 #include "base/logging.h"
-#include "sandbox/linux/syscall_broker/broker_common.h"
+#include "sandbox/linux/syscall_broker/broker_command.h"
 
 namespace sandbox {
 namespace syscall_broker {

sandbox/linux/syscall_broker/broker_process.cc

@@ -32,13 +32,15 @@ namespace syscall_broker {
 
 BrokerProcess::BrokerProcess(
     int denied_errno,
+    const syscall_broker::BrokerCommandSet& allowed_command_set,
     const std::vector<syscall_broker::BrokerFilePermission>& permissions,
     bool fast_check_in_client,
     bool quiet_failures_for_tests)
     : initialized_(false),
+      broker_pid_(-1),
       fast_check_in_client_(fast_check_in_client),
       quiet_failures_for_tests_(quiet_failures_for_tests),
-      broker_pid_(-1),
+      allowed_command_set_(allowed_command_set),
       broker_policy_(denied_errno, permissions) {}
 
 BrokerProcess::~BrokerProcess() {
@@ -74,9 +76,9 @@ bool BrokerProcess::Init(
     // We are the parent and we have just forked our broker process.
     ipc_reader.reset();
     broker_pid_ = child_pid;
-    broker_client_.reset(new BrokerClient(broker_policy_, std::move(ipc_writer),
-                                          fast_check_in_client_,
-                                          quiet_failures_for_tests_));
+    broker_client_ = std::make_unique<BrokerClient>(
+        broker_policy_, std::move(ipc_writer), allowed_command_set_,
+        fast_check_in_client_, quiet_failures_for_tests_);
     initialized_ = true;
     return true;
   }
@@ -85,7 +87,8 @@ bool BrokerProcess::Init(
   // we get notified if the client disappears.
   ipc_writer.reset();
   CHECK(broker_process_init_callback.Run());
-  BrokerHost broker_host(broker_policy_, std::move(ipc_reader));
+  BrokerHost broker_host(broker_policy_, allowed_command_set_,
+                         std::move(ipc_reader));
   for (;;) {
     switch (broker_host.HandleRequest()) {
       case BrokerHost::RequestStatus::LOST_CLIENT:

sandbox/linux/syscall_broker/broker_process.h

@@ -14,6 +14,7 @@
 #include "base/pickle.h"
 #include "base/process/process.h"
 #include "sandbox/linux/bpf_dsl/trap_registry.h"
+#include "sandbox/linux/syscall_broker/broker_command.h"
 #include "sandbox/linux/syscall_broker/broker_policy.h"
 #include "sandbox/sandbox_export.h"
 
@@ -36,9 +37,16 @@ class BrokerFilePermission;
 // 4. Use open_broker.Open() to open files.
 class SANDBOX_EXPORT BrokerProcess {
  public:
+  // Handler to be used with a bpf_dsl Trap() function to forward system calls
+  // to the methods below.
+  static intptr_t SIGSYS_Handler(const arch_seccomp_data& args,
+                                 void* aux_broker_process);
+
   // |denied_errno| is the error code returned when methods such as Open()
   // or Access() are invoked on a file which is not in the whitelist (EACCESS
-  // would be a typical value). |permissions| describes the whitelisted set
+  // would be a typical value).  |allowed_command_mask| is a bitwise-or of
+  // kBrokerCommand*Mask constants from broker_command.h that further restrict
+  // the syscalls to execute. |permissions| describes the whitelisted set
   // of files the broker is is allowed to access. |fast_check_in_client|
   // controls whether doomed requests are first filtered on the client side
   // before being proxied. Apart from tests, this should always be true since
@@ -50,6 +58,7 @@ class SANDBOX_EXPORT BrokerProcess {
   // don't use it.
   BrokerProcess(
       int denied_errno,
+      const syscall_broker::BrokerCommandSet& allowed_command_set,
       const std::vector<syscall_broker::BrokerFilePermission>& permissions,
       bool fast_check_in_client = true,
       bool quiet_failures_for_tests = false);
@@ -62,38 +71,34 @@ class SANDBOX_EXPORT BrokerProcess {
   // after fork() returns.
   bool Init(const base::Callback<bool(void)>& broker_process_init_callback);
 
-  // Can be used in place of access(). Will be async signal safe.
+  // Return the PID of the child created by Init().
+  int broker_pid() const { return broker_pid_; }
+
+  // The following methods are used in place of the equivalently-named
+  // syscalls by the trap handler. They, in turn, forward the call onto
+  // |broker_client_| for further processing. They will all be async signal
+  // safe. They all return -errno on errors.
+
+  // Can be used in place of access().
   // X_OK will always return an error in practice since the broker process
   // doesn't support execute permissions.
-  // It's similar to the access() system call and will return -errno on errors.
   int Access(const char* pathname, int mode) const;
 
-  // Can be used in place of open(). Will be async signal safe.
+  // Can be used in place of open()
   // The implementation only supports certain white listed flags and will
   // return -EPERM on other flags.
-  // It's similar to the open() system call and will return -errno on errors.
   int Open(const char* pathname, int flags) const;
 
-  // Can be used in place of stat()/stat64(). Will be async signal safe.
-  // It's similar to the stat() system call and will return -errno on errors.
+  // Can be used in place of stat()/stat64().
   int Stat(const char* pathname, struct stat* sb) const;
   int Stat64(const char* pathname, struct stat64* sb) const;
 
-  // Can be used in place of rename(). Will be async signal safe.
-  // It's similar to the rename() system call and will return -errno on errors.
+  // Can be used in place of rename().
   int Rename(const char* oldpath, const char* newpath) const;
 
-  // Can be used in place of readlink(). Will be async signal safe.
-  // It's similar to the read() system call and will return -errno on errors.
+  // Can be used in place of readlink().
   int Readlink(const char* path, char* buf, size_t bufsize) const;
 
-  int broker_pid() const { return broker_pid_; }
-
-  // Handler to be used with a bpf_dsl Trap() function to forward system calls
-  // to the methods above.
-  static intptr_t SIGSYS_Handler(const arch_seccomp_data& args,
-                                 void* aux_broker_process);
-
  private:
   friend class BrokerProcessTestHelper;
 
@@ -102,10 +107,11 @@ class SANDBOX_EXPORT BrokerProcess {
   void CloseChannel();
 
   bool initialized_;  // Whether we've been through Init() yet.
+  pid_t broker_pid_;  // The PID of the broker (child) created in Init().
   const bool fast_check_in_client_;
   const bool quiet_failures_for_tests_;
-  pid_t broker_pid_;  // The PID of the broker (child).
-  syscall_broker::BrokerPolicy broker_policy_;  // Access policy to enforce.
+  syscall_broker::BrokerCommandSet allowed_command_set_;
+  syscall_broker::BrokerPolicy broker_policy_;  // File access whitelist.
   std::unique_ptr<syscall_broker::BrokerClient> broker_client_;
 
   DISALLOW_COPY_AND_ASSIGN(BrokerProcess);

services/service_manager/sandbox/linux/sandbox_linux.cc

@@ -480,12 +480,13 @@ bool SandboxLinux::LimitAddressSpace(const std::string& process_type,
 
 void SandboxLinux::StartBrokerProcess(
     BPFBasePolicy* client_sandbox_policy,
+    const sandbox::syscall_broker::BrokerCommandSet& allowed_command_set,
     std::vector<sandbox::syscall_broker::BrokerFilePermission> permissions,
     PreSandboxHook broker_side_hook,
     const Options& options) {
   // Leaked at shutdown, so use bare |new|.
   broker_process_ = new sandbox::syscall_broker::BrokerProcess(
-      BPFBasePolicy::GetFSDeniedErrno(), permissions);
+      BPFBasePolicy::GetFSDeniedErrno(), allowed_command_set, permissions);
 
   // The initialization callback will perform generic initialization and then
   // call broker_sandboxer_callback.

services/service_manager/sandbox/linux/sandbox_linux.h

@@ -12,6 +12,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/posix/global_descriptors.h"
+#include "sandbox/linux/syscall_broker/broker_command.h"
 #include "sandbox/linux/syscall_broker/broker_file_permission.h"
 #include "services/service_manager/sandbox/export.h"
 #include "services/service_manager/sandbox/linux/sandbox_seccomp_bpf_linux.h"
@@ -209,6 +210,7 @@ class SERVICE_MANAGER_SANDBOX_EXPORT SandboxLinux {
   // vital to the process.
   void StartBrokerProcess(
       BPFBasePolicy* client_sandbox_policy,
+      const sandbox::syscall_broker::BrokerCommandSet& allowed_command_set,
       std::vector<sandbox::syscall_broker::BrokerFilePermission> permissions,
       PreSandboxHook broker_side_hook,
       const Options& options);