From f10dfc6e304be3928f77e47f21f2960d1795169e Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 7 Jul 2021 17:22:27 -0400 Subject: [PATCH] PodSecurity: restricted capabilities: cleanup Fix formatting of container names, Add unit test for containers missing drop, containers with invalid adds Consolidate integration test fixtures --- .../policy/check_capabilities_restricted.go | 45 +++++++++++--- .../check_capabilities_restricted_test.go | 59 +++++++++++++++++++ .../test/fixtures_capabilities_restricted.go | 11 ++-- 3 files changed, 102 insertions(+), 13 deletions(-) create mode 100644 staging/src/k8s.io/pod-security-admission/policy/check_capabilities_restricted_test.go diff --git a/staging/src/k8s.io/pod-security-admission/policy/check_capabilities_restricted.go b/staging/src/k8s.io/pod-security-admission/policy/check_capabilities_restricted.go index 383e662db7e..4042108477d 100644 --- a/staging/src/k8s.io/pod-security-admission/policy/check_capabilities_restricted.go +++ b/staging/src/k8s.io/pod-security-admission/policy/check_capabilities_restricted.go @@ -32,6 +32,25 @@ const ( capabilityNetBindService = "NET_BIND_SERVICE" ) +/* +Containers must drop ALL, and may only add NET_BIND_SERVICE. + +**Restricted Fields:** +spec.containers[*].securityContext.capabilities.drop +spec.initContainers[*].securityContext.capabilities.drop + +**Allowed Values:** +Must include "ALL" + +**Restricted Fields:** +spec.containers[*].securityContext.capabilities.add +spec.initContainers[*].securityContext.capabilities.add + +**Allowed Values:** +undefined / empty +"NET_BIND_SERVICE" +*/ + func init() { addCheck(CheckCapabilitiesRestricted) } @@ -64,36 +83,46 @@ func capabilitiesRestricted_1_22(podMetadata *metav1.ObjectMeta, podSpec *corev1 return } - found := false + droppedAll := false for _, c := range container.SecurityContext.Capabilities.Drop { if c == capabilityAll { - found = true + droppedAll = true break } } - if !found { + if !droppedAll { containersMissingDropAll = append(containersMissingDropAll, container.Name) } + addedForbidden := false for _, c := range container.SecurityContext.Capabilities.Add { if c != capabilityNetBindService { - containersAddingForbidden = append(containersAddingForbidden, container.Name) + addedForbidden = true forbiddenCapabilities.Insert(string(c)) } } - + if addedForbidden { + containersAddingForbidden = append(containersAddingForbidden, container.Name) + } }) var forbiddenDetails []string if len(containersMissingDropAll) > 0 { - forbiddenDetails = append(forbiddenDetails, fmt.Sprintf("containers %q must drop ALL", containersMissingDropAll)) + forbiddenDetails = append(forbiddenDetails, fmt.Sprintf( + `%s %s must set securityContext.capabilities.drop=["ALL"]`, + pluralize("container", "containers", len(containersMissingDropAll)), + joinQuote(containersMissingDropAll))) } if len(containersAddingForbidden) > 0 { - forbiddenDetails = append(forbiddenDetails, fmt.Sprintf("containers %q must not add %q", containersAddingForbidden, forbiddenCapabilities.List())) + forbiddenDetails = append(forbiddenDetails, fmt.Sprintf( + `%s %s must not include %s in securityContext.capabilities.add`, + pluralize("container", "containers", len(containersAddingForbidden)), + joinQuote(containersAddingForbidden), + joinQuote(forbiddenCapabilities.List()))) } if len(forbiddenDetails) > 0 { return CheckResult{ Allowed: false, - ForbiddenReason: "containers must restrict capabilities", + ForbiddenReason: "unrestricted capabilities", ForbiddenDetail: strings.Join(forbiddenDetails, "; "), } } diff --git a/staging/src/k8s.io/pod-security-admission/policy/check_capabilities_restricted_test.go b/staging/src/k8s.io/pod-security-admission/policy/check_capabilities_restricted_test.go new file mode 100644 index 00000000000..4e6dcd6e318 --- /dev/null +++ b/staging/src/k8s.io/pod-security-admission/policy/check_capabilities_restricted_test.go @@ -0,0 +1,59 @@ +/* +Copyright 2021 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 policy + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" +) + +func TestCapabilitiesRestricted(t *testing.T) { + tests := []struct { + name string + pod *corev1.Pod + expectReason string + expectDetail string + }{ + { + name: "multiple containers", + pod: &corev1.Pod{Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "a", SecurityContext: &corev1.SecurityContext{Capabilities: &corev1.Capabilities{Add: []corev1.Capability{"FOO", "BAR"}}}}, + {Name: "b", SecurityContext: &corev1.SecurityContext{Capabilities: &corev1.Capabilities{Add: []corev1.Capability{"BAR", "BAZ"}}}}, + {Name: "c", SecurityContext: &corev1.SecurityContext{Capabilities: &corev1.Capabilities{Add: []corev1.Capability{"NET_BIND_SERVICE", "CHOWN"}, Drop: []corev1.Capability{"ALL", "FOO"}}}}, + }}}, + expectReason: `unrestricted capabilities`, + expectDetail: `containers "a", "b" must set securityContext.capabilities.drop=["ALL"]; containers "a", "b", "c" must not include "BAR", "BAZ", "CHOWN", "FOO" in securityContext.capabilities.add`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := capabilitiesRestricted_1_22(&tc.pod.ObjectMeta, &tc.pod.Spec) + if result.Allowed { + t.Fatal("expected disallowed") + } + if e, a := tc.expectReason, result.ForbiddenReason; e != a { + t.Errorf("expected\n%s\ngot\n%s", e, a) + } + if e, a := tc.expectDetail, result.ForbiddenDetail; e != a { + t.Errorf("expected\n%s\ngot\n%s", e, a) + } + }) + } +} diff --git a/staging/src/k8s.io/pod-security-admission/test/fixtures_capabilities_restricted.go b/staging/src/k8s.io/pod-security-admission/test/fixtures_capabilities_restricted.go index 3cca372d851..98f098e7860 100644 --- a/staging/src/k8s.io/pod-security-admission/test/fixtures_capabilities_restricted.go +++ b/staging/src/k8s.io/pod-security-admission/test/fixtures_capabilities_restricted.go @@ -30,14 +30,10 @@ containerFields: []string{ func init() { fixtureData_1_22 := fixtureGenerator{ - expectErrorSubstring: "restrict capabilities", + expectErrorSubstring: "unrestricted capabilities", generatePass: func(p *corev1.Pod) []*corev1.Pod { p = ensureCapabilities(p) return []*corev1.Pod{ - tweak(p, func(p *corev1.Pod) { - p.Spec.Containers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{"ALL"} - p.Spec.InitContainers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{"ALL"} - }), tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{"ALL"} p.Spec.InitContainers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{"ALL"} @@ -49,10 +45,15 @@ func init() { generateFail: func(p *corev1.Pod) []*corev1.Pod { p = ensureCapabilities(p) return []*corev1.Pod{ + // test container tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{} + }), + // test initContainer + tweak(p, func(p *corev1.Pod) { p.Spec.InitContainers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{} }), + // dropping all individual capabilities is not sufficient tweak(p, func(p *corev1.Pod) { p.Spec.Containers[0].SecurityContext.Capabilities.Drop = []corev1.Capability{ "SYS_TIME", "SYS_MODULE", "SYS_RAWIO", "SYS_PACCT", "SYS_ADMIN", "SYS_NICE",