Add baseline check for procMount type #103340

MadhavJivrajani · 2021-06-30T08:08:15Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Will not allow if a container (init or not) sets the proc mount type to anything other than Default
Include fixture for proc mount baseline generation and the consequent genreated test data

Which issue(s) this PR fixes:

Special notes for your reviewer:

The integration tests currently are not passing -- I'm having some difficulty understanding why, would greatly appreciate pointers if any :)

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://git.k8s.io/enhancements/keps/sig-auth/2579-psp-replacement

/assign @tallclair @liggitt

k8s-ci-robot · 2021-06-30T08:08:17Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

MadhavJivrajani · 2021-06-30T08:08:32Z

/sig auth

MadhavJivrajani · 2021-06-30T08:08:50Z

/priority important-soon
/triage accepted

MadhavJivrajani · 2021-06-30T08:10:52Z

Command used to run integration tests:

make test-integration WHAT=./test/integration/auth GOFLAGS="-v" KUBE_TEST_ARGS="-run PodSecurity"

The tests fail for the fail category of procmount, the logs for one such failure are:

=== RUN   TestPodSecurity/podsecurity-baseline-1-0_fail_procMount
    run.go:283: 0: expected error creating {"metadata":{"name":"test","creationTimestamp":null},"spec":{"initContainers":[{"name":"initcontainer1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"procMount":""}}],"containers":[{"name":"container1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"procMount":"Unmasked"}}],"serviceAccountName":"default","securityContext":{}},"status":{}}, got none
    run.go:284: 0: expected warnings creating {"metadata":{"creationTimestamp":null,"labels":{"test":"true"}},"spec":{"initContainers":[{"name":"initcontainer1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"procMount":""}}],"containers":[{"name":"container1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"procMount":"Unmasked"}}],"securityContext":{}},"status":{}}, got none
    run.go:283: 1: expected error creating {"metadata":{"name":"test","creationTimestamp":null},"spec":{"initContainers":[{"name":"initcontainer1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"procMount":"Unmasked"}}],"containers":[{"name":"container1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"procMount":""}}],"serviceAccountName":"default","securityContext":{}},"status":{}}, got none
    run.go:284: 1: expected warnings creating {"metadata":{"creationTimestamp":null,"labels":{"test":"true"}},"spec":{"initContainers":[{"name":"initcontainer1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"procMount":"Unmasked"}}],"containers":[{"name":"container1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"procMount":""}}],"securityContext":{}},"status":{}}, got none

After adding logs inside the visitor function to see what the proc mount types were being worked with for each container in the pod spec, I get the following:

2021/06/30 15:55:19 Container name: container1 proc mount: Default
2021/06/30 15:55:19 Container name: container1 proc mount: Default
    run.go:283: 0: expected error creating {"metadata":{"name":"test","creationTimestamp":null},"spec":{"initContainers":[{"name":"initcontainer1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"allowPrivilegeEscalation":false}}],"containers":[{"name":"container1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"allowPrivilegeEscalation":false,"procMount":"Unmasked"}}],"serviceAccountName":"default","securityContext":{"runAsNonRoot":true}},"status":{}}, got none
2021/06/30 15:55:19 Container name: container1 proc mount: Default
    run.go:284: 0: expected warnings creating {"metadata":{"creationTimestamp":null,"labels":{"test":"true"}},"spec":{"initContainers":[{"name":"initcontainer1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"allowPrivilegeEscalation":false}}],"containers":[{"name":"container1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"allowPrivilegeEscalation":false,"procMount":"Unmasked"}}],"securityContext":{"runAsNonRoot":true}},"status":{}}, got none
2021/06/30 15:55:19 Container name: initcontainer1 proc mount: Default
2021/06/30 15:55:19 Container name: initcontainer1 proc mount: Default
    run.go:283: 1: expected error creating {"metadata":{"name":"test","creationTimestamp":null},"spec":{"initContainers":[{"name":"initcontainer1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"allowPrivilegeEscalation":false,"procMount":"Unmasked"}}],"containers":[{"name":"container1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"allowPrivilegeEscalation":false}}],"serviceAccountName":"default","securityContext":{"runAsNonRoot":true}},"status":{}}, got none
