From 2afea7502d4e94f37b83a62369c70cb1e92cec3c Mon Sep 17 00:00:00 2001
From: Michal Pristas <michal.pristas@gmail.com>
Date: Fri, 27 Oct 2023 16:55:47 +0200
Subject: [PATCH] Disable exposing metrics server on all interfaces (#3662)

This PR disables option to configure agent in a way that it will expose monitoring agent on all interfaces.
Empty value for agent.monitoring.http.host is now disabled and is replaced by localhost

Agents upgraded from <8.5 do have empty host due to change in defaults in 8.5. These agents are exposing metrics endpoint on all interfaces not on purpose.

Guard is implemented at the time of configuration unpacking and at the time of server creation to minimize future misbehavior with possible code changes
---
 .../agent/application/monitoring/server.go    |  5 ++
 internal/pkg/core/monitoring/config/config.go | 51 +++++++++++++++++--
 .../pkg/core/monitoring/config/config_test.go | 51 +++++++++++++++++++
 3 files changed, 103 insertions(+), 4 deletions(-)

diff --git a/internal/pkg/agent/application/monitoring/server.go b/internal/pkg/agent/application/monitoring/server.go
index 47561d29e49..94b28807700 100644
--- a/internal/pkg/agent/application/monitoring/server.go
+++ b/internal/pkg/agent/application/monitoring/server.go
@@ -20,6 +20,7 @@ import (
 	"github.com/elastic/elastic-agent-libs/config"
 	"github.com/elastic/elastic-agent-libs/monitoring"
 	"github.com/elastic/elastic-agent/internal/pkg/agent/application/coordinator"
+	monitoringCfg "github.com/elastic/elastic-agent/internal/pkg/core/monitoring/config"
 	"github.com/elastic/elastic-agent/pkg/core/logger"
 )
 
@@ -38,6 +39,10 @@ func NewServer(
 		log.Warnf("failed to create monitoring drop: %v", err)
 	}
 
+	if strings.TrimSpace(endpointConfig.Host) == "" {
+		endpointConfig.Host = monitoringCfg.DefaultHost
+	}
+
 	cfg, err := config.NewConfigFrom(endpointConfig)
 	if err != nil {
 		return nil, err
diff --git a/internal/pkg/core/monitoring/config/config.go b/internal/pkg/core/monitoring/config/config.go
index 1f326e29a8d..cc6f345e123 100644
--- a/internal/pkg/core/monitoring/config/config.go
+++ b/internal/pkg/core/monitoring/config/config.go
@@ -4,10 +4,20 @@
 
 package config
 
-import "time"
+import (
+	"strings"
+	"time"
 
-const defaultPort = 6791
-const defaultNamespace = "default"
+	c "github.com/elastic/elastic-agent-libs/config"
+)
+
+const (
+	defaultPort      = 6791
+	defaultNamespace = "default"
+
+	// DefaultHost is used when host is not defined or empty
+	DefaultHost = "localhost"
+)
 
 // MonitoringConfig describes a configuration of a monitoring
 type MonitoringConfig struct {
@@ -33,6 +43,39 @@ type MonitoringHTTPConfig struct {
 	Buffer  *BufferConfig `yaml:"buffer" config:"buffer"`
 }
 
+// Unpack reads a config object into the settings.
+func (c *MonitoringHTTPConfig) Unpack(cfg *c.C) error {
+	// do not use MonitoringHTTPConfig, it will end up in a loop
+	tmp := struct {
+		Enabled bool          `yaml:"enabled" config:"enabled"`
+		Host    string        `yaml:"host" config:"host"`
+		Port    int           `yaml:"port" config:"port" validate:"min=0,max=65535,nonzero"`
+		Buffer  *BufferConfig `yaml:"buffer" config:"buffer"`
+	}{
+		Enabled: c.Enabled,
+		Host:    c.Host,
+		Port:    c.Port,
+		Buffer:  c.Buffer,
+	}
+
+	if err := cfg.Unpack(&tmp); err != nil {
+		return err
+	}
+
+	if strings.TrimSpace(tmp.Host) == "" {
+		tmp.Host = DefaultHost
+	}
+
+	*c = MonitoringHTTPConfig{
+		Enabled: tmp.Enabled,
+		Host:    tmp.Host,
+		Port:    tmp.Port,
+		Buffer:  tmp.Buffer,
+	}
+
+	return nil
+}
+
 // PprofConfig is a struct for the pprof enablement flag.
 // It is a nil struct by default to allow the agent to use the a value that the user has injected into fleet.yml as the source of truth that is passed to beats
 // TODO get this value from Kibana?
@@ -55,7 +98,7 @@ func DefaultConfig() *MonitoringConfig {
 		MonitorTraces:  false,
 		HTTP: &MonitoringHTTPConfig{
 			Enabled: false,
-			Host:    "localhost",
+			Host:    DefaultHost,
 			Port:    defaultPort,
 		},
 		Namespace:   defaultNamespace,
diff --git a/internal/pkg/core/monitoring/config/config_test.go b/internal/pkg/core/monitoring/config/config_test.go
index 2363f46ae50..70ae6a77b4c 100644
--- a/internal/pkg/core/monitoring/config/config_test.go
+++ b/internal/pkg/core/monitoring/config/config_test.go
@@ -13,6 +13,57 @@ import (
 	"github.com/elastic/elastic-agent/internal/pkg/config"
 )
 
+func TestHost(t *testing.T) {
+	testCases := []struct {
+		name         string
+		config       string
+		expectedHost string
+	}{
+		{"no host", `enabled: true
+logs: true
+metrics: true
+http:
+  enabled: true`, DefaultHost},
+		{"empty host", `enabled: true
+logs: true
+metrics: true
+http:
+  enabled: true
+  host: ""`, DefaultHost},
+		{"whitespace host", `enabled: true
+logs: true
+metrics: true
+http:
+  enabled: true
+  host: "   "`, DefaultHost},
+		{"default", `enabled: true
+logs: true
+metrics: true
+http:
+  enabled: true
+  host: localhost`, DefaultHost},
+		{"custom host", `enabled: true
+logs: true
+metrics: true
+http:
+  enabled: true
+  host: custom`, "custom"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			c, err := config.NewConfigFrom(tc.config)
+			require.NoError(t, err, "failed to create config")
+
+			cfg := DefaultConfig()
+			err = c.Unpack(&cfg)
+			require.NoError(t, err, "failed to unpack config")
+
+			require.Equal(t, tc.expectedHost, cfg.HTTP.Host)
+		})
+	}
+}
+
 func TestAPMConfig(t *testing.T) {
 	tcs := map[string]struct {
 		in  map[string]interface{}