Bug 1126437 - Reorganize content sandbox params extracted from libxul APIs. r=gcp

The end goal is to allow the seccomp-bpf policy to vary based on the
content sandbox level.

Rather than add yet another parameter to SetContentProcessSandbox to
pass down the sandbox level, this collects the values that have to be
computed in libxul into a struct, and moves the code that computes it so
it's not cluttering up ContentChild.

MozReview-Commit-ID: L0dyQwHQKhc

dom/ipc/ContentChild.cpp

@@ -1666,39 +1666,14 @@ ContentChild::RecvSetProcessSandbox(const MaybeFileDesc& aBroker)
 #if defined(MOZ_CONTENT_SANDBOX)
   bool sandboxEnabled = true;
 #if defined(XP_LINUX)
-  // Otherwise, sandboxing is best-effort.
+  // On Linux, we have to support systems that can't use any sandboxing.
   if (!SandboxInfo::Get().CanSandboxContent()) {
     sandboxEnabled = false;
   }
 
   if (sandboxEnabled) {
-    int brokerFd = -1;
+    sandboxEnabled =
-    if (aBroker.type() == MaybeFileDesc::TFileDescriptor) {
+      SetContentProcessSandbox(ContentProcessSandboxParams::ForThisProcess(aBroker));
-      auto fd = aBroker.get_FileDescriptor().ClonePlatformHandle();
-      brokerFd = fd.release();
-      // brokerFd < 0 means to allow direct filesystem access, so
-      // make absolutely sure that doesn't happen if the parent
-      // didn't intend it.
-      MOZ_RELEASE_ASSERT(brokerFd >= 0);
-    }
-    // Allow user overrides of seccomp-bpf syscall filtering
-    std::vector<int> syscallWhitelist;
-    nsAutoCString extraSyscalls;
-    nsresult rv =
-      Preferences::GetCString("security.sandbox.content.syscall_whitelist",
-                              extraSyscalls);
-    if (NS_SUCCEEDED(rv)) {
-      for (const nsACString& callNrString : extraSyscalls.Split(',')) {
-        int callNr = PromiseFlatCString(callNrString).ToInteger(&rv);
-        if (NS_SUCCEEDED(rv)) {
-          syscallWhitelist.push_back(callNr);
-        }
-      }
-    }
-    ContentChild* cc = ContentChild::GetSingleton();
-    bool isFileProcess = cc->GetRemoteType().EqualsLiteral(FILE_REMOTE_TYPE);
-    sandboxEnabled = SetContentProcessSandbox(brokerFd, isFileProcess,
-                                              syscallWhitelist);
   }
 #elif defined(XP_WIN)
   mozilla::SandboxTarget::Instance()->StartSandbox();

security/sandbox/linux/Sandbox.cpp

@@ -597,29 +597,30 @@ SetCurrentProcessSandbox(UniquePtr<sandbox::bpf_dsl::Policy> aPolicy)
  * Will normally make the process exit on failure.
 */
 bool
