Linux sandbox: make policy use ShouldBrokerHandleSyscall()

BPF sandbox policies normal use this code sequence if they are using a
broker:
  auto* broker_process = SandboxLinux::GetInstance()->broker_process();
  if (broker_process->IsSyscallAllowed(sysno)) {
    return Trap(BrokerProcess::SIGSYS_Handler, broker_process);
  }

Switch this to:
  auto* sandbox_linux = SandboxLinux::GetInstance();
  if (sandbox_linux->ShouldBrokerHandleSyscall(sysno))
    return sandbox_linux->HandleViaBroker();

...which has the advantage of being clearer.

This also makes it easier to land SECCOMP_RET_USER_NOTIF support, as
if USER_NOTIF is supported we will use bpf_dsl::UserNotif instead of
bpf_dsl::Trap, and users of the sandbox shouldn't have to care which
one is used.

Bug: 1117351
Change-Id: I809fdb4118fef39d8b142fdd571743c49e0812a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2377422
Commit-Queue: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808573}

sandbox/policy/linux/bpf_audio_policy_linux.cc

@@ -128,9 +128,9 @@ ResultExpr AudioProcessPolicy::EvaluateSyscall(int system_call_number) const {
         return Allow();
 #endif
 
-      auto* broker_process = SandboxLinux::GetInstance()->broker_process();
-      if (broker_process->IsSyscallAllowed(system_call_number))
-        return Trap(BrokerProcess::SIGSYS_Handler, broker_process);
+      auto* sandbox_linux = SandboxLinux::GetInstance();
+      if (sandbox_linux->ShouldBrokerHandleSyscall(system_call_number))
+        return sandbox_linux->HandleViaBroker();
 
       return BPFBasePolicy::EvaluateSyscall(system_call_number);
   }

sandbox/policy/linux/bpf_broker_policy_linux.h

