Skip to content
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

[receiver/otlp, internal, featuregate] Add featuregate to switch to localhost defaults for server-like components #8622

Merged
merged 17 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 26 additions & 0 deletions .chloggen/mx-psi_featuregate-localhost-2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Use this changelog template to create an entry for release notes.

# 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: component

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Add `component.UseLocalHostAsDefaultHost` feature gate that changes default endpoints from 0.0.0.0 to localhost"

# One or more tracking issues or pull requests related to the change
issues: [8510]

# (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: |
The only component in this repository affected by this is the OTLP receiver.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []

26 changes: 26 additions & 0 deletions .chloggen/mx-psi_featuregate-localhost.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Use this changelog template to create an entry for release notes.

# 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: featuregate

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Add the `featuregate.ErrAlreadyRegistered` error, which is returned by `featuregate.Registry`'s `Register` when adding a feature gate that is already registered."

# One or more tracking issues or pull requests related to the change
issues: [8622]

# (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: |
Use `errors.Is` to check for this error.

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: [api]
20 changes: 17 additions & 3 deletions config/internal/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,30 @@ go 1.20

require (
github.com/stretchr/testify v1.8.4
go.opentelemetry.io/collector v0.92.0
go.uber.org/goleak v1.3.0
go.uber.org/zap v1.26.0
)

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/kr/pretty v0.3.1 // indirect
github.com/hashicorp/go-version v1.6.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rogpeppe/go-internal v1.10.0 // indirect
go.opentelemetry.io/collector/featuregate v1.0.1 // indirect
go.uber.org/multierr v1.11.0 // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace go.opentelemetry.io/collector => ../../

replace go.opentelemetry.io/collector/featuregate => ../../featuregate

replace go.opentelemetry.io/collector/confmap => ../../confmap

replace go.opentelemetry.io/collector/config/configtelemetry => ../configtelemetry

replace go.opentelemetry.io/collector/pdata => ../../pdata

replace go.opentelemetry.io/collector/consumer => ../../consumer

replace go.opentelemetry.io/collector/component => ../../component
12 changes: 2 additions & 10 deletions config/internal/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion config/internal/warning.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
"strings"

"go.uber.org/zap"

"go.opentelemetry.io/collector/internal/localhostgate"
)

