From 06caf32ecd70df0f3b1a04946e76ce160a64626f Mon Sep 17 00:00:00 2001 From: Tim Allclair Date: Mon, 4 Mar 2024 10:41:20 -0800 Subject: [PATCH] Validate localhost profile max length --- pkg/apis/core/types.go | 1 + pkg/apis/core/validation/validation.go | 11 ++++-- pkg/apis/core/validation/validation_test.go | 44 +++++++++++++++------ pkg/registry/core/pod/strategy.go | 4 ++ pkg/registry/core/pod/strategy_test.go | 18 +++++++++ 5 files changed, 63 insertions(+), 15 deletions(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 2330cac7a6c..6057a04666e 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3650,6 +3650,7 @@ type AppArmorProfile struct { LocalhostProfile *string } +// +enum type AppArmorProfileType string const ( diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 003746c3185..76c14dbeb84 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4663,7 +4663,7 @@ func validateSeccompProfileType(fldPath *field.Path, seccompProfileType core.Sec } } -func validateAppArmorProfileField(profile *core.AppArmorProfile, fldPath *field.Path) field.ErrorList { +func ValidateAppArmorProfileField(profile *core.AppArmorProfile, fldPath *field.Path) field.ErrorList { if profile == nil { return nil } @@ -4681,6 +4681,11 @@ func validateAppArmorProfileField(profile *core.AppArmorProfile, fldPath *field. } else if localhostProfile == "" { allErrs = append(allErrs, field.Required(fldPath.Child("localhostProfile"), "must be set when AppArmor type is Localhost")) } + + const maxLocalhostProfileLength = 4095 // PATH_MAX - 1 + if len(*profile.LocalhostProfile) > maxLocalhostProfileLength { + allErrs = append(allErrs, field.TooLongMaxLength(fldPath.Child("localhostProfile"), *profile.LocalhostProfile, maxLocalhostProfileLength)) + } } case core.AppArmorProfileTypeRuntimeDefault, core.AppArmorProfileTypeUnconfined: @@ -4894,7 +4899,7 @@ func validatePodSpecSecurityContext(securityContext *core.PodSecurityContext, sp allErrs = append(allErrs, validateSeccompProfileField(securityContext.SeccompProfile, fldPath.Child("seccompProfile"))...) allErrs = append(allErrs, validateWindowsSecurityContextOptions(securityContext.WindowsOptions, fldPath.Child("windowsOptions"))...) - allErrs = append(allErrs, validateAppArmorProfileField(securityContext.AppArmorProfile, fldPath.Child("appArmorProfile"))...) + allErrs = append(allErrs, ValidateAppArmorProfileField(securityContext.AppArmorProfile, fldPath.Child("appArmorProfile"))...) } return allErrs @@ -7181,7 +7186,7 @@ func ValidateSecurityContext(sc *core.SecurityContext, fldPath *field.Path) fiel } allErrs = append(allErrs, validateWindowsSecurityContextOptions(sc.WindowsOptions, fldPath.Child("windowsOptions"))...) - allErrs = append(allErrs, validateAppArmorProfileField(sc.AppArmorProfile, fldPath.Child("appArmorProfile"))...) + allErrs = append(allErrs, ValidateAppArmorProfileField(sc.AppArmorProfile, fldPath.Child("appArmorProfile"))...) return allErrs } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index e96dc5291e3..abd0046b5ea 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -10294,7 +10294,7 @@ func TestValidatePod(t *testing.T) { Name: "123", Namespace: "ns", Annotations: map[string]string{ - v1.AppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.AppArmorBetaProfileRuntimeDefault, + v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.DeprecatedAppArmorBetaProfileRuntimeDefault, }, }, Spec: validPodSpec(nil), @@ -10304,7 +10304,7 @@ func TestValidatePod(t *testing.T) { Name: "123", Namespace: "ns", Annotations: map[string]string{ - v1.AppArmorBetaContainerAnnotationKeyPrefix + "init-ctr": v1.AppArmorBetaProfileRuntimeDefault, + v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "init-ctr": v1.DeprecatedAppArmorBetaProfileRuntimeDefault, }, }, Spec: core.PodSpec{ @@ -10319,7 +10319,7 @@ func TestValidatePod(t *testing.T) { Name: "123", Namespace: "ns", Annotations: map[string]string{ - v1.AppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.AppArmorBetaProfileNamePrefix + "foo", + v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.DeprecatedAppArmorBetaProfileNamePrefix + "foo", }, }, Spec: validPodSpec(nil), @@ -11983,9 +11983,9 @@ func TestValidatePod(t *testing.T) { Name: "123", Namespace: "ns", Annotations: map[string]string{ - v1.AppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.AppArmorBetaProfileRuntimeDefault, - v1.AppArmorBetaContainerAnnotationKeyPrefix + "init-ctr": v1.AppArmorBetaProfileRuntimeDefault, - v1.AppArmorBetaContainerAnnotationKeyPrefix + "fake-ctr": v1.AppArmorBetaProfileRuntimeDefault, + v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.DeprecatedAppArmorBetaProfileRuntimeDefault, + v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "init-ctr": v1.DeprecatedAppArmorBetaProfileRuntimeDefault, + v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "fake-ctr": v1.DeprecatedAppArmorBetaProfileRuntimeDefault, }, }, Spec: core.PodSpec{ @@ -12003,7 +12003,7 @@ func TestValidatePod(t *testing.T) { Name: "123", Namespace: "ns", Annotations: map[string]string{ - v1.AppArmorBetaContainerAnnotationKeyPrefix + "ctr": "bad-name", + v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "ctr": "bad-name", }, }, Spec: validPodSpec(nil), @@ -12016,7 +12016,7 @@ func TestValidatePod(t *testing.T) { Name: "123", Namespace: "ns", Annotations: map[string]string{ - v1.AppArmorBetaContainerAnnotationKeyPrefix + "ctr": "runtime/foo", + v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "ctr": "runtime/foo", }, }, Spec: validPodSpec(nil), @@ -12159,6 +12159,26 @@ func TestValidatePod(t *testing.T) { }, }, }, + "too long AppArmor localhost profile": { + expectedError: "Too long: may not be longer than 4095", + spec: core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "123", + Namespace: "ns", + }, + Spec: core.PodSpec{ + Containers: []core.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}}, + RestartPolicy: core.RestartPolicyAlways, + DNSPolicy: core.DNSDefault, + SecurityContext: &core.PodSecurityContext{ + AppArmorProfile: &core.AppArmorProfile{ + Type: core.AppArmorProfileTypeLocalhost, + LocalhostProfile: ptr.To(strings.Repeat("a", 4096)), + }, + }, + }, + }, + }, "mismatched AppArmor field and annotation types": { expectedError: "Forbidden: apparmor type in annotation and field must match", spec: core.Pod{ @@ -25186,11 +25206,11 @@ func TestValidateAppArmorProfileFormat(t *testing.T) { expectValid bool }{ {"", true}, - {v1.AppArmorBetaProfileRuntimeDefault, true}, - {v1.AppArmorBetaProfileNameUnconfined, true}, + {v1.DeprecatedAppArmorBetaProfileRuntimeDefault, true}, + {v1.DeprecatedAppArmorBetaProfileNameUnconfined, true}, {"baz", false}, // Missing local prefix. - {v1.AppArmorBetaProfileNamePrefix + "/usr/sbin/ntpd", true}, - {v1.AppArmorBetaProfileNamePrefix + "foo-bar", true}, + {v1.DeprecatedAppArmorBetaProfileNamePrefix + "/usr/sbin/ntpd", true}, + {v1.DeprecatedAppArmorBetaProfileNamePrefix + "foo-bar", true}, } for _, test := range tests { diff --git a/pkg/registry/core/pod/strategy.go b/pkg/registry/core/pod/strategy.go index 76c572f9912..927d15f69b0 100644 --- a/pkg/registry/core/pod/strategy.go +++ b/pkg/registry/core/pod/strategy.go @@ -802,6 +802,10 @@ func applyAppArmorVersionSkew(pod *api.Pod) { } } else if containerProfile == nil { newField := apparmorFieldForAnnotation(annotation) + if errs := corevalidation.ValidateAppArmorProfileField(newField, &field.Path{}); len(errs) > 0 { + // Skip copying invalid value. + newField = nil + } // Only copy the annotation to the field if it is different from the pod-level profile. if newField != nil && !apiequality.Semantic.DeepEqual(newField, podProfile) { diff --git a/pkg/registry/core/pod/strategy_test.go b/pkg/registry/core/pod/strategy_test.go index 064955ddd08..9b28597b107 100644 --- a/pkg/registry/core/pod/strategy_test.go +++ b/pkg/registry/core/pod/strategy_test.go @@ -22,6 +22,7 @@ import ( "net/http" "net/url" "reflect" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -2583,6 +2584,23 @@ func TestApplyAppArmorVersionSkew(t *testing.T) { assert.Nil(t, pod.Spec.Containers[0].SecurityContext) assert.Nil(t, pod.Spec.SecurityContext) }, + }, { + description: "Invalid localhost annotation", + pod: &api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + api.DeprecatedAppArmorAnnotationKeyPrefix + "ctr": api.DeprecatedAppArmorAnnotationValueLocalhostPrefix + strings.Repeat("a", 4096), + }, + }, + Spec: api.PodSpec{ + Containers: []api.Container{{Name: "ctr"}}, + }, + }, + validation: func(t *testing.T, pod *api.Pod) { + assert.Contains(t, pod.Annotations, api.DeprecatedAppArmorAnnotationKeyPrefix+"ctr") + assert.Nil(t, pod.Spec.Containers[0].SecurityContext) + assert.Nil(t, pod.Spec.SecurityContext) + }, }, { description: "Invalid field type", pod: &api.Pod{