Bug 1500297 - Require a broker client in ContentSandboxPolicy at level > 1. r=gcp
authorJed Davis <jld@mozilla.com>
Wed, 27 Feb 2019 15:23:25 +0000
changeset 519399 56f39977c72c62e0fdff0e5f68e72d6091b221db
parent 519398 bacaa3d582814d0a1ba3769de92e68a01d16a777
child 519400 48431f63d84227177951f65c9c828548d9a8bbb2
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgcp
bugs1500297, 1511560
milestone67.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 1500297 - Require a broker client in ContentSandboxPolicy at level > 1. r=gcp ContentSandboxPolicy currently allows direct filesystem access if it isn't given a broker client; this is a legacy design from the B2G era, before the current idea of "sandbox level". With this patch, it allows filesystem access at level 1, and above that it requires brokering. This is both to reduce the opportunities for accidentally having a too-permissive sandbox and to prepare for refactoring the broker glue in bug 1511560. Depends on D14519 Differential Revision: https://phabricator.services.mozilla.com/D14520
security/sandbox/linux/SandboxFilter.cpp
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -848,18 +848,22 @@ class ContentSandboxPolicy : public Sand
     const auto& whitelist = mParams.mSyscallWhitelist;
     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);
       }
       return Allow();
     }
-    if (mBroker) {
+
+    // Level 1 allows direct filesystem access; higher levels use
+    // brokering.
+    if (!BelowLevel(2)) {
       // Have broker; route the appropriate syscalls to it.
+      MOZ_RELEASE_ASSERT(mBroker != nullptr);
       switch (sysno) {
         case __NR_open:
           return Trap(OpenTrap, mBroker);
         case __NR_openat:
           return Trap(OpenAtTrap, mBroker);
         case __NR_access:
           return Trap(AccessTrap, mBroker);
         case __NR_faccessat:
@@ -886,16 +890,17 @@ class ContentSandboxPolicy : public Sand
           return Trap(UnlinkTrap, mBroker);
         case __NR_readlink:
           return Trap(ReadlinkTrap, mBroker);
         case __NR_readlinkat:
           return Trap(ReadlinkAtTrap, mBroker);
       }
     } else {
       // No broker; allow the syscalls directly.  )-:
+      MOZ_ASSERT(!mBroker);
       switch (sysno) {
         case __NR_open:
         case __NR_openat:
         case __NR_access:
         case __NR_faccessat:
         CASES_FOR_stat:
         CASES_FOR_lstat:
         CASES_FOR_fstatat: