diff --git a/changelog/fragments/1688657261-Don't-trigger-IOC-alert-on-Windows-uninstall.yaml b/changelog/fragments/1688657261-Don't-trigger-IOC-alert-on-Windows-uninstall.yaml new file mode 100644 index 00000000000..106e24ef946 --- /dev/null +++ b/changelog/fragments/1688657261-Don't-trigger-IOC-alert-on-Windows-uninstall.yaml @@ -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: Don't trigger IOC alert on Windows uninstall + +# 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: + +# Affected component; a word indicating the component this changeset affects. +component: uninstall + +# 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/elastic/elastic-agent/pull/3014 + +# 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/2970 diff --git a/internal/pkg/agent/install/uninstall.go b/internal/pkg/agent/install/uninstall.go index 05c7a7ad7f7..a6bfac4b400 100644 --- a/internal/pkg/agent/install/uninstall.go +++ b/internal/pkg/agent/install/uninstall.go @@ -8,10 +8,10 @@ import ( "context" "fmt" "os" - "os/exec" "path/filepath" "runtime" "strings" + "time" "github.com/kardianos/service" @@ -78,13 +78,8 @@ func Uninstall(cfgFile, topPath, uninstallToken string) error { } // remove existing directory - err = os.RemoveAll(topPath) + err = RemovePath(topPath) if err != nil { - if runtime.GOOS == "windows" { //nolint:goconst // it is more readable this way - // possible to fail on Windows, because elastic-agent.exe is running from - // this directory. - return nil - } return errors.New( err, fmt.Sprintf("failed to remove installation directory (%s)", paths.Top()), @@ -95,15 +90,37 @@ func Uninstall(cfgFile, topPath, uninstallToken string) error { } // RemovePath helps with removal path where there is a probability -// of running into self which might prevent removal. -// Removal will be initiated 2 seconds after a call. +// of running into an executable running that might prevent removal +// on Windows. +// +// On Windows it is possible that a removal can spuriously error due +// to an ERROR_SHARING_VIOLATION. RemovePath will retry up to 2 +// seconds if it keeps getting that error. func RemovePath(path string) error { - cleanupErr := os.RemoveAll(path) - if cleanupErr != nil && isBlockingOnSelf(cleanupErr) { - delayedRemoval(path) + const arbitraryTimeout = 2 * time.Second + var start time.Time + nextSleep := 1 * time.Millisecond + for { + err := os.RemoveAll(path) + if err == nil { + return nil + } + if isBlockingOnExe(err) { + // try to remove the blocking exe + err = removeBlockingExe(err) + } + if err == nil { + return nil + } + if !isRetryableError(err) { + return err + } + if start.IsZero() { + start = time.Now() + } else if d := time.Since(start) + nextSleep; d >= arbitraryTimeout { + return err + } } - - return cleanupErr } func RemoveBut(path string, bestEffort bool, exceptions ...string) error { @@ -146,27 +163,6 @@ func containsString(str string, a []string, caseSensitive bool) bool { return false } -func isBlockingOnSelf(err error) bool { - // cannot remove self, this is expected on windows - // fails with remove {path}}\elastic-agent.exe: Access is denied - return runtime.GOOS == "windows" && - err != nil && - strings.Contains(err.Error(), "elastic-agent.exe") && - strings.Contains(err.Error(), "Access is denied") -} - -func delayedRemoval(path string) { - // The installation path will still exists because we are executing from that - // directory. So cmd.exe is spawned that sleeps for 2 seconds (using ping, recommend way from - // from Windows) then rmdir is performed. - //nolint:gosec // it's not tainted - rmdir := exec.Command( - filepath.Join(os.Getenv("windir"), "system32", "cmd.exe"), - "/C", "ping", "-n", "2", "127.0.0.1", "&&", "rmdir", "/s", "/q", path) - _ = rmdir.Start() - -} - func uninstallComponents(ctx context.Context, cfgFile string, uninstallToken string) error { log, err := logger.NewWithLogpLevel("", logp.ErrorLevel, false) if err != nil { diff --git a/internal/pkg/agent/install/uninstall_unix.go b/internal/pkg/agent/install/uninstall_unix.go new file mode 100644 index 00000000000..d63d4bcf2d4 --- /dev/null +++ b/internal/pkg/agent/install/uninstall_unix.go @@ -0,0 +1,19 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +//go:build !windows + +package install + +func isBlockingOnExe(_ error) bool { + return false +} + +func removeBlockingExe(_ error) error { + return nil +} + +func isRetryableError(_ error) bool { + return false +} diff --git a/internal/pkg/agent/install/uninstall_windows.go b/internal/pkg/agent/install/uninstall_windows.go new file mode 100644 index 00000000000..5339cd376b3 --- /dev/null +++ b/internal/pkg/agent/install/uninstall_windows.go @@ -0,0 +1,162 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +//go:build windows + +package install + +import ( + "errors" + "fmt" + "io/fs" + "syscall" + "unsafe" + + "golang.org/x/sys/windows" +) + +func isBlockingOnExe(err error) bool { + if err == nil { + return false + } + path, errno := getPathFromError(err) + if path == "" { + return false + } + return errno == syscall.ERROR_ACCESS_DENIED +} + +func isRetryableError(err error) bool { + if err == nil { + return false + } + path, errno := getPathFromError(err) + if path == "" { + return false + } + return errno == syscall.ERROR_ACCESS_DENIED || errno == windows.ERROR_SHARING_VIOLATION +} + +func removeBlockingExe(blockingErr error) error { + path, _ := getPathFromError(blockingErr) + if path == "" { + return nil + } + + // open handle for delete only + h, err := openDeleteHandle(path) + if err != nil { + return fmt.Errorf("failed to open handle for %q: %w", path, err) + } + + // rename handle + err = renameHandle(h) + _ = windows.CloseHandle(h) + if err != nil { + return fmt.Errorf("failed to rename handle for %q: %w", path, err) + } + + // re-open handle + h, err = openDeleteHandle(path) + if err != nil { + return fmt.Errorf("failed to open handle after rename for %q: %w", path, err) + } + + // dispose of the handle + err = disposeHandle(h) + _ = windows.CloseHandle(h) + if err != nil { + return fmt.Errorf("failed to dispose handle for %q: %w", path, err) + } + return nil +} + +func getPathFromError(blockingErr error) (string, syscall.Errno) { + var perr *fs.PathError + if errors.As(blockingErr, &perr) { + var errno syscall.Errno + if errors.As(perr.Err, &errno) { + return perr.Path, errno + } + } + return "", 0 +} + +func openDeleteHandle(path string) (windows.Handle, error) { + wPath, err := windows.UTF16PtrFromString(path) + if err != nil { + return 0, err + } + handle, err := windows.CreateFile( + wPath, + windows.DELETE, + 0, + nil, + windows.OPEN_EXISTING, + windows.FILE_ATTRIBUTE_NORMAL, + 0, + ) + if err != nil { + return 0, err + } + return handle, nil +} + +func renameHandle(hHandle windows.Handle) error { + wRename, err := windows.UTF16FromString(":agentrm") + if err != nil { + return err + } + + var rename fileRenameInfo + lpwStream := &wRename[0] + rename.FileNameLength = uint32(unsafe.Sizeof(lpwStream)) + + _, _, _ = windows.NewLazyDLL("kernel32.dll").NewProc("RtlCopyMemory").Call( + uintptr(unsafe.Pointer(&rename.FileName[0])), + uintptr(unsafe.Pointer(lpwStream)), + unsafe.Sizeof(lpwStream), + ) + + err = windows.SetFileInformationByHandle( + hHandle, + windows.FileRenameInfo, + (*byte)(unsafe.Pointer(&rename)), + uint32(unsafe.Sizeof(rename)+unsafe.Sizeof(lpwStream)), + ) + if err != nil { + return err + } + return nil +} + +func disposeHandle(hHandle windows.Handle) error { + var deleteFile fileDispositionInfo + deleteFile.DeleteFile = true + + err := windows.SetFileInformationByHandle( + hHandle, + windows.FileDispositionInfo, + (*byte)(unsafe.Pointer(&deleteFile)), + uint32(unsafe.Sizeof(deleteFile)), + ) + if err != nil { + return err + } + return nil +} + +type fileRenameInfo struct { + Union struct { + ReplaceIfExists bool + Flags uint32 + } + RootDirectory windows.Handle + FileNameLength uint32 + FileName [1]uint16 +} + +type fileDispositionInfo struct { + DeleteFile bool +} diff --git a/internal/pkg/agent/install/uninstall_windows_test.go b/internal/pkg/agent/install/uninstall_windows_test.go new file mode 100644 index 00000000000..31812988b68 --- /dev/null +++ b/internal/pkg/agent/install/uninstall_windows_test.go @@ -0,0 +1,55 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +//go:build windows + +package install + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +const simpleBlockForever = ` +package main + +import ( + "math" + "time" +) + +func main() { + <-time.After(time.Duration(math.MaxInt64)) +} +` + +func TestRemovePath(t *testing.T) { + dir := filepath.Join(t.TempDir(), "subdir") + err := os.Mkdir(dir, 0644) + require.NoError(t, err) + + src := filepath.Join(dir, "main.go") + err = os.WriteFile(src, []byte(simpleBlockForever), 0644) + require.NoError(t, err) + + binary := filepath.Join(dir, "main.exe") + cmd := exec.Command("go", "build", "-o", binary, src) + _, err = cmd.CombinedOutput() + require.NoError(t, err) + + cmd = exec.Command(binary) + err = cmd.Start() + require.NoError(t, err) + defer func() { + _ = cmd.Process.Kill() + _ = cmd.Wait() + }() + + err = RemovePath(dir) + require.NoError(t, err) +}