diff --git a/changelog/fragments/1732656422-add-actionable-error-message-for-enroll-command.yaml b/changelog/fragments/1732656422-add-actionable-error-message-for-enroll-command.yaml new file mode 100644 index 00000000000..5bc0e037318 --- /dev/null +++ b/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 diff --git a/internal/pkg/agent/cmd/enroll.go b/internal/pkg/agent/cmd/enroll.go index 924ab373f8f..b1f82e6989e 100644 --- a/internal/pkg/agent/cmd/enroll.go +++ b/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 } diff --git a/internal/pkg/agent/cmd/enroll_match_fileowner_unix.go b/internal/pkg/agent/cmd/enroll_match_fileowner_unix.go new file mode 100644 index 00000000000..6e0df49a060 --- /dev/null +++ b/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 +} diff --git a/internal/pkg/agent/cmd/enroll_match_fileowner_unix_test.go b/internal/pkg/agent/cmd/enroll_match_fileowner_unix_test.go new file mode 100644 index 00000000000..9ad3b22db8c --- /dev/null +++ b/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) +} diff --git a/internal/pkg/agent/cmd/enroll_match_fileowner_windows.go b/internal/pkg/agent/cmd/enroll_match_fileowner_windows.go new file mode 100644 index 00000000000..75ccacd0997 --- /dev/null +++ b/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 +} diff --git a/internal/pkg/agent/cmd/enroll_match_fileowner_windows_test.go b/internal/pkg/agent/cmd/enroll_match_fileowner_windows_test.go new file mode 100644 index 00000000000..6af8b984014 --- /dev/null +++ b/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)) +} diff --git a/pkg/testing/tools/tools.go b/pkg/testing/tools/tools.go index 5bc9346842e..b36e9a4fc81 100644 --- a/pkg/testing/tools/tools.go +++ b/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 } diff --git a/testing/integration/enroll_unprivileged_test.go b/testing/integration/enroll_unprivileged_test.go new file mode 100644 index 00000000000..48bf6ab4f30 --- /dev/null +++ b/testing/integration/enroll_unprivileged_test.go @@ -0,0 +1,78 @@ +// 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 integration + +package integration + +import ( + "context" + "os" + "testing" + + "github.com/gofrs/uuid/v5" + "github.com/stretchr/testify/require" + + "github.com/elastic/elastic-agent-libs/kibana" + "github.com/elastic/elastic-agent/internal/pkg/agent/cmd" + atesting "github.com/elastic/elastic-agent/pkg/testing" + "github.com/elastic/elastic-agent/pkg/testing/define" + "github.com/elastic/elastic-agent/pkg/testing/tools" + "github.com/elastic/elastic-agent/pkg/testing/tools/fleettools" +) + +func TestEnrollUnprivileged(t *testing.T) { + info := define.Require(t, define.Requirements{ + Group: Default, + Stack: &define.Stack{}, + Sudo: true, + }) + t.Run("unenrolled unprivileged agent re-enrolls successfully using root user", func(t *testing.T) { + ctx := context.Background() + fixture, err := define.NewFixtureFromLocalBuild(t, define.Version()) + require.NoError(t, err) + installOpts := atesting.InstallOpts{ + NonInteractive: true, + Force: true, + Privileged: false, + } + + randId := uuid.Must(uuid.NewV4()).String() + policyReq := kibana.AgentPolicy{ + Name: "test-policy-" + randId, + Namespace: "default", + Description: "Test policy " + randId, + MonitoringEnabled: []kibana.MonitoringEnabledOption{ + kibana.MonitoringEnabledLogs, + kibana.MonitoringEnabledMetrics, + }, + } + policy, err := info.KibanaClient.CreatePolicy(ctx, policyReq) + require.NoError(t, err) + + enrollmentApiKey, err := tools.CreateEnrollmentToken(t, ctx, info.KibanaClient, policy.ID) + require.NoError(t, err) + + err = tools.InstallAgentForPolicyWithToken(ctx, t, installOpts, fixture, info.KibanaClient, policy.ID, enrollmentApiKey) + require.NoError(t, err) + + hostname, err := os.Hostname() + require.NoError(t, err) + + agent, err := fleettools.GetAgentByPolicyIDAndHostnameFromList(ctx, info.KibanaClient, policy.ID, hostname) + require.NoError(t, err) + + _, err = info.KibanaClient.UnEnrollAgent(ctx, kibana.UnEnrollAgentRequest{ID: agent.ID}) + require.NoError(t, err) + + enrollUrl, err := fleettools.DefaultURL(ctx, info.KibanaClient) + require.NoError(t, err) + + enrollArgs := []string{"enroll", "--url", enrollUrl, "--enrollment-token", enrollmentApiKey.APIKey, "--force"} + + out, err := fixture.Exec(ctx, enrollArgs) + require.Error(t, err) + require.Contains(t, string(out), cmd.UserOwnerMismatchError.Error()) + }) +}