Seccomp-bpf: first pass at a non controverial policy cleanup.

content/common/sandbox_seccomp_bpf_linux.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 <asm/unistd.h>
 #include <dlfcn.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/audit.h>
 #include <linux/filter.h>
@@ skipping 179 matching lines @@
     // case __NR_umask:
       return true;
     default:
       return false;
   }
 }
 
 // System calls that directly access the file system. They might aquire
 // a new file descriptor or otherwise perform an operation directly
 // via a path.
-// For many, EPERM is a valid errno, but not for all of them.
+// Both EPERM and ENOENT are valid errno unless otherwise noted in comment.
 bool IsFileSystemSyscall(int sysno) {
   switch (sysno) {
-    case __NR_access:
-    // case __NR_chmod:
-    // case __NR_chown:
-    // case __NR_creat:
+    case __NR_access:          // EPERM not a valid errno.
+    case __NR_chmod:
+    case __NR_chown:
+    case __NR_creat:
     case __NR_execve:
-    // case __NR_faccessat:
-    // case __NR_fchmodat:
-    // case __NR_fchownat:  // Should be called chownat ?
-    // case __NR_futimesat:  // Should be called utimesat ?
-    // case __NR_getdents:
-    // case __NR_getdents64:
-    // case __NR_lchown:
-    // case __NR_link:
-    // case __NR_linkat:
-    // case __NR_lookup_dcookie:
-    case __NR_lstat:
+    case __NR_faccessat:       // EPERM not a valid errno.
+    case __NR_fchmodat:
+    case __NR_fchownat:        // Should be called chownat ?
+    case __NR_futimesat:       // Should be called utimesat ?
+    case __NR_getdents:
+    case __NR_getdents64:

[Chris Evans, 2012/08/09 07:09:15]

getdents() is not a filesystem syscall. It takes an fd, no?

[jln (very slow on Chromium), 2012/08/09 16:59:12]

Oh, wow, I missed that, thanks!

+    case __NR_lchown:
+    case __NR_link:
+    case __NR_linkat:
+    case __NR_lookup_dcookie:  // ENOENT not a valid errno.
+    case __NR_lstat:           // EPERM not a valid errno.
     case __NR_mkdir:
     case __NR_mkdirat:
     case __NR_mknod:
     case __NR_mknodat:
-    // case __NR_newfstatat:  // Should be called statat ?
+    case __NR_newfstatat:      // EPERM not a valid errno.
+                               // Should be called statat ?
     case __NR_open:
     case __NR_openat:
-    case __NR_readlink:
+    case __NR_readlink:        // EPERM not a valid errno.
     case __NR_readlinkat:
-    // case __NR_rename:
-    // case __NR_renameat:
-    // case __NR_rmdir:
-    case __NR_stat:
-    // case __NR_statfs:
-    // case __NR_symlink:
-    // case __NR_symlinkat:
-    // case __NR_truncate:
-    // case __NR_unlink:
-    // case __NR_unlinkat:
-    // case __NR_uselib:
-    // case __NR_ustat:  // Deprecated.
-    // case __NR_utime:
-    // case __NR_utimensat:   // New.
-    // case __NR_utimes:
+    case __NR_rename:
+    case __NR_renameat:
+    case __NR_rmdir:
+    case __NR_stat:            // EPERM not a valid errno.
+    case __NR_statfs:          // EPERM not a valid errno.
+    case __NR_symlink:
+    case __NR_symlinkat:
+    case __NR_truncate:
+    case __NR_unlink:
+    case __NR_unlinkat:
+    case __NR_uselib:          // Neither EPERM, nor ENOENT are valid errno.
+    case __NR_ustat:           // Same as above. Deprecated.
+    case __NR_utime:
+    case __NR_utimensat:       // New.

[Chris Evans, 2012/08/09 07:09:15]

Will this compile on Ubuntu 10.04 ?
Could you quickly check everything against the newest syscall defined in 10.04
header files?

[jln (very slow on Chromium), 2012/08/09 16:59:12]

This problem was getting annoying, so I solved it in a previous CL: we now have
our own header that lists all system calls.

+    case __NR_utimes:
       return true;
     default:
       return false;
   }
 }
 
