sandbox: Fix RedirectToUserSpacePolicyWrapper

sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc

 // Copyright (c) 2012 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 <errno.h>
 #include <pthread.h>
 #include <sched.h>
 #include <signal.h>
 #include <sys/prctl.h>
 #include <sys/ptrace.h>
@@ skipping 486 matching lines @@
 }
 
 ErrorCode GreyListedPolicy(SandboxBPF* sandbox, int sysno, int* aux) {
   // Set the global environment for unsafe traps once.
   if (sysno == MIN_SYSCALL) {
     EnableUnsafeTraps();
   }
 
   // Some system calls must always be allowed, if our policy wants to make
   // use of UnsafeTrap()
-  if (sysno == __NR_rt_sigprocmask || sysno == __NR_rt_sigreturn
-#if defined(__NR_sigprocmask)
-      ||
-      sysno == __NR_sigprocmask
-#endif
-#if defined(__NR_sigreturn)
-      ||
-      sysno == __NR_sigreturn
-#endif
-      ) {
+  if (SandboxBPF::IsRequiredForUnsafeTrap(sysno)) {
     return ErrorCode(ErrorCode::ERR_ALLOWED);
   } else if (sysno == __NR_getpid) {
     // Disallow getpid()
     return ErrorCode(EPERM);
   } else if (SandboxBPF::IsValidSyscallNumber(sysno)) {
     // Allow (and count) all other system calls.
     return sandbox->UnsafeTrap(CountSyscalls, aux);
   } else {
     return ErrorCode(ENOSYS);
   }
@@ skipping 103 matching lines @@
 };
 
 ErrorCode RedirectAllSyscallsPolicy::EvaluateSyscall(SandboxBPF* sandbox,
                                                      int sysno) const {
   DCHECK(SandboxBPF::IsValidSyscallNumber(sysno));
   setenv(kSandboxDebuggingEnv, "t", 0);
   Die::SuppressInfoMessages(true);
 
   // Some system calls must always be allowed, if our policy wants to make
   // use of UnsafeTrap()
-  if (sysno == __NR_rt_sigprocmask || sysno == __NR_rt_sigreturn
-#if defined(__NR_sigprocmask)
-      ||
-      sysno == __NR_sigprocmask
-#endif
-#if defined(__NR_sigreturn)
-      ||
-      sysno == __NR_sigreturn
-#endif
-      ) {
+  if (SandboxBPF::IsRequiredForUnsafeTrap(sysno))
     return ErrorCode(ErrorCode::ERR_ALLOWED);
-  }
   return sandbox->UnsafeTrap(AllowRedirectedSyscall, NULL);
 }
 
 int bus_handler_fd_ = -1;
 
 void SigBusHandler(int, siginfo_t* info, void* void_context) {
   BPF_ASSERT(write(bus_handler_fd_, "\x55", 1) == 1);
 }
 
 BPF_TEST_C(SandboxBPF, SigBus, RedirectAllSyscallsPolicy) {
@@ skipping 1592 matching lines @@
 #if !defined(THREAD_SANITIZER)
 SANDBOX_DEATH_TEST(SandboxBPF, StartSingleThreadedAsMultiThreaded,
     DEATH_MESSAGE("Cannot start sandbox; process may be single-threaded when "
                   "reported as not")) {
   SandboxBPF sandbox;
   sandbox.SetSandboxPolicy(new AllowAllPolicy());
   BPF_ASSERT(!sandbox.StartSandbox(SandboxBPF::PROCESS_MULTI_THREADED));
 }
 #endif  // !defined(THREAD_SANITIZER)
 
+// A stub handler for the UnsafeTrap. Never called.
+intptr_t NoOpHandler(const struct arch_seccomp_data& args, void*) {
+  return -1;
+}
+
+class UnsafeTrapWithCondPolicy : public SandboxBPFPolicy {
+ public:
+  UnsafeTrapWithCondPolicy() {}
+  virtual ErrorCode EvaluateSyscall(SandboxBPF* sandbox,
+                                    int sysno) const OVERRIDE {
+    DCHECK(SandboxBPF::IsValidSyscallNumber(sysno));
+    setenv(kSandboxDebuggingEnv, "t", 0);
+    Die::SuppressInfoMessages(true);
+
+    if (SandboxBPF::IsRequiredForUnsafeTrap(sysno))
+      return ErrorCode(ErrorCode::ERR_ALLOWED);
+
+    switch (sysno) {
+      case __NR_close:
+        return sandbox->Cond(0,
+                             ErrorCode::TP_32BIT,
+                             ErrorCode::OP_EQUAL,
+                             100,
+                             ErrorCode(ErrorCode::ERR_ALLOWED),
+                             ErrorCode(EPERM));
+      case __NR_setgid:
+        return sandbox->Cond(0,
+                             ErrorCode::TP_32BIT,
+                             ErrorCode::OP_EQUAL,
+                             100,
+                             ErrorCode(ErrorCode(ENOMEM)),
+                             sandbox->Cond(0,
+                                           ErrorCode::TP_32BIT,
+                                           ErrorCode::OP_EQUAL,
+                                           200,
+                                           ErrorCode(ENOSYS),
+                                           ErrorCode(EPERM)));
+      case __NR_write:
+      case __NR_exit_group:
+        return ErrorCode(ErrorCode::ERR_ALLOWED);
+      case __NR_getppid:
+        return sandbox->UnsafeTrap(NoOpHandler, NULL);
+      default:
+        return ErrorCode(EPERM);
+    }
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(UnsafeTrapWithCondPolicy);
+};
+
+BPF_TEST_C(SandboxBPF, UnsafeTrapWithCond, UnsafeTrapWithCondPolicy) {
+  BPF_ASSERT_EQ(-1, syscall(__NR_close, 100));

[jln (very slow on Chromium), 2014/09/02 18:41:54]

In general it's not a good idea to add strong assumptions of how file
descriptors are going to look like. We're inheriting this from where the test
suite runs, and it runs on a variety of platforms, sometimes as root, etc..

I'm also a little uneasy disallowing close() given how fundamental a system call
it is, in case of test failure, it may be called in some important way.

How about using something less fundamental like uname(2), or madvise(2)?

[leecam, 2014/09/02 19:00:12]

I agree, I was trying to think of a syscall that takes a simple parameter (not a
userspace pointer) that can be checked by the policy, exists on all
architectures and will return the same result when ran as root or not. 

uname and madvise take pointers and I wasnt keen on that. I thought setuid(0)
but that would succeed with root and fail when not. So I picked close with a
high fd but agree its not ideal. I'll go through the list and try pick something
better. Maybe just uname(-1) which will fail consistently if the policy failed
to block it.  

+  BPF_ASSERT_EQ(EBADF, errno);
+
+  BPF_ASSERT_EQ(-1, syscall(__NR_close, 101));
+  BPF_ASSERT_EQ(EPERM, errno);
+
+  BPF_ASSERT_EQ(-1, syscall(__NR_setgid, 100));
+  BPF_ASSERT_EQ(ENOMEM, errno);
+
+  BPF_ASSERT_EQ(-1, syscall(__NR_setgid, 200));
+  BPF_ASSERT_EQ(ENOSYS, errno);
+
+  BPF_ASSERT_EQ(-1, syscall(__NR_setgid, 300));
+  BPF_ASSERT_EQ(EPERM, errno);
+}
+
 }  // namespace
 
 }  // namespace sandbox