From 3653eff0ed21100a15d6360c59fdc49026cfdbff Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 2 Oct 2019 14:46:08 -0400 Subject: [PATCH 1/2] bump gopkg.in/yaml.v2 v2.2.4 Kubernetes-commit: 8aeffa8716dcf986544df74444264ef639d61a7a --- go.mod | 19 ++++++++++--------- go.sum | 6 ++---- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 36a71eaf5..9466f2cf9 100644 --- a/go.mod +++ b/go.mod @@ -46,12 +46,12 @@ require ( google.golang.org/grpc v1.23.0 gopkg.in/natefinch/lumberjack.v2 v2.0.0 gopkg.in/square/go-jose.v2 v2.2.2 - gopkg.in/yaml.v2 v2.2.2 + gopkg.in/yaml.v2 v2.2.4 gotest.tools v2.2.0+incompatible // indirect - k8s.io/api v0.0.0-20190927115716-5d581ce610b0 - k8s.io/apimachinery v0.0.0-20191001195453-082230a5ffdd - k8s.io/client-go v0.0.0-20191001195819-0ebb3d5f4902 - k8s.io/component-base v0.0.0-20190926082537-804254d56004 + k8s.io/api v0.0.0 + k8s.io/apimachinery v0.0.0 + k8s.io/client-go v0.0.0 + k8s.io/component-base v0.0.0 k8s.io/klog v1.0.0 k8s.io/kube-openapi v0.0.0-20190816220812-743ec37842bf k8s.io/utils v0.0.0-20190920012459-5008bf6f8cd6 @@ -67,8 +67,9 @@ replace ( golang.org/x/sys => golang.org/x/sys v0.0.0-20190209173611-3b5209105503 golang.org/x/text => golang.org/x/text v0.3.1-0.20181227161524-e6919f6577db golang.org/x/time => golang.org/x/time v0.0.0-20161028155119-f51c12702a4d - k8s.io/api => k8s.io/api v0.0.0-20190927115716-5d581ce610b0 - k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20191001195453-082230a5ffdd - k8s.io/client-go => k8s.io/client-go v0.0.0-20191001195819-0ebb3d5f4902 - k8s.io/component-base => k8s.io/component-base v0.0.0-20190926082537-804254d56004 + k8s.io/api => ../api + k8s.io/apimachinery => ../apimachinery + k8s.io/apiserver => ../apiserver + k8s.io/client-go => ../client-go + k8s.io/component-base => ../component-base ) diff --git a/go.sum b/go.sum index 7e4346cf0..bfdac08ce 100644 --- a/go.sum +++ b/go.sum @@ -285,15 +285,13 @@ gopkg.in/yaml.v2 v2.2.1 h1:mUhvW9EsL+naU5Q3cakzfE91YhliOondGd6ZrsDBHQE= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= +gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= -k8s.io/api v0.0.0-20190927115716-5d581ce610b0/go.mod h1:l2ZHS8QbgqodGx7yrYsOSwIxOR76BpGiW1OywXo9PFI= -k8s.io/apimachinery v0.0.0-20191001195453-082230a5ffdd/go.mod h1:grJJH0hgilA2pYoUiJcPu2EDUal95NTq1vpxxvMLSu8= -k8s.io/client-go v0.0.0-20191001195819-0ebb3d5f4902/go.mod h1:Ffzajf+CyEz64mn6gHeT33NcKLbBO+z6xZKupz7Q91Y= -k8s.io/component-base v0.0.0-20190926082537-804254d56004/go.mod h1:+sedDd0Yj/9lFSZjan8FdX4Jednr2we+Q0ZDeicbKSc= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= k8s.io/klog v0.0.0-20181102134211-b9b56d5dfc92/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= k8s.io/klog v0.3.0/go.mod h1:Gq+BEi5rUBO/HRz0bTSXDUcqjScdoY3a9IHpCEIOOfk= From f7fbf2eee4ec8e1e233d8a89d85a983d89bbe257 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Fri, 27 Sep 2019 16:36:48 -0400 Subject: [PATCH 2/2] Limit YAML/JSON decode size Kubernetes-commit: 8ef4566cefebf49f9a806a36df2105c9149785a1 --- .../handlers/fieldmanager/fieldmanager.go | 4 ++-- pkg/endpoints/handlers/patch.go | 18 +++++++++++++++++ pkg/server/config.go | 20 +++++++++---------- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/pkg/endpoints/handlers/fieldmanager/fieldmanager.go b/pkg/endpoints/handlers/fieldmanager/fieldmanager.go index 21f408813..981741817 100644 --- a/pkg/endpoints/handlers/fieldmanager/fieldmanager.go +++ b/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/pkg/endpoints/handlers/patch.go b/pkg/endpoints/handlers/patch.go index 8d589cc91..029513f93 100644 --- a/pkg/endpoints/handlers/patch.go +++ b/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/pkg/server/config.go b/pkg/server/config.go index b75422a6e..998999e8a 100644 --- a/pkg/server/config.go +++ b/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