fix(security): enhance timeout configurations and body size limits fo… #3984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

andaaron merged 13 commits into project-zot:main from rchincha:sec-fix

Apr 26, 2026

pkg/api/config/config.go

-Original file line number
+Diff line change
@@ Expand Up / @@ -376,10 +376,18 @@ type RatelimitConfig struct { @@
     //nolint:maligned
     type HTTPConfig struct {
-    	Address       string
-    	ExternalURL   string `mapstructure:",omitempty"`
-    	Port          string
-    	AllowOrigin   string // comma separated
+    	Address     string
+    	ExternalURL string `mapstructure:",omitempty"`
+    	Port        string
+    	AllowOrigin string // comma separated
+    	// ReadTimeout controls maximum duration for reading the entire request (including body).
+    	// When unset (nil), server-level defaults may apply. When explicitly set to <= 0,
+    	// the HTTP server treats it as no timeout.
+    	ReadTimeout *time.Duration `mapstructure:"readTimeout,omitempty"`
+    	// WriteTimeout controls maximum duration before timing out response writes.
+    	// When unset (nil), server-level defaults may apply. When explicitly set to <= 0,
+    	// the HTTP server treats it as no timeout.
+    	WriteTimeout  *time.Duration `mapstructure:"writeTimeout,omitempty"`
     	TLS           *TLSConfig
     	Auth          *AuthConfig
     	AccessControl *AccessControlConfig `mapstructure:"accessControl,omitempty"`
@@ Expand Down Expand Up / @@ -661,8 +669,12 @@ func New() *Config { @@
     				Retention:  ImageRetention{},
     			},
     		},
-    		HTTP: HTTPConfig{Address: "127.0.0.1", Port: "8080", Auth: &AuthConfig{FailDelay: 0}},
-    		Log:  &LogConfig{Level: "debug"},
+    		HTTP: HTTPConfig{
+    			Address: "127.0.0.1",
+    			Port:    "8080",
+    			Auth:    &AuthConfig{FailDelay: 0},
+    		},
+    		Log: &LogConfig{Level: "debug"},
     	}
     }
@@ Expand Down Expand Up / @@ -1117,6 +1129,38 @@ func (c *Config) GetHTTPPort() string { @@
     	return c.HTTP.Port
     }
+    // GetHTTPReadTimeout returns the configured HTTP server read timeout.
+    func (c *Config) GetHTTPReadTimeout() time.Duration {
+    	if c == nil {
+    		return 0
+    	}
+    	c.mu.RLock()
+    	defer c.mu.RUnlock()
+    	if c.HTTP.ReadTimeout == nil {
+    		return 0
+    	}
+    	return *c.HTTP.ReadTimeout
+    }
+    // GetHTTPWriteTimeout returns the configured HTTP server write timeout.
+    func (c *Config) GetHTTPWriteTimeout() time.Duration {
+    	if c == nil {
+    		return 0
+    	}
+    	c.mu.RLock()
+    	defer c.mu.RUnlock()
+    	if c.HTTP.WriteTimeout == nil {
+    		return 0
+    	}
+    	return *c.HTTP.WriteTimeout
+    }
     // GetAllowOrigin returns the CORS allow origin configuration.
     func (c *Config) GetAllowOrigin() string {
     	if c == nil {
@@ Expand Down @@

pkg/api/config/config_test.go

-Original file line number
+Diff line change
@@ Expand Up / @@ -3364,3 +3364,41 @@ func TestConfig(t *testing.T) { @@
     		})
     	})
     }
