Add max_content_length, max_request_body_size (TykTechnologies#5043)

<!-- Provide a general summary of your changes in the Title above -->

## Description

The PR implements a `max_content_length` addition in the area of
`max_request_body_size`.

The check for `max_content_length` responds with HTTP 411 Length
Required if the content-length header is not present. A handled
exception to this is `Transfer-Encoding: chunked`, which doesn't provide
a content length header and thus can't handle before reading the full
request body. Per-API middleware has access to the body and can validate
length.

The `max_request_body_size` limits the amount of bytes read for the
request if the content-length is unknown or unset. Because there are
exceptions that bypass `max_content_length`, this has a separate flag.
Depending on wanted behaviour, one or the other option may be
configured.

Both flags issue a response with HTTP 413 Entity too big, based on the
content-length header value only.

<!-- Describe your changes in detail -->

## Related Issue

https://tyktech.atlassian.net/browse/TT-5186

<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

## Motivation and Context

<!-- Why is this change required? What problem does it solve? -->

## How This Has Been Tested

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

## Screenshots (if appropriate)

## Types of changes

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

## Checklist

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

---------

Co-authored-by: Tit Petric <[email protected]>

cli/linter/schema.json

@@ -614,6 +614,9 @@
             "type": "string"
           }
         },
+        "max_content_length": {
+          "type": "integer"
+        },
         "max_request_body_size": {
           "type": "integer"
         }

config/config.go

