From 2332b4301b836b2008563a7d699c5119702306a9 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Wed, 19 Jun 2024 14:41:19 -0700 Subject: [PATCH 01/12] Add explicit check for token and tamper protection in Uninstall func --- ...r-tamper-protection-when-uninstalling.yaml | 34 +++++++++++++++++++ internal/pkg/agent/install/uninstall.go | 8 +++++ 2 files changed, 42 insertions(+) create mode 100644 changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml diff --git a/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml b/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml new file mode 100644 index 00000000000..f6a5efd1378 --- /dev/null +++ b/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml @@ -0,0 +1,34 @@ +# 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 + +# Change summary; a 80ish characters long description of the change. +summary: Check for tamper protection when uninstalling + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. +description: | + The uninstall function will now explictily check if tamper protection is enabled + and if a token has been passed before proceeding. + +# Affected component; a word indicating the component this changeset affects. +component: + +# PR URL; 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/owner/repo/1234 + +# Issue URL; 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/4506 diff --git a/internal/pkg/agent/install/uninstall.go b/internal/pkg/agent/install/uninstall.go index ed8902d9ba2..8c65165d7d2 100644 --- a/internal/pkg/agent/install/uninstall.go +++ b/internal/pkg/agent/install/uninstall.go @@ -37,6 +37,14 @@ import ( // Uninstall uninstalls persistently Elastic Agent on the system. func Uninstall(cfgFile, topPath, uninstallToken string, log *logp.Logger, pt *progressbar.ProgressBar) error { + // Immediatly fail it tamper protection is enabled but no uninstallToken is specified + if features.TamperProtection() && uninstallToken == "" { + return aerrors.New( + fmt.Errorf("missing uninstall token"), + "tamper protection detected, elastic-agent uninstall command must be ran with a valid --uninstall-token arg", + ) + } + cwd, err := os.Getwd() if err != nil { return fmt.Errorf("unable to get current working directory") From b27ce208ab54807f302cf71963b55d84a2dfdf8a Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Wed, 19 Jun 2024 16:45:48 -0700 Subject: [PATCH 02/12] fix typo --- internal/pkg/agent/install/uninstall.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/agent/install/uninstall.go b/internal/pkg/agent/install/uninstall.go index 8c65165d7d2..8423601a5b3 100644 --- a/internal/pkg/agent/install/uninstall.go +++ b/internal/pkg/agent/install/uninstall.go @@ -37,7 +37,7 @@ import ( // Uninstall uninstalls persistently Elastic Agent on the system. func Uninstall(cfgFile, topPath, uninstallToken string, log *logp.Logger, pt *progressbar.ProgressBar) error { - // Immediatly fail it tamper protection is enabled but no uninstallToken is specified + // Immediately fail it tamper protection is enabled but no uninstallToken is specified if features.TamperProtection() && uninstallToken == "" { return aerrors.New( fmt.Errorf("missing uninstall token"), From bcceff5e7606e2e6f42032440039a82f79c474a2 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Fri, 21 Jun 2024 16:57:01 -0700 Subject: [PATCH 03/12] Load features from config, fix protection flag load --- internal/pkg/agent/install/uninstall.go | 44 ++++++++++++++++--------- pkg/features/features.go | 10 +++++- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/internal/pkg/agent/install/uninstall.go b/internal/pkg/agent/install/uninstall.go index 8423601a5b3..29524786506 100644 --- a/internal/pkg/agent/install/uninstall.go +++ b/internal/pkg/agent/install/uninstall.go @@ -37,14 +37,6 @@ import ( // Uninstall uninstalls persistently Elastic Agent on the system. func Uninstall(cfgFile, topPath, uninstallToken string, log *logp.Logger, pt *progressbar.ProgressBar) error { - // Immediately fail it tamper protection is enabled but no uninstallToken is specified - if features.TamperProtection() && uninstallToken == "" { - return aerrors.New( - fmt.Errorf("missing uninstall token"), - "tamper protection detected, elastic-agent uninstall command must be ran with a valid --uninstall-token arg", - ) - } - cwd, err := os.Getwd() if err != nil { return fmt.Errorf("unable to get current working directory") @@ -54,6 +46,34 @@ func Uninstall(cfgFile, topPath, uninstallToken string, log *logp.Logger, pt *pr return fmt.Errorf("uninstall must be run from outside the installed path '%s'", topPath) } + ctx := context.Background() + + // check if the agent was installed using --unprivileged by checking the file vault for the agent secret (needed on darwin to correctly load the vault) + unprivileged, err := checkForUnprivilegedVault(ctx) + if err != nil { + return fmt.Errorf("error checking for unprivileged vault: %w", err) + } + + // Load config so we can check the tamper protection feature + cfg, err := operations.LoadFullAgentConfig(ctx, log, cfgFile, false, unprivileged) + if err != nil { + return fmt.Errorf("error loading agent config: %w", err) + } + cfg, err = applyDynamics(ctx, log, cfg) + if err != nil { + return fmt.Errorf("error applying dynamic inputs: %w", err) + } + if err := features.Apply(cfg); err != nil { + return fmt.Errorf("could not parse and apply feature flags config: %w", err) + } + // Fail if tamper protection is enabled but no uninstallToken is specified + if features.TamperProtection() && uninstallToken == "" { + return aerrors.New( + fmt.Errorf("missing uninstall token"), + "tamper protection detected, elastic-agent uninstall command must be ran with a valid --uninstall-token arg", + ) + } + // ensure service is stopped status, err := EnsureStoppedService(topPath, pt) if err != nil { @@ -66,14 +86,6 @@ func Uninstall(cfgFile, topPath, uninstallToken string, log *logp.Logger, pt *pr return fmt.Errorf("failed trying to kill any running watcher: %w", err) } - ctx := context.Background() - - // check if the agent was installed using --unprivileged by checking the file vault for the agent secret (needed on darwin to correctly load the vault) - unprivileged, err := checkForUnprivilegedVault(ctx) - if err != nil { - return fmt.Errorf("error checking for unprivileged vault: %w", err) - } - // Uninstall components first if err := uninstallComponents(ctx, cfgFile, uninstallToken, log, pt, unprivileged); err != nil { // If service status was running it was stopped to uninstall the components. diff --git a/pkg/features/features.go b/pkg/features/features.go index d9220ef0d6d..d531b5cd1bb 100644 --- a/pkg/features/features.go +++ b/pkg/features/features.go @@ -49,6 +49,10 @@ type cfg struct { Enabled bool `json:"enabled" yaml:"enabled" config:"enabled"` } `json:"tamper_protection,omitempty" yaml:"tamper_protection,omitempty" config:"tamper_protection,omitempty"` } `json:"features" yaml:"features" config:"features"` + // Agent config has protection as a top level field. + Protection *struct { + Enabled bool `json:"enabled" yaml:"enabled" config:"enabled"` + } `json:"protection,omitempty" yaml:"protection,omitempty" config:"protection,omitempty"` } `json:"agent" yaml:"agent" config:"agent"` } @@ -179,9 +183,13 @@ func Parse(policy any) (*Flags, error) { flags := new(Flags) flags.setFQDN(parsedFlags.Agent.Features.FQDN.Enabled) - // Tamper protection flag is optional, fallback on default value if missing + // Optional value defined by agent.features.tamper_protection.enabled is preffered. + // If missing, optional value provided by agent.protection.enabled is used + // Otherwise fallback on default value if parsedFlags.Agent.Features.TamperProtection != nil { flags.setTamperProtection(parsedFlags.Agent.Features.TamperProtection.Enabled) + } else if parsedFlags.Agent.Protection != nil { + flags.setTamperProtection(parsedFlags.Agent.Protection.Enabled) } else { flags.setTamperProtection(defaultTamperProtection) } From f0b7d8811191584261427289814ed59afaaa850a Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Tue, 25 Jun 2024 12:40:19 -0700 Subject: [PATCH 04/12] Change approach to execute elastic-agent uninstall command Change the approach that is taken when "elastic-agent install -f" is ran to use an exec to run "elastic-agent uninstall -f" in cases where the agent is installed. this allows the process that runs the uninstall to use all the correct path values for the installed agent instead of the values associated with the binary that the install command is ran from. --- ...r-tamper-protection-when-uninstalling.yaml | 7 ++-- internal/pkg/agent/cmd/install.go | 18 ++++++++++ internal/pkg/agent/install/install.go | 20 ----------- internal/pkg/agent/install/uninstall.go | 36 +++++-------------- pkg/features/features.go | 10 +----- 5 files changed, 31 insertions(+), 60 deletions(-) diff --git a/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml b/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml index f6a5efd1378..604cbac811a 100644 --- a/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml +++ b/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml @@ -11,14 +11,15 @@ kind: bug # Change summary; a 80ish characters long description of the change. -summary: Check for tamper protection when uninstalling +summary: Check for tamper protection when install --force is used # Long description; in case the summary is not enough to describe the change # this field accommodate a description without length limits. # NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. description: | - The uninstall function will now explictily check if tamper protection is enabled - and if a token has been passed before proceeding. + When using "elastic-agent install -f", the agent will exec "elastic-agent uninstall -f" + so that all path references are correctly loaded and tamper protection errors will cause + the install attempt to fail. # Affected component; a word indicating the component this changeset affects. component: diff --git a/internal/pkg/agent/cmd/install.go b/internal/pkg/agent/cmd/install.go index 55f5f77067e..3c7bfd5b37e 100644 --- a/internal/pkg/agent/cmd/install.go +++ b/internal/pkg/agent/cmd/install.go @@ -221,6 +221,24 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error { var ownership utils.FileOwner cfgFile := paths.ConfigFile() + if status == install.Installed { + // Uninstall the agent + progBar.Describe("Uninstalling current Elastic Agent") + args := []string{ + "uninstall", + "--force", + } + uninstall := exec.Command("elastic-agent", args...) // TODO find elastic-agent binary in $PATH + uninstall.Stdout = streams.Out + uninstall.Stderr = streams.Err + if err := uninstall.Start(); err != nil { + return fmt.Errorf("unable to start elastic-agent uninstall: %w", err) + } + if err := uninstall.Wait(); err != nil { + return fmt.Errorf("failed to uninstall elastic-agent: %w", err) + } + progBar.Describe("Successfully uninstalled Elastic Agent") + } if status != install.PackageInstall { ownership, err = install.Install(cfgFile, topPath, unprivileged, log, progBar, streams) if err != nil { diff --git a/internal/pkg/agent/install/install.go b/internal/pkg/agent/install/install.go index f6b7e10fb05..87ac26da542 100644 --- a/internal/pkg/agent/install/install.go +++ b/internal/pkg/agent/install/install.go @@ -46,26 +46,6 @@ func Install(cfgFile, topPath string, unprivileged bool, log *logp.Logger, pt *p return utils.FileOwner{}, errors.New(err, "failed to discover the source directory for installation", errors.TypeFilesystem) } - // We only uninstall Agent if it is currently installed. - status, _ := Status(topPath) - if status == Installed { - // Uninstall current installation - // - // There is no uninstall token for "install" command. - // Uninstall will fail on protected agent. - // The protected Agent will need to be uninstalled first before it can be installed. - pt.Describe("Uninstalling current Elastic Agent") - err = Uninstall(cfgFile, topPath, "", log, pt) - if err != nil { - pt.Describe("Failed to uninstall current Elastic Agent") - return utils.FileOwner{}, errors.New( - err, - fmt.Sprintf("failed to uninstall Agent at (%s)", filepath.Dir(topPath)), - errors.M("directory", filepath.Dir(topPath))) - } - pt.Describe("Successfully uninstalled current Elastic Agent") - } - var ownership utils.FileOwner username := "" groupName := "" diff --git a/internal/pkg/agent/install/uninstall.go b/internal/pkg/agent/install/uninstall.go index ff17c219aa2..703aef69e4e 100644 --- a/internal/pkg/agent/install/uninstall.go +++ b/internal/pkg/agent/install/uninstall.go @@ -46,34 +46,6 @@ func Uninstall(cfgFile, topPath, uninstallToken string, log *logp.Logger, pt *pr return fmt.Errorf("uninstall must be run from outside the installed path '%s'", topPath) } - ctx := context.Background() - - // check if the agent was installed using --unprivileged by checking the file vault for the agent secret (needed on darwin to correctly load the vault) - unprivileged, err := checkForUnprivilegedVault(ctx) - if err != nil { - return fmt.Errorf("error checking for unprivileged vault: %w", err) - } - - // Load config so we can check the tamper protection feature - cfg, err := operations.LoadFullAgentConfig(ctx, log, cfgFile, false, unprivileged) - if err != nil { - return fmt.Errorf("error loading agent config: %w", err) - } - cfg, err = applyDynamics(ctx, log, cfg) - if err != nil { - return fmt.Errorf("error applying dynamic inputs: %w", err) - } - if err := features.Apply(cfg); err != nil { - return fmt.Errorf("could not parse and apply feature flags config: %w", err) - } - // Fail if tamper protection is enabled but no uninstallToken is specified - if features.TamperProtection() && uninstallToken == "" { - return aerrors.New( - fmt.Errorf("missing uninstall token"), - "tamper protection detected, elastic-agent uninstall command must be ran with a valid --uninstall-token arg", - ) - } - // ensure service is stopped status, err := EnsureStoppedService(topPath, pt) if err != nil { @@ -86,6 +58,14 @@ func Uninstall(cfgFile, topPath, uninstallToken string, log *logp.Logger, pt *pr return fmt.Errorf("failed trying to kill any running watcher: %w", err) } + ctx := context.Background() + + // check if the agent was installed using --unprivileged by checking the file vault for the agent secret (needed on darwin to correctly load the vault) + unprivileged, err := checkForUnprivilegedVault(ctx) + if err != nil { + return fmt.Errorf("error checking for unprivileged vault: %w", err) + } + // Uninstall components first if err := uninstallComponents(ctx, cfgFile, uninstallToken, log, pt, unprivileged); err != nil { // If service status was running it was stopped to uninstall the components. diff --git a/pkg/features/features.go b/pkg/features/features.go index d531b5cd1bb..d9220ef0d6d 100644 --- a/pkg/features/features.go +++ b/pkg/features/features.go @@ -49,10 +49,6 @@ type cfg struct { Enabled bool `json:"enabled" yaml:"enabled" config:"enabled"` } `json:"tamper_protection,omitempty" yaml:"tamper_protection,omitempty" config:"tamper_protection,omitempty"` } `json:"features" yaml:"features" config:"features"` - // Agent config has protection as a top level field. - Protection *struct { - Enabled bool `json:"enabled" yaml:"enabled" config:"enabled"` - } `json:"protection,omitempty" yaml:"protection,omitempty" config:"protection,omitempty"` } `json:"agent" yaml:"agent" config:"agent"` } @@ -183,13 +179,9 @@ func Parse(policy any) (*Flags, error) { flags := new(Flags) flags.setFQDN(parsedFlags.Agent.Features.FQDN.Enabled) - // Optional value defined by agent.features.tamper_protection.enabled is preffered. - // If missing, optional value provided by agent.protection.enabled is used - // Otherwise fallback on default value + // Tamper protection flag is optional, fallback on default value if missing if parsedFlags.Agent.Features.TamperProtection != nil { flags.setTamperProtection(parsedFlags.Agent.Features.TamperProtection.Enabled) - } else if parsedFlags.Agent.Protection != nil { - flags.setTamperProtection(parsedFlags.Agent.Protection.Enabled) } else { flags.setTamperProtection(defaultTamperProtection) } From b210ac265b6c921ce879fbf5766d56df44049c19 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Tue, 25 Jun 2024 13:40:42 -0700 Subject: [PATCH 05/12] Add e2e test --- testing/integration/endpoint_security_test.go | 158 ++++++++++-------- 1 file changed, 87 insertions(+), 71 deletions(-) diff --git a/testing/integration/endpoint_security_test.go b/testing/integration/endpoint_security_test.go index c0a1bc47dee..d8376643001 100644 --- a/testing/integration/endpoint_security_test.go +++ b/testing/integration/endpoint_security_test.go @@ -133,22 +133,10 @@ func TestInstallWithEndpointSecurityAndRemoveEndpointIntegration(t *testing.T) { } } -// buildPolicyWithTamperProtection helper function to build the policy request with or without tamper protection -func buildPolicyWithTamperProtection(policy kibana.AgentPolicy, protected bool) kibana.AgentPolicy { - if protected { - policy.AgentFeatures = append(policy.AgentFeatures, map[string]interface{}{ - "name": "tamper_protection", - "enabled": true, - }) - } - policy.IsProtected = protected - return policy -} - -func testInstallAndCLIUninstallWithEndpointSecurity(t *testing.T, info *define.Info, protected bool) { - deadline := time.Now().Add(10 * time.Minute) - ctx, cancel := testcontext.WithDeadline(t, context.Background(), deadline) - defer cancel() +// installSecurityAgent is a helper function to install an elastic-agent in priviliged mode with the force+non-interactve flags. +// the policy the agent is enrolled with can have protection enabled if passed +func installSecurityAgent(ctx context.Context, t *testing.T, info *define.Info, protected bool) (*atesting.Fixture, kibana.PolicyResponse) { + t.Helper() // Get path to agent executable. fixture, err := define.NewFixtureFromLocalBuild(t, define.Version()) @@ -179,6 +167,27 @@ func testInstallAndCLIUninstallWithEndpointSecurity(t *testing.T, info *define.I policy, err := tools.InstallAgentWithPolicy(ctx, t, installOpts, fixture, info.KibanaClient, createPolicyReq) require.NoError(t, err, "failed to install agent with policy") + return fixture, policy +} + +// buildPolicyWithTamperProtection helper function to build the policy request with or without tamper protection +func buildPolicyWithTamperProtection(policy kibana.AgentPolicy, protected bool) kibana.AgentPolicy { + if protected { + policy.AgentFeatures = append(policy.AgentFeatures, map[string]interface{}{ + "name": "tamper_protection", + "enabled": true, + }) + } + policy.IsProtected = protected + return policy +} + +func testInstallAndCLIUninstallWithEndpointSecurity(t *testing.T, info *define.Info, protected bool) { + deadline := time.Now().Add(10 * time.Minute) + ctx, cancel := testcontext.WithDeadline(t, context.Background(), deadline) + defer cancel() + + fixture, policy := installSecurityAgent(ctx, t, info, protected) t.Cleanup(func() { t.Log("Un-enrolling Elastic Agent...") @@ -210,39 +219,13 @@ func testInstallAndCLIUninstallWithEndpointSecurity(t *testing.T, info *define.I } func testInstallAndUnenrollWithEndpointSecurity(t *testing.T, info *define.Info, protected bool) { - // Get path to agent executable. - fixture, err := define.NewFixtureFromLocalBuild(t, define.Version()) - require.NoError(t, err) - - t.Log("Enrolling the agent in Fleet") - policyUUID := uuid.New().String() - createPolicyReq := buildPolicyWithTamperProtection( - kibana.AgentPolicy{ - Name: "test-policy-" + policyUUID, - Namespace: "default", - Description: "Test policy " + policyUUID, - MonitoringEnabled: []kibana.MonitoringEnabledOption{ - kibana.MonitoringEnabledLogs, - kibana.MonitoringEnabledMetrics, - }, - }, - protected, - ) - - installOpts := atesting.InstallOpts{ - NonInteractive: true, - Force: true, - Privileged: true, - } - ctx, cn := testcontext.WithDeadline(t, context.Background(), time.Now().Add(10*time.Minute)) defer cn() - policy, err := tools.InstallAgentWithPolicy(ctx, t, installOpts, fixture, info.KibanaClient, createPolicyReq) - require.NoError(t, err) + fixture, policy := installSecurityAgent(ctx, t, info, protected) t.Log("Installing Elastic Defend") - _, err = installElasticDefendPackage(t, info, policy.ID) + _, err := installElasticDefendPackage(t, info, policy.ID) require.NoError(t, err) t.Log("Polling for endpoint-security to become Healthy") @@ -323,36 +306,10 @@ func testInstallAndUnenrollWithEndpointSecurity(t *testing.T, info *define.Info, } func testInstallWithEndpointSecurityAndRemoveEndpointIntegration(t *testing.T, info *define.Info, protected bool) { - // Get path to agent executable. - fixture, err := define.NewFixtureFromLocalBuild(t, define.Version()) - require.NoError(t, err) - - t.Log("Enrolling the agent in Fleet") - policyUUID := uuid.New().String() - createPolicyReq := buildPolicyWithTamperProtection( - kibana.AgentPolicy{ - Name: "test-policy-" + policyUUID, - Namespace: "default", - Description: "Test policy " + policyUUID, - MonitoringEnabled: []kibana.MonitoringEnabledOption{ - kibana.MonitoringEnabledLogs, - kibana.MonitoringEnabledMetrics, - }, - }, - protected, - ) - - installOpts := atesting.InstallOpts{ - NonInteractive: true, - Force: true, - Privileged: true, - } - ctx, cn := testcontext.WithDeadline(t, context.Background(), time.Now().Add(10*time.Minute)) defer cn() - policy, err := tools.InstallAgentWithPolicy(ctx, t, installOpts, fixture, info.KibanaClient, createPolicyReq) - require.NoError(t, err) + fixture, policy := installSecurityAgent(ctx, t, info, protected) t.Log("Installing Elastic Defend") pkgPolicyResp, err := installElasticDefendPackage(t, info, policy.ID) @@ -874,3 +831,62 @@ func agentIsHealthyNoEndpoint(t *testing.T, ctx context.Context, agentClient cli return true } + +// TestForceInstallOverProtectedPolicy tests that running `elastic-agent install -f` +// when an installed agent is running a policy with tamper protection enabled fails. +func TestForceInstallOverProtectedPolicy(t *testing.T) { + info := define.Require(t, define.Requirements{ + Group: Fleet, + Stack: &define.Stack{}, + Local: false, // requires Agent installation + Sudo: true, // requires Agent installation + OS: []define.OS{ + {Type: define.Linux}, + }, + }) + + deadline := time.Now().Add(10 * time.Minute) + ctx, cancel := testcontext.WithDeadline(t, context.Background(), deadline) + defer cancel() + + fixture, policy := installSecurityAgent(ctx, t, info, true) + + t.Cleanup(func() { + t.Log("Un-enrolling Elastic Agent...") + // Use a separate context as the one in the test body will have been cancelled at this point. + cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), time.Minute) + defer cleanupCancel() + assert.NoError(t, fleettools.UnEnrollAgent(cleanupCtx, info.KibanaClient, policy.ID)) + }) + + t.Log("Installing Elastic Defend") + pkgPolicyResp, err := installElasticDefendPackage(t, info, policy.ID) + require.NoErrorf(t, err, "Policy Response was: %v", pkgPolicyResp) + + t.Log("Polling for endpoint-security to become Healthy") + ctx, cancel = context.WithTimeout(ctx, endpointHealthPollingTimeout) + defer cancel() + + agentClient := fixture.Client() + err = agentClient.Connect(ctx) + require.NoError(t, err, "could not connect to local agent") + + require.Eventually(t, + func() bool { return agentAndEndpointAreHealthy(t, ctx, agentClient) }, + endpointHealthPollingTimeout, + time.Second, + "Endpoint component or units are not healthy.", + ) + t.Log("Verified endpoint component and units are healthy") + + t.Log("Run elastic-agent install -f...") + fixture2, err := define.NewFixtureFromLocalBuild(t, define.Version()) + require.NoError(t, err, "could not create agent fixture") + // We use the same policy with tamper protection enabled for this test and expect it to fail. + err = tools.InstallAgentForPolicy(ctx, t, atesting.InstallOpts{ + NonInteractive: true, + Force: true, + Privileged: true, + }, fixture2, info.KibanaClient, policy.ID) + require.Error(t, err) +} From 71ba9e75c622e26f74bbc163d7a7f0f32f3b3c36 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Tue, 25 Jun 2024 15:02:08 -0700 Subject: [PATCH 06/12] lookup agent binary on path, fix test --- internal/pkg/agent/cmd/install.go | 6 ++++- testing/integration/endpoint_security_test.go | 22 ++++++++++++++----- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/internal/pkg/agent/cmd/install.go b/internal/pkg/agent/cmd/install.go index 3c7bfd5b37e..f8234a5f04f 100644 --- a/internal/pkg/agent/cmd/install.go +++ b/internal/pkg/agent/cmd/install.go @@ -228,7 +228,11 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error { "uninstall", "--force", } - uninstall := exec.Command("elastic-agent", args...) // TODO find elastic-agent binary in $PATH + execPath, err := exec.LookPath(paths.BinaryName) + if err != nil { + return fmt.Errorf("unable to find %s on path: %w", paths.BinaryName, err) + } + uninstall := exec.Command(execPath, args...) uninstall.Stdout = streams.Out uninstall.Stderr = streams.Err if err := uninstall.Start(); err != nil { diff --git a/testing/integration/endpoint_security_test.go b/testing/integration/endpoint_security_test.go index d8376643001..d53f8cfd1ca 100644 --- a/testing/integration/endpoint_security_test.go +++ b/testing/integration/endpoint_security_test.go @@ -882,11 +882,23 @@ func TestForceInstallOverProtectedPolicy(t *testing.T) { t.Log("Run elastic-agent install -f...") fixture2, err := define.NewFixtureFromLocalBuild(t, define.Version()) require.NoError(t, err, "could not create agent fixture") + // We use the same policy with tamper protection enabled for this test and expect it to fail. - err = tools.InstallAgentForPolicy(ctx, t, atesting.InstallOpts{ - NonInteractive: true, - Force: true, - Privileged: true, - }, fixture2, info.KibanaClient, policy.ID) + token, err := info.KibanaClient.CreateEnrollmentAPIKey(ctx, kibana.CreateEnrollmentAPIKeyRequest{ + PolicyID: policy.ID, + }) + require.NoError(t, err) + url, err := fleettools.DefaultURL(ctx, info.KibanaClient) + require.NoError(t, err) + + args := []string{ + "install", + "--force", + "--url", + url, + "--enrollment-token", + token, + } + _, err = fixture2.Exec(ctx, args) require.Error(t, err) } From 2a527d5d567009ea0ea9917e1a5b9e9aa3335b9b Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Tue, 25 Jun 2024 16:52:09 -0700 Subject: [PATCH 07/12] fix test --- testing/integration/endpoint_security_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/integration/endpoint_security_test.go b/testing/integration/endpoint_security_test.go index d53f8cfd1ca..903d1a3da6f 100644 --- a/testing/integration/endpoint_security_test.go +++ b/testing/integration/endpoint_security_test.go @@ -897,7 +897,7 @@ func TestForceInstallOverProtectedPolicy(t *testing.T) { "--url", url, "--enrollment-token", - token, + token.APIKey, } _, err = fixture2.Exec(ctx, args) require.Error(t, err) From dc6616240056031db4dcd9e73c2c19ebe5a7f9aa Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Wed, 26 Jun 2024 11:38:25 -0700 Subject: [PATCH 08/12] Add flag that preserves old approach --- ...r-tamper-protection-when-uninstalling.yaml | 6 +- internal/pkg/agent/cmd/install.go | 65 +++++++++++++------ testing/integration/endpoint_security_test.go | 4 +- 3 files changed, 51 insertions(+), 24 deletions(-) diff --git a/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml b/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml index 604cbac811a..52e54368e86 100644 --- a/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml +++ b/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml @@ -18,8 +18,10 @@ summary: Check for tamper protection when install --force is used # NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. description: | When using "elastic-agent install -f", the agent will exec "elastic-agent uninstall -f" - so that all path references are correctly loaded and tamper protection errors will cause - the install attempt to fail. + using the agent found in the system's path. This ensures all path references are correctly + loaded and tamper protection errors will cause the install attempt to fail. Add the + --run-uninstall-from-binary flag that preserves old behaviour and uses the same binary + that is running install to uninstall the existing agent. # Affected component; a word indicating the component this changeset affects. component: diff --git a/internal/pkg/agent/cmd/install.go b/internal/pkg/agent/cmd/install.go index f8234a5f04f..22786fd8a9e 100644 --- a/internal/pkg/agent/cmd/install.go +++ b/internal/pkg/agent/cmd/install.go @@ -23,10 +23,11 @@ import ( ) const ( - flagInstallBasePath = "base-path" - flagInstallUnprivileged = "unprivileged" - flagInstallDevelopment = "develop" - flagInstallNamespace = "namespace" + flagInstallBasePath = "base-path" + flagInstallUnprivileged = "unprivileged" + flagInstallDevelopment = "develop" + flagInstallNamespace = "namespace" + flagInstallRunUninstallFromBinary = "run-uninstall-from-binary" ) func newInstallCommandWithArgs(_ []string, streams *cli.IOStreams) *cobra.Command { @@ -50,6 +51,7 @@ would like the Agent to operate. cmd.Flags().BoolP("non-interactive", "n", false, "Install Elastic Agent in non-interactive mode which will not prompt on missing parameters but fails instead.") cmd.Flags().String(flagInstallBasePath, paths.DefaultBasePath, "The path where the Elastic Agent will be installed. It must be an absolute path.") cmd.Flags().Bool(flagInstallUnprivileged, false, "Install in unprivileged mode, limiting the access of the Elastic Agent. (beta)") + cmd.Flags().Bool(flagInstallRunUninstallFromBinary, false, "Run the uninstall command from this binary instead of using the binary found in the system's path.") cmd.Flags().String(flagInstallNamespace, "", "Install into an isolated namespace. Allows multiple Elastic Agents to be installed at once. (experimental)") _ = cmd.Flags().MarkHidden(flagInstallNamespace) // For internal use only. @@ -110,6 +112,11 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error { return fmt.Errorf("already installed at: %s", topPath) } + runUninstallBinary, _ := cmd.Flags().GetBool(flagInstallRunUninstallFromBinary) + if status == install.Installed && force && runUninstallBinary { + fmt.Fprintln(streams.Out, "Uninstall will not be ran from the agent installed in system path, components may persist.") + } + nonInteractive, _ := cmd.Flags().GetBool("non-interactive") if nonInteractive { fmt.Fprintln(streams.Out, "Installing in non-interactive mode.") @@ -224,22 +231,18 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error { if status == install.Installed { // Uninstall the agent progBar.Describe("Uninstalling current Elastic Agent") - args := []string{ - "uninstall", - "--force", - } - execPath, err := exec.LookPath(paths.BinaryName) - if err != nil { - return fmt.Errorf("unable to find %s on path: %w", paths.BinaryName, err) - } - uninstall := exec.Command(execPath, args...) - uninstall.Stdout = streams.Out - uninstall.Stderr = streams.Err - if err := uninstall.Start(); err != nil { - return fmt.Errorf("unable to start elastic-agent uninstall: %w", err) - } - if err := uninstall.Wait(); err != nil { - return fmt.Errorf("failed to uninstall elastic-agent: %w", err) + if !runUninstallBinary { + err := execUninstall(streams) + if err != nil { + progBar.Describe("Uninstall failed") + return err + } + } else { + err := install.Uninstall(cfgFile, topPath, "", log, progBar) + if err != nil { + progBar.Describe("Uninstall from binary failed") + return err + } } progBar.Describe("Successfully uninstalled Elastic Agent") } @@ -322,3 +325,25 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error { fmt.Fprint(streams.Out, "\nElastic Agent has been successfully installed.\n") return nil } + +// execUninstall execs "elastic-agent uninstall --force" from the elastic agent installed on the system (found in PATH) +func execUninstall(streams *cli.IOStreams) error { + args := []string{ + "uninstall", + "--force", + } + execPath, err := exec.LookPath(paths.BinaryName) + if err != nil { + return fmt.Errorf("unable to find %s on path: %w", paths.BinaryName, err) + } + uninstall := exec.Command(execPath, args...) + uninstall.Stdout = streams.Out + uninstall.Stderr = streams.Err + if err := uninstall.Start(); err != nil { + return fmt.Errorf("unable to start elastic-agent uninstall: %w", err) + } + if err := uninstall.Wait(); err != nil { + return fmt.Errorf("failed to uninstall elastic-agent: %w", err) + } + return nil +} diff --git a/testing/integration/endpoint_security_test.go b/testing/integration/endpoint_security_test.go index 903d1a3da6f..31d786910ee 100644 --- a/testing/integration/endpoint_security_test.go +++ b/testing/integration/endpoint_security_test.go @@ -899,6 +899,6 @@ func TestForceInstallOverProtectedPolicy(t *testing.T) { "--enrollment-token", token.APIKey, } - _, err = fixture2.Exec(ctx, args) - require.Error(t, err) + out, err = fixture2.Exec(ctx, args) + require.Errorf(t, err, "No error detected, command output: %s", out) } From a700b5010eed7b527bb99b5bf93a75c3be5f2cf2 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Wed, 26 Jun 2024 15:03:21 -0700 Subject: [PATCH 09/12] fix typo --- testing/integration/endpoint_security_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/integration/endpoint_security_test.go b/testing/integration/endpoint_security_test.go index 31d786910ee..773f35c3030 100644 --- a/testing/integration/endpoint_security_test.go +++ b/testing/integration/endpoint_security_test.go @@ -899,6 +899,6 @@ func TestForceInstallOverProtectedPolicy(t *testing.T) { "--enrollment-token", token.APIKey, } - out, err = fixture2.Exec(ctx, args) + out, err := fixture2.Exec(ctx, args) require.Errorf(t, err, "No error detected, command output: %s", out) } From 30487159b1b30cf1d326e19f6835b1bd1e9fed95 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Wed, 26 Jun 2024 16:44:42 -0700 Subject: [PATCH 10/12] change args format in test --- testing/integration/endpoint_security_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/testing/integration/endpoint_security_test.go b/testing/integration/endpoint_security_test.go index 773f35c3030..62f83d9ff57 100644 --- a/testing/integration/endpoint_security_test.go +++ b/testing/integration/endpoint_security_test.go @@ -894,10 +894,8 @@ func TestForceInstallOverProtectedPolicy(t *testing.T) { args := []string{ "install", "--force", - "--url", - url, - "--enrollment-token", - token.APIKey, + "--url=" + url, + "--enrollment-token=" + token.APIKey, } out, err := fixture2.Exec(ctx, args) require.Errorf(t, err, "No error detected, command output: %s", out) From 5ffd08a6f6cd4486217c1f69c26699083a63e198 Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Wed, 26 Jun 2024 18:38:04 -0700 Subject: [PATCH 11/12] Use same fixture --- testing/integration/endpoint_security_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/testing/integration/endpoint_security_test.go b/testing/integration/endpoint_security_test.go index 62f83d9ff57..c3e20d8f43c 100644 --- a/testing/integration/endpoint_security_test.go +++ b/testing/integration/endpoint_security_test.go @@ -880,9 +880,6 @@ func TestForceInstallOverProtectedPolicy(t *testing.T) { t.Log("Verified endpoint component and units are healthy") t.Log("Run elastic-agent install -f...") - fixture2, err := define.NewFixtureFromLocalBuild(t, define.Version()) - require.NoError(t, err, "could not create agent fixture") - // We use the same policy with tamper protection enabled for this test and expect it to fail. token, err := info.KibanaClient.CreateEnrollmentAPIKey(ctx, kibana.CreateEnrollmentAPIKeyRequest{ PolicyID: policy.ID, @@ -894,9 +891,11 @@ func TestForceInstallOverProtectedPolicy(t *testing.T) { args := []string{ "install", "--force", - "--url=" + url, - "--enrollment-token=" + token.APIKey, + "--url", + url, + "--enrollment-token", + token.APIKey, } - out, err := fixture2.Exec(ctx, args) + out, err := fixture.Exec(ctx, args) require.Errorf(t, err, "No error detected, command output: %s", out) } From 848da370107695c013ddb5bd01af9ddd8279ce1c Mon Sep 17 00:00:00 2001 From: michel-laterman Date: Thu, 27 Jun 2024 11:38:20 -0700 Subject: [PATCH 12/12] Hide new option --- ...33116-Check-for-tamper-protection-when-uninstalling.yaml | 6 ++---- internal/pkg/agent/cmd/install.go | 2 ++ 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml b/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml index 52e54368e86..1495198ae0d 100644 --- a/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml +++ b/changelog/fragments/1718833116-Check-for-tamper-protection-when-uninstalling.yaml @@ -11,7 +11,7 @@ kind: bug # Change summary; a 80ish characters long description of the change. -summary: Check for tamper protection when install --force is used +summary: Use installed agent to uninstall itself when install -f is used. # Long description; in case the summary is not enough to describe the change # this field accommodate a description without length limits. @@ -19,9 +19,7 @@ summary: Check for tamper protection when install --force is used description: | When using "elastic-agent install -f", the agent will exec "elastic-agent uninstall -f" using the agent found in the system's path. This ensures all path references are correctly - loaded and tamper protection errors will cause the install attempt to fail. Add the - --run-uninstall-from-binary flag that preserves old behaviour and uses the same binary - that is running install to uninstall the existing agent. + loaded and tamper protection errors will cause the install attempt to fail. # Affected component; a word indicating the component this changeset affects. component: diff --git a/internal/pkg/agent/cmd/install.go b/internal/pkg/agent/cmd/install.go index 22786fd8a9e..3668967abbc 100644 --- a/internal/pkg/agent/cmd/install.go +++ b/internal/pkg/agent/cmd/install.go @@ -51,7 +51,9 @@ would like the Agent to operate. cmd.Flags().BoolP("non-interactive", "n", false, "Install Elastic Agent in non-interactive mode which will not prompt on missing parameters but fails instead.") cmd.Flags().String(flagInstallBasePath, paths.DefaultBasePath, "The path where the Elastic Agent will be installed. It must be an absolute path.") cmd.Flags().Bool(flagInstallUnprivileged, false, "Install in unprivileged mode, limiting the access of the Elastic Agent. (beta)") + cmd.Flags().Bool(flagInstallRunUninstallFromBinary, false, "Run the uninstall command from this binary instead of using the binary found in the system's path.") + _ = cmd.Flags().MarkHidden(flagInstallRunUninstallFromBinary) // Advanced option to force a new agent to override an existing installation, it may orphan installed components. cmd.Flags().String(flagInstallNamespace, "", "Install into an isolated namespace. Allows multiple Elastic Agents to be installed at once. (experimental)") _ = cmd.Flags().MarkHidden(flagInstallNamespace) // For internal use only.