+    func TestHTTPTimeoutAccessors(t *testing.T) {
+    	Convey("GetHTTPReadTimeout returns configured values", t, func() {
+    		cfg := config.New()
+    		So(cfg.GetHTTPReadTimeout(), ShouldEqual, 0)
+    		zero := time.Duration(0)
+    		cfg.HTTP.ReadTimeout = &zero
+    		So(cfg.GetHTTPReadTimeout(), ShouldEqual, 0)
+    		negative := -5 * time.Second
+    		cfg.HTTP.ReadTimeout = &negative
+    		So(cfg.GetHTTPReadTimeout(), ShouldEqual, negative)
+    		positive := 45 * time.Second
+    		cfg.HTTP.ReadTimeout = &positive
+    		So(cfg.GetHTTPReadTimeout(), ShouldEqual, positive)
+    	})
+    	Convey("GetHTTPWriteTimeout returns configured values", t, func() {
+    		cfg := config.New()
+    		So(cfg.GetHTTPWriteTimeout(), ShouldEqual, 0)
+    		zero := time.Duration(0)
+    		cfg.HTTP.WriteTimeout = &zero
+    		So(cfg.GetHTTPWriteTimeout(), ShouldEqual, 0)
+    		negative := -5 * time.Second
+    		cfg.HTTP.WriteTimeout = &negative
+    		So(cfg.GetHTTPWriteTimeout(), ShouldEqual, negative)
+    		positive := 1 * time.Minute
+    		cfg.HTTP.WriteTimeout = &positive
+    		So(cfg.GetHTTPWriteTimeout(), ShouldEqual, positive)
+    	})
+    }

pkg/api/constants/consts.go

