Add seccomp sandbox for non-SFI NaCl

components/nacl/loader/nonsfi/nonsfi_sandbox.cc

+// Copyright 2014 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 "components/nacl/loader/nonsfi/nonsfi_sandbox.h"
+
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/net.h>
+#include <sys/prctl.h>
+#include <sys/ptrace.h>
+#include <sys/mman.h>
+#include <sys/socket.h>
+#include <sys/syscall.h>
+
+#include "base/basictypes.h"
+#include "base/logging.h"
+#include "build/build_config.h"
+#include "content/public/common/sandbox_init.h"
+#include "sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.h"
+#include "sandbox/linux/seccomp-bpf/sandbox_bpf.h"
+#include "sandbox/linux/seccomp-bpf/sandbox_bpf_policy.h"
+#include "sandbox/linux/seccomp-bpf/trap.h"
+#include "sandbox/linux/services/linux_syscalls.h"
+
+#if defined(__arm__) && !defined(MAP_STACK)
+#define MAP_STACK 0x20000  // Daisy build environment has old headers.

[Mark Seaborn, 2014/04/15 23:03:14]

"Daisy" -> "cros_daisy (ARM)", perhaps?  The term "Daisy" is really obscure so
I'm not sure exactly what is means.

[hamaji, 2014/04/16 02:10:48]

I changed this to "Chrome OS Daisy (ARM)" to let this be more explicit. I copied
this comment from syscall_parameters_restrictions.cc, so I also modified the
original comment.

