New issue
Advanced search Search tips

Issue 738451 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 730623



Sign in to add a comment

Use Minijail in chaps

Project Member Reported by jorgelo@chromium.org, Jun 30 2017

Issue description

Chaps has "bespoke" priv-dropping code, which makes it harder to add further sandboxing (and also had a bug where priv-dropping failures were not being checked). Replace with Minijail (and at least add no_new_privs).
 
Blocking: 730623
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/df39a3d77853a656912ccdea0a53c18a70ed06fc

commit df39a3d77853a656912ccdea0a53c18a70ed06fc
Author: Jorge Lucangeli Obes <jorgelo@chromium.org>
Date: Fri Jun 30 23:02:49 2017

chaps: Depend on Minijail.

BUG= chromium:738451 
TEST=emerge-kevin chaps

Change-Id: Id1a355b2fdb404a71ca73abb421cc13b589de9fb
Reviewed-on: https://chromium-review.googlesource.com/558384
Commit-Ready: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/df39a3d77853a656912ccdea0a53c18a70ed06fc/chromeos-base/chaps/chaps-9999.ebuild

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/8e4499a20dc99684b4300182e4bedc4804990f43

commit 8e4499a20dc99684b4300182e4bedc4804990f43
Author: Jorge Lucangeli Obes <jorgelo@chromium.org>
Date: Fri Jun 30 23:02:53 2017

chaps: Move SetProcessUserAndGroup() to chapsd.cc.

-In preparation for using Minijail in Chaps, move the priv drop
function away from chaps_utility.cc so that we don't have to link
libminijail to every compilation unit.

-Remove the |real| parameter in the function since it's always
set to true.

-Finally, check the return value of the function. The function does not
abort on failure so there was a bug where we would not drop privileges
but continue running as root.

BUG= chromium:738451 
TEST=trybot

Change-Id: I2b3b7c7929a5721d55a222546bce6e0f42f66ef8
Reviewed-on: https://chromium-review.googlesource.com/558267
Commit-Ready: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/8e4499a20dc99684b4300182e4bedc4804990f43/chaps/chapsd.cc
[modify] https://crrev.com/8e4499a20dc99684b4300182e4bedc4804990f43/chaps/chaps_utility.cc
[modify] https://crrev.com/8e4499a20dc99684b4300182e4bedc4804990f43/chaps/chaps_utility.h

Components: OS>Systems
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2814d157528022b321445ef2c73482b5bbc2855e

commit 2814d157528022b321445ef2c73482b5bbc2855e
Author: Jorge Lucangeli Obes <jorgelo@chromium.org>
Date: Thu Jul 06 04:54:56 2017

chaps: Use Minijail for priv dropping.

Now that we're using Minijail, we can also add no_new_privs since Chaps
shouldn't be elevating privs at all.

Also, Minijail will abort if priv-dropping fails so we no longer need
to check return values.

BUG= chromium:738451 
TEST=Build, deploy, Chaps starts.
TEST=cat /proc/`pgrep chaps`/status
Name:    chapsd
Uid:    223    223    223    223
Gid:    1001    1001    1001    1001
...
CapInh:    0000000000000000
CapPrm:    0000000000000000
CapEff:    0000000000000000
CapBnd:    0000003fffffffff
CapAmb:    0000000000000000
NoNewPrivs:    1

TEST=Trybot.
CQ-DEPEND=CL:558384

Change-Id: I433585519a13e39f8b882e830ff0a1012acecd16
Reviewed-on: https://chromium-review.googlesource.com/559729
Commit-Ready: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/2814d157528022b321445ef2c73482b5bbc2855e/chaps/chaps.gyp
[modify] https://crrev.com/2814d157528022b321445ef2c73482b5bbc2855e/chaps/chapsd.cc

Status: Fixed (was: Untriaged)

Comment 7 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 8 by vapier@chromium.org, Jun 21 2018

Status: Fixed (was: Archived)
Summary: Use Minijail in chaps (was: Use MInijail in chaps)

Sign in to add a comment