diff --git a/CHANGELOG.md b/CHANGELOG.md index d2eb0db0fce..7d1a977e9f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Return spec-compliant `TraceIdRatioBased` description. This is a breaking behavioral change, but it is necessary to make the implementation [spec-compliant](https://opentelemetry.io/docs/specs/otel/trace/sdk/#traceidratiobased). (#8027) - Fix a race condition in `go.opentelemetry.io/otel/sdk/metric` where the lastvalue aggregation could collect the value 0 even when no zero-value measurements were recorded. (#8056) +- Limit HTTP response body to 4 MiB in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`, `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`, and `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` to mitigate excessive memory usage caused by a misconfigured or malicious server. Responses exceeding the limit are treated as non-retryable errors. (#8108) - `WithHostID` detector in `go.opentelemetry.io/otel/sdk/resource` to use full path for `kenv` command on BSD. (#8113) - Fix missing `request.GetBody` in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` to correctly handle HTTP2 GOAWAY frame. (#8096) diff --git a/exporters/otlp/otlplog/otlploghttp/client.go b/exporters/otlp/otlplog/otlploghttp/client.go index 2a25e3db4bb..8272d1a543a 100644 --- a/exporters/otlp/otlplog/otlploghttp/client.go +++ b/exporters/otlp/otlplog/otlploghttp/client.go @@ -47,6 +47,13 @@ var exporterN atomic.Int64 var errInsecureEndpointWithTLS = errors.New("insecure HTTP endpoint cannot use TLS client configuration") +// maxResponseBodySize is the maximum number of bytes to read from a response +// body. It is set to 4 MiB per the OTLP specification recommendation to +// mitigate excessive memory usage caused by a misconfigured or malicious +// server. If exceeded, the response is treated as a not-retryable error. +// This is a variable to allow tests to override it. +var maxResponseBodySize int64 = 4 * 1024 * 1024 + // nextExporterID returns the next unique ID for an exporter. func nextExporterID() int64 { const inc = 1 @@ -194,7 +201,11 @@ func (c *httpClient) uploadLogs(ctx context.Context, data []*logpb.ResourceLogs) // Read the partial success message, if any. var respData bytes.Buffer - if _, err := io.Copy(&respData, resp.Body); err != nil { + if _, err := io.Copy(&respData, http.MaxBytesReader(nil, resp.Body, maxResponseBodySize)); err != nil { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { + return fmt.Errorf("response body too large: exceeded %d bytes", maxBytesErr.Limit) + } return err } if respData.Len() == 0 { @@ -225,7 +236,11 @@ func (c *httpClient) uploadLogs(ctx context.Context, data []*logpb.ResourceLogs) // message to be returned. It will help in // debugging the actual issue. var respData bytes.Buffer - if _, err := io.Copy(&respData, resp.Body); err != nil { + if _, err := io.Copy(&respData, http.MaxBytesReader(nil, resp.Body, maxResponseBodySize)); err != nil { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { + return fmt.Errorf("response body too large: exceeded %d bytes", maxBytesErr.Limit) + } return err } respStr := strings.TrimSpace(respData.String()) diff --git a/exporters/otlp/otlplog/otlploghttp/client_test.go b/exporters/otlp/otlplog/otlploghttp/client_test.go index c66c99745fb..12c122214d2 100644 --- a/exporters/otlp/otlplog/otlploghttp/client_test.go +++ b/exporters/otlp/otlplog/otlploghttp/client_test.go @@ -1017,6 +1017,58 @@ func TestClientInstrumentation(t *testing.T) { metricdatatest.AssertEqual(t, want, got.ScopeMetrics[0], opt...) } +func TestResponseBodySizeLimit(t *testing.T) { + // Override the limit to 1 byte so any non-empty response body exceeds it. + orig := maxResponseBodySize + maxResponseBodySize = 1 + t.Cleanup(func() { maxResponseBodySize = orig }) + + // largeBody is larger than the 1-byte limit. + largeBody := []byte("xx") + + tests := []struct { + name string + status int + contentType string + }{ + { + name: "success response body too large", + status: http.StatusOK, + contentType: "application/x-protobuf", + }, + { + name: "error response body too large", + status: http.StatusServiceUnavailable, + contentType: "text/plain", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var calls int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls++ + w.Header().Set("Content-Type", tc.contentType) + w.WriteHeader(tc.status) + _, _ = w.Write(largeBody) + })) + t.Cleanup(srv.Close) + + opts := []Option{ + WithEndpoint(srv.Listener.Addr().String()), + WithInsecure(), + WithRetry(RetryConfig{Enabled: false}), + } + cfg := newConfig(opts) + c, err := newHTTPClient(t.Context(), cfg) + require.NoError(t, err) + + err = c.UploadLogs(t.Context(), make([]*lpb.ResourceLogs, 1)) + assert.ErrorContains(t, err, "response body too large") + assert.Equal(t, 1, calls, "request must not be retried after body-too-large error") + }) + } +} + func BenchmarkExporterExportLogs(b *testing.B) { const n = 10 diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go index e85c10c0e83..1958f9d1b37 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go @@ -54,6 +54,13 @@ var ourTransport = &http.Transport{ var errInsecureEndpointWithTLS = errors.New("insecure HTTP endpoint cannot use TLS client configuration") +// maxResponseBodySize is the maximum number of bytes to read from a response +// body. It is set to 4 MiB per the OTLP specification recommendation to +// mitigate excessive memory usage caused by a misconfigured or malicious +// server. If exceeded, the response is treated as a not-retryable error. +// This is a variable to allow tests to override it. +var maxResponseBodySize int64 = 4 * 1024 * 1024 + // newClient creates a new HTTP metric client. func newClient(cfg oconf.Config) (*client, error) { if cfg.Metrics.Insecure && cfg.Metrics.TLSCfg != nil { @@ -174,7 +181,11 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou // Read the partial success message, if any. var respData bytes.Buffer - if _, err := io.Copy(&respData, resp.Body); err != nil { + if _, err := io.Copy(&respData, http.MaxBytesReader(nil, resp.Body, maxResponseBodySize)); err != nil { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { + return fmt.Errorf("response body too large: exceeded %d bytes", maxBytesErr.Limit) + } return err } if respData.Len() == 0 { @@ -205,7 +216,11 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou // message to be returned. It will help in // debugging the actual issue. var respData bytes.Buffer - if _, err := io.Copy(&respData, resp.Body); err != nil { + if _, err := io.Copy(&respData, http.MaxBytesReader(nil, resp.Body, maxResponseBodySize)); err != nil { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { + return fmt.Errorf("response body too large: exceeded %d bytes", maxBytesErr.Limit) + } return err } respStr := strings.TrimSpace(respData.String()) diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go index 0a622c775a3..d2681564858 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go @@ -379,3 +379,56 @@ func TestGetBodyCalledOnRedirect(t *testing.T) { assert.NotEmpty(t, requestBodies[0], "original request body should not be empty") assert.Equal(t, requestBodies[0], requestBodies[1], "redirect body should match original") } + +func TestResponseBodySizeLimit(t *testing.T) { + // Override the limit to 1 byte so any non-empty response body exceeds it. + orig := maxResponseBodySize + maxResponseBodySize = 1 + t.Cleanup(func() { maxResponseBodySize = orig }) + + // largeBody is larger than the 1-byte limit. + largeBody := []byte("xx") + + tests := []struct { + name string + status int + contentType string + }{ + { + name: "success response body too large", + status: http.StatusOK, + contentType: "application/x-protobuf", + }, + { + name: "error response body too large", + status: http.StatusServiceUnavailable, + contentType: "text/plain", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var calls int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls++ + w.Header().Set("Content-Type", tc.contentType) + w.WriteHeader(tc.status) + _, _ = w.Write(largeBody) + })) + t.Cleanup(srv.Close) + + opts := []Option{ + WithEndpoint(srv.Listener.Addr().String()), + WithInsecure(), + WithRetry(RetryConfig{Enabled: false}), + } + cfg := oconf.NewHTTPConfig(asHTTPOptions(opts)...) + c, err := newClient(cfg) + require.NoError(t, err) + t.Cleanup(func() { _ = c.Shutdown(t.Context()) }) + + err = c.UploadMetrics(t.Context(), &mpb.ResourceMetrics{}) + assert.ErrorContains(t, err, "response body too large") + assert.Equal(t, 1, calls, "request must not be retried after body-too-large error") + }) + } +} diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go index ef1665e8e38..4ae569ff4b1 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/client.go +++ b/exporters/otlp/otlptrace/otlptracehttp/client.go @@ -32,6 +32,13 @@ import ( const contentTypeProto = "application/x-protobuf" +// maxResponseBodySize is the maximum number of bytes to read from a response +// body. It is set to 4 MiB per the OTLP specification recommendation to +// mitigate excessive memory usage caused by a misconfigured or malicious +// server. If exceeded, the response is treated as a not-retryable error. +// This is a variable to allow tests to override it. +var maxResponseBodySize int64 = 4 * 1024 * 1024 + var gzPool = sync.Pool{ New: func() any { w := gzip.NewWriter(io.Discard) @@ -203,7 +210,11 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc // Success, do not retry. // Read the partial success message, if any. var respData bytes.Buffer - if _, err := io.Copy(&respData, resp.Body); err != nil { + if _, err := io.Copy(&respData, http.MaxBytesReader(nil, resp.Body, maxResponseBodySize)); err != nil { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { + return fmt.Errorf("response body too large: exceeded %d bytes", maxBytesErr.Limit) + } return err } if respData.Len() == 0 { @@ -234,7 +245,11 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc // message to be returned. It will help in // debugging the actual issue. var respData bytes.Buffer - if _, err := io.Copy(&respData, resp.Body); err != nil { + if _, err := io.Copy(&respData, http.MaxBytesReader(nil, resp.Body, maxResponseBodySize)); err != nil { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { + return fmt.Errorf("response body too large: exceeded %d bytes", maxBytesErr.Limit) + } return err } respStr := strings.TrimSpace(respData.String()) diff --git a/exporters/otlp/otlptrace/otlptracehttp/client_test.go b/exporters/otlp/otlptrace/otlptracehttp/client_test.go index 62b241bbac9..5918c627f5f 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/client_test.go +++ b/exporters/otlp/otlptrace/otlptracehttp/client_test.go @@ -597,6 +597,58 @@ func TestClientInstrumentation(t *testing.T) { metricdatatest.AssertEqual(t, want, got.ScopeMetrics[0], opt...) } +func TestResponseBodySizeLimit(t *testing.T) { + // Override the limit to 1 byte so any non-empty response body exceeds it. + orig := *otlptracehttp.MaxResponseBodySize + *otlptracehttp.MaxResponseBodySize = 1 + t.Cleanup(func() { *otlptracehttp.MaxResponseBodySize = orig }) + + // largeBody is larger than the 1-byte limit. + largeBody := []byte("xx") + + tests := []struct { + name string + status int + contentType string + }{ + { + name: "success response body too large", + status: http.StatusOK, + contentType: "application/x-protobuf", + }, + { + name: "error response body too large", + status: http.StatusServiceUnavailable, + contentType: "text/plain", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var calls int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls++ + w.Header().Set("Content-Type", tc.contentType) + w.WriteHeader(tc.status) + _, _ = w.Write(largeBody) + })) + t.Cleanup(srv.Close) + + client := otlptracehttp.NewClient( + otlptracehttp.WithEndpointURL(srv.URL), + otlptracehttp.WithInsecure(), + otlptracehttp.WithRetry(otlptracehttp.RetryConfig{Enabled: false}), + ) + exporter, err := otlptrace.New(t.Context(), client) + require.NoError(t, err) + t.Cleanup(func() { _ = exporter.Shutdown(t.Context()) }) + + err = exporter.ExportSpans(t.Context(), otlptracetest.SingleReadOnlySpan()) + assert.ErrorContains(t, err, "response body too large") + assert.Equal(t, 1, calls, "request must not be retried after body-too-large error") + }) + } +} + func TestGetBodyCalledOnRedirect(t *testing.T) { // Test that req.GetBody is set correctly, allowing the HTTP transport // to re-send the body on 307 redirects. diff --git a/exporters/otlp/otlptrace/otlptracehttp/export_test.go b/exporters/otlp/otlptrace/otlptracehttp/export_test.go new file mode 100644 index 00000000000..bbc3376f9b1 --- /dev/null +++ b/exporters/otlp/otlptrace/otlptracehttp/export_test.go @@ -0,0 +1,8 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package otlptracehttp + +// MaxResponseBodySize exposes the package-level maxResponseBodySize variable +// to allow tests to override it. +var MaxResponseBodySize = &maxResponseBodySize