Bug 1372428 - Extend file pre-opening for sandboxed media plugins. r=gcp
authorJed Davis <jld@mozilla.com>
Fri, 07 Jul 2017 08:58:50 -0600
changeset 416498 52e1b27c1cb085997440183cf28686c36f5591b3
parent 416497 9d96ca099f2106d59bf65c0e0b77d0422f2dd51b
child 416499 f07caa23cdbbd19408002a0a0e39866cfee5898c
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1372428
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1372428 - Extend file pre-opening for sandboxed media plugins. r=gcp MozReview-Commit-ID: JoyYocxnk94
security/sandbox/linux/Sandbox.cpp
security/sandbox/linux/SandboxFilter.cpp
security/sandbox/linux/SandboxFilter.h
security/sandbox/linux/SandboxOpenedFiles.cpp
security/sandbox/linux/SandboxOpenedFiles.h
security/sandbox/linux/moz.build
--- a/security/sandbox/linux/Sandbox.cpp
+++ b/security/sandbox/linux/Sandbox.cpp
@@ -8,16 +8,19 @@
 
 #include "LinuxCapabilities.h"
 #include "LinuxSched.h"
 #include "SandboxBrokerClient.h"
 #include "SandboxChroot.h"
 #include "SandboxFilter.h"
 #include "SandboxInternal.h"
 #include "SandboxLogging.h"
+#ifdef MOZ_GMP_SANDBOX
+#include "SandboxOpenedFiles.h"
+#endif
 #include "SandboxReporterClient.h"
 #include "SandboxUtil.h"
 
 #include <dirent.h>
 #ifdef NIGHTLY_BUILD
 #include "dlfcn.h"
 #endif
 #include <errno.h>
@@ -29,19 +32,22 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/prctl.h>
 #include <sys/ptrace.h>
 #include <sys/syscall.h>
 #include <sys/time.h>
 #include <unistd.h>
 
+#include "mozilla/Array.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/Maybe.h"
+#include "mozilla/Range.h"
 #include "mozilla/SandboxInfo.h"
+#include "mozilla/Span.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Unused.h"
 #include "sandbox/linux/bpf_dsl/codegen.h"
 #include "sandbox/linux/bpf_dsl/dump_bpf.h"
 #include "sandbox/linux/bpf_dsl/policy.h"
 #include "sandbox/linux/bpf_dsl/policy_compiler.h"
 #include "sandbox/linux/bpf_dsl/seccomp_macros.h"
 #include "sandbox/linux/seccomp-bpf/trap.h"
