Skip to content

Commit 06caf32

Browse files
tallclairtallclair
committed
Validate localhost profile max length
1 parent 0eb5f52 commit 06caf32

File tree

5 files changed

+63
-15
lines changed

5 files changed

+63
-15
lines changed

‎pkg/apis/core/types.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3650,6 +3650,7 @@ type AppArmorProfile struct {
36503650
LocalhostProfile *string
36513651
}
36523652

3653+
// +enum
36533654
type AppArmorProfileType string
36543655

36553656
const (

‎pkg/apis/core/validation/validation.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4663,7 +4663,7 @@ func validateSeccompProfileType(fldPath *field.Path, seccompProfileType core.Sec
46634663
}
46644664
}
46654665

4666-
func validateAppArmorProfileField(profile *core.AppArmorProfile, fldPath *field.Path) field.ErrorList {
4666+
func ValidateAppArmorProfileField(profile *core.AppArmorProfile, fldPath *field.Path) field.ErrorList {
46674667
if profile == nil {
46684668
return nil
46694669
}
@@ -4681,6 +4681,11 @@ func validateAppArmorProfileField(profile *core.AppArmorProfile, fldPath *field.
46814681
} else if localhostProfile == "" {
46824682
allErrs = append(allErrs, field.Required(fldPath.Child("localhostProfile"), "must be set when AppArmor type is Localhost"))
46834683
}
4684+
4685+
const maxLocalhostProfileLength = 4095 // PATH_MAX - 1
4686+
if len(*profile.LocalhostProfile) > maxLocalhostProfileLength {
4687+
allErrs = append(allErrs, field.TooLongMaxLength(fldPath.Child("localhostProfile"), *profile.LocalhostProfile, maxLocalhostProfileLength))
4688+
}
46844689
}
46854690

46864691
case core.AppArmorProfileTypeRuntimeDefault, core.AppArmorProfileTypeUnconfined:
@@ -4894,7 +4899,7 @@ func validatePodSpecSecurityContext(securityContext *core.PodSecurityContext, sp
48944899

48954900
allErrs = append(allErrs, validateSeccompProfileField(securityContext.SeccompProfile, fldPath.Child("seccompProfile"))...)
48964901
allErrs = append(allErrs, validateWindowsSecurityContextOptions(securityContext.WindowsOptions, fldPath.Child("windowsOptions"))...)
4897-
allErrs = append(allErrs, validateAppArmorProfileField(securityContext.AppArmorProfile, fldPath.Child("appArmorProfile"))...)
4902+
allErrs = append(allErrs, ValidateAppArmorProfileField(securityContext.AppArmorProfile, fldPath.Child("appArmorProfile"))...)
48984903
}
48994904

49004905
return allErrs
@@ -7181,7 +7186,7 @@ func ValidateSecurityContext(sc *core.SecurityContext, fldPath *field.Path) fiel
71817186
}
71827187

71837188
allErrs = append(allErrs, validateWindowsSecurityContextOptions(sc.WindowsOptions, fldPath.Child("windowsOptions"))...)
7184-
allErrs = append(allErrs, validateAppArmorProfileField(sc.AppArmorProfile, fldPath.Child("appArmorProfile"))...)
7189+
allErrs = append(allErrs, ValidateAppArmorProfileField(sc.AppArmorProfile, fldPath.Child("appArmorProfile"))...)
71857190

71867191
return allErrs
71877192
}

