Skip to content

Commit

Permalink
Windows, prevent uninstall from within installed directory (#4108)
Browse files Browse the repository at this point in the history
* Windows, prevent uninstall from within installed directory

- immediate error if trying to uninstall within installed directory
- log to in memory logger to prevent writing logs to directory we are
  trying to delete.

* make change in logging at cmd level

* add changelog fragment

* switch to strings.EqualFold

* fix changelog
  • Loading branch information
leehinman authored Jan 25, 2024
1 parent ca5fdac commit 1cc6585
Show file tree
Hide file tree
Showing 11 changed files with 311 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Kind can be one of:
# - breaking-change: a change to previously-documented behavior
# - deprecation: functionality that is being removed in a later release
# - bug-fix: fixes a problem in a previous version
# - enhancement: extends functionality but does not break or fix existing behavior
# - feature: new functionality
# - known-issue: problems that we are aware of in a given version
# - security: impacts on the security of a product or a user’s deployment.
# - upgrade: important information for someone upgrading from a prior version
# - other: does not fit into any of the other categories
kind: bug-fix

# Change summary; a 80ish characters long description of the change.
summary: On Windows prevent uninstall from within installed directory

# 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: 'On Windows, prevent uninstall from within installed directory to prevent a sharing violation. Also for install and uninstall, print debug logs to stderr if command fails.'

# Affected component; a word indicating the component this changeset affects.
component: agent

# 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/owner/repo/1234
14 changes: 14 additions & 0 deletions internal/pkg/agent/application/paths/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,3 +326,17 @@ func ControlSocketFromPath(platform string, path string) string {
// for it to be used, but needs to be unique per Agent (in the case that multiple are running)
return utils.SocketURLWithFallback(socketPath, path)
}

func pathSplit(path string) []string {
dir, file := filepath.Split(path)
if dir == "" && file == "" {
return []string{}
}
if dir == "" && file != "" {
return []string{file}
}
if dir == path {
return []string{}
}
return append(pathSplit(filepath.Clean(dir)), file)
}
52 changes: 52 additions & 0 deletions internal/pkg/agent/application/paths/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,55 @@ func TestExecDir(t *testing.T) {
})
}
}

func TestPathSplitUnix(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Skipping Unix tests on Windows")
}
tests := map[string]struct {
path string
want []string
}{
"empty string": {path: "", want: []string{}},
"just file": {path: "test.txt", want: []string{"test.txt"}},
"just dir": {path: "/", want: []string{}},
"top dir": {path: "/test.txt", want: []string{"test.txt"}},
"simple": {path: "/a/b", want: []string{"a", "b"}},
"long": {path: "/a/b c/d-e/f_g", want: []string{"a", "b c", "d-e", "f_g"}},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
got := pathSplit(tc.path)
if !cmp.Equal(tc.want, got) {
t.Fatalf("not equal got: %v, want: %v", got, tc.want)
}
})
}
}

func TestPathSplitWindows(t *testing.T) {
if runtime.GOOS != "windows" {
t.Skip("Skipping Windows tests on non Windows")
}
tests := map[string]struct {
path string
want []string
}{
"empty string": {path: "", want: []string{}},
"just file": {path: "test.txt", want: []string{"test.txt"}},
"just dir": {path: "C:\\", want: []string{}},
"top dir": {path: "C:\\test.txt", want: []string{"test.txt"}},
"simple": {path: "C:\\a\\b", want: []string{"a", "b"}},
"long": {path: "C:\\a\\b c\\d-e\\f_g", want: []string{"a", "b c", "d-e", "f_g"}},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
got := pathSplit(tc.path)
if !cmp.Equal(tc.want, got) {
t.Fatalf("not equal got: %v, want: %v", got, tc.want)
}
})
}
}
59 changes: 59 additions & 0 deletions internal/pkg/agent/application/paths/paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,62 @@ func TestEqual(t *testing.T) {
})
}
}

func TestHasPrefixUnix(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("Skipping unix HasPrefix tests on Windows host")
}
tests := map[string]struct {
path string
prefix string
want bool
}{
"simple true": {path: "/a/b", prefix: "/a", want: true},
"just root": {path: "/", prefix: "/", want: true},
"root one dir": {path: "/a", prefix: "/", want: true},
"simple false": {path: "/a/b", prefix: "/c/d", want: false},
"prefix too long": {path: "/a/b", prefix: "/a/b/c/d", want: false},
"trailing slash": {path: "/a/b/", prefix: "/a", want: true},
"no path": {path: "", prefix: "/a", want: false},
"no prefix": {path: "/a/b", prefix: "", want: false},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
got := HasPrefix(tc.path, tc.prefix)
if got != tc.want {
t.Fatalf("got %v, expected %v", got, tc.want)
}
})
}
}