-Original file line number
+Diff line change
@@ Expand Up / @@ -24,7 +24,9 @@ const ( @@
     	// OCI manifest JSON is always small metadata; 4 MiB is well above any realistic manifest.
     	MaxManifestBodySize = 4 * 1024 * 1024
     	// MaxAPIKeyBodySize is the maximum number of bytes accepted for an API-key creation request body.
-    	MaxAPIKeyBodySize            = 4 * 1024
+    	MaxAPIKeyBodySize = 8 * 1024
+    	// MaxImageTrustBodySize is the maximum number of bytes accepted for image-trust key/certificate uploads.
+    	MaxImageTrustBodySize        = 8 * 1024 * 1024
     	BlobUploadUUID               = "Blob-Upload-UUID"
     	DefaultMediaType             = "application/json"
     	BinaryMediaType              = "application/octet-stream"
@@ Expand Down @@

pkg/api/controller.go

-Original file line number
+Diff line change
@@ Expand Up / @@ -172,9 +172,12 @@ func (c *Controller) Run() error { @@
     	port := c.Config.GetHTTPPort()
     	addr := fmt.Sprintf("%s:%s", c.Config.GetHTTPAddress(), port)
     	server := &http.Server{
     		Addr:              addr,
     		Handler:           c.Router,
+    		ReadTimeout:       c.Config.GetHTTPReadTimeout(),
+    		WriteTimeout:      c.Config.GetHTTPWriteTimeout(),
     		IdleTimeout:       idleTimeout,
     		ReadHeaderTimeout: readHeaderTimeout,
     	}
@@ Expand Down @@

pkg/api/proxy.go

-Original file line number
+Diff line change
@@ -1,7 +1,6 @@
     package api
     import (
-    	"bytes"
     	"context"
     	"fmt"
     	"io"
@@ Expand Down Expand Up / @@ -131,15 +130,28 @@ @@
     	cloneURL.Scheme = proxyQueryScheme
     	cloneURL.Host = targetMember
-    	clonedBody := cloneRequestBody(req)
+    	requestBody := io.Reader(http.NoBody)
+    	if req.Body != nil {
+    		requestBody = req.Body
+    	}
-    	fwdRequest, err := http.NewRequestWithContext(ctx, req.Method, cloneURL.String(), clonedBody)
+    	fwdRequest, err := http.NewRequestWithContext(ctx, req.Method, cloneURL.String(), requestBody)
     	if err != nil {
     		return nil, err
     	}
     	copyHeader(fwdRequest.Header, req.Header)
+    	// Preserve ContentLength from original request, including explicit zero-length
+    	// bodies, so empty requests are not forwarded as unknown-length chunked bodies.
+    	if req.ContentLength >= 0 {
+    		fwdRequest.ContentLength = req.ContentLength
+    		if req.ContentLength == 0 {
+    			fwdRequest.Body = http.NoBody
+    		}
+    	}
     	// always set hop count to 1 for now.
     	// the handler wrapper above will terminate the process if it sees a request that
     	// already has a hop count but is due for proxying.
@@ Expand All / @@ -156,7 +168,7 @@ @@
     	tlsConfig := clusterConfig.TLS
     	if tlsConfig != nil {
     		clientOpts.CertOptions.ClientCertFile = tlsConfig.Cert
     		clientOpts.CertOptions.ClientKeyFile = tlsConfig.Key
     		clientOpts.CertOptions.RootCaCertFile = tlsConfig.CACert
     	}
@@ Expand All / @@ -171,42 +183,9 @@ @@
     		return nil, err
     	}
-    	var clonedRespBody bytes.Buffer
-    	// copy out the contents into a new buffer as the response body
-    	// stream should be closed to get all the data out.
-    	_, _ = io.Copy(&clonedRespBody, resp.Body)
-    	resp.Body.Close()
-    	// after closing the original body, substitute it with a new reader
-    	// using the buffer that was just created.
-    	// this buffer should be closed later by the consumer of the response.
-    	resp.Body = io.NopCloser(bytes.NewReader(clonedRespBody.Bytes()))
     	return resp, nil
     }
-    func cloneRequestBody(src *http.Request) io.Reader {
-    	var bCloneForOriginal, bCloneForCopy bytes.Buffer
-    	multiWriter := io.MultiWriter(&bCloneForOriginal, &bCloneForCopy)
-    	numBytesCopied, _ := io.Copy(multiWriter, src.Body)
-    	// if the body is a type of io.NopCloser and length is 0,
-    	// the Content-Length header is not sent in the proxied request.
-    	// explicitly returning http.NoBody allows the implementation
-    	// to set the header.
-    	// ref: https://github.com/golang/go/issues/34295
-    	if numBytesCopied == 0 {
-    		src.Body = http.NoBody
-    		return http.NoBody
-    	}
-    	src.Body = io.NopCloser(&bCloneForOriginal)
-    	return bytes.NewReader(bCloneForCopy.Bytes())
-    }
     func copyHeader(dst, src http.Header) {
     	for k, vv := range src {
     		for _, v := range vv {
@@ Expand Down @@

pkg/api/proxy_internal_test.go

-Original file line number
+Diff line change
@@ -0,0 +1,113 @@
+    package api
+    import (
+    	"context"
+    	"io"
+    	"net/http"
+    	"net/http/httptest"
+    	"net/url"
+    	"strings"
+    	"testing"
+    	. "github.com/smartystreets/goconvey/convey"
+    	"zotregistry.dev/zot/v2/pkg/api/config"
+    	"zotregistry.dev/zot/v2/pkg/api/constants"
+    )
+    func TestProxyHTTPRequestStreamsBodyAndResponse(t *testing.T) {
+    	Convey("proxyHTTPRequest forwards request body/headers and returns streamed response", t, func() {
+    		requestPayload := strings.Repeat("payload-", 1024)
+    		responsePayload := strings.Repeat("response-", 2048)
+    		type backendResult struct {
+    			body     string
+    			hopCount string
+    			err      error
+    		}
+    		resultCh := make(chan backendResult, 1)
+    		backend := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+    			body, err := io.ReadAll(request.Body)
+    			resultCh <- backendResult{
+    				body:     string(body),
+    				hopCount: request.Header.Get(constants.ScaleOutHopCountHeader),
+    				err:      err,
+    			}
+    			response.WriteHeader(http.StatusCreated)
+    			_, _ = io.WriteString(response, responsePayload)
+    		}))
+    		defer backend.Close()
+    		backendURL, err := url.Parse(backend.URL)
+    		So(err, ShouldBeNil)
+    		conf := config.New()
+    		conf.Cluster = &config.ClusterConfig{Members: []string{backendURL.Host}, HashKey: "loremipsumdolors"}
+    		ctrlr := &Controller{Config: conf}
+    		req, err := http.NewRequestWithContext(context.Background(), http.MethodPut,
+    			"http://example.com/v2/repo/manifests/latest", strings.NewReader(requestPayload))
+    		So(err, ShouldBeNil)
+    		resp, err := proxyHTTPRequest(context.Background(), req, backendURL.Host, ctrlr)
+    		So(err, ShouldBeNil)
+    		So(resp, ShouldNotBeNil)
+    		defer resp.Body.Close()
+    		respBody, err := io.ReadAll(resp.Body)
+    		So(err, ShouldBeNil)
+    		result := <-resultCh
+    		So(result.err, ShouldBeNil)
+    		remainingReqBody, err := io.ReadAll(req.Body)
+    		So(err, ShouldBeNil)
+    		So(resp.StatusCode, ShouldEqual, http.StatusCreated)
+    		So(string(respBody), ShouldEqual, responsePayload)
+    		So(result.body, ShouldEqual, requestPayload)
+    		So(result.hopCount, ShouldEqual, "1")
+    		So(len(remainingReqBody), ShouldEqual, 0)
+    	})
+    }
+    func TestProxyHTTPRequestPreservesExplicitEmptyBody(t *testing.T) {
+    	Convey("proxyHTTPRequest preserves explicit zero-length request bodies", t, func() {
+    		resultCh := make(chan *http.Request, 1)
+    		backend := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+    			resultCh <- request
+    			response.WriteHeader(http.StatusNoContent)
+    		}))
+    		defer backend.Close()
+    		backendURL, err := url.Parse(backend.URL)
+    		So(err, ShouldBeNil)
+    		conf := config.New()
+    		conf.Cluster = &config.ClusterConfig{Members: []string{backendURL.Host}, HashKey: "loremipsumdolors"}
+    		ctrlr := &Controller{Config: conf}
+    		req, err := http.NewRequestWithContext(context.Background(), http.MethodPost,
+    			"http://example.com/v2/repo/manifests/latest", http.NoBody)
+    		So(err, ShouldBeNil)
+    		So(req.ContentLength, ShouldEqual, 0)
+    		resp, err := proxyHTTPRequest(context.Background(), req, backendURL.Host, ctrlr)
+    		So(err, ShouldBeNil)
+    		So(resp, ShouldNotBeNil)
+    		defer resp.Body.Close()
+    		backendReq := <-resultCh
+    		So(resp.StatusCode, ShouldEqual, http.StatusNoContent)
+    		So(backendReq.ContentLength, ShouldEqual, 0)
+    		So(backendReq.Body, ShouldEqual, http.NoBody)
+    		So(backendReq.TransferEncoding, ShouldBeEmpty)
+    	})
+    }

