Add support for MaskPaths and ReadonlyPaths

[mlaventure]

It is currently not possible for a user to specify libcontainer MaskPaths and ReadonlyPaths fields content through the bundle json configuration.

They should be added to the spec.

[jessfraz]

+1 ;)

On Thu, Feb 4, 2016 at 8:55 AM, Kenfe-Mickaël Laventure <
notifications@github.com> wrote:

It is currently not possible for a user to specify libcontainer MaskPaths
https://github.com/opencontainers/runc/blob/d66c9632bfc945547f7df4390bff35681023a24b/libcontainer/configs/config.go#L157
and ReadonlyPaths
https://github.com/opencontainers/runc/blob/d66c9632bfc945547f7df4390bff35681023a24b/libcontainer/configs/config.go#L161
fields content through the bundle json configuration.

They should be added to the spec.

—
Reply to this email directly or view it on GitHub
#320.

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

[jessfraz]

although readonly could probably be done by using mounts

On Thu, Feb 4, 2016 at 8:59 AM, Jessica Frazelle me@jessfraz.com wrote:

+1 ;)

On Thu, Feb 4, 2016 at 8:55 AM, Kenfe-Mickaël Laventure <
notifications@github.com> wrote:

It is currently not possible for a user to specify libcontainer MaskPaths
https://github.com/opencontainers/runc/blob/d66c9632bfc945547f7df4390bff35681023a24b/libcontainer/configs/config.go#L157
and ReadonlyPaths
https://github.com/opencontainers/runc/blob/d66c9632bfc945547f7df4390bff35681023a24b/libcontainer/configs/config.go#L161
fields content through the bundle json configuration.

They should be added to the spec.

—
Reply to this email directly or view it on GitHub
#320.

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu
http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

[mlaventure]

MaskPaths too can be done with mount by adding a bind mount of the paths to /dev/null, but that's a technicality (and I have no idea what mechanism would be needed on Windows).

[wking]

On Thu, Feb 04, 2016 at 08:55:08AM -0800, Kenfe-Mickaël Laventure wrote:

They should be added to the spec.

There's some previous discussion in #186.

[tonistiigi]

although readonly could probably be done by using mounts
MaskPaths too can be done with mount by adding a bind mount of the paths to /dev/null

In runc this isn't possible atm, because you are not allowed to create a mount inside /proc

[wking]

On Thu, Feb 25, 2016 at 09:28:24AM -0800, Tõnis Tiigi wrote:

although readonly could probably be done by using mounts MaskPaths
too can be done with mount by adding a bind mount of the paths to
/dev/null

In runc this isn't possible atm, because you are not allowed to
create a mount inside /proc

Really? It looks like bind-mounting /dev/null over /proc entries is
allowed. On a vanilla 4.3 kernel:
—
$ unshare -Umprf --mount-proc sh -c 'mount --bind /dev/null /proc/1/mounts && file /proc/1/mounts'
/proc/1/mounts: character special (1/3)

And I don't see anything about restricting mounts in a quick skim
through 1.

[tonistiigi]

@wking This is not allowed in runc https://github.com/opencontainers/runc/blob/4951f5821b0840d42b83e76d8ba1e881858db9eb/libcontainer/rootfs_linux.go#L310

[wking]

On Thu, Feb 25, 2016 at 06:39:23PM -0800, Tõnis Tiigi wrote:

@wking This is not allowed in runc
https://github.com/opencontainers/runc/blob/4951f5821b0840d42b83e76d8ba1e881858db9eb/libcontainer/rootfs_linux.go#L310

Ah, got it. I'm skeptical about “we need to work around restrictions
imposed by a runtime” as motivation for spec changes. If
bind-mounting /dev/null over (or remounting read-only) generic paths
(including /proc and children) is the goal, then softening that runC
restriction to allow that behavior via mounts 1 seems simpler than
adding separate fields (MaskPaths and ReadonlyPaths) to work around an
overly-strict check.

[hqhq]

Yes, restriction in runC can be softened, and we already do that for fuse. So it's both doable for specs and implementations, I tend to keep this out of specs for now.

[crosbymichael]

We will just have this list in runc for now. You cannot specify these things as mounts because of the order of mounts and how devices are created.

[crosbymichael]

We are hitting this issue over and over again. Without this in the spec you are splitting things across runtime and spec settings. It would be much cleaner to have these two fields in the spec or else we need some type of --no-mask-proc flag on the runtime to achieve this functionality.

[mlaventure]

For my 2 cents, it doesn't make sense to have so many behavior specific flags in runC especially if the goal is to be able to get the same behavior just by shipping a bundle.

If to run my container correctly I have to do more than runc start id . there's something wrong with the specs.

Or to sum up: I'm for putting this in the specs and not adding it as a flag to the runtime

[wking]

On Wed, Mar 30, 2016 at 02:04:45PM -0700, Michael Crosby wrote:

We are hitting this issue over and over again. Without this in the
spec you are splitting things across runtime and spec settings. It
would be much cleaner to have these two fields in the spec…

Can you unpack that a bit? As I understand it, folks who want this
can setup bind-mounts over anything they want masked or read-only 1.
runC had some code to block that for some paths 2, but those checks
could be softened 3. There was a separate issue with the order of
config-specified mounts and runtime-initiated mounts 4, but the
solution to that seems to be “setup the config-specified mounts after
setting up the runtime-initiated mounts” 5 or “let the
config-specified mounts setup whatever they want, and have the runtime
not clobber a spec-required mount that the config has already
supplied”.