Merge pull request #92355 from wawa0210/fix-91482

fix windows container root validate

pkg/kubelet/kuberuntime/BUILD

@@ -28,6 +28,8 @@ go_library(
         "labels.go",
         "legacy.go",
         "security_context.go",
+        "security_context_others.go",
+        "security_context_windows.go",
     ],
     importpath = "k8s.io/kubernetes/pkg/kubelet/kuberuntime",
     deps = [
@@ -106,7 +108,8 @@ go_test(
         "kuberuntime_sandbox_test.go",
         "labels_test.go",
         "legacy_test.go",
-        "security_context_test.go",
+        "security_context_others_test.go",
+        "security_context_windows_test.go",
     ],
     embed = [":go_default_library"],
     deps = [

pkg/kubelet/kuberuntime/security_context.go

@@ -17,8 +17,6 @@ limitations under the License.
 package kuberuntime
 
 import (
-	"fmt"
-
 	v1 "k8s.io/api/core/v1"
 	runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
 	"k8s.io/kubernetes/pkg/security/apparmor"
@@ -76,31 +74,6 @@ func (m *kubeGenericRuntimeManager) determineEffectiveSecurityContext(pod *v1.Po
 	return synthesized
 }
 
-// verifyRunAsNonRoot verifies RunAsNonRoot.
-func verifyRunAsNonRoot(pod *v1.Pod, container *v1.Container, uid *int64, username string) error {
-	effectiveSc := securitycontext.DetermineEffectiveSecurityContext(pod, container)
-	// 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
-	}
-
-	if effectiveSc.RunAsUser != nil {
-		if *effectiveSc.RunAsUser == 0 {
-			return fmt.Errorf("container's runAsUser breaks non-root policy")
-		}
-		return nil
-	}
-
-	switch {
-	case uid != nil && *uid == 0:
-		return fmt.Errorf("container has runAsNonRoot and image will run as root")
-	case uid == nil && len(username) > 0:
-		return fmt.Errorf("container has runAsNonRoot and image has non-numeric user (%s), cannot verify user is non-root", username)
-	default:
-		return nil
-	}
-}
-
 // convertToRuntimeSecurityContext converts v1.SecurityContext to runtimeapi.SecurityContext.
 func convertToRuntimeSecurityContext(securityContext *v1.SecurityContext) *runtimeapi.LinuxContainerSecurityContext {
 	if securityContext == nil {

pkg/kubelet/kuberuntime/security_context_others.go

@@ -0,0 +1,51 @@
+// +build !windows
+
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package kuberuntime
+
+import (
+	"fmt"
+
+	"k8s.io/api/core/v1"
+	"k8s.io/kubernetes/pkg/securitycontext"
+)
+
+// verifyRunAsNonRoot verifies RunAsNonRoot.
+func verifyRunAsNonRoot(pod *v1.Pod, container *v1.Container, uid *int64, username string) error {
+	effectiveSc := securitycontext.DetermineEffectiveSecurityContext(pod, container)
+	// 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
+	}
+
+	if effectiveSc.RunAsUser != nil {
+		if *effectiveSc.RunAsUser == 0 {
+			return fmt.Errorf("container's runAsUser breaks non-root policy")
+		}
+		return nil
+	}
+
+	switch {
+	case uid != nil && *uid == 0:
+		return fmt.Errorf("container has runAsNonRoot and image will run as root")
+	case uid == nil && len(username) > 0:
+		return fmt.Errorf("container has runAsNonRoot and image has non-numeric user (%s), cannot verify user is non-root", username)
+	default:
+		return nil
+	}
+}

pkg/kubelet/kuberuntime/security_context_others_test.go

@@ -1,5 +1,7 @@
+// +build !windows
+
 /*
-Copyright 2016 The Kubernetes Authors.
+Copyright 2020 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

pkg/kubelet/kuberuntime/security_context_windows.go

@@ -0,0 +1,65 @@
+// +build windows
+
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package kuberuntime
+
+import (
+	"fmt"
+	"k8s.io/api/core/v1"
+	"k8s.io/klog/v2"
+	"k8s.io/kubernetes/pkg/securitycontext"
+)
+
+var (
+	windowsRootUserName = "ContainerAdministrator"
+)
+
+// verifyRunAsNonRoot verifies RunAsNonRoot on windows.
+// https://github.com/kubernetes/enhancements/blob/master/keps/sig-windows/20190103-windows-node-support.md#v1container
+// Windows does not have a root user, we cannot judge the root identity of the windows container by the way to judge the root(uid=0) of the linux container.
+// According to the discussion of sig-windows, at present, we assume that ContainerAdministrator is the windows container root user,
+// and then optimize this logic according to the best time.
+// https://docs.google.com/document/d/1Tjxzjjuy4SQsFSUVXZbvqVb64hjNAG5CQX8bK7Yda9w
+func verifyRunAsNonRoot(pod *v1.Pod, container *v1.Container, uid *int64, username string) error {
+	effectiveSc := securitycontext.DetermineEffectiveSecurityContext(pod, container)
+	// 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
+	}
+	if effectiveSc.RunAsUser != nil {
+		klog.Warningf("Windows container does not support SecurityContext.RunAsUser, please use SecurityContext.WindowsOptions")
+	}
+	if effectiveSc.SELinuxOptions != nil {
+		klog.Warningf("Windows container does not support SecurityContext.SELinuxOptions, please use SecurityContext.WindowsOptions")
+	}
+	if effectiveSc.RunAsGroup != nil {
+		klog.Warningf("Windows container does not support SecurityContext.RunAsGroup")
+	}
+	if effectiveSc.WindowsOptions != nil {
+		if effectiveSc.WindowsOptions.RunAsUserName != nil {
+			if *effectiveSc.WindowsOptions.RunAsUserName == windowsRootUserName {
+				return fmt.Errorf("container's runAsUser (%s) which will be regarded as root identity and will break non-root policy", username)
+			}
+			return nil
+		}
+	}
+	if len(username) > 0 && username == windowsRootUserName {
+		return fmt.Errorf("container's runAsUser (%s) which will be regarded as root identity and will break non-root policy", username)
+	}
+	return nil
+}

pkg/kubelet/kuberuntime/security_context_windows_test.go

@@ -0,0 +1,135 @@
+// +build windows
+
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package kuberuntime
+
+import (
+	"k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestVerifyRunAsNonRoot(t *testing.T) {
+	pod := &v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			UID:       "12345678",
+			Name:      "bar",
+			Namespace: "new",
+		},
+		Spec: v1.PodSpec{
+			Containers: []v1.Container{
+				{
+					Name:            "foo",
+					Image:           "windows",
+					ImagePullPolicy: v1.PullIfNotPresent,
+					Command:         []string{"testCommand"},
+					WorkingDir:      "testWorkingDir",
+				},
+			},
+		},
+	}
+	rootUser := "ContainerAdministrator"
+	anyUser := "anyone"
+	runAsNonRootTrue := true
+	runAsNonRootFalse := false
+	for _, test := range []struct {
+		desc     string
+		sc       *v1.SecurityContext
+		uid      *int64
+		username string
+		fail     bool
+	}{
+		{
+			desc:     "Pass if SecurityContext is not set",
+			sc:       nil,
+			username: rootUser,
+			fail:     false,
+		},
+		{
+			desc: "Pass if RunAsNonRoot is not set",
+			sc: &v1.SecurityContext{
+				RunAsNonRoot: nil,
+			},
+			username: rootUser,
+			fail:     false,
+		},
+		{
+			desc: "Pass if RunAsNonRoot is false (image user is root)",
+			sc: &v1.SecurityContext{
+				RunAsNonRoot: &runAsNonRootFalse,
+			},
+			username: rootUser,
+			fail:     false,
+		},
+		{
+			desc: "Pass if RunAsNonRoot is false (WindowsOptions RunAsUserName is root)",
+			sc: &v1.SecurityContext{
+				RunAsNonRoot: &runAsNonRootFalse,
+				WindowsOptions: &v1.WindowsSecurityContextOptions{
+					RunAsUserName: &rootUser,
+				},
+			},
+			username: rootUser,
+			fail:     false,
+		},
+		{
+			desc: "Fail if container's RunAsUser is root and RunAsNonRoot is true",
+			sc: &v1.SecurityContext{
+				RunAsNonRoot: &runAsNonRootTrue,
+				WindowsOptions: &v1.WindowsSecurityContextOptions{
+					RunAsUserName: &rootUser,
+				},
+			},
+			username: rootUser,
+			fail:     true,
+		},
+		{
+			desc: "Fail if image's user is root and RunAsNonRoot is true",
+			sc: &v1.SecurityContext{
+				RunAsNonRoot: &runAsNonRootTrue,
+			},
+			username: rootUser,
+			fail:     true,
+		},
+		{
+			desc: "Pass if image's user is non-root and RunAsNonRoot is true",
+			sc: &v1.SecurityContext{
+				RunAsNonRoot: &runAsNonRootTrue,
+			},
+			username: anyUser,
+			fail:     false,
+		},
+		{
+			desc: "Pass if container's user and image's user aren't set and RunAsNonRoot is true",
+			sc: &v1.SecurityContext{
+				RunAsNonRoot: &runAsNonRootTrue,
+			},
+			fail: false,
+		},
+	} {
+		pod.Spec.Containers[0].SecurityContext = test.sc
+		err := verifyRunAsNonRoot(pod, &pod.Spec.Containers[0], test.uid, test.username)
+		if test.fail {
+			assert.Error(t, err, test.desc)
+		} else {
+			assert.NoError(t, err, test.desc)
+		}
+	}
+}