From ccde63b9c16241780f42d25550c200aa38e53989 Mon Sep 17 00:00:00 2001 From: wawa0210 Date: Sun, 21 Jun 2020 12:01:16 +0800 Subject: [PATCH] fix windows container root validate --- pkg/kubelet/kuberuntime/BUILD | 5 +- pkg/kubelet/kuberuntime/security_context.go | 27 ---- .../kuberuntime/security_context_others.go | 51 +++++++ ...est.go => security_context_others_test.go} | 4 +- .../kuberuntime/security_context_windows.go | 65 +++++++++ .../security_context_windows_test.go | 135 ++++++++++++++++++ 6 files changed, 258 insertions(+), 29 deletions(-) create mode 100644 pkg/kubelet/kuberuntime/security_context_others.go rename pkg/kubelet/kuberuntime/{security_context_test.go => security_context_others_test.go} (98%) create mode 100644 pkg/kubelet/kuberuntime/security_context_windows.go create mode 100644 pkg/kubelet/kuberuntime/security_context_windows_test.go diff --git a/pkg/kubelet/kuberuntime/BUILD b/pkg/kubelet/kuberuntime/BUILD index f22fa2f23b9..c5a2ca980b0 100644 --- a/pkg/kubelet/kuberuntime/BUILD +++ b/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 = [ @@ -105,7 +107,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 = [ diff --git a/pkg/kubelet/kuberuntime/security_context.go b/pkg/kubelet/kuberuntime/security_context.go index c8ebe103dbf..0401125d092 100644 --- a/pkg/kubelet/kuberuntime/security_context.go +++ b/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 { diff --git a/pkg/kubelet/kuberuntime/security_context_others.go b/pkg/kubelet/kuberuntime/security_context_others.go new file mode 100644 index 00000000000..1f847a56784 --- /dev/null +++ b/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 + } +} diff --git a/pkg/kubelet/kuberuntime/security_context_test.go b/pkg/kubelet/kuberuntime/security_context_others_test.go similarity index 98% rename from pkg/kubelet/kuberuntime/security_context_test.go rename to pkg/kubelet/kuberuntime/security_context_others_test.go index d0aa922d5f5..fe84a152c01 100644 --- a/pkg/kubelet/kuberuntime/security_context_test.go +++ b/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. diff --git a/pkg/kubelet/kuberuntime/security_context_windows.go b/pkg/kubelet/kuberuntime/security_context_windows.go new file mode 100644 index 00000000000..400116f7d1c --- /dev/null +++ b/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 +} diff --git a/pkg/kubelet/kuberuntime/security_context_windows_test.go b/pkg/kubelet/kuberuntime/security_context_windows_test.go new file mode 100644 index 00000000000..fd438050ba2 --- /dev/null +++ b/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) + } + } +}