Allow specifying the primary group id of the container #756

krmayankk · 2017-06-22T05:12:50Z

Allows specifying the Primary Group ID of the container, in addition to Primary User Id when running in Docker similar to how docker supports this.

krmayankk · 2017-06-22T05:17:09Z

@pmorie @thockin @pineking @vpal @jheiss

krmayankk · 2017-06-22T05:23:59Z

@kubernetes/sig-node-proposals

pmorie

Promising start to this proposal, left some comments.

pmorie · 2017-06-22T13:08:47Z

contributors/design-proposals/runas-groupid.md

+## Abstract
+
+
+As a Kubernetes User, i should be able to specify both user id and group id for the containers running 


pmorie · 2017-06-22T13:08:59Z

contributors/design-proposals/runas-groupid.md

+inside a pod on a per Container basis, similar to how docker allows that using docker run options -u, 
+--user="" Username or UID (format: <name|uid>[:<group|gid>]) format.
+
+PodSecurityContext allows Kubernetes users to specify RUnAsUser which can be overriden by RunAsUser


typo: RUnAsUser

pmorie · 2017-06-22T13:09:10Z

contributors/design-proposals/runas-groupid.md

+--user="" Username or UID (format: <name|uid>[:<group|gid>]) format.
+
+PodSecurityContext allows Kubernetes users to specify RUnAsUser which can be overriden by RunAsUser
+in SecurtiyContext on a per Container basis. There is no equivalent field for specifying the primary


typo: SecurityContext

pmorie · 2017-06-22T13:10:24Z

contributors/design-proposals/runas-groupid.md

+## Motivation
+
+Enterprise Kubernetes users want to run containers as non root. This means running containers with a 
+non zero user id and non zero group id. This gives Enterprises confidence that their customer code


Group zero essentially means nothing and has zero security implications. I'm not saying that this isn't a valid use case, but you don't gain anything, to my knowledge, by running as GID 0.

I would buy that someone wants to be able to use GIDs as part of a security scheme, though, possibly derived from LDAP or another identity system.

pmorie · 2017-06-22T13:13:26Z

contributors/design-proposals/runas-groupid.md

+## Use Cases
+
+Use case 1:
+As a cluster operator, i should be able to control both user id and primary group id of containers 


I think this is 'as a Kubernetes user', and what's the "so that I can _____________" part of this use-case?

pmorie · 2017-06-22T13:13:38Z

contributors/design-proposals/runas-groupid.md

+launched using Kubernetes.
+
+Use case 2:
+As a cluster operator, i should be able to override user id and primary group id of a container as 


Isn't this the same use case as the first one?

pmorie · 2017-06-22T13:13:58Z

contributors/design-proposals/runas-groupid.md

