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 45 users
Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux, Chrome
Pri: 2
Type: Bug


Sign in to add a comment
Replace the setuid sandbox with unprivileged namespaces.
Project Member Reported by jln@chromium.org, Oct 28 2013 Back to list
The setuid sandbox has to go:

- For each "chroot me" event, the current mechanism requires having previously started a setuid helper. This is not compatible with a more generic and universal Zygote. It is in conflict with the flexibility we want for Mojo.
- chroot helpers are tied to new PID and network namespaces. Again, this conflicts with flexibility as network namespaces use a lot of kernel memory.
- Shipping a setuid binary on Linux and on Chrome OS is bad for security.
- Having to update a setuid executable on our bots when we want to make changes is problematic

Hopefully we can fix the following issues as well:
- To be isolated from each other, renderers need to be marked as non dumpable. Since Breakpad needs to be able to ptrace() renderers, they become dumpable again on SIGSEGV.
To prevent a renderer from sending SIGSEGV to another renderer, we put in place an authentication mechanism which require a kernel fix (CVE-2011-1182). This fix has now regressed. Also see issue 169369.

Both seccomp-bpf and Yama can offer an alternate way to protect processes in the same PID namespace from each other.

- Searching for the "real pid" of children under a new PID namespace is tremendously complicated and requires going through each process in /proc (which is itself much harder when the processes are not dumpable). We should investigate the cost of having one PID namespace per process as it would allow the parent to immediately know the real PID. It would also solve the "dumpable" issue neatly as processes wouldn't need to be non dumpable.

Plans:

- Use unprivileged namespaces (through CLONE_NEWUSER) when available. Hopefully this will be available by default with Ubuntu 14.04 LTS.
I have made a proof-of-concept and it looks like it'll work.
With unprivileged namespaces processes could keep the CAP_SYS_CHROOT capability until the sandbox is fully engaged, which would solve a lot of the current issues.

- Get CLONE_NEWUSER in Chrome OS with kernel 3.9+

- Link Chromium against libcap2. If that's not possible, rewrite a small POSIX.1e library.

- We may have to support the current setuid sandbox mechanism alongside the new mechanism until support for Ubuntu Precise is dropped. It's unfortunate as complexity is high.

 
Comment 1 by jln@chromium.org, Oct 28 2013
Blockedon: chromium:312382
Getting CLONE_NEWUSER on Chrome OS is tracked in issue 312382.
Comment 2 by jln@chromium.org, Oct 28 2013
Blockedon: chromium:312384
libcap2 tracked in issue 312384
Comment 3 by jln@chromium.org, Oct 28 2013
Blockedon: chromium:312388
Chrome OS is abusing the setuid sandbox to touch /sys/kernel/mm/chromeos-low_mem/margin. Tracked in issue 312388.
We're also gonna need to make the chromeos-chrome ebuild depend on libcap. Tracked in issue 313847.
Comment 5 by jln@chromium.org, Oct 31 2013
Blockedon: chromium:313847
Project Member Comment 6 by bugdroid1@chromium.org, Nov 1 2013
------------------------------------------------------------------------
r232280 | jln@chromium.org | 2013-11-01T02:30:52.675328Z

Changed paths:
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=232280&r2=232279&pathrev=232280
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux.gypi?r1=232280&r2=232279&pathrev=232280
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=232280&r2=232279&pathrev=232280
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux_test_sources.gypi?r1=232280&r2=232279&pathrev=232280
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=232280&r2=232279&pathrev=232280

Linux: add a Credentials class to handle Linux capabilities.

BUG=312380
R=jorgelo@chromium.org

Review URL: https://codereview.chromium.org/51113009
------------------------------------------------------------------------
Project Member Comment 7 by bugdroid1@chromium.org, Nov 1 2013
------------------------------------------------------------------------
r232281 | jln@chromium.org | 2013-11-01T02:47:12.184368Z

Changed paths:
   D http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=232281&r2=232280&pathrev=232281
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux.gypi?r1=232281&r2=232280&pathrev=232281
   D http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=232281&r2=232280&pathrev=232281
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux_test_sources.gypi?r1=232281&r2=232280&pathrev=232281
   D http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=232281&r2=232280&pathrev=232281

Revert 232280 "Linux: add a Credentials class to handle Linux ca..."

> Linux: add a Credentials class to handle Linux capabilities.
> 
> BUG=312380
> R=jorgelo@chromium.org
> 
> Review URL: https://codereview.chromium.org/51113009

TBR=jln@chromium.org

Review URL: https://codereview.chromium.org/54463008
------------------------------------------------------------------------
Project Member Comment 8 by bugdroid1@chromium.org, Nov 5 2013
------------------------------------------------------------------------
r232730 | thestig@chromium.org | 2013-11-04T18:02:56.950832Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/linux/sysroot_scripts/packagelist.debian.wheezy.i386?r1=232730&r2=232729&pathrev=232730
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/linux/sysroot_scripts/packagelist.debian.wheezy.amd64?r1=232730&r2=232729&pathrev=232730
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/linux/sysroot_scripts/sysroot-creator-debian.wheezy.sh?r1=232730&r2=232729&pathrev=232730

Linux: Update Debian Wheezy sysroot package data, also include libpcap.

BUG=312380
R=mmoss@chromium.org

Review URL: https://codereview.chromium.org/51383005
------------------------------------------------------------------------
Project Member Comment 9 by bugdroid1@chromium.org, Nov 5 2013
------------------------------------------------------------------------
r232739 | thestig@chromium.org | 2013-11-04T18:20:08.110631Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/linux/sysroot_scripts/install-debian.wheezy.sysroot.py?r1=232739&r2=232738&pathrev=232739

Linux: Update sysroot script to pull new tarballs with libpcap.

BUG=312380
NOTRY=true
TBR=mmoss@chromium.org

Review URL: https://codereview.chromium.org/57773003
------------------------------------------------------------------------
Project Member Comment 10 by bugdroid1@chromium.org, Nov 5 2013
------------------------------------------------------------------------
r232816 | sbc@chromium.org | 2013-11-04T22:25:24.349085Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/build/linux/install-arm-sysroot.py?r1=232816&r2=232815&pathrev=232816

Update arm sysroot image used by arm/linux builder.

This brings in the new libpcap2 package which was added
to the build script here:
https://codereview.chromium.org/51323006/

Also, remove support for install armel sysroot now that
all the builders are on armhf.

BUG=312380
R=thestig@chromium.org

Review URL: https://codereview.chromium.org/55233006
------------------------------------------------------------------------
Project Member Comment 11 by bugdroid1@chromium.org, Nov 5 2013
------------------------------------------------------------------------
r232837 | jln@chromium.org | 2013-11-04T23:41:46.588098Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux_test_sources.gypi?r1=232837&r2=232836&pathrev=232837
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=232837&r2=232836&pathrev=232837
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=232837&r2=232836&pathrev=232837
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux.gypi?r1=232837&r2=232836&pathrev=232837
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=232837&r2=232836&pathrev=232837

Linux: add a Credentials class to handle Linux capabilities.

