Return actionable error message when enrolling (#6144) (#6397)

* enhancement(4889): updated enroll command to reexec the command if the executing user and binary owner don't match, added tests

* enhancement(4889): added savepassword function

* enhancement(4889): removed user impersonation, implemented noop for windows, updated integration tests

* enhancement(4889): added changelog

* enhancement(4889): added windows implementation

* enhancement(4889): added license headers

* enhancement(4889): update test case to execute different commands based on os

* enhancement(4889): ran mage clean

* enhancement(4889): updated function name, updated integration tests

* enhancmenet(4889): updated function name

* enhancement(4889): updated function name, fixed test assertions

* enhancement(4889): update error messages

* enhancement(4889): ran mage update

* enhancements(4889): fix integration test

* enhancement(4889): remove commented code

* enhancement(4889): commiting now, don't push though

* enhancement(4889): added windows unit test

* enhancement(4889): close test file

* enhancement(4889): updated isOwnerExec function, added tests

* enchancement(4889): added log in test

* enhancement(4889): remove unused types

* enhancement(4889): added test for isOwnerExec for windows

* enhancement(4889): added unix tests, ran mage addLicenseHeaders

* enhancement(4889): added testisfileownerunix test

* enhancement(4889): added TestIsOwnerExecUnix test

* enhancement(4889): updated test function name

* enhancement(4889): set file ownership of the test file

* enhancement(4889): remove unnecessary change

* enhancment(4889): updated iOwnerExec windows test

* enhancement(4889): fix file creation in tests

* Update internal/pkg/agent/cmd/enroll.go

Co-authored-by: Paolo Chilà <[email protected]>

* Update internal/pkg/agent/cmd/enroll_match_fileowner_windows.go

Co-authored-by: Paolo Chilà <[email protected]>

* Update internal/pkg/agent/cmd/enroll_match_fileowner_unix.go

Co-authored-by: Paolo Chilà <[email protected]>

* enhancement(4889): updated summary and issue in changelog

* enhancement(4889): updated variable name, wrap error when returning error

* enhancement(4889): added comments describing isOwnerExec. fixed type problems with stat.Uid

* enhancement(4889): refactored isOwnerExec return

* enhancement(4889): differentiating between sid errors

* enhancment(4889): removed redundant unit tests

* enhancement(4889): update command execution in integration tests

---------

Co-authored-by: Paolo Chilà <[email protected]>
(cherry picked from commit c4835ca)

Co-authored-by: Kaan Yalti <[email protected]>

changelog/fragments/1732656422-add-actionable-error-message-for-enroll-command.yaml

@@ -0,0 +1,31 @@
+# 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: enhancement
+
+# Change summary; a 80ish characters long description of the change.
+summary: Elastic agent returns an actionable error message when a the use trying to execute the enroll command is not the same as the onwer of the elastic-agent program files
+
+# 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; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: elastic-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/elastic/elastic-agent/pull/6144
+# 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/4889

internal/pkg/agent/cmd/enroll.go

@@ -351,6 +351,24 @@ func enroll(streams *cli.IOStreams, cmd *cobra.Command) error {
 
 	fromInstall, _ := cmd.Flags().GetBool(fromInstallArg)
 
+	hasRoot, err := utils.HasRoot()
+	if err != nil {
+		return fmt.Errorf("checking if running with root/Administrator privileges: %w", err)
+	}
+	if hasRoot && !fromInstall {
+		binPath, err := os.Executable()
+		if err != nil {
+			return fmt.Errorf("error while getting executable path: %w", err)
+		}
+		isOwner, err := isOwnerExec(binPath)
+		if err != nil {
+			return fmt.Errorf("ran into an error while figuring out if user is allowed to execute the enroll command: %w", err)
+		}
+		if !isOwner {
+			return UserOwnerMismatchError
+		}
+	}
+
 	pathConfigFile := paths.ConfigFile()
 	rawConfig, err := config.LoadFile(pathConfigFile)
 	if err != nil {
@@ -525,7 +543,6 @@ func enroll(streams *cli.IOStreams, cmd *cobra.Command) error {
 		pathConfigFile,
 		store,
 	)
-
 	if err != nil {
 		return err
 	}

internal/pkg/agent/cmd/enroll_match_fileowner_unix.go

@@ -0,0 +1,59 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License 2.0;
+// you may not use this file except in compliance with the Elastic License 2.0.
+
+//go:build !windows
+
+package cmd
+
+import (
+	"fmt"
+	"os"
+	"strconv"
+	"syscall"
+
+	"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
+)
+
+var UserOwnerMismatchError = errors.New("the command is executed as root but the program files are not owned by the root user. execute the command as the user that owns the program files")
+
+func getFileOwner(filePath string) (string, error) {
+	fileInfo, err := os.Stat(filePath)
+	if err != nil {
+		return "", fmt.Errorf("failed to get file info: %w", err)
+	}
+
+	stat, ok := fileInfo.Sys().(*syscall.Stat_t)
+	if !ok {
+		return "", fmt.Errorf("failed to get system specific file info: %w", err)
+	}
+	return strconv.Itoa(int(stat.Uid)), nil
+}
+
+func getCurrentUser() (string, error) {
+	return strconv.Itoa(os.Geteuid()), nil
+}
+
+func isFileOwner(curUser string, fileOwner string) (bool, error) {
+	return curUser == fileOwner, nil
+}
+
+// Checks if the provided file is owned by the user that initiated the process
+func isOwnerExec(filePath string) (bool, error) {
+	owner, err := getFileOwner(filePath)
+	if err != nil {
+		return false, fmt.Errorf("failed to get file owner: %w", err)
+	}
+
+	curUser, err := getCurrentUser()
+	if err != nil {
+		return false, fmt.Errorf("failed to get current user: %w", err)
+	}
+
+	isOwner, err := isFileOwner(curUser, owner)
+	if err != nil {
+		return false, fmt.Errorf("error while checking if current user is the file owner: %w", err)
+	}
+
+	return isOwner, nil
+}

internal/pkg/agent/cmd/enroll_match_fileowner_unix_test.go

@@ -0,0 +1,28 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License 2.0;
+// you may not use this file except in compliance with the Elastic License 2.0.
+
+//go:build !windows
+
+package cmd
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestIsOwnerExecUnix(t *testing.T) {
+	path := t.TempDir()
+	fp := filepath.Join(path, "testfile")
+	fi, err := os.Create(fp)
+	require.NoError(t, err)
+	defer fi.Close()
+
+	isOwner, err := isOwnerExec(fp)
+	require.NoError(t, err)
+
+	require.True(t, isOwner)
+}

internal/pkg/agent/cmd/enroll_match_fileowner_windows.go

@@ -0,0 +1,92 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License 2.0;
+// you may not use this file except in compliance with the Elastic License 2.0.
+
+//go:build windows
+
+package cmd
+
+import (
+	"fmt"
+
+	"golang.org/x/sys/windows"
+
+	"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
+)
+
+var UserOwnerMismatchError = errors.New("the command is executed as root but the program files are not owned by the root user.")
+
+func getFileOwner(filePath string) (string, error) {
+	// Get security information of the file
+	sd, err := windows.GetNamedSecurityInfo(
+		filePath,
+		windows.SE_FILE_OBJECT,
+		windows.OWNER_SECURITY_INFORMATION,
+	)
+	if err != nil {
+		return "", fmt.Errorf("failed to get security info: %w", err)
+	}
+	owner, _, err := sd.Owner()
+	if err != nil {
+		return "", fmt.Errorf("failed to get security descriptor owner: %w", err)
+	}
+
+	return owner.String(), nil
+}
+
+// Helper to get the current user's SID
+func getCurrentUser() (string, error) {
+	// Get the token for the current process
+	var token windows.Token
+	err := windows.OpenProcessToken(windows.CurrentProcess(), windows.TOKEN_QUERY, &token)
+	if err != nil {
+		return "", fmt.Errorf("failed to open process token: %w", err)
+	}
+	defer token.Close()
+
+	// Get the token use
+	tokenUser, err := token.GetTokenUser()
+	if err != nil {
+		return "", fmt.Errorf("failed to get token user: %w", err)
+	}
+
+	return tokenUser.User.Sid.String(), nil
+}
+
+func isFileOwner(curUser string, fileOwner string) (bool, error) {
+	var cSid *windows.SID
+	err := windows.ConvertStringSidToSid(windows.StringToUTF16Ptr(curUser), &cSid)
+	if err != nil {
+		return false, fmt.Errorf("failed to convert user SID string to SID: %w", err)
+	}
+
+	var fSid *windows.SID
+	err = windows.ConvertStringSidToSid(windows.StringToUTF16Ptr(fileOwner), &fSid)
+	if err != nil {
+		return false, fmt.Errorf("failed to convert file SID string to SID: %w", err)
+	}
+
+	isEqual := fSid.Equals(cSid)
+
+	return isEqual, nil
+}
+
+// Checks if the provided file is owned by the user that initiated the process
+func isOwnerExec(filePath string) (bool, error) {
+	fileOwner, err := getFileOwner(filePath)
+	if err != nil {
+		return false, fmt.Errorf("getting file owner: %w", err)
+	}
+
+	user, err := getCurrentUser()
+	if err != nil {
+		return false, fmt.Errorf("ran into an error while retrieving current user: %w", err)
+	}
+
+	isOwner, err := isFileOwner(user, fileOwner)
+	if err != nil {
+		return false, fmt.Errorf("error while checking if current user is the file owner: %w", err)
+	}
+
+	return isOwner, nil
+}

internal/pkg/agent/cmd/enroll_match_fileowner_windows_test.go

@@ -0,0 +1,52 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License 2.0;
+// you may not use this file except in compliance with the Elastic License 2.0.
+
+//go:build windows
+
+package cmd
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/windows"
+)
+
+func TestIsOwnerExecWindows(t *testing.T) {
+	path := t.TempDir()
+	fp := filepath.Join(path, "testfile")
+	fi, err := os.Create(fp)
+	require.NoError(t, err)
+	defer fi.Close()
+
+	var token windows.Token
+	err = windows.OpenProcessToken(windows.CurrentProcess(), windows.TOKEN_QUERY, &token)
+	require.NoError(t, err)
+	defer token.Close()
+
+	tokenUser, err := token.GetTokenUser()
+	require.NoError(t, err)
+
+	err = windows.SetNamedSecurityInfo(
+		fp,
+		windows.SE_FILE_OBJECT,
+		windows.OWNER_SECURITY_INFORMATION,
+		tokenUser.User.Sid,
+		nil,
+		nil,
+		nil,
+	)
+	require.NoError(t, err)
+
+	require.NoError(t, err)
+	defer fi.Close()
+
+	isOwner, err := isOwnerExec(fp)
+	require.NoError(t, err)
+
+	require.True(t, isOwner, fmt.Sprintf("expected isOwnerExec to return \"true\", received \"%v\"", isOwner))
+}

pkg/testing/tools/tools.go

@@ -42,7 +42,8 @@ func InstallAgentWithPolicy(ctx context.Context, t *testing.T,
 	installOpts atesting.InstallOpts,
 	agentFixture *atesting.Fixture,
 	kibClient *kibana.Client,
-	createPolicyReq kibana.AgentPolicy) (kibana.PolicyResponse, error) {
+	createPolicyReq kibana.AgentPolicy,
+) (kibana.PolicyResponse, error) {
 	t.Helper()
 
 	// Create policy
@@ -85,21 +86,41 @@ func InstallAgentForPolicy(ctx context.Context, t *testing.T,
 	installOpts atesting.InstallOpts,
 	agentFixture *atesting.Fixture,
 	kibClient *kibana.Client,
-	policyID string) error {
-	t.Helper()
+	policyID string,
+) error {
+	enrollmentToken, err := CreateEnrollmentToken(t, ctx, kibClient, policyID)
+	if err != nil {
+		return fmt.Errorf("failed to create enrollment token while preparing to install agent for policy: %w", err)
+	}
+	return InstallAgentForPolicyWithToken(ctx, t, installOpts, agentFixture, kibClient, policyID, enrollmentToken)
+}
 
+func CreateEnrollmentToken(t *testing.T, ctx context.Context, kibClient *kibana.Client, policyID string) (kibana.CreateEnrollmentAPIKeyResponse, error) {
 	// Create enrollment API key
 	createEnrollmentAPIKeyReq := kibana.CreateEnrollmentAPIKeyRequest{
 		PolicyID: policyID,
 	}
 
+	t.Logf("Creating enrollment API key...")
+	enrollmentToken, err := kibClient.CreateEnrollmentAPIKey(ctx, createEnrollmentAPIKeyReq)
+	if err != nil {
+		return kibana.CreateEnrollmentAPIKeyResponse{}, fmt.Errorf("failed creating enrollment API key: %w", err)
+	}
+
+	return enrollmentToken, nil
+}
+
+func InstallAgentForPolicyWithToken(ctx context.Context, t *testing.T,
+	installOpts atesting.InstallOpts,
+	agentFixture *atesting.Fixture,
+	kibClient *kibana.Client,
+	policyID string,
+	enrollmentToken kibana.CreateEnrollmentAPIKeyResponse,
+) error {
+	t.Helper()
+
 	if installOpts.EnrollmentToken == "" {
 		t.Logf("Creating enrollment API key...")
-		enrollmentToken, err := kibClient.CreateEnrollmentAPIKey(ctx, createEnrollmentAPIKeyReq)
-		if err != nil {
-			return fmt.Errorf("failed creating enrollment API key: %w", err)
-		}
-
 		installOpts.EnrollmentToken = enrollmentToken.APIKey
 	}
 
@@ -138,5 +159,6 @@ func InstallAgentForPolicy(ctx context.Context, t *testing.T,
 		10*time.Second,
 		"Elastic Agent status is not online",
 	)
+
 	return nil
 }