pkg/cli/server/root.go

-Original file line number
+Diff line change
@@ Expand Up / @@ -33,6 +33,11 @@ import ( @@
     	storageConstants "zotregistry.dev/zot/v2/pkg/storage/constants"
     )
+    const (
+    	defaultReadTimeout  = 60 * time.Second
+    	defaultWriteTimeout = 60 * time.Second
+    )
     // metadataConfig reports metadata after parsing, which we use to track
     // errors.
     func metadataConfig(md *mapstructure.Metadata) viper.DecoderConfigOption {
@@ Expand Down Expand Up @@
     		config.Storage.SubPaths[name] = storageConfig
     	}
+    	if config.HTTP.ReadTimeout == nil {
+    		readTimeout := defaultReadTimeout
+    		config.HTTP.ReadTimeout = &readTimeout
+    	}
+    	if config.HTTP.WriteTimeout == nil {
+    		writeTimeout := defaultWriteTimeout
+    		config.HTTP.WriteTimeout = &writeTimeout
+    	}
     	// if OpenID authentication is enabled,
     	// API Keys are also enabled in order to provide data path authentication
     	if config.HTTP.Auth != nil && config.HTTP.Auth.OpenID != nil {
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(security): enhance timeout configurations and body size limits fo… #3984

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Check failure

Uh oh!

Uh oh!

Uh oh!