Skip to content

Commit

Permalink
[receiver/otlp, internal, featuregate] Add featuregate to switch to l…
Browse files Browse the repository at this point in the history
…ocalhost defaults for server-like components (#8622)

**Description:** 

- Define `component.UseLocalHostAsDefaultHost` in the
`internal/localhostgate` package.
- Define `featuregate.ErrIsAlreadyRegistered` error, returned by
`Register` when a gate is already registered.
- Adds support for the localhost gate on the OTLP receiver.

This PR does not remove the current warning in any way, we can remove
this separately.

**Link to tracking Issue:** Updates #8510

**Testing:** Adds unit tests

**Documentation:** Document on OTLP receiver template and add related
logging.
  • Loading branch information
mx-psi committed Jan 24, 2024
1 parent 5cf53ac commit 9047c0e
Show file tree
Hide file tree
Showing 13 changed files with 220 additions and 19 deletions.
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 @@ import (
"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 shouldWarn(endpoint string) bool {
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.",
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),
)
}
}
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
}
})
} else if err != nil {
panic(err)
}

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),
)
}
}
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

0 comments on commit 9047c0e

Please sign in to comment.