Bug 986397 covers a lot of territory, and a lot of it has annoying prerequisites, but: 1. GeckoMediaPlugin child processes are already well-behaved in terms of resource access. 2. Everything besides PID namespace isolation can be done without changes to how child processes are started. 3. Unprivileged user namespaces allow avoiding the difficulties of shipping a setuid root executable, at the cost of universal support. So this bug is for that combination of constraints: GMP (not content), applying chroot and network namespace and SysV IPC namespace isolation (but not pid namespaces), on kernels that support unprivileged user namespaces. This will borrow the part of bug 1088387 about having a hook into sandboxing while the child process is single-threaded (but not the questionable cross-thread forwarding of arbitrary syscalls which was the focus of that bug).

This is trivial cleanup and could be squashed into the next patch, but it's clearer where #includes are/aren't being added, this way.

This is, mostly, the part that was broken out of bug 1088387. Needs IPC review for touching early content process startup.

…and this is where most of the actual code for this bug is.

Comment on attachment 8588801 [details] [diff] [review] Step 2: Apply net/ipc namespace separation and chroot to media plugins. Review of attachment 8588801 [details] [diff] [review]: ----------------------------------------------------------------- ::: security/sandbox/linux/common/SandboxInfo.cpp @@ +116,5 @@ > + // > + // Also, unshare never works under valgrind, but testing with clone > + // might return a false positive: > +#ifdef MOZ_VALGRIND > + if (RUNNING_ON_VALGRIND) { Insufficient ifdef; will fix.

