From 7cf9f4c56258c4a16cb27009a2eae55e92a5e2c2 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Thu, 26 Jan 2023 13:06:41 -0800 Subject: [PATCH 1/4] Remove forcing localhost for insecure bootstrap --- ...9-fix-insecure-fleet-server-bootstrap.yaml | 31 +++++++++++++++++++ internal/pkg/agent/cmd/enroll_cmd.go | 4 --- 2 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 changelog/fragments/1674766899-fix-insecure-fleet-server-bootstrap.yaml diff --git a/changelog/fragments/1674766899-fix-insecure-fleet-server-bootstrap.yaml b/changelog/fragments/1674766899-fix-insecure-fleet-server-bootstrap.yaml new file mode 100644 index 00000000000..9e7eed1e96f --- /dev/null +++ b/changelog/fragments/1674766899-fix-insecure-fleet-server-bootstrap.yaml @@ -0,0 +1,31 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: bug-fix + +# Change summary; a 80ish characters long description of the change. +summary: fix insecure fleet-server bootstrap + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +description: Disable passing `localhost` when bootstrapping an insecure fleet-server instance. + +# Affected component; a word indicating the component this changeset affects. +component: fleet-server + +# PR number; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +#pr: 1234 + +# Issue number; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +issue: 2197 diff --git a/internal/pkg/agent/cmd/enroll_cmd.go b/internal/pkg/agent/cmd/enroll_cmd.go index 612c6eb0203..79dd39c8128 100644 --- a/internal/pkg/agent/cmd/enroll_cmd.go +++ b/internal/pkg/agent/cmd/enroll_cmd.go @@ -395,10 +395,6 @@ func (c *enrollCmd) prepareFleetTLS() error { } if c.options.FleetServer.Cert == "" && c.options.FleetServer.CertKey == "" { if c.options.FleetServer.Insecure { - // running insecure, force the binding to localhost (unless specified) - if c.options.FleetServer.Host == "" { - c.options.FleetServer.Host = defaultFleetServerInternalHost - } c.options.URL = fmt.Sprintf("http://%s:%d", host, port) c.options.Insecure = true return nil From 62b52d319cefd5280b35101663408b0f29a6127a Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Mon, 30 Jan 2023 14:27:19 -0800 Subject: [PATCH 2/4] Add unit tests --- internal/pkg/agent/cmd/enroll_cmd_test.go | 58 +++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/internal/pkg/agent/cmd/enroll_cmd_test.go b/internal/pkg/agent/cmd/enroll_cmd_test.go index a2835890778..0cb62c0f9cc 100644 --- a/internal/pkg/agent/cmd/enroll_cmd_test.go +++ b/internal/pkg/agent/cmd/enroll_cmd_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/pkg/errors" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/elastic/elastic-agent/internal/pkg/agent/configuration" @@ -324,6 +325,7 @@ func TestEnroll(t *testing.T) { require.False(t, store.Called) }, )) + } func TestValidateArgs(t *testing.T) { @@ -372,6 +374,62 @@ func TestValidateArgs(t *testing.T) { require.NotContains(t, cleanedTags, "") } +func TestPrepareFleetTLS(t *testing.T) { + log, _ := logger.New("tst", false) + t.Run("fleet-server insecure", func(t *testing.T) { + store := &mockStore{} + cmd, err := newEnrollCmdWithStore( + log, + &enrollCmdOption{ + FleetServer: enrollCmdFleetServerOption{ + ConnStr: "https://localhost:9200", + ServiceToken: "test-service-token", + PolicyID: "test-fleet-policy", + Insecure: true, + }, + }, + "", + store, + ) + require.NoError(t, err) + err = cmd.prepareFleetTLS() + require.NoError(t, err) + + assert.True(t, cmd.options.Insecure) + assert.Equal(t, "http://localhost:8220", cmd.options.URL) + assert.Equal(t, "localhost:8221", cmd.options.InternalURL) + + assert.Equal(t, "", cmd.options.FleetServer.Host) + }) + t.Run("fleet-server secure", func(t *testing.T) { + store := &mockStore{} + cmd, err := newEnrollCmdWithStore( + log, + &enrollCmdOption{ + FleetServer: enrollCmdFleetServerOption{ + ConnStr: "https://localhost:9200", + ServiceToken: "test-service-token", + PolicyID: "test-fleet-policy", + }, + }, + "", + store, + ) + require.NoError(t, err) + err = cmd.prepareFleetTLS() + require.NoError(t, err) + + assert.False(t, cmd.options.Insecure) + assert.NotEmpty(t, cmd.options.FleetServer.Cert) + assert.NotEmpty(t, cmd.options.FleetServer.CertKey) + assert.Equal(t, "https://localhost:8220", cmd.options.URL) + assert.NotEmpty(t, cmd.options.FleetServer.CAs) + assert.Equal(t, "localhost:8221", cmd.options.InternalURL) + + assert.Equal(t, "", cmd.options.FleetServer.Host) + }) +} + func withServer( m func(t *testing.T) *http.ServeMux, test func(t *testing.T, host string), From 21c811e02d87fc3297b6fbcd427f63903e0f4d3e Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Mon, 30 Jan 2023 14:30:45 -0800 Subject: [PATCH 3/4] Fix typo --- internal/pkg/agent/cmd/enroll_cmd_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/agent/cmd/enroll_cmd_test.go b/internal/pkg/agent/cmd/enroll_cmd_test.go index 0cb62c0f9cc..1520bdbdae6 100644 --- a/internal/pkg/agent/cmd/enroll_cmd_test.go +++ b/internal/pkg/agent/cmd/enroll_cmd_test.go @@ -423,7 +423,7 @@ func TestPrepareFleetTLS(t *testing.T) { assert.NotEmpty(t, cmd.options.FleetServer.Cert) assert.NotEmpty(t, cmd.options.FleetServer.CertKey) assert.Equal(t, "https://localhost:8220", cmd.options.URL) - assert.NotEmpty(t, cmd.options.FleetServer.CAs) + assert.NotEmpty(t, cmd.options.CAs) assert.Equal(t, "localhost:8221", cmd.options.InternalURL) assert.Equal(t, "", cmd.options.FleetServer.Host) From d555bdc21205cf1fad4257032cfc3be868a7061d Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Mon, 30 Jan 2023 14:57:13 -0800 Subject: [PATCH 4/4] Fix tests and linter --- internal/pkg/agent/cmd/enroll_cmd_test.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/internal/pkg/agent/cmd/enroll_cmd_test.go b/internal/pkg/agent/cmd/enroll_cmd_test.go index 1520bdbdae6..1a25c13c6f7 100644 --- a/internal/pkg/agent/cmd/enroll_cmd_test.go +++ b/internal/pkg/agent/cmd/enroll_cmd_test.go @@ -8,6 +8,8 @@ import ( "bytes" "context" "crypto/tls" + "errors" + "fmt" "io" "io/ioutil" "net" @@ -18,7 +20,6 @@ import ( "strconv" "testing" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -43,7 +44,7 @@ func (m *mockStore) Save(in io.Reader) error { } buf := new(bytes.Buffer) - io.Copy(buf, in) // nolint:errcheck //not required + io.Copy(buf, in) //nolint:errcheck // not required m.Content = buf.Bytes() return nil } @@ -397,7 +398,6 @@ func TestPrepareFleetTLS(t *testing.T) { assert.True(t, cmd.options.Insecure) assert.Equal(t, "http://localhost:8220", cmd.options.URL) - assert.Equal(t, "localhost:8221", cmd.options.InternalURL) assert.Equal(t, "", cmd.options.FleetServer.Host) }) @@ -422,9 +422,10 @@ func TestPrepareFleetTLS(t *testing.T) { assert.False(t, cmd.options.Insecure) assert.NotEmpty(t, cmd.options.FleetServer.Cert) assert.NotEmpty(t, cmd.options.FleetServer.CertKey) - assert.Equal(t, "https://localhost:8220", cmd.options.URL) assert.NotEmpty(t, cmd.options.CAs) - assert.Equal(t, "localhost:8221", cmd.options.InternalURL) + + hostname, _ := os.Hostname() + assert.Equal(t, fmt.Sprintf("https://%s:8220", hostname), cmd.options.URL) assert.Equal(t, "", cmd.options.FleetServer.Host) }) @@ -460,7 +461,7 @@ func withTLSServer( port := listener.Addr().(*net.TCPAddr).Port - s := http.Server{ + s := http.Server{ //nolint:gosec // test server config Handler: m(t), TLSConfig: &tls.Config{ Certificates: []tls.Certificate{serverCert}, @@ -469,7 +470,7 @@ func withTLSServer( } // Uses the X509KeyPair pair defined in the TLSConfig struct instead of file on disk. - go s.ServeTLS(listener, "", "") // nolint:errcheck //not required + go s.ServeTLS(listener, "", "") //nolint:errcheck // not required test(t, ca.Crt(), "localhost:"+strconv.Itoa(port)) } @@ -480,7 +481,7 @@ func bytesToTMPFile(b []byte) (string, error) { if err != nil { return "", err } - f.Write(b) // nolint:errcheck //not required + f.Write(b) //nolint:errcheck // not required if err := f.Close(); err != nil { return "", err }