From b971b12d3cfa8887aa87957fbba92b90e3e99dc6 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Wed, 6 Feb 2019 16:58:24 -0800 Subject: [PATCH] Adding a limit on the maximum bytes accepted to be decoded in a resource write request. --- .../app/options/options_test.go | 1 + .../apimachinery/pkg/api/errors/errors.go | 24 +++++++++++++++++++ .../apimachinery/pkg/apis/meta/v1/types.go | 4 ++++ .../apiserver/pkg/endpoints/groupversion.go | 4 ++++ .../pkg/endpoints/handlers/create.go | 2 +- .../pkg/endpoints/handlers/delete.go | 4 ++-- .../apiserver/pkg/endpoints/handlers/patch.go | 2 +- .../apiserver/pkg/endpoints/handlers/rest.go | 21 ++++++++++++++-- .../pkg/endpoints/handlers/update.go | 2 +- .../apiserver/pkg/endpoints/installer.go | 2 ++ .../src/k8s.io/apiserver/pkg/server/config.go | 12 +++++++++- .../apiserver/pkg/server/genericapiserver.go | 5 ++++ .../pkg/server/options/server_run_options.go | 13 +++++++++- .../server/options/server_run_options_test.go | 22 +++++++++++++++++ 14 files changed, 109 insertions(+), 9 deletions(-) diff --git a/cmd/kube-apiserver/app/options/options_test.go b/cmd/kube-apiserver/app/options/options_test.go index 3f7984e85aa..ddee32d8ee5 100644 --- a/cmd/kube-apiserver/app/options/options_test.go +++ b/cmd/kube-apiserver/app/options/options_test.go @@ -130,6 +130,7 @@ func TestAddFlags(t *testing.T) { RequestTimeout: time.Duration(2) * time.Minute, MinRequestTimeout: 1800, JSONPatchMaxCopyBytes: int64(10 * 1024 * 1024), + MaxRequestBodyBytes: int64(10 * 1024 * 1024), }, Admission: &kubeoptions.AdmissionOptions{ GenericAdmission: &apiserveroptions.AdmissionOptions{ diff --git a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go index 81e5d48bf70..fa748a45061 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/errors/errors.go @@ -365,6 +365,17 @@ func NewTooManyRequestsError(message string) *StatusError { }} } +// NewRequestEntityTooLargeError returns an error indicating that the request +// entity was too large. +func NewRequestEntityTooLargeError(message string) *StatusError { + return &StatusError{metav1.Status{ + Status: metav1.StatusFailure, + Code: http.StatusRequestEntityTooLarge, + Reason: metav1.StatusReasonRequestEntityTooLarge, + Message: fmt.Sprintf("Request entity too large: %s", message), + }} +} + // NewGenericServerResponse returns a new error for server responses that are not in a recognizable form. func NewGenericServerResponse(code int, verb string, qualifiedResource schema.GroupResource, name, serverMessage string, retryAfterSeconds int, isUnexpectedResponse bool) *StatusError { reason := metav1.StatusReasonUnknown @@ -551,6 +562,19 @@ func IsTooManyRequests(err error) bool { return false } +// IsRequestEntityTooLargeError determines if err is an error which indicates +// the request entity is too large. +func IsRequestEntityTooLargeError(err error) bool { + if ReasonForError(err) == metav1.StatusReasonRequestEntityTooLarge { + return true + } + switch t := err.(type) { + case APIStatus: + return t.Status().Code == http.StatusRequestEntityTooLarge + } + return false +} + // IsUnexpectedServerError returns true if the server response was not in the expected API format, // and may be the result of another HTTP actor. func IsUnexpectedServerError(err error) bool { diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go index a85355a8333..0d82c2d5240 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go @@ -746,6 +746,10 @@ const ( // Status code 406 StatusReasonNotAcceptable StatusReason = "NotAcceptable" + // StatusReasonRequestEntityTooLarge means that the request entity is too large. + // Status code 413 + StatusReasonRequestEntityTooLarge StatusReason = "RequestEntityTooLarge" + // StatusReasonUnsupportedMediaType means that the content type sent by the client is not acceptable // to the server - for instance, attempting to send protobuf for a resource that supports only json and yaml. // API calls that return UnsupportedMediaType can never succeed. diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go index a1ba7bdbbeb..79cfefe4669 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/groupversion.go @@ -87,6 +87,10 @@ type APIGroupVersion struct { // OpenAPIModels exposes the OpenAPI models to each individual handler. OpenAPIModels openapiproto.Models + + // The limit on the request body size that would be accepted and decoded in a write request. + // 0 means no limit. + MaxRequestBodyBytes int64 } // InstallREST registers the REST handlers (storage, watch, proxy and redirect) into a restful Container. diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go index cd95727fde9..fdb85eca6af 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go @@ -85,7 +85,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, admit admission.Inte decoder := scope.Serializer.DecoderToVersion(s.Serializer, scope.HubGroupVersion) - body, err := readBody(req) + body, err := limitedReadBody(req, scope.MaxRequestBodyBytes) if err != nil { scope.err(err, w, req) return diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go index 1353ed82503..4246a7fd234 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go @@ -72,7 +72,7 @@ func DeleteResource(r rest.GracefulDeleter, allowsOptions bool, scope RequestSco options := &metav1.DeleteOptions{} if allowsOptions { - body, err := readBody(req) + body, err := limitedReadBody(req, scope.MaxRequestBodyBytes) if err != nil { scope.err(err, w, req) return @@ -226,7 +226,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco options := &metav1.DeleteOptions{} if checkBody { - body, err := readBody(req) + body, err := limitedReadBody(req, scope.MaxRequestBodyBytes) if err != nil { scope.err(err, w, req) return 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 dd41e93eb9e..4c2fdeea806 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -96,7 +96,7 @@ func PatchResource(r rest.Patcher, scope RequestScope, admit admission.Interface return } - patchBytes, err := readBody(req) + patchBytes, err := limitedReadBody(req, scope.MaxRequestBodyBytes) if err != nil { scope.err(err, w, req) return diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index 744ab6ea1ba..299be6e67dc 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -20,6 +20,7 @@ import ( "context" "encoding/hex" "fmt" + "io" "io/ioutil" "net/http" "net/url" @@ -70,6 +71,8 @@ type RequestScope struct { // HubGroupVersion indicates what version objects read from etcd or incoming requests should be converted to for in-memory handling. HubGroupVersion schema.GroupVersion + + MaxRequestBodyBytes int64 } func (scope *RequestScope) err(err error, w http.ResponseWriter, req *http.Request) { @@ -333,9 +336,23 @@ func summarizeData(data []byte, maxLength int) string { } } -func readBody(req *http.Request) ([]byte, error) { +func limitedReadBody(req *http.Request, limit int64) ([]byte, error) { defer req.Body.Close() - return ioutil.ReadAll(req.Body) + if limit <= 0 { + return ioutil.ReadAll(req.Body) + } + lr := &io.LimitedReader{ + R: req.Body, + N: limit + 1, + } + data, err := ioutil.ReadAll(lr) + if err != nil { + return nil, err + } + if lr.N <= 0 { + return nil, errors.NewRequestEntityTooLargeError(fmt.Sprintf("limit is %d", limit)) + } + return data, nil } func parseTimeout(str string) time.Duration { diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index 7289d9495f1..bf65532143d 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go @@ -70,7 +70,7 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac return } - body, err := readBody(req) + body, err := limitedReadBody(req, scope.MaxRequestBodyBytes) if err != nil { scope.err(err, w, req) return diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go index 4b9650b5575..d34869c6f84 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go @@ -513,6 +513,8 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag HubGroupVersion: schema.GroupVersion{Group: fqKindToRegister.Group, Version: runtime.APIVersionInternal}, MetaGroupVersion: metav1.SchemeGroupVersion, + + MaxRequestBodyBytes: a.group.MaxRequestBodyBytes, } if a.group.MetaGroupVersion != nil { reqScope.MetaGroupVersion = *a.group.MetaGroupVersion diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index 0ae03047023..653c72baab7 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -159,6 +159,9 @@ 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. + // 0 means no limit. + MaxRequestBodyBytes int64 // MaxRequestsInFlight is the maximum number of parallel non-long-running requests. Every further // request has to wait. Applies only to non-mutating requests. MaxRequestsInFlight int @@ -268,7 +271,13 @@ func NewConfig(codecs serializer.CodecFactory) *Config { // on the size increase the "copy" operations in a json patch // can cause. See // https://github.com/etcd-io/etcd/blob/release-3.3/etcdserver/server.go#L90. - JSONPatchMaxCopyBytes: int64(10 * 1024 * 1024), + JSONPatchMaxCopyBytes: int64(10 * 1024 * 1024), + // 10MB is the recommended maximum client request size in bytes + // the etcd server should accept. Thus, we set it as the + // maximum bytes accepted to be decoded in a resource write + // request. See + // https://github.com/etcd-io/etcd/blob/release-3.3/etcdserver/server.go#L90. + MaxRequestBodyBytes: int64(10 * 1024 * 1024), EnableAPIResponseCompression: utilfeature.DefaultFeatureGate.Enabled(features.APIResponseCompression), // Default to treating watch as a long-running operation @@ -461,6 +470,7 @@ func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*G DiscoveryGroupManager: discovery.NewRootAPIsHandler(c.DiscoveryAddresses, c.Serializer), enableAPIResponseCompression: c.EnableAPIResponseCompression, + maxRequestBodyBytes: c.MaxRequestBodyBytes, } for { diff --git a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go index 3974bf9b5e9..2c31271405b 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go +++ b/staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go @@ -156,6 +156,10 @@ type GenericAPIServer struct { // HandlerChainWaitGroup allows you to wait for all chain handlers finish after the server shutdown. HandlerChainWaitGroup *utilwaitgroup.SafeWaitGroup + + // The limit on the request body size that would be accepted and decoded in a write request. + // 0 means no limit. + maxRequestBodyBytes int64 } // DelegationTarget is an interface which allows for composition of API servers with top level handling that works @@ -336,6 +340,7 @@ func (s *GenericAPIServer) installAPIResources(apiPrefix string, apiGroupInfo *A apiGroupVersion.OptionsExternalVersion = apiGroupInfo.OptionsExternalVersion } apiGroupVersion.OpenAPIModels = openAPIModels + apiGroupVersion.MaxRequestBodyBytes = s.maxRequestBodyBytes if err := apiGroupVersion.InstallREST(s.Handler.GoRestfulContainer); err != nil { return fmt.Errorf("unable to setup API %v: %v", apiGroupInfo, err) diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go b/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go index 78a4ddcf9c9..02639bf93b9 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go @@ -45,7 +45,12 @@ type ServerRunOptions struct { // We intentionally did not add a flag for this option. Users of the // apiserver library can wire it to a flag. JSONPatchMaxCopyBytes int64 - TargetRAMMB int + // The limit on the request body size that would be accepted and + // decoded in a write request. 0 means no limit. + // We intentionally did not add a flag for this option. Users of the + // apiserver library can wire it to a flag. + MaxRequestBodyBytes int64 + TargetRAMMB int } func NewServerRunOptions() *ServerRunOptions { @@ -56,6 +61,7 @@ func NewServerRunOptions() *ServerRunOptions { RequestTimeout: defaults.RequestTimeout, MinRequestTimeout: defaults.MinRequestTimeout, JSONPatchMaxCopyBytes: defaults.JSONPatchMaxCopyBytes, + MaxRequestBodyBytes: defaults.MaxRequestBodyBytes, } } @@ -68,6 +74,7 @@ func (s *ServerRunOptions) ApplyTo(c *server.Config) error { c.RequestTimeout = s.RequestTimeout c.MinRequestTimeout = s.MinRequestTimeout c.JSONPatchMaxCopyBytes = s.JSONPatchMaxCopyBytes + c.MaxRequestBodyBytes = s.MaxRequestBodyBytes c.PublicAddress = s.AdvertiseAddress return nil @@ -116,6 +123,10 @@ func (s *ServerRunOptions) Validate() []error { errors = append(errors, fmt.Errorf("--json-patch-max-copy-bytes can not be negative value")) } + if s.MaxRequestBodyBytes < 0 { + errors = append(errors, fmt.Errorf("--max-resource-write-bytes can not be negative value")) + } + return errors } diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go index bd00eb84c4d..bb7e618230e 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go @@ -41,6 +41,7 @@ func TestServerRunOptionsValidate(t *testing.T) { RequestTimeout: time.Duration(2) * time.Minute, MinRequestTimeout: 1800, JSONPatchMaxCopyBytes: 10 * 1024 * 1024, + MaxRequestBodyBytes: 10 * 1024 * 1024, TargetRAMMB: -65536, }, expectErr: "--target-ram-mb can not be negative value", @@ -55,6 +56,7 @@ func TestServerRunOptionsValidate(t *testing.T) { RequestTimeout: time.Duration(2) * time.Minute, MinRequestTimeout: 1800, JSONPatchMaxCopyBytes: 10 * 1024 * 1024, + MaxRequestBodyBytes: 10 * 1024 * 1024, TargetRAMMB: 65536, }, expectErr: "--max-requests-inflight can not be negative value", @@ -69,6 +71,7 @@ func TestServerRunOptionsValidate(t *testing.T) { RequestTimeout: time.Duration(2) * time.Minute, MinRequestTimeout: 1800, JSONPatchMaxCopyBytes: 10 * 1024 * 1024, + MaxRequestBodyBytes: 10 * 1024 * 1024, TargetRAMMB: 65536, }, expectErr: "--max-mutating-requests-inflight can not be negative value", @@ -83,6 +86,7 @@ func TestServerRunOptionsValidate(t *testing.T) { RequestTimeout: -time.Duration(2) * time.Minute, MinRequestTimeout: 1800, JSONPatchMaxCopyBytes: 10 * 1024 * 1024, + MaxRequestBodyBytes: 10 * 1024 * 1024, TargetRAMMB: 65536, }, expectErr: "--request-timeout can not be negative value", @@ -97,6 +101,7 @@ func TestServerRunOptionsValidate(t *testing.T) { RequestTimeout: time.Duration(2) * time.Minute, MinRequestTimeout: -1800, JSONPatchMaxCopyBytes: 10 * 1024 * 1024, + MaxRequestBodyBytes: 10 * 1024 * 1024, TargetRAMMB: 65536, }, expectErr: "--min-request-timeout can not be negative value", @@ -111,10 +116,26 @@ func TestServerRunOptionsValidate(t *testing.T) { RequestTimeout: time.Duration(2) * time.Minute, MinRequestTimeout: 1800, JSONPatchMaxCopyBytes: -10 * 1024 * 1024, + MaxRequestBodyBytes: 10 * 1024 * 1024, TargetRAMMB: 65536, }, expectErr: "--json-patch-max-copy-bytes can not be negative value", }, + { + name: "Test when MaxRequestBodyBytes is negative value", + testOptions: &ServerRunOptions{ + AdvertiseAddress: net.ParseIP("192.168.10.10"), + CorsAllowedOriginList: []string{"10.10.10.100", "10.10.10.200"}, + MaxRequestsInFlight: 400, + MaxMutatingRequestsInFlight: 200, + RequestTimeout: time.Duration(2) * time.Minute, + MinRequestTimeout: 1800, + JSONPatchMaxCopyBytes: 10 * 1024 * 1024, + MaxRequestBodyBytes: -10 * 1024 * 1024, + TargetRAMMB: 65536, + }, + expectErr: "--max-resource-write-bytes can not be negative value", + }, { name: "Test when ServerRunOptions is valid", testOptions: &ServerRunOptions{ @@ -125,6 +146,7 @@ func TestServerRunOptionsValidate(t *testing.T) { RequestTimeout: time.Duration(2) * time.Minute, MinRequestTimeout: 1800, JSONPatchMaxCopyBytes: 10 * 1024 * 1024, + MaxRequestBodyBytes: 10 * 1024 * 1024, TargetRAMMB: 65536, }, },