From 635a78699551432addc3f321725bab17930ec1b4 Mon Sep 17 00:00:00 2001 From: Anderson Queiroz Date: Thu, 12 Oct 2023 08:38:17 +0200 Subject: [PATCH] remove PGP signature verification skip for DEV builds --- dev-tools/cmd/buildpgp/build_pgp.go | 6 +- .../application/coordinator/coordinator.go | 4 +- .../upgrade/artifact/download/fs/verifier.go | 99 ++++---------- .../artifact/download/fs/verifier_test.go | 11 +- .../artifact/download/http/elastic_test.go | 2 +- .../artifact/download/http/verifier.go | 121 +++++++++--------- .../artifact/download/localremote/verifier.go | 4 +- .../artifact/download/snapshot/verifier.go | 2 +- .../upgrade/artifact/download/verifier.go | 98 ++++++++++++-- .../application/upgrade/step_download.go | 6 +- internal/pkg/agent/cmd/run.go | 2 +- internal/pkg/release/pgp.go | 6 +- internal/pkg/release/version.go | 2 +- testing/integration/upgrade_gpg_test.go | 8 +- 14 files changed, 199 insertions(+), 172 deletions(-) diff --git a/dev-tools/cmd/buildpgp/build_pgp.go b/dev-tools/cmd/buildpgp/build_pgp.go index 8559ea04c32..659fc1acbd1 100644 --- a/dev-tools/cmd/buildpgp/build_pgp.go +++ b/dev-tools/cmd/buildpgp/build_pgp.go @@ -49,9 +49,9 @@ func init() { pgpBytes = packer.MustUnpack("{{ .Pack }}")["GPG-KEY-elasticsearch"] } -// PGP return pgpbytes and a flag describing whether or not no pgp is valid. -func PGP() (bool, []byte) { - return allowEmptyPgp == "true", pgpBytes +// PGP return pgpbytes. +func PGP() []byte { + return pgpBytes } `)) diff --git a/internal/pkg/agent/application/coordinator/coordinator.go b/internal/pkg/agent/application/coordinator/coordinator.go index 856c624076f..ff0d0617936 100644 --- a/internal/pkg/agent/application/coordinator/coordinator.go +++ b/internal/pkg/agent/application/coordinator/coordinator.go @@ -108,7 +108,7 @@ type RuntimeManager interface { // it performs diagnostics for all current units. PerformDiagnostics(context.Context, ...runtime.ComponentUnitDiagnosticRequest) []runtime.ComponentUnitDiagnostic - //PerformComponentDiagnostics executes the diagnostic action for the provided components. If no components are provided, + // PerformComponentDiagnostics executes the diagnostic action for the provided components. If no components are provided, // then it performs the diagnostics for all current units. PerformComponentDiagnostics(ctx context.Context, additionalMetrics []cproto.AdditionalDiagnosticRequest, req ...component.Component) ([]runtime.ComponentDiagnostic, error) } @@ -425,7 +425,7 @@ func (c *Coordinator) ReExec(callback reexec.ShutdownCallbackFn, argOverrides .. // Upgrade runs the upgrade process. // Called from external goroutines. func (c *Coordinator) Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, skipVerifyOverride bool, skipDefaultPgp bool, pgpBytes ...string) error { - // early check outside of upgrader before overridding the state + // early check outside of upgrader before overriding the state if !c.upgradeMgr.Upgradeable() { return ErrNotUpgradable } 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 263f6ea8bf5..53716f90ae9 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier.go +++ b/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier.go @@ -6,7 +6,6 @@ package fs import ( "fmt" - "io/ioutil" "net/http" "os" "path/filepath" @@ -26,11 +25,10 @@ const ( // The signature is validated against Elastic's public GPG key that is // embedded into Elastic Agent. type Verifier struct { - config *artifact.Config - client http.Client - pgpBytes []byte - allowEmptyPgp bool - log *logger.Logger + config *artifact.Config + client http.Client + defaultKey []byte + log *logger.Logger } func (v *Verifier) Name() string { @@ -39,8 +37,8 @@ func (v *Verifier) Name() string { // NewVerifier creates a verifier checking downloaded package on preconfigured // location against a key stored on elastic.co website. -func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool, pgp []byte) (*Verifier, error) { - if len(pgp) == 0 && !allowEmptyPgp { +func NewVerifier(log *logger.Logger, config *artifact.Config, pgp []byte) (*Verifier, error) { + if len(pgp) == 0 { return nil, errors.New("expecting PGP but retrieved none", errors.TypeSecurity) } @@ -55,11 +53,10 @@ func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool } v := &Verifier{ - config: config, - client: *client, - allowEmptyPgp: allowEmptyPgp, - pgpBytes: pgp, - log: log, + config: config, + client: *client, + defaultKey: pgp, + log: log, } return v, nil @@ -70,24 +67,22 @@ func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool func (v *Verifier) Verify(a artifact.Artifact, version string, skipDefaultPgp bool, pgpBytes ...string) error { filename, err := artifact.GetArtifactName(a, version, v.config.OS(), v.config.Arch()) if err != nil { - return errors.New(err, "retrieving package name") + return fmt.Errorf("could not get artifact name: %w", err) } - fullPath := filepath.Join(v.config.TargetDirectory, filename) + artifactPath := filepath.Join(v.config.TargetDirectory, filename) - if err = download.VerifySHA512Hash(fullPath); err != nil { - var checksumMismatchErr *download.ChecksumMismatchError - if errors.As(err, &checksumMismatchErr) { - os.Remove(fullPath) - os.Remove(fullPath + ".sha512") - } - return err + if err = download.VerifySHA512HashWithCleanup(v.log, artifactPath); err != nil { + return fmt.Errorf("failed to verify SHA512 hash: %w", err) } - if err = v.verifyAsc(fullPath, skipDefaultPgp, pgpBytes...); err != nil { + if err = v.verifyAsc(artifactPath, skipDefaultPgp, pgpBytes...); err != nil { var invalidSignatureErr *download.InvalidSignatureError if errors.As(err, &invalidSignatureErr) { - os.Remove(fullPath + ".asc") + if err := os.Remove(artifactPath + ".asc"); err != nil { + v.log.Warnf("failed clean up after signature verification: failed to remove %q: %v", + artifactPath+".asc", err) + } } return err } @@ -113,63 +108,25 @@ func (v *Verifier) Reload(c *artifact.Config) error { return nil } -func (v *Verifier) verifyAsc(fullPath string, skipDefaultPgp bool, pgpSources ...string) error { +func (v *Verifier) verifyAsc(fullPath string, skipDefaultKey bool, pgpSources ...string) error { var pgpBytes [][]byte - if len(v.pgpBytes) > 0 && !skipDefaultPgp { - v.log.Infof("Default PGP being appended") - pgpBytes = append(pgpBytes, v.pgpBytes) - } - - for _, check := range pgpSources { - if len(check) == 0 { - continue - } - raw, err := download.PgpBytesFromSource(v.log, check, &v.client) - if err != nil { - return err - } - - if len(raw) == 0 { - continue - } - - pgpBytes = append(pgpBytes, raw) - } - - if len(pgpBytes) == 0 { - // no pgp available skip verification process - v.log.Infof("No checks defined") - return nil + pgpBytes, err := download.FetchPGPKeys( + v.log, v.client, v.defaultKey, skipDefaultKey, pgpSources) + if err != nil { + return fmt.Errorf("could not fetch pgp keys: %w", err) } - v.log.Infof("Using %d PGP keys", len(pgpBytes)) ascBytes, err := v.getPublicAsc(fullPath) - if err != nil && v.allowEmptyPgp { - // asc not available but we allow empty for dev use-case - return nil - } else if err != nil { - return err - } - - for i, check := range pgpBytes { - err = download.VerifyGPGSignature(fullPath, ascBytes, check) - if err == nil { - // verify successful - v.log.Infof("Verification with PGP[%d] successful", i) - return nil - } - v.log.Warnf("Verification with PGP[%d] succfailed: %v", i, err) + if err != nil { + return fmt.Errorf("could not get .asc file: %v", err) } - v.log.Warnf("Verification failed") - - // return last error - return err + return download.VerifyPGPSignatures(v.log, fullPath, ascBytes, pgpBytes) } func (v *Verifier) getPublicAsc(fullPath string) ([]byte, error) { fullPath = fmt.Sprintf("%s%s", fullPath, ascSuffix) - b, err := ioutil.ReadFile(fullPath) + b, err := os.ReadFile(fullPath) if err != nil { return nil, errors.New(err, fmt.Sprintf("fetching asc file from '%s'", fullPath), errors.TypeFilesystem, errors.M(errors.MetaKeyPath, fullPath)) } diff --git a/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier_test.go b/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier_test.go index 5012e8244dd..ed2e586a16e 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier_test.go +++ b/internal/pkg/agent/application/upgrade/artifact/download/fs/verifier_test.go @@ -64,7 +64,7 @@ func TestFetchVerify(t *testing.T) { assert.NoError(t, err) downloader := NewDownloader(config) - verifier, err := NewVerifier(log, config, true, nil) + verifier, err := NewVerifier(log, config, nil) assert.NoError(t, err) // first download verify should fail: @@ -96,12 +96,9 @@ func TestFetchVerify(t *testing.T) { err = verifier.Verify(s, version, false) assert.NoError(t, err) - // Enable GPG signature validation. - verifier.allowEmptyPgp = false - // Bad GPG public key. { - verifier.pgpBytes = []byte("garbage") + verifier.defaultKey = []byte("garbage") // Don't delete anything. assertFileExists(t, targetFilePath) @@ -109,7 +106,7 @@ func TestFetchVerify(t *testing.T) { } // Setup proper GPG public key. - _, verifier.pgpBytes = release.PGP() + verifier.defaultKey = release.PGP() // Missing .asc file. { @@ -216,7 +213,7 @@ func TestVerify(t *testing.T) { _, err = os.Stat(artifact) require.NoError(t, err) - testVerifier, err := NewVerifier(log, config, true, nil) + testVerifier, err := NewVerifier(log, config, nil) require.NoError(t, err) err = testVerifier.Verify(beatSpec, version, false, tc.RemotePGPUris...) diff --git a/internal/pkg/agent/application/upgrade/artifact/download/http/elastic_test.go b/internal/pkg/agent/application/upgrade/artifact/download/http/elastic_test.go index bd1564cab2b..6af2efa67e2 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/http/elastic_test.go +++ b/internal/pkg/agent/application/upgrade/artifact/download/http/elastic_test.go @@ -122,7 +122,7 @@ func TestVerify(t *testing.T) { t.Fatal(err) } - testVerifier, err := NewVerifier(log, config, true, nil) + testVerifier, err := NewVerifier(log, config, nil) if err != nil { t.Fatal(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 84f3f6045f0..995d4fe49e4 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go +++ b/internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go @@ -29,11 +29,10 @@ const ( // Verifier verifies a downloaded package by comparing with public ASC // file from elastic.co website. type Verifier struct { - config *artifact.Config - client http.Client - pgpBytes []byte - allowEmptyPgp bool - log progressLogger + config *artifact.Config + client http.Client + defaultKey []byte + log progressLogger } func (v *Verifier) Name() string { @@ -42,8 +41,8 @@ func (v *Verifier) Name() string { // NewVerifier create a verifier checking downloaded package on preconfigured // location against a key stored on elastic.co website. -func NewVerifier(log progressLogger, config *artifact.Config, allowEmptyPgp bool, pgp []byte) (*Verifier, error) { - if len(pgp) == 0 && !allowEmptyPgp { +func NewVerifier(log progressLogger, config *artifact.Config, pgp []byte) (*Verifier, error) { + if len(pgp) == 0 { return nil, errors.New("expecting PGP but retrieved none", errors.TypeSecurity) } @@ -58,11 +57,10 @@ func NewVerifier(log progressLogger, config *artifact.Config, allowEmptyPgp bool } v := &Verifier{ - config: config, - client: *client, - allowEmptyPgp: allowEmptyPgp, - pgpBytes: pgp, - log: log, + config: config, + client: *client, + defaultKey: pgp, + log: log, } return v, nil @@ -89,24 +87,26 @@ func (v *Verifier) Reload(c *artifact.Config) error { // Verify checks downloaded package on preconfigured // location against a key stored on elastic.co website. func (v *Verifier) Verify(a artifact.Artifact, version string, skipDefaultPgp bool, pgpBytes ...string) error { - fullPath, err := artifact.GetArtifactPath(a, version, v.config.OS(), v.config.Arch(), v.config.TargetDirectory) + artifactPath, err := artifact.GetArtifactPath(a, version, v.config.OS(), v.config.Arch(), v.config.TargetDirectory) if err != nil { return errors.New(err, "retrieving package path") } - if err = download.VerifySHA512Hash(fullPath); err != nil { - var checksumMismatchErr *download.ChecksumMismatchError - if errors.As(err, &checksumMismatchErr) { - os.Remove(fullPath) - os.Remove(fullPath + ".sha512") - } - return err + if err = download.VerifySHA512HashWithCleanup(v.log, artifactPath); err != nil { + return fmt.Errorf("failed to verify SHA512 hash: %w", err) } if err = v.verifyAsc(a, version, skipDefaultPgp, pgpBytes...); err != nil { var invalidSignatureErr *download.InvalidSignatureError if errors.As(err, &invalidSignatureErr) { - os.Remove(fullPath + ".asc") + if err := os.Remove(artifactPath); err != nil { + v.log.Warnf("failed clean up after signature verification: failed to remove %q: %v", + artifactPath, err) + } + if err := os.Remove(artifactPath + ascSuffix); err != nil { + v.log.Warnf("failed clean up after sha512 check: failed to remove %q: %v", + artifactPath+ascSuffix, err) + } } return err } @@ -114,11 +114,41 @@ func (v *Verifier) Verify(a artifact.Artifact, version string, skipDefaultPgp bo return nil } -func (v *Verifier) verifyAsc(a artifact.Artifact, version string, skipDefaultPgp bool, pgpSources ...string) error { +func (v *Verifier) verifyAsc(a artifact.Artifact, version string, skipDefaultKey bool, pgpSources ...string) error { + filename, err := artifact.GetArtifactName(a, version, v.config.OS(), v.config.Arch()) + if err != nil { + return errors.New(err, "retrieving package name") + } + + fullPath, err := artifact.GetArtifactPath(a, version, v.config.OS(), v.config.Arch(), v.config.TargetDirectory) + if err != nil { + return errors.New(err, "retrieving package path") + } + + ascURI, err := v.composeURI(filename, a.Artifact) + if err != nil { + return errors.New(err, "composing URI for fetching asc file", errors.TypeNetwork) + } + + ascBytes, err := v.getPublicAsc(ascURI) + if err != nil { + return errors.New(err, fmt.Sprintf("fetching asc file from %s", ascURI), errors.TypeNetwork, errors.M(errors.MetaKeyURI, ascURI)) + } + + pgpBytes, err := download.FetchPGPKeys( + v.log, v.client, v.defaultKey, skipDefaultKey, pgpSources) + if err != nil { + return fmt.Errorf("could not fetch pgp keys: %w", err) + } + + return download.VerifyPGPSignatures(v.log, fullPath, ascBytes, pgpBytes) +} + +func funcName(v *Verifier, skipDefaultPgp bool, pgpSources []string) ([][]byte, error, bool) { var pgpBytes [][]byte - if len(v.pgpBytes) > 0 && !skipDefaultPgp { + if len(v.defaultKey) > 0 && !skipDefaultPgp { v.log.Infof("Default PGP being appended") - pgpBytes = append(pgpBytes, v.pgpBytes) + pgpBytes = append(pgpBytes, v.defaultKey) } for _, check := range pgpSources { @@ -127,7 +157,7 @@ func (v *Verifier) verifyAsc(a artifact.Artifact, version string, skipDefaultPgp } raw, err := download.PgpBytesFromSource(v.log, check, &v.client) if err != nil { - return err + return nil, err, true } if len(raw) == 0 { @@ -140,47 +170,10 @@ func (v *Verifier) verifyAsc(a artifact.Artifact, version string, skipDefaultPgp if len(pgpBytes) == 0 { // no pgp available skip verification process v.log.Infof("No checks defined") - return nil + return nil, nil, true } v.log.Infof("Using %d PGP keys", len(pgpBytes)) - - filename, err := artifact.GetArtifactName(a, version, v.config.OS(), v.config.Arch()) - if err != nil { - return errors.New(err, "retrieving package name") - } - - fullPath, err := artifact.GetArtifactPath(a, version, v.config.OS(), v.config.Arch(), v.config.TargetDirectory) - if err != nil { - return errors.New(err, "retrieving package path") - } - - ascURI, err := v.composeURI(filename, a.Artifact) - if err != nil { - return errors.New(err, "composing URI for fetching asc file", errors.TypeNetwork) - } - - ascBytes, err := v.getPublicAsc(ascURI) - if err != nil && v.allowEmptyPgp { - // asc not available but we allow empty for dev use-case - return nil - } else if err != nil { - return errors.New(err, fmt.Sprintf("fetching asc file from %s", ascURI), errors.TypeNetwork, errors.M(errors.MetaKeyURI, ascURI)) - } - - for i, check := range pgpBytes { - err = download.VerifyGPGSignature(fullPath, ascBytes, check) - if err == nil { - // verify successful - v.log.Infof("Verification with PGP[%d] successful", i) - return nil - } - v.log.Warnf("Verification with PGP[%d] failed: %v", i, err) - } - - v.log.Warnf("Verification failed") - - // return last error - return err + return pgpBytes, nil, false } func (v *Verifier) composeURI(filename, artifactName string) (string, error) { diff --git a/internal/pkg/agent/application/upgrade/artifact/download/localremote/verifier.go b/internal/pkg/agent/application/upgrade/artifact/download/localremote/verifier.go index fc2c3a806be..3e64fb7867e 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/localremote/verifier.go +++ b/internal/pkg/agent/application/upgrade/artifact/download/localremote/verifier.go @@ -20,7 +20,7 @@ import ( func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool, pgp []byte) (download.Verifier, error) { verifiers := make([]download.Verifier, 0, 3) - fsVer, err := fs.NewVerifier(log, config, allowEmptyPgp, pgp) + fsVer, err := fs.NewVerifier(log, config, pgp) if err != nil { return nil, err } @@ -38,7 +38,7 @@ func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool } } - remoteVer, err := http.NewVerifier(log, config, allowEmptyPgp, pgp) + remoteVer, err := http.NewVerifier(log, config, pgp) if err != nil { return nil, err } diff --git a/internal/pkg/agent/application/upgrade/artifact/download/snapshot/verifier.go b/internal/pkg/agent/application/upgrade/artifact/download/snapshot/verifier.go index 302aa93e766..71c8341a7ea 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/snapshot/verifier.go +++ b/internal/pkg/agent/application/upgrade/artifact/download/snapshot/verifier.go @@ -29,7 +29,7 @@ func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool if err != nil { return nil, err } - v, err := http.NewVerifier(log, cfg, allowEmptyPgp, pgp) + v, err := http.NewVerifier(log, cfg, pgp) if err != nil { return nil, errors.New(err, "failed to create snapshot verifier") } diff --git a/internal/pkg/agent/application/upgrade/artifact/download/verifier.go b/internal/pkg/agent/application/upgrade/artifact/download/verifier.go index 662367f4909..a314b9384f1 100644 --- a/internal/pkg/agent/application/upgrade/artifact/download/verifier.go +++ b/internal/pkg/agent/application/upgrade/artifact/download/verifier.go @@ -70,18 +70,41 @@ func (e *InvalidSignatureError) Error() string { // Unwrap returns the cause. func (e *InvalidSignatureError) Unwrap() error { return e.Err } -// Verifier is an interface verifying the SHA512 checksum and GPG signature and +// Verifier is an interface verifying the SHA512 checksum and PGP signature and // of a downloaded artifact. type Verifier interface { Name() string - // Verify should verify the artifact and return if succeed status (true|false) and an error if any checks fail. + // Verify should verify the artifact, returning an error if any checks fail. // If the checksum does no match Verify returns a - // *download.ChecksumMismatchError. And if the GPG signature is invalid then + // *download.ChecksumMismatchError. Ff the PGP signature is invalid then // Verify returns a *download.InvalidSignatureError. Use errors.As() to // check error types. Verify(a artifact.Artifact, version string, skipDefaultPgp bool, pgpBytes ...string) error } +// VerifySHA512HashWithCleanup calls VerifySHA512Hash and, in case of a +// *ChecksumMismatchError, performs a cleanup by deleting both the filename and +// filename.sha512 files. If the cleanup fails, it logs a warning. +func VerifySHA512HashWithCleanup(log logger, filename string) error { + if err := VerifySHA512Hash(filename); err != nil { + var checksumMismatchErr *ChecksumMismatchError + if errors.As(err, &checksumMismatchErr) { + if err := os.Remove(filename); err != nil { + log.Warnf("failed clean up after sha512 verification: failed to remove %q: %v", + filename, err) + } + if err := os.Remove(filename + ".sha512"); err != nil { + log.Warnf("failed clean up after sha512 check: failed to remove %q: %v", + filename+".sha512", err) + } + } + + return err + } + + return nil +} + // VerifySHA512Hash checks that a sidecar file containing a sha512 checksum // exists and that the checksum in the sidecar file matches the checksum of // the file. It returns an error if validation fails. @@ -89,7 +112,7 @@ func VerifySHA512Hash(filename string) error { // Read expected checksum. expectedHash, err := readChecksumFile(filename+".sha512", filepath.Base(filename)) if err != nil { - return err + return fmt.Errorf("could not read checksum file: %v", err) } // Compute sha512 checksum. @@ -101,10 +124,10 @@ func VerifySHA512Hash(filename string) error { hash := sha512.New() if _, err := io.Copy(hash, f); err != nil { - return err + return fmt.Errorf("faled to read file to calculate hash") } - computedHash := hex.EncodeToString(hash.Sum(nil)) + computedHash := hex.EncodeToString(hash.Sum(nil)) if computedHash != expectedHash { return &ChecksumMismatchError{ Expected: expectedHash, @@ -157,11 +180,34 @@ func readChecksumFile(checksumFile, filename string) (string, error) { return checksum, nil } -// VerifyGPGSignature verifies the GPG signature of a file. It accepts the path +// progressLogger is a logger that only needs to implement Infof and Warnf, as those are the only functions +// that the downloadProgressReporter uses. +type logger interface { + Infof(format string, args ...interface{}) + Warnf(format string, args ...interface{}) +} + +func VerifyPGPSignatures( + log logger, file string, asciiArmorSignature []byte, publicKeys [][]byte) error { + var err error + for i, key := range publicKeys { + err := VerifyPGPSignature(file, asciiArmorSignature, key) + if err == nil { + log.Infof("Verification with PGP[%d] successful", i) + return nil + } + log.Warnf("Verification with PGP[%d] failed: %v", i, err) + } + + log.Warnf("Verification failed: %v", err) + return fmt.Errorf("could not verify PGP signature of %q: %w", file, err) +} + +// VerifyPGPSignature verifies the GPG signature of a file. It accepts the path // to the file to verify, the ASCII armored signature, and the public key to // check against. If there is a problem with the signature then a // *download.InvalidSignatureError is returned. -func VerifyGPGSignature(file string, asciiArmorSignature, publicKey []byte) error { +func VerifyPGPSignature(file string, asciiArmorSignature, publicKey []byte) error { keyring, err := openpgp.ReadArmoredKeyRing(bytes.NewReader(publicKey)) if err != nil { return errors.New(err, "read armored key ring", errors.TypeSecurity) @@ -181,6 +227,39 @@ func VerifyGPGSignature(file string, asciiArmorSignature, publicKey []byte) erro return nil } +func FetchPGPKeys(log logger, client http.Client, defaultPGPKey []byte, skipDefaultPGP bool, pgpSources []string) ([][]byte, error) { + var pgpKeys [][]byte + if len(defaultPGPKey) > 0 && !skipDefaultPGP { + pgpKeys = append(pgpKeys, defaultPGPKey) + log.Infof("Default PGP appended") + } + + for _, check := range pgpSources { + if len(check) == 0 { + continue + } + + raw, err := PgpBytesFromSource(log, check, &client) + if err != nil { + return nil, err + } + + if len(raw) == 0 { + continue + } + + pgpKeys = append(pgpKeys, raw) + } + + if len(pgpKeys) == 0 { + log.Infof("No PGP key available, skipping verification process") + return nil, nil + } + + log.Infof("Using %d PGP keys", len(pgpKeys)) + return pgpKeys, nil +} + func PgpBytesFromSource(log warnLogger, source string, client HTTPClient) ([]byte, error) { if strings.HasPrefix(source, PgpSourceRawPrefix) { return []byte(strings.TrimPrefix(source, PgpSourceRawPrefix)), nil @@ -191,7 +270,8 @@ func PgpBytesFromSource(log warnLogger, source string, client HTTPClient) ([]byt 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") + log.Warnf("Failed to fetch remote PGP key from %q: %v", + strings.TrimPrefix(source, PgpSourceURIPrefix), err) } return pgpBytes, nil diff --git a/internal/pkg/agent/application/upgrade/step_download.go b/internal/pkg/agent/application/upgrade/step_download.go index a273460f337..3f46e2b2570 100644 --- a/internal/pkg/agent/application/upgrade/step_download.go +++ b/internal/pkg/agent/application/upgrade/step_download.go @@ -143,13 +143,13 @@ func newDownloader(version *agtversion.ParsedSemVer, log *logger.Logger, setting } func newVerifier(version *agtversion.ParsedSemVer, log *logger.Logger, settings *artifact.Config) (download.Verifier, error) { - allowEmptyPgp, pgp := release.PGP() + pgp := release.PGP() if !version.IsSnapshot() { return localremote.NewVerifier(log, settings, allowEmptyPgp, pgp) } - fsVerifier, err := fs.NewVerifier(log, settings, allowEmptyPgp, pgp) + fsVerifier, err := fs.NewVerifier(log, settings, pgp) if err != nil { return nil, err } @@ -159,7 +159,7 @@ func newVerifier(version *agtversion.ParsedSemVer, log *logger.Logger, settings return nil, err } - remoteVerifier, err := http.NewVerifier(log, settings, allowEmptyPgp, pgp) + remoteVerifier, err := http.NewVerifier(log, settings, pgp) if err != nil { return nil, err } diff --git a/internal/pkg/agent/cmd/run.go b/internal/pkg/agent/cmd/run.go index 353c9d1e7a7..4ea084cac93 100644 --- a/internal/pkg/agent/cmd/run.go +++ b/internal/pkg/agent/cmd/run.go @@ -218,7 +218,7 @@ func run(override cfgOverrider, testingMode bool, fleetInitTimeout time.Duration l.Error(errors.New(err, "failed to invoke rollback watcher")) } - if allowEmptyPgp, _ := release.PGP(); allowEmptyPgp { + if _ := release.PGP(); allowEmptyPgp { l.Info("Elastic Agent has been built with security disabled. Elastic Agent will not verify signatures of upgrade artifact.") } diff --git a/internal/pkg/release/pgp.go b/internal/pkg/release/pgp.go index 49d4cfc32a9..9644f6778f3 100644 --- a/internal/pkg/release/pgp.go +++ b/internal/pkg/release/pgp.go @@ -19,7 +19,7 @@ func init() { pgpBytes = packer.MustUnpack("eJyMlsuOtLoBhPd5jH9/JBua0U+ksxguhobGjI0v4B1gNDQ2NNPNTNNEeffoRMoqipR1Va1KVfr+8Sv5SP7I4+aPwbaP7do/hvbej7/+/utSge1SwZBynbFrQCh8/+RIhCy20TnekKyCkMV+VL3+8oEttMr2C1475/R2jvW3FkF6TpvXZXr/Lhj5zGNdisovWunBITR5OENKuRdSY44qxT/E4ICiMdZJVlazd2pssJMJOTT2AHHx3iYclLVKZI1bNtmMwfWQdlz6SI9Vst6wwTkxJCfVdqVAfWjX3pqZuE1NDixX2lod6AN9FA6eZY0vRMJkqLagn3BRxRi3sDk6uB59vAYE0kwB/NKOd29l8VOSNRJyX7nkRzHRXRv/KlhG+UIJjtWjSNe6cdT1AouTEPZNwLGuuILVgrA23GMSVZKhq4Yi1Mv600vksFi34Xw7OGh2DoOPHNIQC/Sqku3F+Rj2DmxysJqGKYORfejo80dHKtIGugqiskuzx2DsRyk0z6Et8bKy3MV7lZC8EPZycZDCNbp1YC/b9N2jL/88JOPEoYpasO8lwkwnt13a284P+6V5Rjo4ykKsNZuzEzVeqoF/uwBPUWc39P32ah0YqgkrBfCu+P5WJejWA4T6VAUdGzmZY5czWxcTispUBcSIkHCUFigTYsFKxqdnI3Q52LWmi1XFrJfQbOh/aYJlj27Oti4V7cCzQtrx3sc2HpKtlWidmwmHg0Xpf+dNLlm2aHk+qtkjnbOFDCAqYkQrjjIFRSTkOqnaLpXMAg1tQSBaqhi5FdcJlzoiEP2wOAslx4EGIqoSjchzTXCMAuLoO2fBdyHhlc32g9dZNcxroAB6CCRoIeHSHfi7OlRI4XhtJ9oQvjparDE9gnPF91XU5N6j4HuYehjO3qGmoBTOuPOJehSoR1cHsHHpQhzIabidB2uBmOzcR3YXL/+qoYKVwaWOt6Y98KHSVRGeqRzaRxFjO1x/5xgUL+qKlAD/0cynZ5eKiFnikWXMu/kTdLzw2tCfKmCOgYkvbW+udpCjHQ6Kmtqe9ztGzZ1KP+HO6WA2uHRk5UOCYxGrl5g064QiQ4zDBqI3xd6fDVhvfZpdhmSD0kDCkdr7OGM5WGeKEBisoDT0Cw7U3MbjVylFnMPRhtNosIvdHI7OEJ9fDT8B5Xi8Q9lba557Z2nVR8GtuPoZA/reCztXvPd6qzcO9rJLxxuZMpA7xZMv+EvFGuPwd65E9uBIe12srsLACdfBRtMg02l2FxB/DyabWwc2LMIOO7KmgNrRDmR0trSZt5AiVHHgnTDYAi51gPkquvf1rQGwxpU/6dq2fBrHDuGxAdveInzVs8/6Gl0HM14aKKic4WMwHq6A5oRnCT2Qx5dPr79ublP5s14Qwcbnoc3euKGOsMVP49pJu8VJik+XJhR2TJ/KVGQlE9eytrao/GZg4ruDAerYONFwu/XJ/t0jfudxRqW5QRnfXlX1yAv59NhsT0NEXA2DVCYi6uJ9bxz6XSTIU+Hm5DBz1VT8SCTyflFTA/qXFKtoRQxaq9vW8U89iJ/YHU8YZF/5cy3++iRqthS7+MyNcpQIEsGClh8qp8unR8G6txaHjRkrPm9jVz2AkrAuXFErqGa5jFA7WBDntDe1LRXMQGi2lS6N11r0UkfGK6GRXlY1RBYMtebEHaccqIQJHXRJFg2zuXeM1jTcqBC6VobSInr3ROXXjAX3/ggu2u1zWmvWvnxBU8E7d0w7s4lippSILOAcha1ASJksVCD7a8c5sSj/T+9tijHh6Kcyoqwm23b1qtp5uxTv61QAmBFuUSnHgE/nZ1ejXIB96t2x0GlW9YeCzDS7WLInuT5Ad/0NsUACO/6JMLQKk31gYxPN6K5cnBQsy0NLRz17OeawaLjgXaIXicx9MNSqyR6DoEQmPm1c+hxmPDKr4k4Ko53fO3dsUUQYMulfu9h3hBtgfvVVkdxyvGQbBnbpDawFpw1FgdPBIG0NfBRLvLeC2oGPdyZWNAiRlwhfeged24h+icm6fWQvHS9O5PXwiKuxlluBn6vDmHDLBC/arD+k1kcf4aSbuEfqsdbJWBZwzKXE30wqppMt00IvfNKiWM73hp33AvWOnj0xIA46cHryBOJwxrBfsOKptWIKShn6Fy6FUNzDGmhOzdnhfGQXB33zGaVcCreV6723VnWQljpRjy4dM81sIyVciREVr37nvQjiLtZVF/uLsNSU9SiUGZPWXUPMsgkfAfogN79k2Y98v/2bbS5clDT8Pxjo888/f/3zb/8KAAD///dAGpU=")["GPG-KEY-elasticsearch"] } -// PGP return pgpbytes and a flag describing whether or not no pgp is valid. -func PGP() (bool, []byte) { - return allowEmptyPgp == "true", pgpBytes +// PGP return pgpbytes. +func PGP() []byte { + return pgpBytes } diff --git a/internal/pkg/release/version.go b/internal/pkg/release/version.go index 5ff0be1dc29..9c1644f7cb7 100644 --- a/internal/pkg/release/version.go +++ b/internal/pkg/release/version.go @@ -25,7 +25,7 @@ var complete = "ELASTIC_AGENT_COMPLETE" // allowEmptyPgp is used as a debug flag and allows working // without valid pgp -var allowEmptyPgp string +// var allowEmptyPgp string // allowUpgrade is used as a debug flag and allows working // with upgrade without requiring Agent to be installed correctly diff --git a/testing/integration/upgrade_gpg_test.go b/testing/integration/upgrade_gpg_test.go index 56d4378d147..e2001dc1eca 100644 --- a/testing/integration/upgrade_gpg_test.go +++ b/testing/integration/upgrade_gpg_test.go @@ -11,7 +11,6 @@ import ( "strings" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/elastic/elastic-agent/internal/pkg/release" @@ -55,7 +54,7 @@ func TestStandaloneUpgradeWithGPGFallback(t *testing.T) { t.Logf("Testing Elastic Agent upgrade from %s to %s...", define.Version(), upgradeToVersion) - _, defaultPGP := release.PGP() + defaultPGP := release.PGP() firstSeven := string(defaultPGP[:7]) newPgp := strings.Replace( string(defaultPGP), @@ -73,7 +72,7 @@ func TestStandaloneUpgradeWithGPGFallback(t *testing.T) { upgradetest.WithSourceURI(""), upgradetest.WithCustomPGP(customPGP), upgradetest.WithSkipVerify(false)) - assert.NoError(t, err) + require.NoError(t, err, "perform upgrade failed") } func TestStandaloneUpgradeWithGPGFallbackOneRemoteFailing(t *testing.T) { @@ -110,7 +109,7 @@ func TestStandaloneUpgradeWithGPGFallbackOneRemoteFailing(t *testing.T) { t.Logf("Testing Elastic Agent upgrade from %s to %s...", define.Version(), upgradeToVersion) - _, defaultPGP := release.PGP() + defaultPGP := release.PGP() firstSeven := string(defaultPGP[:7]) newPgp := strings.Replace( string(defaultPGP), @@ -129,4 +128,5 @@ func TestStandaloneUpgradeWithGPGFallbackOneRemoteFailing(t *testing.T) { upgradetest.WithSourceURI(""), upgradetest.WithCustomPGP(customPGP), upgradetest.WithSkipVerify(false)) + require.NoError(t, err, "perform upgrade failed") }