From 7fad6226161c83a032a50cea6d6b1b9df0b3a45f Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Tue, 23 Jul 2024 11:04:45 -0400 Subject: [PATCH] PSA: allow procMount type Unmasked in baseline a masked proc mount has traditionally been used to prevent untrusted containers from accessing leaky kernel APIs. However, within a user namespace, typical ID checks protect better than masked proc. Further, allowing unmasked proc with a user namespace gives access to a container mounting sub procs, which opens avenues for container-in-container use cases. Update PSS for baseline to allow a container to access an unmasked /proc, if it's in a user namespace and if the UserNamespacesPodSecurityStandards feature is enabled. Signed-off-by: Peter Hunt Kubernetes-commit: 17521f04a40c8e21a22cfaa6725797d2d2ce71a8 --- policy/check_procMount.go | 11 +++++++++ policy/check_procMount_test.go | 42 +++++++++++++++++++++++++++------- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/policy/check_procMount.go b/policy/check_procMount.go index 282dbb3..8ec585b 100644 --- a/policy/check_procMount.go +++ b/policy/check_procMount.go @@ -35,6 +35,9 @@ spec.initContainers[*].securityContext.procMount **Allowed Values:** undefined/null, "Default" +However, if the pod is in a user namespace (`hostUsers: false`), and the +UserNamespacesPodSecurityStandards feature is enabled, all values are allowed. + */ func init() { @@ -58,6 +61,14 @@ func CheckProcMount() Check { } func procMount_1_0(podMetadata *metav1.ObjectMeta, podSpec *corev1.PodSpec) CheckResult { + // TODO: When we remove the UserNamespacesPodSecurityStandards feature gate (and GA this relaxation), + // create a new policy version. + // Note: pod validation will check for well formed procMount type, so avoid double validation and allow everything + // here. + if relaxPolicyForUserNamespacePod(podSpec) { + return CheckResult{Allowed: true} + } + var badContainers []string forbiddenProcMountTypes := sets.NewString() visitContainers(podSpec, func(container *corev1.Container) { diff --git a/policy/check_procMount_test.go b/policy/check_procMount_test.go index 1f1c833..576be1e 100644 --- a/policy/check_procMount_test.go +++ b/policy/check_procMount_test.go @@ -29,10 +29,12 @@ func TestProcMount(t *testing.T) { hostUsers := false tests := []struct { - name string - pod *corev1.Pod - expectReason string - expectDetail string + name string + pod *corev1.Pod + expectReason string + expectDetail string + expectAllowed bool + relaxForUserNS bool }{ { name: "procMount", @@ -46,16 +48,40 @@ func TestProcMount(t *testing.T) { }, HostUsers: &hostUsers, }}, - expectReason: `procMount`, - expectDetail: `containers "d", "e" must not set securityContext.procMount to "Unmasked", "other"`, + expectReason: `procMount`, + expectAllowed: false, + expectDetail: `containers "d", "e" must not set securityContext.procMount to "Unmasked", "other"`, + }, + { + 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}}, + }, + HostUsers: &hostUsers, + }}, + expectReason: "", + expectDetail: "", + expectAllowed: true, + relaxForUserNS: true, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + if tc.relaxForUserNS { + RelaxPolicyForUserNamespacePods(true) + t.Cleanup(func() { + RelaxPolicyForUserNamespacePods(false) + }) + } result := procMount_1_0(&tc.pod.ObjectMeta, &tc.pod.Spec) - if result.Allowed { - t.Fatal("expected disallowed") + if result.Allowed != tc.expectAllowed { + t.Fatalf("expected Allowed to be %v was %v", tc.expectAllowed, result.Allowed) } if e, a := tc.expectReason, result.ForbiddenReason; e != a { t.Errorf("expected\n%s\ngot\n%s", e, a)