Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: enable root user to re-enroll unprivileged agent for mac and linux

# 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/9603

# 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/8544
46 changes: 25 additions & 21 deletions internal/pkg/agent/cmd/enroll.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,24 +374,6 @@ func doEnroll(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 {
Expand Down Expand Up @@ -492,6 +474,11 @@ func doEnroll(streams *cli.IOStreams, cmd *cobra.Command) error {
ctx = eCtx
}

hasRoot, err := utils.HasRoot()
if err != nil {
return fmt.Errorf("checking if running with root/Administrator privileges: %w", err)
}

// On MacOS Ventura and above, fixing the permissions on enrollment during installation fails with the error:
// Error: failed to fix permissions: chown /Library/Elastic/Agent/data/elastic-agent-c13f91/elastic-agent.app: operation not permitted
// This is because we are fixing permissions twice, once during installation and again during the enrollment step.
Expand All @@ -504,9 +491,26 @@ func doEnroll(streams *cli.IOStreams, cmd *cobra.Command) error {
return err
}
fixPermissions = &perms
}
if runtime.GOOS == "darwin" {
fixPermissions = nil

if runtime.GOOS == "darwin" {
fixPermissions = nil
}
} else {
agentState, err := getDaemonState(ctx)
if err != nil {
return err
}
if hasRoot && agentState.Info.Unprivileged && runtime.GOOS != "windows" { // windows implementation is a no-op, will be addressed in a separate PR
binPath, err := os.Executable()
if err != nil {
return fmt.Errorf("error while getting executable path: %w", err)
}

perms, err := getFileOwnerFromPath(binPath)
if err == nil {
fixPermissions = &perms
}
}
}

options := enroll.EnrollOptions{
Expand Down
55 changes: 0 additions & 55 deletions internal/pkg/agent/cmd/enroll_match_fileowner_unix.go

This file was deleted.

28 changes: 0 additions & 28 deletions internal/pkg/agent/cmd/enroll_match_fileowner_unix_test.go

This file was deleted.

12 changes: 0 additions & 12 deletions internal/pkg/agent/cmd/enroll_match_fileowner_windows.go

This file was deleted.

28 changes: 0 additions & 28 deletions internal/pkg/agent/cmd/enroll_match_fileowner_windows_test.go

This file was deleted.

15 changes: 15 additions & 0 deletions internal/pkg/agent/cmd/enroll_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ package cmd

import (
"fmt"
"os"
"syscall"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -51,3 +53,16 @@ func getIDFromCmd(cmd *cobra.Command, param string) (int, error) {
}
return int(id), nil
}

// getFileOwnerFromPath returns the UID and GID owning the provided file path.
func getFileOwnerFromPath(path string) (utils.FileOwner, error) {
fi, err := os.Stat(path)
if err != nil {
return utils.FileOwner{}, fmt.Errorf("failed to stat %q: %w", path, err)
}
stat, ok := fi.Sys().(*syscall.Stat_t)
if !ok {
return utils.FileOwner{}, fmt.Errorf("failed to get system-specific stat for %q", path)
}
return utils.FileOwner{UID: int(stat.Uid), GID: int(stat.Gid)}, nil
}
65 changes: 65 additions & 0 deletions internal/pkg/agent/cmd/enroll_unix_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// 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"

"github.com/elastic/elastic-agent/pkg/utils"
)

type testCase struct {
setup func(t *testing.T) string
wantErr bool
}

func TestGetFileOwnerFromPath(t *testing.T) {
testCases := map[string]testCase{
"returns current owner for regular file": {
setup: func(t *testing.T) string {
dir := t.TempDir()
fp := filepath.Join(dir, "tmpfile")
f, err := os.Create(fp)
require.NoError(t, err, "failed to create temp file")
require.NoError(t, f.Close(), "failed to close temp file")
return fp
},
},
"returns current owner for directory": {
setup: func(t *testing.T) string {
return t.TempDir()
},
},
"returns error for nonexistent path": {
setup: func(t *testing.T) string {
return filepath.Join(t.TempDir(), "does", "not", "exist")
},
wantErr: true,
},
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
path := tc.setup(t)
owner, err := getFileOwnerFromPath(path)
if tc.wantErr {
require.Error(t, err, "expected error for path %q", path)
return
}
require.NoError(t, err, "unexpected error for path %q", path)

cur, err := utils.CurrentFileOwner()
require.NoError(t, err, "failed to get current file owner")
require.Equal(t, cur.UID, owner.UID, "uid mismatch for %q", path)
require.Equal(t, cur.GID, owner.GID, "gid mismatch for %q", path)
})
}
}
5 changes: 5 additions & 0 deletions internal/pkg/agent/cmd/enroll_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,8 @@ func getSIDFromCmd(cmd *cobra.Command, param string) (*windows.SID, error) {
}
return nil, nil
}

func getFileOwnerFromPath(path string) (utils.FileOwner, error) {
// TODO: Implement this
return utils.FileOwner{}, nil
}
14 changes: 5 additions & 9 deletions testing/integration/ess/re-enroll_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"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"
Expand All @@ -33,11 +32,9 @@ type testCase struct {
assertion AssertFunc
}

// TestReEnrollUnprivileged verifies that re-enrollment as a privileged user fails
// when the agent was installed unprivileged. This enforces the file ownership check
// on Unix platforms. On Windows, this check is a no-op as of PR #8503, so this test
// is not run for windows. See the discussion in PR #8503 (https://github.com/elastic/elastic-agent/pull/8503)
// and comment (https://github.com/elastic/elastic-agent/pull/8503#discussion_r2152603141) for context.
// Verifies that re-enrollment as a privileged user succeeds when the agent was
// installed unprivileged. Windows implementation is a no-op and will be addressed
// in a separate PR. Relevant issue: https://github.com/elastic/elastic-agent/issues/8544
func TestReEnrollUnprivileged(t *testing.T) {
info := define.Require(t, define.Requirements{
Group: integration.Default,
Expand All @@ -53,9 +50,8 @@ func TestReEnrollUnprivileged(t *testing.T) {

fixture, enrollArgs := prepareAgentforReEnroll(t, ctx, info, false)

out, err := fixture.Exec(ctx, enrollArgs)
require.Error(t, err)
require.Contains(t, string(out), cmd.UserOwnerMismatchError.Error())
_, err := fixture.Exec(ctx, enrollArgs)
require.NoError(t, err)

assert.Eventuallyf(t, func() bool {
err := fixture.IsHealthy(t.Context())
Expand Down
Loading