Guillaume has the code to do this already, so assigning to him. Unfortunately, we can only do this for Gonk, because all of our usages of content processes rely on normal privileges currently.

Patch is also at https://github.com/gdestuynder/mozilla-central/commit/5340be48aacb764e105e17eb1455fdb8da73856b Some explanations: - as long as we don't use SHM/FS using a single UID should be fine (incl. IPDL via SHM I guess) - I'm using app_0 on Android instead of nobody, as the UID needs to be hard-coded, and nobody may be used by other daemons - app_0 cannot be renamed without modifying Bionic the libc, it's hard-coded as well - we can use one UID per app in the future if wanted/needed as long as we track the number of child processes - the 65534 UID fallback is arbitrary, albeit generally "nobody" on many distros (but not all) I have tested this patch with the current OOP applications such as the browser and the calculator with success so far.

Comment on attachment 645071 [details] [diff] [review] Setuid support >diff --git a/ipc/chromium/src/base/process_util_linux.cc b/ipc/chromium/src/base/process_util_linux.cc >index 1b013fe..2457789 100644 >--- a/ipc/chromium/src/base/process_util_linux.cc >+++ b/ipc/chromium/src/base/process_util_linux.cc >@@ -22,6 +22,9 @@ > > #ifdef ANDROID > #include <pthread.h> >+#include <private/android_filesystem_config.h> >+#define CHILD_UID AID_APP >+#define CHILD_GID AID_APP Please document these. It probably makes sense to call these CHILD_UNPRIVILEGED_GID/UID. >+#define CHILD_UID 65534 >+#define CHILD_GID 65534 And here too. > #endif > > namespace { >@@ -200,6 +206,18 @@ bool LaunchApp(const std::vector<std::string>& argv, > argv_cstr[i] = const_cast<char*>(argv[i].c_str()); > argv_cstr[argv.size()] = NULL; > >+ if (setgid(CHILD_GID) != 0) { Let's add an argument to LaunchApp() enum ChildPrivileges { UNPRIVILEGED, SAME_PRIVILEGES_AS_PARENT }; and only do the setuid/setgid when ChildPrivileges == UNPRIVILEGED. We want to pass UNPRIVILEGED in GeckoChildProcessHost.cpp when we're on gonk (it won't work anywhere else, and would break things). >+ if (chdir("/") != 0) >+ gProcessLog.print("==> could not chdir()\n"); What is this for? Looking good. Would like to see the next version of the patch.

I added the changes that I believe were requested. Hopefully I understood them correctly, in particular the places where it could be using SAME_PRIVILEGES_AS_PARENT vs UNPRIVILEGED. chdir() just make sure we're in a directory that won't fail to list, it's not really a security issue in that case. Also if we have seccomp later on, no operation related to that should occur (at least in theory). I'm fine dropping it. Note: for the seccomp patch, it would probably start in toolkit/xre/nsEmbedFunctions.cpp at XRE_InitChildProcess, as we may need a few more fds open and some syscalls to be run before seccomp filters are set. While I like to setuid early on (before execve), it is also possible to setuid there *and* there is a distinction between process types at this stage as well. (likewise it's possible to apply the seccomp filter before execve and those will be inherited via 'no new privs', but we probably wouldn't be able to make the filters as tight as this place)

D'oh! I didn't see the updated patch. Please re-request review from me if I don't r+ the first time :).

Comment on attachment 645149 [details] [diff] [review] Same patch with requested changed Looks good. I'm about to attach a version for landing with a commit message (make sure you include those in the future), some whitespace fixes, and one minor addition: disable no-rights subprocesses by default, but enable them on gonk. While it would warm my heart to enable them by default, we would break a slew of plugins and extensions on other platforms :/. One day! :)

Guillaume, it looks like this is your first gecko patch, so congratulations! :) Let's get some more landed after this ;).

thanks for the tips and changes! yeah i wasn't sure because of the r removed instead of r- :)

Oh, forgot to mention https://hg.mozilla.org/integration/mozilla-inbound/rev/201612a0b133 Your first gecko backout, too! :) I fixed up the patch locally, will try to reland tonight.