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.

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

kuberuntime: check the value of RunAsNonRoot when verifying #47009

Merged
merged 2 commits into from Jun 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/kubelet/kuberuntime/kuberuntime_container_test.go
Expand Up @@ -228,7 +228,7 @@ func TestGenerateContainerConfig(t *testing.T) {
assert.Equal(t, expectedConfig, containerConfig, "generate container config for kubelet runtime v1.")

runAsUser := types.UnixUserID(0)
RunAsNonRoot := false
runAsNonRootTrue := true
podWithContainerSecurityContext := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
UID: "12345678",
Expand All @@ -244,7 +244,7 @@ func TestGenerateContainerConfig(t *testing.T) {
Command: []string{"testCommand"},
WorkingDir: "testWorkingDir",
SecurityContext: &v1.SecurityContext{
RunAsNonRoot: &RunAsNonRoot,
RunAsNonRoot: &runAsNonRootTrue,
Copy link
Member

Choose a reason for hiding this comment

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

What is the following expectedConfig used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably can be removed. @feiskyer
But I don't want to turn this into a test cleanup PR. That can be done in a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

Missing one assertion

	assert.Equal(t, expectedConfig, containerConfig, "generate container config for kubelet runtime v1.")

RunAsUser: &runAsUser,
},
},
Expand Down
3 changes: 2 additions & 1 deletion pkg/kubelet/kuberuntime/security_context.go
Expand Up @@ -72,7 +72,8 @@ func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Po
// verifyRunAsNonRoot verifies RunAsNonRoot.
func verifyRunAsNonRoot(pod *v1.Pod, container *v1.Container, uid int64) error {
effectiveSc := securitycontext.DetermineEffectiveSecurityContext(pod, container)
if effectiveSc == nil || effectiveSc.RunAsNonRoot == nil {
// If the option is not set, or if running as root is allowed, return nil.
if effectiveSc == nil || effectiveSc.RunAsNonRoot == nil || !*effectiveSc.RunAsNonRoot {
Copy link
Member

Choose a reason for hiding this comment

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

effectiveSc.GetRunAsNonRoot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a k8s api pod object. I don't think we have getters for them.

Copy link
Member

Choose a reason for hiding this comment

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

ACK. Confused initially.

return nil
}

Expand Down
110 changes: 61 additions & 49 deletions pkg/kubelet/kuberuntime/security_context_test.go
Expand Up @@ -45,60 +45,72 @@ func TestVerifyRunAsNonRoot(t *testing.T) {
},
}

err := verifyRunAsNonRoot(pod, &pod.Spec.Containers[0], int64(0))
assert.NoError(t, err)

runAsUser := types.UnixUserID(0)
RunAsNonRoot := false
podWithContainerSecurityContext := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
UID: "12345678",
Name: "bar",
Namespace: "new",
rootUser := types.UnixUserID(0)
runAsNonRootTrue := true
runAsNonRootFalse := false
imageRootUser := int64(0)
imageNonRootUser := int64(123)
for _, test := range []struct {
desc string
sc *v1.SecurityContext
imageUser int64
fail bool
}{
{
desc: "Pass if SecurityContext is not set",
sc: nil,
imageUser: imageRootUser,
fail: false,
},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "foo",
Image: "busybox",
ImagePullPolicy: v1.PullIfNotPresent,
Command: []string{"testCommand"},
WorkingDir: "testWorkingDir",
SecurityContext: &v1.SecurityContext{
RunAsNonRoot: &RunAsNonRoot,
RunAsUser: &runAsUser,
},
},
{
desc: "Pass if RunAsNonRoot is not set",
sc: &v1.SecurityContext{
RunAsUser: &rootUser,
},
imageUser: imageRootUser,
fail: false,
},
}

err2 := verifyRunAsNonRoot(podWithContainerSecurityContext, &podWithContainerSecurityContext.Spec.Containers[0], int64(0))
assert.EqualError(t, err2, "container's runAsUser breaks non-root policy")

RunAsNonRoot = false
podWithContainerSecurityContext = &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
UID: "12345678",
Name: "bar",
Namespace: "new",
{
desc: "Pass if RunAsNonRoot is false (image user is root)",
sc: &v1.SecurityContext{
RunAsNonRoot: &runAsNonRootFalse,
},
imageUser: imageRootUser,
fail: false,
},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "foo",
Image: "busybox",
ImagePullPolicy: v1.PullIfNotPresent,
Command: []string{"testCommand"},
WorkingDir: "testWorkingDir",
SecurityContext: &v1.SecurityContext{
RunAsNonRoot: &RunAsNonRoot,
},
},
{
desc: "Pass if RunAsNonRoot is false (RunAsUser is root)",
sc: &v1.SecurityContext{
RunAsNonRoot: &runAsNonRootFalse,
RunAsUser: &rootUser,
},
imageUser: imageNonRootUser,
fail: false,
},
{
desc: "Fail if container's RunAsUser is root and RunAsNonRoot is true",
sc: &v1.SecurityContext{
RunAsNonRoot: &runAsNonRootTrue,
RunAsUser: &rootUser,
},
imageUser: imageNonRootUser,
fail: true,
},
{
desc: "Fail if image's user is root and RunAsNonRoot is true",
sc: &v1.SecurityContext{
RunAsNonRoot: &runAsNonRootTrue,
},
imageUser: imageRootUser,
fail: true,
},
} {
pod.Spec.Containers[0].SecurityContext = test.sc
err := verifyRunAsNonRoot(pod, &pod.Spec.Containers[0], int64(0))
if test.fail {
assert.Error(t, err, test.desc)
} else {
assert.NoError(t, err, test.desc)
}
}

err3 := verifyRunAsNonRoot(podWithContainerSecurityContext, &podWithContainerSecurityContext.Spec.Containers[0], int64(0))
assert.EqualError(t, err3, "container has runAsNonRoot and image will run as root")
}