+// TODO(jln): these should be denied gracefully as well.
 bool IsAllowedFileSystemCapabilitySyscall(int sysno) {
   switch (sysno) {
     // case __NR_fadvise64:
     // case __NR_flock:
     case __NR_fstat:
     // case __NR_fstatfs:  // Give information about the whole filesystem.
     // case __NR_fsync:
     // case __NR_fdatasync:
     // case __NR_sync_file_range:
       return true;
+    default:
+      return false;
+  }
+}
+
+// EPERM is a good errno for any of these.
+bool IsDeniedFileSystemCapabilitySyscall(int sysno) {
+  switch (sysno) {
     case __NR_fallocate:
     case __NR_fchmod:
     case __NR_fchown:
     case __NR_ftruncate:
+      return true;
     default:
       return false;
   }
 }
 
 bool IsGetProcessIdSyscall(int sysno) {

[Chris Evans, 2012/08/09 07:09:15]

Not sure about the name. It's getting process ids _and_ uid/gid, etc. Maybe
"IsGetSimpleIdSyscall" ?

[jln (very slow on Chromium), 2012/08/09 16:59:12]

Done.

   switch (sysno) {
     // case __NR_capget:
     case __NR_getegid:
     case __NR_geteuid:
     case __NR_getgid:
-    // case __NR_getgroups:
-    // case __NR_getpid:
-    // case __NR_getppid:
-    // case __NR_getresgid:
-    // case __NR_getresuid:
-    // case __NR_getsid:
+    case __NR_getgroups:
+    case __NR_getpid:
+    case __NR_getppid:
+    case __NR_getresgid:
+    case __NR_getresuid:
+    case __NR_getsid:
     case __NR_gettid:
     case __NR_getuid:
       return true;
     default:
       return false;
   }
 }
 
 bool IsProcessPrivilegeChange(int sysno) {
   switch (sysno) {
@@ skipping 43 matching lines @@
       return true;
     default:
       return false;
   }
 }
 
 bool IsOperationOnFd(int sysno) {
   switch (sysno) {
     case __NR_close:
     case __NR_dup:
-    // case __NR_dup2:
-    // case __NR_dup3:
+    case __NR_dup2:
+    case __NR_dup3:
     case __NR_fcntl:

[Chris Evans, 2012/08/09 07:09:15]

Might as well add the TODO on fcntl() arg lockdown whilst we're here?

[jln (very slow on Chromium), 2012/08/09 16:59:12]

Yeah, not on the top of the list but worth investigating.

     case __NR_shutdown:
       return true;
     default:
       return false;
   }
 }
 
 bool IsKernelInteralApi(int sysno) {
   switch (sysno) {
     case __NR_restart_syscall:
       return true;
     default:
       return false;
   }
 }
 
+// This should be thought through in conjunction with IsFutex().
 bool IsAllowedProcessStartOrDeath(int sysno) {
   switch (sysno) {
     case __NR_clone:  // TODO(jln): restrict flags.
     case __NR_exit:
     case __NR_exit_group:

[Chris Evans, 2012/08/09 07:09:15]

Just an observation: in the generic case, it'll probably be highly desirable to
have exit() separated out from the other stuff.

[jln (very slow on Chromium), 2012/08/09 16:59:12]

I would think the only one we may want to restrict that is not restricted yet
would be clone, wouldn't it ?

Maybe clone/fork/get_thread*/set_*/vfork below really belong somewhere else?

I think wait* are harmless and should be allow even when clone isn't, after all
we could have some pre-warming that forks a new process. We may even want our
own helper process ((for open etc..) to be a child of the main process).

I think we should probably allow the get_/set_ functions here also, but I wanted
to do more research first.

     // case __NR_fork:
     // case __NR_get_thread_area:
     // case __NR_set_thread_area:
     // case __NR_set_tid_address:
     // case __NR_unshare:
     // case __NR_vfork:
-    // case __NR_wait4:
-    // case __NR_waitid:
+    case __NR_wait4:
+    case __NR_waitid:
       return true;
     case __NR_setns:  // Privileged.
     default:
       return false;
   }
 }
 
