From e6ad8f8e4896604e43785b7c2b97417d08929059 Mon Sep 17 00:00:00 2001 From: Yu-Ju Hong Date: Mon, 10 Jul 2017 17:47:17 -0700 Subject: [PATCH] dockershim: clean up unused security context code Also remove references to kubernetes api objects --- pkg/kubelet/dockershim/BUILD | 5 +- pkg/kubelet/dockershim/security_context.go | 11 +- .../dockershim/security_context_test.go | 25 +- pkg/kubelet/dockershim/securitycontext/BUILD | 53 --- pkg/kubelet/dockershim/securitycontext/doc.go | 18 - .../dockershim/securitycontext/fake.go | 35 -- .../dockershim/securitycontext/provider.go | 131 ------- .../securitycontext/provider_test.go | 334 ------------------ .../dockershim/securitycontext/types.go | 41 --- .../dockershim/securitycontext/util.go | 59 ---- pkg/kubelet/dockershim/selinux_util.go | 92 +++++ pkg/kubelet/dockershim/selinux_util_test.go | 54 +++ 12 files changed, 162 insertions(+), 696 deletions(-) delete mode 100644 pkg/kubelet/dockershim/securitycontext/BUILD delete mode 100644 pkg/kubelet/dockershim/securitycontext/doc.go delete mode 100644 pkg/kubelet/dockershim/securitycontext/fake.go delete mode 100644 pkg/kubelet/dockershim/securitycontext/provider.go delete mode 100644 pkg/kubelet/dockershim/securitycontext/provider_test.go delete mode 100644 pkg/kubelet/dockershim/securitycontext/types.go delete mode 100644 pkg/kubelet/dockershim/securitycontext/util.go create mode 100644 pkg/kubelet/dockershim/selinux_util.go create mode 100644 pkg/kubelet/dockershim/selinux_util_test.go diff --git a/pkg/kubelet/dockershim/BUILD b/pkg/kubelet/dockershim/BUILD index 915b6b8485d..38ddd56272f 100644 --- a/pkg/kubelet/dockershim/BUILD +++ b/pkg/kubelet/dockershim/BUILD @@ -27,6 +27,7 @@ go_library( "helpers_linux.go", "naming.go", "security_context.go", + "selinux_util.go", ], tags = ["automanaged"], deps = [ @@ -39,7 +40,6 @@ go_library( "//pkg/kubelet/dockershim/cm:go_default_library", "//pkg/kubelet/dockershim/errors:go_default_library", "//pkg/kubelet/dockershim/libdocker:go_default_library", - "//pkg/kubelet/dockershim/securitycontext:go_default_library", "//pkg/kubelet/leaky:go_default_library", "//pkg/kubelet/network:go_default_library", "//pkg/kubelet/network/cni:go_default_library", @@ -86,6 +86,7 @@ go_test( "helpers_test.go", "naming_test.go", "security_context_test.go", + "selinux_util_test.go", ], data = [ "fixtures/seccomp/sub/subtest", @@ -99,7 +100,6 @@ go_test( "//pkg/kubelet/container/testing:go_default_library", "//pkg/kubelet/dockershim/errors:go_default_library", "//pkg/kubelet/dockershim/libdocker:go_default_library", - "//pkg/kubelet/dockershim/securitycontext:go_default_library", "//pkg/kubelet/dockershim/testing:go_default_library", "//pkg/kubelet/network:go_default_library", "//pkg/kubelet/network/testing:go_default_library", @@ -134,7 +134,6 @@ filegroup( "//pkg/kubelet/dockershim/errors:all-srcs", "//pkg/kubelet/dockershim/libdocker:all-srcs", "//pkg/kubelet/dockershim/remote:all-srcs", - "//pkg/kubelet/dockershim/securitycontext:all-srcs", "//pkg/kubelet/dockershim/testing:all-srcs", ], tags = ["automanaged"], diff --git a/pkg/kubelet/dockershim/security_context.go b/pkg/kubelet/dockershim/security_context.go index 71edd77f216..7c4fd57b85d 100644 --- a/pkg/kubelet/dockershim/security_context.go +++ b/pkg/kubelet/dockershim/security_context.go @@ -24,9 +24,7 @@ import ( "github.com/blang/semver" dockercontainer "github.com/docker/engine-api/types/container" - "k8s.io/api/core/v1" runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "k8s.io/kubernetes/pkg/kubelet/dockershim/securitycontext" knetwork "k8s.io/kubernetes/pkg/kubelet/network" ) @@ -101,14 +99,9 @@ func modifyHostConfig(sc *runtimeapi.LinuxContainerSecurityContext, hostConfig * hostConfig.CapDrop = sc.GetCapabilities().DropCapabilities } if sc.SelinuxOptions != nil { - hostConfig.SecurityOpt = securitycontext.ModifySecurityOptions( + hostConfig.SecurityOpt = addSELinuxOptions( hostConfig.SecurityOpt, - &v1.SELinuxOptions{ - User: sc.SelinuxOptions.User, - Role: sc.SelinuxOptions.Role, - Type: sc.SelinuxOptions.Type, - Level: sc.SelinuxOptions.Level, - }, + sc.SelinuxOptions, separator, ) } diff --git a/pkg/kubelet/dockershim/security_context_test.go b/pkg/kubelet/dockershim/security_context_test.go index 868beed8af5..067e5e79755 100644 --- a/pkg/kubelet/dockershim/security_context_test.go +++ b/pkg/kubelet/dockershim/security_context_test.go @@ -26,7 +26,6 @@ import ( "github.com/stretchr/testify/assert" runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" - "k8s.io/kubernetes/pkg/kubelet/dockershim/securitycontext" ) func TestModifyContainerConfig(t *testing.T) { @@ -83,10 +82,10 @@ func TestModifyHostConfig(t *testing.T) { } setSELinuxHC := &dockercontainer.HostConfig{ SecurityOpt: []string{ - fmt.Sprintf("%s:%s", securitycontext.DockerLabelUser('='), "user"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelRole('='), "role"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelType('='), "type"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelLevel('='), "level"), + fmt.Sprintf("%s:%s", selinuxLabelUser('='), "user"), + fmt.Sprintf("%s:%s", selinuxLabelRole('='), "role"), + fmt.Sprintf("%s:%s", selinuxLabelType('='), "type"), + fmt.Sprintf("%s:%s", selinuxLabelLevel('='), "level"), }, } @@ -184,10 +183,10 @@ func TestModifyHostConfigAndNamespaceOptionsForContainer(t *testing.T) { } setSELinuxHC := &dockercontainer.HostConfig{ SecurityOpt: []string{ - fmt.Sprintf("%s:%s", securitycontext.DockerLabelUser('='), "user"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelRole('='), "role"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelType('='), "type"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelLevel('='), "level"), + fmt.Sprintf("%s:%s", selinuxLabelUser('='), "user"), + fmt.Sprintf("%s:%s", selinuxLabelRole('='), "role"), + fmt.Sprintf("%s:%s", selinuxLabelType('='), "type"), + fmt.Sprintf("%s:%s", selinuxLabelLevel('='), "level"), }, IpcMode: dockercontainer.IpcMode(sandboxNSMode), NetworkMode: dockercontainer.NetworkMode(sandboxNSMode), @@ -415,10 +414,10 @@ func fullValidHostConfig() *dockercontainer.HostConfig { CapAdd: []string{"addCapA", "addCapB"}, CapDrop: []string{"dropCapA", "dropCapB"}, SecurityOpt: []string{ - fmt.Sprintf("%s:%s", securitycontext.DockerLabelUser('='), "user"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelRole('='), "role"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelType('='), "type"), - fmt.Sprintf("%s:%s", securitycontext.DockerLabelLevel('='), "level"), + fmt.Sprintf("%s:%s", selinuxLabelUser('='), "user"), + fmt.Sprintf("%s:%s", selinuxLabelRole('='), "role"), + fmt.Sprintf("%s:%s", selinuxLabelType('='), "type"), + fmt.Sprintf("%s:%s", selinuxLabelLevel('='), "level"), }, } } diff --git a/pkg/kubelet/dockershim/securitycontext/BUILD b/pkg/kubelet/dockershim/securitycontext/BUILD deleted file mode 100644 index a4da55bbf98..00000000000 --- a/pkg/kubelet/dockershim/securitycontext/BUILD +++ /dev/null @@ -1,53 +0,0 @@ -package(default_visibility = ["//visibility:public"]) - -licenses(["notice"]) - -load( - "@io_bazel_rules_go//go:def.bzl", - "go_library", - "go_test", -) - -go_library( - name = "go_default_library", - srcs = [ - "doc.go", - "fake.go", - "provider.go", - "types.go", - "util.go", - ], - tags = ["automanaged"], - deps = [ - "//pkg/kubelet/container:go_default_library", - "//pkg/kubelet/leaky:go_default_library", - "//pkg/securitycontext:go_default_library", - "//vendor/github.com/docker/engine-api/types/container:go_default_library", - "//vendor/k8s.io/api/core/v1:go_default_library", - ], -) - -go_test( - name = "go_default_test", - srcs = ["provider_test.go"], - library = ":go_default_library", - tags = ["automanaged"], - deps = [ - "//pkg/api/testing:go_default_library", - "//vendor/github.com/docker/engine-api/types/container:go_default_library", - "//vendor/k8s.io/api/core/v1:go_default_library", - ], -) - -filegroup( - name = "package-srcs", - srcs = glob(["**"]), - tags = ["automanaged"], - visibility = ["//visibility:private"], -) - -filegroup( - name = "all-srcs", - srcs = [":package-srcs"], - tags = ["automanaged"], -) diff --git a/pkg/kubelet/dockershim/securitycontext/doc.go b/pkg/kubelet/dockershim/securitycontext/doc.go deleted file mode 100644 index dd9a0a2291d..00000000000 --- a/pkg/kubelet/dockershim/securitycontext/doc.go +++ /dev/null @@ -1,18 +0,0 @@ -/* -Copyright 2014 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 securitycontext contains security context api implementations -package securitycontext // import "k8s.io/kubernetes/pkg/kubelet/dockershim/securitycontext" diff --git a/pkg/kubelet/dockershim/securitycontext/fake.go b/pkg/kubelet/dockershim/securitycontext/fake.go deleted file mode 100644 index 3217dcff1fe..00000000000 --- a/pkg/kubelet/dockershim/securitycontext/fake.go +++ /dev/null @@ -1,35 +0,0 @@ -/* -Copyright 2014 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 securitycontext - -import ( - "k8s.io/api/core/v1" - - dockercontainer "github.com/docker/engine-api/types/container" -) - -// NewFakeSecurityContextProvider creates a new, no-op security context provider. -func NewFakeSecurityContextProvider() SecurityContextProvider { - return FakeSecurityContextProvider{} -} - -type FakeSecurityContextProvider struct{} - -func (p FakeSecurityContextProvider) ModifyContainerConfig(pod *v1.Pod, container *v1.Container, config *dockercontainer.Config) { -} -func (p FakeSecurityContextProvider) ModifyHostConfig(pod *v1.Pod, container *v1.Container, hostConfig *dockercontainer.HostConfig, supplementalGids []int64) { -} diff --git a/pkg/kubelet/dockershim/securitycontext/provider.go b/pkg/kubelet/dockershim/securitycontext/provider.go deleted file mode 100644 index 0899b0c8a65..00000000000 --- a/pkg/kubelet/dockershim/securitycontext/provider.go +++ /dev/null @@ -1,131 +0,0 @@ -/* -Copyright 2014 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 securitycontext - -import ( - "fmt" - "strconv" - - "k8s.io/api/core/v1" - kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" - "k8s.io/kubernetes/pkg/kubelet/leaky" - "k8s.io/kubernetes/pkg/securitycontext" - - dockercontainer "github.com/docker/engine-api/types/container" -) - -// NewSimpleSecurityContextProvider creates a new SimpleSecurityContextProvider. -func NewSimpleSecurityContextProvider(securityOptSeparator rune) SecurityContextProvider { - return SimpleSecurityContextProvider{securityOptSeparator} -} - -// SimpleSecurityContextProvider is the default implementation of a SecurityContextProvider. -type SimpleSecurityContextProvider struct { - securityOptSeparator rune -} - -// ModifyContainerConfig is called before the Docker createContainer call. -// The security context provider can make changes to the Config with which -// the container is created. -func (p SimpleSecurityContextProvider) ModifyContainerConfig(pod *v1.Pod, container *v1.Container, config *dockercontainer.Config) { - effectiveSC := securitycontext.DetermineEffectiveSecurityContext(pod, container) - if effectiveSC == nil { - return - } - if effectiveSC.RunAsUser != nil { - config.User = strconv.Itoa(int(*effectiveSC.RunAsUser)) - } -} - -// ModifyHostConfig is called before the Docker runContainer call. The -// security context provider can make changes to the HostConfig, affecting -// security options, whether the container is privileged, volume binds, etc. -func (p SimpleSecurityContextProvider) ModifyHostConfig(pod *v1.Pod, container *v1.Container, hostConfig *dockercontainer.HostConfig, supplementalGids []int64) { - // Apply supplemental groups - if container.Name != leaky.PodInfraContainerName { - // TODO: We skip application of supplemental groups to the - // infra container to work around a runc issue which - // requires containers to have the '/etc/group'. For - // more information see: - // https://github.com/opencontainers/runc/pull/313 - // This can be removed once the fix makes it into the - // required version of docker. - if pod.Spec.SecurityContext != nil { - for _, group := range pod.Spec.SecurityContext.SupplementalGroups { - hostConfig.GroupAdd = append(hostConfig.GroupAdd, strconv.Itoa(int(group))) - } - if pod.Spec.SecurityContext.FSGroup != nil { - hostConfig.GroupAdd = append(hostConfig.GroupAdd, strconv.Itoa(int(*pod.Spec.SecurityContext.FSGroup))) - } - } - - for _, group := range supplementalGids { - hostConfig.GroupAdd = append(hostConfig.GroupAdd, strconv.Itoa(int(group))) - } - } - - // Apply effective security context for container - effectiveSC := securitycontext.DetermineEffectiveSecurityContext(pod, container) - if effectiveSC == nil { - return - } - - if effectiveSC.Privileged != nil { - hostConfig.Privileged = *effectiveSC.Privileged - } - - if effectiveSC.Capabilities != nil { - add, drop := kubecontainer.MakeCapabilities(effectiveSC.Capabilities.Add, effectiveSC.Capabilities.Drop) - hostConfig.CapAdd = add - hostConfig.CapDrop = drop - } - - if effectiveSC.SELinuxOptions != nil { - hostConfig.SecurityOpt = ModifySecurityOptions(hostConfig.SecurityOpt, effectiveSC.SELinuxOptions, p.securityOptSeparator) - } -} - -// ModifySecurityOptions adds SELinux options to config using the given -// separator. -func ModifySecurityOptions(config []string, selinuxOpts *v1.SELinuxOptions, separator rune) []string { - // Note, strictly speaking, we are actually mutating the values of these - // keys, rather than formatting name and value into a string. Docker re- - // uses the same option name multiple times (it's just 'label') with - // different values which are themselves key-value pairs. For example, - // the SELinux type is represented by the security opt: - // - // labeltype: - // - // In Docker API versions before 1.23, the separator was the `:` rune; in - // API version 1.23 it changed to the `=` rune. - config = modifySecurityOption(config, DockerLabelUser(separator), selinuxOpts.User) - config = modifySecurityOption(config, DockerLabelRole(separator), selinuxOpts.Role) - config = modifySecurityOption(config, DockerLabelType(separator), selinuxOpts.Type) - config = modifySecurityOption(config, DockerLabelLevel(separator), selinuxOpts.Level) - - return config -} - -// modifySecurityOption adds the security option of name to the config array -// with value in the form of name:value. -func modifySecurityOption(config []string, name, value string) []string { - if len(value) > 0 { - config = append(config, fmt.Sprintf("%s:%s", name, value)) - } - - return config -} diff --git a/pkg/kubelet/dockershim/securitycontext/provider_test.go b/pkg/kubelet/dockershim/securitycontext/provider_test.go deleted file mode 100644 index 1113ac25d87..00000000000 --- a/pkg/kubelet/dockershim/securitycontext/provider_test.go +++ /dev/null @@ -1,334 +0,0 @@ -/* -Copyright 2014 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 securitycontext - -import ( - "fmt" - "reflect" - "strconv" - "testing" - - dockercontainer "github.com/docker/engine-api/types/container" - "k8s.io/api/core/v1" - apitesting "k8s.io/kubernetes/pkg/api/testing" -) - -func TestModifyContainerConfig(t *testing.T) { - userID := int64(123) - overrideUserID := int64(321) - - cases := []struct { - name string - podSc *v1.PodSecurityContext - sc *v1.SecurityContext - expected *dockercontainer.Config - }{ - { - name: "container.SecurityContext.RunAsUser set", - sc: &v1.SecurityContext{ - RunAsUser: &userID, - }, - expected: &dockercontainer.Config{ - User: strconv.FormatInt(int64(userID), 10), - }, - }, - { - name: "no RunAsUser value set", - sc: &v1.SecurityContext{}, - expected: &dockercontainer.Config{}, - }, - { - name: "pod.Spec.SecurityContext.RunAsUser set", - podSc: &v1.PodSecurityContext{ - RunAsUser: &userID, - }, - expected: &dockercontainer.Config{ - User: strconv.FormatInt(int64(userID), 10), - }, - }, - { - name: "container.SecurityContext.RunAsUser overrides pod.Spec.SecurityContext.RunAsUser", - podSc: &v1.PodSecurityContext{ - RunAsUser: &userID, - }, - sc: &v1.SecurityContext{ - RunAsUser: &overrideUserID, - }, - expected: &dockercontainer.Config{ - User: strconv.FormatInt(int64(overrideUserID), 10), - }, - }, - } - - provider := NewSimpleSecurityContextProvider('=') - dummyContainer := &v1.Container{} - for _, tc := range cases { - pod := &v1.Pod{Spec: v1.PodSpec{SecurityContext: tc.podSc}} - dummyContainer.SecurityContext = tc.sc - dockerCfg := &dockercontainer.Config{} - - provider.ModifyContainerConfig(pod, dummyContainer, dockerCfg) - - if e, a := tc.expected, dockerCfg; !reflect.DeepEqual(e, a) { - t.Errorf("%v: unexpected modification of docker config\nExpected:\n\n%#v\n\nGot:\n\n%#v", tc.name, e, a) - } - } -} - -func TestModifyHostConfig(t *testing.T) { - priv := true - setPrivSC := &v1.SecurityContext{} - setPrivSC.Privileged = &priv - setPrivHC := &dockercontainer.HostConfig{ - Privileged: true, - } - - setCapsHC := &dockercontainer.HostConfig{ - CapAdd: []string{"addCapA", "addCapB"}, - CapDrop: []string{"dropCapA", "dropCapB"}, - } - - setSELinuxHC := &dockercontainer.HostConfig{} - setSELinuxHC.SecurityOpt = []string{ - fmt.Sprintf("%s:%s", DockerLabelUser(':'), "user"), - fmt.Sprintf("%s:%s", DockerLabelRole(':'), "role"), - fmt.Sprintf("%s:%s", DockerLabelType(':'), "type"), - fmt.Sprintf("%s:%s", DockerLabelLevel(':'), "level"), - } - - // seLinuxLabelsSC := fullValidSecurityContext() - // seLinuxLabelsHC := fullValidHostConfig() - - cases := []struct { - name string - podSc *v1.PodSecurityContext - sc *v1.SecurityContext - expected *dockercontainer.HostConfig - }{ - { - name: "fully set container.SecurityContext", - sc: fullValidSecurityContext(), - expected: fullValidHostConfig(), - }, - { - name: "container.SecurityContext.Privileged", - sc: setPrivSC, - expected: setPrivHC, - }, - { - name: "container.SecurityContext.Capabilities", - sc: &v1.SecurityContext{ - Capabilities: inputCapabilities(), - }, - expected: setCapsHC, - }, - { - name: "container.SecurityContext.SELinuxOptions", - sc: &v1.SecurityContext{ - SELinuxOptions: inputSELinuxOptions(), - }, - expected: setSELinuxHC, - }, - { - name: "pod.Spec.SecurityContext.SELinuxOptions", - podSc: &v1.PodSecurityContext{ - SELinuxOptions: inputSELinuxOptions(), - }, - expected: setSELinuxHC, - }, - { - name: "container.SecurityContext overrides pod.Spec.SecurityContext", - podSc: overridePodSecurityContext(), - sc: fullValidSecurityContext(), - expected: fullValidHostConfig(), - }, - } - - provider := NewSimpleSecurityContextProvider(':') - dummyContainer := &v1.Container{} - - for _, tc := range cases { - pod := &v1.Pod{Spec: v1.PodSpec{SecurityContext: tc.podSc}} - dummyContainer.SecurityContext = tc.sc - dockerCfg := &dockercontainer.HostConfig{} - - provider.ModifyHostConfig(pod, dummyContainer, dockerCfg, nil) - - if e, a := tc.expected, dockerCfg; !reflect.DeepEqual(e, a) { - t.Errorf("%v: unexpected modification of host config\nExpected:\n\n%#v\n\nGot:\n\n%#v", tc.name, e, a) - } - } -} - -func TestModifyHostConfigPodSecurityContext(t *testing.T) { - supplementalGroupsSC := &v1.PodSecurityContext{} - supplementalGroupsSC.SupplementalGroups = []int64{2222} - supplementalGroupHC := fullValidHostConfig() - supplementalGroupHC.GroupAdd = []string{"2222"} - fsGroupHC := fullValidHostConfig() - fsGroupHC.GroupAdd = []string{"1234"} - extraSupplementalGroupHC := fullValidHostConfig() - extraSupplementalGroupHC.GroupAdd = []string{"1234"} - bothHC := fullValidHostConfig() - bothHC.GroupAdd = []string{"2222", "1234"} - fsGroup := int64(1234) - extraSupplementalGroup := []int64{1234} - - testCases := map[string]struct { - securityContext *v1.PodSecurityContext - expected *dockercontainer.HostConfig - extraSupplementalGroups []int64 - }{ - "nil": { - securityContext: nil, - expected: fullValidHostConfig(), - extraSupplementalGroups: nil, - }, - "SupplementalGroup": { - securityContext: supplementalGroupsSC, - expected: supplementalGroupHC, - extraSupplementalGroups: nil, - }, - "FSGroup": { - securityContext: &v1.PodSecurityContext{FSGroup: &fsGroup}, - expected: fsGroupHC, - extraSupplementalGroups: nil, - }, - "FSGroup + SupplementalGroups": { - securityContext: &v1.PodSecurityContext{ - SupplementalGroups: []int64{2222}, - FSGroup: &fsGroup, - }, - expected: bothHC, - extraSupplementalGroups: nil, - }, - "ExtraSupplementalGroup": { - securityContext: nil, - expected: extraSupplementalGroupHC, - extraSupplementalGroups: extraSupplementalGroup, - }, - "ExtraSupplementalGroup + SupplementalGroups": { - securityContext: supplementalGroupsSC, - expected: bothHC, - extraSupplementalGroups: extraSupplementalGroup, - }, - } - - provider := NewSimpleSecurityContextProvider(':') - dummyContainer := &v1.Container{} - dummyContainer.SecurityContext = fullValidSecurityContext() - dummyPod := &v1.Pod{ - Spec: apitesting.V1DeepEqualSafePodSpec(), - } - - for k, v := range testCases { - dummyPod.Spec.SecurityContext = v.securityContext - dockerCfg := &dockercontainer.HostConfig{} - provider.ModifyHostConfig(dummyPod, dummyContainer, dockerCfg, v.extraSupplementalGroups) - if !reflect.DeepEqual(v.expected, dockerCfg) { - t.Errorf("unexpected modification of host config for %s. Expected: %#v Got: %#v", k, v.expected, dockerCfg) - } - } -} - -func TestModifySecurityOption(t *testing.T) { - testCases := []struct { - name string - config []string - optName string - optVal string - expected []string - }{ - { - name: "Empty val", - config: []string{"a:b", "c:d"}, - optName: "optA", - optVal: "", - expected: []string{"a:b", "c:d"}, - }, - { - name: "Valid", - config: []string{"a:b", "c:d"}, - optName: "e", - optVal: "f", - expected: []string{"a:b", "c:d", "e:f"}, - }, - } - - for _, tc := range testCases { - actual := modifySecurityOption(tc.config, tc.optName, tc.optVal) - if !reflect.DeepEqual(tc.expected, actual) { - t.Errorf("Failed to apply options correctly for tc: %s. Expected: %v but got %v", tc.name, tc.expected, actual) - } - } -} - -func overridePodSecurityContext() *v1.PodSecurityContext { - return &v1.PodSecurityContext{ - SELinuxOptions: &v1.SELinuxOptions{ - User: "user2", - Role: "role2", - Type: "type2", - Level: "level2", - }, - } -} - -func fullValidPodSecurityContext() *v1.PodSecurityContext { - return &v1.PodSecurityContext{ - SELinuxOptions: inputSELinuxOptions(), - } -} - -func fullValidSecurityContext() *v1.SecurityContext { - priv := true - return &v1.SecurityContext{ - Privileged: &priv, - Capabilities: inputCapabilities(), - SELinuxOptions: inputSELinuxOptions(), - } -} - -func inputCapabilities() *v1.Capabilities { - return &v1.Capabilities{ - Add: []v1.Capability{"addCapA", "addCapB"}, - Drop: []v1.Capability{"dropCapA", "dropCapB"}, - } -} - -func inputSELinuxOptions() *v1.SELinuxOptions { - return &v1.SELinuxOptions{ - User: "user", - Role: "role", - Type: "type", - Level: "level", - } -} - -func fullValidHostConfig() *dockercontainer.HostConfig { - return &dockercontainer.HostConfig{ - Privileged: true, - CapAdd: []string{"addCapA", "addCapB"}, - CapDrop: []string{"dropCapA", "dropCapB"}, - SecurityOpt: []string{ - fmt.Sprintf("%s:%s", DockerLabelUser(':'), "user"), - fmt.Sprintf("%s:%s", DockerLabelRole(':'), "role"), - fmt.Sprintf("%s:%s", DockerLabelType(':'), "type"), - fmt.Sprintf("%s:%s", DockerLabelLevel(':'), "level"), - }, - } -} diff --git a/pkg/kubelet/dockershim/securitycontext/types.go b/pkg/kubelet/dockershim/securitycontext/types.go deleted file mode 100644 index bf4768a7752..00000000000 --- a/pkg/kubelet/dockershim/securitycontext/types.go +++ /dev/null @@ -1,41 +0,0 @@ -/* -Copyright 2014 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 securitycontext - -import ( - "k8s.io/api/core/v1" - - dockercontainer "github.com/docker/engine-api/types/container" -) - -type SecurityContextProvider interface { - // ModifyContainerConfig is called before the Docker createContainer call. - // The security context provider can make changes to the Config with which - // the container is created. - ModifyContainerConfig(pod *v1.Pod, container *v1.Container, config *dockercontainer.Config) - - // ModifyHostConfig is called before the Docker createContainer call. - // The security context provider can make changes to the HostConfig, affecting - // security options, whether the container is privileged, volume binds, etc. - // An error is returned if it's not possible to secure the container as requested - // with a security context. - // - // - pod: the pod to modify the docker hostconfig for - // - container: the container to modify the hostconfig for - // - supplementalGids: additional supplemental GIDs associated with the pod's volumes - ModifyHostConfig(pod *v1.Pod, container *v1.Container, hostConfig *dockercontainer.HostConfig, supplementalGids []int64) -} diff --git a/pkg/kubelet/dockershim/securitycontext/util.go b/pkg/kubelet/dockershim/securitycontext/util.go deleted file mode 100644 index faf6ed53925..00000000000 --- a/pkg/kubelet/dockershim/securitycontext/util.go +++ /dev/null @@ -1,59 +0,0 @@ -/* -Copyright 2017 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 securitycontext - -import ( - "fmt" -) - -// DockerLabelUser returns the fragment of a Docker security opt that -// describes the SELinux user. Note that strictly speaking this is not -// actually the name of the security opt, but a fragment of the whole key- -// value pair necessary to set the opt. -func DockerLabelUser(separator rune) string { - return fmt.Sprintf("label%cuser", separator) -} - -// DockerLabelRole returns the fragment of a Docker security opt that -// describes the SELinux role. Note that strictly speaking this is not -// actually the name of the security opt, but a fragment of the whole key- -// value pair necessary to set the opt. -func DockerLabelRole(separator rune) string { - return fmt.Sprintf("label%crole", separator) -} - -// DockerLabelType returns the fragment of a Docker security opt that -// describes the SELinux type. Note that strictly speaking this is not -// actually the name of the security opt, but a fragment of the whole key- -// value pair necessary to set the opt. -func DockerLabelType(separator rune) string { - return fmt.Sprintf("label%ctype", separator) -} - -// DockerLabelLevel returns the fragment of a Docker security opt that -// describes the SELinux level. Note that strictly speaking this is not -// actually the name of the security opt, but a fragment of the whole key- -// value pair necessary to set the opt. -func DockerLabelLevel(separator rune) string { - return fmt.Sprintf("label%clevel", separator) -} - -// DockerLaelDisable returns the Docker security opt that disables SELinux for -// the container. -func DockerLabelDisable(separator rune) string { - return fmt.Sprintf("label%cdisable", separator) -} diff --git a/pkg/kubelet/dockershim/selinux_util.go b/pkg/kubelet/dockershim/selinux_util.go new file mode 100644 index 00000000000..e8e2a07f623 --- /dev/null +++ b/pkg/kubelet/dockershim/selinux_util.go @@ -0,0 +1,92 @@ +/* +Copyright 2017 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 dockershim + +import ( + "fmt" + + runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" +) + +// selinuxLabelUser returns the fragment of a Docker security opt that +// describes the SELinux user. Note that strictly speaking this is not +// actually the name of the security opt, but a fragment of the whole key- +// value pair necessary to set the opt. +func selinuxLabelUser(separator rune) string { + return fmt.Sprintf("label%cuser", separator) +} + +// selinuxLabelRole returns the fragment of a Docker security opt that +// describes the SELinux role. Note that strictly speaking this is not +// actually the name of the security opt, but a fragment of the whole key- +// value pair necessary to set the opt. +func selinuxLabelRole(separator rune) string { + return fmt.Sprintf("label%crole", separator) +} + +// selinuxLabelType returns the fragment of a Docker security opt that +// describes the SELinux type. Note that strictly speaking this is not +// actually the name of the security opt, but a fragment of the whole key- +// value pair necessary to set the opt. +func selinuxLabelType(separator rune) string { + return fmt.Sprintf("label%ctype", separator) +} + +// selinuxLabelLevel returns the fragment of a Docker security opt that +// describes the SELinux level. Note that strictly speaking this is not +// actually the name of the security opt, but a fragment of the whole key- +// value pair necessary to set the opt. +func selinuxLabelLevel(separator rune) string { + return fmt.Sprintf("label%clevel", separator) +} + +// dockerLaelDisable returns the Docker security opt that disables SELinux for +// the container. +func selinuxLabelDisable(separator rune) string { + return fmt.Sprintf("label%cdisable", separator) +} + +// addSELinuxOptions adds SELinux options to config using the given +// separator. +func addSELinuxOptions(config []string, selinuxOpts *runtimeapi.SELinuxOption, separator rune) []string { + // Note, strictly speaking, we are actually mutating the values of these + // keys, rather than formatting name and value into a string. Docker re- + // uses the same option name multiple times (it's just 'label') with + // different values which are themselves key-value pairs. For example, + // the SELinux type is represented by the security opt: + // + // labeltype: + // + // In Docker API versions before 1.23, the separator was the `:` rune; in + // API version 1.23 it changed to the `=` rune. + config = modifySecurityOption(config, selinuxLabelUser(separator), selinuxOpts.User) + config = modifySecurityOption(config, selinuxLabelRole(separator), selinuxOpts.Role) + config = modifySecurityOption(config, selinuxLabelType(separator), selinuxOpts.Type) + config = modifySecurityOption(config, selinuxLabelLevel(separator), selinuxOpts.Level) + + return config +} + +// modifySecurityOption adds the security option of name to the config array +// with value in the form of name:value. +func modifySecurityOption(config []string, name, value string) []string { + if len(value) > 0 { + config = append(config, fmt.Sprintf("%s:%s", name, value)) + } + + return config +} diff --git a/pkg/kubelet/dockershim/selinux_util_test.go b/pkg/kubelet/dockershim/selinux_util_test.go new file mode 100644 index 00000000000..93ef091723e --- /dev/null +++ b/pkg/kubelet/dockershim/selinux_util_test.go @@ -0,0 +1,54 @@ +/* +Copyright 2017 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 dockershim + +import ( + "reflect" + "testing" +) + +func TestModifySecurityOptions(t *testing.T) { + testCases := []struct { + name string + config []string + optName string + optVal string + expected []string + }{ + { + name: "Empty val", + config: []string{"a:b", "c:d"}, + optName: "optA", + optVal: "", + expected: []string{"a:b", "c:d"}, + }, + { + name: "Valid", + config: []string{"a:b", "c:d"}, + optName: "e", + optVal: "f", + expected: []string{"a:b", "c:d", "e:f"}, + }, + } + + for _, tc := range testCases { + actual := modifySecurityOption(tc.config, tc.optName, tc.optVal) + if !reflect.DeepEqual(tc.expected, actual) { + t.Errorf("Failed to apply options correctly for tc: %s. Expected: %v but got %v", tc.name, tc.expected, actual) + } + } +}