diff --git a/staging/src/k8s.io/pod-security-admission/policy/check_appArmorProfile.go b/staging/src/k8s.io/pod-security-admission/policy/check_appArmorProfile.go index b4942a884cf..c4342b81c57 100644 --- a/staging/src/k8s.io/pod-security-admission/policy/check_appArmorProfile.go +++ b/staging/src/k8s.io/pod-security-admission/policy/check_appArmorProfile.go @@ -23,6 +23,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/pod-security-admission/api" ) @@ -35,6 +36,14 @@ profile, or restrict overrides to an allowed set of profiles. metadata.annotations['container.apparmor.security.beta.kubernetes.io/*'] **Allowed Values:** 'runtime/default', 'localhost/*', empty, undefined + +**Restricted Fields:** +spec.securityContext.appArmorProfile.type +spec.containers[*].securityContext.appArmorProfile.type +spec.initContainers[*].securityContext.appArmorProfile.type +spec.ephemeralContainers[*].securityContext.appArmorProfile.type + +**Allowed Values:** 'RuntimeDefault', 'Localhost', undefined */ func init() { addCheck(CheckAppArmorProfile) @@ -55,25 +64,78 @@ func CheckAppArmorProfile() Check { } } -func allowedProfile(profile string) bool { +func allowedAnnotationValue(profile string) bool { return len(profile) == 0 || profile == corev1.AppArmorBetaProfileRuntimeDefault || strings.HasPrefix(profile, corev1.AppArmorBetaProfileNamePrefix) } +func allowedProfileType(profile corev1.AppArmorProfileType) bool { + switch profile { + case corev1.AppArmorProfileTypeRuntimeDefault, + corev1.AppArmorProfileTypeLocalhost: + return true + default: + return false + } +} + func appArmorProfile_1_0(podMetadata *metav1.ObjectMeta, podSpec *corev1.PodSpec) CheckResult { - var forbiddenValues []string - for k, v := range podMetadata.Annotations { - if strings.HasPrefix(k, corev1.AppArmorBetaContainerAnnotationKeyPrefix) && !allowedProfile(v) { - forbiddenValues = append(forbiddenValues, fmt.Sprintf("%s=%q", k, v)) + var badSetters []string // things that explicitly set appArmorProfile.type to a bad value + badValues := sets.NewString() + + if podSpec.SecurityContext != nil && podSpec.SecurityContext.AppArmorProfile != nil { + if !allowedProfileType(podSpec.SecurityContext.AppArmorProfile.Type) { + badSetters = append(badSetters, "pod") + badValues.Insert(string(podSpec.SecurityContext.AppArmorProfile.Type)) } } - if len(forbiddenValues) > 0 { - sort.Strings(forbiddenValues) + + var badContainers []string // containers that set apparmorProfile.type to a bad value + visitContainers(podSpec, func(c *corev1.Container) { + if c.SecurityContext != nil && c.SecurityContext.AppArmorProfile != nil { + if !allowedProfileType(c.SecurityContext.AppArmorProfile.Type) { + badContainers = append(badContainers, c.Name) + badValues.Insert(string(c.SecurityContext.AppArmorProfile.Type)) + } + } + }) + + if len(badContainers) > 0 { + badSetters = append( + badSetters, + fmt.Sprintf( + "%s %s", + pluralize("container", "containers", len(badContainers)), + joinQuote(badContainers), + ), + ) + } + + var forbiddenAnnotations []string + for k, v := range podMetadata.Annotations { + if strings.HasPrefix(k, corev1.AppArmorBetaContainerAnnotationKeyPrefix) && !allowedAnnotationValue(v) { + forbiddenAnnotations = append(forbiddenAnnotations, fmt.Sprintf("%s=%q", k, v)) + } + } + + badValueList := badValues.List() + if len(forbiddenAnnotations) > 0 { + sort.Strings(forbiddenAnnotations) + badValueList = append(badValueList, forbiddenAnnotations...) + badSetters = append(badSetters, pluralize("annotation", "annotations", len(forbiddenAnnotations))) + } + + // pod or containers explicitly set bad apparmorProfiles + if len(badSetters) > 0 { return CheckResult{ Allowed: false, - ForbiddenReason: pluralize("forbidden AppArmor profile", "forbidden AppArmor profiles", len(forbiddenValues)), - ForbiddenDetail: strings.Join(forbiddenValues, ", "), + ForbiddenReason: pluralize("forbidden AppArmor profile", "forbidden AppArmor profiles", len(badValueList)), + ForbiddenDetail: fmt.Sprintf( + "%s must not set AppArmor profile type to %s", + strings.Join(badSetters, " and "), + joinQuote(badValueList), + ), } } diff --git a/staging/src/k8s.io/pod-security-admission/policy/check_appArmorProfile_test.go b/staging/src/k8s.io/pod-security-admission/policy/check_appArmorProfile_test.go index e16b3312b4d..91295ef913f 100644 --- a/staging/src/k8s.io/pod-security-admission/policy/check_appArmorProfile_test.go +++ b/staging/src/k8s.io/pod-security-admission/policy/check_appArmorProfile_test.go @@ -24,69 +24,145 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestCheckAppArmor(t *testing.T) { - +func TestCheckAppArmor_Allowed(t *testing.T) { testCases := []struct { - name string - metaData *metav1.ObjectMeta - podSpec *corev1.PodSpec - expectedResult *CheckResult + name string + metaData *metav1.ObjectMeta + podSpec *corev1.PodSpec }{ { name: "container with default AppArmor + extra annotations", metaData: &metav1.ObjectMeta{Annotations: map[string]string{ corev1.AppArmorBetaProfileNamePrefix + "test": "runtime/default", "env": "prod", - }, - }, - podSpec: &corev1.PodSpec{}, - expectedResult: &CheckResult{Allowed: true}, + }}, + podSpec: &corev1.PodSpec{}, }, { name: "container with local AppArmor + extra annotations", metaData: &metav1.ObjectMeta{Annotations: map[string]string{ corev1.AppArmorBetaProfileNamePrefix + "test": "localhost/sec-profile01", "env": "dev", - }, - }, - podSpec: &corev1.PodSpec{}, - expectedResult: &CheckResult{Allowed: true}, + }}, + podSpec: &corev1.PodSpec{}, }, { name: "container with no AppArmor annotations", metaData: &metav1.ObjectMeta{Annotations: map[string]string{ "env": "dev", - }, - }, - podSpec: &corev1.PodSpec{}, - expectedResult: &CheckResult{Allowed: true}, + }}, + podSpec: &corev1.PodSpec{}, }, { - name: "container with no annotations", - metaData: &metav1.ObjectMeta{}, - podSpec: &corev1.PodSpec{}, - expectedResult: &CheckResult{Allowed: true}, + name: "container with no annotations", + metaData: &metav1.ObjectMeta{}, + podSpec: &corev1.PodSpec{}, + }, + { + name: "pod with runtime default", + metaData: &metav1.ObjectMeta{}, + podSpec: &corev1.PodSpec{ + SecurityContext: &corev1.PodSecurityContext{ + AppArmorProfile: &corev1.AppArmorProfile{ + Type: corev1.AppArmorProfileTypeRuntimeDefault, + }, + }, + }, + }, + { + name: "container with localhost profile", + metaData: &metav1.ObjectMeta{}, + podSpec: &corev1.PodSpec{ + Containers: []corev1.Container{{ + Name: "foo", + SecurityContext: &corev1.SecurityContext{ + AppArmorProfile: &corev1.AppArmorProfile{ + Type: corev1.AppArmorProfileTypeRuntimeDefault, + }, + }, + }}, + }, }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - result := appArmorProfile_1_0(testCase.metaData, nil) - if result.Allowed != testCase.expectedResult.Allowed { - t.Errorf("Expected result was Allowed=%v for annotations %v", - testCase.expectedResult.Allowed, testCase.metaData.Annotations) + result := appArmorProfile_1_0(testCase.metaData, testCase.podSpec) + if !result.Allowed { + t.Errorf("Should be allowed") } }) } } -func TestAppArmorProfile(t *testing.T) { +func TestCheckAppArmor_Forbidden(t *testing.T) { tests := []struct { name string pod *corev1.Pod expectReason string expectDetail string }{ + { + name: "unconfined pod", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + SecurityContext: &corev1.PodSecurityContext{ + AppArmorProfile: &corev1.AppArmorProfile{ + Type: corev1.AppArmorProfileTypeUnconfined, + }, + }, + }, + }, + expectReason: "forbidden AppArmor profile", + expectDetail: `pod must not set AppArmor profile type to "Unconfined"`, + }, + { + name: "unconfined container", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + SecurityContext: &corev1.PodSecurityContext{ + AppArmorProfile: &corev1.AppArmorProfile{ + Type: corev1.AppArmorProfileTypeRuntimeDefault, + }, + }, + Containers: []corev1.Container{{ + Name: "foo", + SecurityContext: &corev1.SecurityContext{ + AppArmorProfile: &corev1.AppArmorProfile{ + Type: corev1.AppArmorProfileTypeUnconfined, + }, + }, + }}, + }, + }, + expectReason: "forbidden AppArmor profile", + expectDetail: `container "foo" must not set AppArmor profile type to "Unconfined"`, + }, + { + name: "unconfined init container", + pod: &corev1.Pod{ + Spec: corev1.PodSpec{ + SecurityContext: &corev1.PodSecurityContext{ + AppArmorProfile: &corev1.AppArmorProfile{ + Type: corev1.AppArmorProfileTypeRuntimeDefault, + }, + }, + Containers: []corev1.Container{{ + Name: "foo", + }}, + InitContainers: []corev1.Container{{ + Name: "bar", + SecurityContext: &corev1.SecurityContext{ + AppArmorProfile: &corev1.AppArmorProfile{ + Type: corev1.AppArmorProfileTypeUnconfined, + }, + }, + }}, + }, + }, + expectReason: "forbidden AppArmor profile", + expectDetail: `container "bar" must not set AppArmor profile type to "Unconfined"`, + }, { name: "multiple containers", pod: &corev1.Pod{ @@ -102,11 +178,11 @@ func TestAppArmorProfile(t *testing.T) { }, }, }, - expectReason: `forbidden AppArmor profiles`, - expectDetail: strings.Join([]string{ - `container.apparmor.security.beta.kubernetes.io/="bogus"`, - `container.apparmor.security.beta.kubernetes.io/e="unconfined"`, - `container.apparmor.security.beta.kubernetes.io/f="unknown"`, + expectReason: "forbidden AppArmor profiles", + expectDetail: "annotations must not set AppArmor profile type to " + strings.Join([]string{ + `"container.apparmor.security.beta.kubernetes.io/="bogus""`, + `"container.apparmor.security.beta.kubernetes.io/e="unconfined""`, + `"container.apparmor.security.beta.kubernetes.io/f="unknown""`, }, ", "), }, }