PodSecurity check for AppArmor fields

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
+	var badSetters []string // things that explicitly set appArmorProfile.type to a bad value
-	for k, v := range podMetadata.Annotations {
+	badValues := sets.NewString()
-		if strings.HasPrefix(k, corev1.AppArmorBetaContainerAnnotationKeyPrefix) && !allowedProfile(v) {
+
-			forbiddenValues = append(forbiddenValues, fmt.Sprintf("%s=%q", k, v))
+	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)),
+			ForbiddenReason: pluralize("forbidden AppArmor profile", "forbidden AppArmor profiles", len(badValueList)),
-			ForbiddenDetail: strings.Join(forbiddenValues, ", "),
+			ForbiddenDetail: fmt.Sprintf(
+				"%s must not set AppArmor profile type to %s",
+				strings.Join(badSetters, " and "),
+				joinQuote(badValueList),
+			),
 		}
 	}
 

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
+		name     string
-		metaData       *metav1.ObjectMeta
+		metaData *metav1.ObjectMeta
-		podSpec        *corev1.PodSpec
+		podSpec  *corev1.PodSpec
-		expectedResult *CheckResult
 	}{
 		{
 			name: "container with default AppArmor + extra annotations",
 			metaData: &metav1.ObjectMeta{Annotations: map[string]string{
 				corev1.AppArmorBetaProfileNamePrefix + "test": "runtime/default",
 				"env": "prod",
-			},
+			}},
-			},
+			podSpec: &corev1.PodSpec{},
-			podSpec:        &corev1.PodSpec{},
-			expectedResult: &CheckResult{Allowed: true},
 		},
 		{
 			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{},
-			podSpec:        &corev1.PodSpec{},
-			expectedResult: &CheckResult{Allowed: true},
 		},
 		{
 			name: "container with no AppArmor annotations",
 			metaData: &metav1.ObjectMeta{Annotations: map[string]string{
 				"env": "dev",
-			},
+			}},
-			},
+			podSpec: &corev1.PodSpec{},
-			podSpec:        &corev1.PodSpec{},
-			expectedResult: &CheckResult{Allowed: true},
 		},
 		{
-			name:           "container with no annotations",
+			name:     "container with no annotations",
-			metaData:       &metav1.ObjectMeta{},
+			metaData: &metav1.ObjectMeta{},
-			podSpec:        &corev1.PodSpec{},
+			podSpec:  &corev1.PodSpec{},
-			expectedResult: &CheckResult{Allowed: true},
+		},
+		{
+			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)
+			result := appArmorProfile_1_0(testCase.metaData, testCase.podSpec)
-			if result.Allowed != testCase.expectedResult.Allowed {
+			if !result.Allowed {
-				t.Errorf("Expected result was Allowed=%v for annotations %v",
+				t.Errorf("Should be allowed")
-					testCase.expectedResult.Allowed, testCase.metaData.Annotations)
 			}
 		})
 	}
 }
 
-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`,
+			expectReason: "forbidden AppArmor profiles",
-			expectDetail: strings.Join([]string{
+			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/="bogus""`,
-				`container.apparmor.security.beta.kubernetes.io/e="unconfined"`,
+				`"container.apparmor.security.beta.kubernetes.io/e="unconfined""`,
-				`container.apparmor.security.beta.kubernetes.io/f="unknown"`,
+				`"container.apparmor.security.beta.kubernetes.io/f="unknown""`,
 			}, ", "),
 		},
 	}