[reland] Remove SandboxBPF's dependency on CompatibilityPolicy

SandboxBPF users are now required to always provide a SandboxBPFPolicy
instead of a SyscallEvaluator. CompatibilityPolicy can't be removed
just yet though because it's still used by the deprecated BPF_TEST
macros.

Review URL: https://codereview.chromium.org/286903005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271814 0039d316-1c4b-4281-b951-d872f2087c98

sandbox/linux/seccomp-bpf/demo.cc

@@ -26,12 +26,15 @@
 #include <time.h>
 #include <unistd.h>
 
+#include "base/macros.h"
 #include "base/posix/eintr_wrapper.h"
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
+#include "sandbox/linux/seccomp-bpf/sandbox_bpf_policy.h"
 #include "sandbox/linux/services/linux_syscalls.h"
 
 using sandbox::ErrorCode;
 using sandbox::SandboxBPF;
+using sandbox::SandboxBPFPolicy;
 using sandbox::arch_seccomp_data;
 
 #define ERR EPERM
@@ -237,7 +240,17 @@ intptr_t DefaultHandler(const struct arch_seccomp_data& data, void *) {
   return -ERR;
 }
 
-ErrorCode Evaluator(SandboxBPF* sandbox, int sysno, void *) {
+class DemoPolicy : public SandboxBPFPolicy {
+ public:
+  DemoPolicy() {}
+  virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox,
+                                    int sysno) const OVERRIDE;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(DemoPolicy);
+};
+
+ErrorCode DemoPolicy::EvaluateSyscall(SandboxBPF* sandbox, int sysno) const {
   switch (sysno) {
 #if defined(__NR_accept)
   case __NR_accept: case __NR_accept4:
@@ -420,7 +433,7 @@ int main(int argc, char *argv[]) {
   }
   SandboxBPF sandbox;
   sandbox.set_proc_fd(proc_fd);
-  sandbox.SetSandboxPolicyDeprecated(Evaluator, NULL);
+  sandbox.SetSandboxPolicy(new DemoPolicy());
   if (!sandbox.StartSandbox(SandboxBPF::PROCESS_SINGLE_THREADED)) {
     fprintf(stderr, "StartSandbox() failed");
     _exit(1);

sandbox/linux/seccomp-bpf/sandbox_bpf.cc

@@ -22,10 +22,10 @@
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/posix/eintr_wrapper.h"
 #include "sandbox/linux/seccomp-bpf/codegen.h"
-#include "sandbox/linux/seccomp-bpf/sandbox_bpf_compatibility_policy.h"
 #include "sandbox/linux/seccomp-bpf/sandbox_bpf_policy.h"
 #include "sandbox/linux/seccomp-bpf/syscall.h"
 #include "sandbox/linux/seccomp-bpf/syscall_iterator.h"
@@ -57,20 +57,26 @@ void WriteFailedStderrSetupMessage(int out_fd) {
 
 // We define a really simple sandbox policy. It is just good enough for us
 // to tell that the sandbox has actually been activated.
-ErrorCode ProbeEvaluator(SandboxBPF*, int sysnum, void*) __attribute__((const));
-ErrorCode ProbeEvaluator(SandboxBPF*, int sysnum, void*) {
-  switch (sysnum) {
-    case __NR_getpid:
-      // Return EPERM so that we can check that the filter actually ran.
-      return ErrorCode(EPERM);
-    case __NR_exit_group:
-      // Allow exit() with a non-default return code.
-      return ErrorCode(ErrorCode::ERR_ALLOWED);
-    default:
-      // Make everything else fail in an easily recognizable way.
-      return ErrorCode(EINVAL);
+class ProbePolicy : public SandboxBPFPolicy {
+ public:
+  ProbePolicy() {}
+  virtual ErrorCode EvaluateSyscall(SandboxBPF*, int sysnum) const OVERRIDE {
+    switch (sysnum) {
+      case __NR_getpid:
+        // Return EPERM so that we can check that the filter actually ran.
+        return ErrorCode(EPERM);
+      case __NR_exit_group:
+        // Allow exit() with a non-default return code.
+        return ErrorCode(ErrorCode::ERR_ALLOWED);
+      default:
+        // Make everything else fail in an easily recognizable way.
+        return ErrorCode(EINVAL);
+    }
   }
-}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ProbePolicy);
+};
 
 void ProbeProcess(void) {
   if (syscall(__NR_getpid) < 0 && errno == EPERM) {
@@ -78,10 +84,17 @@ void ProbeProcess(void) {
   }
 }
 
-ErrorCode AllowAllEvaluator(SandboxBPF*, int sysnum, void*) {
-  DCHECK(SandboxBPF::IsValidSyscallNumber(sysnum));
-  return ErrorCode(ErrorCode::ERR_ALLOWED);
-}
+class AllowAllPolicy : public SandboxBPFPolicy {
+ public:
+  AllowAllPolicy() {}
+  virtual ErrorCode EvaluateSyscall(SandboxBPF*, int sysnum) const OVERRIDE {
+    DCHECK(SandboxBPF::IsValidSyscallNumber(sysnum));
+    return ErrorCode(ErrorCode::ERR_ALLOWED);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AllowAllPolicy);
+};
 
 void TryVsyscallProcess(void) {
   time_t current_time;
@@ -239,8 +252,7 @@ bool SandboxBPF::IsValidSyscallNumber(int sysnum) {
 }
 
 bool SandboxBPF::RunFunctionInPolicy(void (*code_in_sandbox)(),
-                                     EvaluateSyscall syscall_evaluator,
-                                     void* aux) {
+                                     scoped_ptr<SandboxBPFPolicy> policy) {
   // Block all signals before forking a child process. This prevents an
   // attacker from manipulating our test by sending us an unexpected signal.
   sigset_t old_mask, new_mask;
@@ -310,7 +322,7 @@ bool SandboxBPF::RunFunctionInPolicy(void (*code_in_sandbox)(),
 #endif
     }
 
-    SetSandboxPolicyDeprecated(syscall_evaluator, aux);
+    SetSandboxPolicy(policy.release());
     if (!StartSandbox(PROCESS_SINGLE_THREADED)) {
       SANDBOX_DIE(NULL);
     }
@@ -359,8 +371,11 @@ bool SandboxBPF::RunFunctionInPolicy(void (*code_in_sandbox)(),
 }
 
 bool SandboxBPF::KernelSupportSeccompBPF() {
-  return RunFunctionInPolicy(ProbeProcess, ProbeEvaluator, 0) &&
-         RunFunctionInPolicy(TryVsyscallProcess, AllowAllEvaluator, 0);
+  return RunFunctionInPolicy(ProbeProcess,
+                             scoped_ptr<SandboxBPFPolicy>(new ProbePolicy())) &&
+         RunFunctionInPolicy(
+             TryVsyscallProcess,
+             scoped_ptr<SandboxBPFPolicy>(new AllowAllPolicy()));
 }
 
 SandboxBPF::SandboxStatus SandboxBPF::SupportsSeccompSandbox(int proc_fd) {
@@ -475,15 +490,6 @@ void SandboxBPF::PolicySanityChecks(SandboxBPFPolicy* policy) {
   return;
 }
 
-// Deprecated API, supported with a wrapper to the new API.
-void SandboxBPF::SetSandboxPolicyDeprecated(EvaluateSyscall syscall_evaluator,
-                                            void* aux) {
-  if (sandbox_has_started_ || !conds_) {
-    SANDBOX_DIE("Cannot change policy after sandbox has started");
-  }
-  SetSandboxPolicy(new CompatibilityPolicy<void>(syscall_evaluator, aux));
-}
-
 // Don't take a scoped_ptr here, polymorphism make their use awkward.
 void SandboxBPF::SetSandboxPolicy(SandboxBPFPolicy* policy) {
   DCHECK(!policy_);

sandbox/linux/seccomp-bpf/sandbox_bpf.h

@@ -65,14 +65,6 @@ class SANDBOX_EXPORT SandboxBPF {
     PROCESS_MULTI_THREADED,   // The program may be multi-threaded.
   };
 
-  // When calling setSandboxPolicy(), the caller can provide an arbitrary
-  // pointer in |aux|. This pointer will then be forwarded to the sandbox
-  // policy each time a call is made through an EvaluateSyscall function
-  // pointer.  One common use case would be to pass the "aux" pointer as an
-  // argument to Trap() functions.
-  typedef ErrorCode (*EvaluateSyscall)(SandboxBPF* sandbox_compiler,
-                                       int system_call_number,
-                                       void* aux);
   // A vector of BPF instructions that need to be installed as a filter
   // program in the kernel.
   typedef std::vector<struct sock_filter> Program;
@@ -109,20 +101,6 @@ class SANDBOX_EXPORT SandboxBPF {
   // eventually close it when "StartSandbox()" executes.
   void set_proc_fd(int proc_fd);
 
-  // The system call evaluator function is called with the system
-  // call number. It can decide to allow the system call unconditionally
-  // by returning ERR_ALLOWED; it can deny the system call unconditionally by
-  // returning an appropriate "errno" value; or it can request inspection
-  // of system call argument(s) by returning a suitable ErrorCode.
-  // The "aux" parameter can be used to pass optional data to the system call
-  // evaluator. There are different possible uses for this data, but one of the
-  // use cases would be for the policy to then forward this pointer to a Trap()
-  // handler. In this case, of course, the data that is pointed to must remain
-  // valid for the entire time that Trap() handlers can be called; typically,
-  // this would be the lifetime of the program.
-  // DEPRECATED: use the policy interface below.
-  void SetSandboxPolicyDeprecated(EvaluateSyscall syscallEvaluator, void* aux);
-
   // Set the BPF policy as |policy|. Ownership of |policy| is transfered here
   // to the sandbox object.
   void SetSandboxPolicy(SandboxBPFPolicy* policy);
@@ -229,8 +207,7 @@ class SANDBOX_EXPORT SandboxBPF {
   // policy. The caller has to make sure that "this" has not yet been
   // initialized with any other policies.
   bool RunFunctionInPolicy(void (*code_in_sandbox)(),
-                           EvaluateSyscall syscall_evaluator,
-                           void* aux);
+                           scoped_ptr<SandboxBPFPolicy> policy);
 
   // Performs a couple of sanity checks to verify that the kernel supports the
   // features that we need for successful sandboxing.

sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc

@@ -22,6 +22,7 @@
 
 #include "base/bind.h"
 #include "base/logging.h"
+#include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "build/build_config.h"
 #include "sandbox/linux/seccomp-bpf/bpf_tests.h"
@@ -84,29 +85,38 @@ intptr_t FakeGetPid(const struct arch_seccomp_data& args, void* aux) {
   return (*pid_ptr)++;
 }
 
-ErrorCode VerboseAPITestingPolicy(SandboxBPF* sandbox, int sysno, void* aux) {
-  if (!SandboxBPF::IsValidSyscallNumber(sysno)) {
-    return ErrorCode(ENOSYS);
-  } else if (sysno == __NR_getpid) {
-    return sandbox->Trap(FakeGetPid, aux);
-  } else {
+class VerboseAPITestingPolicy : public SandboxBPFPolicy {
+ public:
+  VerboseAPITestingPolicy(pid_t* pid_ptr) : pid_ptr_(pid_ptr) {}
+
+  virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox,
+                                    int sysno) const OVERRIDE {
+    DCHECK(SandboxBPF::IsValidSyscallNumber(sysno));
+    if (sysno == __NR_getpid) {
+      return sandbox->Trap(FakeGetPid, pid_ptr_);
+    }
     return ErrorCode(ErrorCode::ERR_ALLOWED);
   }
-}
+
+ private:
+  pid_t* pid_ptr_;
+  DISALLOW_COPY_AND_ASSIGN(VerboseAPITestingPolicy);
+};
 
 SANDBOX_TEST(SandboxBPF, DISABLE_ON_TSAN(VerboseAPITesting)) {
   if (SandboxBPF::SupportsSeccompSandbox(-1) ==
       sandbox::SandboxBPF::STATUS_AVAILABLE) {
-    pid_t test_var = 0;
+    pid_t pid = 0;
+
     SandboxBPF sandbox;
-    sandbox.SetSandboxPolicyDeprecated(VerboseAPITestingPolicy, &test_var);
+    sandbox.SetSandboxPolicy(new VerboseAPITestingPolicy(&pid));
     BPF_ASSERT(sandbox.StartSandbox(SandboxBPF::PROCESS_SINGLE_THREADED));
 
-    BPF_ASSERT(test_var == 0);
-    BPF_ASSERT(syscall(__NR_getpid) == 0);
-    BPF_ASSERT(test_var == 1);
-    BPF_ASSERT(syscall(__NR_getpid) == 1);
-    BPF_ASSERT(test_var == 2);
+    BPF_ASSERT_EQ(0, pid);
+    BPF_ASSERT_EQ(0, syscall(__NR_getpid));
+    BPF_ASSERT_EQ(1, pid);
+    BPF_ASSERT_EQ(1, syscall(__NR_getpid));
+    BPF_ASSERT_EQ(2, pid);
 
     // N.B.: Any future call to getpid() would corrupt the stack.
     //       This is OK. The SANDBOX_TEST() macro is guaranteed to
@@ -284,43 +294,53 @@ BPF_TEST(SandboxBPF, ErrnoTest, ErrnoTestPolicy) {
 
 // Testing the stacking of two sandboxes
 
-ErrorCode StackingPolicyPartOne(SandboxBPF* sandbox, int sysno, void*) {
-  if (!SandboxBPF::IsValidSyscallNumber(sysno)) {
-    return ErrorCode(ENOSYS);
+class StackingPolicyPartOne : public SandboxBPFPolicy {
+ public:
+  StackingPolicyPartOne() {}
+  virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox,
+                                    int sysno) const OVERRIDE {
+    DCHECK(SandboxBPF::IsValidSyscallNumber(sysno));
+    switch (sysno) {
+      case __NR_getppid:
+        return sandbox->Cond(0,
+                             ErrorCode::TP_32BIT,
+                             ErrorCode::OP_EQUAL,
+                             0,
+                             ErrorCode(ErrorCode::ERR_ALLOWED),
+                             ErrorCode(EPERM));
+      default:
+        return ErrorCode(ErrorCode::ERR_ALLOWED);
+    }
   }
 
-  switch (sysno) {
-    case __NR_getppid:
-      return sandbox->Cond(0,
-                           ErrorCode::TP_32BIT,
-                           ErrorCode::OP_EQUAL,
-                           0,
-                           ErrorCode(ErrorCode::ERR_ALLOWED),
-                           ErrorCode(EPERM));
-    default:
-      return ErrorCode(ErrorCode::ERR_ALLOWED);
-  }
-}
+ private:
+  DISALLOW_COPY_AND_ASSIGN(StackingPolicyPartOne);
+};
 
-ErrorCode StackingPolicyPartTwo(SandboxBPF* sandbox, int sysno, void*) {
-  if (!SandboxBPF::IsValidSyscallNumber(sysno)) {
-    return ErrorCode(ENOSYS);
+class StackingPolicyPartTwo : public SandboxBPFPolicy {
+ public:
+  StackingPolicyPartTwo() {}
+  virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox,
+                                    int sysno) const OVERRIDE {
+    DCHECK(SandboxBPF::IsValidSyscallNumber(sysno));
+    switch (sysno) {
+      case __NR_getppid:
+        return sandbox->Cond(0,
+                             ErrorCode::TP_32BIT,
+                             ErrorCode::OP_EQUAL,
+                             0,
+                             ErrorCode(EINVAL),
+                             ErrorCode(ErrorCode::ERR_ALLOWED));
+      default:
+        return ErrorCode(ErrorCode::ERR_ALLOWED);
+    }
   }
 
-  switch (sysno) {
-    case __NR_getppid:
-      return sandbox->Cond(0,
-                           ErrorCode::TP_32BIT,
-                           ErrorCode::OP_EQUAL,
-                           0,
-                           ErrorCode(EINVAL),
-                           ErrorCode(ErrorCode::ERR_ALLOWED));
-    default:
-      return ErrorCode(ErrorCode::ERR_ALLOWED);
-  }
-}
+ private:
+  DISALLOW_COPY_AND_ASSIGN(StackingPolicyPartTwo);
+};
 
-BPF_TEST(SandboxBPF, StackingPolicy, StackingPolicyPartOne) {
+BPF_TEST_C(SandboxBPF, StackingPolicy, StackingPolicyPartOne) {
   errno = 0;
   BPF_ASSERT(syscall(__NR_getppid, 0) > 0);
   BPF_ASSERT(errno == 0);
@@ -331,7 +351,7 @@ BPF_TEST(SandboxBPF, StackingPolicy, StackingPolicyPartOne) {
   // Stack a second sandbox with its own policy. Verify that we can further
   // restrict filters, but we cannot relax existing filters.
   SandboxBPF sandbox;
-  sandbox.SetSandboxPolicyDeprecated(StackingPolicyPartTwo, NULL);
+  sandbox.SetSandboxPolicy(new StackingPolicyPartTwo());
   BPF_ASSERT(sandbox.StartSandbox(SandboxBPF::PROCESS_SINGLE_THREADED));
 
   errno = 0;