Add a ProcMount option to the SecurityContext & AllowedProcMountTypes to PodSecurityPolicy #64283

jessfraz · 2018-05-24T18:52:21Z

So there is a bit of a chicken and egg problem here in that the CRI runtimes will need to implement this for there to be any sort of e2e testing.

What this PR does / why we need it: This PR implements design proposal kubernetes/community#1934. This adds a ProcMount option to the SecurityContext and AllowedProcMountTypes to PodSecurityPolicy

Relies on google/cadvisor#1967

Release note:

ProcMount added to SecurityContext and AllowedProcMounts added to PodSecurityPolicy to allow paths in the container's /proc to not be masked.

cc @Random-Liu @mrunalp

dims · 2018-05-24T21:36:47Z

pkg/security/podsecuritypolicy/provider.go

@@ -271,6 +271,21 @@ func (s *simpleProvider) ValidateContainerSecurityContext(pod *api.Pod, containe
 		allErrs = append(allErrs, field.Invalid(fldPath.Child("privileged"), *privileged, "Privileged containers are not allowed"))
 	}

+	procMount := sc.ProcMount()
+	allowedProcMounts := s.psp.Spec.AllowedProcMountTypes
+	if allowedProcMounts == nil {


Check for empty as well? (in addition to nil)

So a length check against 0 would catch both.

dims · 2018-05-24T21:39:40Z

@jessfraz we need a Feature for this right? (entry in kube_features.go)

jessfraz · 2018-05-24T22:34:31Z

Oh thanks for reminding me I'll do that

AkihiroSuda · 2018-05-25T00:19:02Z

pkg/kubelet/apis/cri/runtime/v1alpha2/api.proto

@@ -586,6 +586,12 @@ message LinuxContainerSecurityContext {
    // no_new_privs defines if the flag for no_new_privs should be set on the
    // container.
    bool no_new_privs = 11;
+    // masked_paths is a slice of paths that should be masked by the container
+    // runtime, this can be passed directly to the OCI spec.
+    repeated string masked_paths = 13;


nit: 13 -> 12?

run_as_group is 12 its just out of order, it got me on the compile :D

y, i scratched my head for a few mins and realized that

is there any guidance for runtimes on how to handle these two fields? containerd/cri@3e4cec8#diff-c656bacd6cb0fe9a7f64814b01256decR358 cri-containerd seems to handle it that way but I believe there must be guidance in the api itself, @Random-Liu @jessfraz

dims · 2018-06-09T14:16:16Z

/unassign
/uncc

smarterclayton · 2018-08-31T18:07:17Z

/approve

k8s-ci-robot · 2018-08-31T18:07:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, jessfraz, smarterclayton, tpepper

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~Godeps/OWNERS~~ [smarterclayton]
~~api/OWNERS~~ [smarterclayton]
~~docs/api-reference/OWNERS~~ [smarterclayton]
~~pkg/OWNERS~~ [smarterclayton]
~~staging/src/k8s.io/api/OWNERS~~ [smarterclayton]
~~staging/src/k8s.io/apiextensions-apiserver/Godeps/OWNERS~~ [smarterclayton]
~~staging/src/k8s.io/apiserver/Godeps/OWNERS~~ [smarterclayton]
~~staging/src/k8s.io/kube-aggregator/Godeps/OWNERS~~ [smarterclayton]
~~staging/src/k8s.io/sample-apiserver/Godeps/OWNERS~~ [smarterclayton]
~~vendor/OWNERS~~ [smarterclayton]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jessfraz · 2018-08-31T20:32:36Z

/test pull-kubernetes-e2e-kops-aws

k8s-github-robot · 2018-08-31T22:24:16Z

/test all [submit-queue is verifying that this PR is safe to merge]

jessfraz · 2018-08-31T23:28:51Z

/test pull-kubernetes-e2e-kops-aws

k8s-github-robot · 2018-08-31T23:46:33Z

Automatic merge from submit-queue (batch tested with PRs 64283, 67910, 67803, 68100). If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

derekwaynecarr · 2018-09-01T19:14:19Z

Can someone confirm this does not break support for docker 1.13?

annismckenzie · 2018-09-01T19:46:31Z

It doesn't – as long as your pod specs don't request/use the feature. According to https://github.com/kubernetes/kubernetes/pull/64283/files#diff-9a9f71a82ddbbcd0627de3aea90c6649R262 it'll be available starting with Docker API version 1.38 (which isn't even in a released version yet – see here). At that point in the code you also see that it'll fail to admit pods that ask for the feature with Docker not being at that version.

dims · 2018-09-01T23:11:24Z

@derekwaynecarr as i mentioned earlier in this PR - one of the side results of the docker/docker update in this PR is that we switch from docker api version 1.31 to 1.38 if we want to keep the max to the earlier 1.31 docker API version then we need something like:
#66511

jessfraz · 2018-09-02T00:17:10Z

My bad will do next week when back from vacation

yujuhong · 2018-09-05T23:49:43Z

I know the PR is already in, but I'd rather not add more Docker API checking logic in kubelet itself (with dockershim being the exception). Can we move that logic out of kubelet, and just let dockershim return an error when trying to start the pod? The pod will be pending but it's no worse than what this PR currently does IMO.

k8s-ci-robot · 2018-09-09T01:22:08Z

@jessfraz: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-local-e2e-containerized	`1a4cf7a`	link	`/test pull-kubernetes-local-e2e-containerized`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Docker >= 18.06 is needed for securityContext.procMount (Kubernetes >= 1.12) See kubernetes/kubernetes#64283 and docker-archive/docker-ce@67fe100 Signed-off-by: Akihiro Suda <[email protected]>

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 24, 2018

k8s-github-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label May 24, 2018

jessfraz mentioned this pull request May 24, 2018

add ProcMount option kubernetes/community#1934

Merged

k8s-ci-robot requested review from davidopp and dims May 24, 2018 18:52

dims reviewed May 24, 2018

View reviewed changes

AkihiroSuda reviewed May 25, 2018

View reviewed changes

jessfraz force-pushed the ProcMountType branch from e195cbc to fa808ad Compare May 31, 2018 21:02

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 31, 2018

jessfraz force-pushed the ProcMountType branch from fa808ad to b719f09 Compare June 1, 2018 02:05

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 1, 2018

jessfraz force-pushed the ProcMountType branch from b719f09 to 58e9a12 Compare June 1, 2018 02:23

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 2, 2018

jessfraz mentioned this pull request Jun 14, 2018

vendor: update docker import google/cadvisor#1967

Merged

jessfraz added sig/node Categorizes an issue or PR as relevant to SIG Node. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Jun 14, 2018

jessfraz added this to the v1.12 milestone Jun 14, 2018

jessfraz force-pushed the ProcMountType branch from 58e9a12 to 947e1ba Compare June 14, 2018 22:51

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 14, 2018

jessfraz force-pushed the ProcMountType branch 2 times, most recently from ce9482c to 605cddf Compare June 15, 2018 00:01

k8s-github-robot added the milestone/incomplete-labels label Jun 20, 2018

jessfraz added the kind/feature Categorizes issue or PR as related to a new feature. label Jun 20, 2018

k8s-github-robot added milestone/needs-approval and removed milestone/incomplete-labels labels Jun 20, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 31, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 31, 2018

k8s-github-robot merged commit 39004e8 into kubernetes:master Aug 31, 2018

jessfraz deleted the ProcMountType branch August 31, 2018 23:49

Random-Liu mentioned this pull request Sep 4, 2018

Add CRI Validation Test for new CRI fields kubernetes-sigs/cri-tools#373

Closed

dims mentioned this pull request Sep 5, 2018

Add max docker api version support #66511

Closed

AkihiroSuda mentioned this pull request Oct 4, 2018

Initial support for Kubernetes 1.12+ kubernetes/minikube#3180

Merged

AkihiroSuda mentioned this pull request Oct 5, 2018

Upgrade Docker, from 17.12.1-ce to 18.06.1-ce kubernetes/minikube#3223

Merged

PatrickLang mentioned this pull request Oct 19, 2018

Kubelet failing to start with error client version 1.38 is too new with Docker EE-basic 18.03 #69996

Closed

dims mentioned this pull request Nov 8, 2018

Updating vendored dependencies for Kubernetes 1.13 #69776

Closed

89 tasks

liggitt mentioned this pull request Dec 19, 2018

ProcMount - Update DropDisabled[Alpha]Fields behaviour #72213

Merged

wgliang mentioned this pull request Dec 22, 2018

Upgrade kubelet from v1.11.3 to v.12.3 without draining pods causes containers to restart continuously #72296

Closed

gaorong mentioned this pull request Jun 2, 2019

upgrading from v1.13 to v1.14 causes daemonset's container restart unexpectedly #78633

Closed

jnewland mentioned this pull request Dec 28, 2019

Reduce privileges granted to docker container urcomputeringpal/actions-runner-kubernetes#1

Open

geofft mentioned this pull request Apr 14, 2020

Rootless containers don't work from unprivileged non-root Docker container (operation not permitted for mounting procfs) opencontainers/runc#1658

Closed

rptaylor mentioned this pull request Mar 6, 2021

run singularity in (unprivileged) k8s pod apptainer/singularity#5857

Closed

haircommander mentioned this pull request Oct 2, 2023

add ProcMount option kubernetes/enhancements#4265

Open

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a ProcMount option to the SecurityContext & AllowedProcMountTypes to PodSecurityPolicy #64283

Add a ProcMount option to the SecurityContext & AllowedProcMountTypes to PodSecurityPolicy #64283

jessfraz commented May 24, 2018 •

edited

Loading

dims May 24, 2018

annismckenzie May 24, 2018

dims commented May 24, 2018

jessfraz commented May 24, 2018

AkihiroSuda May 25, 2018

jessfraz May 25, 2018

dims May 25, 2018

runcom Oct 1, 2018

dims commented Jun 9, 2018

smarterclayton commented Aug 31, 2018

k8s-ci-robot commented Aug 31, 2018

jessfraz commented Aug 31, 2018

k8s-github-robot commented Aug 31, 2018

jessfraz commented Aug 31, 2018

k8s-github-robot commented Aug 31, 2018

derekwaynecarr commented Sep 1, 2018

annismckenzie commented Sep 1, 2018

dims commented Sep 1, 2018

jessfraz commented Sep 2, 2018 via email •

edited

Loading

yujuhong commented Sep 5, 2018

k8s-ci-robot commented Sep 9, 2018

Add a ProcMount option to the SecurityContext & AllowedProcMountTypes to PodSecurityPolicy #64283

Add a ProcMount option to the SecurityContext & AllowedProcMountTypes to PodSecurityPolicy #64283

Conversation

jessfraz commented May 24, 2018 • edited Loading

dims May 24, 2018

Choose a reason for hiding this comment

annismckenzie May 24, 2018

Choose a reason for hiding this comment

dims commented May 24, 2018

jessfraz commented May 24, 2018

AkihiroSuda May 25, 2018

Choose a reason for hiding this comment

jessfraz May 25, 2018

Choose a reason for hiding this comment

dims May 25, 2018

Choose a reason for hiding this comment

runcom Oct 1, 2018

Choose a reason for hiding this comment

dims commented Jun 9, 2018

smarterclayton commented Aug 31, 2018

k8s-ci-robot commented Aug 31, 2018

jessfraz commented Aug 31, 2018

k8s-github-robot commented Aug 31, 2018

jessfraz commented Aug 31, 2018

k8s-github-robot commented Aug 31, 2018

derekwaynecarr commented Sep 1, 2018

annismckenzie commented Sep 1, 2018

dims commented Sep 1, 2018

jessfraz commented Sep 2, 2018 via email • edited Loading

yujuhong commented Sep 5, 2018

k8s-ci-robot commented Sep 9, 2018

jessfraz commented May 24, 2018 •

edited

Loading

jessfraz commented Sep 2, 2018 via email •

edited

Loading