Comment on attachment 8588801 [details] [diff] [review] Step 2: Apply net/ipc namespace separation and chroot to media plugins. Review of attachment 8588801 [details] [diff] [review]: ----------------------------------------------------------------- ::: security/sandbox/linux/LinuxCapabilities.h @@ +66,5 @@ > + return SetCurrentRaw(); > + } > + > + void Normalize() { > + for (size_t i = 0; i < _LINUX_CAPABILITY_U32S_3; ++i) { …and there are Android kernel header issues, because of course there are. I thought I'd checked for that locally, but maybe not on the final version of the patch. Will fix.

Better idea: handle the possibilty of valgrind by directly testing whether unshare() is supported. (The syscall been in the kernel since 2.6.16, so calling it with no flags – which is explicitly documented as being allowed and a no-op — should fail only in the case of that kind of interposition.) Also, breaking this out into its own patch.

Now with assorted fixes to the B2G build. Verified on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=49479ce9e0df

Comment on attachment 8588800 [details] [diff] [review] Step 1: Add Linux sandboxing hook for when child processes are still single-threaded. Review of attachment 8588800 [details] [diff] [review]: ----------------------------------------------------------------- ::: security/sandbox/linux/SandboxUtil.cpp @@ +23,5 @@ > + MOZ_DIAGNOSTIC_ASSERT(false, "Couldn't access /proc/self/task!"); > + return false; > + } > + MOZ_DIAGNOSTIC_ASSERT(sb.st_nlink >= 3); > + return sb.st_nlink == 3; How does this work? (on a regular linux here i see < 3 hardlinks for a regular process, as in non-gecko without threads)

(In reply to Guillaume Destuynder [:kang] from comment #8) > Comment on attachment 8588800 [details] [diff] [review] > > + MOZ_DIAGNOSTIC_ASSERT(sb.st_nlink >= 3); > > + return sb.st_nlink == 3; > > How does this work? (on a regular linux here i see < 3 hardlinks for a > regular process, as in non-gecko without threads) The three links should be the link from the parent directory, the "." link, and the ".." link from the child. Can you pastebin the output of "stat /proc/self/task" where you're seeing fewer links for single-threaded processes?

Comment on attachment 8588800 [details] [diff] [review] Step 1: Add Linux sandboxing hook for when child processes are still single-threaded. Review of attachment 8588800 [details] [diff] [review]: ----------------------------------------------------------------- r=me on the ContentChild changes ::: security/sandbox/linux/Sandbox.cpp @@ +422,5 @@ > BroadcastSetThreadSandbox(aType); > } > > +void > +SandboxEarlyInit(GeckoProcessType aType, bool aIsNuwa) I don't get it, |aIsNuwa| is unused? ::: security/sandbox/linux/Sandbox.h @@ +24,5 @@ > > namespace mozilla { > > +// This must be called early, while the process is still single-threaded. > +MOZ_SANDBOX_EXPORT void SandboxEarlyInit(GeckoProcessType aType, bool aIsNuwa); I *think* we can forward-declare enums now, so you shouldn't need the nsXULAppAPI include here.

(In reply to Jed Davis [:jld] {UTC-7} from comment #9) > (In reply to Guillaume Destuynder [:kang] from comment #8) > > Comment on attachment 8588800 [details] [diff] [review] > > > + MOZ_DIAGNOSTIC_ASSERT(sb.st_nlink >= 3); > > > + return sb.st_nlink == 3; > > > > How does this work? (on a regular linux here i see < 3 hardlinks for a > > regular process, as in non-gecko without threads) > > The three links should be the link from the parent directory, the "." link, > and the ".." link from the child. Can you pastebin the output of "stat > /proc/self/task" where you're seeing fewer links for single-threaded > processes? My method of calling stat (via find -links) didnt actually call stat the way i expected, it works fine calling stat directly. All good!

Comment on attachment 8589139 [details] [diff] [review] bug1151607-p2-isolate-hg0.diff Review of attachment 8589139 [details] [diff] [review]: ----------------------------------------------------------------- ::: security/sandbox/linux/SandboxChroot.cpp @@ +73,5 @@ > + > +static int > +OpenDeletedDirectory() > +{ > + // We don't need (nor want) this to persist between invocations of it still *may* persist until reboot/cleanup if deletion fails. Won't really cause a problem tho, mkdtemp should choose the next available name. @@ +159,5 @@ > + caps.Effective(CAP_SYS_CHROOT) = true; > + // And leave everything else off. > + if (!caps.SetCurrent()) { > + SANDBOX_LOG_ERROR("capset: %s", strerror(errno)); > + MOZ_CRASH("Can't limit chroot thread's capabilities"); how do we ensure we generally get this cap right now?

(In reply to Ben Turner [:bent] (use the needinfo flag!) from comment #10) > > +void > > +SandboxEarlyInit(GeckoProcessType aType, bool aIsNuwa) > > I don't get it, |aIsNuwa| is unused? Most of the body of that function is added in the last patch. (I originally had a comment saying that, which the later patch removed, but I took it out at some point for some reason.) > > +// This must be called early, while the process is still single-threaded. > > +MOZ_SANDBOX_EXPORT void SandboxEarlyInit(GeckoProcessType aType, bool aIsNuwa); > > I *think* we can forward-declare enums now, so you shouldn't need the > nsXULAppAPI include here. It looks like the enum needs to be defined with an explicit base type (e.g., `enum GeckoProcessType : uint8_t { ... }`) to be able to do that.

(In reply to Guillaume Destuynder [:kang] (PTO til 2015-04-19) from comment #12) > @@ +159,5 @@ > > + caps.Effective(CAP_SYS_CHROOT) = true; > > + // And leave everything else off. > > + if (!caps.SetCurrent()) { > > + SANDBOX_LOG_ERROR("capset: %s", strerror(errno)); > > + MOZ_CRASH("Can't limit chroot thread's capabilities"); > > how do we ensure we generally get this cap right now? This is in SandboxChroot::ThreadMain, which is called from SandboxChroot::StaticThreadMain, which is called by the pthread_create in SandboxChroot::Prepare, which checks for the cap before creating the thread, which is created as a clone of pthread_create's caller. I'll add a comment.

https://hg.mozilla.org/integration/mozilla-inbound/rev/53a41684adcb https://hg.mozilla.org/integration/mozilla-inbound/rev/4ed5d64f054b https://hg.mozilla.org/integration/mozilla-inbound/rev/9a186f904c5d https://hg.mozilla.org/integration/mozilla-inbound/rev/acc410f0b28c

https://hg.mozilla.org/mozilla-central/rev/53a41684adcb https://hg.mozilla.org/mozilla-central/rev/4ed5d64f054b https://hg.mozilla.org/mozilla-central/rev/9a186f904c5d https://hg.mozilla.org/mozilla-central/rev/acc410f0b28c