Bug 1372428 - Extend file pre-opening for sandboxed media plugins. r=gcp

MozReview-Commit-ID: JoyYocxnk94

security/sandbox/linux/Sandbox.cpp

@@ -13,6 +13,9 @@
 #include "SandboxFilter.h"
 #include "SandboxInternal.h"
 #include "SandboxLogging.h"
+#ifdef MOZ_GMP_SANDBOX
+#include "SandboxOpenedFiles.h"
+#endif
 #include "SandboxReporterClient.h"
 #include "SandboxUtil.h"
 
@@ -34,9 +37,12 @@
 #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"
@@ -82,13 +88,6 @@ 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*);
@@ -713,26 +712,32 @@ SetContentProcessSandbox(int aBrokerFd, std::vector<int>& aSyscallWhitelist)
 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
 

security/sandbox/linux/SandboxFilter.cpp

@@ -11,6 +11,9 @@
 #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"
 
@@ -895,7 +898,7 @@ 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;
 
@@ -916,20 +919,15 @@ class GMPSandboxPolicy : public SandboxPolicyCommon {
       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;
   }
@@ -979,13 +977,11 @@ class GMPSandboxPolicy : public SandboxPolicyCommon {
     }
   }
 
-  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;
 
@@ -996,7 +992,7 @@ public:
     case __NR_open:
 #endif
     case __NR_openat:
-      return Trap(OpenTrap, mPlugin);
+      return Trap(OpenTrap, mFiles);
 
       // ipc::Shmem
     case __NR_mprotect:
@@ -1035,6 +1031,9 @@ public:
     CASES_FOR_fcntl:
       return Trap(FcntlTrap, nullptr);
 
+    case __NR_dup:
+      return Allow();
+
     default:
       return SandboxPolicyCommon::EvaluateSyscall(sysno);
     }
@@ -1042,11 +1041,11 @@ public:
 };
 
 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

security/sandbox/linux/SandboxFilter.h

@@ -9,6 +9,7 @@
 
 #include <vector>
 #include "mozilla/Atomics.h"
+#include "mozilla/Range.h"
 #include "mozilla/UniquePtr.h"
 
 namespace sandbox {
@@ -27,12 +28,10 @@ UniquePtr<sandbox::bpf_dsl::Policy> GetContentSandboxPolicy(SandboxBrokerClient*
 #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

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

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

security/sandbox/linux/moz.build

@@ -72,6 +72,11 @@ SOURCES += [
     '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.