+#endif
+
+using sandbox::ErrorCode;
+using sandbox::SandboxBPF;
+
+namespace nacl {
+namespace nonsfi {
+namespace {
+
+ErrorCode RestrictFcntlCommandsForNaClNonSfi(SandboxBPF* sb) {

[Mark Seaborn, 2014/04/15 23:03:14]

Now that the Non-SFI sandbox policy is in its own file, you can remove all the
"ForNaClNonSfi" suffixes from the function names, for readability.

[hamaji, 2014/04/16 02:10:48]

Done.

+  ErrorCode::ArgType mask_long_type;
+  if (sizeof(long) == 8)

[Mark Seaborn, 2014/04/15 23:03:14]

Nit: when if/elses are non-trivial (e.g. multi-line), I think the style is to
use {}s around the inner statements.

[hamaji, 2014/04/16 02:10:48]

Done, also modified syscall_parameters_restrictions.cc.

+    mask_long_type = ErrorCode::TP_64BIT;
+  else if (sizeof(long) == 4)
+    mask_long_type = ErrorCode::TP_32BIT;
+  else
+    NOTREACHED();
+  // We allow following cases:
+  // 1. F_SETFD + FD_CLOEXEC: libevent's epoll_init uses this.
+  // 2. F_GETFL: Used by SetNonBlocking in
+  // message_pump_libevent.cc and Channel::ChannelImpl::CreatePipe
+  // in ipc_channel_posix.cc. Note that the latter does not work
+  // with EPERM.
+  // 3. F_SETFL: Used by evutil_make_socket_nonblocking in
+  // libevent and SetNonBlocking. As the latter mix O_NONBLOCK to
+  // the return value of F_GETFL, so we need to allow O_ACCMODE in
+  // addition to O_NONBLOCK.
+  const unsigned long denied_mask = ~(O_ACCMODE | O_NONBLOCK);
+  return sb->Cond(1, ErrorCode::TP_32BIT,
+                  ErrorCode::OP_EQUAL, F_SETFD,
+                  sb->Cond(2, mask_long_type,
+                           ErrorCode::OP_EQUAL, FD_CLOEXEC,
+                           ErrorCode(ErrorCode::ERR_ALLOWED),
+                  sb->Trap(sandbox::CrashSIGSYS_Handler, NULL)),
+         sb->Cond(1, ErrorCode::TP_32BIT,
+                  ErrorCode::OP_EQUAL, F_GETFL,
+                  ErrorCode(ErrorCode::ERR_ALLOWED),
+         sb->Cond(1, ErrorCode::TP_32BIT,
+                  ErrorCode::OP_EQUAL, F_SETFL,
+                  sb->Cond(2, mask_long_type,
+                           ErrorCode::OP_HAS_ANY_BITS, denied_mask,
+                           sb->Trap(sandbox::CrashSIGSYS_Handler, NULL),
+                           ErrorCode(ErrorCode::ERR_ALLOWED)),
+         sb->Trap(sandbox::CrashSIGSYS_Handler, NULL))));
+}
+
+ErrorCode RestrictCloneForNaClNonSfi(SandboxBPF* sb) {
+  // We allow clone only for new thread creation.
+  return sb->Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
+                  CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND |
+                  CLONE_THREAD | CLONE_SYSVSEM | CLONE_SETTLS |
+                  CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID,
+                  ErrorCode(ErrorCode::ERR_ALLOWED),
+         sb->Trap(sandbox::SIGSYSCloneFailure, NULL));
+}
+
+ErrorCode RestrictPrctlForNaClNonSfi(SandboxBPF* sb) {
+  // base::PlatformThread::SetName() uses PR_SET_NAME so we return
+  // EPERM for it. Otherwise, we will raise SIGSYS.
+  return sb->Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
+                  PR_SET_NAME, ErrorCode(EPERM),
+         sb->Trap(sandbox::SIGSYSPrctlFailure, NULL));
+}
+
+#if defined(__i386__)
+ErrorCode RestrictSocketcallForNaClNonSfi(SandboxBPF* sb) {
+  // We only allow socketpair, sendmsg, and recvmsg.
+  return sb->Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
+                  SYS_SOCKETPAIR,
+                  ErrorCode(ErrorCode::ERR_ALLOWED),
+         sb->Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
+                  SYS_SENDMSG,
+                  ErrorCode(ErrorCode::ERR_ALLOWED),
+         sb->Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
+                  SYS_RECVMSG,
+                  ErrorCode(ErrorCode::ERR_ALLOWED),
+         sb->Cond(0, ErrorCode::TP_32BIT, ErrorCode::OP_EQUAL,
+                  SYS_SHUTDOWN,
+                  ErrorCode(ErrorCode::ERR_ALLOWED),
+                  sb->Trap(sandbox::CrashSIGSYS_Handler, NULL)))));
+}
+#endif
+
+ErrorCode RestrictMemoryProtectionForNaClNonSfi(SandboxBPF* sb, int argno) {
+  // TODO(jln, keescook, drewry): Limit the use of mmap/mprotect by
+  // adding some features to linux kernel.
+  const uint32_t denied_mask = ~(PROT_READ | PROT_WRITE | PROT_EXEC);
+  return sb->Cond(argno, ErrorCode::TP_32BIT,
+                  ErrorCode::OP_HAS_ANY_BITS,
+                  denied_mask,
+         sb->Trap(sandbox::CrashSIGSYS_Handler, NULL),
+                  ErrorCode(ErrorCode::ERR_ALLOWED));
+}
+
+ErrorCode RestrictMmapForNaClNonSfi(SandboxBPF* sb) {
+  const uint32_t denied_flag_mask = ~(MAP_SHARED | MAP_PRIVATE |
+                                      MAP_ANONYMOUS | MAP_STACK | MAP_FIXED);
+  // TODO(hamaji): Disallow RWX mmap.
+  return sb->Cond(3, ErrorCode::TP_32BIT,
+                  ErrorCode::OP_HAS_ANY_BITS,
+                  denied_flag_mask,
+         sb->Trap(sandbox::CrashSIGSYS_Handler, NULL),
+                  RestrictMemoryProtectionForNaClNonSfi(sb, 2));
+}
+
+ErrorCode RestrictSocketpairForNaClNonSfi(SandboxBPF* sb) {
+  // Only allow AF_UNIX, PF_UNIX. Crash if anything else is seen.
+  COMPILE_ASSERT(AF_UNIX == PF_UNIX, af_unix_pf_unix_different);
+  return sb->Cond(0, ErrorCode::TP_32BIT,
+                  ErrorCode::OP_EQUAL, AF_UNIX,
+                  ErrorCode(ErrorCode::ERR_ALLOWED),
+         sb->Trap(sandbox::CrashSIGSYS_Handler, NULL));
+}
+
+bool IsGracefullyDenied(int sysno) {
+  switch (sysno) {
+    // third_party/libevent uses them, but we can just return -1 from
+    // them as it is just checking getuid() != geteuid() and
+    // getgid() != getegid()
+#if defined(__i386__) || defined(__arm__)
+    case __NR_getegid32:
+    case __NR_geteuid32:
+    case __NR_getgid32:
+    case __NR_getuid32:
+#endif
+#if defined(__x86_64__)
+    case __NR_getegid:
+    case __NR_geteuid:
+    case __NR_getgid:
+    case __NR_getuid:
+#endif
+    // tcmalloc calls madvise in TCMalloc_SystemRelease.
+    case __NR_madvise:
+    // EPERM instead of SIGSYS as glibc tries to open files in /proc.
+    // TODO(hamaji): Remove this when we switch to newlib.
+    case __NR_open:
+    // For RunSandboxSanityChecks().
+    case __NR_ptrace:
+    // glibc uses this for its pthread implementation. If we return
+    // EPERM for this, glibc will stop using this.
+    // TODO(hamaji): newlib does not use this. Make this SIGTRAP once
+    // we have switched to newlib.
+    case __NR_set_robust_list:
+    // This is obsolete in ARM EABI, but x86 glibc indirectly calls
+    // this in sysconf.
+#if defined(__i386__) || defined(__x86_64__)
+    case __NR_time:
+#endif
+      return true;
+
+    default:
+      return false;
+  }
+}
+
+void RunSandboxSanityChecks() {
+  errno = 0;
+  // Make a ptrace request with an invalid PID.
+  long ptrace_ret = ptrace(PTRACE_PEEKUSER, -1 /* pid */, NULL, NULL);
+  CHECK_EQ(-1, ptrace_ret);
+  // Without the sandbox on, this ptrace call would ESRCH instead.
+  CHECK_EQ(EPERM, errno);
+}
+
+}  // namespace
+
+ErrorCode NaClNonSfiBPFSandboxPolicy::EvaluateSyscall(
+    SandboxBPF* sb, int sysno) const {
+  return EvaluateSyscallImpl(sb, sysno, NULL);
+}
+
+ErrorCode NaClNonSfiBPFSandboxPolicy::EvaluateSyscallImpl(
+    SandboxBPF* sb, int sysno, void*) {
+  switch (sysno) {
+    // Allowed syscalls.
+#if defined(__i386__) || defined(__arm__)
+    case __NR__llseek:
+#endif
+#if defined(__x86_64__)

[Mark Seaborn, 2014/04/15 23:03:14]

Nit: maybe use #elif for brevity/readability in all cases like this that are
obviously mutually-exclusive alternatives?

[hamaji, 2014/04/16 02:10:48]

Done.

+    case __NR_lseek:
+#endif
+    case __NR_clock_gettime:
+    case __NR_close:
+    case __NR_dup:
+    case __NR_dup2:
+    case __NR_epoll_create:
+    case __NR_epoll_ctl:
+    case __NR_epoll_wait:
+    case __NR_exit:
+    case __NR_exit_group:
+#if defined(__i386__) || defined(__arm__)
+    case __NR_fstat64:
+#endif
+#if defined(__x86_64__)

[Mark Seaborn, 2014/04/15 23:03:14]

Similarly, #elif?

[hamaji, 2014/04/16 02:10:48]

Done.

+    case __NR_fstat:
+#endif
+    case __NR_futex:

[Mark Seaborn, 2014/04/15 23:03:14]

[For future] We need to restrict this to only allow operations with
FUTEX_PRIVATE_FLAG.

Otherwise, there is a big, not-so-covert communications channel between
processes:  two processes that map a common file (e.g. nacl_helper) can wake
each other by signalling a futex on the common memory pages.

This used to work even on read-only pages, and maybe it still does -- I've not
checked.  (Yes, read-only pages can still be used for sending messages.)

[hamaji, 2014/04/16 02:10:48]

Got it. I assume we want TODOs for them? Added some information.

+    case __NR_gettid:

[Mark Seaborn, 2014/04/15 23:03:14]

[For future] Do we actually need this?  Ideally we would not reveal PIDs and
TIDs.

[hamaji, 2014/04/16 02:10:48]

FYI: I tried to change this to EPERM and a test was failing.

One caller of this is PlatformThread::CurrentId().

+    case __NR_gettimeofday:
+    case __NR_munmap:
+    case __NR_nanosleep:
+    case __NR_pipe:

[Mark Seaborn, 2014/04/15 23:03:14]

[For future] Do we need pipe()?  We should aim to disallow it.

[hamaji, 2014/04/16 02:10:48]

FYI: base::MessagePumpLibevent::Init crashes without this.

+    case __NR_pread64:
+    case __NR_read:
+    case __NR_restart_syscall:
+    case __NR_sched_yield:
+    case __NR_sigaltstack:

[Mark Seaborn, 2014/04/15 23:03:14]

We shouldn't be using sigaltstack at the moment.  If the tests pass on x86-32
with this removed, I'd like to remove it, just to make it clear if/when we're
adding it back.

[jln (very slow on Chromium), 2014/04/16 01:36:11]

Seconded, I would love to not have it.

[hamaji, 2014/04/16 02:10:48]

Done.

+    case __NR_write:
+    case __NR_pwrite64:

[jln (very slow on Chromium), 2014/04/16 01:36:11]

I missed this: let's just keep them sorted.

[hamaji, 2014/04/16 02:10:48]

Done.

+#if defined(__arm__)
+    case __ARM_NR_cacheflush:
+#endif
+    // NaCl runtime exposes clock_getres to untrusted code.
+    case __NR_clock_getres:

[Mark Seaborn, 2014/04/15 23:03:14]

Nit: put next to clock_gettime?

[jln (very slow on Chromium), 2014/04/16 01:36:11]

Could you actually re-sort? There are a few which are no longer sorted.

[hamaji, 2014/04/16 02:10:48]

Done.

[hamaji, 2014/04/16 02:10:48]

Done.

+    // __NR_times needed as clock() is called by CommandBufferHelper, which is
+    // used by NaCl applications that use Pepper's 3D interfaces.
+    // See crbug.com/264856 for details.
+    case __NR_times:
+      return ErrorCode(ErrorCode::ERR_ALLOWED);
+
+    case __NR_clone:
+      return RestrictCloneForNaClNonSfi(sb);
+
+#if defined(__x86_64__)
+    case __NR_fcntl:
+#endif
+#if defined(__i386__) || defined(__arm__)
+    case __NR_fcntl64:
+#endif
+      return RestrictFcntlCommandsForNaClNonSfi(sb);
+
+#if defined(__x86_64__)
+    case __NR_mmap:
+#endif
+#if defined(__i386__) || defined(__arm__)
+    case __NR_mmap2:
+#endif
+      return RestrictMmapForNaClNonSfi(sb);
+    case __NR_mprotect:
+      return RestrictMemoryProtectionForNaClNonSfi(sb, 2);
+
+    case __NR_prctl:
+      return RestrictPrctlForNaClNonSfi(sb);
+
+#if defined(__i386__)
+    case __NR_socketcall:
+      return RestrictSocketcallForNaClNonSfi(sb);
+#endif
+#if defined(__x86_64__) || defined(__arm__)
+    case __NR_recvmsg:
+    case __NR_sendmsg:
+    case __NR_shutdown:
+      return ErrorCode(ErrorCode::ERR_ALLOWED);
+    case __NR_socketpair:
+      return RestrictSocketpairForNaClNonSfi(sb);
+#endif
+
+    case __NR_brk:
+      // The behavior of brk on Linux is different from other system
+      // calls. It does not return errno but the current break on
+      // failure. glibc thinks brk failed the return value of brk

[Mark Seaborn, 2014/04/15 23:03:14]

Missing "if": "brk failed if"

[hamaji, 2014/04/16 02:10:48]

Done.

+      // is lesser than the requested address (i.e., brk(addr) < addr)

[Mark Seaborn, 2014/04/15 23:03:14]

"less than".  Also add "." at end of line.

[hamaji, 2014/04/16 02:10:48]

Done.

+      // So, glibc thinks brk succeeded if we return -EPERM and we
+      // need to return zero instead.
+      return ErrorCode(0);
+
+    default:
+      if (IsGracefullyDenied(sysno))
+        return ErrorCode(EPERM);
+      return sb->Trap(sandbox::CrashSIGSYS_Handler, NULL);
+  }
+}
+
+bool InitializeBPFSandbox() {
+  bool sandbox_is_initialized = content::InitializeSandbox(
+      scoped_ptr<sandbox::SandboxBPFPolicy>(
+          new nacl::nonsfi::NaClNonSfiBPFSandboxPolicy()));
+  if (!sandbox_is_initialized)
+    return false;
+  RunSandboxSanityChecks();
+  return true;
+}
+
+}  // namespace nonsfi
+}  // namespace nacl