From 6f8d5ef6c4bc1912b4ee4fc2b789a7accbc7d05b Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Mon, 5 Oct 2026 14:51:26 +0200 Subject: [PATCH] fix(discovery): cap registered presentation size and raise the client response cap (#4597) Backport of #4597 to V6.2. Registered Verifiable Presentations are now limited to 64 KiB, checked first in verifyRegistration and returned as ErrInvalidPresentation (HTTP 400). The Discovery Service client reads responses of up to 10 MiB from the operator-configured Discovery Server instead of the 1 MiB the strict HTTP client applies to all other outbound calls; the cap is now a per-client setting (WithMaxResponseSize), defaulting to the existing 1 MiB. Previously a client could no longer synchronize a service once a response exceeded 1 MiB, which a few hundred registrations, or two deliberately padded ones, could cause. Part of #4596 (cherry picked from commit 1b3d7c753d70ab907bdda3c059533e856102d22b) Assisted by AI --- discovery/api/server/client/http.go | 6 +-- discovery/module.go | 20 +++++++- discovery/module_test.go | 12 +++++ docs/pages/deployment/discovery.rst | 3 ++ docs/pages/release_notes.rst | 7 +++ http/client/client.go | 80 ++++++++++++++++++----------- http/client/client_test.go | 37 ++++++++++++- 7 files changed, 128 insertions(+), 37 deletions(-) diff --git a/discovery/api/server/client/http.go b/discovery/api/server/client/http.go index fd2733f89c..5e67f64bc4 100644 --- a/discovery/api/server/client/http.go +++ b/discovery/api/server/client/http.go @@ -33,10 +33,10 @@ import ( "time" ) -// New creates a new DefaultHTTPClient. -func New(timeout time.Duration) *DefaultHTTPClient { +// New creates a new DefaultHTTPClient. Options are passed to the underlying strict HTTP client. +func New(timeout time.Duration, options ...client.Option) *DefaultHTTPClient { return &DefaultHTTPClient{ - client: client.New(timeout), + client: client.New(timeout, options...), } } diff --git a/discovery/module.go b/discovery/module.go index 9251b22b4e..1c145cbdf5 100644 --- a/discovery/module.go +++ b/discovery/module.go @@ -28,6 +28,7 @@ import ( "github.com/nuts-foundation/nuts-node/v6/core" "github.com/nuts-foundation/nuts-node/v6/discovery/api/server/client" "github.com/nuts-foundation/nuts-node/v6/discovery/log" + httpclient "github.com/nuts-foundation/nuts-node/v6/http/client" "github.com/nuts-foundation/nuts-node/v6/storage" "github.com/nuts-foundation/nuts-node/v6/vcr" "github.com/nuts-foundation/nuts-node/v6/vcr/credential" @@ -57,6 +58,20 @@ var ( errRetractionContainsCredentials = errors.New("retraction presentation must not contain credentials") errInvalidRetractionJTIClaim = errors.New("invalid/missing 'retract_jti' claim for retraction presentation") errCyclicForwardingDetected = errors.New("cyclic forwarding detected") + errPresentationTooLarge = fmt.Errorf("presentation exceeds maximum size of %d bytes", maxPresentationSize) +) + +const ( + // maxPresentationSize is the maximum size (in bytes) of a Verifiable Presentation that can be registered on a Discovery Service. + // Legitimate registrations are a few KB; a VP carrying an X509Credential with a full PKIoverheid certificate chain + // measures about 16 KB. The cap bounds what a single participant can add to the service, so that a few + // registrations cannot push the service's responses over what clients are willing to read (maxResponseSize). + maxPresentationSize = 64 * 1024 + // maxResponseSize is the maximum size (in bytes) of a response from a remote Discovery Service that the client reads. + // It is larger than the default of the strict HTTP client, since the endpoint is operator-configured (through the + // service definition) and a service's full list of presentations can exceed 1 MB. It is bounded because the + // response is buffered and parsed in memory. + maxResponseSize = 10 * 1024 * 1024 ) var _ core.Injectable = &Module{} @@ -106,7 +121,7 @@ func (m *Module) Configure(serverConfig core.ServerConfig) error { return err } - m.httpClient = client.New(serverConfig.HTTPClient.Timeout) + m.httpClient = client.New(serverConfig.HTTPClient.Timeout, httpclient.WithMaxResponseSize(maxResponseSize)) return m.loadDefinitions() @@ -229,6 +244,9 @@ func (m *Module) Register(context context.Context, serviceID string, presentatio func (m *Module) verifyRegistration(definition ServiceDefinition, presentation vc.VerifiablePresentation) error { // First, simple sanity checks + if len(presentation.Raw()) > maxPresentationSize { + return errors.Join(ErrInvalidPresentation, errPresentationTooLarge) + } if presentation.Format() != vc.JWTPresentationProofFormat { return errors.Join(ErrInvalidPresentation, errUnsupportedPresentationFormat) } diff --git a/discovery/module_test.go b/discovery/module_test.go index 18a78b0f04..f3ef4ac288 100644 --- a/discovery/module_test.go +++ b/discovery/module_test.go @@ -40,6 +40,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" "gorm.io/gorm" + "strings" "sync" "sync/atomic" "testing" @@ -118,6 +119,17 @@ func Test_Module_Register(t *testing.T) { assert.EqualError(t, err, "presentation is invalid for registration\npresentation is valid for too long (max 1s)") }) + t.Run("too large", func(t *testing.T) { + m, _ := setupModule(t, storageEngine) + // pad the VP with a claim to push it over the maximum size, a real one is a few KB + largeVP := createPresentationCustom(aliceDID, func(claims map[string]interface{}, _ *vc.VerifiablePresentation) { + claims["padding"] = strings.Repeat("a", maxPresentationSize) + }, vcAlice) + + err := m.Register(ctx, testServiceID, largeVP) + + assert.EqualError(t, err, "presentation is invalid for registration\npresentation exceeds maximum size of 65536 bytes") + }) t.Run("no expiration", func(t *testing.T) { m, _ := setupModule(t, storageEngine) err := m.Register(ctx, testServiceID, createPresentationCustom(aliceDID, func(claims map[string]interface{}, _ *vc.VerifiablePresentation) { diff --git a/docs/pages/deployment/discovery.rst b/docs/pages/deployment/discovery.rst index aa10f7ad2e..879a051c41 100644 --- a/docs/pages/deployment/discovery.rst +++ b/docs/pages/deployment/discovery.rst @@ -184,4 +184,7 @@ A service definition consists of: - ``presentation_max_validity``: the maximum validity of the Verifiable Presentation in seconds - ``presentation_definition``: the presentation definition that specifies the required Verifiable Credentials (see `Presentation Definitions `_) +Registered Verifiable Presentations may be at most 64 KiB. This is a fixed limit of the Nuts node, not configurable per service definition. +Legitimate presentations are a few KB (about 16 KB when they carry an ``X509Credential`` with a full certificate chain), the limit keeps a single participant from inflating the service's responses. + For details see `Nuts RFC022 `_. \ No newline at end of file diff --git a/docs/pages/release_notes.rst b/docs/pages/release_notes.rst index ecb28524d0..bec19130ab 100644 --- a/docs/pages/release_notes.rst +++ b/docs/pages/release_notes.rst @@ -2,6 +2,13 @@ Release notes ############# +**************** +Unreleased +**************** + +## Security +- #4596: Discovery Service: registered Verifiable Presentations are now limited to 64 KiB, and the Discovery Service client reads responses of up to 10 MiB from the (operator-configured) Discovery Server instead of the 1 MiB applied to other outbound HTTP calls. Previously a client could no longer synchronize a service whose response exceeded 1 MiB, which a few hundred registrations or a couple of deliberately padded ones could cause. By @reinkrul in https://github.com/nuts-foundation/nuts-node/pull/4597 + **************** Peanut (v6.2.14) **************** diff --git a/http/client/client.go b/http/client/client.go index a6e6e0c7c4..743577cfc9 100644 --- a/http/client/client.go +++ b/http/client/client.go @@ -90,26 +90,47 @@ func checkRedirect(req *http.Request, via []*http.Request) error { // This of course heavily depends on the use case, but 1MB is a reasonable default. const DefaultMaxHttpResponseSize = 1024 * 1024 -// limitedReadAll reads the given reader until the DefaultMaxHttpResponseSize is reached. -// It returns an error if more data is available than DefaultMaxHttpResponseSize. -func limitedReadAll(reader io.Reader) ([]byte, error) { - result, err := io.ReadAll(io.LimitReader(reader, DefaultMaxHttpResponseSize+1)) - if len(result) > DefaultMaxHttpResponseSize { - return nil, fmt.Errorf("data to read exceeds max. safety limit of %d bytes", DefaultMaxHttpResponseSize) +// limitedReadAll reads the given reader until the given limit is reached. +// It returns an error if more data is available than the limit. +func limitedReadAll(reader io.Reader, limit int64) ([]byte, error) { + result, err := io.ReadAll(io.LimitReader(reader, limit+1)) + if int64(len(result)) > limit { + return nil, fmt.Errorf("data to read exceeds max. safety limit of %d bytes", limit) } return result, err } +// Option configures a StrictHTTPClient. +type Option func(*StrictHTTPClient) + +// WithMaxResponseSize overrides the maximum HTTP response body size (in bytes) the client reads. +// The default is DefaultMaxHttpResponseSize. Only raise it for endpoints that are operator-configured +// and known to return large responses, since the response is buffered in memory. +func WithMaxResponseSize(size int64) Option { + return func(client *StrictHTTPClient) { + client.maxResponseSize = size + } +} + // New creates a new HTTP client with the given timeout. -func New(timeout time.Duration) *StrictHTTPClient { +func New(timeout time.Duration, options ...Option) *StrictHTTPClient { transport := getTransport(SafeHttpTransport) - return &StrictHTTPClient{ - client: &http.Client{ - Transport: transport, - Timeout: timeout, - CheckRedirect: checkRedirect, - }, + return newStrictHTTPClient(&http.Client{ + Transport: transport, + Timeout: timeout, + CheckRedirect: checkRedirect, + }, options) +} + +func newStrictHTTPClient(httpClient *http.Client, options []Option) *StrictHTTPClient { + result := &StrictHTTPClient{ + client: httpClient, + maxResponseSize: DefaultMaxHttpResponseSize, + } + for _, option := range options { + option(result) } + return result } // getTransport wraps the given transport with OpenTelemetry instrumentation if tracing is enabled. @@ -125,34 +146,31 @@ func getTransport(base http.RoundTripper) http.RoundTripper { // NewWithCache creates a new HTTP client with the given timeout. // It uses the DefaultCachingTransport as the underlying transport. -func NewWithCache(timeout time.Duration) *StrictHTTPClient { +func NewWithCache(timeout time.Duration, options ...Option) *StrictHTTPClient { transport := getTransport(DefaultCachingTransport) - return &StrictHTTPClient{ - client: &http.Client{ - Transport: transport, - Timeout: timeout, - CheckRedirect: checkRedirect, - }, - } + return newStrictHTTPClient(&http.Client{ + Transport: transport, + Timeout: timeout, + CheckRedirect: checkRedirect, + }, options) } // NewWithTLSConfig creates a new HTTP client with the given timeout and TLS configuration. // It copies the http.DefaultTransport and sets the TLSClientConfig to the given tls.Config. // As such, it can't be used in conjunction with the CachingRoundTripper. -func NewWithTLSConfig(timeout time.Duration, tlsConfig *tls.Config) *StrictHTTPClient { +func NewWithTLSConfig(timeout time.Duration, tlsConfig *tls.Config, options ...Option) *StrictHTTPClient { transport := SafeHttpTransport.Clone() transport.TLSClientConfig = tlsConfig - return &StrictHTTPClient{ - client: &http.Client{ - Transport: getTransport(transport), - Timeout: timeout, - CheckRedirect: checkRedirect, - }, - } + return newStrictHTTPClient(&http.Client{ + Transport: getTransport(transport), + Timeout: timeout, + CheckRedirect: checkRedirect, + }, options) } type StrictHTTPClient struct { - client *http.Client + client *http.Client + maxResponseSize int64 } func (s *StrictHTTPClient) Do(req *http.Request) (*http.Response, error) { @@ -165,7 +183,7 @@ func (s *StrictHTTPClient) Do(req *http.Request) (*http.Response, error) { return nil, err } if result.Body != nil { - body, err := limitedReadAll(result.Body) + body, err := limitedReadAll(result.Body, s.maxResponseSize) if err != nil { return nil, err } diff --git a/http/client/client_test.go b/http/client/client_test.go index 3ab8539136..a7abed884a 100644 --- a/http/client/client_test.go +++ b/http/client/client_test.go @@ -21,6 +21,7 @@ package client import ( "crypto/tls" "fmt" + "io" "net/http" "net/http/httptest" "strings" @@ -163,20 +164,52 @@ func TestStrictHTTPClient_RedirectScheme(t *testing.T) { func TestLimitedReadAll(t *testing.T) { t.Run("less than limit", func(t *testing.T) { data := strings.Repeat("a", 10) - result, err := limitedReadAll(strings.NewReader(data)) + result, err := limitedReadAll(strings.NewReader(data), DefaultMaxHttpResponseSize) assert.NoError(t, err) assert.Equal(t, []byte(data), result) }) t.Run("more than limit", func(t *testing.T) { data := strings.Repeat("a", DefaultMaxHttpResponseSize+1) - result, err := limitedReadAll(strings.NewReader(data)) + result, err := limitedReadAll(strings.NewReader(data), DefaultMaxHttpResponseSize) assert.EqualError(t, err, "data to read exceeds max. safety limit of 1048576 bytes") assert.Nil(t, result) }) } +func TestWithMaxResponseSize(t *testing.T) { + body := strings.Repeat("a", DefaultMaxHttpResponseSize+1) + server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, _ *http.Request) { + _, _ = writer.Write([]byte(body)) + })) + t.Cleanup(server.Close) + + t.Run("default limit rejects response", func(t *testing.T) { + request, _ := http.NewRequest(http.MethodGet, server.URL, nil) + + _, err := New(time.Second).Do(request) + + assert.EqualError(t, err, "data to read exceeds max. safety limit of 1048576 bytes") + }) + t.Run("raised limit accepts response", func(t *testing.T) { + request, _ := http.NewRequest(http.MethodGet, server.URL, nil) + + response, err := New(time.Second, WithMaxResponseSize(2*DefaultMaxHttpResponseSize)).Do(request) + + require.NoError(t, err) + data, _ := io.ReadAll(response.Body) + assert.Len(t, data, len(body)) + }) + t.Run("applies to all constructors", func(t *testing.T) { + option := WithMaxResponseSize(123) + assert.Equal(t, int64(123), New(time.Second, option).maxResponseSize) + assert.Equal(t, int64(123), NewWithCache(time.Second, option).maxResponseSize) + assert.Equal(t, int64(123), NewWithTLSConfig(time.Second, &tls.Config{}, option).maxResponseSize) + assert.Equal(t, int64(DefaultMaxHttpResponseSize), New(time.Second).maxResponseSize) + }) +} + func TestMaxConns(t *testing.T) { oldStrictMode := StrictMode StrictMode = false