func TestHasPrefixWindows(t *testing.T) {
if runtime.GOOS != "windows" {
t.Skip("Skipping windows HasPrefix tests on non Windows host")
}
tests := map[string]struct {
path string
prefix string
want bool
}{
"simple true": {path: "c:\\a\\b", prefix: "c:\\a", want: true},
"just root": {path: "c:\\", prefix: "c:\\", want: true},
"root one dir": {path: "c:\\a", prefix: "c:\\", want: true},
"simple false": {path: "c:\\a\\b", prefix: "c:\\c\\d", want: false},
"prefix too long": {path: "c:\\a\\b", prefix: "c:\\a\\b\\c\\d", want: false},
"trailing slash": {path: "c:\\a\\b\\", prefix: "c:\\a", want: true},
"no path": {path: "", prefix: "c:\\a", want: false},
"no prefix": {path: "c:\\a\\b", prefix: "", want: false},
"case insensitive": {path: "C:\\A\\B", prefix: "c:\\a", want: true},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
got := HasPrefix(tc.path, tc.prefix)
if got != tc.want {
t.Fatalf("got %v, expected %v", got, tc.want)
}
})
}
}
26 changes: 26 additions & 0 deletions internal/pkg/agent/application/paths/paths_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package paths

import (
"path/filepath"
"runtime"
)

Expand All @@ -16,3 +17,28 @@ func initialControlSocketPath(topPath string) string {

// ResolveControlSocket does nothing on non-Windows hosts.
func ResolveControlSocket() {}

// HasPrefix tests if the path starts with the prefix.
func HasPrefix(path string, prefix string) bool {
if path == "" || prefix == "" {
return false
}

if filepath.VolumeName(path) != filepath.VolumeName(prefix) {
return false
}

prefixParts := pathSplit(filepath.Clean(prefix))
pathParts := pathSplit(filepath.Clean(path))

if len(prefixParts) > len(pathParts) {
return false
}

for i := 0; i < len(prefixParts); i++ {
if prefixParts[i] != pathParts[i] {
return false
}
}
return true
}
25 changes: 25 additions & 0 deletions internal/pkg/agent/application/paths/paths_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,28 @@ func ResolveControlSocket() {
SetControlSocket(WindowsControlSocketInstalledPath)
}
}

// HasPrefix tests if the path starts with the prefix.
func HasPrefix(path string, prefix string) bool {
if path == "" || prefix == "" {
return false
}

if !strings.EqualFold(filepath.VolumeName(path), filepath.VolumeName(prefix)) {
return false
}

prefixParts := pathSplit(filepath.Clean(prefix))
pathParts := pathSplit(filepath.Clean(path))

if len(prefixParts) > len(pathParts) {
return false
}

for i := 0; i < len(prefixParts); i++ {
if !strings.EqualFold(prefixParts[0], pathParts[0]) {
return false
}
}
return true
}
34 changes: 31 additions & 3 deletions internal/pkg/agent/cmd/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ import (

"github.com/spf13/cobra"

"github.com/elastic/elastic-agent-libs/logp"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/filelock"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
"github.com/elastic/elastic-agent/internal/pkg/agent/install"
"github.com/elastic/elastic-agent/internal/pkg/cli"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/utils"
)

Expand Down Expand Up @@ -54,7 +56,9 @@ would like the Agent to operate.
}