‎pkg/apis/core/validation/validation_test.go‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10294,7 +10294,7 @@ func TestValidatePod(t *testing.T) {
1029410294
Name: "123",
1029510295
Namespace: "ns",
1029610296
Annotations: map[string]string{
10297-
v1.AppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.AppArmorBetaProfileRuntimeDefault,
10297+
v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.DeprecatedAppArmorBetaProfileRuntimeDefault,
1029810298
},
1029910299
},
1030010300
Spec: validPodSpec(nil),
@@ -10304,7 +10304,7 @@ func TestValidatePod(t *testing.T) {
1030410304
Name: "123",
1030510305
Namespace: "ns",
1030610306
Annotations: map[string]string{
10307-
v1.AppArmorBetaContainerAnnotationKeyPrefix + "init-ctr": v1.AppArmorBetaProfileRuntimeDefault,
10307+
v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "init-ctr": v1.DeprecatedAppArmorBetaProfileRuntimeDefault,
1030810308
},
1030910309
},
1031010310
Spec: core.PodSpec{
@@ -10319,7 +10319,7 @@ func TestValidatePod(t *testing.T) {
1031910319
Name: "123",
1032010320
Namespace: "ns",
1032110321
Annotations: map[string]string{
10322-
v1.AppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.AppArmorBetaProfileNamePrefix + "foo",
10322+
v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.DeprecatedAppArmorBetaProfileNamePrefix + "foo",
1032310323
},
1032410324
},
1032510325
Spec: validPodSpec(nil),
@@ -11983,9 +11983,9 @@ func TestValidatePod(t *testing.T) {
1198311983
Name: "123",
1198411984
Namespace: "ns",
1198511985
Annotations: map[string]string{
11986-
v1.AppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.AppArmorBetaProfileRuntimeDefault,
11987-
v1.AppArmorBetaContainerAnnotationKeyPrefix + "init-ctr": v1.AppArmorBetaProfileRuntimeDefault,
11988-
v1.AppArmorBetaContainerAnnotationKeyPrefix + "fake-ctr": v1.AppArmorBetaProfileRuntimeDefault,
11986+
v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "ctr": v1.DeprecatedAppArmorBetaProfileRuntimeDefault,
11987+
v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "init-ctr": v1.DeprecatedAppArmorBetaProfileRuntimeDefault,
11988+
v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "fake-ctr": v1.DeprecatedAppArmorBetaProfileRuntimeDefault,
1198911989
},
1199011990
},
1199111991
Spec: core.PodSpec{
@@ -12003,7 +12003,7 @@ func TestValidatePod(t *testing.T) {
1200312003
Name: "123",
1200412004
Namespace: "ns",
1200512005
Annotations: map[string]string{
12006-
v1.AppArmorBetaContainerAnnotationKeyPrefix + "ctr": "bad-name",
12006+
v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "ctr": "bad-name",
1200712007
},
1200812008
},
1200912009
Spec: validPodSpec(nil),
@@ -12016,7 +12016,7 @@ func TestValidatePod(t *testing.T) {
1201612016
Name: "123",
1201712017
Namespace: "ns",
1201812018
Annotations: map[string]string{
12019-
v1.AppArmorBetaContainerAnnotationKeyPrefix + "ctr": "runtime/foo",
12019+
v1.DeprecatedAppArmorBetaContainerAnnotationKeyPrefix + "ctr": "runtime/foo",
1202012020
},
1202112021
},
1202212022
Spec: validPodSpec(nil),
@@ -12159,6 +12159,26 @@ func TestValidatePod(t *testing.T) {
1215912159
},
1216012160
},
1216112161
},
12162+
"too long AppArmor localhost profile": {
12163+
expectedError: "Too long: may not be longer than 4095",
12164+
spec: core.Pod{
12165+
ObjectMeta: metav1.ObjectMeta{
12166+
Name: "123",
12167+
Namespace: "ns",
12168+
},
12169+
Spec: core.PodSpec{
12170+
Containers: []core.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}},
12171+
RestartPolicy: core.RestartPolicyAlways,
12172+
DNSPolicy: core.DNSDefault,
12173+
SecurityContext: &core.PodSecurityContext{
12174+
AppArmorProfile: &core.AppArmorProfile{
12175+
Type: core.AppArmorProfileTypeLocalhost,
12176+
LocalhostProfile: ptr.To(strings.Repeat("a", 4096)),
12177+
},
12178+
},
12179+
},
12180+
},
12181+
},
1216212182
"mismatched AppArmor field and annotation types": {
1216312183
expectedError: "Forbidden: apparmor type in annotation and field must match",
1216412184
spec: core.Pod{
@@ -25186,11 +25206,11 @@ func TestValidateAppArmorProfileFormat(t *testing.T) {
2518625206
expectValid bool
2518725207
}{
2518825208
{"", true},
25189-
{v1.AppArmorBetaProfileRuntimeDefault, true},
25190-
{v1.AppArmorBetaProfileNameUnconfined, true},
25209+
{v1.DeprecatedAppArmorBetaProfileRuntimeDefault, true},
25210+
{v1.DeprecatedAppArmorBetaProfileNameUnconfined, true},
2519125211
{"baz", false}, // Missing local prefix.
25192-
{v1.AppArmorBetaProfileNamePrefix + "/usr/sbin/ntpd", true},
25193-
{v1.AppArmorBetaProfileNamePrefix + "foo-bar", true},
25212+
{v1.DeprecatedAppArmorBetaProfileNamePrefix + "/usr/sbin/ntpd", true},
25213+
{v1.DeprecatedAppArmorBetaProfileNamePrefix + "foo-bar", true},
2519425214
}
2519525215

2519625216
for _, test := range tests {

‎pkg/registry/core/pod/strategy.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,10 @@ func applyAppArmorVersionSkew(pod *api.Pod) {
802802
}
803803
} else if containerProfile == nil {
804804
newField := apparmorFieldForAnnotation(annotation)
805+
if errs := corevalidation.ValidateAppArmorProfileField(newField, &field.Path{}); len(errs) > 0 {
806+
// Skip copying invalid value.
807+
newField = nil
808+
}
805809

806810
// Only copy the annotation to the field if it is different from the pod-level profile.
807811
if newField != nil && !apiequality.Semantic.DeepEqual(newField, podProfile) {

‎pkg/registry/core/pod/strategy_test.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"net/http"
2323
"net/url"
2424
"reflect"
25+
"strings"
2526
"testing"
2627

2728
"github.com/google/go-cmp/cmp"
@@ -2583,6 +2584,23 @@ func TestApplyAppArmorVersionSkew(t *testing.T) {
25832584
assert.Nil(t, pod.Spec.Containers[0].SecurityContext)
25842585
assert.Nil(t, pod.Spec.SecurityContext)
25852586
},
2587+
}, {
2588+
description: "Invalid localhost annotation",
2589+
pod: &api.Pod{
2590+
ObjectMeta: metav1.ObjectMeta{
2591+
Annotations: map[string]string{
2592+
api.DeprecatedAppArmorAnnotationKeyPrefix + "ctr": api.DeprecatedAppArmorAnnotationValueLocalhostPrefix + strings.Repeat("a", 4096),
2593+
},
2594+
},
2595+
Spec: api.PodSpec{
2596+
Containers: []api.Container{{Name: "ctr"}},
2597+
},
2598+
},
2599+
validation: func(t *testing.T, pod *api.Pod) {
2600+
assert.Contains(t, pod.Annotations, api.DeprecatedAppArmorAnnotationKeyPrefix+"ctr")
2601+
assert.Nil(t, pod.Spec.Containers[0].SecurityContext)
2602+
assert.Nil(t, pod.Spec.SecurityContext)
2603+
},
25862604
}, {
25872605
description: "Invalid field type",
25882606
pod: &api.Pod{

0 commit comments

Comments
 (0)