Merge pull request #47009 from yujuhong/run-as-non-root

Automatic merge from submit-queue (batch tested with PRs 46775, 47009)

kuberuntime: check the value of RunAsNonRoot when verifying

The verification function is fixed to check the value of RunAsNonRoot,
not just the existence of it. Also adds unit tests to verify the correct
behavior.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #46996

**Release note**:

```release-note
Fix the bug where container cannot run as root when SecurityContext.RunAsNonRoot is false.
```

pkg/kubelet/kuberuntime/kuberuntime_container_test.go

@@ -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",
@@ -244,7 +244,7 @@ func TestGenerateContainerConfig(t *testing.T) {
 					Command:         []string{"testCommand"},
 					WorkingDir:      "testWorkingDir",
 					SecurityContext: &v1.SecurityContext{
-						RunAsNonRoot: &RunAsNonRoot,
+						RunAsNonRoot: &runAsNonRootTrue,
 						RunAsUser:    &runAsUser,
 					},
 				},

pkg/kubelet/kuberuntime/security_context.go

@@ -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 {
 		return nil
 	}
 

pkg/kubelet/kuberuntime/security_context_test.go

@@ -45,60 +45,72 @@ func TestVerifyRunAsNonRoot(t *testing.T) {
 		},
 	}
 
+	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,
+		},
+		{
+			desc: "Pass if RunAsNonRoot is not set",
+			sc: &v1.SecurityContext{
+				RunAsUser: &rootUser,
+			},
+			imageUser: imageRootUser,
+			fail:      false,
+		},
+		{
+			desc: "Pass if RunAsNonRoot is false (image user is root)",
+			sc: &v1.SecurityContext{
+				RunAsNonRoot: &runAsNonRootFalse,
+			},
+			imageUser: imageRootUser,
+			fail:      false,
+		},
+		{
+			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))
-	assert.NoError(t, err)
-
-	runAsUser := types.UnixUserID(0)
-	RunAsNonRoot := false
-	podWithContainerSecurityContext := &v1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			UID:       "12345678",
-			Name:      "bar",
-			Namespace: "new",
-		},
-		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,
-					},
-				},
-			},
-		},
+		if test.fail {
+			assert.Error(t, err, test.desc)
+		} else {
+			assert.NoError(t, err, test.desc)
 		}
-
-	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",
-		},
-		Spec: v1.PodSpec{
-			Containers: []v1.Container{
-				{
-					Name:            "foo",
-					Image:           "busybox",
-					ImagePullPolicy: v1.PullIfNotPresent,
-					Command:         []string{"testCommand"},
-					WorkingDir:      "testWorkingDir",
-					SecurityContext: &v1.SecurityContext{
-						RunAsNonRoot: &RunAsNonRoot,
-					},
-				},
-			},
-		},
 	}
-
-	err3 := verifyRunAsNonRoot(podWithContainerSecurityContext, &podWithContainerSecurityContext.Spec.Containers[0], int64(0))
-	assert.EqualError(t, err3, "container has runAsNonRoot and image will run as root")
 }