Linux sandbox: more paranoid checks

content/common/sandbox_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 <fcntl.h>
 #include <sys/resource.h>
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/types.h>
 
 #include <limits>
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/time.h"
 #include "content/common/sandbox_linux.h"
 #include "content/common/sandbox_seccomp_bpf_linux.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/sandbox_linux.h"
@@ skipping 50 matching lines @@
   LinuxSandbox* instance = Singleton<LinuxSandbox>::get();
   CHECK(instance);
   return instance;
 }
 
 #if defined(ADDRESS_SANITIZER) && defined(OS_LINUX)
 // ASan API call to notify the tool the sandbox is going to be turned on.
 extern "C" void __sanitizer_sandbox_on_notify(void *reserved);
 #endif
 
-void LinuxSandbox::PreinitializeSandboxBegin() {
+void LinuxSandbox::PreinitializeSandbox() {
   CHECK(!pre_initialized_);
   seccomp_bpf_supported_ = false;
 #if defined(ADDRESS_SANITIZER) && defined(OS_LINUX)
   // ASan needs to open some resources before the sandbox is enabled.
   // This should not fork, not launch threads, not open a directory.
   __sanitizer_sandbox_on_notify(/*reserved*/NULL);
 #endif
+
+#if !defined(NDEBUG)
+  // Open proc_fd_ only in Debug mode so that forgetting to close it doesn't
+  // produce a sandbox escape in Release mode.
+  proc_fd_ = open("/proc", O_DIRECTORY | O_RDONLY);
+  CHECK(proc_fd_ >= 0);
+#endif  // !defined(NDEBUG)
   // We "pre-warm" the code that detects supports for seccomp BPF.
-  // TODO(jln): Use proc_fd_ here once we're comfortable it does not create
-  // an additional security risk.
   if (SandboxSeccompBpf::IsSeccompBpfDesired()) {
     if (!SandboxSeccompBpf::SupportsSandbox()) {
       VLOG(1) << "Lacking support for seccomp-bpf sandbox.";
     } else {
       seccomp_bpf_supported_ = true;
     }
   }
   pre_initialized_ = true;
 }
 
-// Once we finally know our process type, we can cleanup proc_fd_.
-void LinuxSandbox::PreinitializeSandboxFinish(
-    const std::string& process_type) {
-  CHECK(pre_initialized_);
-  // TODO(jln): move this to InitializeSandbox.
-  if (proc_fd_ >= 0) {
-    CHECK_EQ(HANDLE_EINTR(close(proc_fd_)), 0);
-    proc_fd_ = -1;
-  }
-}
-
-void LinuxSandbox::PreinitializeSandbox(const std::string& process_type) {
-  PreinitializeSandboxBegin();
-  PreinitializeSandboxFinish(process_type);
-}
-
 bool LinuxSandbox::InitializeSandbox() {
   bool seccomp_bpf_started = false;
   LinuxSandbox* linux_sandbox = LinuxSandbox::GetInstance();
+  // We need to make absolutely sure that our sandbox is "sealed" before
+  // InitializeSandbox does exit.
+  base::ScopedClosureRunner sandbox_sealer(
+      base::Bind(&LinuxSandbox::SealSandbox, base::Unretained(linux_sandbox)));
   const std::string process_type =
       CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
           switches::kProcessType);
 
   // No matter what, it's always an error to call InitializeSandbox() after
   // threads have been created.
   if (!linux_sandbox->IsSingleThreaded()) {
     std::string error_message = "InitializeSandbox() called with multiple "
                                 "threads in process " + process_type;
-    // TODO(jln): change this into a CHECK() once we are more comfortable it
-    // does not trigger.
+    // The GPU process is allowed to call InitializeSandbox() with threads for
+    // now, because it loads third party libraries.
+    if (process_type != switches::kGpuProcess)
+      DCHECK(false) << error_message;
     LOG(ERROR) << error_message;
     return false;
   }
 
   // Attempt to limit the future size of the address space of the process.
   linux_sandbox->LimitAddressSpace(process_type);
 
   // First, try to enable seccomp-bpf.
   seccomp_bpf_started = linux_sandbox->StartSeccompBpf(process_type);
 
@@ skipping 14 matching lines @@
   if (seccomp_bpf_supported() &&
       SandboxSeccompBpf::ShouldEnableSeccompBpf(switches::kRendererProcess)) {
     // We report whether the sandbox will be activated when renderers go
     // through sandbox initialization.
     sandbox_flags |= kSandboxLinuxSeccompBpf;
   }
 
   return sandbox_flags;
 }
 