func shouldWarn(endpoint string) bool {
Expand Down Expand Up @@ -38,11 +40,12 @@
func WarnOnUnspecifiedHost(logger *zap.Logger, endpoint string) {
if shouldWarn(endpoint) {
logger.Warn(
"Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks",
"Using the 0.0.0.0 address exposes this server to every network interface, which may facilitate Denial of Service attacks. Enable the feature gate to change the default and remove this warning.",

Check warning on line 43 in config/internal/warning.go

View check run for this annotation

Codecov / codecov/patch

config/internal/warning.go#L43

Added line #L43 was not covered by tests
zap.String(
"documentation",
"https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/security-best-practices.md#safeguards-against-denial-of-service-attacks",
),
zap.String("feature gate ID", localhostgate.UseLocalHostAsDefaultHostID),

Check warning on line 48 in config/internal/warning.go

View check run for this annotation

Codecov / codecov/patch

config/internal/warning.go#L48

Added line #L48 was not covered by tests
)
}
}
2 changes: 2 additions & 0 deletions docs/security-best-practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ receivers:
Generally, `localhost`-like addresses should be preferred over the 0.0.0.0 address.
For more information, see [CWE-1327](https://cwe.mitre.org/data/definitions/1327.html).

To change the default endpoint to be `localhost`-bound in all components, enable the `component.UseLocalHostAsDefaultHost` feature gate. This feature gate will be enabled by default in the Collector in a future release.

## Processors

Processors sit between receivers and exporters. They are responsible for
Expand Down
8 changes: 7 additions & 1 deletion featuregate/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package featuregate // import "go.opentelemetry.io/collector/featuregate"

import (
"errors"
"fmt"
"net/url"
"regexp"
Expand All @@ -22,6 +23,11 @@ var (
idRegexp = regexp.MustCompile(`^[0-9a-zA-Z\.]*$`)
)

var (
// ErrAlreadyRegistered is returned when adding a Gate that is already registered.
ErrAlreadyRegistered = errors.New("gate is already registered")
)

// GlobalRegistry returns the global Registry.
func GlobalRegistry() *Registry {
return globalRegistry
Expand Down Expand Up @@ -157,7 +163,7 @@ func (r *Registry) Register(id string, stage Stage, opts ...RegisterOption) (*Ga
}

if _, loaded := r.gates.LoadOrStore(id, g); loaded {
return nil, fmt.Errorf("attempted to add pre-existing gate %q", id)
return nil, fmt.Errorf("failed to register %q: %w", id, ErrAlreadyRegistered)
}
return g, nil
}
Expand Down
2 changes: 1 addition & 1 deletion featuregate/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestRegistry(t *testing.T) {
assert.False(t, g.IsEnabled())

_, err = r.Register(id, StageBeta)
assert.Error(t, err)
assert.ErrorIs(t, err, ErrAlreadyRegistered)
assert.Panics(t, func() {
r.MustRegister(id, StageBeta)
})
Expand Down
68 changes: 68 additions & 0 deletions internal/localhostgate/featuregate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

// package localhostgate defines a feature gate that controls whether server-like receivers and extensions use localhost as the default host for their endpoints.
// This package is duplicated across core and contrib to avoid exposing the feature gate as part of the public API.
// To do this we define a `registerOrLoad` helper and try to register the gate in both modules.
// IMPORTANT NOTE: ANY CHANGES TO THIS PACKAGE MUST BE MIRRORED IN THE CONTRIB COUNTERPART.
package localhostgate // import "go.opentelemetry.io/collector/internal/localhostgate"

import (
"errors"
"fmt"

"go.uber.org/zap"

"go.opentelemetry.io/collector/featuregate"
)

const UseLocalHostAsDefaultHostID = "component.UseLocalHostAsDefaultHost"

// useLocalHostAsDefaultHostfeatureGate is the feature gate that controls whether
// server-like receivers and extensions such as the OTLP receiver use localhost as the default host for their endpoints.
var useLocalHostAsDefaultHostfeatureGate = mustRegisterOrLoad(
featuregate.GlobalRegistry(),
UseLocalHostAsDefaultHostID,
featuregate.StageAlpha,
featuregate.WithRegisterDescription("controls whether server-like receivers and extensions such as the OTLP receiver use localhost as the default host for their endpoints"),
)

// mustRegisterOrLoad tries to register the feature gate and loads it if it already exists.
// It panics on any other error.
func mustRegisterOrLoad(reg *featuregate.Registry, id string, stage featuregate.Stage, opts ...featuregate.RegisterOption) *featuregate.Gate {
gate, err := reg.Register(id, stage, opts...)

if errors.Is(err, featuregate.ErrAlreadyRegistered) {
// Gate is already registered; find it.
// Only a handful of feature gates are registered, so it's fine to iterate over all of them.
reg.VisitAll(func(g *featuregate.Gate) {
if g.ID() == id {
gate = g
return
}

Check warning on line 42 in internal/localhostgate/featuregate.go

View check run for this annotation

Codecov / codecov/patch

internal/localhostgate/featuregate.go#L36-L42

Added lines #L36 - L42 were not covered by tests
})
} else if err != nil {
panic(err)

Check warning on line 45 in internal/localhostgate/featuregate.go

View check run for this annotation

Codecov / codecov/patch

internal/localhostgate/featuregate.go#L45

Added line #L45 was not covered by tests
}

return gate
}

// EndpointForPort gets the endpoint for a given port using localhost or 0.0.0.0 depending on the feature gate.
func EndpointForPort(port int) string {
host := "localhost"
if !useLocalHostAsDefaultHostfeatureGate.IsEnabled() {
host = "0.0.0.0"
}
return fmt.Sprintf("%s:%d", host, port)
}

// LogAboutUseLocalHostAsDefault logs about the upcoming change from 0.0.0.0 to localhost on server-like components.
func LogAboutUseLocalHostAsDefault(logger *zap.Logger) {
if !useLocalHostAsDefaultHostfeatureGate.IsEnabled() {
logger.Warn(
"The default endpoints for all servers in components will change to use localhost instead of 0.0.0.0 in a future version. Use the feature gate to preview the new default.",
zap.String("feature gate ID", UseLocalHostAsDefaultHostID),
)
}

Check warning on line 67 in internal/localhostgate/featuregate.go

View check run for this annotation

Codecov / codecov/patch

internal/localhostgate/featuregate.go#L61-L67

Added lines #L61 - L67 were not covered by tests
}
57 changes: 57 additions & 0 deletions internal/localhostgate/featuregate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package localhostgate

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/featuregate"
)

func setFeatureGateForTest(t testing.TB, gate *featuregate.Gate, enabled bool) func() {
originalValue := gate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(gate.ID(), enabled))
return func() {
require.NoError(t, featuregate.GlobalRegistry().Set(gate.ID(), originalValue))
}
}

func TestEndpointForPort(t *testing.T) {
tests := []struct {
port int
enabled bool
endpoint string
}{
{
port: 4317,
enabled: false,
endpoint: "0.0.0.0:4317",
},
{
port: 4317,
enabled: true,
endpoint: "localhost:4317",
},
{
port: 0,
enabled: false,
endpoint: "0.0.0.0:0",
},
{
port: 0,
enabled: true,
endpoint: "localhost:0",
},
}

for _, tt := range tests {
t.Run(tt.endpoint, func(t *testing.T) {
defer setFeatureGateForTest(t, useLocalHostAsDefaultHostfeatureGate, tt.enabled)()
assert.Equal(t, EndpointForPort(tt.port), tt.endpoint)
})
}
}
4 changes: 3 additions & 1 deletion receiver/otlpreceiver/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ The following settings are configurable:

- `endpoint` (default = 0.0.0.0:4317 for grpc protocol, 0.0.0.0:4318 http protocol):
host:port to which the receiver is going to receive data. The valid syntax is
described at https://github.com/grpc/grpc/blob/master/doc/naming.md.
described at https://github.com/grpc/grpc/blob/master/doc/naming.md. The
`component.UseLocalHostAsDefaultHost` feature gate changes these to localhost:4317 and
localhost:4318 respectively. This will become the default in a future release.

## Advanced Configuration

Expand Down
7 changes: 5 additions & 2 deletions receiver/otlpreceiver/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@ import (
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/internal/localhostgate"
"go.opentelemetry.io/collector/internal/sharedcomponent"
"go.opentelemetry.io/collector/receiver"
"go.opentelemetry.io/collector/receiver/otlpreceiver/internal/metadata"
)

const (
grpcPort = 4317
httpPort = 4318
defaultGRPCEndpoint = "0.0.0.0:4317"
defaultHTTPEndpoint = "0.0.0.0:4318"

Expand All @@ -42,15 +45,15 @@ func createDefaultConfig() component.Config {
Protocols: Protocols{
GRPC: &configgrpc.GRPCServerSettings{
NetAddr: confignet.NetAddr{
Endpoint: defaultGRPCEndpoint,
Endpoint: localhostgate.EndpointForPort(grpcPort),
Transport: "tcp",
},
// We almost write 0 bytes, so no need to tune WriteBufferSize.
ReadBufferSize: 512 * 1024,
},
HTTP: &HTTPConfig{
HTTPServerSettings: &confighttp.HTTPServerSettings{
Endpoint: defaultHTTPEndpoint,
Endpoint: localhostgate.EndpointForPort(httpPort),
},
TracesURLPath: defaultTracesURLPath,
MetricsURLPath: defaultMetricsURLPath,
Expand Down
2 changes: 2 additions & 0 deletions service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"go.opentelemetry.io/collector/connector"
"go.opentelemetry.io/collector/exporter"
"go.opentelemetry.io/collector/extension"
"go.opentelemetry.io/collector/internal/localhostgate"
"go.opentelemetry.io/collector/internal/obsreportconfig"
"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/processor"
Expand Down Expand Up @@ -163,6 +164,7 @@ func (srv *Service) Start(ctx context.Context) error {
}

srv.telemetrySettings.Logger.Info("Everything is ready. Begin running and processing data.")
localhostgate.LogAboutUseLocalHostAsDefault(srv.telemetrySettings.Logger)
return nil
}

Expand Down