New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 680859 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner: ----
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

need method for setting up device nodes

Project Member Reported by vapier@chromium.org, Jan 13 2017

Issue description

we set up /dev as a tmpfs mount point, but those aren't processed until we enter the minijail to execute the program.  that means any device nodes we add and then try to create earlier in container_start() (via container_create_device) will be run against the read-only rootfs and thus fail.

we need to extend the minijail API to include creating device nodes, then have the container queue them up when processing the device nodes, then have minijail create all of them in minijail_enter() after it's entered the chroot/pivoted (since that's where mounts are set up).
 
We have a draft CL to add a callback mechanism for libminijail. Maybe that could be used for this?

https://android-review.googlesource.com/302757

Comment 2 by vapier@chromium.org, Jan 13 2017

that framework could be used instead for us.  i guess the question is whether we think mj should be responsible for dev node management.  i can see the argument that it's a reduced edge case currently, but i also think locking down device nodes is a useful thing for all daemons mj wants to protect.

consider: many daemons don't need the majority of device nodes, somewhat akin to how we offer an easy way to remount /proc read-only or creating a clean /tmp.

what if we added a new -d flag that would umount /dev (if it was mounted), mounted a tmpfs there, and then only created /dev/{zero,null,full}.  we could add a -D flag so callers could specify a few extra nodes like /dev/urandom.
Personally, I don't think Minijail should be creating random dev nodes, but I think an option to remount /dev with a minimal set of nodes makes sense.

Comment 4 by dgreid@chromium.org, Jan 20 2017

Can we leverage ejcaruso's device jail to avoid doing this for our containers?

Comment 5 by dgreid@chromium.org, Jan 20 2017

Cc: ejcaruso@chromium.org
device_jail_fs hides most dev nodes currently. The current iteration allows through /dev/{zero,null,full,urandom} and instantiates jails for the USB devices lazily, but we were planning a D-Bus interface that would allow for more fine-grained control.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 5 2017

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

commit acedff934db0e14d83c4f6fee7261f30afe66a2f
Author: Dylan Reid <dgreid@chromium.org>
Date: Wed Apr 05 03:42:52 2017

libcontainer: factor out device setup.

The start function is big and long, break out device setup.

BUG= 680859 
TEST=updated security_RunOci

Change-Id: I6c2df4e1feb363c6a68bb43b7b287e784dd27dae
Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/465551
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/acedff934db0e14d83c4f6fee7261f30afe66a2f/libcontainer/libcontainer.c

Project Member

Comment 8 by bugdroid1@chromium.org, May 8 2017

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

commit 4843d6b13cf0d5e8de79964f881ce70ceab8aaec
Author: Dylan Reid <dgreid@chromium.org>
Date: Mon May 08 04:00:57 2017

libcontainer: Track device nodes and cgroup perms separately

Tracking the device nodes to create separately from the device node
permissions will allow device permissions to be set for devices that
aren't created by the container startup process.  In new version of the
OCI spec, these two items are in separate areas of config.json and this
change will make migration to that format easier.

The current add_device interface is unchanged and session_manager can
continue to use it when it parses the old 0.2 OCI config for android's
etontainer. run_oci will be changed to use the new interface in a
following commit.

BUG= 680859 
TEST=updated security_RunOci

Change-Id: I9e2e5932699da8f7406f59677d3a46b946b11664
Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/465552

[modify] https://crrev.com/4843d6b13cf0d5e8de79964f881ce70ceab8aaec/libcontainer/libcontainer.h
[modify] https://crrev.com/4843d6b13cf0d5e8de79964f881ce70ceab8aaec/libcontainer/container_cgroup.c
[modify] https://crrev.com/4843d6b13cf0d5e8de79964f881ce70ceab8aaec/libcontainer/libcontainer.c
[modify] https://crrev.com/4843d6b13cf0d5e8de79964f881ce70ceab8aaec/libcontainer/container_cgroup_unittest.c
[modify] https://crrev.com/4843d6b13cf0d5e8de79964f881ce70ceab8aaec/libcontainer/container_cgroup.h
[modify] https://crrev.com/4843d6b13cf0d5e8de79964f881ce70ceab8aaec/libcontainer/libcontainer_unittest.c

Project Member

Comment 9 by bugdroid1@chromium.org, May 19 2017

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

commit 6985e3b91de0aaa697bb15a2f862ef2e3f590d4c
Author: Dylan Reid <dgreid@chromium.org>
Date: Fri May 19 20:57:54 2017

container_utils: run_oci: Parse device cgroup info

The device cgroup info is under linux>resources>devices in the current
OCI spec.  Parse it from there and pass the configuration to
libcontainer.

BUG= 680859 
TEST=updated security_RunOci

Change-Id: I24d10660ca39ebec54cffb4e800daacd93abf75a
Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/465553
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>

[modify] https://crrev.com/6985e3b91de0aaa697bb15a2f862ef2e3f590d4c/run_oci/run_oci.cc
[modify] https://crrev.com/6985e3b91de0aaa697bb15a2f862ef2e3f590d4c/run_oci/container_config_parser.cc
[modify] https://crrev.com/6985e3b91de0aaa697bb15a2f862ef2e3f590d4c/run_oci/oci_config.h

Project Member

Comment 10 by bugdroid1@chromium.org, May 20 2017

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

commit 43d4e5ce739ed11f93c81c97c7408f39f023c79e
Author: Dylan Reid <dgreid@chromium.org>
Date: Sat May 20 10:55:09 2017

libcontainer: Fix return from device_setup

If the host device file doesn't exists an uninitialized rc was returned.
Make sure to set it.  Also use size_t for i as that matches the use in
the loops.

BUG= 680859 
TEST=security_RunOci

Change-Id: I5e392cc1424a1d7e6a35283fd400d5208c99ceab
Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/468531
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/43d4e5ce739ed11f93c81c97c7408f39f023c79e/libcontainer/libcontainer.c

Project Member

Comment 11 by bugdroid1@chromium.org, May 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/17cd8017be5cd2e1f165457765cd26dcc7cdee70

commit 17cd8017be5cd2e1f165457765cd26dcc7cdee70
Author: Dylan Reid <dgreid@chromium.org>
Date: Wed May 24 06:59:32 2017

security_RunOci: Add device cgroup tests

Add basic tests for device cgroups allowing or denying access to devices
from the OCI "resources" section.

CQ-DEPEND=CL:465553
BUG= 680859 
TEST=this is a test

Change-Id: Iebb226e6d56c84ab8e36060b7b7bee14c4aecd4f
Signed-off-by: Dylan Reid <dgreid@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/466787
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[add] https://crrev.com/17cd8017be5cd2e1f165457765cd26dcc7cdee70/client/site_tests/security_RunOci/src/test-device-cgroup-allow.json
[modify] https://crrev.com/17cd8017be5cd2e1f165457765cd26dcc7cdee70/client/site_tests/security_RunOci/security_RunOci.py
[add] https://crrev.com/17cd8017be5cd2e1f165457765cd26dcc7cdee70/client/site_tests/security_RunOci/src/test-device-cgroup-deny.json

Status: WontFix (was: Available)
Components: OS>Systems>Containers

Sign in to add a comment