From 3579f88e4d681339a46b2ee843bb447c28f8a2e6 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 1 Feb 2021 12:18:05 -0500 Subject: [PATCH] Remove deprecated DenyEscalatingExec / DenyExecOnPrivileged admission --- pkg/kubeapiserver/options/BUILD | 1 - pkg/kubeapiserver/options/plugins.go | 4 - plugin/BUILD | 1 - plugin/pkg/admission/exec/BUILD | 50 ----- plugin/pkg/admission/exec/admission.go | 163 ---------------- plugin/pkg/admission/exec/admission_test.go | 206 -------------------- 6 files changed, 425 deletions(-) delete mode 100644 plugin/pkg/admission/exec/BUILD delete mode 100644 plugin/pkg/admission/exec/admission.go delete mode 100644 plugin/pkg/admission/exec/admission_test.go diff --git a/pkg/kubeapiserver/options/BUILD b/pkg/kubeapiserver/options/BUILD index 5c9e9ba6fd8..b773dd1fb89 100644 --- a/pkg/kubeapiserver/options/BUILD +++ b/pkg/kubeapiserver/options/BUILD @@ -29,7 +29,6 @@ go_library( "//plugin/pkg/admission/defaulttolerationseconds:go_default_library", "//plugin/pkg/admission/deny:go_default_library", "//plugin/pkg/admission/eventratelimit:go_default_library", - "//plugin/pkg/admission/exec:go_default_library", "//plugin/pkg/admission/extendedresourcetoleration:go_default_library", "//plugin/pkg/admission/gc:go_default_library", "//plugin/pkg/admission/imagepolicy:go_default_library", diff --git a/pkg/kubeapiserver/options/plugins.go b/pkg/kubeapiserver/options/plugins.go index 351a8ef8d00..94d93f47780 100644 --- a/pkg/kubeapiserver/options/plugins.go +++ b/pkg/kubeapiserver/options/plugins.go @@ -31,7 +31,6 @@ import ( "k8s.io/kubernetes/plugin/pkg/admission/defaulttolerationseconds" "k8s.io/kubernetes/plugin/pkg/admission/deny" "k8s.io/kubernetes/plugin/pkg/admission/eventratelimit" - "k8s.io/kubernetes/plugin/pkg/admission/exec" "k8s.io/kubernetes/plugin/pkg/admission/extendedresourcetoleration" "k8s.io/kubernetes/plugin/pkg/admission/gc" "k8s.io/kubernetes/plugin/pkg/admission/imagepolicy" @@ -79,8 +78,6 @@ var AllOrderedPlugins = []string{ podpriority.PluginName, // Priority defaulttolerationseconds.PluginName, // DefaultTolerationSeconds podtolerationrestriction.PluginName, // PodTolerationRestriction - exec.DenyEscalatingExec, // DenyEscalatingExec - exec.DenyExecOnPrivileged, // DenyExecOnPrivileged eventratelimit.PluginName, // EventRateLimit extendedresourcetoleration.PluginName, // ExtendedResourceToleration label.PluginName, // PersistentVolumeLabel @@ -113,7 +110,6 @@ func RegisterAllAdmissionPlugins(plugins *admission.Plugins) { defaultingressclass.Register(plugins) deny.Register(plugins) // DEPRECATED as no real meaning eventratelimit.Register(plugins) - exec.Register(plugins) extendedresourcetoleration.Register(plugins) gc.Register(plugins) imagepolicy.Register(plugins) diff --git a/plugin/BUILD b/plugin/BUILD index 26f5245a107..78d4d8d0ddd 100644 --- a/plugin/BUILD +++ b/plugin/BUILD @@ -19,7 +19,6 @@ filegroup( "//plugin/pkg/admission/defaulttolerationseconds:all-srcs", "//plugin/pkg/admission/deny:all-srcs", "//plugin/pkg/admission/eventratelimit:all-srcs", - "//plugin/pkg/admission/exec:all-srcs", "//plugin/pkg/admission/extendedresourcetoleration:all-srcs", "//plugin/pkg/admission/gc:all-srcs", "//plugin/pkg/admission/imagepolicy:all-srcs", diff --git a/plugin/pkg/admission/exec/BUILD b/plugin/pkg/admission/exec/BUILD deleted file mode 100644 index c7500ef3afa..00000000000 --- a/plugin/pkg/admission/exec/BUILD +++ /dev/null @@ -1,50 +0,0 @@ -package(default_visibility = ["//visibility:public"]) - -load( - "@io_bazel_rules_go//go:def.bzl", - "go_library", - "go_test", -) - -go_library( - name = "go_default_library", - srcs = ["admission.go"], - importpath = "k8s.io/kubernetes/plugin/pkg/admission/exec", - deps = [ - "//pkg/api/v1/pod:go_default_library", - "//staging/src/k8s.io/api/core/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library", - "//staging/src/k8s.io/client-go/kubernetes:go_default_library", - "//vendor/k8s.io/klog/v2:go_default_library", - ], -) - -go_test( - name = "go_default_test", - srcs = ["admission_test.go"], - embed = [":go_default_library"], - deps = [ - "//pkg/apis/core:go_default_library", - "//staging/src/k8s.io/api/core/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", - "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", - "//staging/src/k8s.io/client-go/testing: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/plugin/pkg/admission/exec/admission.go b/plugin/pkg/admission/exec/admission.go deleted file mode 100644 index 059651c02b9..00000000000 --- a/plugin/pkg/admission/exec/admission.go +++ /dev/null @@ -1,163 +0,0 @@ -/* -Copyright 2015 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 exec - -import ( - "context" - "fmt" - "io" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apiserver/pkg/admission" - genericadmissioninitializer "k8s.io/apiserver/pkg/admission/initializer" - "k8s.io/client-go/kubernetes" - "k8s.io/klog/v2" - podutil "k8s.io/kubernetes/pkg/api/v1/pod" -) - -const ( - // DenyEscalatingExec indicates name of admission plugin. - // Deprecated, will be removed in v1.18. - // Use of PodSecurityPolicy or a custom admission plugin to limit creation of pods is recommended instead. - DenyEscalatingExec = "DenyEscalatingExec" - // DenyExecOnPrivileged indicates name of admission plugin. - // Deprecated, will be removed in v1.18. - // Use of PodSecurityPolicy or a custom admission plugin to limit creation of pods is recommended instead. - DenyExecOnPrivileged = "DenyExecOnPrivileged" -) - -// Register registers a plugin -func Register(plugins *admission.Plugins) { - plugins.Register(DenyEscalatingExec, func(config io.Reader) (admission.Interface, error) { - klog.Warningf("the %s admission plugin is deprecated and will be removed in v1.18", DenyEscalatingExec) - klog.Warningf("use of PodSecurityPolicy or a custom admission plugin to limit creation of pods is recommended instead") - return NewDenyEscalatingExec(), nil - }) - - // This is for legacy support of the DenyExecOnPrivileged admission controller. Most - // of the time DenyEscalatingExec should be preferred. - plugins.Register(DenyExecOnPrivileged, func(config io.Reader) (admission.Interface, error) { - klog.Warningf("the %s admission plugin is deprecated and will be removed in v1.18", DenyExecOnPrivileged) - klog.Warningf("use of PodSecurityPolicy or a custom admission plugin to limit creation of pods is recommended instead") - return NewDenyExecOnPrivileged(), nil - }) -} - -// DenyExec is an implementation of admission.Interface which says no to a pod/exec on -// a pod using host based configurations. -type DenyExec struct { - *admission.Handler - client kubernetes.Interface - - // these flags control which items will be checked to deny exec/attach - hostNetwork bool - hostIPC bool - hostPID bool - privileged bool -} - -var _ admission.ValidationInterface = &DenyExec{} -var _ = genericadmissioninitializer.WantsExternalKubeClientSet(&DenyExec{}) - -// NewDenyExecOnPrivileged creates a new admission controller that is only checking the privileged -// option. This is for legacy support of the DenyExecOnPrivileged admission controller. -// Most of the time NewDenyEscalatingExec should be preferred. -func NewDenyExecOnPrivileged() *DenyExec { - return &DenyExec{ - Handler: admission.NewHandler(admission.Connect), - hostNetwork: false, - hostIPC: false, - hostPID: false, - privileged: true, - } -} - -// NewDenyEscalatingExec creates a new admission controller that denies an exec operation on a pod -// using host based configurations. -func NewDenyEscalatingExec() *DenyExec { - return &DenyExec{ - Handler: admission.NewHandler(admission.Connect), - hostNetwork: true, - hostIPC: true, - hostPID: true, - privileged: true, - } -} - -// SetExternalKubeClientSet implements the WantsInternalKubeClientSet interface. -func (d *DenyExec) SetExternalKubeClientSet(client kubernetes.Interface) { - d.client = client -} - -// ValidateInitialization implements the InitializationValidator interface. -func (d *DenyExec) ValidateInitialization() error { - if d.client == nil { - return fmt.Errorf("missing client") - } - return nil -} - -// Validate makes an admission decision based on the request attributes -func (d *DenyExec) Validate(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) (err error) { - path := a.GetResource().Resource - if subresource := a.GetSubresource(); subresource != "" { - path = path + "/" + subresource - } - // Only handle exec or attach requests on pods - if path != "pods/exec" && path != "pods/attach" { - return nil - } - pod, err := d.client.CoreV1().Pods(a.GetNamespace()).Get(context.TODO(), a.GetName(), metav1.GetOptions{}) - if err != nil { - return admission.NewForbidden(a, err) - } - - if d.hostNetwork && pod.Spec.HostNetwork { - return admission.NewForbidden(a, fmt.Errorf("cannot exec into or attach to a container using host network")) - } - - if d.hostPID && pod.Spec.HostPID { - return admission.NewForbidden(a, fmt.Errorf("cannot exec into or attach to a container using host pid")) - } - - if d.hostIPC && pod.Spec.HostIPC { - return admission.NewForbidden(a, fmt.Errorf("cannot exec into or attach to a container using host ipc")) - } - - if d.privileged && isPrivileged(pod) { - return admission.NewForbidden(a, fmt.Errorf("cannot exec into or attach to a privileged container")) - } - - return nil -} - -// isPrivileged will return true a pod has any privileged containers -func isPrivileged(pod *corev1.Pod) bool { - var privileged bool - podutil.VisitContainers(&pod.Spec, podutil.AllContainers, func(c *corev1.Container, containerType podutil.ContainerType) bool { - if c.SecurityContext == nil || c.SecurityContext.Privileged == nil { - return true - } - if *c.SecurityContext.Privileged { - privileged = true - return false - } - return true - }) - return privileged -} diff --git a/plugin/pkg/admission/exec/admission_test.go b/plugin/pkg/admission/exec/admission_test.go deleted file mode 100644 index 3eafcec4187..00000000000 --- a/plugin/pkg/admission/exec/admission_test.go +++ /dev/null @@ -1,206 +0,0 @@ -/* -Copyright 2015 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 exec - -import ( - "context" - "testing" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apiserver/pkg/admission" - "k8s.io/client-go/kubernetes/fake" - core "k8s.io/client-go/testing" - api "k8s.io/kubernetes/pkg/apis/core" -) - -// newAllowEscalatingExec returns `admission.Interface` that allows execution on -// "hostIPC", "hostPID" and "privileged". -func newAllowEscalatingExec() *DenyExec { - return &DenyExec{ - Handler: admission.NewHandler(admission.Connect), - hostIPC: false, - hostPID: false, - privileged: false, - } -} - -func TestAdmission(t *testing.T) { - privPod := validPod("privileged") - priv := true - privPod.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{ - Privileged: &priv, - } - - hostPIDPod := validPod("hostPID") - hostPIDPod.Spec.HostPID = true - - hostIPCPod := validPod("hostIPC") - hostIPCPod.Spec.HostIPC = true - - testCases := map[string]struct { - pod *corev1.Pod - shouldAccept bool - }{ - "priv": { - shouldAccept: false, - pod: privPod, - }, - "hostPID": { - shouldAccept: false, - pod: hostPIDPod, - }, - "hostIPC": { - shouldAccept: false, - pod: hostIPCPod, - }, - "non privileged": { - shouldAccept: true, - pod: validPod("nonPrivileged"), - }, - } - - // Get the direct object though to allow testAdmission to inject the client - handler := NewDenyEscalatingExec() - - for _, tc := range testCases { - testAdmission(t, tc.pod, handler, tc.shouldAccept) - } - - // run with a permissive config and all cases should pass - handler = newAllowEscalatingExec() - - for _, tc := range testCases { - testAdmission(t, tc.pod, handler, true) - } - - // run against an init container - handler = NewDenyEscalatingExec() - - for _, tc := range testCases { - tc.pod.Spec.InitContainers = tc.pod.Spec.Containers - tc.pod.Spec.Containers = nil - testAdmission(t, tc.pod, handler, tc.shouldAccept) - } - - // run with a permissive config and all cases should pass - handler = newAllowEscalatingExec() - - for _, tc := range testCases { - testAdmission(t, tc.pod, handler, true) - } -} - -func testAdmission(t *testing.T, pod *corev1.Pod, handler *DenyExec, shouldAccept bool) { - mockClient := &fake.Clientset{} - mockClient.AddReactor("get", "pods", func(action core.Action) (bool, runtime.Object, error) { - if action.(core.GetAction).GetName() == pod.Name { - return true, pod, nil - } - t.Errorf("Unexpected API call: %#v", action) - return true, nil, nil - }) - - handler.SetExternalKubeClientSet(mockClient) - admission.ValidateInitialization(handler) - - // pods/exec - { - err := handler.Validate(context.TODO(), admission.NewAttributesRecord(nil, nil, api.Kind("Pod").WithVersion("version"), "test", pod.Name, api.Resource("pods").WithVersion("version"), "exec", admission.Connect, nil, false, nil), nil) - if shouldAccept && err != nil { - t.Errorf("Unexpected error returned from admission handler: %v", err) - } - if !shouldAccept && err == nil { - t.Errorf("An error was expected from the admission handler. Received nil") - } - } - - // pods/attach - { - err := handler.Validate(context.TODO(), admission.NewAttributesRecord(nil, nil, api.Kind("Pod").WithVersion("version"), "test", pod.Name, api.Resource("pods").WithVersion("version"), "attach", admission.Connect, nil, false, nil), nil) - if shouldAccept && err != nil { - t.Errorf("Unexpected error returned from admission handler: %v", err) - } - if !shouldAccept && err == nil { - t.Errorf("An error was expected from the admission handler. Received nil") - } - } -} - -// Test to ensure legacy admission controller works as expected. -func TestDenyExecOnPrivileged(t *testing.T) { - privPod := validPod("privileged") - priv := true - privPod.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{ - Privileged: &priv, - } - - hostPIDPod := validPod("hostPID") - hostPIDPod.Spec.HostPID = true - - hostIPCPod := validPod("hostIPC") - hostIPCPod.Spec.HostIPC = true - - testCases := map[string]struct { - pod *corev1.Pod - shouldAccept bool - }{ - "priv": { - shouldAccept: false, - pod: privPod, - }, - "hostPID": { - shouldAccept: true, - pod: hostPIDPod, - }, - "hostIPC": { - shouldAccept: true, - pod: hostIPCPod, - }, - "non privileged": { - shouldAccept: true, - pod: validPod("nonPrivileged"), - }, - } - - // Get the direct object though to allow testAdmission to inject the client - handler := NewDenyExecOnPrivileged() - - for _, tc := range testCases { - testAdmission(t, tc.pod, handler, tc.shouldAccept) - } - - // test init containers - for _, tc := range testCases { - tc.pod.Spec.InitContainers = tc.pod.Spec.Containers - tc.pod.Spec.Containers = nil - testAdmission(t, tc.pod, handler, tc.shouldAccept) - } -} - -func validPod(name string) *corev1.Pod { - return &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "test"}, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - {Name: "ctr1", Image: "image"}, - {Name: "ctr2", Image: "image2"}, - }, - }, - } -}