Linux Sandbox: move init processes to new session id.

Linux Sandbox: move init processes to new session id.

The setuid sandbox creates new PID namespaces. Make sure to have
the new init processes be inside a new session id and process group
so that children can't signal processes outside of the PID namespace.

BUG=446680
Committed: https://crrev.com/f48cead580e6c82a8f3c4ac2a493249da780ee28
Cr-Commit-Position: refs/heads/master@{#310394}

[jln (very slow on Chromium), 2015-01-07 00:48:07]

jln@chromium.org changed reviewers:
+ mdempsky@chromium.org

[jln (very slow on Chromium), 2015-01-07 00:48:19]

Matthew: PTAL!

[mdempsky, 2015-01-07 00:58:39]

lgtm

I'm a little worried if this has any usability implications (e.g., maybe a
desktop wants to send SIGHUP to all processes when the user logs out?), but I
think we can deal with those as they come up.  The obvious security win
outweighs the theoretical corner-case semantics change.

[jln (very slow on Chromium), 2015-01-07 01:08:49]

On 2015/01/07 00:58:39, mdempsky wrote:
> I'm a little worried if this has any usability implications (e.g., maybe a
> desktop wants to send SIGHUP to all processes when the user logs out?), but I
> think we can deal with those as they come up.  The obvious security win
> outweighs the theoretical corner-case semantics change.

If the browser process dies, everything else should die. It would be a pretty
severe bug if that wasn't the case.

It's even relatively robust: if our init process dies, the whole PID namespace
will disappear and the init process dies as soon as the Zygote dies. And the
Zygote dies when the browser dies.

For a signal such as SIGHUP, it's true that a Desktop environment could in
theory only send it to session ids it knows about (to not disturb background
processes) and we would now miss these. But I don't think we've ever made use of
this in Chromium.

I'm adding Chris Masone who knows the session manager in Chrome OS well, in case
he has some advice.

[Chris Masone, 2015-01-07 01:43:09]

On 2015/01/07 01:08:49, jln wrote:
> On 2015/01/07 00:58:39, mdempsky wrote:
> > I'm a little worried if this has any usability implications (e.g., maybe a
> > desktop wants to send SIGHUP to all processes when the user logs out?), but
I
> > think we can deal with those as they come up.  The obvious security win
> > outweighs the theoretical corner-case semantics change.
> 
> If the browser process dies, everything else should die. It would be a pretty
> severe bug if that wasn't the case.
> 
> It's even relatively robust: if our init process dies, the whole PID namespace
> will disappear and the init process dies as soon as the Zygote dies. And the
> Zygote dies when the browser dies.
> 
> For a signal such as SIGHUP, it's true that a Desktop environment could in
> theory only send it to session ids it knows about (to not disturb background
> processes) and we would now miss these. But I don't think we've ever made use
of
> this in Chromium.
> 
> I'm adding Chris Masone who knows the session manager in Chrome OS well, in
case
> he has some advice.

We send SIGTERM to the entire browser process group on CrOS when it's time to
sign out/shut down. So there's no concern from that side.

[mdempsky, 2015-01-07 01:49:05]

On 2015/01/07 01:43:09, Chris Masone wrote:
> We send SIGTERM to the entire browser process group on CrOS when it's time to
> sign out/shut down. So there's no concern from that side.

To be clear: moving the zygote init process into a new session means it will
also become a process group leader for a new process group.  (Sessions being
disjoint sets of process groups.)  So if you currently use kill(-42, SIGTERM) to
kill all of the Chrome processes in process group 42, this CL means that system
call will no longer send signals to the zygote init process or its children
because they'll be in a new process group.

That said, Julien and I don't expect that to be an issue because the zygote and
sandboxed processes should still get torn down when the browser exits.

[jln (very slow on Chromium), 2015-01-07 22:19:38]

The CQ bit was checked by jln@chromium.org

[commit-bot: I haz the power, 2015-01-07 22:21:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/837083002/1

[commit-bot: I haz the power, 2015-01-07 22:36:06]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-01-07 22:37:54]

Patchset 1 (id:??) landed as
https://crrev.com/f48cead580e6c82a8f3c4ac2a493249da780ee28
Cr-Commit-Position: refs/heads/master@{#310394}

[jln (very slow on Chromium), 2015-01-12 20:14:39]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/847723003/ by jln@chromium.org.

The reason for reverting is: Preemptively revert in case it caused a performance
regression described in https://crbug.com/447164 even though it seems unlikely..