-SetContentProcessSandbox(int aBrokerFd, bool aFileProcess,
+SetContentProcessSandbox(ContentProcessSandboxParams&& aParams)
-                         std::vector<int>& aSyscallWhitelist)
 {
+  int brokerFd = aParams.mBrokerFd;
+  aParams.mBrokerFd = -1;
+
   if (!SandboxInfo::Get().Test(SandboxInfo::kEnabledForContent)) {
-    if (aBrokerFd >= 0) {
+    if (brokerFd >= 0) {
-      close(aBrokerFd);
+      close(brokerFd);
     }
     return false;
   }
 
-  auto procType = aFileProcess
+  auto procType = aParams.mFileProcess
     ? SandboxReport::ProcType::FILE
     : SandboxReport::ProcType::CONTENT;
   gSandboxReporterClient = new SandboxReporterClient(procType);
 
   // This needs to live until the process exits.
   static SandboxBrokerClient* sBroker;
-  if (aBrokerFd >= 0) {
+  if (brokerFd >= 0) {
-    sBroker = new SandboxBrokerClient(aBrokerFd);
+    sBroker = new SandboxBrokerClient(brokerFd);
   }
 
-  SetCurrentProcessSandbox(GetContentSandboxPolicy(sBroker,
+  SetCurrentProcessSandbox(GetContentSandboxPolicy(sBroker, Move(aParams)));
-                                                   aSyscallWhitelist));
   return true;
 }
 #endif // MOZ_CONTENT_SANDBOX

security/sandbox/linux/Sandbox.h

@@ -17,19 +17,41 @@
 
 namespace mozilla {
 
+namespace dom {
+class MaybeFileDesc;
+} // namespace dom
+
 // This must be called early, before glib creates any worker threads.
 // (See bug 1176099.)
 MOZ_EXPORT void SandboxEarlyInit();
 
 #ifdef MOZ_CONTENT_SANDBOX
+// A collection of sandbox parameters that have to be extracted from
+// prefs or other libxul facilities and passed down, because
+// libmozsandbox can't link against the APIs to read them.
+struct ContentProcessSandboxParams {
+  // Content sandbox level; see also GetEffectiveSandboxLevel in
+  // SandboxSettings.h and the comments for the Linux version of
+  // "security.sandbox.content.level" in browser/app/profile/firefox.js
+  int mLevel = 0;
+  // The filesystem broker client file descriptor, or -1 to allow
+  // direct filesystem access.  (Warning: this is not a RAII class and
+  // will not close the fd on destruction.)
+  int mBrokerFd = -1;
+  // Determines whether we allow reading all files, for processes that
+  // render file:/// URLs.
+  bool mFileProcess = false;
+  // Syscall numbers to allow even if the seccomp-bpf policy otherwise
+  // wouldn't.
+  std::vector<int> mSyscallWhitelist;
+
+  static ContentProcessSandboxParams ForThisProcess(const dom::MaybeFileDesc& aBroker);
+};
+
 // Call only if SandboxInfo::CanSandboxContent() returns true.
 // (No-op if the sandbox is disabled.)
-// aBrokerFd is the filesystem broker client file descriptor,
-// or -1 to allow direct filesystem access.
 // isFileProcess determines whether we allow system wide file reads.
-MOZ_EXPORT bool SetContentProcessSandbox(int aBrokerFd,
+MOZ_EXPORT bool SetContentProcessSandbox(ContentProcessSandboxParams&& aParams);
-                                         bool aFileProcess,
-                                         std::vector<int>& aSyscallWhitelist);
 #endif
 
 #ifdef MOZ_GMP_SANDBOX

security/sandbox/linux/SandboxFilter.cpp

@@ -7,6 +7,7 @@
 #include "SandboxFilter.h"
 #include "SandboxFilterUtil.h"
 
+#include "Sandbox.h" // for ContentProcessSandboxParams
 #include "SandboxBrokerClient.h"
 #include "SandboxInfo.h"
 #include "SandboxInternal.h"
@@ -373,7 +374,7 @@ public:
 class ContentSandboxPolicy : public SandboxPolicyCommon {
 private:
   SandboxBrokerClient* mBroker;
-  std::vector<int> mSyscallWhitelist;
+  ContentProcessSandboxParams mParams;
 
   // Trap handlers for filesystem brokering.
   // (The amount of code duplication here could be improved....)
@@ -578,11 +579,14 @@ private:
   }
 
 public:
-  explicit ContentSandboxPolicy(SandboxBrokerClient* aBroker,
+  ContentSandboxPolicy(SandboxBrokerClient* aBroker,
-                                const std::vector<int>& aSyscallWhitelist)
+                       ContentProcessSandboxParams&& aParams)
-    : mBroker(aBroker),
+    : mBroker(aBroker)
-      mSyscallWhitelist(aSyscallWhitelist) {}
+    , mParams(Move(aParams))
+    { }
+
   ~ContentSandboxPolicy() override = default;
+
   Maybe<ResultExpr> EvaluateSocketCall(int aCall) const override {
     switch(aCall) {
     case SYS_RECVFROM:
@@ -658,8 +662,9 @@ public:
 
   ResultExpr EvaluateSyscall(int sysno) const override {
     // Straight allow for anything that got overriden via prefs
-    if (std::find(mSyscallWhitelist.begin(), mSyscallWhitelist.end(), sysno)
+    const auto& whitelist = mParams.mSyscallWhitelist;
-        != mSyscallWhitelist.end()) {
+    if (std::find(whitelist.begin(), whitelist.end(), sysno)
+        != whitelist.end()) {
       if (SandboxInfo::Get().Test(SandboxInfo::kVerbose)) {
         SANDBOX_LOG_ERROR("Allowing syscall nr %d via whitelist", sysno);
       }
@@ -1003,9 +1008,9 @@ public:
 
 UniquePtr<sandbox::bpf_dsl::Policy>
 GetContentSandboxPolicy(SandboxBrokerClient* aMaybeBroker,
-                        const std::vector<int>& aSyscallWhitelist)
+                        ContentProcessSandboxParams&& aParams)
 {
-  return MakeUnique<ContentSandboxPolicy>(aMaybeBroker, aSyscallWhitelist);
+  return MakeUnique<ContentSandboxPolicy>(aMaybeBroker, Move(aParams));
 }
 #endif // MOZ_CONTENT_SANDBOX
 

security/sandbox/linux/SandboxFilter.h

@@ -22,9 +22,10 @@ namespace mozilla {
 
 #ifdef MOZ_CONTENT_SANDBOX
 class SandboxBrokerClient;
+struct ContentProcessSandboxParams;
 
 UniquePtr<sandbox::bpf_dsl::Policy> GetContentSandboxPolicy(SandboxBrokerClient* aMaybeBroker,
-                                                            const std::vector<int>& aSyscallWhitelist);
+                                                            ContentProcessSandboxParams&& aParams);
 #endif
 
 #ifdef MOZ_GMP_SANDBOX

security/sandbox/linux/glue/SandboxPrefBridge.cpp

@@ -0,0 +1,51 @@
+/* -*- 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 "Sandbox.h"
+
+#include "mozilla/Preferences.h"
+#include "mozilla/SandboxSettings.h"
+#include "mozilla/dom/ContentChild.h"
+#include "mozilla/dom/ContentParent.h" // for FILE_REMOTE_TYPE
+
+namespace mozilla {
+
+/* static */ ContentProcessSandboxParams
+ContentProcessSandboxParams::ForThisProcess(const dom::MaybeFileDesc& aBroker)
+{
+  ContentProcessSandboxParams params;
+  params.mLevel = GetEffectiveContentSandboxLevel();
+
+  if (aBroker.type() == dom::MaybeFileDesc::TFileDescriptor) {
+    auto fd = aBroker.get_FileDescriptor().ClonePlatformHandle();
+    params.mBrokerFd = fd.release();
+    // brokerFd < 0 means to allow direct filesystem access, so
+    // make absolutely sure that doesn't happen if the parent
+    // didn't intend it.
+    MOZ_RELEASE_ASSERT(params.mBrokerFd >= 0);
+  }
+  // (Otherwise, mBrokerFd will remain -1 from the default ctor.)
+
+  auto* cc = dom::ContentChild::GetSingleton();
+  params.mFileProcess = cc->GetRemoteType().EqualsLiteral(FILE_REMOTE_TYPE);
+
+  nsAutoCString extraSyscalls;
+  nsresult rv =
+    Preferences::GetCString("security.sandbox.content.syscall_whitelist",
+                            extraSyscalls);
+  if (NS_SUCCEEDED(rv)) {
+    for (const nsACString& callNrString : extraSyscalls.Split(',')) {
+      int callNr = PromiseFlatCString(callNrString).ToInteger(&rv);
+      if (NS_SUCCEEDED(rv)) {
+        params.mSyscallWhitelist.push_back(callNr);
+      }
+    }
+  }
+
+  return params;
+}
+
+} // namespace mozilla

security/sandbox/linux/glue/moz.build

@@ -8,13 +8,19 @@ SOURCES += [
     '../../chromium/base/strings/safe_sprintf.cc',
     '../SandboxLogging.cpp',
     'SandboxCrash.cpp',
+    'SandboxPrefBridge.cpp',
 ]
 
 # Avoid Chromium logging dependency, because this is going into
 # libxul.  See also the comment in SandboxLogging.h.
 SOURCES['../../chromium/base/strings/safe_sprintf.cc'].flags += ['-DNDEBUG']
 
+# Need this for mozilla::ipc::FileDescriptor etc.
+include('/ipc/chromium/chromium-config.mozbuild')
+
 LOCAL_INCLUDES += [
+    # Need this for safe_sprintf.h used by SandboxLogging.h,
+    # but it has to be after ipc/chromium/src.
     '/security/sandbox/chromium',
     '/security/sandbox/linux',
 ]