@@ -77,23 +83,16 @@ int gSeccompTsyncBroadcastSignum = 0;
 
 namespace mozilla {
 
 static bool gSandboxCrashOnError = false;
 
 // This is initialized by SandboxSetCrashFunc().
 SandboxCrashFunc gSandboxCrashFunc;
 
-#ifdef MOZ_GMP_SANDBOX
-// For media plugins, we can start the sandbox before we dlopen the
-// module, so we have to pre-open the file and simulate the sandboxed
-// open().
-static SandboxOpenedFile gMediaPluginFile;
-#endif
-
 static Maybe<SandboxReporterClient> gSandboxReporterClient;
 static UniquePtr<SandboxChroot> gChrootHelper;
 static void (*gChromiumSigSysHandler)(int, siginfo_t*, void*);
 
 // Test whether a ucontext, interpreted as the state after a syscall,
 // indicates the given error.  See also sandbox::Syscall::PutValueInUcontext.
 static bool
 ContextIsError(const ucontext_t *aContext, int aError)
@@ -708,32 +707,38 @@ SetContentProcessSandbox(int aBrokerFd, 
  * read-only, once, after the sandbox starts; it should be the .so
  * file implementing the not-yet-loaded plugin.
  *
  * Will normally make the process exit on failure.
 */
 void
 SetMediaPluginSandbox(const char *aFilePath)
 {
+  MOZ_RELEASE_ASSERT(aFilePath != nullptr);
   if (!SandboxInfo::Get().Test(SandboxInfo::kEnabledForMedia)) {
     return;
   }
 
   gSandboxReporterClient.emplace(SandboxReport::ProcType::MEDIA_PLUGIN);
 
-  MOZ_ASSERT(!gMediaPluginFile.mPath);
-  if (aFilePath) {
-    gMediaPluginFile.mPath = strdup(aFilePath);
-    gMediaPluginFile.mFd = open(aFilePath, O_RDONLY | O_CLOEXEC);
-    if (gMediaPluginFile.mFd == -1) {
-      SANDBOX_LOG_ERROR("failed to open plugin file %s: %s",
-                        aFilePath, strerror(errno));
-      MOZ_CRASH();
-    }
-  } else {
-    gMediaPluginFile.mFd = -1;
+  SandboxOpenedFile plugin(aFilePath);
+  if (!plugin.IsOpen()) {
+    SANDBOX_LOG_ERROR("failed to open plugin file %s: %s",
+                      aFilePath, strerror(errno));
+    MOZ_CRASH();
   }
+
+  auto files = new SandboxOpenedFiles();
+  files->Add(Move(plugin));
+  files->Add("/dev/urandom", true);
+  files->Add("/sys/devices/system/cpu/cpu0/tsc_freq_khz");
+  files->Add("/sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq");
+  files->Add("/proc/cpuinfo"); // Info also available via CPUID instruction.
+#ifdef __i386__
+  files->Add("/proc/self/auxv"); // Info also in process's address space.
+#endif
+
   // Finally, start the sandbox.
-  SetCurrentProcessSandbox(GetMediaSandboxPolicy(&gMediaPluginFile));
+  SetCurrentProcessSandbox(GetMediaSandboxPolicy(files));
 }
 #endif // MOZ_GMP_SANDBOX
 
 } // namespace mozilla
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -6,16 +6,19 @@
 
 #include "SandboxFilter.h"
 #include "SandboxFilterUtil.h"
 
 #include "SandboxBrokerClient.h"
 #include "SandboxInfo.h"
 #include "SandboxInternal.h"
 #include "SandboxLogging.h"
+#ifdef MOZ_GMP_SANDBOX
+#include "SandboxOpenedFiles.h"
+#endif
 #include "mozilla/PodOperations.h"
 #include "mozilla/UniquePtr.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/ipc.h>
 #include <linux/net.h>
 #include <linux/prctl.h>
@@ -890,17 +893,17 @@ GetContentSandboxPolicy(SandboxBrokerCli
 // to be an effective sandbox by itself, because we allow GMP on Linux
 // systems where that's the only sandboxing mechanism we can use.
 //
 // Be especially careful about what this policy allows.
 class GMPSandboxPolicy : public SandboxPolicyCommon {
   static intptr_t OpenTrap(const sandbox::arch_seccomp_data& aArgs,
                            void* aux)
   {
-    auto plugin = static_cast<SandboxOpenedFile*>(aux);
+    const auto files = static_cast<const SandboxOpenedFiles*>(aux);
     const char* path;
     int flags;
 
     switch (aArgs.nr) {
 #ifdef __NR_open
     case __NR_open:
       path = reinterpret_cast<const char*>(aArgs.args[0]);
       flags = static_cast<int>(aArgs.args[1]);
@@ -911,30 +914,25 @@ class GMPSandboxPolicy : public SandboxP
       // assertion in ctor) so the dirfd argument is ignored.
       path = reinterpret_cast<const char*>(aArgs.args[1]);
       flags = static_cast<int>(aArgs.args[2]);
       break;
     default:
       MOZ_CRASH("unexpected syscall number");
     }
 
-    if (strcmp(path, plugin->mPath) != 0) {
-      SANDBOX_LOG_ERROR("attempt to open file %s (flags=0%o) which is not the"
-                        " media plugin %s", path, flags, plugin->mPath);
-      return -EPERM;
-    }
     if ((flags & O_ACCMODE) != O_RDONLY) {
       SANDBOX_LOG_ERROR("non-read-only open of file %s attempted (flags=0%o)",
                         path, flags);
-      return -EPERM;
+      return -EROFS;
     }
-    int fd = plugin->mFd.exchange(-1);
+    int fd = files->GetDesc(path);
     if (fd < 0) {
-      SANDBOX_LOG_ERROR("multiple opens of media plugin file unimplemented");
-      return -ENOSYS;
+      // SandboxOpenedFile::GetDesc already logged about this, if appropriate.
+      return -ENOENT;
     }
     return fd;
   }
 
   static intptr_t SchedTrap(const sandbox::arch_seccomp_data& aArgs,
                             void* aux)
   {
     const pid_t tid = syscall(__NR_gettid);
@@ -974,34 +972,32 @@ class GMPSandboxPolicy : public SandboxP
       return O_CLOEXEC;
     case F_SETFD:
       return 0;
     default:
       return -ENOSYS;
     }
   }
 
-  SandboxOpenedFile* mPlugin;
+  const SandboxOpenedFiles* mFiles;
 public:
-  explicit GMPSandboxPolicy(SandboxOpenedFile* aPlugin)
-  : mPlugin(aPlugin)
-  {
-    MOZ_ASSERT(aPlugin->mPath[0] == '/', "plugin path should be absolute");
-  }
+  explicit GMPSandboxPolicy(const SandboxOpenedFiles* aFiles)
+  : mFiles(aFiles)
+  { }
 
   ~GMPSandboxPolicy() override = default;
 
   ResultExpr EvaluateSyscall(int sysno) const override {
     switch (sysno) {
       // Simulate opening the plugin file.
 #ifdef __NR_open
     case __NR_open:
 #endif
     case __NR_openat:
-      return Trap(OpenTrap, mPlugin);
+      return Trap(OpenTrap, mFiles);
 
       // ipc::Shmem
     case __NR_mprotect:
       return Allow();
     case __NR_madvise: {
       Arg<int> advice(2);
       return If(advice == MADV_DONTNEED, Allow())
         .ElseIf(advice == MADV_FREE, Allow())
@@ -1030,23 +1026,26 @@ public:
       return Allow();
 
     // Bug 1372428
     case __NR_uname:
       return Trap(UnameTrap, nullptr);
     CASES_FOR_fcntl:
       return Trap(FcntlTrap, nullptr);
 
+    case __NR_dup:
+      return Allow();
+
     default:
       return SandboxPolicyCommon::EvaluateSyscall(sysno);
     }
   }
 };
 
 UniquePtr<sandbox::bpf_dsl::Policy>
-GetMediaSandboxPolicy(SandboxOpenedFile* aPlugin)
+GetMediaSandboxPolicy(const SandboxOpenedFiles* aFiles)
 {
-  return UniquePtr<sandbox::bpf_dsl::Policy>(new GMPSandboxPolicy(aPlugin));
+  return UniquePtr<sandbox::bpf_dsl::Policy>(new GMPSandboxPolicy(aFiles));
 }
 
 #endif // MOZ_GMP_SANDBOX
 
-}
+} // namespace mozilla
--- a/security/sandbox/linux/SandboxFilter.h
+++ b/security/sandbox/linux/SandboxFilter.h
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_SandboxFilter_h
 #define mozilla_SandboxFilter_h
 
 #include <vector>
 #include "mozilla/Atomics.h"
+#include "mozilla/Range.h"
 #include "mozilla/UniquePtr.h"
 
 namespace sandbox {
 namespace bpf_dsl {
 class Policy;
 }
 }
 
@@ -22,19 +23,17 @@ namespace mozilla {
 #ifdef MOZ_CONTENT_SANDBOX
 class SandboxBrokerClient;
 
 UniquePtr<sandbox::bpf_dsl::Policy> GetContentSandboxPolicy(SandboxBrokerClient* aMaybeBroker,
                                                             const std::vector<int>& aSyscallWhitelist);
 #endif
 
 #ifdef MOZ_GMP_SANDBOX
-struct SandboxOpenedFile {
-  const char *mPath;
-  Atomic<int> mFd;
-};
+class SandboxOpenedFiles;
 
-UniquePtr<sandbox::bpf_dsl::Policy> GetMediaSandboxPolicy(SandboxOpenedFile* aPlugin);
+// The SandboxOpenedFiles object must live until the process exits.
+UniquePtr<sandbox::bpf_dsl::Policy> GetMediaSandboxPolicy(const SandboxOpenedFiles* aFiles);
 #endif
 
 } // namespace mozilla
 
 #endif
new file mode 100644
--- /dev/null
+++ b/security/sandbox/linux/SandboxOpenedFiles.cpp
@@ -0,0 +1,79 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "SandboxOpenedFiles.h"
+
+#include "mozilla/Move.h"
+#include "SandboxLogging.h"
+
+#include <fcntl.h>
+#include <unistd.h>
+
+namespace mozilla {
+
+// The default move constructor almost works, but Atomic isn't
+// move-constructable and the fd needs some special handling.
+SandboxOpenedFile::SandboxOpenedFile(SandboxOpenedFile&& aMoved)
+: mPath(Move(aMoved.mPath))
+, mMaybeFd(aMoved.TakeDesc())
+, mDup(aMoved.mDup)
+, mExpectError(aMoved.mExpectError)
+{ }
+
+SandboxOpenedFile::SandboxOpenedFile(const char* aPath, bool aDup)
+  : mPath(aPath), mDup(aDup), mExpectError(false)
+{
+  MOZ_ASSERT(aPath[0] == '/', "path should be absolute");
+
+  int fd = open(aPath, O_RDONLY | O_CLOEXEC);
+  if (fd < 0) {
+    mExpectError = true;
+  }
+  mMaybeFd = fd;
+}
+
+int
+SandboxOpenedFile::GetDesc() const
+{
+  int fd = -1;
+  if (mDup) {
+    fd = mMaybeFd;
+    if (fd >= 0) {
+      fd = dup(fd);
+      if (fd < 0) {
+        SANDBOX_LOG_ERROR("dup: %s", strerror(errno));
+      }
+    }
+  } else {
+    fd = TakeDesc();
+  }
+  if (fd < 0 && !mExpectError) {
+    SANDBOX_LOG_ERROR("unexpected multiple open of file %s", Path());
+  }
+  return fd;
+}
+
+SandboxOpenedFile::~SandboxOpenedFile()
+{
+  int fd = TakeDesc();
+  if (fd >= 0) {
+    close(fd);
+  }
+}
+
+int
+SandboxOpenedFiles::GetDesc(const char* aPath) const
+{
+  for (const auto& file : mFiles) {
+    if (strcmp(file.Path(), aPath) == 0) {
+      return file.GetDesc();
+    }
+  }
+  SANDBOX_LOG_ERROR("attempt to open unexpected file %s", aPath);
+  return -1;
+}
+
+} // namespace mozilla
new file mode 100644
--- /dev/null
+++ b/security/sandbox/linux/SandboxOpenedFiles.h
@@ -0,0 +1,89 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_SandboxOpenedFiles_h
+#define mozilla_SandboxOpenedFiles_h
+
+#include "mozilla/Atomics.h"
+#include "mozilla/Range.h"
+#include "mozilla/UniquePtr.h"
+
+#include <vector>
+#include <string>
+
+// The use of C++ standard library containers here should be safe; the
+// standard (section container.requirements.dataraces) requires that
+// using const methods/pointers not introduce data races (e.g., from
+// interior mutability or global state).
+//
+// Reentrancy isn't guaranteed, and the library could use async signal
+// unsafe mutexes for "read-only" operations, but I'm assuming that that's
+// not the case at least for simple containers like string and vector.
+
+namespace mozilla {
+
+// This class represents a file that's been pre-opened for a media
+// plugin.  It can be move-constructed but not copied.
+class SandboxOpenedFile final {
+ public:
+  // This constructor opens the named file and saves the descriptor.
+  // If the open fails, IsOpen() will return false and GetDesc() will
+  // quietly return -1.  If aDup is true, GetDesc() will return a
+  // dup() of the descriptor every time it's called; otherwise, the
+  // first call will return the descriptor and any further calls will
+  // log an error message and return -1.
+  explicit SandboxOpenedFile(const char* aPath, bool aDup = false);
+
+  // Simulates opening the pre-opened file; see the constructor's
+  // comment for details.  Does not set errno on error, but may modify
+  // it as a side-effect.  Thread-safe and intended to be async signal safe.
+  int GetDesc() const;
+
+  const char* Path() const { return mPath.c_str(); }
+
+  bool IsOpen() const { return mMaybeFd >= 0; }
+
+  ~SandboxOpenedFile();
+
+  MOZ_IMPLICIT SandboxOpenedFile(SandboxOpenedFile&& aMoved);
+
+ private:
+  std::string mPath;
+  mutable Atomic<int> mMaybeFd;
+  bool mDup;
+  bool mExpectError;
+
+  int TakeDesc() const { return mMaybeFd.exchange(-1); }
+};
+
+// This class represents a collection of files to be used to handle
+// open() calls from the media plugin (and the dynamic loader).
+// Because the seccomp-bpf policy exists until the process exits, this
+// object must not be destroyed after the syscall filter is installed.
+class SandboxOpenedFiles {
+public:
+  SandboxOpenedFiles() = default;
+
+  template<typename... Args>
+  void Add(Args&&... aArgs) {
+    mFiles.emplace_back(Forward<Args>(aArgs)...);
+  }
+
+  int GetDesc(const char* aPath) const;
+
+private:
+  std::vector<SandboxOpenedFile> mFiles;
+
+  // We could allow destroying instances of this class that aren't
+  // used with seccomp-bpf (e.g., for unit testing) by having the
+  // destructor check a flag set by the syscall policy and crash,
+  // but let's not write that code until we actually need it.
+  ~SandboxOpenedFiles() = delete;
+};
+
+} // namespace mozilla
+
+#endif // mozilla_SandboxOpenedFiles_h
--- a/security/sandbox/linux/moz.build
+++ b/security/sandbox/linux/moz.build
@@ -67,16 +67,21 @@ SOURCES += [
     'SandboxFilterUtil.cpp',
     'SandboxHooks.cpp',
     'SandboxInfo.cpp',
     'SandboxLogging.cpp',
     'SandboxReporterClient.cpp',
     'SandboxUtil.cpp',
 ]
 
+if CONFIG['MOZ_GMP_SANDBOX']:
+    SOURCES += [
+        'SandboxOpenedFiles.cpp',
+    ]
+
 # This copy of SafeSPrintf doesn't need to avoid the Chromium logging
 # dependency like the one in libxul does, but this way the behavior is
 # consistent.  See also the comment in SandboxLogging.h.
 SOURCES['../chromium/base/strings/safe_sprintf.cc'].flags += ['-DNDEBUG']
 
 # Keep clang from warning about intentional 'switch' fallthrough in icu_utf.cc:
 if CONFIG['CLANG_CXX']:
     SOURCES['../chromium/base/third_party/icu/icu_utf.cc'].flags += ['-Wno-implicit-fallthrough']