From 18350a85ef0794ceba3d9e866518e6f7ce8b1528 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Mon, 2 Oct 2023 21:48:53 +0200 Subject: [PATCH] [Feature] Secondary fallback for package signature verification (#3453) Adds a capability to check for fleet server hosted PGP as the last resort in case elastic artifact API is unavailable. Fleet server will host this PGP per version. any snapshot flags or other version qualifiers are ignored. (cherry picked from commit cdca2116142efde46775cf8e6dd77051238f110b) --- ...ck-for-package-signature-verification.yaml | 31 +++++ internal/pkg/agent/application/application.go | 2 +- .../pkg/agent/application/managed_mode.go | 53 ++++---- .../upgrade/artifact/download/fs/verifier.go | 2 +- .../artifact/download/http/verifier.go | 2 +- .../upgrade/artifact/download/verifier.go | 16 ++- .../artifact/download/verifier_test.go | 127 ++++++++++++++++++ .../application/upgrade/step_download.go | 30 ++++- .../application/upgrade/step_download_test.go | 35 +++-- .../pkg/agent/application/upgrade/upgrade.go | 21 ++- internal/pkg/remote/client.go | 4 + 11 files changed, 278 insertions(+), 45 deletions(-) create mode 100644 changelog/fragments/1695289867-Secondary-fallback-for-package-signature-verification.yaml create mode 100644 internal/pkg/agent/application/upgrade/artifact/download/verifier_test.go diff --git a/changelog/fragments/1695289867-Secondary-fallback-for-package-signature-verification.yaml b/changelog/fragments/1695289867-Secondary-fallback-for-package-signature-verification.yaml new file mode 100644 index 00000000000..07c8c4e5cf8 --- /dev/null +++ b/changelog/fragments/1695289867-Secondary-fallback-for-package-signature-verification.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: feature + +# Change summary; a 80ish characters long description of the change. +summary: Secondary fallback for package signature verification + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +description: Ability to upgrade securely in Air gapped environment where fleet server is the only reachable URI. + +# Affected component; a word indicating the component this changeset affects. +component: elastic-agent + +# 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: https://github.com/elastic/elastic-agent/pull/3453 + +# 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: https://github.com/elastic/elastic-agent/issues/3264 diff --git a/internal/pkg/agent/application/application.go b/internal/pkg/agent/application/application.go index 445bba8434f..c0d82cd0fbc 100644 --- a/internal/pkg/agent/application/application.go +++ b/internal/pkg/agent/application/application.go @@ -160,7 +160,7 @@ func New( EndpointSignedComponentModifier(), ) - managed, err = newManagedConfigManager(ctx, log, agentInfo, cfg, store, runtime, fleetInitTimeout) + managed, err = newManagedConfigManager(ctx, log, agentInfo, cfg, store, runtime, fleetInitTimeout, upgrader) if err != nil { return nil, nil, nil, err } diff --git a/internal/pkg/agent/application/managed_mode.go b/internal/pkg/agent/application/managed_mode.go index f29be7fe258..58aebd06451 100644 --- a/internal/pkg/agent/application/managed_mode.go +++ b/internal/pkg/agent/application/managed_mode.go @@ -10,6 +10,7 @@ import ( "time" "github.com/elastic/elastic-agent-client/v7/pkg/client" + "github.com/elastic/elastic-agent/internal/pkg/agent/application/actions" "github.com/elastic/elastic-agent/internal/pkg/agent/application/actions/handlers" "github.com/elastic/elastic-agent/internal/pkg/agent/application/coordinator" "github.com/elastic/elastic-agent/internal/pkg/agent/application/dispatcher" @@ -38,17 +39,18 @@ import ( const dispatchFlushInterval = time.Minute * 5 type managedConfigManager struct { - log *logger.Logger - agentInfo *info.AgentInfo - cfg *configuration.Configuration - client *remote.Client - store storage.Store - stateStore *store.StateStore - actionQueue *queue.ActionQueue - dispatcher *dispatcher.ActionDispatcher - runtime *runtime.Manager - coord *coordinator.Coordinator - fleetInitTimeout time.Duration + log *logger.Logger + agentInfo *info.AgentInfo + cfg *configuration.Configuration + client *remote.Client + store storage.Store + stateStore *store.StateStore + actionQueue *queue.ActionQueue + dispatcher *dispatcher.ActionDispatcher + runtime *runtime.Manager + coord *coordinator.Coordinator + fleetInitTimeout time.Duration + initialClientSetters []actions.ClientSetter ch chan coordinator.ConfigChange errCh chan error @@ -62,6 +64,7 @@ func newManagedConfigManager( storeSaver storage.Store, runtime *runtime.Manager, fleetInitTimeout time.Duration, + clientSetters ...actions.ClientSetter, ) (*managedConfigManager, error) { client, err := fleetclient.NewAuthWithConfig(log, cfg.Fleet.AccessAPIKey, cfg.Fleet.Client) if err != nil { @@ -88,18 +91,19 @@ func newManagedConfigManager( } return &managedConfigManager{ - log: log, - agentInfo: agentInfo, - cfg: cfg, - client: client, - store: storeSaver, - stateStore: stateStore, - actionQueue: actionQueue, - dispatcher: actionDispatcher, - runtime: runtime, - fleetInitTimeout: fleetInitTimeout, - ch: make(chan coordinator.ConfigChange), - errCh: make(chan error), + log: log, + agentInfo: agentInfo, + cfg: cfg, + client: client, + store: storeSaver, + stateStore: stateStore, + actionQueue: actionQueue, + dispatcher: actionDispatcher, + runtime: runtime, + fleetInitTimeout: fleetInitTimeout, + ch: make(chan coordinator.ConfigChange), + errCh: make(chan error), + initialClientSetters: clientSetters, }, nil } @@ -196,6 +200,9 @@ func (m *managedConfigManager) Run(ctx context.Context) error { policyChanger.AddSetter(gateway) policyChanger.AddSetter(ack) } + for _, cs := range m.initialClientSetters { + policyChanger.AddSetter(cs) + } // Proxy errors from the gateway to our own channel. gatewayErrorsRunner := runner.Start(context.Background(), func(ctx context.Context) error { diff --git a/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier.go b/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier.go index 72066129222..263f6ea8bf5 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier.go +++ b/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier.go @@ -124,7 +124,7 @@ func (v *Verifier) verifyAsc(fullPath string, skipDefaultPgp bool, pgpSources .. if len(check) == 0 { continue } - raw, err := download.PgpBytesFromSource(v.log, check, v.client) + raw, err := download.PgpBytesFromSource(v.log, check, &v.client) if err != nil { return err } diff --git a/internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go b/internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go index 61992e08816..84f3f6045f0 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go +++ b/internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go @@ -125,7 +125,7 @@ func (v *Verifier) verifyAsc(a artifact.Artifact, version string, skipDefaultPgp if len(check) == 0 { continue } - raw, err := download.PgpBytesFromSource(v.log, check, v.client) + raw, err := download.PgpBytesFromSource(v.log, check, &v.client) if err != nil { return err } diff --git a/internal/pkg/agent/application/upgrade/artifact/download/verifier.go b/internal/pkg/agent/application/upgrade/artifact/download/verifier.go index 2c716cb2844..662367f4909 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/verifier.go +++ b/internal/pkg/agent/application/upgrade/artifact/download/verifier.go @@ -36,6 +36,7 @@ const ( var ( ErrRemotePGPDownloadFailed = errors.New("Remote PGP download failed") ErrInvalidLocation = errors.New("Remote PGP location is invalid") + ErrUnknownPGPSource = errors.New("unknown pgp source") ) // warnLogger is a logger that only needs to implement Warnf, as that is the only functions @@ -180,7 +181,7 @@ func VerifyGPGSignature(file string, asciiArmorSignature, publicKey []byte) erro return nil } -func PgpBytesFromSource(log warnLogger, source string, client http.Client) ([]byte, error) { +func PgpBytesFromSource(log warnLogger, source string, client HTTPClient) ([]byte, error) { if strings.HasPrefix(source, PgpSourceRawPrefix) { return []byte(strings.TrimPrefix(source, PgpSourceRawPrefix)), nil } @@ -189,11 +190,14 @@ func PgpBytesFromSource(log warnLogger, source string, client http.Client) ([]by pgpBytes, err := fetchPgpFromURI(strings.TrimPrefix(source, PgpSourceURIPrefix), client) if errors.Is(err, ErrRemotePGPDownloadFailed) || errors.Is(err, ErrInvalidLocation) { log.Warnf("Skipped remote PGP located at %q because it's unavailable: %v", strings.TrimPrefix(source, PgpSourceURIPrefix), err) + } else if err != nil { + log.Warnf("Failed to fetch remote PGP") } + return pgpBytes, nil } - return nil, errors.New("unknown pgp source") + return nil, ErrUnknownPGPSource } func CheckValidDownloadUri(rawURI string) error { @@ -209,7 +213,7 @@ func CheckValidDownloadUri(rawURI string) error { return nil } -func fetchPgpFromURI(uri string, client http.Client) ([]byte, error) { +func fetchPgpFromURI(uri string, client HTTPClient) ([]byte, error) { if err := CheckValidDownloadUri(uri); err != nil { return nil, err } @@ -221,7 +225,7 @@ func fetchPgpFromURI(uri string, client http.Client) ([]byte, error) { if err != nil { return nil, err } - resp, err := http.DefaultClient.Do(req) + resp, err := client.Do(req) if err != nil { return nil, multierror.Append(err, ErrRemotePGPDownloadFailed) } @@ -233,3 +237,7 @@ func fetchPgpFromURI(uri string, client http.Client) ([]byte, error) { return ioutil.ReadAll(resp.Body) } + +type HTTPClient interface { + Do(*http.Request) (*http.Response, error) +} diff --git a/internal/pkg/agent/application/upgrade/artifact/download/verifier_test.go b/internal/pkg/agent/application/upgrade/artifact/download/verifier_test.go new file mode 100644 index 00000000000..05ad9a96b91 --- /dev/null +++ b/internal/pkg/agent/application/upgrade/artifact/download/verifier_test.go @@ -0,0 +1,127 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package download + +import ( + "bytes" + "io" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/elastic/elastic-agent/internal/pkg/agent/errors" + "github.com/elastic/elastic-agent/pkg/core/logger" +) + +func TestPgpBytesFromSource(t *testing.T) { + testCases := []struct { + Name string + Source string + ClientDoErr error + ClientBody []byte + ClientStatus int + + ExpectedPGP []byte + ExpectedErr error + ExpectedLogMessage string + }{ + { + "successful call", + PgpSourceURIPrefix + "https://location/path", + nil, + []byte("pgp-body"), + 200, + []byte("pgp-body"), + nil, + "", + }, + { + "unknown source call", + "https://location/path", + nil, + []byte("pgp-body"), + 200, + nil, + ErrUnknownPGPSource, + "", + }, + { + "invalid location is filtered call", + PgpSourceURIPrefix + "http://location/path", + nil, + []byte("pgp-body"), + 200, + nil, + nil, + "Skipped remote PGP located ", + }, + { + "do error is filtered", + PgpSourceURIPrefix + "https://location/path", + errors.New("error"), + []byte("pgp-body"), + 200, + nil, + nil, + "Skipped remote PGP located", + }, + { + "invalid status code is filtered out", + PgpSourceURIPrefix + "https://location/path", + nil, + []byte("pgp-body"), + 500, + nil, + nil, + "Failed to fetch remote PGP", + }, + { + "invalid status code is filtered out", + PgpSourceURIPrefix + "https://location/path", + nil, + []byte("pgp-body"), + 404, + nil, + nil, + "Failed to fetch remote PGP", + }, + } + + for _, tc := range testCases { + t.Run(tc.Name, func(t *testing.T) { + log, obs := logger.NewTesting(tc.Name) + mockClient := &MockClient{ + DoFunc: func(req *http.Request) (*http.Response, error) { + if tc.ClientDoErr != nil { + return nil, tc.ClientDoErr + } + + return &http.Response{ + StatusCode: tc.ClientStatus, + Body: io.NopCloser(bytes.NewReader(tc.ClientBody)), + }, nil + }, + } + + resPgp, resErr := PgpBytesFromSource(log, tc.Source, mockClient) + require.Equal(t, tc.ExpectedErr, resErr) + require.Equal(t, tc.ExpectedPGP, resPgp) + if tc.ExpectedLogMessage != "" { + logs := obs.FilterMessageSnippet(tc.ExpectedLogMessage) + require.NotEqual(t, 0, logs.Len()) + } + + }) + } +} + +type MockClient struct { + DoFunc func(req *http.Request) (*http.Response, error) +} + +func (m *MockClient) Do(req *http.Request) (*http.Response, error) { + return m.DoFunc(req) +} diff --git a/internal/pkg/agent/application/upgrade/step_download.go b/internal/pkg/agent/application/upgrade/step_download.go index 1bb0d3ac9f8..a273460f337 100644 --- a/internal/pkg/agent/application/upgrade/step_download.go +++ b/internal/pkg/agent/application/upgrade/step_download.go @@ -7,6 +7,7 @@ package upgrade import ( "context" "fmt" + "net/url" "os" "strings" "time" @@ -30,7 +31,8 @@ import ( ) const ( - defaultUpgradeFallbackPGP = "https://artifacts.elastic.co/GPG-KEY-elastic-agent" + defaultUpgradeFallbackPGP = "https://artifacts.elastic.co/GPG-KEY-elastic-agent" + fleetUpgradeFallbackPGPFormat = "/api/agents/upgrades/%d.%d.%d/pgp-public-key" ) func (u *Upgrader) downloadArtifact(ctx context.Context, version, sourceURI string, skipVerifyOverride bool, skipDefaultPgp bool, pgpBytes ...string) (_ string, err error) { @@ -40,7 +42,7 @@ func (u *Upgrader) downloadArtifact(ctx context.Context, version, sourceURI stri span.End() }() - pgpBytes = appendFallbackPGP(pgpBytes) + pgpBytes = u.appendFallbackPGP(version, pgpBytes) // do not update source config settings := *u.settings @@ -87,13 +89,35 @@ func (u *Upgrader) downloadArtifact(ctx context.Context, version, sourceURI stri return path, nil } -func appendFallbackPGP(pgpBytes []string) []string { +func (u *Upgrader) appendFallbackPGP(targetVersion string, pgpBytes []string) []string { if pgpBytes == nil { pgpBytes = make([]string, 0, 1) } fallbackPGP := download.PgpSourceURIPrefix + defaultUpgradeFallbackPGP pgpBytes = append(pgpBytes, fallbackPGP) + + // add a secondary fallback if fleet server is configured + u.log.Debugf("Considering fleet server uri for pgp check fallback %q", u.fleetServerURI) + if u.fleetServerURI != "" { + tpv, err := agtversion.ParseVersion(targetVersion) + if err != nil { + // best effort, log failure + u.log.Warnf("failed to parse agent version (%q) for secondary GPG fallback: %v", targetVersion, err) + } else { + secondaryPath, err := url.JoinPath( + u.fleetServerURI, + fmt.Sprintf(fleetUpgradeFallbackPGPFormat, tpv.Major(), tpv.Minor(), tpv.Patch()), + ) + if err != nil { + u.log.Warnf("failed to compose Fleet Server URI: %v", err) + } else { + secondaryFallback := download.PgpSourceURIPrefix + secondaryPath + pgpBytes = append(pgpBytes, secondaryFallback) + } + } + } + return pgpBytes } diff --git a/internal/pkg/agent/application/upgrade/step_download_test.go b/internal/pkg/agent/application/upgrade/step_download_test.go index 1670eb3e5a6..330a60f5288 100644 --- a/internal/pkg/agent/application/upgrade/step_download_test.go +++ b/internal/pkg/agent/application/upgrade/step_download_test.go @@ -7,6 +7,7 @@ package upgrade import ( "context" "fmt" + "strings" "testing" "time" @@ -32,22 +33,40 @@ func (md *mockDownloader) Download(ctx context.Context, agentArtifact artifact.A func TestFallbackIsAppended(t *testing.T) { testCases := []struct { - name string - passedBytes []string - expectedLen int + name string + passedBytes []string + expectedLen int + expectedDefaultIdx int + expectedSecondaryIdx int + fleetServerURI string + targetVersion string }{ - {"nil input", nil, 1}, - {"empty input", []string{}, 1}, - {"valid input", []string{"pgp-bytes"}, 2}, + {"nil input", nil, 1, 0, -1, "", ""}, + {"empty input", []string{}, 1, 0, -1, "", ""}, + {"valid input with pgp", []string{"pgp-bytes"}, 2, 1, -1, "", ""}, + {"valid input with pgp and version, no fleet uri", []string{"pgp-bytes"}, 2, 1, -1, "", "1.2.3"}, + {"valid input with pgp and version and fleet uri", []string{"pgp-bytes"}, 3, 1, 2, "some-uri", "1.2.3"}, + {"valid input with pgp and fleet uri no version", []string{"pgp-bytes"}, 2, 1, -1, "some-uri", ""}, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - res := appendFallbackPGP(tc.passedBytes) + l, _ := logger.NewTesting(tc.name) + u := Upgrader{ + fleetServerURI: tc.fleetServerURI, + log: l, + } + res := u.appendFallbackPGP(tc.targetVersion, tc.passedBytes) // check default fallback is passed and is very last require.NotNil(t, res) require.Equal(t, tc.expectedLen, len(res)) - require.Equal(t, download.PgpSourceURIPrefix+defaultUpgradeFallbackPGP, res[len(res)-1]) + require.Equal(t, download.PgpSourceURIPrefix+defaultUpgradeFallbackPGP, res[tc.expectedDefaultIdx]) + + if tc.expectedSecondaryIdx >= 0 { + // last element is fleet uri + expectedPgpURI := download.PgpSourceURIPrefix + tc.fleetServerURI + strings.Replace(fleetUpgradeFallbackPGPFormat, "%d.%d.%d", tc.targetVersion, 1) + require.Equal(t, expectedPgpURI, res[len(res)-1]) + } }) } } diff --git a/internal/pkg/agent/application/upgrade/upgrade.go b/internal/pkg/agent/application/upgrade/upgrade.go index e653cf54525..3cc9272a271 100644 --- a/internal/pkg/agent/application/upgrade/upgrade.go +++ b/internal/pkg/agent/application/upgrade/upgrade.go @@ -23,6 +23,7 @@ import ( "github.com/elastic/elastic-agent/internal/pkg/agent/errors" "github.com/elastic/elastic-agent/internal/pkg/fleetapi" "github.com/elastic/elastic-agent/internal/pkg/fleetapi/acker" + fleetclient "github.com/elastic/elastic-agent/internal/pkg/fleetapi/client" "github.com/elastic/elastic-agent/internal/pkg/release" "github.com/elastic/elastic-agent/pkg/core/logger" ) @@ -45,10 +46,11 @@ var ErrSameVersion = errors.New("upgrade did not occur because its the same vers // Upgrader performs an upgrade type Upgrader struct { - log *logger.Logger - settings *artifact.Config - agentInfo *info.AgentInfo - upgradeable bool + log *logger.Logger + settings *artifact.Config + agentInfo *info.AgentInfo + upgradeable bool + fleetServerURI string } // IsUpgradeable when agent is installed and running as a service or flag was provided. @@ -68,6 +70,17 @@ func NewUpgrader(log *logger.Logger, settings *artifact.Config, agentInfo *info. } } +// SetClient reloads URI based on up to date fleet client +func (u *Upgrader) SetClient(c fleetclient.Sender) { + if c == nil { + u.log.Debug("client nil, resetting Fleet Server URI") + u.fleetServerURI = "" + } + + u.fleetServerURI = c.URI() + u.log.Debugf("Set client changed URI to %s", u.fleetServerURI) +} + // Reload reloads the artifact configuration for the upgrader. func (u *Upgrader) Reload(rawConfig *config.Config) error { type reloadConfig struct { diff --git a/internal/pkg/remote/client.go b/internal/pkg/remote/client.go index 078016d1d87..9a3f3d974d3 100644 --- a/internal/pkg/remote/client.go +++ b/internal/pkg/remote/client.go @@ -247,6 +247,10 @@ func (c *Client) checkApiVersionHeaders(reqID string, resp *http.Response) { // URI returns the remote URI. func (c *Client) URI() string { host := c.config.GetHosts()[0] + if strings.HasPrefix(host, string(ProtocolHTTPS)+"://") || + strings.HasPrefix(host, string(ProtocolHTTP)+"://") { + return host + "/" + c.config.Path + } return string(c.config.Protocol) + "://" + host + "/" + c.config.Path }