2021/06/30 15:55:19 Container name: initcontainer1 proc mount: Default
    run.go:284: 1: expected warnings creating {"metadata":{"creationTimestamp":null,"labels":{"test":"true"}},"spec":{"initContainers":[{"name":"initcontainer1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"allowPrivilegeEscalation":false,"procMount":"Unmasked"}}],"containers":[{"name":"container1","image":"k8s.gcr.io/pause","resources":{},"securityContext":{"allowPrivilegeEscalation":false}}],"securityContext":{"runAsNonRoot":true}},"status":{}}, got none

I noticed that even if the spec that is logged out as part of the error message shows the container initcontainer has proc mount Unmasked, the logs that were added later show that the src mount type is still Default

liggitt

Looks good overall, thanks for the PR. Just a few tweaks. Go ahead and squash updates to a single commit.

staging/src/k8s.io/pod-security-admission/policy/check_procMount.go

staging/src/k8s.io/pod-security-admission/test/fixtures_procMount.go

MadhavJivrajani · 2021-06-30T15:20:34Z

Thank you @liggitt! I've made the changes :)

MadhavJivrajani · 2021-06-30T15:39:39Z

/hold
for #103365 to be merged

staging/src/k8s.io/pod-security-admission/test/fixtures_procMount.go

- Will not allow if a container (init or not) sets the proc mount type to anything other than `Default` - Include fixture for proc mount baseline generation and the consequent genreated test data Signed-off-by: Madhav Jivrajani <[email protected]>

MadhavJivrajani · 2021-07-01T14:33:29Z

/hold cancel

liggitt · 2021-07-01T14:37:32Z

/lgtm
/approve

k8s-ci-robot · 2021-07-01T14:37:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, MadhavJivrajani

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:

~~staging/src/k8s.io/pod-security-admission/OWNERS~~ [liggitt]

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

k8s-ci-robot assigned liggitt Jun 30, 2021

k8s-ci-robot assigned tallclair Jun 30, 2021

k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 30, 2021

k8s-ci-robot requested review from krmayankk and liggitt June 30, 2021 08:08

k8s-ci-robot removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jun 30, 2021

MadhavJivrajani force-pushed the proc-mount-baseline branch from c960a9b to 5099312 Compare June 30, 2021 09:16

liggitt reviewed Jun 30, 2021

View reviewed changes

MadhavJivrajani force-pushed the proc-mount-baseline branch from 5099312 to 5db3277 Compare June 30, 2021 15:19

MadhavJivrajani marked this pull request as ready for review June 30, 2021 15:20

MadhavJivrajani changed the title ~~[WIP] Add baseline check for procMount type~~ Add baseline check for procMount type Jun 30, 2021

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 30, 2021

liggitt mentioned this pull request Jun 30, 2021

PodSecurity: make failure integration tests feature-aware #103365

Merged

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 30, 2021

liggitt reviewed Jun 30, 2021

View reviewed changes

staging/src/k8s.io/pod-security-admission/test/fixtures_procMount.go Outdated Show resolved Hide resolved

MadhavJivrajani force-pushed the proc-mount-baseline branch from 5db3277 to c7bb761 Compare June 30, 2021 15:45

liggitt unassigned tallclair Jun 30, 2021

Add baseline check for procMount type

f0ffba7

- Will not allow if a container (init or not) sets the proc mount type to anything other than `Default` - Include fixture for proc mount baseline generation and the consequent genreated test data Signed-off-by: Madhav Jivrajani <[email protected]>

MadhavJivrajani force-pushed the proc-mount-baseline branch from c7bb761 to f0ffba7 Compare July 1, 2021 14:33

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 1, 2021

MadhavJivrajani requested a review from liggitt July 1, 2021 14:33

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 1, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 1, 2021

k8s-ci-robot merged commit e513598 into kubernetes:master Jul 1, 2021

k8s-ci-robot added this to the v1.22 milestone Jul 1, 2021

MadhavJivrajani deleted the proc-mount-baseline branch July 1, 2021 16:53

liggitt mentioned this pull request Jul 10, 2021

PodSecurity admission (PodSecurityPolicy replacement) kubernetes/enhancements#2579

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add baseline check for procMount type #103340

Add baseline check for procMount type #103340

MadhavJivrajani commented Jun 30, 2021

k8s-ci-robot commented Jun 30, 2021

MadhavJivrajani commented Jun 30, 2021

MadhavJivrajani commented Jun 30, 2021

MadhavJivrajani commented Jun 30, 2021 •

edited

Loading

liggitt left a comment •

edited

Loading

MadhavJivrajani commented Jun 30, 2021

MadhavJivrajani commented Jun 30, 2021

MadhavJivrajani commented Jul 1, 2021

liggitt commented Jul 1, 2021

k8s-ci-robot commented Jul 1, 2021

Add baseline check for procMount type #103340

Add baseline check for procMount type #103340

Conversation

MadhavJivrajani commented Jun 30, 2021

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Jun 30, 2021

MadhavJivrajani commented Jun 30, 2021

MadhavJivrajani commented Jun 30, 2021

MadhavJivrajani commented Jun 30, 2021 • edited Loading

liggitt left a comment • edited Loading

Choose a reason for hiding this comment

MadhavJivrajani commented Jun 30, 2021

MadhavJivrajani commented Jun 30, 2021

MadhavJivrajani commented Jul 1, 2021

liggitt commented Jul 1, 2021

k8s-ci-robot commented Jul 1, 2021

MadhavJivrajani commented Jun 30, 2021 •

edited

Loading

liggitt left a comment •

edited

Loading