diff --git a/.chloggen/mx-psi_add-logging-0.0.0.0.yaml b/.chloggen/mx-psi_add-logging-0.0.0.0.yaml new file mode 100755 index 00000000000..2f3f005642d --- /dev/null +++ b/.chloggen/mx-psi_add-logging-0.0.0.0.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: receiver/otlp + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add warning when using unspecified (`0.0.0.0`) address on HTTP or gRPC servers + +# One or more tracking issues or pull requests related to the change +issues: [6151] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 13c198ade05..28f22e5b61d 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -42,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") @@ -273,6 +274,14 @@ 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) { + + switch gss.NetAddr.Transport { + case "tcp", "tcp4", "tcp6", "udp", "udp4", "udp6": + if err := internal.WarnOnUnspecifiedHost(settings.Logger, gss.NetAddr.Endpoint); err != nil { + return nil, fmt.Errorf("failed to parse endpoint: %w", err) + } + } + var opts []grpc.ServerOption if gss.TLSSetting != nil { diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index 4ac4dccb869..7ae3c28e531 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" @@ -162,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...) @@ -206,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()) @@ -370,6 +380,57 @@ 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, + }, + { + 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) { + 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 f5b49b7b7bc..aeb3fb9134f 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -30,6 +30,7 @@ import ( "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" @@ -259,6 +260,10 @@ 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 err := internal.WarnOnUnspecifiedHost(settings.Logger, hss.Endpoint); err != nil { + return nil, err + } + serverOpts := &toServerOptions{} for _, o := range opts { o(serverOpts) diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index 4f7877a4299..ae9b12e8be3 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 @@ -687,6 +728,7 @@ func TestHttpCorsInvalidSettings(t *testing.T) { func TestHttpCorsWithAuthentication(t *testing.T) { hss := &HTTPServerSettings{ + Endpoint: "localhost:0", CORS: &CORSSettings{ AllowedOrigins: []string{"*"}, }, @@ -884,6 +926,7 @@ func TestServerAuth(t *testing.T) { // prepare authCalled := false hss := HTTPServerSettings{ + Endpoint: "localhost:0", Auth: &configauth.Authentication{ AuthenticatorID: config.NewComponentID("mock"), }, @@ -931,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"), }, 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) + }) + } + +}