(This is a re-land of https://chromiumcodereview.appspot.com/51113009/)

BUG=312380
TBR=jorgelo@chromium.org

Review URL: https://codereview.chromium.org/55603003
------------------------------------------------------------------------
Project Member Comment 12 by bugdroid1@chromium.org, Nov 5 2013
------------------------------------------------------------------------
r232842 | jln@chromium.org | 2013-11-05T00:07:10.039539Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux_test_sources.gypi?r1=232842&r2=232841&pathrev=232842
   D http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=232842&r2=232841&pathrev=232842
   D http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=232842&r2=232841&pathrev=232842
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux.gypi?r1=232842&r2=232841&pathrev=232842
   D http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=232842&r2=232841&pathrev=232842

Revert 232837 "Linux: add a Credentials class to handle Linux ca..."

> Linux: add a Credentials class to handle Linux capabilities.
> 
> (This is a re-land of https://chromiumcodereview.appspot.com/51113009/)
> 
> BUG=312380
> TBR=jorgelo@chromium.org
> 
> Review URL: https://codereview.chromium.org/55603003

TBR=jln@chromium.org

Review URL: https://codereview.chromium.org/45923006
------------------------------------------------------------------------
Project Member Comment 13 by bugdroid1@chromium.org, Nov 5 2013
------------------------------------------------------------------------
r232852 | jln@chromium.org | 2013-11-05T00:42:38.139923Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux_test_sources.gypi?r1=232852&r2=232851&pathrev=232852
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=232852&r2=232851&pathrev=232852
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=232852&r2=232851&pathrev=232852
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux.gypi?r1=232852&r2=232851&pathrev=232852
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=232852&r2=232851&pathrev=232852

Linux: add a Credentials class to handle Linux capabilities.

(This is a re-land of https://chromiumcodereview.appspot.com/51113009/)

BUG=312380
TBR=jorgelo@chromium.org

Review URL: https://codereview.chromium.org/58693002
------------------------------------------------------------------------
Project Member Comment 14 by bugdroid1@chromium.org, Nov 5 2013
------------------------------------------------------------------------
r232927 | ilevy@chromium.org | 2013-11-05T06:35:47.515456Z

Changed paths:
   D http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=232927&r2=232926&pathrev=232927
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux.gypi?r1=232927&r2=232926&pathrev=232927
   D http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=232927&r2=232926&pathrev=232927
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux_test_sources.gypi?r1=232927&r2=232926&pathrev=232927
   D http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=232927&r2=232926&pathrev=232927

Revert 232852 "Linux: add a Credentials class to handle Linux ca..."

Causing 30-50% rate failure in slave* trybots.  I'm sorry Julien. :-\

> Linux: add a Credentials class to handle Linux capabilities.
> 
> (This is a re-land of https://chromiumcodereview.appspot.com/51113009/)
> 
> BUG=312380
> TBR=jorgelo@chromium.org
> 
> Review URL: https://codereview.chromium.org/58693002

TBR=jln@chromium.org

Review URL: https://codereview.chromium.org/59073002
------------------------------------------------------------------------
Project Member Comment 15 by bugdroid1@chromium.org, Nov 5 2013
------------------------------------------------------------------------
r233027 | jln@chromium.org | 2013-11-05T18:17:13.498607Z

Changed paths:
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=233027&r2=233026&pathrev=233027
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux.gypi?r1=233027&r2=233026&pathrev=233027
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=233027&r2=233026&pathrev=233027
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux_test_sources.gypi?r1=233027&r2=233026&pathrev=233027
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=233027&r2=233026&pathrev=233027

Linux: add a Credentials class to handle Linux capabilities.

(This is a re-land of https://chromiumcodereview.appspot.com/51113009/)

BUG=312380, 312572
TBR=jorgelo@chromium.org

Review URL: https://codereview.chromium.org/60513003
------------------------------------------------------------------------
Project Member Comment 16 by bugdroid1@chromium.org, Nov 5 2013
------------------------------------------------------------------------
r233041 | jln@chromium.org | 2013-11-05T18:56:13.860028Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/tests/main.cc?r1=233041&r2=233040&pathrev=233041
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=233041&r2=233040&pathrev=233041
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=233041&r2=233040&pathrev=233041
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=233041&r2=233040&pathrev=233041

Linux: add basic unprivileged namespace support.

The Credentials class now has basic support for unprivileged namespaces.

BUG=312380
R=jorgelo@chromium.org

Review URL: https://codereview.chromium.org/54643010
------------------------------------------------------------------------
Project Member Comment 17 by bugdroid1@chromium.org, Nov 5 2013
------------------------------------------------------------------------
r233097 | jln@chromium.org | 2013-11-05T22:04:03.326356Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=233097&r2=233096&pathrev=233097

Linux sandbox: print kernel support in credentials unittests.

Make sure that we flush stdout after printing status.

BUG=312380
R=jorgelo@chromium.org

Review URL: https://codereview.chromium.org/59843005
------------------------------------------------------------------------
Project Member Comment 18 by bugdroid1@chromium.org, Nov 26 2013
------------------------------------------------------------------------
r237242 | jln@chromium.org | 2013-11-26T03:32:28.885441Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/sandbox_linux.cc?r1=237242&r2=237241&pathrev=237242
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=237242&r2=237241&pathrev=237242
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=237242&r2=237241&pathrev=237242
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=237242&r2=237241&pathrev=237242

Linux sandbox: move CurrentProcessHasOpenDirectories

Move CurrentProcessHasOpenDirectories() to the Credentials class and rename
it to HasOpenDirectory().
Also add some unittests.

BUG=312380
R=jorgelo@chromium.org

Review URL: https://codereview.chromium.org/85403011
------------------------------------------------------------------------
Project Member Comment 19 by bugdroid1@chromium.org, Nov 26 2013
------------------------------------------------------------------------
r237251 | jln@chromium.org | 2013-11-26T04:14:23.937208Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=237251&r2=237250&pathrev=237251
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=237251&r2=237250&pathrev=237251
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/sandbox_linux.cc?r1=237251&r2=237250&pathrev=237251
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=237251&r2=237250&pathrev=237251

Revert 237242 "Linux sandbox: move CurrentProcessHasOpenDirectories"

> Linux sandbox: move CurrentProcessHasOpenDirectories
> 
> Move CurrentProcessHasOpenDirectories() to the Credentials class and rename
> it to HasOpenDirectory().
> Also add some unittests.
> 
> BUG=312380
> R=jorgelo@chromium.org
> 
> Review URL: https://codereview.chromium.org/85403011

TBR=jln@chromium.org

Review URL: https://codereview.chromium.org/85343005
------------------------------------------------------------------------
Project Member Comment 20 by bugdroid1@chromium.org, Nov 26 2013
------------------------------------------------------------------------
r237390 | jln@chromium.org | 2013-11-26T19:45:15.364581Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/linux/rpm/expected_deps_i386?r1=237390&r2=237389&pathrev=237390
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=237390&r2=237389&pathrev=237390
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/linux/rpm/expected_deps_x86_64?r1=237390&r2=237389&pathrev=237390
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=237390&r2=237389&pathrev=237390
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/sandbox_linux.cc?r1=237390&r2=237389&pathrev=237390
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=237390&r2=237389&pathrev=237390
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/linux/debian/expected_deps?r1=237390&r2=237389&pathrev=237390

Linux sandbox: move CurrentProcessHasOpenDirectories

Move CurrentProcessHasOpenDirectories() to the Credentials class and rename
it to HasOpenDirectory().
Also add some unittests.

This is a re-land of https://codereview.chromium.org/85403011/.

BUG=312380
R=jorgelo@chromium.org, mmoss@google.com

Review URL: https://codereview.chromium.org/88243003
------------------------------------------------------------------------
Project Member Comment 21 by bugdroid1@chromium.org, Nov 27 2013
------------------------------------------------------------------------
r237518 | jln@chromium.org | 2013-11-27T05:14:55.781147Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/suid/client/setuid_sandbox_client.h?r1=237518&r2=237517&pathrev=237518
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/sandbox_linux.gypi?r1=237518&r2=237517&pathrev=237518
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/zygote/zygote_main_linux.cc?r1=237518&r2=237517&pathrev=237518
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/init_process_reaper.cc?r1=237518&r2=237517&pathrev=237518
   A http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/init_process_reaper.h?r1=237518&r2=237517&pathrev=237518
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/suid/client/setuid_sandbox_client.cc?r1=237518&r2=237517&pathrev=237518

Extract CreateInitProcessReaper() from the Zygote.

CreateInitProcessReaper() is useful for the CLONE_NEWUSER-based
sandbox as well as for the current Zygote. Extract it so that it
can be used independantly of content/.

CreateInitProcessReaper() is now exposed to content through the
setuid sandbox client.

BUG=312380

Review URL: https://codereview.chromium.org/90243002
------------------------------------------------------------------------
Project Member Comment 22 by bugdroid1@chromium.org, Nov 28 2013
------------------------------------------------------------------------
r237723 | jln@chromium.org | 2013-11-28T07:07:33.207225Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/init_process_reaper.cc?r1=237723&r2=237722&pathrev=237723

Linux: init_process_reaper comment cleanup.

Cleanup a few comments in init_process_reaper.cc

BUG=312380

Review URL: https://codereview.chromium.org/92963002
------------------------------------------------------------------------
Project Member Comment 23 by bugdroid1@chromium.org, Mar 3 2014
------------------------------------------------------------------------
r254433 | jln@chromium.org | 2014-03-03T07:50:50.124047Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials_unittest.cc?r1=254433&r2=254432&pathrev=254433
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.cc?r1=254433&r2=254432&pathrev=254433
   M http://src.chromium.org/viewvc/chrome/trunk/src/sandbox/linux/services/credentials.h?r1=254433&r2=254432&pathrev=254433

Linux Sandbox: add Credentials::SupportsNewUserNS()

We need to be able to check for unprivileged namespace support without
actually unsharing the current namespace.

We add a method that creates a new process with a new user namespace and
see if this succeeds.

BUG=312380
NOTRY=true

Review URL: https://codereview.chromium.org/182453004
------------------------------------------------------------------------
Project Member Comment 24 by bugdroid1@chromium.org, Mar 3 2014
------------------------------------------------------------------------
r254560 | jln@chromium.org | 2014-03-03T21:04:43.065560Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/zygote/zygote_main_linux.cc?r1=254560&r2=254559&pathrev=254560

Linux Sandbox: setuid sandbox initialization cleanup.

Small cleanup of the setuid sandbox initialization. This is in
preparation for the introduction of the USER_NS sandbox.

BUG=312380
NOTRY=true

Review URL: https://codereview.chromium.org/184393006
------------------------------------------------------------------------
Project Member Comment 25 by bugdroid1@chromium.org, Dec 16 2014
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d8a593bceaf0a4b38d06a8c13b948202d205f1b6

commit d8a593bceaf0a4b38d06a8c13b948202d205f1b6
Author: rickyz <rickyz@chromium.org>
Date: Tue Dec 16 02:40:18 2014

Use the libc clone wrapper in sys_clone.

Previously, we directly invoked the syscall, which would not update
libc's PID cache in the child. Although the libc wrapper function
updates the PID cache, it unfortunately requires that the child run on a
different stack, even if CLONE_VM is not specified. We work around this
by briefly switching stacks in the child, then using longjmp to switch
back. This gives us a version of clone with fork-like behavior, which is
what we need for starting processes in new namespaces.

BUG=312380

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

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

[modify] http://crrev.com/d8a593bceaf0a4b38d06a8c13b948202d205f1b6/sandbox/linux/services/syscall_wrappers.cc
[modify] http://crrev.com/d8a593bceaf0a4b38d06a8c13b948202d205f1b6/sandbox/linux/services/syscall_wrappers.h
[modify] http://crrev.com/d8a593bceaf0a4b38d06a8c13b948202d205f1b6/sandbox/linux/services/syscall_wrappers_unittest.cc

Project Member Comment 26 by bugdroid1@chromium.org, Dec 16 2014
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ddc896504d83fa943ede4732ed2c922c7e494154

commit ddc896504d83fa943ede4732ed2c922c7e494154
Author: oshima <oshima@chromium.org>
Date: Tue Dec 16 19:34:23 2014

Revert "Use the libc clone wrapper in sys_clone."

This reverts commit d8a593bceaf0a4b38d06a8c13b948202d205f1b6.
Reason for revert: see crbug.com/442817

> Previously, we directly invoked the syscall, which would not update
> libc's PID cache in the child. Although the libc wrapper function
> updates the PID cache, it unfortunately requires that the child run on a
> different stack, even if CLONE_VM is not specified. We work around this
> by briefly switching stacks in the child, then using longjmp to switch
> back. This gives us a version of clone with fork-like behavior, which is
> what we need for starting processes in new namespaces.
>
> BUG=312380
> Committed: https://crrev.com/d8a593bceaf0a4b38d06a8c13b948202d205f1b6
> Cr-Commit-Position: refs/heads/master@{#308510}

BUG=312380, 442817
TBR=rickyz@chromium.org

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

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

[modify] http://crrev.com/ddc896504d83fa943ede4732ed2c922c7e494154/sandbox/linux/services/syscall_wrappers.cc
[modify] http://crrev.com/ddc896504d83fa943ede4732ed2c922c7e494154/sandbox/linux/services/syscall_wrappers.h
[modify] http://crrev.com/ddc896504d83fa943ede4732ed2c922c7e494154/sandbox/linux/services/syscall_wrappers_unittest.cc

Project Member Comment 27 by bugdroid1@chromium.org, Dec 17 2014
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9eb564175dbd452196f782da2b28e3e8e79c49a5

commit 9eb564175dbd452196f782da2b28e3e8e79c49a5
Author: rickyz <rickyz@chromium.org>
Date: Wed Dec 17 04:33:23 2014

Add a ForkWithFlags wrapper using the libc clone wrapper.

sys_clone directly invokes the syscall, which does not update
libc's PID cache in the child. Although the libc wrapper function
updates the PID cache, it unfortunately requires that the child run on a
different stack, even if CLONE_VM is not specified. We work around this
by briefly switching stacks in the child, then using longjmp to switch
back. This gives us a version of clone with fork-like behavior, which is
what we need for starting processes in new namespaces.

This is a 2nd attempt at crrev.com/801033002, which caused failures
under valgrind and FORTIFY_SOURCE.

BUG=312380, 442817, 442912

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

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

[modify] http://crrev.com/9eb564175dbd452196f782da2b28e3e8e79c49a5/sandbox/linux/services/syscall_wrappers.cc
[modify] http://crrev.com/9eb564175dbd452196f782da2b28e3e8e79c49a5/sandbox/linux/services/syscall_wrappers.h
[modify] http://crrev.com/9eb564175dbd452196f782da2b28e3e8e79c49a5/sandbox/linux/services/syscall_wrappers_unittest.cc

Comment 28 by jln@chromium.org, Dec 17 2014
Owner: rickyz@chromium.org
Assigning to rickyz@, as per our discussion.

The current plan is to:

1. Have a working ForkWithFlags() implementation
2. When unprivileged namespaces are available in the kernel, use them instead of the setuid sandbox. (The Zygote would be launched directly in new PID and Network namespaces and would start with the CAP_SYS_CHROOT capability).

(Note: if all of Chrome is started from a chroot, then unprivileged namespaces won't be available in the kernel, regardless of the kernel version).
3. The final steps is a big boost in security: try and get every renderer to be in its own PID namespace
Project Member Comment 29 by bugdroid1@chromium.org, Dec 18 2014
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc28c34fba747ed19dfc4579590822735c004257

commit cc28c34fba747ed19dfc4579590822735c004257
Author: rickyz <rickyz@chromium.org>
Date: Thu Dec 18 05:32:47 2014

Use ForkWithFlags on the zygote.

The goal of this change is to field test the new sandbox::ForkWithFlags
function prior to getting it in base/.

BUG=312380

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

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

[modify] http://crrev.com/cc28c34fba747ed19dfc4579590822735c004257/content/zygote/zygote_linux.cc

Project Member Comment 30 by bugdroid1@chromium.org, Jan 6 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e632ac2a948f2c07b6ed45f97a30f2d3abd23d2

commit 2e632ac2a948f2c07b6ed45f97a30f2d3abd23d2
Author: rickyz <rickyz@chromium.org>
Date: Tue Jan 06 20:59:27 2015

Remove the open directory fd check.

Linux Zygote sandboxing code keeps an fd for /proc while starting the
BPF sandbox (but ensures that it is closed later). This moves the
responsibility to the caller to ensure that no directory fds are present
after sandboxing is enabled.

Also adds WARN_UNUSED_RESULT to some important functions that return a
bool indicating success or failure.

BUG=312380

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

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

[modify] http://crrev.com/2e632ac2a948f2c07b6ed45f97a30f2d3abd23d2/sandbox/linux/services/credentials.cc
[modify] http://crrev.com/2e632ac2a948f2c07b6ed45f97a30f2d3abd23d2/sandbox/linux/services/credentials.h
[modify] http://crrev.com/2e632ac2a948f2c07b6ed45f97a30f2d3abd23d2/sandbox/linux/services/credentials_unittest.cc

Project Member Comment 31 by bugdroid1@chromium.org, Jan 7 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a

commit e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a
Author: rickyz <rickyz@chromium.org>
Date: Wed Jan 07 19:10:04 2015

Move ForkWithFlags from sandbox/ to base/ and plug it into LaunchProcess.

ForkWithFlags is a wrapper around the clone syscall that uses the libc
clone wrapper, and thus updates the libc's pid cache if it has one
(using sys_clone directly does not update the pid cache, so getpid may
return an incorrect result in the child). This exposes the ability to
set clone flags, which is needed to use Linux namespaces.

BUG=312380

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

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

[modify] http://crrev.com/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a/base/process/launch.h
[modify] http://crrev.com/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a/base/process/launch_posix.cc
[modify] http://crrev.com/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a/base/process/process.h
[modify] http://crrev.com/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a/base/process/process_linux.cc
[modify] http://crrev.com/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a/base/process/process_unittest.cc
[modify] http://crrev.com/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a/base/process/process_util_unittest.cc
[modify] http://crrev.com/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a/content/zygote/zygote_linux.cc
[modify] http://crrev.com/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a/sandbox/linux/services/syscall_wrappers.cc
[modify] http://crrev.com/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a/sandbox/linux/services/syscall_wrappers.h
[modify] http://crrev.com/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a/sandbox/linux/services/syscall_wrappers_unittest.cc

Project Member Comment 32 by bugdroid1@chromium.org, Jan 7 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3e31085d2fe944c8fca2d2c06ca416469cc1942e

commit 3e31085d2fe944c8fca2d2c06ca416469cc1942e
Author: mek <mek@chromium.org>
Date: Wed Jan 07 20:33:29 2015

Revert of Move ForkWithFlags from sandbox/ to base/ and plug it into LaunchProcess. (patchset #2 id:20001 of https://codereview.chromium.org/831373002/)

Reason for revert:
PrcessTest.CloneWithFlags fails on Linux ChromiumOS Ozone Tests (1):
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Ozone%20Tests%20%281%29/builds/8139/steps/base_unittests/logs/CloneFlags

ProcessTest.CloneFlags (run #1):
[ RUN      ] ProcessTest.CloneFlags
../../base/process/process_unittest.cc:241: Failure
Value of: process.IsValid()
  Actual: false
Expected: true
[  FAILED  ] ProcessTest.CloneFlags (0 ms)

Original issue's description:
> Move ForkWithFlags from sandbox/ to base/ and plug it into LaunchProcess.
>
> ForkWithFlags is a wrapper around the clone syscall that uses the libc
> clone wrapper, and thus updates the libc's pid cache if it has one
> (using sys_clone directly does not update the pid cache, so getpid may
> return an incorrect result in the child). This exposes the ability to
> set clone flags, which is needed to use Linux namespaces.
>
> BUG=312380
>
> Committed: https://crrev.com/e12d6652ece2a3ab72bb05837cfd7f0b0b9ecf3a
> Cr-Commit-Position: refs/heads/master@{#310327}

TBR=jln@chromium.org,mdempsky@chromium.org,thestig@chromium.org,mark@chromium.org,rickyz@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=312380

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

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

[modify] http://crrev.com/3e31085d2fe944c8fca2d2c06ca416469cc1942e/base/process/launch.h
[modify] http://crrev.com/3e31085d2fe944c8fca2d2c06ca416469cc1942e/base/process/launch_posix.cc
[modify] http://crrev.com/3e31085d2fe944c8fca2d2c06ca416469cc1942e/base/process/process.h
[modify] http://crrev.com/3e31085d2fe944c8fca2d2c06ca416469cc1942e/base/process/process_linux.cc
[modify] http://crrev.com/3e31085d2fe944c8fca2d2c06ca416469cc1942e/base/process/process_unittest.cc
[modify] http://crrev.com/3e31085d2fe944c8fca2d2c06ca416469cc1942e/base/process/process_util_unittest.cc
[modify] http://crrev.com/3e31085d2fe944c8fca2d2c06ca416469cc1942e/content/zygote/zygote_linux.cc
[modify] http://crrev.com/3e31085d2fe944c8fca2d2c06ca416469cc1942e/sandbox/linux/services/syscall_wrappers.cc
[modify] http://crrev.com/3e31085d2fe944c8fca2d2c06ca416469cc1942e/sandbox/linux/services/syscall_wrappers.h
[modify] http://crrev.com/3e31085d2fe944c8fca2d2c06ca416469cc1942e/sandbox/linux/services/syscall_wrappers_unittest.cc

Project Member Comment 33 by bugdroid1@chromium.org, Jan 13 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f1eb9ccb53367a38340b05caa74769c7b492ad73

commit f1eb9ccb53367a38340b05caa74769c7b492ad73
Author: rickyz <rickyz@chromium.org>
Date: Tue Jan 13 22:59:48 2015

Move ForkWithFlags from sandbox/ to base/ and plug it into LaunchProcess.

ForkWithFlags is a wrapper around the clone syscall that uses the libc clone
wrapper, and thus updates the libc's pid cache if it has one (using sys_clone
directly does not update the pid cache, so getpid may return an incorrect
result in the child). This exposes the ability to set clone flags, which is
needed to use Linux namespaces.

BUG=312380

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

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

[modify] http://crrev.com/f1eb9ccb53367a38340b05caa74769c7b492ad73/base/process/launch.h
[modify] http://crrev.com/f1eb9ccb53367a38340b05caa74769c7b492ad73/base/process/launch_posix.cc
[modify] http://crrev.com/f1eb9ccb53367a38340b05caa74769c7b492ad73/base/process/process.h
[modify] http://crrev.com/f1eb9ccb53367a38340b05caa74769c7b492ad73/base/process/process_linux.cc
[modify] http://crrev.com/f1eb9ccb53367a38340b05caa74769c7b492ad73/base/process/process_unittest.cc
[modify] http://crrev.com/f1eb9ccb53367a38340b05caa74769c7b492ad73/base/process/process_util_unittest.cc
[modify] http://crrev.com/f1eb9ccb53367a38340b05caa74769c7b492ad73/content/zygote/zygote_linux.cc
[modify] http://crrev.com/f1eb9ccb53367a38340b05caa74769c7b492ad73/sandbox/linux/services/syscall_wrappers.cc
[modify] http://crrev.com/f1eb9ccb53367a38340b05caa74769c7b492ad73/sandbox/linux/services/syscall_wrappers.h
[modify] http://crrev.com/f1eb9ccb53367a38340b05caa74769c7b492ad73/sandbox/linux/services/syscall_wrappers_unittest.cc

Project Member Comment 34 by bugdroid1@chromium.org, Jan 16 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbe41b9dda56f3f34bdf9012c5564b5cd15058e2

commit cbe41b9dda56f3f34bdf9012c5564b5cd15058e2
Author: rickyz <rickyz@chromium.org>
Date: Fri Jan 16 00:28:25 2015

Convert DropFileSystemAccess to use ForkWithFlags.

Previously, this used a thread, but this did not work within a PID
namespace because /proc/<tid> can refer to a completely different
process.

BUG=312380

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

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

[modify] http://crrev.com/cbe41b9dda56f3f34bdf9012c5564b5cd15058e2/sandbox/linux/services/credentials.cc
[modify] http://crrev.com/cbe41b9dda56f3f34bdf9012c5564b5cd15058e2/sandbox/linux/services/credentials_unittest.cc

Project Member Comment 35 by bugdroid1@chromium.org, Jan 16 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0b860bf256bc5c847eaa0533e01736c2844e771

commit a0b860bf256bc5c847eaa0533e01736c2844e771
Author: rickyz <rickyz@chromium.org>
Date: Fri Jan 16 18:19:34 2015

Add the ability to run a callback between fork and exec.

This will be used along with user namespaces allow blocking the child
from execing until the uid and gid map has been written.

BUG=312380

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

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

[modify] http://crrev.com/a0b860bf256bc5c847eaa0533e01736c2844e771/base/process/launch.cc
[modify] http://crrev.com/a0b860bf256bc5c847eaa0533e01736c2844e771/base/process/launch.h
[modify] http://crrev.com/a0b860bf256bc5c847eaa0533e01736c2844e771/base/process/launch_posix.cc
[modify] http://crrev.com/a0b860bf256bc5c847eaa0533e01736c2844e771/base/process/process_unittest.cc

Project Member Comment 36 by bugdroid1@chromium.org, Jan 23 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1753adb1e43060ec322880f9a7bf9b483bac0ace

commit 1753adb1e43060ec322880f9a7bf9b483bac0ace
Author: jln <jln@chromium.org>
Date: Fri Jan 23 01:25:28 2015

Linux sandbox: Make ChrootToSafeEmptyDir() faster.

Use a vfork()-like system call instead of fork() in ChrootToSafeEmptyDir()
to avoid duplicating page tables, which is slow.

BUG=312380

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

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

[modify] http://crrev.com/1753adb1e43060ec322880f9a7bf9b483bac0ace/sandbox/linux/services/credentials.cc
[modify] http://crrev.com/1753adb1e43060ec322880f9a7bf9b483bac0ace/sandbox/linux/services/credentials_unittest.cc

Project Member Comment 37 by bugdroid1@chromium.org, Jan 27 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/41fb1457a6a221b5417608154bbdc7d433520f0d

commit 41fb1457a6a221b5417608154bbdc7d433520f0d
Author: rickyz <rickyz@chromium.org>
Date: Tue Jan 27 03:57:58 2015

Move a couple of utility functions to a new namespace_utils class.

The implementations are minorly different - uses /proc/self/ns/* files
to detect namespaces support, and always make sure to write uid_map in a
single write.

BUG=312380

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

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

[modify] http://crrev.com/41fb1457a6a221b5417608154bbdc7d433520f0d/sandbox/linux/BUILD.gn
[modify] http://crrev.com/41fb1457a6a221b5417608154bbdc7d433520f0d/sandbox/linux/sandbox_linux.gypi
[modify] http://crrev.com/41fb1457a6a221b5417608154bbdc7d433520f0d/sandbox/linux/sandbox_linux_test_sources.gypi
[modify] http://crrev.com/41fb1457a6a221b5417608154bbdc7d433520f0d/sandbox/linux/services/credentials.cc
[modify] http://crrev.com/41fb1457a6a221b5417608154bbdc7d433520f0d/sandbox/linux/services/credentials.h
[modify] http://crrev.com/41fb1457a6a221b5417608154bbdc7d433520f0d/sandbox/linux/services/credentials_unittest.cc
[add] http://crrev.com/41fb1457a6a221b5417608154bbdc7d433520f0d/sandbox/linux/services/namespace_utils.cc
[add] http://crrev.com/41fb1457a6a221b5417608154bbdc7d433520f0d/sandbox/linux/services/namespace_utils.h
[add] http://crrev.com/41fb1457a6a221b5417608154bbdc7d433520f0d/sandbox/linux/services/namespace_utils_unittest.cc

Project Member Comment 38 by bugdroid1@chromium.org, Feb 3 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f235daf6d29e05f8a1949864da493e910a3ddd1

commit 8f235daf6d29e05f8a1949864da493e910a3ddd1
Author: rickyz <rickyz@chromium.org>
Date: Tue Feb 03 07:26:19 2015

Add namespace sandbox class.

BUG=312380

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

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

[modify] http://crrev.com/8f235daf6d29e05f8a1949864da493e910a3ddd1/sandbox/linux/BUILD.gn
[modify] http://crrev.com/8f235daf6d29e05f8a1949864da493e910a3ddd1/sandbox/linux/sandbox_linux.gypi
[modify] http://crrev.com/8f235daf6d29e05f8a1949864da493e910a3ddd1/sandbox/linux/sandbox_linux_test_sources.gypi
[add] http://crrev.com/8f235daf6d29e05f8a1949864da493e910a3ddd1/sandbox/linux/services/namespace_sandbox.cc
[add] http://crrev.com/8f235daf6d29e05f8a1949864da493e910a3ddd1/sandbox/linux/services/namespace_sandbox.h
[add] http://crrev.com/8f235daf6d29e05f8a1949864da493e910a3ddd1/sandbox/linux/services/namespace_sandbox_unittest.cc
[modify] http://crrev.com/8f235daf6d29e05f8a1949864da493e910a3ddd1/sandbox/linux/services/namespace_utils.cc
[modify] http://crrev.com/8f235daf6d29e05f8a1949864da493e910a3ddd1/sandbox/linux/services/namespace_utils.h
[modify] http://crrev.com/8f235daf6d29e05f8a1949864da493e910a3ddd1/sandbox/linux/services/namespace_utils_unittest.cc
[modify] http://crrev.com/8f235daf6d29e05f8a1949864da493e910a3ddd1/sandbox/linux/tests/main.cc

Project Member Comment 39 by bugdroid1@chromium.org, Feb 6 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ef7ee1ee899087d8632a4f905a3a11697a20b81

commit 1ef7ee1ee899087d8632a4f905a3a11697a20b81
Author: rickyz <rickyz@chromium.org>
Date: Fri Feb 06 21:54:01 2015

Add namespace sandbox to about page.

This unindents the lines about PID/network namespaces, since those now
apply to both the setuid or unprivileged namespace sandbox.

BUG=312380

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

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

[modify] http://crrev.com/1ef7ee1ee899087d8632a4f905a3a11697a20b81/chrome/app/generated_resources.grd
[modify] http://crrev.com/1ef7ee1ee899087d8632a4f905a3a11697a20b81/chrome/browser/ui/webui/about_ui.cc
[modify] http://crrev.com/1ef7ee1ee899087d8632a4f905a3a11697a20b81/content/common/sandbox_linux/sandbox_linux.cc
[modify] http://crrev.com/1ef7ee1ee899087d8632a4f905a3a11697a20b81/content/public/common/sandbox_linux.h

Project Member Comment 40 by bugdroid1@chromium.org, Feb 7 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/660e2d468bf22b4c315463173d0651af4fe6143f

commit 660e2d468bf22b4c315463173d0651af4fe6143f
Author: rickyz <rickyz@chromium.org>
Date: Sat Feb 07 03:51:41 2015

Allow using the namespace sandbox in zygote host.

Currently, this is gated behind the enable-namespace-sandbox switch.
Furthermore, the namespace sandbox is only used if seccomp-bpf is
supported.

BUG=312380

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

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

[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/chrome/installer/linux/debian/expected_deps_ia32
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/chrome/installer/linux/debian/expected_deps_x64
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/chrome/installer/linux/rpm/expected_deps_i386
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/chrome/installer/linux/rpm/expected_deps_x86_64
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/components/nacl.gyp
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/components/nacl/loader/sandbox_linux/nacl_sandbox_linux.cc
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/components/nacl/zygote/nacl_fork_delegate_linux.cc
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/content/browser/zygote_host/zygote_host_impl_linux.cc
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/content/browser/zygote_host/zygote_host_impl_linux.h
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/content/public/common/content_switches.cc
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/content/public/common/content_switches.h
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/content/zygote/zygote_main_linux.cc
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/sandbox/linux/services/namespace_sandbox.cc
[modify] http://crrev.com/660e2d468bf22b4c315463173d0651af4fe6143f/sandbox/linux/services/namespace_sandbox.h

Project Member Comment 41 by bugdroid1@chromium.org, Feb 10 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2037cb15035e20d0ff63e2b8ec5cc94379d2063f

commit 2037cb15035e20d0ff63e2b8ec5cc94379d2063f
Author: rickyz <rickyz@chromium.org>
Date: Tue Feb 10 00:10:34 2015

Add extra check that the namespace sandbox is engaged.

BUG=312380

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

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

[modify] http://crrev.com/2037cb15035e20d0ff63e2b8ec5cc94379d2063f/content/zygote/zygote_main_linux.cc

Comment 42 by jln@chromium.org, Feb 10 2015
Blocking: chromium:457066
Not sure whether anyone has already noticed, but since the following commit:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=be7c6dba2332cef0677fbabb606e279ae76652c3

It will cause a failure of (unprivileged) writing the gid_map (and thus the gid is going to be set to nogroup/65534) and in the current implementation will also cause a failure of the CHECK().

The mentioned change has already been backported to 3.18.2, but haven't checked whether earlier versions are affected.

Not sure however whether removing the CHECK() plus the checks in GetRESIds() is harmful if it comes to security. I think it's ugly but as long as it maps to an unprivileged group it shouldn't be an issue, right?
Thanks for the heads up! I had heard about the bug with groups that restrict permissions, but I didn't realize that the fix was to reqiure CAP_SETGID for writing to gid_map.

From what I can tell, there is no serious security downsides to not writing the uid or gid mapping, since we drop filesystem access pretty soon after. However, we currently rely on nested user namespaces, which requires that there are valid uid/gid mappings in the parent user namespace: http://lxr.free-electrons.com/source/kernel/user_namespace.c#L77

We need nested user namespaces because capabilities are not inherited across execve (unless the right capability inheritance bit is set on the file being execved), and we need CAP_SYS_CHROOT after execve in order to drop filesystem access. http://lwn.net/Articles/631498/ will remove our need for nested user namespaces when it lands, but until then, this means that the current user namespace code is broken on newer kernels.

One workaround that I know of is to use a uid 0 mapping when we create a new user namespace. Then we can keep CAP_SYS_CHROOT across execve without having to enter a nested user namespace. I'll have to read through the code more carefully to see whether some combination of PR_SET_SECUREBITS or PR_CAPBSET_DROP is sufficient to prevent this from leading to additional privileges in the sandboxed process.
Comment 45 by j...@panix.com, Feb 10 2015
Should Credentials::DropAllCapabilities assert that the process is single-threaded, somehow?  Unless I'm missing something, capset(2)/cap_set_proc(3) will affect only the calling thread, as usual for Linux features that operate on task credentials.

Also, I'm a little unclear on why the "ChrootMe" test passes, currently -- execve should clear the effective and permitted capability sets, unless the caller is uid 0 or there are capabilities in the intersection of the process's inherited set and the exec'ed file's inherited set (both of which are normally empty).
Hi, glad to have some more eyes on this code :-) The assert sounds like a good suggestion (though in case we ever exlicitly want to drop capabilities on a specific thread, maybe we should just make sure to document this clearly and have callers check for single threadedness as necessary - jln@ actually just added a convenient function to do this).

The ChrootMe test obtains CAP_SYS_CHROOT by entering a nested user namespace:
https://code.google.com/p/chromium/codesearch#chromium/src/sandbox/linux/services/namespace_sandbox_unittest.cc&l=87
Project Member Comment 47 by bugdroid1@chromium.org, Feb 10 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c2ebd2f01adde40009316a641c10de05e653b1a

commit 4c2ebd2f01adde40009316a641c10de05e653b1a
Author: rickyz <rickyz@chromium.org>
Date: Tue Feb 10 10:35:26 2015

Revert "Add extra check that the namespace sandbox is engaged."

Reason for revert:
See crbug/456993.

This reverts commit 2037cb15035e20d0ff63e2b8ec5cc94379d2063f.

TBR=jln
BUG=456993
Original issue's description:
> Add extra check that the namespace sandbox is engaged.
>
> BUG=312380
> Committed: https://crrev.com/2037cb15035e20d0ff63e2b8ec5cc94379d2063f
> Cr-Commit-Position: refs/heads/master@{#315446}

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

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

[modify] http://crrev.com/4c2ebd2f01adde40009316a641c10de05e653b1a/content/zygote/zygote_main_linux.cc

Project Member Comment 48 by bugdroid1@chromium.org, Feb 10 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/30c2c5d06c4cb04fda4985481155c5fc9c36d846

commit 30c2c5d06c4cb04fda4985481155c5fc9c36d846
Author: rickyz <rickyz@chromium.org>
Date: Tue Feb 10 11:58:37 2015

Revert "Add namespace sandbox to about page."

Reason for revert:
See crbug/456993.

This reverts commit 1ef7ee1ee899087d8632a4f905a3a11697a20b81.

TBR=jln,jhawkins,nasko,mdempsky
BUG=456993
Original issue's description:
> Add namespace sandbox to about page.
>
> This unindents the lines about PID/network namespaces, since those now
> apply to both the setuid or unprivileged namespace sandbox.
>
> BUG=312380
> Committed: https://crrev.com/1ef7ee1ee899087d8632a4f905a3a11697a20b81
> Cr-Commit-Position: refs/heads/master@{#315116}

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

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

[modify] http://crrev.com/30c2c5d06c4cb04fda4985481155c5fc9c36d846/chrome/app/generated_resources.grd
[modify] http://crrev.com/30c2c5d06c4cb04fda4985481155c5fc9c36d846/chrome/browser/ui/webui/about_ui.cc
[modify] http://crrev.com/30c2c5d06c4cb04fda4985481155c5fc9c36d846/content/common/sandbox_linux/sandbox_linux.cc
[modify] http://crrev.com/30c2c5d06c4cb04fda4985481155c5fc9c36d846/content/public/common/sandbox_linux.h

Ah, I had the same issue with nested user namespaces while doing my implementation.

So if at one point all capabilities are dropped even when the UID is 0 within the namespace, it shouldn't be possible to break out of the chroot, or did I miss something?
Here's a quick & dirty POC to illustrate what I mean:

https://gist.github.com/aszlig/d14a279b9f14759e5a69

Do you see any problems with that approach (well, except that I'm using libcap-ng)? I mean with that the inner uid 0 shouldn't have any more privileges than any uid > 0.
Also, if the seccomp BPF sandbox is active, the user shouldn't be able to break out as well, because it can't use chroot(2) or fchdir(2).

That's what I was relying on in my implementation prior to Chromium version 42:

https://github.com/NixOS/nixpkgs/blob/7a6af47cca7000d766873900efc211169d83b101/pkgs/applications/networking/browsers/chromium/source/sandbox_userns_36.patch
Comment 52 by jln@chromium.org, Feb 10 2015
#45: Yeah good point, we should assert being single threaded!

#49: yes, correct once capabilities are dropped we don't think it's possible to escape the chroot(). (Minus the classic issues, such as having directory file descriptors open).
We test that the kernel doesn't regress for this here: https://code.google.com/p/chromium/codesearch#chromium/src/sandbox/linux/services/credentials_unittest.cc&l=149
Comment 53 by jln@chromium.org, Feb 10 2015
#51: we try to have the layer-1 sandbox (setuid, userns) be self sufficient as much as we can.

One reason to set the uid mapping is that code could break / assume privileges if uid is 0 or different from the "outside" uid.

I am less concerned about gid, so I think we could accept failure when writing the gid map. Ricky, WDYT?
Comment 54 by jln@chromium.org, Feb 10 2015
I created issue 457362 to deal with restrictions to writing the gid_map.

Given that writing to the gid_map has now been restored, we should use this new facility.

Thanks neusepoff for the report!
Comment 55 by jln@chromium.org, Feb 10 2015
Blockedon: chromium:457362
Comment 56 by j...@panix.com, Feb 10 2015
#46: Thanks; I missed that the first time around.

#53: You might have noticed this already (and I didn't notice it until just now, which is why I was confused about the namespace nesting), but writing the gid_map is necessary for creating a nested user namespace --- the calling process's uid and gid have to be mapped into its current user namespace in order create a new one.  This is documented in the unshare(2) man page and not in the clone(2) man page, but it appears to apply to both syscalls.

So this definitely needs issue 457362 (or else making the sandboxed process have subjective uid 0 and not nesting namespaces).
Comment 57 by jln@chromium.org, Feb 10 2015
Blockedon: chromium:457377
Comment 58 by jln@chromium.org, Feb 10 2015
I created issue 457377 to consider adding assertions in DropCapabilities() for being single threaded rather than having the caller be responsible, as reported by jld@.
Comment 59 by rickyz@google.com, Feb 10 2015
Yeah, that's exactly the issue I was mentioning about how this breaks our nested namespaces. jln@ pointed out that the fix for reallowing gid_map writing appeared the same day as that commit, so what we should probably do is just unset USERNS_SETGROUPS_ALLOWED via /proc/self/setgroups if it exists before entering a user namespace.
Project Member Comment 60 by bugdroid1@chromium.org, Feb 12 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c72852c0917f4b4c0b457c41897e79d8c99ab209

commit c72852c0917f4b4c0b457c41897e79d8c99ab209
Author: rickyz <rickyz@chromium.org>
Date: Thu Feb 12 02:13:35 2015

Add namespace sandbox status to LinuxSandbox::GetStatus.

Also add an extra check that the namespace sandbox is enabled in the zygote.

BUG=312380
TBR=nasko@chromium.org

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

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

[modify] http://crrev.com/c72852c0917f4b4c0b457c41897e79d8c99ab209/content/common/sandbox_linux/sandbox_linux.cc
[modify] http://crrev.com/c72852c0917f4b4c0b457c41897e79d8c99ab209/content/public/common/sandbox_linux.h
[modify] http://crrev.com/c72852c0917f4b4c0b457c41897e79d8c99ab209/content/zygote/zygote_main_linux.cc

Project Member Comment 61 by bugdroid1@chromium.org, Feb 12 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b94f6817d3a0e20ec5c3393a4eb13dd360acbd4e

commit b94f6817d3a0e20ec5c3393a4eb13dd360acbd4e
Author: jln <jln@chromium.org>
Date: Thu Feb 12 04:53:04 2015

Namespace sandbox: add important security checks

When engaging the namespace sandbox, add important checks that the process
is single threaded and has no directory file descriptor open.

As part of this change, move the function engaging the namespace
sandbox from the Zygote to the LinuxSandbox class.

BUG=457377, 312380

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

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

[modify] http://crrev.com/b94f6817d3a0e20ec5c3393a4eb13dd360acbd4e/components/nacl/loader/sandbox_linux/nacl_sandbox_linux.cc
[add] http://crrev.com/b94f6817d3a0e20ec5c3393a4eb13dd360acbd4e/content/common/sandbox_linux/sandbox_debug_handling_linux.cc
[add] http://crrev.com/b94f6817d3a0e20ec5c3393a4eb13dd360acbd4e/content/common/sandbox_linux/sandbox_debug_handling_linux.h
[modify] http://crrev.com/b94f6817d3a0e20ec5c3393a4eb13dd360acbd4e/content/common/sandbox_linux/sandbox_linux.cc
[modify] http://crrev.com/b94f6817d3a0e20ec5c3393a4eb13dd360acbd4e/content/common/sandbox_linux/sandbox_linux.h
[modify] http://crrev.com/b94f6817d3a0e20ec5c3393a4eb13dd360acbd4e/content/content_common.gypi
[modify] http://crrev.com/b94f6817d3a0e20ec5c3393a4eb13dd360acbd4e/content/zygote/zygote_main_linux.cc
[modify] http://crrev.com/b94f6817d3a0e20ec5c3393a4eb13dd360acbd4e/sandbox/linux/services/credentials.h

Project Member Comment 62 by bugdroid1@chromium.org, Feb 12 2015
Project  : chromiumos/third_party/autotest
Branch   : master
Author   : Ricky Zhou <rickyz@chromium.org>
Committer: ChromeOS Commit Bot <chromeos-commit-bot@chromium.org>
Commit   : 669393837af4367f7f655d0b5bbf70915a6e1152

Code-Review  0 : ChromeOS Commit Bot, Julien Tinnes, Ricky Zhou
Code-Review  +1: Matthew Dempsky
Code-Review  +2: Jorge Lucangeli Obes
Commit-Queue 0 : ChromeOS Commit Bot, Jorge Lucangeli Obes, Julien Tinnes, Matthew Dempsky
Commit-Queue +1: Ricky Zhou
Verified     0 : ChromeOS Commit Bot, Jorge Lucangeli Obes, Julien Tinnes, Matthew Dempsky
Verified     +1: Ricky Zhou
Commit Queue   : Chumped
Change-Id      : If7034dd158d63564d5179c4e96be51812baf9058
Reviewed-at    : https://chromium-review.googlesource.com/248931

Test that chrome thinks we are adequately sandboxed.

This moves the decision making about whether the sandboxing is
sufficient to chromium, which should be more future-proof.

BUG=chromium:312380
TEST=Tested on daisy with and without https://codereview.chromium.org/916153003/.

client/site_tests/security_SandboxStatus/security_SandboxStatus.py
Project Member Comment 63 by bugdroid1@chromium.org, Feb 12 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e1274d29f116c9eba6bc3d16d2b5d53a77b99a9a

commit e1274d29f116c9eba6bc3d16d2b5d53a77b99a9a
Author: rickyz <rickyz@chromium.org>
Date: Thu Feb 12 08:30:56 2015

Add namespace sandbox to about page.

This unindents the lines about PID/network namespaces, since those now apply to both the setuid or unprivileged namespace sandbox.

This is a resubmit of https://codereview.chromium.org/873283004/

TBR=jhawkins@chromium.org
BUG=312380

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

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

[modify] http://crrev.com/e1274d29f116c9eba6bc3d16d2b5d53a77b99a9a/chrome/app/generated_resources.grd
[modify] http://crrev.com/e1274d29f116c9eba6bc3d16d2b5d53a77b99a9a/chrome/browser/ui/webui/about_ui.cc
[modify] http://crrev.com/e1274d29f116c9eba6bc3d16d2b5d53a77b99a9a/chrome/test/data/webui/sandboxstatus_browsertest.js

Project Member Comment 64 by bugdroid1@chromium.org, Feb 13 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21750b1dac21f0d534f218c28fca8d588885f90a

commit 21750b1dac21f0d534f218c28fca8d588885f90a
Author: rickyz <rickyz@chromium.org>
Date: Fri Feb 13 08:49:18 2015

Default to enabling the namespace sandbox when possible.

This can be disabled using the --disable-namespace-sandbox flag.

BUG=312380

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

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

[modify] http://crrev.com/21750b1dac21f0d534f218c28fca8d588885f90a/content/browser/zygote_host/zygote_host_impl_linux.cc
[modify] http://crrev.com/21750b1dac21f0d534f218c28fca8d588885f90a/content/public/common/content_switches.cc
[modify] http://crrev.com/21750b1dac21f0d534f218c28fca8d588885f90a/content/public/common/content_switches.h

Haven't looked into this yet, because I only have a slow and very unstable connection, but launch_posix.cc prints errors about execvp() failing, see here:

https://headcounter.org/hydra/build/640185/log#line-1426

But apart from that, the browser seems to be working at least with simple pages, like:

https://headcounter.org/hydra/build/640185/download/1/dev_startup_done.png
https://headcounter.org/hydra/build/640185/download/1/dev_emptywin.png
https://headcounter.org/hydra/build/640185/download/1/dev_sandbox.png

These are from automated tests, because - as said - I can't test it locally right now :-/

Full test log is available here (of all channels, not just the dev channel):

https://headcounter.org/hydra/build/640185/download/1/log.html

Build log is available here:

https://headcounter.org/hydra/build/640187/nixlog/2/raw

Other information, not sure if it's helpful, but in case you want the gory details about the build itself:

Patch used is this one:

https://github.com/aszlig/nixpkgs/blob/4f67733edfd69abe84baa5f705fdd4ddc2869164/pkgs/applications/networking/browsers/chromium/source/nix_plugin_paths_42.patch

And the following is patched using sed:

https://github.com/aszlig/nixpkgs/blob/4f67733edfd69abe84baa5f705fdd4ddc2869164/pkgs/applications/networking/browsers/chromium/source/default.nix#L70-L81

The main build expression:
https://github.com/aszlig/nixpkgs/blob/4f67733edfd69abe84baa5f705fdd4ddc2869164/pkgs/applications/networking/browsers/chromium/common.nix
okay, never mind, this was because of me patching out the suid sandbox error without the execution of the process itself. sorry for the noise.
Comment 67 by jln@chromium.org, Feb 23 2015
Blocking: chromium:460972
Comment 68 by jln@chromium.org, Feb 23 2015
Labels: M-42
neusepoff: you can run without the setuid sandbox if you use --allow-sandbox-debugging (but yeah, as you noted, you still need to patch-out the check of "existence").

With issue 460972, we should make it no longer necessary to pass the (scary, unsupported) --allow-sandbox-debugging flag.
Comment 69 by jln@chromium.org, Feb 23 2015
Blockedon: chromium:459724
Project Member Comment 70 by bugdroid1@chromium.org, Feb 24 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4d91216184b506a9f0a623919862250f65d4f3e4

commit 4d91216184b506a9f0a623919862250f65d4f3e4
Author: jln <jln@chromium.org>
Date: Tue Feb 24 05:54:26 2015

Linux sandbox: better APIs with /proc/ arguments

Unify sandbox:: APIs to always take /proc/ file descriptors
instead of /proc/self/ or /proc/self/task/.

Moreover, require |proc_fd| arguments to critical APIs rather
than rely on the caller to perform the right checks.

A descriptor to /proc is a better choice than a descriptor to
/proc/self/* because it keeps the same semantics after a fork().

BUG=312380, 457377
TBR=nasko

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

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

[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/components/nacl/loader/nonsfi/nonsfi_sandbox.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/components/nacl/loader/nonsfi/nonsfi_sandbox.h
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/components/nacl/loader/sandbox_linux/nacl_bpf_sandbox_linux.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/components/nacl/loader/sandbox_linux/nacl_bpf_sandbox_linux.h
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/components/nacl/loader/sandbox_linux/nacl_sandbox_linux.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/content/common/sandbox_linux/sandbox_init_linux.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/content/common/sandbox_linux/sandbox_linux.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/content/common/sandbox_linux/sandbox_seccomp_bpf_linux.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/content/common/sandbox_linux/sandbox_seccomp_bpf_linux.h
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/content/public/common/sandbox_init.h
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/seccomp-bpf/sandbox_bpf.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/seccomp-bpf/sandbox_bpf.h
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/seccomp-bpf/sandbox_bpf_unittest.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/services/credentials.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/services/credentials.h
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/services/credentials_unittest.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/services/namespace_sandbox_unittest.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/services/proc_util.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/services/proc_util.h
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/services/proc_util_unittest.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/services/thread_helpers.cc
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/services/thread_helpers.h
[modify] http://crrev.com/4d91216184b506a9f0a623919862250f65d4f3e4/sandbox/linux/services/thread_helpers_unittests.cc

Labels: -M-42 M-43 MovedFrom-42
[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
Can this be marked Fixed?
Comment 73 by jln@chromium.org, Mar 24 2015
Blockedon: chromium:460972
The setuid sandbox is still required as a setuid helper binary because renderers need to be non-dumpable.

Once issue 460972 is implemented (one PID namespace per renderer), we should be able to remove this and run entirely without the setuid sandbox present.
Comment 74 by jln@chromium.org, Mar 24 2015
Blocking: -chromium:460972
Comment 75 by jln@chromium.org, Mar 24 2015
This is still targeted to M-43. If we do fail to have one PID NS per renderer in M-43, we can still try and remove non-dumpability for the cases where Yama is running for instance.

This should allow us to unblock issue 457066 in any case.
Yeah, we should remove the requirement on Chrome OS at least.
Comment 77 by jln@chromium.org, Mar 27 2015
There is a little bit of complexity in getting this to work with NaCl: we need to install signal handlers that NaCl won't let us do.

Because of this, given the schedule, let's postpone to M-44. We can discuss in issue 457066 whether or not that's ok or if we should remove non-dumpability before this issue is resolved.
Feel free to postpone.
Labels: -M-43 MovedFrom-43
[AUTO] This issue has already been moved once and is lower than Priority 1,therefore removing mstone.
Project Member Comment 80 by bugdroid1@chromium.org, Jun 18 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3cf743a247fbd50936c0eaf395c2b1345813661b

commit 3cf743a247fbd50936c0eaf395c2b1345813661b
Author: mostynb <mostynb@opera.com>
Date: Thu Jun 18 21:12:52 2015

ifdef out ProcessUtilTest.CloneFlags on linux with old kernel headers

This allows the rest of base_unittests to build/run on such
systems.

BUG=312380

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

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

[modify] http://crrev.com/3cf743a247fbd50936c0eaf395c2b1345813661b/base/process/process_util_unittest.cc

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

commit d372b27a99e6827fd87a95bc802431ee83b81c08
Author: mostynb <mostynb@opera.com>
Date: Wed Nov 18 02:10:57 2015

don't try to adjust oom score with the suid sandbox if there is no such binary

BUG=312380

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

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

[modify] http://crrev.com/d372b27a99e6827fd87a95bc802431ee83b81c08/content/browser/zygote_host/zygote_host_impl_linux.cc

Comment 82 by jln@chromium.org, Dec 11 2015
As discussed off-thread, it looks like we may be able to finally kill the setuid sandbox as Chrome OS kernel are new enough.
> it looks like we may be able to finally kill the setuid sandbox as Chrome OS kernel are new enough.

Will non-ChromeOS Linux also be affected? Note that Arch Linux (and possibly others) explicitly disables unprivileged CLONE_NEWUSER in their kernel builds.

https://bugs.archlinux.org/task/36969

And on Debian-based distros, unprivileged CLONE_NEWUSER usually must be enabled by sysctl before it can be used (unless the default changed recently, which is possible).

But I for one would love to see Chrome drop the setuid sandbox as it would (presumably?) allow me to run Chrome itself inside of a userns. So I'm not objecting, just curious.
Comment 84 by jln@chromium.org, Dec 11 2015
Cc: kerrnel@chromium.org
Thanks for the info. I didn't realize that some major distros disabled it by default. Ubuntu (Debian based though) seems to enable it by default.

That's something we'll need to discuss a little more. The setuid sandbox adds a lot of complexity and cruft. Removing it from Chrome OS only wouldn't be that useful.

I don't think we should silently disable the first layer of sandboxing, and requiring users to enable CLONE_NEWUSER doesn't seem ideal either, but I'm erring on the latter (and allowing users to pass a command-line flag instead if the want to live dangerously).
Comment 85 by ngomp...@gmail.com, Dec 11 2015
It might be a good idea to talk to the major distros (specifically their Chromium packagers) about this particular issue. That will let you guys get a better sense of things.

From the Fedora side, I *think* userns stuff is controlled through SELinux somehow, because I believe it's not disabled (it's definitely enabled as of RHEL/CentOS 7.2, that's for sure).


Project Member Comment 86 by bugdroid1@chromium.org, Dec 16 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/7f88e66f9040b85d5de15219b550e1f55930931d

commit 7f88e66f9040b85d5de15219b550e1f55930931d
Author: Ricky Zhou <rickyz@chromium.org>
Date: Tue Dec 15 01:32:28 2015

Require USER_NS for Chrome's layer 1 sandbox.

BUG=chromium:312380
TEST=Tested that this passes on an amd64-generic VM.

Change-Id: I34b987963b537ae584188d4060c4e5b726db9f3e
Reviewed-on: https://chromium-review.googlesource.com/318048
Commit-Ready: Ricky Zhou <rickyz@chromium.org>
Tested-by: Ricky Zhou <rickyz@chromium.org>
Reviewed-by: Kees Cook <keescook@chromium.org>

[modify] http://crrev.com/7f88e66f9040b85d5de15219b550e1f55930931d/client/site_tests/kernel_ConfigVerify/kernel_ConfigVerify.py

Project Member Comment 87 by bugdroid1@chromium.org, Jan 5 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/baf4d84696ebd6088363888239c3aef5f5a735f7

commit baf4d84696ebd6088363888239c3aef5f5a735f7
Author: rickyz <rickyz@chromium.org>
Date: Tue Jan 05 22:39:31 2016

Do not use suid helper when using the namespace sandbox.

After https://codereview.chromium.org/1519753002, sandboxed processes
are no longer marked non-dumpable under the namespace sandbox. This
means that we no longer need to use a suid binary to change oom scores
for those processes.

BUG=312380

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

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

[modify] http://crrev.com/baf4d84696ebd6088363888239c3aef5f5a735f7/content/browser/zygote_host/zygote_host_impl_linux.cc

Blockedon: chromium:576409
Cc: dpranke@chromium.org
dpranke points out desktop Linux Chromium complains about lack of a chrome-sandbox binary.  My understanding of the current state of things (including issue 576409) is that we should only need chrome-sandbox on CrOS, so the fatal errors he saw shouldn't be triggering.
Cc: -jorgelo@chromium.org
I filed bug 598454 to turn off the check on desktop.
Owner: ----
Status: Untriaged
Sign in to add a comment