+ }
+
+
+type PoddSecurityContext struct {


typo: PodSecurityContext

pmorie · 2017-06-22T13:14:24Z

contributors/design-proposals/runas-groupid.md

+     // +optional
+     RunAsUser *int64
+     // The GID to run the entrypoint of the container process.
+     // Defaults to user specified in image metadata if unspecified.


Defaults to the group?

pmorie · 2017-06-22T13:15:02Z

contributors/design-proposals/runas-groupid.md

+- If no RunAsGroup is provided in the PodSecurityContext and SecurityContext, and none in the image,
+  the container will run with primary Group as root(0).
+
+## Not In Scope


How is this not in scope? We need to add the policy control surface to govern features like this as we implement them.

Any additions to PodSecurityContext will changes to PodSecurityPolicy - both are part of the API and should evolve together.

erictune · 2017-07-14T00:47:56Z

Can you at least compare to the alternative, which is pushing an /etc/groups file to all your nodes and using that for group membership?

krmayankk · 2017-08-15T07:20:19Z

@pmorie @smarterclayton addressed comments. @erictune Are you suggesting controlling the gid through the file, while controlling uid through existing runtime mechanisms. It can be done, but not dynamically configurable and one /etc/groups file will affect all containers of a host. Do you still think we should discuss that option in the proposal ?

krmayankk · 2017-08-15T07:39:00Z

@erictune Also isnt /etc/groups only for controlling secondary/supplemental group ids? Here we are talking about controlling the primary group id the container runs as

vpal · 2017-08-15T08:28:32Z

@krmayankk, @erictune I think that the groups file on the host OS does not affect UID and GID allocation in the container.

sudo docker run --rm -ti --user 1000 ubuntu:16.04 /bin/bash
I have no name!@80925fc54764:/$ id
uid=1000 gid=0(root) groups=0(root)
I have no name!@80925fc54764:/$

I have user and group 1000 in the group file on the host.
So what you would need to do is rebuild your images with the /etc/groups file, which is the thing we would like to avoid here. That is what the --user is for in docker.
If you have the chance to rebuild the image you can also solve this in other ways like providing UID and GID in the Dockerfile for example.
But if you just want to run something straight from a third party repository and want it to have a specific UID and GID overriding the default one you do not have the option right now. You can only specify the UID and supplemental groups.

smarterclayton · 2017-08-15T22:50:43Z

I have no objections to this as stated. For symmetry I see no reason why this should be prohibited when the primary UID is allowed.

smarterclayton · 2017-08-15T22:51:43Z

Something seems to be wrong with lowercase 'i''s in the markdown being converted to capital I.

smarterclayton · 2017-08-15T22:53:00Z

@kubernetes/sig-auth-feature-requests can you chime in if you see any additional implications. I don't see any reason not to add this or enable it as long as the PSP work is done at the same time.

erictune · 2017-08-15T23:14:35Z

Disregard my comment about /etc/groups.

…

On Tue, Aug 15, 2017 at 3:53 PM, Clayton Coleman ***@***.***> wrote: @kubernetes/sig-auth-feature-requests <https://github.com/orgs/kubernetes/teams/sig-auth-feature-requests> can you chime in if you see any additional implications. I don't see any reason not to add this or enable it as long as the PSP work is done at the same time. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#756 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHuudqchstCVUhYMmVuyd88UUTdIH9Rqks5sYiFYgaJpZM4OB0P7> .

smarterclayton · 2017-08-22T22:52:34Z

I'd like at least another sig node reviewer to comment before I mark lgtm.

pmorie

Close to LGTM; a couple nits and one ask to explicitly define behavior when these fields are unspecified.

pmorie · 2017-08-30T13:59:33Z

contributors/design-proposals/runas-groupid.md

+
+As a Kubernetes User, we should be able to specify both user id and group id for the containers running 
+inside a pod on a per Container basis, similar to how docker allows that using docker run options `-u, 
+--user="" Username or UID (format: <name|uId>[:<group|gid>]) format`.


typo: "uId"

pmorie · 2017-08-30T14:00:00Z

contributors/design-proposals/runas-groupid.md

+     // May also be set in PodSecurityContext.  If set in both SecurityContext and
+     // PodSecurityContext, the value specified in SecurityContext takes precedence.
+     // +optional
+     RunAsUser *Int64


nit: int64

pmorie · 2017-08-30T14:00:18Z

contributors/design-proposals/runas-groupid.md

+
+PodSecurityPolicy defines strategies or conditions that a pod must run with in order to be accepted
+into the system. Two of the relevant strategies are RunAsUser and SupplementalGroups. We introduce 
+a new strategy called RunAsGroup which will support the following options:-


pmorie · 2017-08-30T15:37:31Z

contributors/design-proposals/runas-groupid.md

+
+### Model
+
+Introduce a new API field in SecurityContext and PodSecurityContext called `RunAsGroup`


Missing period at the end of this sentence. Can we mention explicitly what the behavior is if these fields are not defined?

krmayankk · 2017-08-30T22:06:59Z

@pmorie there is a Behavior heading already at the bottom of this design proposal. Let me know if its missing something ?

smarterclayton · 2017-09-15T21:56:28Z

I think given that people may have NFS or other shared storage that might be impacted by root group 0, that RunAsNonRootGroup makes sense for its clear parallel to user in the API as well us not having a good reason for it to be inconsistent.

…

On Fri, Sep 15, 2017 at 5:42 PM, krmayankk ***@***.***> wrote: This was in the outdated section so hidden. Repasting this here, so that people can continue the discussion here. @pmorie <https://github.com/pmorie> can you raise your concerns about not adding RunAsNonRootGroup here. @pweil- <https://github.com/pweil-> @smarterclayton <https://github.com/smarterclayton> i have updated the doc to also say we need RunAsNonRootGroup in addition to RunAsNonRoot similar to PSP supporting two different strategies for Group and User. RunAsNonRoot in SecurityContext and PodSecurityContext is specific to uid and hence we need to add one one more bool RunAsNonRootGroup for gid. PTAL and comment here — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#756 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p2mU4G8cg5M4wJpom2M9LBMVsAb-ks5siu9UgaJpZM4OB0P7> .

pmorie · 2017-09-18T15:01:26Z

/lgtm

k8s-github-robot · 2017-09-18T15:06:10Z

Automatic merge from submit-queue

krmayankk · 2017-09-18T18:33:36Z

thanks to @pmorie for initial pointers, review and encouragement. And thanks to @pweil- @smarterclayton @vpal for reviewing and it and getting this in

bgrant0607 · 2017-09-19T04:26:24Z

@krmayankk All of the design proposals moved into SIG-specific subdirectories last week. The security context proposal was moved to auth, so I'd appreciate it if this were moved into that subdirectory, also.

krmayankk · 2017-09-20T15:38:57Z

Sounds good will move it today

On Mon, Sep 18, 2017 at 9:26 PM Brian Grant ***@***.***> wrote: @krmayankk <https://github.com/krmayankk> All of the design proposals moved into SIG-specific subdirectories last week. The security context proposal was moved to auth, so I'd appreciate it if this were moved into that subdirectory, also. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#756 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AP4-NCv-VmOfWuYuMrhRk6VW1zt8YH-Fks5sj0J3gaJpZM4OB0P7> .

-- -Mayank

Automatic merge from submit-queue. add author to my proposal for reachability See here #756

@pweil-

Automatic merge from submit-queue (batch tested with PRs 56589, 56503). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. MustRunAsNonRoot should reject a pod if it has non-numeric USER **What this PR does / why we need it**: This PR modifies kubelet behavior to reject pods with non-numeric USER instead of showing a warning. **Special notes for your reviewer**: Related discussion: kubernetes/community#756 (comment) **Release note**: ```release-note kubelet: fix bug where `runAsUser: MustRunAsNonRoot` strategy didn't reject a pod with a non-numeric `USER`. ``` PTAL @pweil- @tallclair @liggitt @Random-Liu CC @simo5 @adelton

Automatic merge from submit-queue (batch tested with PRs 52077, 60456, 60591). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. API Changes for RunAsGroup First set of api changes for feature kubernetes/community#756 ```release-note Add ability to control primary GID of containers through pod Spec and PodSecurityPolicy ```

Automatic merge from submit-queue (batch tested with PRs 52077, 60456, 60591). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. API Changes for RunAsGroup First set of api changes for feature kubernetes/community#756 ```release-note Add ability to control primary GID of containers through pod Spec and PodSecurityPolicy ``` Kubernetes-commit: 16980f21d167b0166de7fb029e80f5da37ff2ebe

Automatic merge from submit-queue. add author to my proposal for reachability See here kubernetes/community#756

Automatic merge from submit-queue Allow specifying the primary group id of the container Allows specifying the Primary Group ID of the container, in addition to Primary User Id when running in Docker similar to how docker supports this.

Automatic merge from submit-queue. add author to my proposal for reachability See here kubernetes/community#756

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 22, 2017

pmorie reviewed Jun 22, 2017

View reviewed changes

krmayankk force-pushed the runasg branch from 1d26f59 to e2a1e1f Compare August 15, 2017 07:17

krmayankk force-pushed the runasg branch from e2a1e1f to 7f8f180 Compare August 15, 2017 07:31

smarterclayton added sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/node Categorizes an issue or PR as relevant to SIG Node. labels Aug 15, 2017

k8s-github-robot assigned idvoretskyi and calebamiles Aug 15, 2017

k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 15, 2017

krmayankk force-pushed the runasg branch 3 times, most recently from 7ae009f to a3c70ae Compare August 29, 2017 04:32

krmayankk mentioned this pull request Aug 30, 2017

Provide RunAsGroup feature for Containers in a Pod kubernetes/enhancements#213

Closed

5 tasks

pmorie reviewed Aug 30, 2017

View reviewed changes

krmayankk force-pushed the runasg branch 2 times, most recently from 93d65b4 to 8a69db9 Compare August 30, 2017 22:05

krmayankk force-pushed the runasg branch from 8a69db9 to 9a4c60e Compare September 1, 2017 18:28

k8s-ci-robot assigned pmorie Sep 18, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 18, 2017

k8s-github-robot merged commit 65bc9f8 into kubernetes:master Sep 18, 2017

This was referenced Sep 23, 2017

Add a RunAsGroup field to SecurityContext kubernetes/kubernetes#13868

Closed

Provide possibility to specify the group a container is run as kubernetes/kubernetes#22179

Closed

krmayankk mentioned this pull request Sep 29, 2017

add author to my proposal for reachability #1138

Merged

k8s-github-robot pushed a commit that referenced this pull request Oct 12, 2017

Merge pull request #1138 from krmayankk/runasg

586dcd7

Automatic merge from submit-queue. add author to my proposal for reachability See here #756

This was referenced Nov 22, 2017

Rewrite PodSecurityPolicy guide kubernetes/website#6378

Merged

MustRunAsNonRoot should reject a pod if it has non-numeric USER kubernetes/kubernetes#56503

Merged

php-coder mentioned this pull request Feb 12, 2018

Group should be honoured for Docker image when overriding user. openshift/origin#8618

Open

krmayankk mentioned this pull request Apr 6, 2018

NonRootGroup API Changes and Implementation kubernetes/kubernetes#62216

Closed

justaugustus pushed a commit to justaugustus/enhancements that referenced this pull request Sep 3, 2018

Merge pull request kubernetes#1138 from krmayankk/runasg

9a4d07d

Automatic merge from submit-queue. add author to my proposal for reachability See here kubernetes/community#756

gnufied mentioned this pull request Apr 4, 2019

World Writable emptydir directories kubernetes/kubernetes#76158

Closed

MadhavJivrajani pushed a commit to kubernetes/design-proposals-archive that referenced this pull request Nov 30, 2021

Merge pull request #1138 from krmayankk/runasg

fa9443a

Automatic merge from submit-queue. add author to my proposal for reachability See here kubernetes/community#756

MadhavJivrajani pushed a commit to MadhavJivrajani/design-proposals that referenced this pull request Dec 1, 2021

Merge pull request #1138 from krmayankk/runasg

59156df

Automatic merge from submit-queue. add author to my proposal for reachability See here kubernetes/community#756

MadhavJivrajani pushed a commit to MadhavJivrajani/design-proposals that referenced this pull request Dec 1, 2021

Merge pull request #1138 from krmayankk/runasg

3d043dc

Automatic merge from submit-queue. add author to my proposal for reachability See here kubernetes/community#756

MadhavJivrajani pushed a commit to MadhavJivrajani/design-proposals that referenced this pull request Dec 1, 2021

Merge pull request #1138 from krmayankk/runasg

ca55dfc

Automatic merge from submit-queue. add author to my proposal for reachability See here kubernetes/community#756

MadhavJivrajani pushed a commit to kubernetes/design-proposals-archive that referenced this pull request Dec 1, 2021

Merge pull request #1138 from krmayankk/runasg

238570b

Automatic merge from submit-queue. add author to my proposal for reachability See here kubernetes/community#756

MadhavJivrajani pushed a commit to kubernetes/design-proposals-archive that referenced this pull request Dec 1, 2021

Merge pull request #1138 from krmayankk/runasg

b9be3d9

Automatic merge from submit-queue. add author to my proposal for reachability See here kubernetes/community#756

		## Abstract


		As a Kubernetes User, i should be able to specify both user id and group id for the containers running


		### Model

		Introduce a new API field in SecurityContext and PodSecurityContext called `RunAsGroup`

Allow specifying the primary group id of the container #756

Allow specifying the primary group id of the container #756

Conversation

krmayankk commented Jun 22, 2017 • edited Loading

krmayankk commented Jun 22, 2017

krmayankk commented Jun 22, 2017

pmorie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erictune commented Jul 14, 2017

krmayankk commented Aug 15, 2017

krmayankk commented Aug 15, 2017 • edited Loading

vpal commented Aug 15, 2017 • edited Loading

smarterclayton commented Aug 15, 2017 • edited Loading

smarterclayton commented Aug 15, 2017

smarterclayton commented Aug 15, 2017

erictune commented Aug 15, 2017 via email

smarterclayton commented Aug 22, 2017

pmorie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krmayankk commented Aug 30, 2017

smarterclayton commented Sep 15, 2017 via email

pmorie commented Sep 18, 2017

k8s-github-robot commented Sep 18, 2017

krmayankk commented Sep 18, 2017

bgrant0607 commented Sep 19, 2017

krmayankk commented Sep 20, 2017 via email

krmayankk commented Jun 22, 2017 •

edited

Loading

krmayankk commented Aug 15, 2017 •

edited

Loading

vpal commented Aug 15, 2017 •

edited

Loading

smarterclayton commented Aug 15, 2017 •

edited

Loading