From 45e0aeeb0851d73fd7a5a0b3482d2886f79e21f5 Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Tue, 4 Oct 2022 17:39:52 +0200 Subject: [PATCH 1/9] [config/config{grpc,http}] Add warning when using a 0.0.0.0 endpoint --- config/configgrpc/configgrpc.go | 14 ++++++++++++++ config/confighttp/confighttp.go | 15 +++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 13c198ade05..758eb171189 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "net" + "net/url" "strings" "time" @@ -27,6 +28,7 @@ import ( "github.com/mostynb/go-grpc-compression/zstd" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "go.opentelemetry.io/otel" + "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/balancer/roundrobin" "google.golang.org/grpc/credentials" @@ -273,6 +275,18 @@ func (gss *GRPCServerSettings) ToListener() (net.Listener, error) { // ToServerOption maps configgrpc.GRPCServerSettings to a slice of server options for gRPC. func (gss *GRPCServerSettings) ToServerOption(host component.Host, settings component.TelemetrySettings) ([]grpc.ServerOption, error) { + if endpointURL, err := url.Parse(gss.NetAddr.Endpoint); err != nil { + return nil, fmt.Errorf("failed to parse endpoint: %w", err) + } else if endpointURL.Hostname() == "0.0.0.0" { + settings.Logger.Warn( + "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", + zap.String( + "documentation", + "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security.md#safeguards-against-denial-of-service-attacks", + ), + ) + } + var opts []grpc.ServerOption if gss.TLSSetting != nil { diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index f5b49b7b7bc..0967c45e1de 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -17,13 +17,16 @@ package confighttp // import "go.opentelemetry.io/collector/config/confighttp" import ( "crypto/tls" "errors" + "fmt" "net" "net/http" + "net/url" "time" "github.com/rs/cors" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel" + "go.uber.org/zap" "golang.org/x/net/http2" "go.opentelemetry.io/collector/component" @@ -259,6 +262,18 @@ func WithErrorHandler(e errorHandler) ToServerOption { // ToServer creates an http.Server from settings object. func (hss *HTTPServerSettings) ToServer(host component.Host, settings component.TelemetrySettings, handler http.Handler, opts ...ToServerOption) (*http.Server, error) { + if endpointURL, err := url.Parse(hss.Endpoint); err != nil { + return nil, fmt.Errorf("failed to parse endpoint: %w", err) + } else if endpointURL.Hostname() == "0.0.0.0" { + settings.Logger.Warn( + "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", + zap.String( + "documentation", + "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security.md#safeguards-against-denial-of-service-attacks", + ), + ) + } + serverOpts := &toServerOptions{} for _, o := range opts { o(serverOpts) From 03d05f45d0f0f3c0d0b15e86e58b4c43a3d6a893 Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Mon, 10 Oct 2022 13:32:54 +0200 Subject: [PATCH 2/9] Add warning when using unspecified address --- config/configgrpc/configgrpc.go | 5 ++-- config/configgrpc/configgrpc_test.go | 44 ++++++++++++++++++++++++++++ config/confighttp/confighttp.go | 5 ++-- config/confighttp/confighttp_test.go | 41 ++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 6 deletions(-) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 758eb171189..e65b80ac3f1 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "net" - "net/url" "strings" "time" @@ -275,9 +274,9 @@ func (gss *GRPCServerSettings) ToListener() (net.Listener, error) { // ToServerOption maps configgrpc.GRPCServerSettings to a slice of server options for gRPC. func (gss *GRPCServerSettings) ToServerOption(host component.Host, settings component.TelemetrySettings) ([]grpc.ServerOption, error) { - if endpointURL, err := url.Parse(gss.NetAddr.Endpoint); err != nil { + if host, _, err := net.SplitHostPort(gss.NetAddr.Endpoint); err != nil { return nil, fmt.Errorf("failed to parse endpoint: %w", err) - } else if endpointURL.Hostname() == "0.0.0.0" { + } else if host == "0.0.0.0" || host == "::" { settings.Logger.Warn( "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", zap.String( diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index 4ac4dccb869..5cbc93188c8 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -26,6 +26,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" "google.golang.org/grpc" "google.golang.org/grpc/metadata" "google.golang.org/grpc/peer" @@ -370,6 +372,48 @@ func TestUseSecure(t *testing.T) { assert.Len(t, dialOpts, 3) } +func TestGRPCServerWarning(t *testing.T) { + tests := []struct { + name string + settings GRPCServerSettings + len int + }{ + { + settings: GRPCServerSettings{ + NetAddr: confignet.NetAddr{ + Endpoint: "0.0.0.0:1234", + Transport: "tcp", + }, + }, + len: 1, + }, + { + settings: GRPCServerSettings{ + NetAddr: confignet.NetAddr{ + Endpoint: "127.0.0.1:1234", + Transport: "tcp", + }, + }, + len: 0, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + set := componenttest.NewNopTelemetrySettings() + logger, observed := observer.New(zap.DebugLevel) + set.Logger = zap.New(logger) + + opts, err := test.settings.ToServerOption(componenttest.NewNopHost(), set) + require.NoError(t, err) + require.NotNil(t, opts) + _ = grpc.NewServer(opts...) + + require.Len(t, observed.FilterLevelExact(zap.WarnLevel).All(), test.len) + }) + } + +} + func TestGRPCServerSettingsError(t *testing.T) { tests := []struct { settings GRPCServerSettings diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 0967c45e1de..05dda5c9566 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -20,7 +20,6 @@ import ( "fmt" "net" "net/http" - "net/url" "time" "github.com/rs/cors" @@ -262,9 +261,9 @@ func WithErrorHandler(e errorHandler) ToServerOption { // ToServer creates an http.Server from settings object. func (hss *HTTPServerSettings) ToServer(host component.Host, settings component.TelemetrySettings, handler http.Handler, opts ...ToServerOption) (*http.Server, error) { - if endpointURL, err := url.Parse(hss.Endpoint); err != nil { + if host, _, err := net.SplitHostPort(hss.Endpoint); err != nil { return nil, fmt.Errorf("failed to parse endpoint: %w", err) - } else if endpointURL.Hostname() == "0.0.0.0" { + } else if host == "0.0.0.0" || host == "::" { settings.Logger.Warn( "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", zap.String( diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 4f7877a4299..86f181e6a3e 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -29,6 +29,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" "go.opentelemetry.io/collector/client" "go.opentelemetry.io/collector/component" @@ -392,6 +394,45 @@ func TestHTTPServerSettingsError(t *testing.T) { } } +func TestHTTPServerWarning(t *testing.T) { + tests := []struct { + name string + settings HTTPServerSettings + len int + }{ + { + settings: HTTPServerSettings{ + Endpoint: "0.0.0.0:0", + }, + len: 1, + }, + { + settings: HTTPServerSettings{ + Endpoint: "127.0.0.1:0", + }, + len: 0, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + set := componenttest.NewNopTelemetrySettings() + logger, observed := observer.New(zap.DebugLevel) + set.Logger = zap.New(logger) + + _, err := test.settings.ToServer( + componenttest.NewNopHost(), + set, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, errWrite := fmt.Fprint(w, "test") + assert.NoError(t, errWrite) + })) + require.NoError(t, err) + require.Len(t, observed.FilterLevelExact(zap.WarnLevel).All(), test.len) + }) + } + +} + func TestHttpReception(t *testing.T) { tests := []struct { name string From 6254968feb897000fd68dd4f588c5bea7630f93f Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Mon, 10 Oct 2022 13:36:24 +0200 Subject: [PATCH 3/9] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6259e8e7a0d..50773186aba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ - Add config marshaler (#5566) - Add semantic conventions for specification v1.10-v1.13 (#6213) +- Add warning when using unspecified (`0.0.0.0`) address on HTTP or gRPC servers (#6267) ## v0.61.0 Beta From e618a345b0aaf9e8b4994f71c7a834af81e145a7 Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Mon, 10 Oct 2022 18:16:39 +0200 Subject: [PATCH 4/9] Fix tests --- config/configgrpc/configgrpc.go | 24 ++++++++++++++---------- config/configgrpc/configgrpc_test.go | 12 ++++++++++-- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index e65b80ac3f1..628da0cbb14 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -274,16 +274,20 @@ func (gss *GRPCServerSettings) ToListener() (net.Listener, error) { // ToServerOption maps configgrpc.GRPCServerSettings to a slice of server options for gRPC. func (gss *GRPCServerSettings) ToServerOption(host component.Host, settings component.TelemetrySettings) ([]grpc.ServerOption, error) { - if host, _, err := net.SplitHostPort(gss.NetAddr.Endpoint); err != nil { - return nil, fmt.Errorf("failed to parse endpoint: %w", err) - } else if host == "0.0.0.0" || host == "::" { - settings.Logger.Warn( - "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", - zap.String( - "documentation", - "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security.md#safeguards-against-denial-of-service-attacks", - ), - ) + + switch gss.NetAddr.Transport { + case "tcp", "tcp4", "tcp6", "udp", "udp4", "udp6": + if host, _, err := net.SplitHostPort(gss.NetAddr.Endpoint); err != nil { + return nil, fmt.Errorf("failed to parse endpoint: %w", err) + } else if host == "0.0.0.0" || host == "::" { + settings.Logger.Warn( + "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", + zap.String( + "documentation", + "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security.md#safeguards-against-denial-of-service-attacks", + ), + ) + } } var opts []grpc.ServerOption diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index 5cbc93188c8..987dc7864e2 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -164,7 +164,11 @@ func TestAllGrpcClientSettings(t *testing.T) { } func TestDefaultGrpcServerSettings(t *testing.T) { - gss := &GRPCServerSettings{} + gss := &GRPCServerSettings{ + NetAddr: confignet.NetAddr{ + Endpoint: "0.0.0.0:1234", + }, + } opts, err := gss.ToServerOption(componenttest.NewNopHost(), componenttest.NewNopTelemetrySettings()) _ = grpc.NewServer(opts...) @@ -208,7 +212,11 @@ func TestAllGrpcServerSettingsExceptAuth(t *testing.T) { } func TestGrpcServerAuthSettings(t *testing.T) { - gss := &GRPCServerSettings{} + gss := &GRPCServerSettings{ + NetAddr: confignet.NetAddr{ + Endpoint: "0.0.0.0:1234", + }, + } // sanity check _, err := gss.ToServerOption(componenttest.NewNopHost(), componenttest.NewNopTelemetrySettings()) From 6c1ddad884041ebc8e87674800cdfff3c4eb2e16 Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Tue, 11 Oct 2022 10:13:17 +0200 Subject: [PATCH 5/9] Fix HTTP tests --- config/confighttp/confighttp_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 86f181e6a3e..ae9b12e8be3 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -728,6 +728,7 @@ func TestHttpCorsInvalidSettings(t *testing.T) { func TestHttpCorsWithAuthentication(t *testing.T) { hss := &HTTPServerSettings{ + Endpoint: "localhost:0", CORS: &CORSSettings{ AllowedOrigins: []string{"*"}, }, @@ -925,6 +926,7 @@ func TestServerAuth(t *testing.T) { // prepare authCalled := false hss := HTTPServerSettings{ + Endpoint: "localhost:0", Auth: &configauth.Authentication{ AuthenticatorID: config.NewComponentID("mock"), }, @@ -972,6 +974,7 @@ func TestInvalidServerAuth(t *testing.T) { func TestFailedServerAuth(t *testing.T) { // prepare hss := HTTPServerSettings{ + Endpoint: "localhost:0", Auth: &configauth.Authentication{ AuthenticatorID: config.NewComponentID("mock"), }, From 993457ae9aea011ba14daeab0a70c0dbdb14be39 Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Tue, 18 Oct 2022 09:44:57 +0200 Subject: [PATCH 6/9] Apply suggestions from code review Co-authored-by: Alex Boten --- config/configgrpc/configgrpc.go | 2 +- config/confighttp/confighttp.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 628da0cbb14..f5c2879f4cc 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -284,7 +284,7 @@ func (gss *GRPCServerSettings) ToServerOption(host component.Host, settings comp "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", zap.String( "documentation", - "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security.md#safeguards-against-denial-of-service-attacks", + "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security-best-practices.md#safeguards-against-denial-of-service-attacks", ), ) } diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 05dda5c9566..f7b311bcf06 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -268,7 +268,7 @@ func (hss *HTTPServerSettings) ToServer(host component.Host, settings component. "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", zap.String( "documentation", - "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security.md#safeguards-against-denial-of-service-attacks", + "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security-best-practices.md#safeguards-against-denial-of-service-attacks", ), ) } From 7a7c430bb719ced4f1dc877bdbeda06f922849b6 Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Thu, 20 Oct 2022 13:57:13 +0200 Subject: [PATCH 7/9] Use IsUnspecified method --- config/configgrpc/configgrpc.go | 2 +- config/confighttp/confighttp.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index f5c2879f4cc..361ff0ee4af 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -279,7 +279,7 @@ func (gss *GRPCServerSettings) ToServerOption(host component.Host, settings comp case "tcp", "tcp4", "tcp6", "udp", "udp4", "udp6": if host, _, err := net.SplitHostPort(gss.NetAddr.Endpoint); err != nil { return nil, fmt.Errorf("failed to parse endpoint: %w", err) - } else if host == "0.0.0.0" || host == "::" { + } else if ip := net.ParseIP(host); ip != nil && ip.IsUnspecified() { settings.Logger.Warn( "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", zap.String( diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index f7b311bcf06..e0b3261e4ce 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -263,7 +263,7 @@ func WithErrorHandler(e errorHandler) ToServerOption { func (hss *HTTPServerSettings) ToServer(host component.Host, settings component.TelemetrySettings, handler http.Handler, opts ...ToServerOption) (*http.Server, error) { if host, _, err := net.SplitHostPort(hss.Endpoint); err != nil { return nil, fmt.Errorf("failed to parse endpoint: %w", err) - } else if host == "0.0.0.0" || host == "::" { + } else if ip := net.ParseIP(host); ip != nil && ip.IsUnspecified() { settings.Logger.Warn( "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", zap.String( From bbf6043cc764bf73305a1d9c874c688f7aa4c1ef Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Mon, 24 Oct 2022 15:30:43 +0200 Subject: [PATCH 8/9] no else after return --- config/configgrpc/configgrpc.go | 7 +++++-- config/confighttp/confighttp.go | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 361ff0ee4af..19ea767a460 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -277,9 +277,12 @@ func (gss *GRPCServerSettings) ToServerOption(host component.Host, settings comp switch gss.NetAddr.Transport { case "tcp", "tcp4", "tcp6", "udp", "udp4", "udp6": - if host, _, err := net.SplitHostPort(gss.NetAddr.Endpoint); err != nil { + host, _, err := net.SplitHostPort(gss.NetAddr.Endpoint) + if err != nil { return nil, fmt.Errorf("failed to parse endpoint: %w", err) - } else if ip := net.ParseIP(host); ip != nil && ip.IsUnspecified() { + } + + if ip := net.ParseIP(host); ip != nil && ip.IsUnspecified() { settings.Logger.Warn( "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", zap.String( diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index e0b3261e4ce..b1e2faf3e0c 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -261,9 +261,12 @@ func WithErrorHandler(e errorHandler) ToServerOption { // ToServer creates an http.Server from settings object. func (hss *HTTPServerSettings) ToServer(host component.Host, settings component.TelemetrySettings, handler http.Handler, opts ...ToServerOption) (*http.Server, error) { - if host, _, err := net.SplitHostPort(hss.Endpoint); err != nil { + endpointHost, _, err := net.SplitHostPort(hss.Endpoint) + if err != nil { return nil, fmt.Errorf("failed to parse endpoint: %w", err) - } else if ip := net.ParseIP(host); ip != nil && ip.IsUnspecified() { + } + + if ip := net.ParseIP(endpointHost); ip != nil && ip.IsUnspecified() { settings.Logger.Warn( "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", zap.String( From 385cb84eebdac527a4b4b169196a13b86037c21d Mon Sep 17 00:00:00 2001 From: Pablo Baeyens Date: Tue, 25 Oct 2022 12:03:31 +0200 Subject: [PATCH 9/9] Move shared code to internal --- config/configgrpc/configgrpc.go | 15 +------ config/configgrpc/configgrpc_test.go | 9 ++++ config/confighttp/confighttp.go | 18 ++------ config/internal/warning.go | 42 +++++++++++++++++ config/internal/warning_test.go | 67 ++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 28 deletions(-) create mode 100644 config/internal/warning.go create mode 100644 config/internal/warning_test.go diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 19ea767a460..28f22e5b61d 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -27,7 +27,6 @@ import ( "github.com/mostynb/go-grpc-compression/zstd" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "go.opentelemetry.io/otel" - "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/balancer/roundrobin" "google.golang.org/grpc/credentials" @@ -43,6 +42,7 @@ import ( "go.opentelemetry.io/collector/config/configcompression" "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/config/configtls" + "go.opentelemetry.io/collector/config/internal" ) var errMetadataNotFound = errors.New("no request metadata found") @@ -277,20 +277,9 @@ func (gss *GRPCServerSettings) ToServerOption(host component.Host, settings comp switch gss.NetAddr.Transport { case "tcp", "tcp4", "tcp6", "udp", "udp4", "udp6": - host, _, err := net.SplitHostPort(gss.NetAddr.Endpoint) - if err != nil { + if err := internal.WarnOnUnspecifiedHost(settings.Logger, gss.NetAddr.Endpoint); err != nil { return nil, fmt.Errorf("failed to parse endpoint: %w", err) } - - if ip := net.ParseIP(host); ip != nil && ip.IsUnspecified() { - settings.Logger.Warn( - "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", - zap.String( - "documentation", - "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security-best-practices.md#safeguards-against-denial-of-service-attacks", - ), - ) - } } var opts []grpc.ServerOption diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index 987dc7864e2..7ae3c28e531 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -404,6 +404,15 @@ func TestGRPCServerWarning(t *testing.T) { }, len: 0, }, + { + settings: GRPCServerSettings{ + NetAddr: confignet.NetAddr{ + Endpoint: "0.0.0.0:1234", + Transport: "unix", + }, + }, + len: 0, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index b1e2faf3e0c..aeb3fb9134f 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -17,7 +17,6 @@ package confighttp // import "go.opentelemetry.io/collector/config/confighttp" import ( "crypto/tls" "errors" - "fmt" "net" "net/http" "time" @@ -25,13 +24,13 @@ import ( "github.com/rs/cors" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel" - "go.uber.org/zap" "golang.org/x/net/http2" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/configauth" "go.opentelemetry.io/collector/config/configcompression" "go.opentelemetry.io/collector/config/configtls" + "go.opentelemetry.io/collector/config/internal" ) const headerContentEncoding = "Content-Encoding" @@ -261,19 +260,8 @@ func WithErrorHandler(e errorHandler) ToServerOption { // ToServer creates an http.Server from settings object. func (hss *HTTPServerSettings) ToServer(host component.Host, settings component.TelemetrySettings, handler http.Handler, opts ...ToServerOption) (*http.Server, error) { - endpointHost, _, err := net.SplitHostPort(hss.Endpoint) - if err != nil { - return nil, fmt.Errorf("failed to parse endpoint: %w", err) - } - - if ip := net.ParseIP(endpointHost); ip != nil && ip.IsUnspecified() { - settings.Logger.Warn( - "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", - zap.String( - "documentation", - "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security-best-practices.md#safeguards-against-denial-of-service-attacks", - ), - ) + if err := internal.WarnOnUnspecifiedHost(settings.Logger, hss.Endpoint); err != nil { + return nil, err } serverOpts := &toServerOptions{} diff --git a/config/internal/warning.go b/config/internal/warning.go new file mode 100644 index 00000000000..be4e1c2de50 --- /dev/null +++ b/config/internal/warning.go @@ -0,0 +1,42 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package internal // import "go.opentelemetry.io/collector/config/internal" + +import ( + "fmt" + "net" + + "go.uber.org/zap" +) + +// WarnOnUnspecifiedHost emits a warning if an endpoint has an unspecified host. +func WarnOnUnspecifiedHost(logger *zap.Logger, endpoint string) error { + host, _, err := net.SplitHostPort(endpoint) + if err != nil { + return fmt.Errorf("failed to parse endpoint: %w", err) + } + + if ip := net.ParseIP(host); ip != nil && ip.IsUnspecified() { + logger.Warn( + "Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks", + zap.String( + "documentation", + "https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security-best-practices.md#safeguards-against-denial-of-service-attacks", + ), + ) + } + + return nil +} diff --git a/config/internal/warning_test.go b/config/internal/warning_test.go new file mode 100644 index 00000000000..f018aad4c24 --- /dev/null +++ b/config/internal/warning_test.go @@ -0,0 +1,67 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package internal // import "go.opentelemetry.io/collector/config/internal" + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" +) + +func TestWarnOnUnspecifiedHost(t *testing.T) { + tests := []struct { + endpoint string + warn bool + err string + }{ + { + endpoint: "0.0.0.0:0", + warn: true, + }, + { + endpoint: "127.0.0.1:0", + }, + { + endpoint: "localhost:0", + }, + { + endpoint: "localhost::0", + err: "too many colons in address", + }, + } + for _, test := range tests { + t.Run(test.endpoint, func(t *testing.T) { + core, observed := observer.New(zap.DebugLevel) + logger := zap.New(core) + err := WarnOnUnspecifiedHost(logger, test.endpoint) + if test.err != "" { + assert.ErrorContains(t, err, test.err) + return + } + + require.NoError(t, err) + + var len int + if test.warn { + len = 1 + } + require.Len(t, observed.FilterLevelExact(zap.WarnLevel).All(), len) + }) + } + +}