Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug 2086519: scc: set container nonroot=true if the pod/container RunAsUser is non-zero #85

Merged
merged 1 commit into from
Jun 17, 2022
Merged

Bug 2086519: scc: set container nonroot=true if the pod/container RunAsUser is non-zero #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions pkg/securitycontextconstraints/sccadmission/admission_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ func TestAdmitSuccess(t *testing.T) {

seLinuxLevelFromNamespace := namespace.Annotations[securityv1.MCSAnnotation]

trueVal := true
stlaz marked this conversation as resolved.
Show resolved Hide resolved
testCases := map[string]struct {
pod *coreapi.Pod
expectedPodSC *coreapi.PodSecurityContext
Expand All @@ -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),
},
}

Expand Down Expand Up @@ -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{
Expand Down
21 changes: 18 additions & 3 deletions pkg/securitycontextconstraints/sccmatching/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A future me wouldn't know how why this is the case and how this is related to kubelet's logic. Please add some more rationale and references why this is useful.

// 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 {
stlaz marked this conversation as resolved.
Show resolved Hide resolved
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)
stlaz marked this conversation as resolved.
Show resolved Hide resolved
}
}

caps, err := s.capabilitiesStrategy.Generate(pod, container)
Expand Down
149 changes: 149 additions & 0 deletions pkg/securitycontextconstraints/sccmatching/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down