@@ -14,7 +14,7 @@ namespace sandbox {
 namespace policy {
 
 // A broker policy is one for a privileged syscall broker that allows
-// access, open, openat, and (in the non-Chrome OS case) unlink.
+// a limited set of filesystem calls.
 class SANDBOX_POLICY_EXPORT BrokerProcessPolicy : public BPFBasePolicy {
  public:
   explicit BrokerProcessPolicy(

sandbox/policy/linux/bpf_gpu_policy_linux.cc

@@ -100,10 +100,9 @@ ResultExpr GpuProcessPolicy::EvaluateSyscall(int sysno) const {
     return Allow();
 #endif
 
-  auto* broker_process = SandboxLinux::GetInstance()->broker_process();
-  if (broker_process->IsSyscallAllowed(sysno)) {
-    return Trap(BrokerProcess::SIGSYS_Handler, broker_process);
-  }
+  auto* sandbox_linux = SandboxLinux::GetInstance();
+  if (sandbox_linux->ShouldBrokerHandleSyscall(sysno))
+    return sandbox_linux->HandleViaBroker();
 
   // Default on the baseline policy.
   return BPFBasePolicy::EvaluateSyscall(sysno);

sandbox/policy/linux/bpf_ime_policy_linux.cc

@@ -39,9 +39,9 @@ ResultExpr ImeProcessPolicy::EvaluateSyscall(int sysno) const {
       return RestrictGetrusage();
 #endif
     default:
-      auto* broker_process = SandboxLinux::GetInstance()->broker_process();
-      if (broker_process->IsSyscallAllowed(sysno))
-        return Trap(BrokerProcess::SIGSYS_Handler, broker_process);
+      auto* sandbox_linux = SandboxLinux::GetInstance();
+      if (sandbox_linux->ShouldBrokerHandleSyscall(sysno))
+        return sandbox_linux->HandleViaBroker();
 
       return BPFBasePolicy::EvaluateSyscall(sysno);
   }

sandbox/policy/linux/bpf_network_policy_linux.cc

@@ -33,12 +33,11 @@ NetworkProcessPolicy::NetworkProcessPolicy() {}
 NetworkProcessPolicy::~NetworkProcessPolicy() {}
 
 ResultExpr NetworkProcessPolicy::EvaluateSyscall(int sysno) const {
-  auto* broker_process = SandboxLinux::GetInstance()->broker_process();
-  if (broker_process->IsSyscallAllowed(sysno)) {
-    return Trap(BrokerProcess::SIGSYS_Handler, broker_process);
-  }
+  auto* sandbox_linux = SandboxLinux::GetInstance();
+  if (sandbox_linux->ShouldBrokerHandleSyscall(sysno))
+    return sandbox_linux->HandleViaBroker();
 
-  // TODO(tsepez): FIX this.
+  // TODO(mpdenton): FIX this.
   return Allow();
 }
 

sandbox/policy/linux/bpf_speech_recognition_policy_linux.cc

@@ -34,9 +34,9 @@ ResultExpr SpeechRecognitionProcessPolicy::EvaluateSyscall(
       return Allow();
 #endif
     default:
-      auto* broker_process = SandboxLinux::GetInstance()->broker_process();
-      if (broker_process->IsSyscallAllowed(system_call_number))
-        return Trap(BrokerProcess::SIGSYS_Handler, broker_process);
+      auto* sandbox_linux = SandboxLinux::GetInstance();
+      if (sandbox_linux->ShouldBrokerHandleSyscall(system_call_number))
+        return sandbox_linux->HandleViaBroker();
 
       // Default on the content baseline policy.
       return BPFBasePolicy::EvaluateSyscall(system_call_number);

sandbox/policy/linux/bpf_tts_policy_linux.cc

@@ -25,9 +25,9 @@ TtsProcessPolicy::TtsProcessPolicy() {}
 TtsProcessPolicy::~TtsProcessPolicy() {}
 
 ResultExpr TtsProcessPolicy::EvaluateSyscall(int sysno) const {
-  auto* broker_process = SandboxLinux::GetInstance()->broker_process();
-  if (broker_process->IsSyscallAllowed(sysno))
-    return Trap(BrokerProcess::SIGSYS_Handler, broker_process);
+  auto* sandbox_linux = SandboxLinux::GetInstance();
+  if (sandbox_linux->ShouldBrokerHandleSyscall(sysno))
+    return sandbox_linux->HandleViaBroker();
 
   return BPFBasePolicy::EvaluateSyscall(sysno);
 }

sandbox/policy/linux/sandbox_linux.cc

@@ -500,6 +500,15 @@ void SandboxLinux::StartBrokerProcess(
                                              options, allowed_command_set)));
 }
 
+bool SandboxLinux::ShouldBrokerHandleSyscall(int sysno) const {
+  return broker_process_->IsSyscallAllowed(sysno);
+}
+
+sandbox::bpf_dsl::ResultExpr SandboxLinux::HandleViaBroker() const {
+  return sandbox::bpf_dsl::Trap(
+      sandbox::syscall_broker::BrokerProcess::SIGSYS_Handler, broker_process_);
+}
+
 bool SandboxLinux::HasOpenDirectories() const {
   return ProcUtil::HasOpenDirectory(proc_fd_);
 }

sandbox/policy/linux/sandbox_linux.h

@@ -233,9 +233,16 @@ class SANDBOX_POLICY_EXPORT SandboxLinux {
       PreSandboxHook broker_side_hook,
       const Options& options);
 
-  syscall_broker::BrokerProcess* broker_process() const {
-    return broker_process_;
-  }
+  // Returns true if the broker should handle a particular syscall indicated by
+  // |sysno|. This will typically return true for system calls that take
+  // filepaths as arguments.
+  bool ShouldBrokerHandleSyscall(int sysno) const;
+
+  // Returns an expression that indicates the syscall in question should be
+  // handled transparently by the broker process. This is useful for file
+  // syscalls that take pathnames, so we can enforce pathname whitelisting.
+  // Only usable if StartBrokerProcess() was already called.
+  bpf_dsl::ResultExpr HandleViaBroker() const;
 
  private:
   friend struct base::DefaultSingletonTraits<SandboxLinux>;