+bool IsFutex(int sysno) {
+  switch (sysno) {
+    case __NR_futex:
+    // case __NR_get_robust_list:
+    case __NR_set_robust_list:
+      return true;
+    default:
+      return false;
+  }
+}
+
 bool IsAllowedEpoll(int sysno) {
   switch (sysno) {
     case __NR_epoll_create:
     // case __NR_epoll_create1:
     case __NR_epoll_ctl:
     // case __NR_epoll_ctl_old:
     // case __NR_epoll_pwait:
     case __NR_epoll_wait:
     // case __NR_epoll_wait_old:
       return true;
@@ skipping 12 matching lines @@
     case __NR_accept:
     case __NR_accept4:
     case __NR_bind:
     case __NR_connect:
     case __NR_socket:
     case __NR_listen:
       return false;
   }
 }
 
 bool IsNetworkSocketInformation(int sysno) {

[Chris Evans, 2012/08/09 07:09:15]

Did the old policies really allow all of these syscalls below? Some of them are
awful.

[jln (very slow on Chromium), 2012/08/09 16:59:12]

None of them is allowed at the moment.

   switch (sysno) {
     case __NR_getpeername:
     case __NR_getsockname:
     case __NR_getsockopt:
     case __NR_setsockopt:

[Chris Evans, 2012/08/09 07:09:15]

Ew, need to add a TODO to restrict get/setsockopt.

[jln (very slow on Chromium), 2012/08/09 16:59:12]

It's not allowed. It's in the "Watched" list.

       return true;
     default:
       return false;
   }
 }
 
-bool IsFutex(int sysno) {
-  switch (sysno) {
-    case __NR_futex:
-    // case __NR_get_robust_list:
-    case __NR_set_robust_list:
-      return true;
-    default:
-      return false;
-  }
-}
-
 bool IsAllowedAddressSpaceAccess(int sysno) {
   switch (sysno) {
     case __NR_brk:
     case __NR_madvise:
     // case __NR_mincore:
-    // case __NR_mlock:
+    case __NR_mlock:
     // case __NR_mlockall:
     case __NR_mmap:

[Chris Evans, 2012/08/09 07:09:15]

TODO: restrict flags etc :)

[jln (very slow on Chromium), 2012/08/09 16:59:12]

Done.

     // case __NR_modify_ldt:
     case __NR_mprotect:
     // case __NR_mremap:
     // case __NR_msync:
-    // case __NR_munlock:
+    case __NR_munlock:
     // case __NR_munlockall:
     case __NR_munmap:
     // case __NR_readahead:
     // case __NR_remap_file_pages:
       return true;
     default:
       return false;
   }
 }
 
 bool IsAllowedGeneralIo(int sysno) {
   switch (sysno) {
     case __NR_lseek:
-    // case __NR_poll:
-    // case __NR_ppoll:
+    case __NR_poll:
+    case __NR_ppoll:
     // case __NR_pread64:
     // case __NR_preadv:
-    // case __NR_pselect6:
+    case __NR_pselect6:
     // case __NR_pwrite64:
     // case __NR_pwritev:
     case __NR_read:
-    // case __NR_readv:
-    // case __NR_recvfrom:  // Could specify source.
+    case __NR_readv:
+    case __NR_recvfrom:  // Could specify source.

[Chris Evans, 2012/08/09 07:09:15]

Did you mean to enable recvfrom?

[jln (very slow on Chromium), 2012/08/09 16:59:12]

There is a comment in the very first iteration of this CL, where I explain some
of my choices.

The rationale here was that since recvmsg/sendmsg are allowed, there is really
nothing we can do here.

     // case __NR_recvmmsg:  // Could specify source.
     case __NR_recvmsg:   // Could specify source.
-    // case __NR_select:
+    case __NR_select:
     // case __NR_sendfile:
     // case __NR_sendmmsg:  // Could specify destination.
     case __NR_sendmsg:   // Could specify destination.
-    // case __NR_sendto:    // Could specify destination.
+    case __NR_sendto:    // Could specify destination.
     // case __NR_splice:
     // case __NR_tee:
     // case __NR_vmsplice:
     case __NR_write:
-    // case __NR_writev:
+    case __NR_writev:
       return true;
     default:
     case __NR_ioctl:  // Can be very powerful.
       return false;
   }
 }
 
 bool IsAllowedPrctl(int sysno) {
   switch (sysno) {
     case __NR_prctl:
     // case __NR_arch_prctl:
       return true;
     default:
       return false;
   }
 }
 
 bool IsAllowedBasicScheduler(int sysno) {
   switch (sysno) {
     case __NR_sched_yield:
-    // case __NR_pause:
-    // case __NR_nanosleep:
+    case __NR_pause:
+    case __NR_nanosleep:
     // case __NR_getpriority:
       return true;
     case __NR_setpriority:
     default:
       return false;
   }
 }
 
 bool IsAdminOperation(int sysno) {
   switch (sysno) {
@@ skipping 109 matching lines @@
     case __NR_uname:
       return true;
     default:
       return false;
   }
 }
 
 bool IsEventFd(int sysno) {
   switch (sysno) {
     case __NR_eventfd:
-    // case __NR_eventfd2:
+    case __NR_eventfd2:
       return true;
     default:
       return false;
   }
 }
 
 // Asynchronous I/O API.
 bool IsAsyncIo(int sysno) {
   switch (sysno) {
     case __NR_io_cancel:
@@ skipping 161 matching lines @@
     case __NR_vserver:
       return true;
     default:
       return false;
   }
 }
 
 // End of the system call sets section.
 
 // x86_64 only because it references system calls that are multiplexed on IA32.
