From 8ef4566cefebf49f9a806a36df2105c9149785a1 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Fri, 27 Sep 2019 16:36:48 -0400 Subject: [PATCH] Limit YAML/JSON decode size --- .../app/options/options_test.go | 4 +- .../pkg/apiserver/apiserver.go | 1 + .../pkg/apiserver/customresource_handler.go | 10 +- .../test/integration/BUILD | 1 + .../test/integration/limit_test.go | 216 ++++++++++ .../pkg/runtime/serializer/json/BUILD | 2 + .../pkg/runtime/serializer/json/json.go | 20 + .../serializer/json/json_limit_test.go | 170 ++++++++ .../pkg/runtime/serializer/yaml/BUILD | 16 +- .../pkg/runtime/serializer/yaml/yaml_test.go | 402 ++++++++++++++++++ .../k8s.io/apimachinery/pkg/util/json/json.go | 28 +- .../handlers/fieldmanager/fieldmanager.go | 4 +- .../apiserver/pkg/endpoints/handlers/patch.go | 18 + .../src/k8s.io/apiserver/pkg/server/config.go | 20 +- .../apiserver/max_request_body_bytes_test.go | 170 +++++++- .../master/synthetic_master_test.go | 28 +- 16 files changed, 1063 insertions(+), 47 deletions(-) create mode 100644 staging/src/k8s.io/apiextensions-apiserver/test/integration/limit_test.go create mode 100644 staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json_limit_test.go create mode 100644 staging/src/k8s.io/apimachinery/pkg/runtime/serializer/yaml/yaml_test.go diff --git a/cmd/kube-apiserver/app/options/options_test.go b/cmd/kube-apiserver/app/options/options_test.go index 2c8c2df5d4ef..97a2cdf7fe7f 100644 --- a/cmd/kube-apiserver/app/options/options_test.go +++ b/cmd/kube-apiserver/app/options/options_test.go @@ -130,8 +130,8 @@ func TestAddFlags(t *testing.T) { MaxMutatingRequestsInFlight: 200, RequestTimeout: time.Duration(2) * time.Minute, MinRequestTimeout: 1800, - JSONPatchMaxCopyBytes: int64(100 * 1024 * 1024), - MaxRequestBodyBytes: int64(100 * 1024 * 1024), + JSONPatchMaxCopyBytes: int64(3 * 1024 * 1024), + MaxRequestBodyBytes: int64(3 * 1024 * 1024), }, Admission: &kubeoptions.AdmissionOptions{ GenericAdmission: &apiserveroptions.AdmissionOptions{ diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go index e207ba65863c..89b9060e1b7c 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/apiserver.go @@ -201,6 +201,7 @@ func (c completedConfig) New(delegationTarget genericapiserver.DelegationTarget) c.GenericConfig.RequestTimeout, time.Duration(c.GenericConfig.MinRequestTimeout)*time.Second, apiGroupInfo.StaticOpenAPISpec, + c.GenericConfig.MaxRequestBodyBytes, ) if err != nil { return nil, err diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index 204143b4f175..2d4bfdeb1c51 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -125,6 +125,10 @@ type crdHandler struct { // purpose of managing fields, it is how CR handlers get the structure // of TypeMeta and ObjectMeta staticOpenAPISpec *spec.Swagger + + // The limit on the request size that would be accepted and decoded in a write request + // 0 means no limit. + maxRequestBodyBytes int64 } // crdInfo stores enough information to serve the storage for the custom resource @@ -169,7 +173,8 @@ func NewCustomResourceDefinitionHandler( authorizer authorizer.Authorizer, requestTimeout time.Duration, minRequestTimeout time.Duration, - staticOpenAPISpec *spec.Swagger) (*crdHandler, error) { + staticOpenAPISpec *spec.Swagger, + maxRequestBodyBytes int64) (*crdHandler, error) { ret := &crdHandler{ versionDiscoveryHandler: versionDiscoveryHandler, groupDiscoveryHandler: groupDiscoveryHandler, @@ -185,6 +190,7 @@ func NewCustomResourceDefinitionHandler( requestTimeout: requestTimeout, minRequestTimeout: minRequestTimeout, staticOpenAPISpec: staticOpenAPISpec, + maxRequestBodyBytes: maxRequestBodyBytes, } crdInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: ret.createCustomResourceDefinition, @@ -812,6 +818,8 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd TableConvertor: storages[v.Name].CustomResource, Authorizer: r.authorizer, + + MaxRequestBodyBytes: r.maxRequestBodyBytes, } if utilfeature.DefaultFeatureGate.Enabled(features.ServerSideApply) { reqScope := *requestScopes[v.Name] diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD b/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD index bff34280ce3b..1d074466fdba 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/BUILD @@ -15,6 +15,7 @@ go_test( "change_test.go", "defaulting_test.go", "finalization_test.go", + "limit_test.go", "objectmeta_test.go", "pruning_test.go", "registration_test.go", diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/limit_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/limit_test.go new file mode 100644 index 000000000000..f7b687ee1230 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/limit_test.go @@ -0,0 +1,216 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package integration + +import ( + "fmt" + "strings" + "testing" + + "k8s.io/client-go/dynamic" + + apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" + "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + "k8s.io/apiextensions-apiserver/test/integration/fixtures" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" +) + +func TestLimits(t *testing.T) { + tearDown, config, _, err := fixtures.StartDefaultServer(t) + if err != nil { + t.Fatal(err) + } + defer tearDown() + + apiExtensionClient, err := clientset.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + dynamicClient, err := dynamic.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + + noxuDefinition := fixtures.NewNoxuCustomResourceDefinition(apiextensionsv1beta1.ClusterScoped) + noxuDefinition, err = fixtures.CreateNewCustomResourceDefinition(noxuDefinition, apiExtensionClient, dynamicClient) + if err != nil { + t.Fatal(err) + } + + kind := noxuDefinition.Spec.Names.Kind + apiVersion := noxuDefinition.Spec.Group + "/" + noxuDefinition.Spec.Version + + rest := apiExtensionClient.Discovery().RESTClient() + + // Create YAML over 3MB limit + t.Run("create YAML over limit", func(t *testing.T) { + yamlBody := []byte(fmt.Sprintf(` +apiVersion: %s +kind: %s +metadata: + name: test +values: `+strings.Repeat("[", 3*1024*1024), apiVersion, kind)) + + _, err := rest.Post(). + SetHeader("Accept", "application/yaml"). + SetHeader("Content-Type", "application/yaml"). + AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural). + Body(yamlBody). + DoRaw() + if !apierrors.IsRequestEntityTooLargeError(err) { + t.Errorf("expected too large error, got %v", err) + } + }) + + // Create YAML just under 3MB limit, nested + t.Run("create YAML doc under limit, nested", func(t *testing.T) { + yamlBody := []byte(fmt.Sprintf(` + apiVersion: %s + kind: %s + metadata: + name: test + values: `+strings.Repeat("[", 3*1024*1024/2-500)+strings.Repeat("]", 3*1024*1024/2-500), apiVersion, kind)) + + _, err := rest.Post(). + SetHeader("Accept", "application/yaml"). + SetHeader("Content-Type", "application/yaml"). + AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural). + Body(yamlBody). + DoRaw() + if !apierrors.IsBadRequest(err) { + t.Errorf("expected bad request, got %v", err) + } + }) + + // Create YAML just under 3MB limit, not nested + t.Run("create YAML doc under limit, not nested", func(t *testing.T) { + yamlBody := []byte(fmt.Sprintf(` + apiVersion: %s + kind: %s + metadata: + name: test + values: `+strings.Repeat("[", 3*1024*1024-1000), apiVersion, kind)) + + _, err := rest.Post(). + SetHeader("Accept", "application/yaml"). + SetHeader("Content-Type", "application/yaml"). + AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural). + Body(yamlBody). + DoRaw() + if !apierrors.IsBadRequest(err) { + t.Errorf("expected bad request, got %v", err) + } + }) + + // Create JSON over 3MB limit + t.Run("create JSON over limit", func(t *testing.T) { + jsonBody := []byte(fmt.Sprintf(`{ + "apiVersion": %q, + "kind": %q, + "metadata": { + "name": "test" + }, + "values": `+strings.Repeat("[", 3*1024*1024/2)+strings.Repeat("]", 3*1024*1024/2)+"}", apiVersion, kind)) + + _, err := rest.Post(). + SetHeader("Accept", "application/json"). + SetHeader("Content-Type", "application/json"). + AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural). + Body(jsonBody). + DoRaw() + if !apierrors.IsRequestEntityTooLargeError(err) { + t.Errorf("expected too large error, got %v", err) + } + }) + + // Create JSON just under 3MB limit, nested + t.Run("create JSON doc under limit, nested", func(t *testing.T) { + jsonBody := []byte(fmt.Sprintf(`{ + "apiVersion": %q, + "kind": %q, + "metadata": { + "name": "test" + }, + "values": `+strings.Repeat("[", 3*1024*1024/2-500)+strings.Repeat("]", 3*1024*1024/2-500)+"}", apiVersion, kind)) + + _, err := rest.Post(). + SetHeader("Accept", "application/json"). + SetHeader("Content-Type", "application/json"). + AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural). + Body(jsonBody). + DoRaw() + if !apierrors.IsBadRequest(err) { + t.Errorf("expected bad request, got %v", err) + } + }) + + // Create JSON just under 3MB limit, not nested + t.Run("create JSON doc under limit, not nested", func(t *testing.T) { + jsonBody := []byte(fmt.Sprintf(`{ + "apiVersion": %q, + "kind": %q, + "metadata": { + "name": "test" + }, + "values": `+strings.Repeat("[", 3*1024*1024-1000)+"}", apiVersion, kind)) + + _, err := rest.Post(). + SetHeader("Accept", "application/json"). + SetHeader("Content-Type", "application/json"). + AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural). + Body(jsonBody). + DoRaw() + if !apierrors.IsBadRequest(err) { + t.Errorf("expected bad request, got %v", err) + } + }) + + // Create instance to allow patching + { + jsonBody := []byte(fmt.Sprintf(`{"apiVersion": %q, "kind": %q, "metadata": {"name": "test"}}`, apiVersion, kind)) + _, err := rest.Post().AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural).Body(jsonBody).DoRaw() + if err != nil { + t.Fatalf("error creating object: %v", err) + } + } + + t.Run("JSONPatchType nested patch under limit", func(t *testing.T) { + patchBody := []byte(`[{"op":"add","path":"/foo","value":` + strings.Repeat("[", 3*1024*1024/2-100) + strings.Repeat("]", 3*1024*1024/2-100) + `}]`) + err = rest.Patch(types.JSONPatchType).AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural, "test"). + Body(patchBody).Do().Error() + if !apierrors.IsBadRequest(err) { + t.Errorf("expected success or bad request err, got %v", err) + } + }) + t.Run("MergePatchType nested patch under limit", func(t *testing.T) { + patchBody := []byte(`{"value":` + strings.Repeat("[", 3*1024*1024/2-100) + strings.Repeat("]", 3*1024*1024/2-100) + `}`) + err = rest.Patch(types.MergePatchType).AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural, "test"). + Body(patchBody).Do().Error() + if !apierrors.IsBadRequest(err) { + t.Errorf("expected success or bad request err, got %v", err) + } + }) + t.Run("ApplyPatchType nested patch under limit", func(t *testing.T) { + patchBody := []byte(`{"value":` + strings.Repeat("[", 3*1024*1024/2-100) + strings.Repeat("]", 3*1024*1024/2-100) + `}`) + err = rest.Patch(types.ApplyPatchType).Param("fieldManager", "test").AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Version, noxuDefinition.Spec.Names.Plural, "test"). + Body(patchBody).Do().Error() + if !apierrors.IsBadRequest(err) { + t.Errorf("expected bad request err, got %#v", err) + } + }) +} diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/BUILD b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/BUILD index 8ff68498d0a4..3f746100b776 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/BUILD @@ -9,6 +9,7 @@ load( go_test( name = "go_default_test", srcs = [ + "json_limit_test.go", "json_test.go", "meta_test.go", ], @@ -17,6 +18,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/json:go_default_library", ], ) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go index 69ada8ecf9c5..de1a7d677f88 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go @@ -122,7 +122,27 @@ func (customNumberDecoder) Decode(ptr unsafe.Pointer, iter *jsoniter.Iterator) { } iter.ReportError("DecodeNumber", err.Error()) default: + // init depth, if needed + if iter.Attachment == nil { + iter.Attachment = int(1) + } + + // remember current depth + originalAttachment := iter.Attachment + + // increment depth before descending + if i, ok := iter.Attachment.(int); ok { + iter.Attachment = i + 1 + if i > 10000 { + iter.ReportError("parse", "exceeded max depth") + return + } + } + *(*interface{})(ptr) = iter.Read() + + // restore current depth + iter.Attachment = originalAttachment } } diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json_limit_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json_limit_test.go new file mode 100644 index 000000000000..0414ec8a7b21 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json_limit_test.go @@ -0,0 +1,170 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package json + +import ( + gojson "encoding/json" + "strings" + "testing" + + utiljson "k8s.io/apimachinery/pkg/util/json" +) + +type testcase struct { + name string + data []byte + checkErr func(t testing.TB, err error) + + benchmark bool +} + +func testcases() []testcase { + // verify we got an error of some kind + nonNilError := func(t testing.TB, err error) { + if err == nil { + t.Errorf("expected error, got none") + } + } + // verify the parse completed, either with success or a max depth error + successOrMaxDepthError := func(t testing.TB, err error) { + if err != nil && !strings.Contains(err.Error(), "max depth") { + t.Errorf("expected success or error containing 'max depth', got: %v", err) + } + } + + return []testcase{ + { + name: "3MB of deeply nested slices", + checkErr: successOrMaxDepthError, + data: []byte(`{"a":` + strings.Repeat(`[`, 3*1024*1024/2) + strings.Repeat(`]`, 3*1024*1024/2) + "}"), + }, + { + name: "3MB of unbalanced nested slices", + checkErr: nonNilError, + data: []byte(`{"a":` + strings.Repeat(`[`, 3*1024*1024)), + }, + { + name: "3MB of deeply nested maps", + checkErr: successOrMaxDepthError, + data: []byte(strings.Repeat(`{"":`, 3*1024*1024/5/2) + "{}" + strings.Repeat(`}`, 3*1024*1024/5/2)), + }, + { + name: "3MB of unbalanced nested maps", + checkErr: nonNilError, + data: []byte(strings.Repeat(`{"":`, 3*1024*1024/5)), + }, + { + name: "3MB of empty slices", + data: []byte(`{"a":[` + strings.Repeat(`[],`, 3*1024*1024/3-2) + `[]]}`), + benchmark: true, + }, + { + name: "3MB of slices", + data: []byte(`{"a":[` + strings.Repeat(`[0],`, 3*1024*1024/4-2) + `[0]]}`), + benchmark: true, + }, + { + name: "3MB of empty maps", + data: []byte(`{"a":[` + strings.Repeat(`{},`, 3*1024*1024/3-2) + `{}]}`), + benchmark: true, + }, + { + name: "3MB of maps", + data: []byte(`{"a":[` + strings.Repeat(`{"a":0},`, 3*1024*1024/8-2) + `{"a":0}]}`), + benchmark: true, + }, + { + name: "3MB of ints", + data: []byte(`{"a":[` + strings.Repeat(`0,`, 3*1024*1024/2-2) + `0]}`), + benchmark: true, + }, + { + name: "3MB of floats", + data: []byte(`{"a":[` + strings.Repeat(`0.0,`, 3*1024*1024/4-2) + `0.0]}`), + benchmark: true, + }, + { + name: "3MB of bools", + data: []byte(`{"a":[` + strings.Repeat(`true,`, 3*1024*1024/5-2) + `true]}`), + benchmark: true, + }, + { + name: "3MB of empty strings", + data: []byte(`{"a":[` + strings.Repeat(`"",`, 3*1024*1024/3-2) + `""]}`), + benchmark: true, + }, + { + name: "3MB of strings", + data: []byte(`{"a":[` + strings.Repeat(`"abcdefghijklmnopqrstuvwxyz012",`, 3*1024*1024/30-2) + `"abcdefghijklmnopqrstuvwxyz012"]}`), + benchmark: true, + }, + { + name: "3MB of nulls", + data: []byte(`{"a":[` + strings.Repeat(`null,`, 3*1024*1024/5-2) + `null]}`), + benchmark: true, + }, + } +} + +var decoders = map[string]func([]byte, interface{}) error{ + "gojson": gojson.Unmarshal, + "utiljson": utiljson.Unmarshal, + "jsoniter": CaseSensitiveJsonIterator().Unmarshal, +} + +func TestJSONLimits(t *testing.T) { + for _, tc := range testcases() { + if tc.benchmark { + continue + } + t.Run(tc.name, func(t *testing.T) { + for decoderName, decoder := range decoders { + t.Run(decoderName, func(t *testing.T) { + v := map[string]interface{}{} + err := decoder(tc.data, &v) + + if tc.checkErr != nil { + tc.checkErr(t, err) + } else if err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } + }) + } +} + +func BenchmarkJSONLimits(b *testing.B) { + for _, tc := range testcases() { + b.Run(tc.name, func(b *testing.B) { + for decoderName, decoder := range decoders { + b.Run(decoderName, func(b *testing.B) { + for i := 0; i < b.N; i++ { + v := map[string]interface{}{} + err := decoder(tc.data, &v) + + if tc.checkErr != nil { + tc.checkErr(b, err) + } else if err != nil { + b.Errorf("unexpected error: %v", err) + } + } + }) + } + }) + } +} diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/yaml/BUILD b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/yaml/BUILD index 83712ab41b9e..7ba8c288ceb0 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/yaml/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/yaml/BUILD @@ -1,9 +1,6 @@ package(default_visibility = ["//visibility:public"]) -load( - "@io_bazel_rules_go//go:def.bzl", - "go_library", -) +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -29,3 +26,14 @@ filegroup( srcs = [":package-srcs"], tags = ["automanaged"], ) + +go_test( + name = "go_default_test", + srcs = ["yaml_test.go"], + data = glob(["testdata/**"]), + embed = [":go_default_library"], + deps = [ + "//staging/src/k8s.io/apimachinery/pkg/util/yaml:go_default_library", + "//vendor/sigs.k8s.io/yaml:go_default_library", + ], +) diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/yaml/yaml_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/yaml/yaml_test.go new file mode 100644 index 000000000000..0a0628c8724a --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/yaml/yaml_test.go @@ -0,0 +1,402 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package yaml + +import ( + "strings" + "testing" + + "k8s.io/apimachinery/pkg/util/yaml" + sigsyaml "sigs.k8s.io/yaml" +) + +type testcase struct { + name string + data []byte + error string + + benchmark bool +} + +func testcases() []testcase { + return []testcase{ + { + name: "arrays of string aliases", + error: "excessive aliasing", + data: []byte(` +apiVersion: v1 +data: +a: &a ["webwebwebwebwebweb","webwebwebwebwebweb","webwebwebwebwebweb","webwebwebwebwebweb","webwebwebwebwebweb","webwebwebwebwebweb","webwebwebwebwebweb","webwebwebwebwebweb","webwebwebwebwebweb"] +b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a] +c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b] +d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c] +e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d] +f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e] +g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f] +h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g] +i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h] +kind: ConfigMap +metadata: +name: yaml-bomb +namespace: default +`), + }, + { + name: "arrays of empty string aliases", + error: "excessive aliasing", + data: []byte(` +apiVersion: v1 +data: +a: &a ["","","","","","","","",""] +b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a] +c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b] +d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c] +e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d] +f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e] +g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f] +h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g] +i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h] +kind: ConfigMap +metadata: +name: yaml-bomb +namespace: default +`), + }, + { + name: "arrays of null aliases", + error: "excessive aliasing", + data: []byte(` +apiVersion: v1 +data: +a: &a [null,null,null,null,null,null,null,null,null] +b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a] +c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b] +d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c] +e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d] +f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e] +g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f] +h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g] +i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h] +kind: ConfigMap +metadata: +name: yaml-bomb +namespace: default +`), + }, + { + name: "arrays of zero int aliases", + error: "excessive aliasing", + data: []byte(` +apiVersion: v1 +data: +a: &a [0,0,0,0,0,0,0,0,0] +b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a] +c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b] +d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c] +e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d] +f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e] +g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f] +h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g] +i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h] +kind: ConfigMap +metadata: +name: yaml-bomb +namespace: default +`), + }, + { + name: "arrays of zero float aliases", + error: "excessive aliasing", + data: []byte(` +apiVersion: v1 +data: +a: &a [0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0,0.0] +b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a] +c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b] +d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c] +e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d] +f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e] +g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f] +h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g] +i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h] +kind: ConfigMap +metadata: +name: yaml-bomb +namespace: default +`), + }, + { + name: "arrays of big float aliases", + error: "excessive aliasing", + data: []byte(` +apiVersion: v1 +data: +a: &a [1234567890.12345678,1234567890.12345678,1234567890.12345678,1234567890.12345678,1234567890.12345678,1234567890.12345678,1234567890.12345678,1234567890.12345678,1234567890.12345678] +b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a] +c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b] +d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c] +e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d] +f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e] +g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f] +h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g] +i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h] +kind: ConfigMap +metadata: +name: yaml-bomb +namespace: default +`), + }, + { + name: "arrays of bool aliases", + error: "excessive aliasing", + data: []byte(` +apiVersion: v1 +data: +a: &a [true,true,true,true,true,true,true,true,true] +b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a,*a] +c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b,*b] +d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c,*c] +e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d,*d] +f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e,*e] +g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f,*f] +h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g,*g] +i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h,*h] +kind: ConfigMap +metadata: +name: yaml-bomb +namespace: default +`), + }, + { + name: "map key aliases", + error: "excessive aliasing", + data: []byte(` +apiVersion: v1 +data: +a: &a {"verylongkey1":"","verylongkey2":"","verylongkey3":"","verylongkey4":"","verylongkey5":"","verylongkey6":"","verylongkey7":"","verylongkey8":"","verylongkey9":""} +b: &b {"verylongkey1":*a,"verylongkey2":*a,"verylongkey3":*a,"verylongkey4":*a,"verylongkey5":*a,"verylongkey6":*a,"verylongkey7":*a,"verylongkey8":*a,"verylongkey9":*a} +c: &c {"verylongkey1":*b,"verylongkey2":*b,"verylongkey3":*b,"verylongkey4":*b,"verylongkey5":*b,"verylongkey6":*b,"verylongkey7":*b,"verylongkey8":*b,"verylongkey9":*b} +d: &d {"verylongkey1":*c,"verylongkey2":*c,"verylongkey3":*c,"verylongkey4":*c,"verylongkey5":*c,"verylongkey6":*c,"verylongkey7":*c,"verylongkey8":*c,"verylongkey9":*c} +e: &e {"verylongkey1":*d,"verylongkey2":*d,"verylongkey3":*d,"verylongkey4":*d,"verylongkey5":*d,"verylongkey6":*d,"verylongkey7":*d,"verylongkey8":*d,"verylongkey9":*d} +f: &f {"verylongkey1":*e,"verylongkey2":*e,"verylongkey3":*e,"verylongkey4":*e,"verylongkey5":*e,"verylongkey6":*e,"verylongkey7":*e,"verylongkey8":*e,"verylongkey9":*e} +g: &g {"verylongkey1":*f,"verylongkey2":*f,"verylongkey3":*f,"verylongkey4":*f,"verylongkey5":*f,"verylongkey6":*f,"verylongkey7":*f,"verylongkey8":*f,"verylongkey9":*f} +h: &h {"verylongkey1":*g,"verylongkey2":*g,"verylongkey3":*g,"verylongkey4":*g,"verylongkey5":*g,"verylongkey6":*g,"verylongkey7":*g,"verylongkey8":*g,"verylongkey9":*g} +i: &i {"verylongkey1":*h,"verylongkey2":*h,"verylongkey3":*h,"verylongkey4":*h,"verylongkey5":*h,"verylongkey6":*h,"verylongkey7":*h,"verylongkey8":*h,"verylongkey9":*h} +kind: ConfigMap +metadata: +name: yaml-bomb +namespace: default +`), + }, + { + name: "map value aliases", + error: "excessive aliasing", + data: []byte(` +apiVersion: v1 +data: +a: &a {"1":"verylongmapvalue","2":"verylongmapvalue","3":"verylongmapvalue","4":"verylongmapvalue","5":"verylongmapvalue","6":"verylongmapvalue","7":"verylongmapvalue","8":"verylongmapvalue","9":"verylongmapvalue"} +b: &b {"1":*a,"2":*a,"3":*a,"4":*a,"5":*a,"6":*a,"7":*a,"8":*a,"9":*a} +c: &c {"1":*b,"2":*b,"3":*b,"4":*b,"5":*b,"6":*b,"7":*b,"8":*b,"9":*b} +d: &d {"1":*c,"2":*c,"3":*c,"4":*c,"5":*c,"6":*c,"7":*c,"8":*c,"9":*c} +e: &e {"1":*d,"2":*d,"3":*d,"4":*d,"5":*d,"6":*d,"7":*d,"8":*d,"9":*d} +f: &f {"1":*e,"2":*e,"3":*e,"4":*e,"5":*e,"6":*e,"7":*e,"8":*e,"9":*e} +g: &g {"1":*f,"2":*f,"3":*f,"4":*f,"5":*f,"6":*f,"7":*f,"8":*f,"9":*f} +h: &h {"1":*g,"2":*g,"3":*g,"4":*g,"5":*g,"6":*g,"7":*g,"8":*g,"9":*g} +i: &i {"1":*h,"2":*h,"3":*h,"4":*h,"5":*h,"6":*h,"7":*h,"8":*h,"9":*h} +kind: ConfigMap +metadata: +name: yaml-bomb +namespace: default +`), + }, + { + name: "nested map aliases", + error: "excessive aliasing", + data: []byte(` +apiVersion: v1 +data: +a: &a {"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{"":{}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}} +b: &b {"1":*a,"2":*a,"3":*a,"4":*a,"5":*a,"6":*a,"7":*a,"8":*a,"9":*a} +c: &c {"1":*b,"2":*b,"3":*b,"4":*b,"5":*b,"6":*b,"7":*b,"8":*b,"9":*b} +d: &d {"1":*c,"2":*c,"3":*c,"4":*c,"5":*c,"6":*c,"7":*c,"8":*c,"9":*c} +e: &e {"1":*d,"2":*d,"3":*d,"4":*d,"5":*d,"6":*d,"7":*d,"8":*d,"9":*d} +f: &f {"1":*e,"2":*e,"3":*e,"4":*e,"5":*e,"6":*e,"7":*e,"8":*e,"9":*e} +g: &g {"1":*f,"2":*f,"3":*f,"4":*f,"5":*f,"6":*f,"7":*f,"8":*f,"9":*f} +h: &h {"1":*g,"2":*g,"3":*g,"4":*g,"5":*g,"6":*g,"7":*g,"8":*g,"9":*g} +i: &i {"1":*h,"2":*h,"3":*h,"4":*h,"5":*h,"6":*h,"7":*h,"8":*h,"9":*h} +kind: ConfigMap +metadata: +name: yaml-bomb +namespace: default +`), + }, + { + name: "nested slice aliases", + error: "excessive aliasing", + data: []byte(` +apiVersion: v1 +data: +a: &a [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[""]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]] +b: &b [[[[[[[[[[*a]]]]]]]]],[[[[[[[[[*a]]]]]]]]],[[[[[[[[[*a]]]]]]]]],[[[[[[[[[*a]]]]]]]]],[[[[[[[[[*a]]]]]]]]]] +c: &c [[[[[[[[[[*b]]]]]]]]],[[[[[[[[[*b]]]]]]]]],[[[[[[[[[*b]]]]]]]]],[[[[[[[[[*b]]]]]]]]],[[[[[[[[[*b]]]]]]]]]] +d: &d [[[[[[[[[[*c]]]]]]]]],[[[[[[[[[*c]]]]]]]]],[[[[[[[[[*c]]]]]]]]],[[[[[[[[[*c]]]]]]]]],[[[[[[[[[*c]]]]]]]]]] +e: &e [[[[[[[[[[*d]]]]]]]]],[[[[[[[[[*d]]]]]]]]],[[[[[[[[[*d]]]]]]]]],[[[[[[[[[*d]]]]]]]]],[[[[[[[[[*d]]]]]]]]]] +f: &f [[[[[[[[[[*e]]]]]]]]],[[[[[[[[[*e]]]]]]]]],[[[[[[[[[*e]]]]]]]]],[[[[[[[[[*e]]]]]]]]],[[[[[[[[[*e]]]]]]]]]] +g: &g [[[[[[[[[[*f]]]]]]]]],[[[[[[[[[*f]]]]]]]]],[[[[[[[[[*f]]]]]]]]],[[[[[[[[[*f]]]]]]]]],[[[[[[[[[*f]]]]]]]]]] +h: &h [[[[[[[[[[*g]]]]]]]]],[[[[[[[[[*g]]]]]]]]],[[[[[[[[[*g]]]]]]]]],[[[[[[[[[*g]]]]]]]]],[[[[[[[[[*g]]]]]]]]]] +i: &i [[[[[[[[[[*h]]]]]]]]],[[[[[[[[[*h]]]]]]]]],[[[[[[[[[*h]]]]]]]]],[[[[[[[[[*h]]]]]]]]],[[[[[[[[[*h]]]]]]]]]] +kind: ConfigMap +metadata: +name: yaml-bomb +namespace: default +`), + }, + { + name: "3MB map without alias", + data: []byte(`a: &a [{a}` + strings.Repeat(`,{a}`, 3*1024*1024/4) + `]`), + }, + { + name: "3MB map with alias", + error: "excessive aliasing", + data: []byte(` +a: &a [{a}` + strings.Repeat(`,{a}`, 3*1024*1024/4) + `] +b: &b [*a]`), + }, + { + name: "deeply nested slices", + error: "max depth", + data: []byte(strings.Repeat(`[`, 3*1024*1024)), + }, + { + name: "deeply nested maps", + error: "max depth", + data: []byte("x: " + strings.Repeat(`{`, 3*1024*1024)), + }, + { + name: "deeply nested indents", + error: "max depth", + data: []byte(strings.Repeat(`- `, 3*1024*1024)), + }, + { + name: "3MB of 1000-indent lines", + data: []byte(strings.Repeat(strings.Repeat(`- `, 1000)+"\n", 3*1024/2)), + benchmark: true, + }, + { + name: "3MB of empty slices", + data: []byte(`[` + strings.Repeat(`[],`, 3*1024*1024/3-2) + `[]]`), + benchmark: true, + }, + { + name: "3MB of slices", + data: []byte(`[` + strings.Repeat(`[0],`, 3*1024*1024/4-2) + `[0]]`), + benchmark: true, + }, + { + name: "3MB of empty maps", + data: []byte(`[` + strings.Repeat(`{},`, 3*1024*1024/3-2) + `{}]`), + benchmark: true, + }, + { + name: "3MB of maps", + data: []byte(`[` + strings.Repeat(`{a},`, 3*1024*1024/4-2) + `{a}]`), + benchmark: true, + }, + { + name: "3MB of ints", + data: []byte(`[` + strings.Repeat(`0,`, 3*1024*1024/2-2) + `0]`), + benchmark: true, + }, + { + name: "3MB of floats", + data: []byte(`[` + strings.Repeat(`0.0,`, 3*1024*1024/4-2) + `0.0]`), + benchmark: true, + }, + { + name: "3MB of bools", + data: []byte(`[` + strings.Repeat(`true,`, 3*1024*1024/5-2) + `true]`), + benchmark: true, + }, + { + name: "3MB of empty strings", + data: []byte(`[` + strings.Repeat(`"",`, 3*1024*1024/3-2) + `""]`), + benchmark: true, + }, + { + name: "3MB of strings", + data: []byte(`[` + strings.Repeat(`"abcdefghijklmnopqrstuvwxyz012",`, 3*1024*1024/30-2) + `"abcdefghijklmnopqrstuvwxyz012"]`), + benchmark: true, + }, + { + name: "3MB of nulls", + data: []byte(`[` + strings.Repeat(`null,`, 3*1024*1024/5-2) + `null]`), + benchmark: true, + }, + } +} + +var decoders = map[string]func([]byte) ([]byte, error){ + "sigsyaml": sigsyaml.YAMLToJSON, + "utilyaml": yaml.ToJSON, +} + +func TestYAMLLimits(t *testing.T) { + for _, tc := range testcases() { + if tc.benchmark { + continue + } + t.Run(tc.name, func(t *testing.T) { + for decoderName, decoder := range decoders { + t.Run(decoderName, func(t *testing.T) { + _, err := decoder(tc.data) + if len(tc.error) == 0 { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + } else { + if err == nil || !strings.Contains(err.Error(), tc.error) { + t.Errorf("expected %q error, got %v", tc.error, err) + } + } + }) + } + }) + } +} + +func BenchmarkYAMLLimits(b *testing.B) { + for _, tc := range testcases() { + b.Run(tc.name, func(b *testing.B) { + for decoderName, decoder := range decoders { + b.Run(decoderName, func(b *testing.B) { + for i := 0; i < b.N; i++ { + _, err := decoder(tc.data) + if len(tc.error) == 0 { + if err != nil { + b.Errorf("unexpected error: %v", err) + } + } else { + if err == nil || !strings.Contains(err.Error(), tc.error) { + b.Errorf("expected %q error, got %v", tc.error, err) + } + } + } + }) + } + }) + } +} diff --git a/staging/src/k8s.io/apimachinery/pkg/util/json/json.go b/staging/src/k8s.io/apimachinery/pkg/util/json/json.go index 10c8cb837ed5..0e2e30175476 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/json/json.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/json/json.go @@ -19,6 +19,7 @@ package json import ( "bytes" "encoding/json" + "fmt" "io" ) @@ -34,6 +35,9 @@ func Marshal(v interface{}) ([]byte, error) { return json.Marshal(v) } +// limit recursive depth to prevent stack overflow errors +const maxDepth = 10000 + // Unmarshal unmarshals the given data // If v is a *map[string]interface{}, numbers are converted to int64 or float64 func Unmarshal(data []byte, v interface{}) error { @@ -48,7 +52,7 @@ func Unmarshal(data []byte, v interface{}) error { return err } // If the decode succeeds, post-process the map to convert json.Number objects to int64 or float64 - return convertMapNumbers(*v) + return convertMapNumbers(*v, 0) case *[]interface{}: // Build a decoder from the given data @@ -60,7 +64,7 @@ func Unmarshal(data []byte, v interface{}) error { return err } // If the decode succeeds, post-process the map to convert json.Number objects to int64 or float64 - return convertSliceNumbers(*v) + return convertSliceNumbers(*v, 0) default: return json.Unmarshal(data, v) @@ -69,16 +73,20 @@ func Unmarshal(data []byte, v interface{}) error { // convertMapNumbers traverses the map, converting any json.Number values to int64 or float64. // values which are map[string]interface{} or []interface{} are recursively visited -func convertMapNumbers(m map[string]interface{}) error { +func convertMapNumbers(m map[string]interface{}, depth int) error { + if depth > maxDepth { + return fmt.Errorf("exceeded max depth of %d", maxDepth) + } + var err error for k, v := range m { switch v := v.(type) { case json.Number: m[k], err = convertNumber(v) case map[string]interface{}: - err = convertMapNumbers(v) + err = convertMapNumbers(v, depth+1) case []interface{}: - err = convertSliceNumbers(v) + err = convertSliceNumbers(v, depth+1) } if err != nil { return err @@ -89,16 +97,20 @@ func convertMapNumbers(m map[string]interface{}) error { // convertSliceNumbers traverses the slice, converting any json.Number values to int64 or float64. // values which are map[string]interface{} or []interface{} are recursively visited -func convertSliceNumbers(s []interface{}) error { +func convertSliceNumbers(s []interface{}, depth int) error { + if depth > maxDepth { + return fmt.Errorf("exceeded max depth of %d", maxDepth) + } + var err error for i, v := range s { switch v := v.(type) { case json.Number: s[i], err = convertNumber(v) case map[string]interface{}: - err = convertMapNumbers(v) + err = convertMapNumbers(v, depth+1) case []interface{}: - err = convertSliceNumbers(v) + err = convertSliceNumbers(v, depth+1) } if err != nil { return err diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go index 21f408813a06..9817418177cb 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/fieldmanager.go @@ -195,11 +195,11 @@ func (f *fieldManager) Apply(liveObj runtime.Object, patch []byte, fieldManager patchObj := &unstructured.Unstructured{Object: map[string]interface{}{}} if err := yaml.Unmarshal(patch, &patchObj.Object); err != nil { - return nil, fmt.Errorf("error decoding YAML: %v", err) + return nil, errors.NewBadRequest(fmt.Sprintf("error decoding YAML: %v", err)) } if patchObj.GetManagedFields() != nil { - return nil, fmt.Errorf("managed fields must be nil but was %v", patchObj.GetManagedFields()) + return nil, errors.NewBadRequest(fmt.Sprintf("metadata.managedFields must be nil")) } if patchObj.GetAPIVersion() != f.groupVersion.String() { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 8d589cc91136..029513f93ee5 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -337,6 +337,15 @@ func (p *jsonPatcher) createNewObject() (runtime.Object, error) { func (p *jsonPatcher) applyJSPatch(versionedJS []byte) (patchedJS []byte, retErr error) { switch p.patchType { case types.JSONPatchType: + // sanity check potentially abusive patches + // TODO(liggitt): drop this once golang json parser limits stack depth (https://github.com/golang/go/issues/31789) + if len(p.patchBytes) > 1024*1024 { + v := []interface{}{} + if err := json.Unmarshal(p.patchBytes, v); err != nil { + return nil, errors.NewBadRequest(fmt.Sprintf("error decoding patch: %v", err)) + } + } + patchObj, err := jsonpatch.DecodePatch(p.patchBytes) if err != nil { return nil, errors.NewBadRequest(err.Error()) @@ -352,6 +361,15 @@ func (p *jsonPatcher) applyJSPatch(versionedJS []byte) (patchedJS []byte, retErr } return patchedJS, nil case types.MergePatchType: + // sanity check potentially abusive patches + // TODO(liggitt): drop this once golang json parser limits stack depth (https://github.com/golang/go/issues/31789) + if len(p.patchBytes) > 1024*1024 { + v := map[string]interface{}{} + if err := json.Unmarshal(p.patchBytes, v); err != nil { + return nil, errors.NewBadRequest(fmt.Sprintf("error decoding patch: %v", err)) + } + } + return jsonpatch.MergePatch(versionedJS, p.patchBytes) default: // only here as a safety net - go-restful filters content-type diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index b75422a6e09e..998999e8ab46 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -180,7 +180,7 @@ type Config struct { // patch may cause. // This affects all places that applies json patch in the binary. JSONPatchMaxCopyBytes int64 - // The limit on the request body size that would be accepted and decoded in a write request. + // The limit on the request size that would be accepted and decoded in a write request // 0 means no limit. MaxRequestBodyBytes int64 // MaxRequestsInFlight is the maximum number of parallel non-long-running requests. Every further @@ -295,22 +295,20 @@ func NewConfig(codecs serializer.CodecFactory) *Config { MinRequestTimeout: 1800, LivezGracePeriod: time.Duration(0), ShutdownDelayDuration: time.Duration(0), - // 10MB is the recommended maximum client request size in bytes + // 1.5MB is the default client request size in bytes // the etcd server should accept. See - // https://github.com/etcd-io/etcd/blob/release-3.3/etcdserver/server.go#L90. + // https://github.com/etcd-io/etcd/blob/release-3.4/embed/config.go#L56. // A request body might be encoded in json, and is converted to - // proto when persisted in etcd. Assuming the upper bound of - // the size ratio is 10:1, we set 100MB as the largest size + // proto when persisted in etcd, so we allow 2x as the largest size // increase the "copy" operations in a json patch may cause. - JSONPatchMaxCopyBytes: int64(100 * 1024 * 1024), - // 10MB is the recommended maximum client request size in bytes + JSONPatchMaxCopyBytes: int64(3 * 1024 * 1024), + // 1.5MB is the recommended client request size in byte // the etcd server should accept. See - // https://github.com/etcd-io/etcd/blob/release-3.3/etcdserver/server.go#L90. + // https://github.com/etcd-io/etcd/blob/release-3.4/embed/config.go#L56. // A request body might be encoded in json, and is converted to - // proto when persisted in etcd. Assuming the upper bound of - // the size ratio is 10:1, we set 100MB as the largest request + // proto when persisted in etcd, so we allow 2x as the largest request // body size to be accepted and decoded in a write request. - MaxRequestBodyBytes: int64(100 * 1024 * 1024), + MaxRequestBodyBytes: int64(3 * 1024 * 1024), // Default to treating watch as a long-running operation // Generic API servers have no inherent long-running subresources diff --git a/test/integration/apiserver/max_request_body_bytes_test.go b/test/integration/apiserver/max_request_body_bytes_test.go index 05816d603638..d5c50b22120f 100644 --- a/test/integration/apiserver/max_request_body_bytes_test.go +++ b/test/integration/apiserver/max_request_body_bytes_test.go @@ -21,11 +21,11 @@ import ( "strings" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/kubernetes/cmd/kube-apiserver/app/options" "k8s.io/kubernetes/test/integration/framework" ) @@ -33,13 +33,11 @@ import ( func TestMaxResourceSize(t *testing.T) { stopCh := make(chan struct{}) defer close(stopCh) - clientSet, _ := framework.StartTestServer(t, stopCh, framework.TestServerSetup{ - ModifyServerRunOptions: func(opts *options.ServerRunOptions) { - opts.GenericServerRunOptions.MaxRequestBodyBytes = 1024 * 1024 - }, - }) + clientSet, _ := framework.StartTestServer(t, stopCh, framework.TestServerSetup{}) + + hugeData := []byte(strings.Repeat("x", 3*1024*1024+1)) - hugeData := []byte(strings.Repeat("x", 1024*1024+1)) + rest := clientSet.Discovery().RESTClient() c := clientSet.CoreV1().RESTClient() t.Run("Create should limit the request body size", func(t *testing.T) { @@ -87,6 +85,38 @@ func TestMaxResourceSize(t *testing.T) { } }) + t.Run("JSONPatchType should handle a patch just under the max limit", func(t *testing.T) { + patchBody := []byte(`[{"op":"add","path":"/foo","value":` + strings.Repeat("[", 3*1024*1024/2-100) + strings.Repeat("]", 3*1024*1024/2-100) + `}]`) + err = rest.Patch(types.JSONPatchType).AbsPath(fmt.Sprintf("/api/v1/namespaces/default/secrets/test")). + Body(patchBody).Do().Error() + if err != nil && !errors.IsBadRequest(err) { + t.Errorf("expected success or bad request err, got %v", err) + } + }) + t.Run("MergePatchType should handle a patch just under the max limit", func(t *testing.T) { + patchBody := []byte(`{"value":` + strings.Repeat("[", 3*1024*1024/2-100) + strings.Repeat("]", 3*1024*1024/2-100) + `}`) + err = rest.Patch(types.MergePatchType).AbsPath(fmt.Sprintf("/api/v1/namespaces/default/secrets/test")). + Body(patchBody).Do().Error() + if err != nil && !errors.IsBadRequest(err) { + t.Errorf("expected success or bad request err, got %v", err) + } + }) + t.Run("StrategicMergePatchType should handle a patch just under the max limit", func(t *testing.T) { + patchBody := []byte(`{"value":` + strings.Repeat("[", 3*1024*1024/2-100) + strings.Repeat("]", 3*1024*1024/2-100) + `}`) + err = rest.Patch(types.StrategicMergePatchType).AbsPath(fmt.Sprintf("/api/v1/namespaces/default/secrets/test")). + Body(patchBody).Do().Error() + if err != nil && !errors.IsBadRequest(err) { + t.Errorf("expected success or bad request err, got %v", err) + } + }) + t.Run("ApplyPatchType should handle a patch just under the max limit", func(t *testing.T) { + patchBody := []byte(`{"value":` + strings.Repeat("[", 3*1024*1024/2-100) + strings.Repeat("]", 3*1024*1024/2-100) + `}`) + err = rest.Patch(types.ApplyPatchType).Param("fieldManager", "test").AbsPath(fmt.Sprintf("/api/v1/namespaces/default/secrets/test")). + Body(patchBody).Do().Error() + if err != nil && !errors.IsBadRequest(err) { + t.Errorf("expected success or bad request err, got %#v", err) + } + }) t.Run("Delete should limit the request body size", func(t *testing.T) { err = c.Delete().AbsPath(fmt.Sprintf("/api/v1/namespaces/default/secrets/test")). Body(hugeData).Do().Error() @@ -98,4 +128,128 @@ func TestMaxResourceSize(t *testing.T) { } }) + + // Create YAML over 3MB limit + t.Run("create should limit yaml parsing", func(t *testing.T) { + yamlBody := []byte(` +apiVersion: v1 +kind: ConfigMap +metadata: + name: mytest +values: ` + strings.Repeat("[", 3*1024*1024)) + + _, err := rest.Post(). + SetHeader("Accept", "application/yaml"). + SetHeader("Content-Type", "application/yaml"). + AbsPath("/api/v1/namespaces/default/configmaps"). + Body(yamlBody). + DoRaw() + if !apierrors.IsRequestEntityTooLargeError(err) { + t.Errorf("expected too large error, got %v", err) + } + }) + + // Create YAML just under 3MB limit, nested + t.Run("create should handle a yaml document just under the maximum size with correct nesting", func(t *testing.T) { + yamlBody := []byte(` +apiVersion: v1 +kind: ConfigMap +metadata: + name: mytest +values: ` + strings.Repeat("[", 3*1024*1024/2-500) + strings.Repeat("]", 3*1024*1024/2-500)) + + _, err := rest.Post(). + SetHeader("Accept", "application/yaml"). + SetHeader("Content-Type", "application/yaml"). + AbsPath("/api/v1/namespaces/default/configmaps"). + Body(yamlBody). + DoRaw() + if !apierrors.IsBadRequest(err) { + t.Errorf("expected bad request, got %v", err) + } + }) + + // Create YAML just under 3MB limit, not nested + t.Run("create should handle a yaml document just under the maximum size with unbalanced nesting", func(t *testing.T) { + yamlBody := []byte(` +apiVersion: v1 +kind: ConfigMap +metadata: + name: mytest +values: ` + strings.Repeat("[", 3*1024*1024-1000)) + + _, err := rest.Post(). + SetHeader("Accept", "application/yaml"). + SetHeader("Content-Type", "application/yaml"). + AbsPath("/api/v1/namespaces/default/configmaps"). + Body(yamlBody). + DoRaw() + if !apierrors.IsBadRequest(err) { + t.Errorf("expected bad request, got %v", err) + } + }) + + // Create JSON over 3MB limit + t.Run("create should limit json parsing", func(t *testing.T) { + jsonBody := []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "mytest" + }, + "values": ` + strings.Repeat("[", 3*1024*1024/2) + strings.Repeat("]", 3*1024*1024/2) + "}") + + _, err := rest.Post(). + SetHeader("Accept", "application/json"). + SetHeader("Content-Type", "application/json"). + AbsPath("/api/v1/namespaces/default/configmaps"). + Body(jsonBody). + DoRaw() + if !apierrors.IsRequestEntityTooLargeError(err) { + t.Errorf("expected too large error, got %v", err) + } + }) + + // Create JSON just under 3MB limit, nested + t.Run("create should handle a json document just under the maximum size with correct nesting", func(t *testing.T) { + jsonBody := []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "mytest" + }, + "values": ` + strings.Repeat("[", 3*1024*1024/2-100) + strings.Repeat("]", 3*1024*1024/2-100) + "}") + + _, err := rest.Post(). + SetHeader("Accept", "application/json"). + SetHeader("Content-Type", "application/json"). + AbsPath("/api/v1/namespaces/default/configmaps"). + Body(jsonBody). + DoRaw() + // TODO(liggitt): expect bad request on deep nesting, rather than success on dropped unknown field data + if err != nil && !apierrors.IsBadRequest(err) { + t.Errorf("expected bad request, got %v", err) + } + }) + + // Create JSON just under 3MB limit, not nested + t.Run("create should handle a json document just under the maximum size with unbalanced nesting", func(t *testing.T) { + jsonBody := []byte(`{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "mytest" + }, + "values": ` + strings.Repeat("[", 3*1024*1024-1000) + "}") + + _, err := rest.Post(). + SetHeader("Accept", "application/json"). + SetHeader("Content-Type", "application/json"). + AbsPath("/api/v1/namespaces/default/configmaps"). + Body(jsonBody). + DoRaw() + if !apierrors.IsBadRequest(err) { + t.Errorf("expected bad request, got %v", err) + } + }) } diff --git a/test/integration/master/synthetic_master_test.go b/test/integration/master/synthetic_master_test.go index 6e19c29e745c..804296c07d65 100644 --- a/test/integration/master/synthetic_master_test.go +++ b/test/integration/master/synthetic_master_test.go @@ -307,30 +307,36 @@ func TestObjectSizeResponses(t *testing.T) { client := clientset.NewForConfigOrDie(&restclient.Config{Host: s.URL}) const DeploymentMegabyteSize = 100000 - const DeploymentTwoMegabyteSize = 1000000 + const DeploymentTwoMegabyteSize = 175000 + const DeploymentThreeMegabyteSize = 250000 expectedMsgFor1MB := `etcdserver: request is too large` expectedMsgFor2MB := `rpc error: code = ResourceExhausted desc = trying to send message larger than max` + expectedMsgFor3MB := `Request entity too large: limit is 3145728` expectedMsgForLargeAnnotation := `metadata.annotations: Too long: must have at most 262144 characters` - deployment1 := constructBody("a", DeploymentMegabyteSize, "labels", t) // >1 MB file - deployment2 := constructBody("a", DeploymentTwoMegabyteSize, "labels", t) // >2 MB file + deployment1 := constructBody("a", DeploymentMegabyteSize, "labels", t) // >1 MB file + deployment2 := constructBody("a", DeploymentTwoMegabyteSize, "labels", t) // >2 MB file + deployment3 := constructBody("a", DeploymentThreeMegabyteSize, "labels", t) // >3 MB file - deployment3 := constructBody("a", DeploymentMegabyteSize, "annotations", t) + deployment4 := constructBody("a", DeploymentMegabyteSize, "annotations", t) - deployment4 := constructBody("sample/sample", DeploymentMegabyteSize, "finalizers", t) // >1 MB file - deployment5 := constructBody("sample/sample", DeploymentTwoMegabyteSize, "finalizers", t) // >2 MB file + deployment5 := constructBody("sample/sample", DeploymentMegabyteSize, "finalizers", t) // >1 MB file + deployment6 := constructBody("sample/sample", DeploymentTwoMegabyteSize, "finalizers", t) // >2 MB file + deployment7 := constructBody("sample/sample", DeploymentThreeMegabyteSize, "finalizers", t) // >3 MB file requests := []struct { size string deploymentObject *appsv1.Deployment expectedMessage string }{ - {"1 MB", deployment1, expectedMsgFor1MB}, - {"2 MB", deployment2, expectedMsgFor2MB}, - {"1 MB", deployment3, expectedMsgForLargeAnnotation}, - {"1 MB", deployment4, expectedMsgFor1MB}, - {"2 MB", deployment5, expectedMsgFor2MB}, + {"1 MB labels", deployment1, expectedMsgFor1MB}, + {"2 MB labels", deployment2, expectedMsgFor2MB}, + {"3 MB labels", deployment3, expectedMsgFor3MB}, + {"1 MB annotations", deployment4, expectedMsgForLargeAnnotation}, + {"1 MB finalizers", deployment5, expectedMsgFor1MB}, + {"2 MB finalizers", deployment6, expectedMsgFor2MB}, + {"3 MB finalizers", deployment7, expectedMsgFor3MB}, } for _, r := range requests {