IT failed pgp test

control_v2.proto

@@ -97,6 +97,11 @@ message UpgradeRequest {
   //
   // If provided Elastic Agent package is checked against these pgp keys as well.
   repeated string pgpBytes = 4;
+
+  // (Optional) Overrides predefined behavior for agent package verification.
+  //
+  // If provided Elastic Agent package embedded PGP key is not checked for signature during upgrade.
+  bool skipDefaultPgp = 5;
 }
 
 // A upgrade response message.

internal/pkg/agent/application/actions/handlers/handler_action_upgrade.go

@@ -53,7 +53,7 @@ func (h *Upgrade) Handle(ctx context.Context, a fleetapi.Action, ack acker.Acker
 	}
 	go func() {
 		h.log.Infof("starting upgrade to version %s in background", action.Version)
-		if err := h.coord.Upgrade(asyncCtx, action.Version, action.SourceURI, action, false); err != nil {
+		if err := h.coord.Upgrade(asyncCtx, action.Version, action.SourceURI, action, false, false); err != nil {
 			h.log.Errorf("upgrade to version %s failed: %v", action.Version, err)
 			// If context is cancelled in getAsyncContext, the actions are acked there
 			if !errors.Is(asyncCtx.Err(), context.Canceled) {

internal/pkg/agent/application/actions/handlers/handler_action_upgrade_test.go

@@ -35,7 +35,7 @@ func (u *mockUpgradeManager) Reload(rawConfig *config.Config) error {
 	return nil
 }
 
-func (u *mockUpgradeManager) Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, skipVerifyOverride bool, pgpBytes ...string) (_ reexec.ShutdownCallbackFn, err error) {
+func (u *mockUpgradeManager) Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, skipVerifyOverride bool, skipDefaultPgp bool, pgpBytes ...string) (_ reexec.ShutdownCallbackFn, err error) {
 	select {
 	case <-time.After(2 * time.Second):
 		u.msgChan <- "completed " + version

internal/pkg/agent/application/coordinator/coordinator.go

@@ -58,7 +58,7 @@ type UpgradeManager interface {
 	Reload(rawConfig *config.Config) error
 
 	// Upgrade upgrades running agent.
-	Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, skipVerifyOverride bool, pgpBytes ...string) (_ reexec.ShutdownCallbackFn, err error)
+	Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, skipVerifyOverride bool, skipDefaultPgp bool, pgpBytes ...string) (_ reexec.ShutdownCallbackFn, err error)
 
 	// Ack is used on startup to check if the agent has upgraded and needs to send an ack for the action
 	Ack(ctx context.Context, acker acker.Acker) error
@@ -394,7 +394,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, pgpBytes ...string) error {
+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
 	if !c.upgradeMgr.Upgradeable() {
 		return ErrNotUpgradable
@@ -425,7 +425,7 @@ func (c *Coordinator) Upgrade(ctx context.Context, version string, sourceURI str
 
 	// override the overall state to upgrading until the re-execution is complete
 	c.SetOverrideState(agentclient.Upgrading, fmt.Sprintf("Upgrading to version %s", version))
-	cb, err := c.upgradeMgr.Upgrade(ctx, version, sourceURI, action, skipVerifyOverride, pgpBytes...)
+	cb, err := c.upgradeMgr.Upgrade(ctx, version, sourceURI, action, skipVerifyOverride, skipDefaultPgp, pgpBytes...)
 	if err != nil {
 		c.ClearOverrideState()
 		return err

internal/pkg/agent/application/coordinator/coordinator_test.go

@@ -447,7 +447,7 @@ func TestCoordinator_Upgrade(t *testing.T) {
 	require.NoError(t, err)
 	cfgMgr.Config(ctx, cfg)
 
-	err = coord.Upgrade(ctx, "9.0.0", "", nil, true)
+	err = coord.Upgrade(ctx, "9.0.0", "", nil, true, false)
 	require.ErrorIs(t, err, ErrNotUpgradable)
 	cancel()
 
@@ -558,7 +558,7 @@ func (f *fakeUpgradeManager) Reload(cfg *config.Config) error {
 	return nil
 }
 
-func (f *fakeUpgradeManager) Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, skipVerifyOverride bool, pgpBytes ...string) (_ reexec.ShutdownCallbackFn, err error) {
+func (f *fakeUpgradeManager) Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, skipVerifyOverride bool, skipDefaultPgp bool, pgpBytes ...string) (_ reexec.ShutdownCallbackFn, err error) {
 	f.upgradeCalled = true
 	if f.upgradeErr != nil {
 		return nil, f.upgradeErr

internal/pkg/agent/application/coordinator/coordinator_unit_test.go

@@ -722,7 +722,7 @@ func TestCoordinatorInitiatesUpgrade(t *testing.T) {
 	}
 
 	// Call upgrade and make sure the upgrade manager receives an Upgrade call
-	err := coord.Upgrade(ctx, "1.2.3", "", nil, false)
+	err := coord.Upgrade(ctx, "1.2.3", "", nil, false, false)
 	assert.True(t, upgradeMgr.upgradeCalled, "Coordinator Upgrade should call upgrade manager Upgrade")
 	assert.Equal(t, upgradeMgr.upgradeErr, err, "Upgrade should report upgrade manager error")
 

internal/pkg/agent/application/upgrade/artifact/download/composed/verifier.go

@@ -31,13 +31,13 @@ func NewVerifier(verifiers ...download.Verifier) *Verifier {
 }
 
 // Verify checks the package from configured source.
-func (e *Verifier) Verify(a artifact.Artifact, version string, pgpBytes ...string) error {
+func (e *Verifier) Verify(a artifact.Artifact, version string, skipDefaultPgp bool, pgpBytes ...string) error {
 	var err error
 	var checksumMismatchErr *download.ChecksumMismatchError
 	var invalidSignatureErr *download.InvalidSignatureError
 
 	for _, v := range e.vv {
-		e := v.Verify(a, version, pgpBytes...)
+		e := v.Verify(a, version, skipDefaultPgp, pgpBytes...)
 		if e == nil {
 			// Success
 			return nil

internal/pkg/agent/application/upgrade/artifact/download/composed/verifier_test.go

@@ -18,7 +18,7 @@ type ErrorVerifier struct {
 	called bool
 }
 
-func (d *ErrorVerifier) Verify(a artifact.Artifact, version string, _ ...string) error {
+func (d *ErrorVerifier) Verify(a artifact.Artifact, version string, _ bool, _ ...string) error {
 	d.called = true
 	return errors.New("failing")
 }
@@ -29,7 +29,7 @@ type FailVerifier struct {
 	called bool
 }
 
-func (d *FailVerifier) Verify(a artifact.Artifact, version string, _ ...string) error {
+func (d *FailVerifier) Verify(a artifact.Artifact, version string, _ bool, _ ...string) error {
 	d.called = true
 	return &download.InvalidSignatureError{}
 }
@@ -40,7 +40,7 @@ type SuccVerifier struct {
 	called bool
 }
 
-func (d *SuccVerifier) Verify(a artifact.Artifact, version string, _ ...string) error {
+func (d *SuccVerifier) Verify(a artifact.Artifact, version string, _ bool, _ ...string) error {
 	d.called = true
 	return nil
 }
@@ -74,7 +74,7 @@ func TestVerifier(t *testing.T) {
 
 	for _, tc := range testCases {
 		d := NewVerifier(tc.verifiers[0], tc.verifiers[1], tc.verifiers[2])
-		err := d.Verify(artifact.Artifact{Name: "a", Cmd: "a", Artifact: "a/a"}, "b")
+		err := d.Verify(artifact.Artifact{Name: "a", Cmd: "a", Artifact: "a/a"}, "b", false)
 
 		assert.Equal(t, tc.expectedResult, err == nil)
 

internal/pkg/agent/application/upgrade/artifact/download/fs/verifier.go

@@ -63,7 +63,7 @@ func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool
 
 // Verify checks downloaded package on preconfigured
 // location against a key stored on elastic.co website.
-func (v *Verifier) Verify(a artifact.Artifact, version string, pgpBytes ...string) error {
+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")
@@ -80,7 +80,7 @@ func (v *Verifier) Verify(a artifact.Artifact, version string, pgpBytes ...strin
 		return err
 	}
 
-	if err = v.verifyAsc(fullPath, pgpBytes...); err != nil {
+	if err = v.verifyAsc(fullPath, skipDefaultPgp, pgpBytes...); err != nil {
 		var invalidSignatureErr *download.InvalidSignatureError
 		if errors.As(err, &invalidSignatureErr) {
 			os.Remove(fullPath + ".asc")
@@ -109,9 +109,9 @@ func (v *Verifier) Reload(c *artifact.Config) error {
 	return nil
 }
 
-func (v *Verifier) verifyAsc(fullPath string, pgpSources ...string) error {
+func (v *Verifier) verifyAsc(fullPath string, skipDefaultPgp bool, pgpSources ...string) error {
 	var pgpBytes [][]byte
-	if len(v.pgpBytes) > 0 {
+	if len(v.pgpBytes) > 0 && !skipDefaultPgp {
 		v.log.Infof("Default PGP being appended")
 		pgpBytes = append(pgpBytes, v.pgpBytes)
 	}

internal/pkg/agent/application/upgrade/artifact/download/fs/verifier_test.go

@@ -70,7 +70,7 @@ func TestFetchVerify(t *testing.T) {
 	// first download verify should fail:
 	// download skipped, as invalid package is prepared upfront
 	// verify fails and cleans download
-	err = verifier.Verify(s, version)
+	err = verifier.Verify(s, version, false)
 	var checksumErr *download.ChecksumMismatchError
 	assert.ErrorAs(t, err, &checksumErr)
 
@@ -93,7 +93,7 @@ func TestFetchVerify(t *testing.T) {
 	_, err = os.Stat(hashTargetFilePath)
 	assert.NoError(t, err)
 
-	err = verifier.Verify(s, version)
+	err = verifier.Verify(s, version, false)
 	assert.NoError(t, err)
 
 	// Enable GPG signature validation.
@@ -113,7 +113,7 @@ func TestFetchVerify(t *testing.T) {
 
 	// Missing .asc file.
 	{
-		err = verifier.Verify(s, version)
+		err = verifier.Verify(s, version, false)
 		require.Error(t, err)
 
 		// Don't delete these files when GPG validation failure.
@@ -126,7 +126,7 @@ func TestFetchVerify(t *testing.T) {
 		err = ioutil.WriteFile(targetFilePath+".asc", []byte("bad sig"), 0o600)
 		require.NoError(t, err)
 
-		err = verifier.Verify(s, version)
+		err = verifier.Verify(s, version, false)
 		var invalidSigErr *download.InvalidSignatureError
 		assert.ErrorAs(t, err, &invalidSigErr)
 
@@ -211,7 +211,7 @@ func TestVerify(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	err = testVerifier.Verify(beatSpec, version)
+	err = testVerifier.Verify(beatSpec, version, false)
 	require.NoError(t, err)
 
 	os.Remove(artifact)

internal/pkg/agent/application/upgrade/artifact/download/http/elastic_test.go

@@ -127,7 +127,7 @@ func TestVerify(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			err = testVerifier.Verify(beatSpec, version)
+			err = testVerifier.Verify(beatSpec, version, false)
 			require.NoError(t, err)
 
 			os.Remove(artifact)

internal/pkg/agent/application/upgrade/artifact/download/http/verifier.go

@@ -82,7 +82,7 @@ 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, pgpBytes ...string) error {
+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)
 	if err != nil {
 		return errors.New(err, "retrieving package path")
@@ -97,7 +97,7 @@ func (v *Verifier) Verify(a artifact.Artifact, version string, pgpBytes ...strin
 		return err
 	}
 
-	if err = v.verifyAsc(a, version, pgpBytes...); err != nil {
+	if err = v.verifyAsc(a, version, skipDefaultPgp, pgpBytes...); err != nil {
 		var invalidSignatureErr *download.InvalidSignatureError
 		if errors.As(err, &invalidSignatureErr) {
 			os.Remove(fullPath + ".asc")
@@ -108,9 +108,9 @@ func (v *Verifier) Verify(a artifact.Artifact, version string, pgpBytes ...strin
 	return nil
 }
 
-func (v *Verifier) verifyAsc(a artifact.Artifact, version string, pgpSources ...string) error {
+func (v *Verifier) verifyAsc(a artifact.Artifact, version string, skipDefaultPgp bool, pgpSources ...string) error {
 	var pgpBytes [][]byte
-	if len(v.pgpBytes) > 0 {
+	if len(v.pgpBytes) > 0 && !skipDefaultPgp {
 		v.log.Infof("Default PGP being appended")
 		pgpBytes = append(pgpBytes, v.pgpBytes)
 	}

internal/pkg/agent/application/upgrade/artifact/download/snapshot/verifier.go

@@ -37,8 +37,8 @@ func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool
 }
 
 // Verify checks the package from configured source.
-func (e *Verifier) Verify(a artifact.Artifact, version string, pgpBytes ...string) error {
-	return e.verifier.Verify(a, version, pgpBytes...)
+func (e *Verifier) Verify(a artifact.Artifact, version string, skipDefaultPgp bool, pgpBytes ...string) error {
+	return e.verifier.Verify(a, version, skipDefaultPgp, pgpBytes...)
 }
 
 func (e *Verifier) Reload(c *artifact.Config) error {

internal/pkg/agent/application/upgrade/artifact/download/verifier.go

@@ -63,7 +63,7 @@ type Verifier interface {
 	// *download.ChecksumMismatchError. And if the GPG signature is invalid then
 	// Verify returns a *download.InvalidSignatureError. Use errors.As() to
 	// check error types.
-	Verify(a artifact.Artifact, version string, pgpBytes ...string) error
+	Verify(a artifact.Artifact, version string, skipDefaultPgp bool, pgpBytes ...string) error
 }
 
 // VerifySHA512Hash checks that a sidecar file containing a sha512 checksum

internal/pkg/agent/application/upgrade/step_download.go

@@ -33,7 +33,7 @@ const (
 	defaultUpgradeFallbackPGP = "https://artifacts.elastic.co/GPG-KEY-elastic-agent"
 )
 
-func (u *Upgrader) downloadArtifact(ctx context.Context, version, sourceURI string, skipVerifyOverride bool, pgpBytes ...string) (_ string, err error) {
+func (u *Upgrader) downloadArtifact(ctx context.Context, version, sourceURI string, skipVerifyOverride bool, skipDefaultPgp bool, pgpBytes ...string) (_ string, err error) {
 	span, ctx := apm.StartSpan(ctx, "downloadArtifact", "app.internal")
 	defer func() {
 		apm.CaptureError(ctx, err).Send()
@@ -81,7 +81,7 @@ func (u *Upgrader) downloadArtifact(ctx context.Context, version, sourceURI stri
 		return "", errors.New(err, "initiating verifier")
 	}
 
-	if err := verifier.Verify(agentArtifact, parsedVersion.VersionWithPrerelease(), pgpBytes...); err != nil {
+	if err := verifier.Verify(agentArtifact, parsedVersion.VersionWithPrerelease(), skipDefaultPgp, pgpBytes...); err != nil {
 		return "", errors.New(err, "failed verification of agent binary")
 	}
 

internal/pkg/agent/application/upgrade/upgrade.go

@@ -108,7 +108,7 @@ func (u *Upgrader) Upgradeable() bool {
 }
 
 // Upgrade upgrades running agent, function returns shutdown callback that must be called by reexec.
-func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, skipVerifyOverride bool, pgpBytes ...string) (_ reexec.ShutdownCallbackFn, err error) {
+func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string, action *fleetapi.ActionUpgrade, skipVerifyOverride bool, skipDefaultPgp bool, pgpBytes ...string) (_ reexec.ShutdownCallbackFn, err error) {
 	u.log.Infow("Upgrading agent", "version", version, "source_uri", sourceURI)
 	span, ctx := apm.StartSpan(ctx, "upgrade", "app.internal")
 	defer span.End()
@@ -119,7 +119,7 @@ func (u *Upgrader) Upgrade(ctx context.Context, version string, sourceURI string
 	}
 
 	sourceURI = u.sourceURI(sourceURI)
-	archivePath, err := u.downloadArtifact(ctx, version, sourceURI, skipVerifyOverride, pgpBytes...)
+	archivePath, err := u.downloadArtifact(ctx, version, sourceURI, skipVerifyOverride, skipDefaultPgp, pgpBytes...)
 	if err != nil {
 		// Run the same pre-upgrade cleanup task to get rid of any newly downloaded files
 		// This may have an issue if users are upgrading to the same version number.

internal/pkg/agent/cmd/upgrade.go

@@ -21,11 +21,12 @@ import (
 )
 
 const (
-	flagSourceURI    = "source-uri"
-	flagSkipVerify   = "skip-verify"
-	flagPGPBytes     = "pgp"
-	flagPGPBytesPath = "pgp-path"
-	flagPGPBytesURI  = "pgp-uri"
+	flagSourceURI      = "source-uri"
+	flagSkipVerify     = "skip-verify"
+	flagSkipDefaultPgp = "skip-default-pgp"
+	flagPGPBytes       = "pgp"
+	flagPGPBytesPath   = "pgp-path"
+	flagPGPBytesURI    = "pgp-uri"
 )
 
 func newUpgradeCommandWithArgs(_ []string, streams *cli.IOStreams) *cobra.Command {
@@ -44,6 +45,7 @@ func newUpgradeCommandWithArgs(_ []string, streams *cli.IOStreams) *cobra.Comman
 
 	cmd.Flags().StringP(flagSourceURI, "s", "", "Source URI to download the new version from")
 	cmd.Flags().BoolP(flagSkipVerify, "", false, "Skips package verification")
+	cmd.Flags().BoolP(flagSkipDefaultPgp, "", false, "Skips package verification with embedded key")
 	cmd.Flags().String(flagPGPBytes, "", "PGP to use for package verification")
 	cmd.Flags().String(flagPGPBytesURI, "", "Path to a web location containing PGP to use for package verification")
 	cmd.Flags().String(flagPGPBytesPath, "", "Path to a file containing PGP to use for package verification")
@@ -92,8 +94,8 @@ func upgradeCmd(streams *cli.IOStreams, cmd *cobra.Command, args []string) error
 			pgpChecks = append(pgpChecks, download.PgpSourceURIPrefix+pgpUri)
 		}
 	}
-
-	version, err = c.Upgrade(context.Background(), version, sourceURI, skipVerification, pgpChecks...)
+	skipDefaultPgp, _ := cmd.Flags().GetBool(flagSkipDefaultPgp)
+	version, err = c.Upgrade(context.Background(), version, sourceURI, skipVerification, skipDefaultPgp, pgpChecks...)
 	if err != nil {
 		return errors.New(err, "Failed trigger upgrade of daemon")
 	}

pkg/control/v2/client/client.go

@@ -150,7 +150,7 @@ type Client interface {
 	// Restart triggers restarting the current running daemon.
 	Restart(ctx context.Context) error
 	// Upgrade triggers upgrade of the current running daemon.
-	Upgrade(ctx context.Context, version string, sourceURI string, skipVerify bool, pgpBytes ...string) (string, error)
+	Upgrade(ctx context.Context, version string, sourceURI string, skipVerify bool, skipDefaultPgp bool, pgpBytes ...string) (string, error)
 	// DiagnosticAgent gathers diagnostics information for the running Elastic Agent.
 	DiagnosticAgent(ctx context.Context) ([]DiagnosticFileResult, error)
 	// DiagnosticUnits gathers diagnostics information from specific units (or all if non are provided).
@@ -277,12 +277,13 @@ func (c *client) Restart(ctx context.Context) error {
 }
 
 // Upgrade triggers upgrade of the current running daemon.
-func (c *client) Upgrade(ctx context.Context, version string, sourceURI string, skipVerify bool, pgpBytes ...string) (string, error) {
+func (c *client) Upgrade(ctx context.Context, version string, sourceURI string, skipVerify bool, skipDefaultPgp bool, pgpBytes ...string) (string, error) {
 	res, err := c.client.Upgrade(ctx, &cproto.UpgradeRequest{
-		Version:    version,
-		SourceURI:  sourceURI,
-		SkipVerify: skipVerify,
-		PgpBytes:   pgpBytes,
+		Version:        version,
+		SourceURI:      sourceURI,
+		SkipVerify:     skipVerify,
+		PgpBytes:       pgpBytes,
+		SkipDefaultPgp: skipDefaultPgp,
 	})
 	if err != nil {
 		return "", err