Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 8 users
Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux, Chrome
Pri: 2
Type: Bug

Blocking:
issue 312380
issue 421748



Sign in to add a comment
Separate renderers and NaCl processes with PID namespaces
Project Member Reported by jln@chromium.org, Feb 23 2015 Back to list
[Background: cross-renderer interaction (and most importantly, integrity violation) is crucial to block in the Chrome security model. Beside site isolation (https://www.chromium.org/developers/design-documents/site-isolation), the fact that many renderers have very high privileges in the Chrome security model makes this necessary]

We should try and separate renderers and have each live in its own PID namespace.

Currently, we enforce renderer separation via:

1. Making renderers non dumpable (weak)
2. seccomp-bpf ( issue 413855 ). Which is error-prone and difficult.

Having each renderer in its own PID namespace would have us rely on a semantic that the kernel understands and enforces for us, which is more robust.
 
Comment 1 by jln@chromium.org, Feb 23 2015
Cc: keescook@chromium.org
- As part of this, we should look into the cost of PID namespaces.

- Network namespace used to be very expensive (see  issue 110756 ).

This was fixed (http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=976a702ac9eeacea09e588456ab165dc06f9ee83), but we should still probably keep all renderers in the same network namespace.
Comment 2 by rickyz@chromium.org, Feb 24 2015
Cc: mdempsky@chromium.org
I looked through the main work that happens in creating new pid namespaces, and the only thing that stood out was how it allocates a page for the pid map. On a sendmsg with SCM_CREDENTIALS, I didn't see anything that jumped out as being slow for translating the pid. mdempsky mentioned in https://codereview.chromium.org/868233011/ that he remembered seeing slowdowns on UnixDomainSocketTest.DoubleNamespace on precise vs. trusty, but hopefully this won't be an issue since we'll only be doing this on machines that support user namespaces.

I did a couple of runs of the startup.warm.blank_page on my work machine and didn't see any obvious differences in startup.warm.blank_page:foreground_tab_load_complete (which is the component that includes renderer startup time). There was a lot of variance though, so not sure how useful this result is.
Comment 3 by jln@chromium.org, Mar 24 2015
Blocking: chromium:312380
Comment 4 by jln@chromium.org, Mar 24 2015
Blockedon: -chromium:312380
Comment 5 by jln@chromium.org, Mar 27 2015
Cc: mseaborn@chromium.org
Summary: Separate renderers and NaCl processes with PID namespaces (was: Separate renderers with PID namespaces)
Changing the title to reflect doing this for NaCl processes as well.

When processes are running with PID = 1, typically terminating signals are ignored, unless a signal handler is explicitly installed.

NaCl doesn't like pre-existing signal handlers, but we already made an exception a while back for crashing signal handlers (this was done for SIGSYS).

We'll need to do something along these lines as well.
Project Member Comment 6 by bugdroid1@chromium.org, Mar 27 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4557699fbb30704f76c68b906d2656d2e322572c

commit 4557699fbb30704f76c68b906d2656d2e322572c
Author: rickyz <rickyz@chromium.org>
Date: Fri Mar 27 22:31:15 2015

Start all children in their own PID namespace.

BUG=460972

Review URL: https://codereview.chromium.org/868233011

Cr-Commit-Position: refs/heads/master@{#322660}

[modify] http://crrev.com/4557699fbb30704f76c68b906d2656d2e322572c/content/common/sandbox_linux/sandbox_linux.cc
[modify] http://crrev.com/4557699fbb30704f76c68b906d2656d2e322572c/content/common/sandbox_linux/sandbox_linux.h
[modify] http://crrev.com/4557699fbb30704f76c68b906d2656d2e322572c/content/zygote/zygote_linux.cc
[modify] http://crrev.com/4557699fbb30704f76c68b906d2656d2e322572c/content/zygote/zygote_main_linux.cc
[modify] http://crrev.com/4557699fbb30704f76c68b906d2656d2e322572c/sandbox/linux/services/credentials.cc
[modify] http://crrev.com/4557699fbb30704f76c68b906d2656d2e322572c/sandbox/linux/services/credentials.h
[modify] http://crrev.com/4557699fbb30704f76c68b906d2656d2e322572c/sandbox/linux/services/credentials_unittest.cc
[modify] http://crrev.com/4557699fbb30704f76c68b906d2656d2e322572c/sandbox/linux/services/namespace_sandbox.cc
[modify] http://crrev.com/4557699fbb30704f76c68b906d2656d2e322572c/sandbox/linux/services/namespace_sandbox.h
[modify] http://crrev.com/4557699fbb30704f76c68b906d2656d2e322572c/sandbox/linux/services/namespace_sandbox_unittest.cc

Project Member Comment 7 by bugdroid1@chromium.org, Mar 30 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/591f7ec686f9550fc4332b6cf7c3f0ea0544d558

commit 591f7ec686f9550fc4332b6cf7c3f0ea0544d558
Author: spang <spang@chromium.org>
Date: Mon Mar 30 22:09:06 2015

Revert of Start all children in their own PID namespace. (patchset #14 id:360001 of https://codereview.chromium.org/868233011/)

Reason for revert:
Appears to cause system lockups on Chrome OS. See  crbug.com/471878 

Original issue's description:
> Start all children in their own PID namespace.
>
> BUG=460972
>
> Committed: https://crrev.com/4557699fbb30704f76c68b906d2656d2e322572c
> Cr-Commit-Position: refs/heads/master@{#322660}

TBR=jln@chromium.org,mdempsky@chromium.org,rickyz@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=460972

Review URL: https://codereview.chromium.org/1041163003

Cr-Commit-Position: refs/heads/master@{#322891}

[modify] http://crrev.com/591f7ec686f9550fc4332b6cf7c3f0ea0544d558/content/common/sandbox_linux/sandbox_linux.cc
[modify] http://crrev.com/591f7ec686f9550fc4332b6cf7c3f0ea0544d558/content/common/sandbox_linux/sandbox_linux.h
[modify] http://crrev.com/591f7ec686f9550fc4332b6cf7c3f0ea0544d558/content/zygote/zygote_linux.cc
[modify] http://crrev.com/591f7ec686f9550fc4332b6cf7c3f0ea0544d558/content/zygote/zygote_main_linux.cc
[modify] http://crrev.com/591f7ec686f9550fc4332b6cf7c3f0ea0544d558/sandbox/linux/services/credentials.cc
[modify] http://crrev.com/591f7ec686f9550fc4332b6cf7c3f0ea0544d558/sandbox/linux/services/credentials.h
[modify] http://crrev.com/591f7ec686f9550fc4332b6cf7c3f0ea0544d558/sandbox/linux/services/credentials_unittest.cc
[modify] http://crrev.com/591f7ec686f9550fc4332b6cf7c3f0ea0544d558/sandbox/linux/services/namespace_sandbox.cc
[modify] http://crrev.com/591f7ec686f9550fc4332b6cf7c3f0ea0544d558/sandbox/linux/services/namespace_sandbox.h
[modify] http://crrev.com/591f7ec686f9550fc4332b6cf7c3f0ea0544d558/sandbox/linux/services/namespace_sandbox_unittest.cc

Labels: -M-43 M-44 MovedFrom-43
[AUTO] Moving all non essential bugs to the next Milestone.  (This decision is based on the labels attached to your ticket.)


Ref: https://sites.google.com/a/chromium.org/dev/developers/ticket-milestone-punting-1
Comment 11 by jln@chromium.org, Apr 17 2015
Blocking: chromium:421748
Comment 12 by jln@chromium.org, May 12 2015
Ricky, any progress on the NaCl side of this?
Labels: -M-44 MovedFrom-44
[AUTO] This issue has already been moved once and is lower than Priority 1,therefore removing mstone.
Project Member Comment 14 by bugdroid1@chromium.org, Jun 5 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/native_client/src/native_client.git/+/3b1b3cf099145cf3db831f7f29a251ac071c5290

commit 3b1b3cf099145cf3db831f7f29a251ac071c5290
Author: rickyz <rickyz@chromium.org>
Date: Fri Jun 05 16:42:22 2015

Add SIGHUP and SIGTERM handlers in NaCl.

When running NaCl code in a PID namespace, we need explicit handlers for
signals that we might receive from outside the PID namespace, such as
SIGTERM. This change installs a signal handler which exits by default.

BUG= https://code.google.com/p/chromium/issues/detail?id=460972

Review URL: https://codereview.chromium.org/1159803003

[modify] http://crrev.com/3b1b3cf099145cf3db831f7f29a251ac071c5290/src/trusted/service_runtime/linux/nacl_signal.c

https://codereview.chromium.org/1158793003/ enables this for normal NaCl, but we're still missing PID namespace per process for nonSFI.
Project Member Comment 16 by bugdroid1@chromium.org, Jun 16 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/native_client/src/native_client.git/+/0f50a4b61659b23188e27ab42485d265d3d063dd

commit 0f50a4b61659b23188e27ab42485d265d3d063dd
Author: rickyz <rickyz@chromium.org>
Date: Tue Jun 16 02:38:12 2015

Exit with -sig when handling signals.

This is more consistent with the behavior in
src/trusted/service_runtime/nacl_signal_common.c.

BUG= http://code.google.com/p/chromium/issues/detail?id=460972

Review URL: https://codereview.chromium.org/1184233002

[modify] http://crrev.com/0f50a4b61659b23188e27ab42485d265d3d063dd/src/nonsfi/linux/irt_exception_handling.c

Project Member Comment 18 by bugdroid1@chromium.org, Jun 19 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d423ad87b1fdc8a59a7c945c000bfbbd9d8b2245

commit d423ad87b1fdc8a59a7c945c000bfbbd9d8b2245
Author: rickyz <rickyz@chromium.org>
Date: Fri Jun 19 08:30:26 2015

Enable one PID namespace per process for NaCl processes.

This CL does two things:
 - Make the NaCl helper fork each process into a new PID namespace via
   ForkInNewPidNamespace.
 - Bring the non-NaCl process termination exit codes in line with NaCl's
   default signal handlers, that is exit with an exit code of -sig &
   0xff.

This change depends on https://codereview.chromium.org/1159803003, which
adds termination signals handlers in SFI NaCl.

BUG=460972

Review URL: https://codereview.chromium.org/1158793003

Cr-Commit-Position: refs/heads/master@{#335223}

[modify] http://crrev.com/d423ad87b1fdc8a59a7c945c000bfbbd9d8b2245/components/nacl/loader/nacl_helper_linux.cc
[modify] http://crrev.com/d423ad87b1fdc8a59a7c945c000bfbbd9d8b2245/components/nacl/loader/nonsfi/irt_exception_handling.cc
[modify] http://crrev.com/d423ad87b1fdc8a59a7c945c000bfbbd9d8b2245/components/nacl/loader/sandbox_linux/nacl_sandbox_linux.cc
[modify] http://crrev.com/d423ad87b1fdc8a59a7c945c000bfbbd9d8b2245/content/zygote/zygote_linux.cc
[modify] http://crrev.com/d423ad87b1fdc8a59a7c945c000bfbbd9d8b2245/sandbox/linux/services/credentials.cc
[modify] http://crrev.com/d423ad87b1fdc8a59a7c945c000bfbbd9d8b2245/sandbox/linux/services/credentials.h
[modify] http://crrev.com/d423ad87b1fdc8a59a7c945c000bfbbd9d8b2245/sandbox/linux/services/namespace_sandbox.cc
[modify] http://crrev.com/d423ad87b1fdc8a59a7c945c000bfbbd9d8b2245/sandbox/linux/services/namespace_sandbox.h
[modify] http://crrev.com/d423ad87b1fdc8a59a7c945c000bfbbd9d8b2245/sandbox/linux/services/namespace_sandbox_unittest.cc

Curious, what work remains at this point?

BTW, I suspect this is causing an annoyance for me. All the renderers' LOG() output now has pid=1, so I have trouble telling renderers apart in console logs. This makes developing for OOPIF slight more confusing.
Comment 20 by j...@panix.com, Jul 28 2015
@#19: If the process wants its outer pid, calling readlink on /proc/self at some point before revoking filesystem access should work.
Comment 21 by jln@chromium.org, Jul 28 2015
Lei: this should be pretty much done. There are some minimal things to figure out, like removing non-dumpability, but it should be other bugs.

For the issue with LOG(). This is somewhat related to issue 501069. rickyz@, could you try to land your CL again (https://chromiumcodereview.appspot.com/1186873006/) ?
Comment 22 by jln@chromium.org, Jul 28 2015
#20: we actually have mechanisms to get one's real pid already. See  issue 357670 . But we don't always want to give it away because we consider it a minor info leak. We will start with an implementation that has LOG print the real pid though. But in Release mode, we might switch to some other unique value at some point.
i have that but it got taken away or got erase and i don't know how. so anyways i need someone to protect my motherboard and board appId on my chromebook.
Owner: ----
Status: Untriaged
Sign in to add a comment