Input Validation / Denial of Service prevention

Proof of Concept: - Affected input: A ResourceSlice that includes a DeviceAttribute with StringValues containing a single string exceeding the per-item maxBytes, e.g., a string composed of 33 copies of the 'é' character. Since 'é' is 2 bytes in UTF-8, 33 copies total 66 bytes, which exceeds the maxBytes (64). - Validation path: The per-item validation now enforces maxBytes and annotates the error with origin 'maxBytes'. Prior to this patch, such an input could pass validation and potentially lead to DoS via oversized strings. Reproduction outline (illustrative, exact type names depend on the compiled API): 1) Generate a long string exceeding 64 bytes in UTF-8, e.g.: 33 times 'é'. - In a shell: python3 -c 'print("é" * 33)' > long_str.txt 2) Create a ResourceSlice manifest that sets: spec: devices: - name: test-device attributes: test.io/list_of_strings: strings: - <contents of long_str.txt> 3) Apply the manifest in a cluster that includes the updated validation code. 4) Expect a validation error similar to: TooLongMaxLength (path ...).WithOrigin("maxBytes").MarkCoveredByDeclarative(), indicating the per-item maxBytes validation rejected the input. Note: The exact error path depends on the path construction in your test/harness, but the key indicators are a TooLong/MaxBytes error with origin set to maxBytes.

diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index 050e7a422c1a3..dc3a3f173e58f 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -1017,7 +1017,7 @@ func validateDeviceAttribute(attribute resource.DeviceAttribute, fldPath *field. allErrs = append(allErrs, field.Invalid(fldPath.Child("strings"), attribute.StringValues, "must not be empty if specified")) } for i, item := range attribute.StringValues { - allErrs = append(allErrs, validateDeviceAttributeStringValue(&item, fldPath.Child("strings").Index(i))...) + allErrs = append(allErrs, validateDeviceAttributeStringValue(&item, fldPath.Child("strings").Index(i)).WithOrigin("maxBytes").MarkCoveredByDeclarative()...) } } if attribute.VersionValues != nil { diff --git a/pkg/apis/resource/validation/validation_resourceslice_test.go b/pkg/apis/resource/validation/validation_resourceslice_test.go index e79e663fe67bf..78400a93d18fd 100644 --- a/pkg/apis/resource/validation/validation_resourceslice_test.go +++ b/pkg/apis/resource/validation/validation_resourceslice_test.go @@ -503,7 +503,7 @@ func TestValidateResourceSlice(t *testing.T) { field.Invalid(field.NewPath("spec", "devices").Index(0).Child("attributes").Key(goodName), badMultipleListValue, "exactly one value must be specified").WithOrigin("union").MarkCoveredByDeclarative(), field.Invalid(field.NewPath("spec", "devices").Index(1).Child("attributes").Key(goodName).Child("strings"), []string{}, "must not be empty if specified"), field.Invalid(field.NewPath("spec", "devices").Index(1).Child("attributes").Key(goodName), badMultipleListValueWithEmptyList, "exactly one value must be specified").WithOrigin("union").MarkCoveredByDeclarative(), - field.TooLongMaxLength(field.NewPath("spec", "devices").Index(2).Child("attributes").Key(goodName).Child("strings").Index(0), badListStringValueTooLong.StringValues[0], resourceapi.DeviceAttributeMaxValueLength), + field.TooLongMaxLength(field.NewPath("spec", "devices").Index(2).Child("attributes").Key(goodName).Child("strings").Index(0), badListStringValueTooLong.StringValues[0], resourceapi.DeviceAttributeMaxValueLength).WithOrigin("maxBytes").MarkCoveredByDeclarative(), field.Invalid(field.NewPath("spec", "devices").Index(3).Child("attributes").Key(goodName).Child("versions").Index(0), badListVersionValueTooLong.VersionValues[0], "must be a string compatible with semver.org spec 2.0.0"), field.TooLongMaxLength(field.NewPath("spec", "devices").Index(3).Child("attributes").Key(goodName).Child("versions").Index(0), badListVersionValueTooLong.VersionValues[0], resourceapi.DeviceAttributeMaxValueLength), field.Invalid(field.NewPath("spec", "devices").Index(4).Child("attributes").Key(goodName).Child("bools"), []bool{}, "must not be empty if specified"), diff --git a/pkg/registry/resource/resourceslice/declarative_validation_test.go b/pkg/registry/resource/resourceslice/declarative_validation_test.go index c5ee783ad5852..623758b8150e6 100644 --- a/pkg/registry/resource/resourceslice/declarative_validation_test.go +++ b/pkg/registry/resource/resourceslice/declarative_validation_test.go @@ -18,6 +18,7 @@ package resourceslice import ( "fmt" + "strings" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -119,6 +120,23 @@ func TestDeclarativeValidate(t *testing.T) { "valid: device attribute list of strings": { input: mkResourceSliceWithDevices(tweakDeviceAttribute("test.io/list_of_strings", resource.DeviceAttribute{StringValues: []string{"a", "b", "c"}})), }, + "valid: device attribute list of strings at max bytes": { + input: mkResourceSliceWithDevices(tweakDeviceAttribute("test.io/list_of_strings", resource.DeviceAttribute{ + // The tag literal is 64, which must stay aligned with DeviceAttributeMaxValueLength. + // "é" is two bytes in UTF-8, so repeating it max/2 times verifies the exact maxBytes boundary. + StringValues: []string{strings.Repeat("é", resource.DeviceAttributeMaxValueLength/2)}, + })), + }, + "invalid: device attribute list of strings with too-long item": { + input: mkResourceSliceWithDevices(tweakDeviceAttribute("test.io/list_of_strings", resource.DeviceAttribute{ + // The tag literal is 64, which must stay aligned with DeviceAttributeMaxValueLength. + // "é" is two bytes in UTF-8, so max/2+1 repeats exceeds the maxBytes boundary. + StringValues: []string{strings.Repeat("é", resource.DeviceAttributeMaxValueLength/2+1)}, + })), + expectedErrs: field.ErrorList{ + field.TooLong(field.NewPath("spec", "devices").Index(0).Child("attributes").Key("test.io/list_of_strings").Child("strings").Index(0), "", resource.DeviceAttributeMaxValueLength).WithOrigin("maxBytes").MarkAlpha(), + }, + }, "valid: device attribute list of versions": { input: mkResourceSliceWithDevices(tweakDeviceAttribute("test.io/list_of_versions", resource.DeviceAttribute{VersionValues: []string{"1.2.3", "2.3.4"}})), }, @@ -335,6 +353,25 @@ func TestDeclarativeValidateUpdate(t *testing.T) { field.Invalid(field.NewPath("spec", "devices").Index(0).Child("attributes").Key("test.io/empty"), "", "").WithOrigin("union").MarkAlpha(), }, }, + "valid update: device attribute list of strings at max bytes": { + old: mkResourceSliceWithDevices(), + update: mkResourceSliceWithDevices(tweakDeviceAttribute("test.io/list_of_strings", resource.DeviceAttribute{ + // The tag literal is 64, which must stay aligned with DeviceAttributeMaxValueLength. + // "é" is two bytes in UTF-8, so repeating it max/2 times verifies the exact maxBytes boundary. + StringValues: []string{strings.Repeat("é", resource.DeviceAttributeMaxValueLength/2)}, + })), + }, + "invalid update: device attribute list of strings with too-long item": { + old: mkResourceSliceWithDevices(), + update: mkResourceSliceWithDevices(tweakDeviceAttribute("test.io/list_of_strings", resource.DeviceAttribute{ + // The tag literal is 64, which must stay aligned with DeviceAttributeMaxValueLength. + // "é" is two bytes in UTF-8, so max/2+1 repeats exceeds the maxBytes boundary. + StringValues: []string{strings.Repeat("é", resource.DeviceAttributeMaxValueLength/2+1)}, + })), + expectedErrs: field.ErrorList{ + field.TooLong(field.NewPath("spec", "devices").Index(0).Child("attributes").Key("test.io/list_of_strings").Child("strings").Index(0), "", resource.DeviceAttributeMaxValueLength).WithOrigin("maxBytes").MarkAlpha(), + }, + }, // spec.sharedCounters "valid update: at limit shared counters": { old: mkResourceSliceWithSharedCounters(), diff --git a/staging/src/k8s.io/api/resource/v1/generated.proto b/staging/src/k8s.io/api/resource/v1/generated.proto index 8910b9eaac83f..4541c925a5cb1 100644 --- a/staging/src/k8s.io/api/resource/v1/generated.proto +++ b/staging/src/k8s.io/api/resource/v1/generated.proto @@ -612,6 +612,7 @@ message DeviceAttribute { // +k8s:listType=atomic // +k8s:alpha(since: "1.36")=+k8s:optional // +k8s:alpha(since: "1.36")=+k8s:unionMember + // +k8s:alpha(since: "1.37")=+k8s:eachVal=+k8s:maxBytes=64 // +featureGate=DRAListTypeAttributes repeated string strings = 8; diff --git a/staging/src/k8s.io/api/resource/v1/types.go b/staging/src/k8s.io/api/resource/v1/types.go index 71151e5fbfd7d..61e2b39bc677e 100644 --- a/staging/src/k8s.io/api/resource/v1/types.go +++ b/staging/src/k8s.io/api/resource/v1/types.go @@ -755,6 +755,7 @@ type DeviceAttribute struct { // +k8s:listType=atomic // +k8s:alpha(since: "1.36")=+k8s:optional // +k8s:alpha(since: "1.36")=+k8s:unionMember + // +k8s:alpha(since: "1.37")=+k8s:eachVal=+k8s:maxBytes=64 // +featureGate=DRAListTypeAttributes StringValues []string `json:"strings,omitempty" protobuf:"bytes,8,opt,name=strings"` diff --git a/staging/src/k8s.io/api/resource/v1beta1/generated.proto b/staging/src/k8s.io/api/resource/v1beta1/generated.proto index aa493c641943e..186cd19b3619f 100644 --- a/staging/src/k8s.io/api/resource/v1beta1/generated.proto +++ b/staging/src/k8s.io/api/resource/v1beta1/generated.proto @@ -618,6 +618,7 @@ message DeviceAttribute { // +listType=atomic // +k8s:alpha(since: "1.36")=+k8s:optional // +k8s:alpha(since: "1.36")=+k8s:unionMember + // +k8s:alpha(since: "1.37")=+k8s:eachVal=+k8s:maxBytes=64 // +featureGate=DRAListTypeAttributes repeated string strings = 8; diff --git a/staging/src/k8s.io/api/resource/v1beta1/types.go b/staging/src/k8s.io/api/resource/v1beta1/types.go index 4e05f07dc3529..00a004b4f7f93 100644 --- a/staging/src/k8s.io/api/resource/v1beta1/types.go +++ b/staging/src/k8s.io/api/resource/v1beta1/types.go @@ -746,6 +746,7 @@ type DeviceAttribute struct { // +listType=atomic // +k8s:alpha(since: "1.36")=+k8s:optional // +k8s:alpha(since: "1.36")=+k8s:unionMember + // +k8s:alpha(since: "1.37")=+k8s:eachVal=+k8s:maxBytes=64 // +featureGate=DRAListTypeAttributes StringValues []string `json:"strings,omitempty" protobuf:"bytes,8,opt,name=strings"` diff --git a/staging/src/k8s.io/api/resource/v1beta2/generated.proto b/staging/src/k8s.io/api/resource/v1beta2/generated.proto index 2c2f677f80bbd..25170f2a70e72 100644 --- a/staging/src/k8s.io/api/resource/v1beta2/generated.proto +++ b/staging/src/k8s.io/api/resource/v1beta2/generated.proto @@ -612,6 +612,7 @@ message DeviceAttribute { // +k8s:listType=atomic // +k8s:alpha(since: "1.36")=+k8s:optional // +k8s:alpha(since: "1.36")=+k8s:unionMember + // +k8s:alpha(since: "1.37")=+k8s:eachVal=+k8s:maxBytes=64 // +featureGate=DRAListTypeAttributes repeated string strings = 8; diff --git a/staging/src/k8s.io/api/resource/v1beta2/types.go b/staging/src/k8s.io/api/resource/v1beta2/types.go index d53807d80084c..fd672427abca6 100644 --- a/staging/src/k8s.io/api/resource/v1beta2/types.go +++ b/staging/src/k8s.io/api/resource/v1beta2/types.go @@ -740,6 +740,7 @@ type DeviceAttribute struct { // +k8s:listType=atomic // +k8s:alpha(since: "1.36")=+k8s:optional // +k8s:alpha(since: "1.36")=+k8s:unionMember + // +k8s:alpha(since: "1.37")=+k8s:eachVal=+k8s:maxBytes=64 // +featureGate=DRAListTypeAttributes StringValues []string `json:"strings,omitempty" protobuf:"bytes,8,opt,name=strings"`