func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error {
err := validateEnrollFlags(cmd)
var err error

err = validateEnrollFlags(cmd)
if err != nil {
return fmt.Errorf("could not validate flags: %w", err)
}
Expand Down Expand Up @@ -189,18 +193,42 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error {

progBar := install.CreateAndStartNewSpinner(streams.Out, "Installing Elastic Agent...")

logCfg := logp.DefaultConfig(logp.DefaultEnvironment)
logCfg.Level = logp.DebugLevel
// Using in memory logger, so we don't write logs to the
// directory we are trying to delete
logp.ToObserverOutput()(&logCfg)

err = logp.Configure(logCfg)
if err != nil {
return fmt.Errorf("error creating logging config: %w", err)
}

log := logger.NewWithoutConfig("")

defer func() {
if err == nil {
return
}
oLogs := logp.ObserverLogs().TakeAll()
fmt.Fprintf(os.Stderr, "Error uninstalling. Printing logs\n")
for _, oLog := range oLogs {
fmt.Fprintf(os.Stderr, "%v\n", oLog.Entry)
}
}()

var ownership utils.FileOwner
cfgFile := paths.ConfigFile()
if status != install.PackageInstall {
ownership, err = install.Install(cfgFile, topPath, unprivileged, progBar, streams)
ownership, err = install.Install(cfgFile, topPath, unprivileged, log, progBar, streams)
if err != nil {
return fmt.Errorf("error installing package: %w", err)
}

defer func() {
if err != nil {
progBar.Describe("Uninstalling")
innerErr := install.Uninstall(cfgFile, topPath, "", progBar)
innerErr := install.Uninstall(cfgFile, topPath, "", log, progBar)
if innerErr != nil {
progBar.Describe("Failed to Uninstall")
} else {
Expand Down
30 changes: 29 additions & 1 deletion internal/pkg/agent/cmd/uninstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ import (

"github.com/spf13/cobra"

"github.com/elastic/elastic-agent-libs/logp"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
"github.com/elastic/elastic-agent/internal/pkg/agent/install"
"github.com/elastic/elastic-agent/internal/pkg/cli"
"github.com/elastic/elastic-agent/pkg/core/logger"
"github.com/elastic/elastic-agent/pkg/utils"
)

Expand All @@ -39,6 +41,8 @@ Unless -f is used this command will ask confirmation before performing removal.
}

func uninstallCmd(streams *cli.IOStreams, cmd *cobra.Command) error {
var err error

isAdmin, err := utils.HasRoot()
if err != nil {
return fmt.Errorf("unable to perform command while checking for administrator rights, %w", err)
Expand Down Expand Up @@ -81,7 +85,31 @@ func uninstallCmd(streams *cli.IOStreams, cmd *cobra.Command) error {

progBar := install.CreateAndStartNewSpinner(streams.Out, "Uninstalling Elastic Agent...")

err = install.Uninstall(paths.ConfigFile(), paths.Top(), uninstallToken, progBar)
logCfg := logp.DefaultConfig(logp.DefaultEnvironment)
logCfg.Level = logp.DebugLevel
// Using in memory logger, so we don't write logs to the
// directory we are trying to delete
logp.ToObserverOutput()(&logCfg)

err = logp.Configure(logCfg)
if err != nil {
return fmt.Errorf("error creating logging config: %w", err)
}

log := logger.NewWithoutConfig("")

defer func() {
if err == nil {
return
}
oLogs := logp.ObserverLogs().TakeAll()
fmt.Fprintf(os.Stderr, "Error uninstalling. Printing logs\n")
for _, oLog := range oLogs {
fmt.Fprintf(os.Stderr, "%v\n", oLog.Entry)
}
}()

err = install.Uninstall(paths.ConfigFile(), paths.Top(), uninstallToken, log, progBar)
if err != nil {
progBar.Describe("Failed to uninstall agent")
return fmt.Errorf("error uninstalling agent: %w", err)
Expand Down
5 changes: 3 additions & 2 deletions internal/pkg/agent/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/otiai10/copy"
"github.com/schollz/progressbar/v3"

"github.com/elastic/elastic-agent-libs/logp"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
"github.com/elastic/elastic-agent/internal/pkg/cli"
Expand All @@ -30,7 +31,7 @@ const (
)

// Install installs Elastic Agent persistently on the system including creating and starting its service.
func Install(cfgFile, topPath string, unprivileged bool, pt *progressbar.ProgressBar, streams *cli.IOStreams) (utils.FileOwner, error) {
func Install(cfgFile, topPath string, unprivileged bool, log *logp.Logger, pt *progressbar.ProgressBar, streams *cli.IOStreams) (utils.FileOwner, error) {
dir, err := findDirectory()
if err != nil {
return utils.FileOwner{}, errors.New(err, "failed to discover the source directory for installation", errors.TypeFilesystem)
Expand All @@ -45,7 +46,7 @@ func Install(cfgFile, topPath string, unprivileged bool, pt *progressbar.Progres
// 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, "", pt)
err = Uninstall(cfgFile, topPath, "", log, pt)
if err != nil {
pt.Describe("Failed to uninstall current Elastic Agent")
return utils.FileOwner{}, errors.New(
Expand Down
Loading

0 comments on commit 1cc6585

Please sign in to comment.