-bool IsGpuAndFlashPolicyAllowed_x86_64(int sysno) {
+bool IsBaselinePolicyAllowed_x86_64(int sysno) {
   if (IsAllowedAddressSpaceAccess(sysno) ||
       IsAllowedBasicScheduler(sysno) ||
       IsAllowedEpoll(sysno) ||
       IsAllowedFileSystemCapabilitySyscall(sysno) ||
       IsAllowedGeneralIo(sysno) ||
       IsAllowedGetOrModifySocket(sysno) ||
       IsAllowedGettimeSyscall(sysno) ||
       IsAllowedPrctl(sysno) ||
       IsAllowedProcessStartOrDeath(sysno) ||
       IsAllowedSignalHandling(sysno) ||
       IsFutex(sysno) ||
       IsGetProcessIdSyscall(sysno) ||
       IsKernelInteralApi(sysno) ||
       IsKillSyscall(sysno) ||
       IsOperationOnFd(sysno)) {
     return true;
   } else {
     return false;
   }
 }
 
 // System calls that will trigger the crashing sigsys handler.
-bool IsGpuAndFlashPolicyWatched_x86_64(int sysno) {
+bool IsBaselinePolicyWatched_x86_64(int sysno) {
   if (IsAdminOperation(sysno) ||
       IsAdvancedScheduler(sysno) ||
       IsAdvancedTimer(sysno) ||
       IsAsyncIo(sysno) ||
       IsDebug(sysno) ||
       IsEventFd(sysno) ||
       IsExtendedAttributes(sysno) ||
       IsFaNotify(sysno) ||
       IsFsControl(sysno) ||
       IsGlobalFSViewChange(sysno) ||
@@ skipping 11 matching lines @@
       IsSystemVMessageQueue(sysno) ||
       IsSystemVSemaphores(sysno) ||
       IsSystemVSharedMemory(sysno) ||
       IsTimer(sysno)) {
     return true;
   } else {
     return false;
   }
 }
 
+playground2::Sandbox::ErrorCode BaselinePolicy_x86_64(int sysno) {
+  if (IsBaselinePolicyAllowed_x86_64(sysno)) {
+    return playground2::Sandbox::SB_ALLOWED;
+  }
+  // TODO(jln): some system calls in those sets are not supposed to
+  // return ENOENT. Return the appropriate error.
+  if (IsFileSystemSyscall(sysno) || IsAmbientFileSystemSyscall(sysno)) {
+    return ENOENT;
+  }
+
+  if (IsDeniedFileSystemCapabilitySyscall(sysno)) {
+    return EPERM;
+  }
+
+  if (IsBaselinePolicyWatched_x86_64(sysno)) {
+    // Previously unseen syscalls. TODO(jln): some of these should
+    // be denied gracefully right away.
+    return playground2::Sandbox::ErrorCode(CrashSIGSYS_Handler, NULL);
+  }
+  // In any other case crash the program with our SIGSYS handler
+  return playground2::Sandbox::ErrorCode(CrashSIGSYS_Handler, NULL);
+}
+
 // x86_64 only because it references system calls that are multiplexed on IA32.
 playground2::Sandbox::ErrorCode GpuProcessPolicy_x86_64(int sysno) {
   switch(sysno) {
-    case __NR_getpid:  // Nvidia binary driver.
-    case __NR_getppid:  // ATI binary driver.
     case __NR_ioctl:
-    case __NR_mlock:
-    case __NR_munlock:
-    case __NR_poll:
-    case __NR_recvfrom:
-    case __NR_writev:
       return playground2::Sandbox::SB_ALLOWED;
     case __NR_socket:
       return EACCES;  // Nvidia binary driver.
-    case __NR_fchmod:
-      return EPERM;  // ATI binary driver.
     case __NR_open:
       // Accelerated video decode is enabled by default only on Chrome OS.
       if (IsAcceleratedVideoDecodeEnabled()) {
         // Accelerated video decode needs to open /dev/dri/card0, and
         // dup()'ing an already open file descriptor does not work.
         // Allow open() even though it severely weakens the sandbox,
         // to test the sandboxing mechanism in general.
         // TODO(jorgelo): remove this once we solve the libva issue.
         return playground2::Sandbox::SB_ALLOWED;
       } else {
         // Hook open() in the GPU process to allow opening /etc/drirc,
         // needed by Mesa.
         // The hook needs dup(), lseek(), and close() to be allowed.
         return playground2::Sandbox::ErrorCode(GpuOpenSIGSYS_Handler, NULL);
       }
     default:
-      if (IsGpuAndFlashPolicyAllowed_x86_64(sysno) || IsEventFd(sysno)) {
+      if (IsEventFd(sysno))
         return playground2::Sandbox::SB_ALLOWED;
-      }
-      // Generally, filename-based syscalls will fail with ENOENT to behave
-      // similarly to a possible future setuid sandbox.
-      if (IsFileSystemSyscall(sysno) || IsAmbientFileSystemSyscall(sysno)) {
-        return ENOENT;
-      }
 
-      if (IsGpuAndFlashPolicyWatched_x86_64(sysno)) {
-          // Previously unseen syscalls. TODO(jln): some of these should
-          // be denied gracefully right away.
-          return playground2::Sandbox::ErrorCode(CrashSIGSYS_Handler, NULL);
-      }
-      // In any other case crash the program with our SIGSYS handler
-      return playground2::Sandbox::ErrorCode(CrashSIGSYS_Handler, NULL);
+      // Default on the baseline policy.
+      return BaselinePolicy_x86_64(sysno);
   }
 }
 
 // x86_64 only because it references system calls that are multiplexed on IA32.
 playground2::Sandbox::ErrorCode FlashProcessPolicy_x86_64(int sysno) {
   switch (sysno) {
     case __NR_sched_getaffinity:
     case __NR_sched_setscheduler:
-    // These are under investigation, and hopefully not here for the long term.
     case __NR_times:
-    case __NR_wait4:
       return playground2::Sandbox::SB_ALLOWED;
     case __NR_ioctl:
       return ENOTTY;  // Flash Access.
     case __NR_socket:
       return EACCES;
     default:
-      if (IsGpuAndFlashPolicyAllowed_x86_64(sysno) ||
-          IsSystemVSharedMemory(sysno)) {
+      // These are under investigation, and hopefully not here for the long
+      // term.
+      if (IsSystemVSharedMemory(sysno))
         return playground2::Sandbox::SB_ALLOWED;
-      }
-      if (IsFileSystemSyscall(sysno) || IsAmbientFileSystemSyscall(sysno)) {
-        return ENOENT;
-      }
-      if (IsGpuAndFlashPolicyWatched_x86_64(sysno)) {
-          // Previously unseen syscalls. TODO(jln): some of these should
-          // be denied gracefully right away.
-          return playground2::Sandbox::ErrorCode(CrashSIGSYS_Handler, NULL);
-      }
-      // In any other case crash the program with our SIGSYS handler.
-      return playground2::Sandbox::ErrorCode(CrashSIGSYS_Handler, NULL);
+
+      // Default on the baseline policy.
+      return  BaselinePolicy_x86_64(sysno);
   }
 }
 #endif
 
 playground2::Sandbox::ErrorCode BlacklistPtracePolicy(int sysno) {
   if (sysno < static_cast<int>(MIN_SYSCALL) ||
       sysno > static_cast<int>(MAX_SYSCALL)) {
     // TODO(jln) we should not have to do that in a trivial policy.
     return ENOSYS;
   }
@@ skipping 135 matching lines @@
       // Process-specific policy.
       ShouldEnableSeccompBpf(process_type) &&
       SupportsSandbox()) {
     return StartBpfSandbox_x86(command_line, process_type);
   }
 #endif
   return false;
 }
 
 }  // namespace content