config: add "umask" field to POSIX "user" section #941

cyphar · 2017-12-06T12:33:15Z

Users may want to specify the umask(2) of the init process in a
container. This value is identical in semantics to POSIX. This is in
order to allow usage of an OCI container for a service which normally
only inherits the umask given to it.

See-also: opencontainers/runc#1650
Requested-by: @leberknecht
Signed-off-by: Aleksa Sarai [email protected]

Users may want to specify the umask(2) of the init process in a container. This value is identical in semantics to POSIX. This is in order to allow usage of an OCI container for a service which normally only inherits the umask given to it. Signed-off-by: Aleksa Sarai <[email protected]>

crosbymichael · 2017-12-06T15:45:30Z

When do you plan to set this during init? Right before exec or when the rootfs is being setup like the existing umask settings?

wking · 2017-12-06T18:40:19Z

config.md

@@ -217,6 +217,7 @@ For POSIX platforms the `user` structure has the following fields:

 * **`uid`** (int, REQUIRED) specifies the user ID in the [container namespace](glossary.md#container-namespace).
 * **`gid`** (int, REQUIRED) specifies the group ID in the [container namespace](glossary.md#container-namespace).
+* **`umask`** (int, OPTIONAL) specifies the [umask][umask_2] of the user. If unspecified, the umask should not be changed from the calling process' umask.


nit: the possessive of “process” is “process's”, see us here and the Linux man pages here.

nit: the “If unspecified…” sentence should go on its own line.

wking · 2017-12-06T18:40:57Z

schema/defs.json

@@ -60,6 +60,9 @@
        "GID": {
            "$ref": "#/definitions/uint32"
        },
+		"Umask": {


The indent here is a bit strange. You can automatically format these files by running make fmt in the schema directory.

wking · 2017-12-06T18:42:47Z

When do you plan to set this during init? Right before exec or when the rootfs is being setup…

The way the spec reads now (#700), runtimes will be able to chose either of those as they see fit.

wking · 2017-12-06T18:54:04Z

config.md

@@ -843,6 +846,7 @@ Here is a full example `config.json` for reference.
 [selinux]:http://selinuxproject.org/page/Main_Page
 [no-new-privs]: https://www.kernel.org/doc/Documentation/prctl/no_new_privs.txt
 [proc_2]: https://www.kernel.org/doc/Documentation/filesystems/proc.txt
+[umask.2]: http://pubs.opengroup.org/onlinepubs/009695399/functions/umask.html


nit: POSIX functions should go be in section 3 (or 3p), so umask.2 should be umask.3. umask.2 would be the Linux kernel docs for umask.

nit: you're linking to the 2004 edition of POSIX. I'd rather stay consistent with our other links and use the 2016 edition (#858).

wking · 2017-12-06T19:02:28Z

specs-go/config.go

@@ -85,6 +85,8 @@ type User struct {
 	UID uint32 `json:"uid" platform:"linux,solaris"`
 	// GID is the group id.
 	GID uint32 `json:"gid" platform:"linux,solaris"`
+	// Umask is the umask for the init process.
+	Umask uint32 `json:"umask,omitempty" platform:"linux,solaris"`


Zero is a valid umask (it means “leave the permissions entirely up to the process itself”), so I think we need a pointer here.

cyphar · 2017-12-06T23:46:53Z

@crosbymichael In runc I would want to do this right before exec because it refers to the container process, no? But maybe there's an argument for it to be done earlier -- I'm not sure?

vbatts · 2018-04-04T22:08:22Z

@cyphar @crosbymichael I interpret this as being set right before exec. Perhaps that could be clearer.

wking · 2018-04-04T22:38:33Z

On Wed, Apr 04, 2018 at 03:08:23PM -0700, Vincent Batts wrote: @cyphar @crosbymichael I interpret this as being set right before exec. Perhaps that could be clearer.

If you want that interpretation to be portable across compliant runtimes, I think you'd want to land wording to that effect around [1], with something like: Runtimes MUST NOT apply process.user.umask as part of creation; that property is only applied during [start](#start). [1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/runtime.md#L103

stain · 2018-10-17T09:45:47Z

As this is still pending approval since April - what needs to be done to progress this PR?

leberknecht · 2018-11-19T15:49:26Z

Anything we can do here?

vbatts · 2018-11-19T16:36:08Z

config.md

@@ -233,6 +234,7 @@ _Note: symbolic name for uid and gid, such as uname and gname respectively, are
    "user": {
        "uid": 1,
        "gid": 1,
+        "umask": 63,


i get that they're just ints, but i wish octals were a viewable in json, and 077 wouldn't be 77

vbatts · 2018-11-19T16:37:34Z

When do you plan to set this during init? Right before exec or when the rootfs is being setup like the existing umask settings?

I would see this as right before exec. What would you're hopes be?

cyphar · 2018-11-19T19:58:19Z

I think right before exec would be most reasonable, since it's a setting about the container and not about configuring devices and others such things. Generally most config.json options aren't meant to impact internal runtime setup (except in cases where it can't be avoided like cgroups).

rhatdan · 2019-08-06T12:52:41Z

This has come up again in a request for Podman. Is there anything we could do to move this forward?

giuseppe · 2019-08-06T13:20:18Z

@crosbymichael In runc I would want to do this right before exec because it refers to the container process, no? But maybe there's an argument for it to be done earlier -- I'm not sure?

doing it just before exec seems like the correct thing to me. Currently runc does it in finalizeRootfs (https://github.com/opencontainers/runc/blob/master/libcontainer/rootfs_linux.go#L151). The umask inside the container is always set to 022

crosbymichael · 2019-08-06T14:29:37Z

LGTM

leberknecht · 2019-10-07T09:57:10Z

@caniszczyk @dqminh @hqhq @mrunalp @philips @tianon @vbatts @vishh
sorry for spamming, but can someone please do the required second review here? :)

caniszczyk · 2019-10-07T12:20:09Z

RFC @opencontainers/runtime-spec-maintainers

tianon · 2019-10-07T14:21:31Z

I think @wking's comments look to be relevant and should be addressed (especially the inconsistent formatting). It also seems like a good idea IMO to clarify when this is expected to be applied, per @crosbymichael's comments.

vbatts · 2019-12-17T19:22:55Z

LGTM
And as this is OPTIONAL it is a non-breaking, forward compatible feature.

cyphar force-pushed the config-umask-option branch from 34da7a6 to 6b04c63 Compare December 6, 2017 12:36

cyphar mentioned this pull request Dec 6, 2017

Option to set umask of exec-user opencontainers/runc#1650

Closed

wking reviewed Dec 6, 2017

View reviewed changes

vbatts reviewed Nov 19, 2018

View reviewed changes

giuseppe mentioned this pull request Aug 6, 2019

Set umask in container cri-o/cri-o#2701

Closed

vbatts merged commit cd13d2d into opencontainers:master Dec 17, 2019

vbatts mentioned this pull request Jan 9, 2020

Release v1.0.2 #1026

Closed

cyphar deleted the config-umask-option branch February 24, 2020 00:09

kolyshkin mentioned this pull request Jul 21, 2022

Umask behavior doesn't match spec opencontainers/runc#3533

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config: add "umask" field to POSIX "user" section #941

config: add "umask" field to POSIX "user" section #941

cyphar commented Dec 6, 2017

crosbymichael commented Dec 6, 2017

wking Dec 6, 2017

wking Dec 6, 2017

wking commented Dec 6, 2017

wking Dec 6, 2017

wking Dec 6, 2017

cyphar commented Dec 6, 2017

vbatts commented Apr 4, 2018

wking commented Apr 4, 2018 via email

stain commented Oct 17, 2018

leberknecht commented Nov 19, 2018

vbatts Nov 19, 2018

vbatts commented Nov 19, 2018

cyphar commented Nov 19, 2018

rhatdan commented Aug 6, 2019

giuseppe commented Aug 6, 2019

crosbymichael commented Aug 6, 2019 •

edited by caniszczyk

Loading

leberknecht commented Oct 7, 2019

caniszczyk commented Oct 7, 2019

tianon commented Oct 7, 2019

vbatts commented Dec 17, 2019 •

edited by caniszczyk

Loading

config: add "umask" field to POSIX "user" section #941

config: add "umask" field to POSIX "user" section #941

Conversation

cyphar commented Dec 6, 2017

crosbymichael commented Dec 6, 2017

wking Dec 6, 2017

Choose a reason for hiding this comment

wking Dec 6, 2017

Choose a reason for hiding this comment

wking commented Dec 6, 2017

wking Dec 6, 2017

Choose a reason for hiding this comment

wking Dec 6, 2017

Choose a reason for hiding this comment

cyphar commented Dec 6, 2017

vbatts commented Apr 4, 2018

wking commented Apr 4, 2018 via email

stain commented Oct 17, 2018

leberknecht commented Nov 19, 2018

vbatts Nov 19, 2018

Choose a reason for hiding this comment

vbatts commented Nov 19, 2018

cyphar commented Nov 19, 2018

rhatdan commented Aug 6, 2019

giuseppe commented Aug 6, 2019

crosbymichael commented Aug 6, 2019 • edited by caniszczyk Loading

leberknecht commented Oct 7, 2019

caniszczyk commented Oct 7, 2019

tianon commented Oct 7, 2019

vbatts commented Dec 17, 2019 • edited by caniszczyk Loading

crosbymichael commented Aug 6, 2019 •

edited by caniszczyk

Loading

vbatts commented Dec 17, 2019 •

edited by caniszczyk

Loading