From 46011031c17949b03ecaabb9c288860ce3438c20 Mon Sep 17 00:00:00 2001 From: Stanislav Laznicka Date: Wed, 15 Jun 2022 15:25:17 +0200 Subject: [PATCH] scc: set container nonroot=true if the pod/container RunAsUser is non-zero PodSecurity admission requires the non-zero flag to be set no matter the container/pod UID. --- .../sccadmission/admission_test.go | 16 +- .../sccmatching/provider.go | 21 ++- .../sccmatching/provider_test.go | 149 ++++++++++++++++++ 3 files changed, 176 insertions(+), 10 deletions(-) diff --git a/pkg/securitycontextconstraints/sccadmission/admission_test.go b/pkg/securitycontextconstraints/sccadmission/admission_test.go index 3f0cb1f35..610b649a8 100644 --- a/pkg/securitycontextconstraints/sccadmission/admission_test.go +++ b/pkg/securitycontextconstraints/sccadmission/admission_test.go @@ -310,6 +310,7 @@ func TestAdmitSuccess(t *testing.T) { seLinuxLevelFromNamespace := namespace.Annotations[securityv1.MCSAnnotation] + trueVal := true testCases := map[string]struct { pod *coreapi.Pod expectedPodSC *coreapi.PodSecurityContext @@ -318,27 +319,27 @@ func TestAdmitSuccess(t *testing.T) { "specifyUIDInRange": { pod: specifyUIDInRange, expectedPodSC: podSC(seLinuxLevelFromNamespace, defaultGroup, defaultGroup), - expectedContainerSC: containerSC(nil, goodUID), + expectedContainerSC: containerSC(nil, goodUID, &trueVal), }, "specifyLabels": { pod: specifyLabels, expectedPodSC: podSC(seLinuxLevelFromNamespace, defaultGroup, defaultGroup), - expectedContainerSC: containerSC(&seLinuxLevelFromNamespace, 1), + expectedContainerSC: containerSC(&seLinuxLevelFromNamespace, 1, &trueVal), }, "specifyFSGroup": { pod: specifyFSGroupInRange, expectedPodSC: podSC(seLinuxLevelFromNamespace, goodFSGroup, defaultGroup), - expectedContainerSC: containerSC(nil, 1), + expectedContainerSC: containerSC(nil, 1, &trueVal), }, "specifySupGroup": { pod: specifySupGroup, expectedPodSC: podSC(seLinuxLevelFromNamespace, defaultGroup, 3), - expectedContainerSC: containerSC(nil, 1), + expectedContainerSC: containerSC(nil, 1, &trueVal), }, "specifyPodLevelSELinuxLevel": { pod: specifyPodLevelSELinux, expectedPodSC: podSC(seLinuxLevelFromNamespace, defaultGroup, defaultGroup), - expectedContainerSC: containerSC(nil, 1), + expectedContainerSC: containerSC(nil, 1, &trueVal), }, } @@ -1344,9 +1345,10 @@ func linuxPod() *coreapi.Pod { } } -func containerSC(seLinuxLevel *string, uid int64) *coreapi.SecurityContext { +func containerSC(seLinuxLevel *string, uid int64, runAsNonRoot *bool) *coreapi.SecurityContext { sc := &coreapi.SecurityContext{ - RunAsUser: &uid, + RunAsUser: &uid, + RunAsNonRoot: runAsNonRoot, } if seLinuxLevel != nil { sc.SELinuxOptions = &coreapi.SELinuxOptions{ diff --git a/pkg/securitycontextconstraints/sccmatching/provider.go b/pkg/securitycontextconstraints/sccmatching/provider.go index 1121fb6a5..27853a152 100644 --- a/pkg/securitycontextconstraints/sccmatching/provider.go +++ b/pkg/securitycontextconstraints/sccmatching/provider.go @@ -170,9 +170,24 @@ func (s *simpleProvider) CreateContainerSecurityContext(pod *api.Pod, container // if we're using the non-root strategy set the marker that this container should not be // run as root which will signal to the kubelet to do a final check either on the runAsUser // or, if runAsUser is not set, the image - if sc.RunAsNonRoot() == nil && sc.RunAsUser() == nil && s.scc.RunAsUser.Type == securityv1.RunAsUserStrategyMustRunAsNonRoot { - nonRoot := true - sc.SetRunAsNonRoot(&nonRoot) + // Alternatively, also set the RunAsNonRoot to true in case the UID value is non-nil and non-zero + // to more easily satisfy the requirements of upstream PodSecurity admission "restricted" profile + // which currently requires all containers to have runAsNonRoot set to true, or to have this set + // in the whole pod's security context + if sc.RunAsNonRoot() == nil { + nonRoot := false + switch runAsUser := sc.RunAsUser(); { + case runAsUser == nil: + if s.scc.RunAsUser.Type == securityv1.RunAsUserStrategyMustRunAsNonRoot { + nonRoot = true + } + case *runAsUser > 0: + nonRoot = true + } + + if nonRoot { + sc.SetRunAsNonRoot(&nonRoot) + } } caps, err := s.capabilitiesStrategy.Generate(pod, container) diff --git a/pkg/securitycontextconstraints/sccmatching/provider_test.go b/pkg/securitycontextconstraints/sccmatching/provider_test.go index 3cbb384da..2127a69ea 100644 --- a/pkg/securitycontextconstraints/sccmatching/provider_test.go +++ b/pkg/securitycontextconstraints/sccmatching/provider_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/validation/field" @@ -932,6 +933,154 @@ func TestGenerateContainerSecurityContextReadOnlyRootFS(t *testing.T) { } } +func TestGenerateNonRootSecurityContextOnNonZeroRunAsUser(t *testing.T) { + userSCC := defaultSCC() + var minRange int64 = 100 + var maxRange int64 = 900 + userSCC.RunAsUser = securityv1.RunAsUserStrategyOptions{ + Type: securityv1.RunAsUserStrategyMustRunAsRange, + UIDRangeMin: &minRange, + UIDRangeMax: &maxRange, + } + + rootMinSCC := defaultSCC() + var rootUID int64 = 0 + rootMinSCC.RunAsUser = securityv1.RunAsUserStrategyOptions{ + Type: securityv1.RunAsUserStrategyMustRunAsRange, + UIDRangeMin: &rootUID, + UIDRangeMax: &maxRange, + } + + nonRootSCC := defaultSCC() + nonRootSCC.RunAsUser = securityv1.RunAsUserStrategyOptions{ + Type: securityv1.RunAsUserStrategyMustRunAsNonRoot, + } + + var useruid int64 = 500 + containerUserPod := defaultPod() + containerUserPod.Spec.Containers[0].SecurityContext.RunAsUser = &useruid + + podUserPod := defaultPod() + podUserPod.Spec.SecurityContext.RunAsUser = &useruid + + zeroPodUserPod := defaultPod() + zeroPodUserPod.Spec.SecurityContext.RunAsUser = &rootUID + + zeroContainerUserPod := defaultPod() + zeroContainerUserPod.Spec.Containers[0].SecurityContext.RunAsUser = &rootUID + + zeroPodUserPodNonRootSCC := zeroPodUserPod.DeepCopy() + zeroContainerUserPodNonRootSCC := zeroContainerUserPod.DeepCopy() + + falseVal := false + trueVal := true + tests := map[string]struct { + pod *api.Pod + scc *securityv1.SecurityContextConstraints + expectedSC *api.SecurityContext + }{ + "generate non-zero user": { + pod: defaultPod(), + scc: userSCC, + expectedSC: &api.SecurityContext{ + RunAsUser: &minRange, + RunAsNonRoot: &trueVal, + Privileged: &falseVal, + }, + }, + "generate zero user": { + pod: defaultPod(), + scc: rootMinSCC, + expectedSC: &api.SecurityContext{ + RunAsUser: &rootUID, + RunAsNonRoot: nil, + Privileged: &falseVal, + }, + }, + "nonzero user set on pod level": { + pod: podUserPod, + scc: userSCC, + expectedSC: &api.SecurityContext{ + RunAsUser: nil, + RunAsNonRoot: &trueVal, + Privileged: &falseVal, + }, + }, + "nonzero user set on container level": { + pod: containerUserPod, + scc: userSCC, + expectedSC: &api.SecurityContext{ + RunAsUser: &useruid, + RunAsNonRoot: &trueVal, + Privileged: &falseVal, + }, + }, + "zero user set on pod level": { + pod: zeroPodUserPod, + scc: rootMinSCC, + expectedSC: &api.SecurityContext{ + RunAsUser: nil, + RunAsNonRoot: nil, + Privileged: &falseVal, + }, + }, + "zero user set on container level": { + pod: zeroContainerUserPod, + scc: rootMinSCC, + expectedSC: &api.SecurityContext{ + RunAsUser: &rootUID, + RunAsNonRoot: nil, + Privileged: &falseVal, + }, + }, + "no user set, nonroot SCC": { + pod: defaultPod(), + scc: nonRootSCC, + expectedSC: &api.SecurityContext{ + RunAsUser: nil, + RunAsNonRoot: &trueVal, + Privileged: &falseVal, + }, + }, + "zero user set on pod level, nonroot SCC": { + pod: zeroPodUserPodNonRootSCC, + scc: nonRootSCC, + expectedSC: &api.SecurityContext{ + RunAsUser: nil, + RunAsNonRoot: nil, + Privileged: &falseVal, + }, + }, + "zero user set on container level, nonroot SCC": { + pod: zeroContainerUserPodNonRootSCC, + scc: nonRootSCC, + expectedSC: &api.SecurityContext{ + RunAsUser: &rootUID, + RunAsNonRoot: nil, + Privileged: &falseVal, + }, + }, + } + + for k, v := range tests { + provider, err := NewSimpleProvider(v.scc) + if err != nil { + t.Errorf("%s unable to create provider %v", k, err) + continue + } + sc, err := provider.CreateContainerSecurityContext(v.pod, &v.pod.Spec.Containers[0]) + if err != nil { + t.Errorf("%s unable to create container security context %v", k, err) + continue + } + + if !equality.Semantic.DeepEqual(v.expectedSC, sc) { + t.Errorf("%s expected security context does not match the actual: %s", k, diff.ObjectDiff(v.expectedSC, sc)) + } + + } +} + func defaultSCC() *securityv1.SecurityContextConstraints { return &securityv1.SecurityContextConstraints{ ObjectMeta: metav1.ObjectMeta{