+// Threads are counted via /proc/self/task. This is a little hairy because of
+// PID namespaces and existing sandboxes, so "self" must really be used instead
+// of using the pid.
 bool LinuxSandbox::IsSingleThreaded() const {
-  // TODO(jln): re-implement this properly and use our proc_fd_ if available.
-  // Possibly racy, but it's ok because this is more of a debug check to catch
-  // new threaded situations arising during development.
-  file_util::FileEnumerator en(base::FilePath("/proc/self/task"), false,
-      file_util::FileEnumerator::FILES |
-      file_util::FileEnumerator::DIRECTORIES);
-  bool found_file = false;
-  while (!en.Next().empty()) {
-    if (found_file)
-      return false;  // Found a second match.
-    found_file = true;
+  struct stat task_stat;
+  int fstat_ret;
+  if (proc_fd_ >= 0) {
+    // If a handle to /proc is available, use it. This allows to bypass file
+    // system restrictions.
+    fstat_ret = fstatat(proc_fd_, "self/task/", &task_stat, 0);
+  } else {
+    // Otherwise, make an attempt to access the file system directly.
+    fstat_ret = fstatat(AT_FDCWD, "/proc/self/task/", &task_stat, 0);
+  }
+  // In Debug mode, it's mandatory to be able to count threads to catch bugs.
+#if !defined(NDEBUG)
+  // Using DCHECK here would be incorrect. DCHECK can be enabled in non
+  // official release mode.
+  CHECK_EQ(0, fstat_ret) << "Could not count threads, the sandbox was not "
+                         << "pre-initialized properly.";
+#endif  // !defined(NDEBUG)
+  if (fstat_ret) {
+    // Pretend to be monothreaded if it can't be determined (for instance the
+    // setuid sandbox is already engaged but no proc_fd_ is available).
+    return true;
   }
 
-  // We pass the test if we found 0 files becase the setuid sandbox will
-  // prevent /proc access in some contexts.
-  return true;
+  // At least "..", "." and the current thread should be present.
+  CHECK_LE(3UL, task_stat.st_nlink);
+  // Counting threads via /proc/self/task could be racy. For the purpose of
+  // determining if the current proces is monothreaded it works: if at any
+  // time it becomes monothreaded, it'll stay so.
+  return task_stat.st_nlink == 3;
 }
 
 bool LinuxSandbox::seccomp_bpf_started() const {
   return seccomp_bpf_started_;
 }
 
 sandbox::SetuidSandboxClient*
     LinuxSandbox::setuid_sandbox_client() const {
   return setuid_sandbox_client_.get();
 }
 
 // For seccomp-bpf, we use the SandboxSeccompBpf class.
 bool LinuxSandbox::StartSeccompBpf(const std::string& process_type) {
   CHECK(!seccomp_bpf_started_);
   if (!pre_initialized_)
-    PreinitializeSandbox(process_type);
+    PreinitializeSandbox();
   if (seccomp_bpf_supported())
     seccomp_bpf_started_ = SandboxSeccompBpf::StartSandbox(process_type);
 
   if (seccomp_bpf_started_)
     LogSandboxStarted("seccomp-bpf");
 
   return seccomp_bpf_started_;
 }
 
 bool LinuxSandbox::seccomp_bpf_supported() const {
@@ skipping 31 matching lines @@
   const rlim_t kNewDataSegmentMaxSize = std::numeric_limits<int>::max();
 
   bool limited_as = AddResourceLimit(RLIMIT_AS, address_space_limit);
   bool limited_data = AddResourceLimit(RLIMIT_DATA, kNewDataSegmentMaxSize);
   return limited_as && limited_data;
 #else
   return false;
 #endif  // !defined(ADDRESS_SANITIZER)
 }
 
+void LinuxSandbox::SealSandbox() {
+  if (proc_fd_ >= 0) {

[palmer, 2013/04/30 21:21:59]

Using proc_fd_ < 0 as a signal to express whether or not the descriptor is
closed is a bit shaky to me. The single source of truth about a descriptor's
state is in the kernel; I would just unconditionally close(proc_fd_) and then
CHECK(0 == ret || EBADF == errno). (I.e. we are happy to tolerate failure due to
EBADF — descriptor already closed — but not other failures.)

[jln (very slow on Chromium), 2013/04/30 22:09:51]

No, this would not be correct. We can't close an arbitrary file descriptor, it
could have been re-allocated to anything else.

-1 is guaranteed to never be a valid descriptor, moreover we only close in one
place (SealSandbox()), so it's easy to guarantee that a closed fd will be set to
-1. In addition, even if there was a bug around this, the CHECK would probably
catch it at some point.

(Also it's the common pattern that we use everywhere in Linux-related code)

+    int ret = HANDLE_EINTR(close(proc_fd_));
+    CHECK_EQ(0, ret);
+    proc_fd_ = -1;
+  }
+}
+
 }  // namespace content