@@ -412,13 +412,35 @@ type HttpServerOptionsConfig struct {
 
 	// MaxRequestBodySize configures the maximum request body size in bytes.
 	//
-	// Tyk API Gateway copies the whole request into memory at the beginning
-	// of the request handling. Large requests could fill up memory if they
-	// are not blocked.
+	// Similarly to `max_content_length`, this option will use the header
+	// value of Content-Length to respond with HTTP 413 Request entity too
+	// large. If the content-length header is not present, it will limit the
+	// amount of the request we're reading in, up to the specified size.
+	//
+	// The gateway will copy the request body into memory after this
+	// check and limit are enforced.
 	//
 	// See more information about setting request size limits here:
 	// https://tyk.io/docs/basic-config-and-security/control-limit-traffic/request-size-limits/#maximum-request-sizes
 	MaxRequestBodySize int64 `json:"max_request_body_size"`
+
+	// MaxContentLength configures maximum request body size validation.
+	//
+	// The option validates the Content-Length header of the request to
+	// limit the request size. It will respond with either "413 Request
+	// entity too large" or "411 Length required".
+	//
+	// With chunked encoding, the Content-Length header is unset. If it's
+	// detected, the request body wouldn't be validated as we'd have to
+	// read in the full request. In order to limit those kind of requests,
+	// a separate `max_request_body_size` option can be configured.
+	//
+	// The gateway will copy the request body into memory after this
+	// check and limit are enforced.
+	//
+	// See more information about setting request size limits here:
+	// https://tyk.io/docs/basic-config-and-security/control-limit-traffic/request-size-limits/#maximum-request-sizes
+	MaxContentLength int64 `json:"max_content_length"`
 }
 
 type AuthOverrideConf struct {

gateway/mw_redis_cache.go

@@ -100,7 +100,10 @@ func addBodyHash(req *http.Request, regex string, h hash.Hash) (err error) {
 }
 
 func readBody(req *http.Request) (bodyBytes []byte, err error) {
-	req.Body = copyBody(req.Body, false)
+	req.Body, err = copyBody(req.Body, false)
+	if err != nil {
+		return nil, err
+	}
 	return ioutil.ReadAll(req.Body)
 }
 

gateway/proxy_muxer.go

@@ -3,6 +3,7 @@ package gateway
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -20,6 +21,7 @@ import (
 
 	"github.com/TykTechnologies/again"
 	"github.com/TykTechnologies/tyk/config"
+	"github.com/TykTechnologies/tyk/internal/httputil"
 	"github.com/TykTechnologies/tyk/tcp"
 
 	"github.com/gorilla/mux"
@@ -30,6 +32,7 @@ import (
 // handleWrapper's only purpose is to allow router to be dynamically replaced
 type handleWrapper struct {
 	router             *mux.Router
+	maxContentLength   int64
 	maxRequestBodySize int64
 }
 
@@ -43,21 +46,62 @@ func (h *h2cWrapper) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	h.h.ServeHTTP(w, r)
 }
 
-func (h *handleWrapper) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	// limit request body size if configured
+func (h *handleWrapper) handleRequestLimits(w http.ResponseWriter, r *http.Request) bool {
+	// Validate Content-Length, limit request size by header value
+	if h.maxContentLength > 0 {
+		// if Content-Length is larger than the configured limit return a status 413
+		if r.ContentLength > h.maxContentLength {
+			httputil.EntityTooLarge(w, r)
+			return false
+		}
+
+		// Transfer-Encoding: chunked does not provide Content-Length
+		if !httputil.IsTransferEncodingChunked(r) {
+			// Request should include Content-Length header (HTTP 411)
+			httputil.LengthRequired(w, r)
+			return false
+		}
+
+		// No length available reasonably at this point. The only thing
+		// we can do from here on is to limit the number of bytes read
+		// from a HTTP request body in the maxRequestBodySize check below.
+	}
+
+	// Limit request body size
 	if h.maxRequestBodySize > 0 {
-		// if content length is set and already larger than the configured limit return a status 413
-		if r.ContentLength >= 0 && r.ContentLength > h.maxRequestBodySize {
-			http.HandlerFunc(requestEntityTooLarge).ServeHTTP(w, r)
-			return
+		// if Content-Length is larger than the configured limit return a status 413
+		if r.ContentLength > h.maxRequestBodySize {
+			httputil.EntityTooLarge(w, r)
+			return false
 		}
 
 		// in case the content length is wrong or not set limit the reader itself
-		r.Body = http.MaxBytesReader(w, r.Body, h.maxRequestBodySize)
+		httputil.LimitReader(r, h.maxRequestBodySize)
+	}
+
+	// make request body to be nopCloser and re-readable
+	// before serve it through chain of middlewares
+	if err := nopCloseRequestBodyErr(r); err != nil {
+		// Handle the max request body size error returned from the body reader.
+		if errors.Is(err, httputil.ErrContentTooLong) {
+			httputil.EntityTooLarge(w, r)
+			return false
+		}
+		log.WithError(err).Error("Error reading request body")
+		return false
+	}
+
+	// No limiting
+	return true
+}
+
+func (h *handleWrapper) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	if r.Body != nil {
+		if !h.handleRequestLimits(w, r) {
+			return
+		}
 	}
 
-	// make request body to be nopCloser and re-readable before serve it through chain of middlewares
-	nopCloseRequestBody(r)
 	if NewRelicApplication != nil {
 		txn := NewRelicApplication.StartTransaction(r.URL.Path, w, r)
 		defer txn.End()
@@ -381,7 +425,6 @@ func (m *proxyMux) swap(new *proxyMux, gw *Gateway) {
 }
 
 func (m *proxyMux) serve(gw *Gateway) {
-
 	conf := gw.GetConfig()
 	for _, p := range m.proxies {
 		if p.listener == nil {
@@ -419,6 +462,7 @@ func (m *proxyMux) serve(gw *Gateway) {
 			h = &handleWrapper{
 				router:             p.router,
 				maxRequestBodySize: conf.HttpServerOptions.MaxRequestBodySize,
+				maxContentLength:   conf.HttpServerOptions.MaxContentLength,
 			}
 			// by default enabling h2c by wrapping handler in h2c. This ensures all features including tracing work
 			// in h2c services.
@@ -509,8 +553,3 @@ func (m *proxyMux) generateListener(listenPort int, protocol string, gw *Gateway
 	}
 	return l, nil
 }
-
-func requestEntityTooLarge(w http.ResponseWriter, _ *http.Request) {
-	status := http.StatusRequestEntityTooLarge
-	http.Error(w, http.StatusText(status), status)
-}

gateway/reverse_proxy.go

@@ -1839,19 +1839,19 @@ func (n *nopCloserBuffer) Close() error {
 	return nil
 }
 
-func copyBody(body io.ReadCloser, isClientResponseBody bool) io.ReadCloser {
+func copyBody(body io.ReadCloser, isClientResponseBody bool) (io.ReadCloser, error) {
 	// check if body was already read and converted into our nopCloser
 	if nc, ok := body.(*nopCloserBuffer); ok {
 		// seek to the beginning to have it ready for next read
 		nc.Seek(0, io.SeekStart)
-		return body
+		return body, nil
 	}
 
 	// body is http's io.ReadCloser - read it up
 	rwc, err := newNopCloserBuffer(body)
 	if err != nil {
 		log.WithError(err).Error("error creating buffered request body")
-		return body
+		return body, nil
 	}
 
 	// Consume reader if it's from a http client response.
@@ -1861,39 +1861,50 @@ func copyBody(body io.ReadCloser, isClientResponseBody bool) io.ReadCloser {
 	if isClientResponseBody {
 		if err := rwc.copy(); err != nil {
 			log.WithError(err).Error("error reading request body")
-			return body
+			return body, err
 		}
 	}
 
 	// use seek-able reader for further body usage
-	return rwc
+	return rwc, nil
 }
 
-func copyRequest(r *http.Request) *http.Request {
+func copyRequest(r *http.Request) (*http.Request, error) {
+	var err error
 	if r.ContentLength == -1 {
-		return r
+		return r, nil
 	}
 
 	if r.Body != nil {
-		r.Body = copyBody(r.Body, false)
+		r.Body, err = copyBody(r.Body, false)
 	}
 
-	return r
+	return r, err
 }
 
-func copyResponse(r *http.Response) *http.Response {
+func copyResponse(r *http.Response) (*http.Response, error) {
+	var err error
 	// If the response is 101 Switching Protocols then the body will contain a
 	// `*http.readWriteCloserBody` which cannot be copied (see stdlib documentation).
 	// In this case we want to return immediately to avoid a silent crash.
 	if r.StatusCode == http.StatusSwitchingProtocols {
-		return r
+		return r, nil
 	}
 
 	if r.Body != nil {
-		r.Body = copyBody(r.Body, true)
+		r.Body, err = copyBody(r.Body, true)
 	}
 
-	return r
+	return r, err
+}
+
+func nopCloseRequestBodyErr(r *http.Request) error {
+	if r == nil {
+		return nil
+	}
+
+	_, err := copyRequest(r)
+	return err
 }
 
 func nopCloseRequestBody(r *http.Request) {

gateway/reverse_proxy_test.go

@@ -1506,8 +1506,8 @@ func BenchmarkCopyRequestResponse(b *testing.B) {
 		req.Body = ioutil.NopCloser(strings.NewReader(str))
 		res.Body = ioutil.NopCloser(strings.NewReader(str))
 		for j := 0; j < 10; j++ {
-			req = copyRequest(req)
-			res = copyResponse(res)
+			req, _ = copyRequest(req)
+			res, _ = copyResponse(res)
 		}
 	}
 }

internal/httputil/Taskfile.yml

@@ -0,0 +1,10 @@
+---
+version: "3"
+
+tasks:
+  default:
+    desc: "Run tests"
+    cmds:
+      - go fmt ./...
+      - goimports -w .
+      - go test -race -count=100 -cover .

internal/httputil/limit_reader.go

@@ -0,0 +1,50 @@
+package httputil
+
+import (
+	"errors"
+	"io"
+	"net/http"
+)
+
+// ErrContentTooLong is an internal error to handle limit overflows.
+var ErrContentTooLong = errors.New("content size over the declared limit")
+
+// LimitReader replaces the request body with a reader designed to
+// error out when the size limit has been exceeded.
+func LimitReader(r *http.Request, limit int64) {
+	r.Body = &limitedRequestBody{
+		body:  r.Body,
+		limit: limit,
+		err:   nil,
+	}
+}
+
+type limitedRequestBody struct {
+	body  io.Reader
+	limit int64
+	err   error
+}
+
+// Read implements io.Reader.
+func (l *limitedRequestBody) Read(p []byte) (n int, err error) {
+	n, err = l.body.Read(p)
+	if l.err != nil {
+		return n, l.err
+	}
+
+	if n > 0 {
+		l.limit -= int64(n)
+		if l.limit < 0 {
+			l.err = errors.New("request entity too large")
+			return n, ErrContentTooLong
+		}
+	}
+
+	return n, err
+}
+
+// Close implements io.Closer.
+func (l *limitedRequestBody) Close() error {
+	l.err = nil
+	return nil
+}

internal/httputil/limit_reader_test.go

@@ -0,0 +1,47 @@
+package httputil_test
+
+import (
+	"errors"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	. "github.com/TykTechnologies/tyk/internal/httputil"
+)
+
+func TestLimitReader(t *testing.T) {
+	loremIpsum := "Lorem Ipsum dolor sit amet"
+
+	// Create a test request with a request body larger than the limit
+	req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(loremIpsum))
+
+	// Create a test response recorder
+	w := httptest.NewRecorder()
+
+	// Call the LimitReader function
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		LimitReader(r, 10)
+
+		// Read the request body
+		body, err := io.ReadAll(r.Body)
+		if err != nil {
+			if errors.Is(err, ErrContentTooLong) {
+				EntityTooLarge(w, r)
+				return
+			}
+			t.Errorf("Failed to read request body: %v", err)
+		}
+
+		// Check if the body matches the expected value
+		expectedBody := "Lorem Ipsu"
+		assert.Equal(t, expectedBody, body)
+	})
+	handler.ServeHTTP(w, req)
+
+	// Check the response status code
+	assert.Equal(t, http.StatusRequestEntityTooLarge, w.Result().StatusCode)
+}