From 4a69c5799230e388df21c6cc4924c53d3efb7c80 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 7 Jul 2021 21:22:03 -0400 Subject: [PATCH] PodSecurity: procMount: cleanup --- .../policy/check_procMount.go | 31 ++++++--- .../policy/check_procMount_test.go | 66 +++++++++++++++++++ .../test/fixtures_procMount.go | 10 +-- 3 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 staging/src/k8s.io/pod-security-admission/policy/check_procMount_test.go diff --git a/staging/src/k8s.io/pod-security-admission/policy/check_procMount.go b/staging/src/k8s.io/pod-security-admission/policy/check_procMount.go index c07dd0014a2..9fa277b5996 100644 --- a/staging/src/k8s.io/pod-security-admission/policy/check_procMount.go +++ b/staging/src/k8s.io/pod-security-admission/policy/check_procMount.go @@ -27,6 +27,18 @@ import ( "k8s.io/pod-security-admission/api" ) +/* + +The default /proc masks are set up to reduce attack surface, and should be required. + +**Restricted Fields:** +spec.containers[*].securityContext.procMount +spec.initContainers[*].securityContext.procMount + +**Allowed Values:** undefined/null, "Default" + +*/ + func init() { addCheck(CheckProcMount) } @@ -41,14 +53,14 @@ func CheckProcMount() Check { Versions: []VersionedCheck{ { MinimumVersion: api.MajorMinorVersion(1, 0), - CheckPod: checkProcMount_1_0, + CheckPod: procMount_1_0, }, }, } } -func checkProcMount_1_0(podMetadata *metav1.ObjectMeta, podSpec *corev1.PodSpec) CheckResult { - forbiddenContainers := sets.NewString() +func procMount_1_0(podMetadata *metav1.ObjectMeta, podSpec *corev1.PodSpec) CheckResult { + var badContainers []string forbiddenProcMountTypes := sets.NewString() visitContainersWithPath(podSpec, field.NewPath("spec"), func(container *corev1.Container, path *field.Path) { // allow if the security context is nil. @@ -61,18 +73,19 @@ func checkProcMount_1_0(podMetadata *metav1.ObjectMeta, podSpec *corev1.PodSpec) } // check if the value of the proc mount type is valid. if *container.SecurityContext.ProcMount != v1.DefaultProcMount { - forbiddenContainers.Insert(container.Name) + badContainers = append(badContainers, container.Name) forbiddenProcMountTypes.Insert(string(*container.SecurityContext.ProcMount)) } }) - if len(forbiddenContainers) > 0 { + if len(badContainers) > 0 { return CheckResult{ Allowed: false, - ForbiddenReason: "forbidden procMount", + ForbiddenReason: "procMount", ForbiddenDetail: fmt.Sprintf( - "containers %q have forbidden procMount types %q", - forbiddenContainers.List(), - forbiddenProcMountTypes.List(), + "%s %s must not set securityContext.procMount to %s", + pluralize("container", "containers", len(badContainers)), + joinQuote(badContainers), + joinQuote(forbiddenProcMountTypes.List()), ), } } diff --git a/staging/src/k8s.io/pod-security-admission/policy/check_procMount_test.go b/staging/src/k8s.io/pod-security-admission/policy/check_procMount_test.go new file mode 100644 index 00000000000..8fd77c9dc6e --- /dev/null +++ b/staging/src/k8s.io/pod-security-admission/policy/check_procMount_test.go @@ -0,0 +1,66 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package policy + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" +) + +func TestProcMount(t *testing.T) { + defaultValue := corev1.DefaultProcMount + unmaskedValue := corev1.UnmaskedProcMount + otherValue := corev1.ProcMountType("other") + + tests := []struct { + name string + pod *corev1.Pod + expectReason string + expectDetail string + }{ + { + name: "procMount", + pod: &corev1.Pod{Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "a", SecurityContext: nil}, + {Name: "b", SecurityContext: &corev1.SecurityContext{}}, + {Name: "c", SecurityContext: &corev1.SecurityContext{ProcMount: &defaultValue}}, + {Name: "d", SecurityContext: &corev1.SecurityContext{ProcMount: &unmaskedValue}}, + {Name: "e", SecurityContext: &corev1.SecurityContext{ProcMount: &otherValue}}, + }, + }}, + expectReason: `procMount`, + expectDetail: `containers "d", "e" must not set securityContext.procMount to "Unmasked", "other"`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := procMount_1_0(&tc.pod.ObjectMeta, &tc.pod.Spec) + if result.Allowed { + t.Fatal("expected disallowed") + } + if e, a := tc.expectReason, result.ForbiddenReason; e != a { + t.Errorf("expected\n%s\ngot\n%s", e, a) + } + if e, a := tc.expectDetail, result.ForbiddenDetail; e != a { + t.Errorf("expected\n%s\ngot\n%s", e, a) + } + }) + } +} diff --git a/staging/src/k8s.io/pod-security-admission/test/fixtures_procMount.go b/staging/src/k8s.io/pod-security-admission/test/fixtures_procMount.go index 83ad61ee956..bfae3a33a3e 100644 --- a/staging/src/k8s.io/pod-security-admission/test/fixtures_procMount.go +++ b/staging/src/k8s.io/pod-security-admission/test/fixtures_procMount.go @@ -25,7 +25,7 @@ import ( func init() { fixtureData_1_0 := fixtureGenerator{ - expectErrorSubstring: "forbidden procMount", + expectErrorSubstring: "procMount", generatePass: func(p *v1.Pod) []*v1.Pod { p = ensureSecurityContext(p) return []*corev1.Pod{ @@ -43,13 +43,13 @@ func init() { return []*corev1.Pod{ // set proc mount of container to a forbidden value tweak(p, func(copy *v1.Pod) { - inValidProcMountType := v1.UnmaskedProcMount - copy.Spec.Containers[0].SecurityContext.ProcMount = &inValidProcMountType + unmaskedProcMountType := v1.UnmaskedProcMount + copy.Spec.Containers[0].SecurityContext.ProcMount = &unmaskedProcMountType }), // set proc mount of init container to a forbidden value tweak(p, func(copy *v1.Pod) { - inValidProcMountType := v1.UnmaskedProcMount - copy.Spec.InitContainers[0].SecurityContext.ProcMount = &inValidProcMountType + unmaskedProcMountType := v1.UnmaskedProcMount + copy.Spec.InitContainers[0].SecurityContext.ProcMount = &unmaskedProcMountType }), } },