From 17521f04a40c8e21a22cfaa6725797d2d2ce71a8 Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Tue, 23 Jul 2024 11:04:45 -0400 Subject: [PATCH 1/2] 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 --- .../policy/check_procMount.go | 11 +++++ .../policy/check_procMount_test.go | 42 +++++++++++++++---- 2 files changed, 45 insertions(+), 8 deletions(-) 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 282dbb32ce9..8ec585b9c98 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 @@ -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/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 index 1f1c833e50a..576be1e743d 100644 --- 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 @@ -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) From 7e750a62a18488c01b6c1c60a33f46ae307aefe9 Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Tue, 23 Jul 2024 11:59:53 -0400 Subject: [PATCH 2/2] PSA: small cleanups for tests that use RelaxPolicyForUserNamespacePods make sure to cleanup after setting RelaxPolicyForUserNamespacePods setup test variables to be a little more terse and similar between tests cleanup Allowed checking Signed-off-by: Peter Hunt --- .../policy/check_runAsNonRoot_test.go | 33 +++++++------- .../policy/check_runAsUser_test.go | 45 +++++++++---------- 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/staging/src/k8s.io/pod-security-admission/policy/check_runAsNonRoot_test.go b/staging/src/k8s.io/pod-security-admission/policy/check_runAsNonRoot_test.go index e8067496cdd..04c58204195 100644 --- a/staging/src/k8s.io/pod-security-admission/policy/check_runAsNonRoot_test.go +++ b/staging/src/k8s.io/pod-security-admission/policy/check_runAsNonRoot_test.go @@ -25,12 +25,12 @@ import ( func TestRunAsNonRoot(t *testing.T) { tests := []struct { - name string - pod *corev1.Pod - expectReason string - expectDetail string - allowed bool - enableUserNamespacesPodSecurityStandards bool + name string + pod *corev1.Pod + expectReason string + expectDetail string + expectAllowed bool + relaxForUserNS bool }{ { name: "no explicit runAsNonRoot", @@ -87,8 +87,8 @@ func TestRunAsNonRoot(t *testing.T) { pod: &corev1.Pod{Spec: corev1.PodSpec{ HostUsers: utilpointer.Bool(false), }}, - allowed: true, - enableUserNamespacesPodSecurityStandards: true, + expectAllowed: true, + relaxForUserNS: true, }, { name: "UserNamespacesPodSecurityStandards enabled with HostUsers", @@ -98,21 +98,24 @@ func TestRunAsNonRoot(t *testing.T) { }, HostUsers: utilpointer.Bool(true), }}, - expectReason: `runAsNonRoot != true`, - expectDetail: `pod or container "a" must set securityContext.runAsNonRoot=true`, - allowed: false, - enableUserNamespacesPodSecurityStandards: true, + expectReason: `runAsNonRoot != true`, + expectDetail: `pod or container "a" must set securityContext.runAsNonRoot=true`, + expectAllowed: false, + relaxForUserNS: true, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - if tc.enableUserNamespacesPodSecurityStandards { + if tc.relaxForUserNS { RelaxPolicyForUserNamespacePods(true) + t.Cleanup(func() { + RelaxPolicyForUserNamespacePods(false) + }) } result := runAsNonRoot_1_0(&tc.pod.ObjectMeta, &tc.pod.Spec) - if result.Allowed && !tc.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) diff --git a/staging/src/k8s.io/pod-security-admission/policy/check_runAsUser_test.go b/staging/src/k8s.io/pod-security-admission/policy/check_runAsUser_test.go index 490b07257cc..d47ab1921a4 100644 --- a/staging/src/k8s.io/pod-security-admission/policy/check_runAsUser_test.go +++ b/staging/src/k8s.io/pod-security-admission/policy/check_runAsUser_test.go @@ -25,12 +25,12 @@ import ( func TestRunAsUser(t *testing.T) { tests := []struct { - name string - pod *corev1.Pod - expectAllow bool - expectReason string - expectDetail string - enableUserNamespacesPodSecurityStandards bool + name string + pod *corev1.Pod + expectAllowed bool + expectReason string + expectDetail string + relaxForUserNS bool }{ { name: "pod runAsUser=0", @@ -51,7 +51,7 @@ func TestRunAsUser(t *testing.T) { {Name: "a", SecurityContext: nil}, }, }}, - expectAllow: true, + expectAllowed: true, }, { name: "pod runAsUser=nil", @@ -61,7 +61,7 @@ func TestRunAsUser(t *testing.T) { {Name: "a", SecurityContext: nil}, }, }}, - expectAllow: true, + expectAllowed: true, }, { name: "containers runAsUser=0", @@ -89,15 +89,15 @@ func TestRunAsUser(t *testing.T) { {Name: "f", SecurityContext: &corev1.SecurityContext{RunAsUser: utilpointer.Int64(4)}}, }, }}, - expectAllow: true, + expectAllowed: true, }, { name: "UserNamespacesPodSecurityStandards enabled without HostUsers", pod: &corev1.Pod{Spec: corev1.PodSpec{ HostUsers: utilpointer.Bool(false), }}, - expectAllow: true, - enableUserNamespacesPodSecurityStandards: true, + expectAllowed: true, + relaxForUserNS: true, }, { name: "UserNamespacesPodSecurityStandards enabled with HostUsers", @@ -108,27 +108,24 @@ func TestRunAsUser(t *testing.T) { }, HostUsers: utilpointer.Bool(true), }}, - expectAllow: false, - expectReason: `runAsUser=0`, - expectDetail: `pod must not set runAsUser=0`, - enableUserNamespacesPodSecurityStandards: true, + expectAllowed: false, + expectReason: `runAsUser=0`, + expectDetail: `pod must not set runAsUser=0`, + relaxForUserNS: true, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - if tc.enableUserNamespacesPodSecurityStandards { + if tc.relaxForUserNS { RelaxPolicyForUserNamespacePods(true) + t.Cleanup(func() { + RelaxPolicyForUserNamespacePods(false) + }) } result := runAsUser_1_23(&tc.pod.ObjectMeta, &tc.pod.Spec) - if tc.expectAllow { - if !result.Allowed { - t.Fatalf("expected to be allowed, disallowed: %s, %s", result.ForbiddenReason, result.ForbiddenDetail) - } - return - } - 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)