fix(http): limit inbound request body size to 1MB

docs/pages/deployment/security-considerations.rst

@@ -39,7 +39,7 @@ The following public APIs accept POST requests:
 - ``/oauth2/{subjectID}/response``
 
 To prevent malicious uploads, you MUST limit the size of the requests.
-As a safeguard, the Nuts node will also limit the size of request bodies.
+As a safeguard, the Nuts node also limits the size of request bodies to 1MB, on both the public and internal API endpoints, responding with HTTP 413 (Request Entity Too Large) when a request exceeds it.
 
 For example, Nginx has a configuration directive to limit the size of the request body:
 

docs/pages/release_notes.rst

@@ -14,6 +14,7 @@ Unreleased
 * #4233: ``request-credential`` API gains an optional ``credential_request_params`` JSON object overlaid on top of the OpenID4VCI Credential Request body sent to the issuer. Lets the wallet talk to issuers that accept additional fields, or to override the credential request entirely.
 
 ## Security
+* #4441: Inbound HTTP request bodies are now limited to 1MB on both the public and internal interfaces; larger requests are rejected with HTTP 413 (Request Entity Too Large). Previously no limit was enforced, contrary to what the deployment documentation stated. The heaviest legitimate requests (OAuth POSTs carrying Verifiable Presentations) stay well below this limit, and it matches the ``client_max_body_size 1M`` reverse proxy configuration the documentation recommends. By @stevenvegt in https://github.com/nuts-foundation/nuts-node/pull/4441
 * #4421: Stop reflecting fetched HTTP response bodies in API responses. The OAuth2 and OpenID4VCI callback handlers no longer place a remote endpoint's response body or error text into the returned ``error_description``, the did:web resolver no longer returns the fetched document body in its parse error, and the Discovery Service client no longer includes the remote server's error response in errors returned through the discovery APIs. Such content is now logged (truncated) for diagnostics instead. Static context such as the endpoint that failed is retained. By @stevenvegt in https://github.com/nuts-foundation/nuts-node/pull/4421
 * #4420: Harden the strict-mode HTTP client against SSRF. In strict mode the client now refuses at connect time to reach non-public addresses (loopback, private/RFC1918, unique local, link-local and unspecified), checked against the resolved IP so DNS-rebinding cannot bypass it, and refuses to follow a redirect that downgrades from HTTPS to HTTP. Cloud provider metadata endpoints are always blocked, following the OWASP SSRF prevention cheat sheet. Deployments that legitimately reach a private address for an internal flow (such as an internal credential offering or OAuth user flow) can permit specific ranges with ``http.client.allowedinternalcidrs``; publicly routable ranges that are internal-only can additionally be blocked with ``http.client.deniedcidrs``, which takes precedence. Reported by @raysabee, fixed by @stevenvegt in https://github.com/nuts-foundation/nuts-node/pull/4420
 

http/engine.go

@@ -30,6 +30,7 @@ import (
 	"time"
 
 	"github.com/labstack/echo/v4"
+	"github.com/labstack/echo/v4/middleware"
 	"github.com/nuts-foundation/nuts-node/core"
 	cryptoEngine "github.com/nuts-foundation/nuts-node/crypto"
 	"github.com/nuts-foundation/nuts-node/http/client"
@@ -42,6 +43,13 @@ import (
 
 const moduleName = "HTTP"
 
+// requestBodySizeLimit caps the size of inbound HTTP request bodies on all interfaces, returning
+// 413 Request Entity Too Large when exceeded. The largest legitimate requests are OAuth POSTs
+// carrying Verifiable Presentations; a deliberately heavy presentation (5 JWT credentials, each
+// with an RSA-4096 x5c chain of 4 certificates) is around 133 kB, so 1 MB leaves ample headroom.
+// Matches the client_max_body_size the deployment documentation recommends for reverse proxies.
+const requestBodySizeLimit = "1M"
+
 // New returns a new HTTP engine. The callback is called when an HTTP interface shuts down unexpectedly.
 func New(serverShutdownCb func(), signingKeyResolver cryptoEngine.KeyResolver) *Engine {
 	return &Engine{
@@ -97,6 +105,7 @@ func (h *Engine) Configure(serverConfig core.ServerConfig) error {
 	h.applyTracingMiddleware(h.server)
 	h.applyRateLimiterMiddleware(h.server, serverConfig)
 	h.applyLoggerMiddleware(h.server, []string{MetricsPath, StatusPath, HealthPath}, h.config.Log)
+	h.server.Use(middleware.BodyLimit(requestBodySizeLimit))
 	return h.applyAuthMiddleware(h.server, InternalPath, h.config.Internal.Auth)
 }
 

http/engine_test.go

@@ -387,6 +387,46 @@ func TestEngine_LoggingMiddleware(t *testing.T) {
 	})
 }
 
+func TestEngine_RequestBodyLimit(t *testing.T) {
+	// Configure sets the package-global client strict mode flag; restore it afterwards.
+	oldStrictMode := client.StrictMode
+	t.Cleanup(func() { client.StrictMode = oldStrictMode })
+	noop := func() {}
+	engine := New(noop, nil)
+	engine.config = createTestConfig()
+	err := engine.Configure(*core.NewServerConfig())
+	require.NoError(t, err)
+	echoBody := func(c echo.Context) error {
+		body, err := io.ReadAll(c.Request().Body)
+		if err != nil {
+			return err
+		}
+		return c.String(http.StatusOK, fmt.Sprintf("%d", len(body)))
+	}
+	engine.Router().POST("/", echoBody)
+	engine.Router().POST("/internal/test", echoBody)
+	require.NoError(t, engine.Start())
+	defer engine.Shutdown()
+	assertServerStarted(t, engine.config.Public.Address)
+	assertServerStarted(t, engine.config.Internal.Address)
+
+	t.Run("accepts a body under the limit", func(t *testing.T) {
+		response, err := http.Post("http://"+engine.config.Public.Address, "application/json", bytes.NewReader(make([]byte, 512*1024)))
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, response.StatusCode)
+	})
+	t.Run("rejects a body over the limit with 413", func(t *testing.T) {
+		response, err := http.Post("http://"+engine.config.Public.Address, "application/json", bytes.NewReader(make([]byte, 2*1024*1024)))
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusRequestEntityTooLarge, response.StatusCode)
+	})
+	t.Run("applies to the internal interface as well", func(t *testing.T) {
+		response, err := http.Post("http://"+engine.config.Internal.Address+"/internal/test", "application/json", bytes.NewReader(make([]byte, 2*1024*1024)))
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusRequestEntityTooLarge, response.StatusCode)
+	})
+}
+
 func assertServerStarted(t *testing.T, address string) {
 	t.Helper()
 	var err error