From dca8086a711618a12b58a386752eb23e6975a20e Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Tue, 3 Dec 2024 21:04:33 +0100 Subject: [PATCH 1/2] Add support for pre existing Active Directory user (#5988) * unpriviledged ad works' * Ensure rights for AD users * changelog * support for mac and unix * Update internal/pkg/agent/cmd/install.go Co-authored-by: Blake Rouse * Update internal/pkg/agent/cmd/unprivileged.go Co-authored-by: Blake Rouse * added e2e tests for darwin and linux * reverted sample_test * mage fmt * do not require password * more strict regex * fix description in changelog * resolved review comments * linter * handle g115 in user_windows.go * more test coverage * fix broken windows UT * coverage * fixed logic for UnprivilegedUser * fixed windows tests --------- Co-authored-by: Blake Rouse (cherry picked from commit dccfb70994b636388c2f1a999c09b39eacc6d449) --- ...-Directory-user-for-unprivileged-mode.yaml | 32 ++++++ internal/pkg/agent/cmd/install.go | 21 +++- internal/pkg/agent/cmd/privileged.go | 2 +- internal/pkg/agent/cmd/unprivileged.go | 20 +++- internal/pkg/agent/install/install.go | 33 ++++-- internal/pkg/agent/install/install_test.go | 45 ++++++++ internal/pkg/agent/install/install_unix.go | 2 +- internal/pkg/agent/install/install_windows.go | 37 +++++- .../pkg/agent/install/install_windows_test.go | 79 +++++++++++++ internal/pkg/agent/install/prereq.go | 11 +- internal/pkg/agent/install/switch.go | 6 +- internal/pkg/agent/install/user_darwin.go | 2 + internal/pkg/agent/install/user_linux.go | 2 + internal/pkg/agent/install/user_test.go | 18 +++ internal/pkg/agent/install/user_windows.go | 23 ++-- pkg/testing/fixture_install.go | 10 ++ testing/installtest/checks.go | 2 + testing/installtest/checks_unix.go | 44 ++++++-- testing/installtest/checks_windows.go | 24 ++-- testing/integration/install_test.go | 106 +++++++++++++----- .../integration/switch_unprivileged_test.go | 71 +++++++++++- 21 files changed, 517 insertions(+), 73 deletions(-) create mode 100644 changelog/fragments/1731314919-Added-support-for-custom-Active-Directory-user-for-unprivileged-mode.yaml create mode 100644 internal/pkg/agent/install/install_windows_test.go create mode 100644 internal/pkg/agent/install/user_test.go diff --git a/changelog/fragments/1731314919-Added-support-for-custom-Active-Directory-user-for-unprivileged-mode.yaml b/changelog/fragments/1731314919-Added-support-for-custom-Active-Directory-user-for-unprivileged-mode.yaml new file mode 100644 index 00000000000..fcbf11fd0e7 --- /dev/null +++ b/changelog/fragments/1731314919-Added-support-for-custom-Active-Directory-user-for-unprivileged-mode.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: feature + +# Change summary; a 80ish characters long description of the change. +summary: Added support for pre-existing Active Directory user for unprivileged mode + +# 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: User can specify custom pre-existing user for running unprivileged mode. This user will be given permissions to log on as a service. + +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: + +# 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/elastic/elastic-agent/issues/4585 diff --git a/internal/pkg/agent/cmd/install.go b/internal/pkg/agent/cmd/install.go index 5182be314ef..b6f8447b779 100644 --- a/internal/pkg/agent/cmd/install.go +++ b/internal/pkg/agent/cmd/install.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "github.com/spf13/cobra" @@ -28,6 +29,10 @@ const ( flagInstallDevelopment = "develop" flagInstallNamespace = "namespace" flagInstallRunUninstallFromBinary = "run-uninstall-from-binary" + + flagInstallCustomUser = "user" + flagInstallCustomGroup = "group" + flagInstallCustomPass = "password" ) func newInstallCommandWithArgs(_ []string, streams *cli.IOStreams) *cobra.Command { @@ -61,6 +66,13 @@ would like the Agent to operate. cmd.Flags().Bool(flagInstallDevelopment, false, "Install into a standardized development namespace, may enable development specific options. Allows multiple Elastic Agents to be installed at once. (experimental)") _ = cmd.Flags().MarkHidden(flagInstallDevelopment) // For internal use only. + // Active directory user specification + cmd.Flags().String(flagInstallCustomUser, "", "Custom user used to run Elastic Agent") + cmd.Flags().String(flagInstallCustomGroup, "", "Custom group used to access Elastic Agent files") + if runtime.GOOS == "windows" { + cmd.Flags().String(flagInstallCustomPass, "", "Password for user used to run Elastic Agent") + } + addEnrollFlags(cmd) return cmd @@ -249,7 +261,14 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command) error { progBar.Describe("Successfully uninstalled Elastic Agent") } if status != install.PackageInstall { - ownership, err = install.Install(cfgFile, topPath, unprivileged, log, progBar, streams) + customUser, _ := cmd.Flags().GetString(flagInstallCustomUser) + customGroup, _ := cmd.Flags().GetString(flagInstallCustomGroup) + customPass := "" + if runtime.GOOS == "windows" { + customPass, _ = cmd.Flags().GetString(flagInstallCustomPass) + } + + ownership, err = install.Install(cfgFile, topPath, unprivileged, log, progBar, streams, customUser, customGroup, customPass) if err != nil { return fmt.Errorf("error installing package: %w", err) } diff --git a/internal/pkg/agent/cmd/privileged.go b/internal/pkg/agent/cmd/privileged.go index c3eaf972031..2d462ec16d9 100644 --- a/internal/pkg/agent/cmd/privileged.go +++ b/internal/pkg/agent/cmd/privileged.go @@ -69,7 +69,7 @@ func privilegedCmd(streams *cli.IOStreams, cmd *cobra.Command) (err error) { } pt := install.CreateAndStartNewSpinner(streams.Out, "Converting Elastic Agent to privileged...") - err = install.SwitchExecutingMode(topPath, pt, "", "") + err = install.SwitchExecutingMode(topPath, pt, "", "", "") if err != nil { // error already adds context return err diff --git a/internal/pkg/agent/cmd/unprivileged.go b/internal/pkg/agent/cmd/unprivileged.go index 00f0cfdbc8c..a6d7ea70f06 100644 --- a/internal/pkg/agent/cmd/unprivileged.go +++ b/internal/pkg/agent/cmd/unprivileged.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "os" + "runtime" "github.com/spf13/cobra" @@ -44,6 +45,13 @@ unprivileged it will still perform all the same work, including stopping and sta cmd.Flags().BoolP("force", "f", false, "Do not prompt for confirmation") cmd.Flags().DurationP("daemon-timeout", "", 0, "Timeout waiting for Elastic Agent daemon restart after the change is applied (-1 = no wait)") + // Custom user specification + cmd.Flags().String(flagInstallCustomUser, "", "Custom user used to run Elastic Agent") + cmd.Flags().String(flagInstallCustomGroup, "", "Custom group used to access Elastic Agent files") + if runtime.GOOS == "windows" { + cmd.Flags().String(flagInstallCustomPass, "", "Password for user used to run Elastic Agent") + } + return cmd } @@ -56,6 +64,13 @@ func unprivilegedCmd(streams *cli.IOStreams, cmd *cobra.Command) (err error) { return fmt.Errorf("unable to perform unprivileged command, not executed with %s permissions", utils.PermissionUser) } + customUser, _ := cmd.Flags().GetString(flagInstallCustomUser) + customGroup, _ := cmd.Flags().GetString(flagInstallCustomGroup) + customPass := "" + if runtime.GOOS == "windows" { + customPass, _ = cmd.Flags().GetString(flagInstallCustomPass) + } + // cannot switch to unprivileged when service components have issues err = ensureNoServiceComponentIssues() if err != nil { @@ -77,7 +92,10 @@ func unprivilegedCmd(streams *cli.IOStreams, cmd *cobra.Command) (err error) { } pt := install.CreateAndStartNewSpinner(streams.Out, "Converting Elastic Agent to unprivileged...") - err = install.SwitchExecutingMode(topPath, pt, install.ElasticUsername, install.ElasticGroupName) + + username, password := install.UnprivilegedUser(customUser, customPass) + groupName := install.UnprivilegedGroup(customGroup) + err = install.SwitchExecutingMode(topPath, pt, username, groupName, password) if err != nil { // error already adds context return err diff --git a/internal/pkg/agent/install/install.go b/internal/pkg/agent/install/install.go index ce70f8e7698..966df65703a 100644 --- a/internal/pkg/agent/install/install.go +++ b/internal/pkg/agent/install/install.go @@ -40,7 +40,7 @@ const ( ) // Install installs Elastic Agent persistently on the system including creating and starting its service. -func Install(cfgFile, topPath string, unprivileged bool, log *logp.Logger, 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, customUser, customGroup, userPassword string) (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) @@ -49,13 +49,15 @@ func Install(cfgFile, topPath string, unprivileged bool, log *logp.Logger, pt *p var ownership utils.FileOwner username := "" groupName := "" + password := "" if unprivileged { - username = ElasticUsername - groupName = ElasticGroupName - ownership, err = EnsureUserAndGroup(username, groupName, pt) + username, password = UnprivilegedUser(customUser, userPassword) + groupName = UnprivilegedGroup(customGroup) + ownership, err = EnsureUserAndGroup(username, groupName, pt, username == ElasticUsername && password == "") // force create only elastic user if err != nil { // error context already added by EnsureUserAndGroup return utils.FileOwner{}, err + } } @@ -147,7 +149,7 @@ func Install(cfgFile, topPath string, unprivileged bool, log *logp.Logger, pt *p // install service pt.Describe("Installing service") - err = InstallService(topPath, ownership, username, groupName) + err = InstallService(topPath, ownership, username, groupName, password) if err != nil { pt.Describe("Failed to install service") // error context already added by InstallService @@ -371,11 +373,12 @@ func StatusService(topPath string) (service.Status, error) { } // InstallService installs the service. -func InstallService(topPath string, ownership utils.FileOwner, username string, groupName string) error { - opts, err := withServiceOptions(username, groupName) +func InstallService(topPath string, ownership utils.FileOwner, username string, groupName string, password string) error { + opts, err := withServiceOptions(username, groupName, password) if err != nil { return fmt.Errorf("error getting service installation options: %w", err) } + svc, err := newService(topPath, opts...) if err != nil { return fmt.Errorf("error creating new service handler for install: %w", err) @@ -482,3 +485,19 @@ func CreateInstallMarker(topPath string, ownership utils.FileOwner) error { _ = handle.Close() return fixInstallMarkerPermissions(markerFilePath, ownership) } + +func UnprivilegedUser(username, password string) (string, string) { + if username != "" { + return username, password + } + + return ElasticUsername, password +} + +func UnprivilegedGroup(groupName string) string { + if groupName != "" { + return groupName + } + + return ElasticGroupName +} diff --git a/internal/pkg/agent/install/install_test.go b/internal/pkg/agent/install/install_test.go index 492ad43c1fe..2f407e6baab 100644 --- a/internal/pkg/agent/install/install_test.go +++ b/internal/pkg/agent/install/install_test.go @@ -5,6 +5,7 @@ package install import ( + "fmt" "os" "path/filepath" "testing" @@ -65,6 +66,50 @@ func TestHasAllSSDs(t *testing.T) { } } +func TestUnprivilegedUser(t *testing.T) { + testCases := []struct { + username string + password string + expectedUsername string + expectedPassword string + }{ + {"", "", ElasticUsername, ""}, + {"", "pass", ElasticUsername, "pass"}, + {"user", "", "user", ""}, + {"user", "pass", "user", "pass"}, + } + for i, tc := range testCases { + t.Run( + fmt.Sprintf("test case #%d: %s:%s", i, tc.username, tc.password), + func(t *testing.T) { + username, password := UnprivilegedUser(tc.username, tc.password) + assert.Equal(t, tc.expectedUsername, username) + assert.Equal(t, password, tc.expectedPassword) + }, + ) + } +} + +func TestUnprivilegedGroup(t *testing.T) { + testCases := []struct { + groupName string + expectedGroupName string + }{ + {"", ElasticGroupName}, + {"custom", "custom"}, + } + + for i, tc := range testCases { + t.Run( + fmt.Sprintf("test case #%d: %s", i, tc.groupName), + func(t *testing.T) { + groupname := UnprivilegedGroup(tc.groupName) + assert.Equal(t, tc.expectedGroupName, groupname) + }, + ) + } +} + var sampleManifestContent = ` version: co.elastic.agent/v1 kind: PackageManifest diff --git a/internal/pkg/agent/install/install_unix.go b/internal/pkg/agent/install/install_unix.go index ca65c5b9e16..427d8886dbb 100644 --- a/internal/pkg/agent/install/install_unix.go +++ b/internal/pkg/agent/install/install_unix.go @@ -28,7 +28,7 @@ func fixInstallMarkerPermissions(markerFilePath string, ownership utils.FileOwne } // withServiceOptions just sets the user/group for the service. -func withServiceOptions(username string, groupName string) ([]serviceOpt, error) { +func withServiceOptions(username string, groupName string, _ string) ([]serviceOpt, error) { return []serviceOpt{withUserGroup(username, groupName)}, nil } diff --git a/internal/pkg/agent/install/install_windows.go b/internal/pkg/agent/install/install_windows.go index 368e9c5c081..d515dc7b6d2 100644 --- a/internal/pkg/agent/install/install_windows.go +++ b/internal/pkg/agent/install/install_windows.go @@ -10,6 +10,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "strings" "golang.org/x/sys/windows" @@ -21,6 +22,13 @@ import ( "github.com/elastic/elastic-agent/version" ) +const ( + // Conforming to https://learn.microsoft.com/en-us/troubleshoot/windows-server/active-directory/naming-conventions-for-computer-domain-site-ou#domain-names + // domain names can contain all alphanumeric characters except for the extended characters that appear in the Disallowed characters list. Names can contain a period, but names can't start with a period. + // Disallowed characters: [, ~ : @ # $ % ^ ' . ( ) { } _ {whitespace} \ / ] + activeDirectoryUsername = `^[A-Za-z0-9]+(?:\.[A-Za-z0-9]+)*\\[A-Za-z0-9.-]{1,104}$` +) + // postInstall performs post installation for Windows systems. func postInstall(topPath string) error { // delete the top-level elastic-agent.exe @@ -55,13 +63,24 @@ func fixInstallMarkerPermissions(markerFilePath string, ownership utils.FileOwne } // withServiceOptions just sets the user/group for the service. -func withServiceOptions(username string, groupName string) ([]serviceOpt, error) { +func withServiceOptions(username string, groupName string, password string) ([]serviceOpt, error) { if username == "" { // not installed with --unprivileged; nothing to do return []serviceOpt{}, nil } - // service requires a password to launch as the user + if password != "" { + if isFullDomainName, err := isWindowsDomainUsername(username); err != nil { + return nil, fmt.Errorf("failed to parse username: %w", err) + } else if !isFullDomainName { + return nil, fmt.Errorf(`username is not in proper format 'domain\username', contains illegal character: ,~:@#$%%^'.(){}_\/ or a whitespace`) + } + + // existing user + return []serviceOpt{withUserGroup(username, groupName), withPassword(password)}, nil + } + + // service requires a password to launch as the use // this sets it to a random password that is only known by the service password, err := RandomPassword() if err != nil { @@ -109,3 +128,17 @@ func serviceConfigure(ownership utils.FileOwner) error { } return nil } + +func isWindowsDomainUsername(username string) (bool, error) { + if !strings.Contains(username, `\`) { + // fail fast + return false, nil + } + + match, err := regexp.MatchString(activeDirectoryUsername, username) + if err != nil { + return false, err + } + + return match, nil +} diff --git a/internal/pkg/agent/install/install_windows_test.go b/internal/pkg/agent/install/install_windows_test.go new file mode 100644 index 00000000000..909316737d7 --- /dev/null +++ b/internal/pkg/agent/install/install_windows_test.go @@ -0,0 +1,79 @@ +// 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. + +package install + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsWindowsUsername(t *testing.T) { + testCases := []struct { + username string + expected bool + }{ + {``, false}, + {`user`, false}, + {`domain\user`, true}, + {`domain/user`, false}, + {`domain/domain\user`, false}, + {`domain\subdomain\user`, false}, + {`dom,ain\user`, false}, + {`doma~in\user`, false}, + {`domai:n\user`, false}, + {`dom.ain\user`, true}, + {`dom.ain\.user`, true}, + {`domain\usér`, false}, + } + + for _, tc := range testCases { + result, err := isWindowsDomainUsername(tc.username) + assert.NoError(t, err) + assert.Equal(t, tc.expected, result) + } +} + +func TestWithServiceOption(t *testing.T) { + testCases := []struct { + name string + groupName string + password string + expectedServiceOpts []serviceOpt + expectedError string + }{ + {"", "", "", []serviceOpt{}, ""}, + {"nonDomainUsername", "", "changeme", []serviceOpt{}, "username is not in proper format 'domain\\username', contains illegal character"}, + {`domain\username`, "", "changeme", []serviceOpt{withUserGroup(`domain\username`, ""), withPassword("changeme")}, ""}, + {`domain\username`, "group", "changeme", []serviceOpt{withUserGroup(`domain\username`, "group"), withPassword("changeme")}, ""}, + } + + for i, tc := range testCases { + t.Run( + fmt.Sprintf("test case #%d: %s:%s:%s", i, tc.name, tc.groupName, tc.password), + func(t *testing.T) { + opts, err := withServiceOptions(tc.name, tc.groupName, tc.password) + + if tc.expectedError != "" { + assert.True(t, strings.Contains(err.Error(), tc.expectedError)) + } + + assert.Equal(t, len(tc.expectedServiceOpts), len(opts)) + expected := serviceOpts{} + actual := serviceOpts{} + for _, opt := range tc.expectedServiceOpts { + opt(&expected) + } + + for _, opt := range opts { + opt(&actual) + } + + assert.EqualExportedValues(t, expected, actual) + }) + } +} diff --git a/internal/pkg/agent/install/prereq.go b/internal/pkg/agent/install/prereq.go index 428f3cd8da4..62c18008005 100644 --- a/internal/pkg/agent/install/prereq.go +++ b/internal/pkg/agent/install/prereq.go @@ -15,7 +15,7 @@ import ( // EnsureUserAndGroup creates the given username and group returning the file ownership information for that // user and group. -func EnsureUserAndGroup(username string, groupName string, pt *progressbar.ProgressBar) (utils.FileOwner, error) { +func EnsureUserAndGroup(username string, groupName string, pt *progressbar.ProgressBar, forceCreate bool) (utils.FileOwner, error) { var err error var ownership utils.FileOwner @@ -24,7 +24,7 @@ func EnsureUserAndGroup(username string, groupName string, pt *progressbar.Progr if err != nil && !errors.Is(err, ErrGroupNotFound) { return utils.FileOwner{}, fmt.Errorf("failed finding group %s: %w", groupName, err) } - if errors.Is(err, ErrGroupNotFound) { + if forceCreate && errors.Is(err, ErrGroupNotFound) { pt.Describe(fmt.Sprintf("Creating group %s", groupName)) ownership.GID, err = CreateGroup(groupName) if err != nil { @@ -39,7 +39,7 @@ func EnsureUserAndGroup(username string, groupName string, pt *progressbar.Progr if err != nil && !errors.Is(err, ErrUserNotFound) { return utils.FileOwner{}, fmt.Errorf("failed finding username %s: %w", username, err) } - if errors.Is(err, ErrUserNotFound) { + if forceCreate && errors.Is(err, ErrUserNotFound) { pt.Describe(fmt.Sprintf("Creating user %s", username)) ownership.UID, err = CreateUser(username, ownership.GID) if err != nil { @@ -53,5 +53,10 @@ func EnsureUserAndGroup(username string, groupName string, pt *progressbar.Progr } pt.Describe(fmt.Sprintf("Successfully created user %s", username)) } + + if err := EnsureRights(username); err != nil { + pt.Describe(fmt.Sprintf("Failed to assign rights to user %s", username)) + return utils.FileOwner{}, fmt.Errorf("failed to set proper rights to user %s: %w", username, err) + } return ownership, nil } diff --git a/internal/pkg/agent/install/switch.go b/internal/pkg/agent/install/switch.go index e5776c30a6e..cd6b02a9623 100644 --- a/internal/pkg/agent/install/switch.go +++ b/internal/pkg/agent/install/switch.go @@ -19,7 +19,7 @@ import ( // // When username and groupName are blank then it switched back to root/Administrator and when a username/groupName is // provided then it switched to running with that username and groupName. -func SwitchExecutingMode(topPath string, pt *progressbar.ProgressBar, username string, groupName string) error { +func SwitchExecutingMode(topPath string, pt *progressbar.ProgressBar, username string, groupName string, password string) error { // ensure service is stopped status, err := EnsureStoppedService(topPath, pt) if err != nil { @@ -38,7 +38,7 @@ func SwitchExecutingMode(topPath string, pt *progressbar.ProgressBar, username s // ensure user/group are created var ownership utils.FileOwner if username != "" && groupName != "" { - ownership, err = EnsureUserAndGroup(username, groupName, pt) + ownership, err = EnsureUserAndGroup(username, groupName, pt, username == ElasticUsername) if err != nil { // context for the error already provided in the EnsureUserAndGroup function return err @@ -78,7 +78,7 @@ func SwitchExecutingMode(topPath string, pt *progressbar.ProgressBar, username s // re-install service pt.Describe("Installing service") - err = InstallService(topPath, ownership, username, groupName) + err = InstallService(topPath, ownership, username, groupName, password) if err != nil { pt.Describe("Failed to install service") // error context already added by InstallService diff --git a/internal/pkg/agent/install/user_darwin.go b/internal/pkg/agent/install/user_darwin.go index 7256990062e..04b5b1858d0 100644 --- a/internal/pkg/agent/install/user_darwin.go +++ b/internal/pkg/agent/install/user_darwin.go @@ -177,3 +177,5 @@ func dsclExec(args ...string) error { } return nil } + +func EnsureRights(_ string) error { return nil } diff --git a/internal/pkg/agent/install/user_linux.go b/internal/pkg/agent/install/user_linux.go index 7bfa79514e6..3386838d2a9 100644 --- a/internal/pkg/agent/install/user_linux.go +++ b/internal/pkg/agent/install/user_linux.go @@ -92,3 +92,5 @@ func getentGetID(database string, key string) (int, error) { } return val, nil } + +func EnsureRights(_ string) error { return nil } diff --git a/internal/pkg/agent/install/user_test.go b/internal/pkg/agent/install/user_test.go new file mode 100644 index 00000000000..19d23f17d6f --- /dev/null +++ b/internal/pkg/agent/install/user_test.go @@ -0,0 +1,18 @@ +// 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 install + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestEnsureRights(t *testing.T) { + // no-op function testing for coverage + assert.NoError(t, EnsureRights("custom")) +} diff --git a/internal/pkg/agent/install/user_windows.go b/internal/pkg/agent/install/user_windows.go index ffaae4cdeea..9b63d96ba8c 100644 --- a/internal/pkg/agent/install/user_windows.go +++ b/internal/pkg/agent/install/user_windows.go @@ -127,38 +127,42 @@ func CreateUser(name string, _ string) (string, error) { return "", fmt.Errorf("call to NetUserAdd failed: status=%d error=%d", ret, parmErr) } + return FindUID(name) +} + +func EnsureRights(name string) error { // adjust the local security policy to ensure that the created user is scoped to a service only sid, _, _, err := gowin32.GetLocalAccountByName(name) if err != nil { - return "", fmt.Errorf("failed to get SID for %s: %w", name, err) + return fmt.Errorf("failed to get SID for %s: %w", name, err) } sp, err := gowin32.OpenLocalSecurityPolicy() if err != nil { - return "", fmt.Errorf("failed to open local security policy: %w", err) + return fmt.Errorf("failed to open local security policy: %w", err) } defer sp.Close() err = sp.AddAccountRight(sid, gowin32.AccountRightDenyInteractiveLogon) if err != nil { - return "", fmt.Errorf("failed to set deny interactive logon: %w", err) + return fmt.Errorf("failed to set deny interactive logon: %w", err) } err = sp.AddAccountRight(sid, gowin32.AccountRightDenyNetworkLogon) if err != nil { - return "", fmt.Errorf("failed to set deny network logon: %w", err) + return fmt.Errorf("failed to set deny network logon: %w", err) } err = sp.AddAccountRight(sid, gowin32.AccountRightDenyRemoteInteractiveLogon) if err != nil { - return "", fmt.Errorf("failed to set deny remote interactive logon: %w", err) + return fmt.Errorf("failed to set deny remote interactive logon: %w", err) } err = sp.AddAccountRight(sid, gowin32.AccountRightServiceLogon) if err != nil { - return "", fmt.Errorf("failed to set service logon: %w", err) + return fmt.Errorf("failed to set service logon: %w", err) } err = sp.AddAccountRight(sid, accountRightCreateSymbolicLink) if err != nil { - return "", fmt.Errorf("failed to add right to create symbolic link: %w", err) + return fmt.Errorf("failed to add right to create symbolic link: %w", err) } - return FindUID(name) + return nil } // AddUserToGroup adds a user to a group. @@ -178,12 +182,13 @@ func AddUserToGroup(username string, groupName string) error { entries[0] = LOCALGROUP_MEMBERS_INFO_0{ Lgrmi0_sid: userSid, } + ret, _, _ := procNetLocalGroupAddMembers.Call( uintptr(0), uintptr(unsafe.Pointer(groupNamePtr)), uintptr(uint32(0)), uintptr(unsafe.Pointer(&entries[0])), - uintptr(uint32(len(entries))), + uintptr(uint32(len(entries))), //nolint:gosec // G115 max 1 ) if ret != 0 { return fmt.Errorf("call to NetLocalGroupAddMembers failed: status=%d", ret) diff --git a/pkg/testing/fixture_install.go b/pkg/testing/fixture_install.go index e48c1ccdbfd..f8107bf58e7 100644 --- a/pkg/testing/fixture_install.go +++ b/pkg/testing/fixture_install.go @@ -112,6 +112,8 @@ type InstallOpts struct { Namespace string // --namespace, not supported for DEB and RPM. Privileged bool // inverse of --unprivileged (as false is the default) + Username string + Group string EnrollOpts FleetBootstrapOpts @@ -151,6 +153,14 @@ func (i *InstallOpts) ToCmdArgs() []string { } } + if i.Username != "" { + args = append(args, "--user", i.Username) + } + + if i.Group != "" { + args = append(args, "--group", i.Group) + } + args = append(args, i.EnrollOpts.toCmdArgs()...) args = append(args, i.FleetBootstrapOpts.toCmdArgs()...) diff --git a/testing/installtest/checks.go b/testing/installtest/checks.go index b28eaf6db53..dcc9baea5ac 100644 --- a/testing/installtest/checks.go +++ b/testing/installtest/checks.go @@ -40,6 +40,8 @@ func NamespaceTopPath(namespace string) string { type CheckOpts struct { Privileged bool Namespace string + Username string + Group string } func CheckSuccess(ctx context.Context, f *atesting.Fixture, topPath string, opts *CheckOpts) error { diff --git a/testing/installtest/checks_unix.go b/testing/installtest/checks_unix.go index 210664023e3..b8030f0bf6a 100644 --- a/testing/installtest/checks_unix.go +++ b/testing/installtest/checks_unix.go @@ -10,6 +10,7 @@ import ( "context" "fmt" "io/fs" + "math" "os" "os/exec" "path/filepath" @@ -24,17 +25,40 @@ import ( func checkPlatform(ctx context.Context, _ *atesting.Fixture, topPath string, opts *CheckOpts) error { if !opts.Privileged { // Check that the elastic-agent user/group exist. - uid, err := install.FindUID(install.ElasticUsername) + username := install.ElasticUsername + if opts.Username != "" { + username = opts.Username + } + uid, err := install.FindUID(username) if err != nil { - return fmt.Errorf("failed to find %s user: %w", install.ElasticUsername, err) + return fmt.Errorf("failed to find %s user: %w", username, err) + } + + group := install.ElasticGroupName + if opts.Username != "" { + group = opts.Group } - gid, err := install.FindGID(install.ElasticGroupName) + gid, err := install.FindGID(group) if err != nil { - return fmt.Errorf("failed to find %s group: %w", install.ElasticGroupName, err) + return fmt.Errorf("failed to find %s group: %w", group, err) + } + + var uid32 uint32 + if uid > math.MaxUint32 { + return fmt.Errorf("provided UID %d does is higher than %d", uid, math.MaxInt32) + } + //nolint:gosec // G115 false positive on conversion + uid32 = uint32(uid) + + var gid32 uint32 + if gid > math.MaxUint32 { + return fmt.Errorf("provided GID %d does is higher than %d", gid, math.MaxInt32) } + //nolint:gosec // G115 false positive on conversion + gid32 = uint32(gid) // Ensure entire installation tree has the correct permissions. - err = validateFileTree(topPath, uint32(uid), uint32(gid)) + err = validateFileTree(topPath, uid32, gid32) if err != nil { // context already added return err @@ -57,11 +81,11 @@ func checkPlatform(ctx context.Context, _ *atesting.Fixture, topPath string, opt if !ok { return fmt.Errorf("failed to convert info.Sys() into *syscall.Stat_t") } - if fs.Uid != uint32(uid) { - return fmt.Errorf("%s not owned by %s user", socketPath, install.ElasticUsername) + if fs.Uid != uid32 { + return fmt.Errorf("%s not owned by %s user", socketPath, username) } - if fs.Gid != uint32(gid) { - return fmt.Errorf("%s not owned by %s group", socketPath, install.ElasticGroupName) + if fs.Gid != gid32 { + return fmt.Errorf("%s not owned by %s group", socketPath, group) } // Executing `elastic-agent status` as the `elastic-agent-user` user should work. @@ -73,7 +97,7 @@ func checkPlatform(ctx context.Context, _ *atesting.Fixture, topPath string, opt var output []byte err = waitForNoError(ctx, func(_ context.Context) error { // #nosec G204 -- user cannot inject any parameters to this command - cmd := exec.Command("sudo", "-u", install.ElasticUsername, shellWrapperName, "status") + cmd := exec.Command("sudo", "-u", username, shellWrapperName, "status") output, err = cmd.CombinedOutput() if err != nil { return fmt.Errorf("elastic-agent status failed: %w (output: %s)", err, output) diff --git a/testing/installtest/checks_windows.go b/testing/installtest/checks_windows.go index e886c29d9ba..c242dbf4079 100644 --- a/testing/installtest/checks_windows.go +++ b/testing/installtest/checks_windows.go @@ -53,31 +53,41 @@ func checkPlatform(ctx context.Context, f *atesting.Fixture, topPath string, opt return fmt.Errorf("failed to get allowed SID's for %s: %w", topPath, err) } if !opts.Privileged { + username := install.ElasticUsername + if opts.Username != "" { + username = opts.Username + } + // Check that the elastic-agent user/group exist. - uid, err := install.FindUID(install.ElasticUsername) + uid, err := install.FindUID(username) if err != nil { - return fmt.Errorf("failed to find %s user: %w", install.ElasticUsername, err) + return fmt.Errorf("failed to find %s user: %w", username, err) } uidSID, err := windows.StringToSid(uid) if err != nil { return fmt.Errorf("failed to convert string to windows.SID %s: %w", uid, err) } - gid, err := install.FindGID(install.ElasticGroupName) + + group := install.ElasticGroupName + if opts.Username != "" { + group = opts.Group + } + gid, err := install.FindGID(group) if err != nil { - return fmt.Errorf("failed to find %s group: %w", install.ElasticGroupName, err) + return fmt.Errorf("failed to find %s group: %w", group, err) } gidSID, err := windows.StringToSid(gid) if err != nil { return fmt.Errorf("failed to convert string to windows.SID %s: %w", uid, err) } if !owner.Equals(uidSID) { - return fmt.Errorf("%s not owned by %s user", topPath, install.ElasticUsername) + return fmt.Errorf("%s not owned by %s user", topPath, username) } if !hasSID(sids, uidSID) { - return fmt.Errorf("path %s should have ACE for %s user", topPath, install.ElasticUsername) + return fmt.Errorf("path %s should have ACE for %s user", topPath, username) } if !hasSID(sids, gidSID) { - return fmt.Errorf("path %s should have ACE for %s group", topPath, install.ElasticGroupName) + return fmt.Errorf("path %s should have ACE for %s group", topPath, group) } // administrators should have access as well if !hasWellKnownSID(sids, windows.WinBuiltinAdministratorsSid) { diff --git a/testing/integration/install_test.go b/testing/integration/install_test.go index cd94c013688..ab149e1557f 100644 --- a/testing/integration/install_test.go +++ b/testing/integration/install_test.go @@ -20,9 +20,11 @@ import ( "testing" "time" + "github.com/schollz/progressbar/v3" "github.com/stretchr/testify/require" "github.com/elastic/elastic-agent/internal/pkg/agent/application/paths" + "github.com/elastic/elastic-agent/internal/pkg/agent/install" atesting "github.com/elastic/elastic-agent/pkg/testing" "github.com/elastic/elastic-agent/pkg/testing/define" "github.com/elastic/elastic-agent/pkg/testing/tools/fleettools" @@ -54,35 +56,41 @@ func TestInstallWithoutBasePath(t *testing.T) { err = fixture.Prepare(ctx) require.NoError(t, err) - // Run `elastic-agent install`. We use `--force` to prevent interactive - // execution. - opts := atesting.InstallOpts{Force: true, Privileged: false} - out, err := fixture.Install(ctx, &opts) - if err != nil { - t.Logf("install output: %s", out) - require.NoError(t, err) - } + testInstallWithoutBasePathWithCustomUser(ctx, t, fixture, "", "") +} - // Check that Agent was installed in default base path - topPath := installtest.DefaultTopPath() - require.NoError(t, installtest.CheckSuccess(ctx, fixture, topPath, &installtest.CheckOpts{Privileged: opts.Privileged})) +func TestInstallWithoutBasePathWithCustomUser(t *testing.T) { + define.Require(t, define.Requirements{ + Group: Default, + // We require sudo for this test to run + // `elastic-agent install` (even though it will + // be installed as non-root). + Sudo: true, - t.Run("check agent package version", testAgentPackageVersion(ctx, fixture, true)) - t.Run("check second agent installs with --develop", testSecondAgentCanInstall(ctx, fixture, "", true, opts)) + // It's not safe to run this test locally as it + // installs Elastic Agent. + Local: false, + OS: []define.OS{ + { + Type: define.Darwin, + }, { + Type: define.Linux, + }, + }, + }) - // Make sure uninstall from within the topPath fails on Windows - if runtime.GOOS == "windows" { - cwd, err := os.Getwd() - require.NoErrorf(t, err, "GetWd failed: %s", err) - err = os.Chdir(topPath) - require.NoErrorf(t, err, "Chdir to topPath failed: %s", err) - t.Cleanup(func() { - _ = os.Chdir(cwd) - }) - out, err = fixture.Uninstall(ctx, &atesting.UninstallOpts{Force: true}) - require.Error(t, err, "uninstall should have failed") - require.Containsf(t, string(out), "uninstall must be run from outside the installed path", "expected error string not found in: %s err: %s", out, err) - } + // Get path to Elastic Agent executable + fixture, err := define.NewFixtureFromLocalBuild(t, define.Version()) + require.NoError(t, err) + + ctx, cancel := testcontext.WithDeadline(t, context.Background(), time.Now().Add(10*time.Minute)) + defer cancel() + + // Prepare the Elastic Agent so the binary is extracted and ready to use. + err = fixture.Prepare(ctx) + require.NoError(t, err) + + testInstallWithoutBasePathWithCustomUser(ctx, t, fixture, "tester", "testing") } func TestInstallWithBasePath(t *testing.T) { @@ -250,6 +258,50 @@ func TestInstallPrivilegedWithBasePath(t *testing.T) { t.Run("check second agent installs with --develop", testSecondAgentCanInstall(ctx, fixture, randomBasePath, true, opts)) } +func testInstallWithoutBasePathWithCustomUser(ctx context.Context, t *testing.T, fixture *atesting.Fixture, customUsername, customGroup string) { + // Run `elastic-agent install`. We use `--force` to prevent interactive + // execution. + // create testing user + if customUsername != "" { + pt := progressbar.NewOptions(-1) + _, err := install.EnsureUserAndGroup(customUsername, customGroup, pt, true) + require.NoError(t, err) + } + + opts := atesting.InstallOpts{Force: true, Privileged: false, Username: customUsername, Group: customGroup} + out, err := fixture.Install(ctx, &opts) + if err != nil { + t.Logf("install output: %s", out) + require.NoError(t, err) + } + + // Check that Agent was installed in default base path + topPath := installtest.DefaultTopPath() + checks := &installtest.CheckOpts{ + Privileged: opts.Privileged, + Username: customUsername, + Group: customGroup, + } + require.NoError(t, installtest.CheckSuccess(ctx, fixture, topPath, checks)) + + t.Run("check agent package version", testAgentPackageVersion(ctx, fixture, true)) + t.Run("check second agent installs with --develop", testSecondAgentCanInstall(ctx, fixture, "", true, opts)) + + // Make sure uninstall from within the topPath fails on Windows + if runtime.GOOS == "windows" { + cwd, err := os.Getwd() + require.NoErrorf(t, err, "GetWd failed: %s", err) + err = os.Chdir(topPath) + require.NoErrorf(t, err, "Chdir to topPath failed: %s", err) + t.Cleanup(func() { + _ = os.Chdir(cwd) + }) + out, err = fixture.Uninstall(ctx, &atesting.UninstallOpts{Force: true}) + require.Error(t, err, "uninstall should have failed") + require.Containsf(t, string(out), "uninstall must be run from outside the installed path", "expected error string not found in: %s err: %s", out, err) + } +} + // Tests that a second agent can be installed in an isolated namespace, using either --develop or --namespace. func testSecondAgentCanInstall(ctx context.Context, fixture *atesting.Fixture, basePath string, develop bool, installOpts atesting.InstallOpts) func(*testing.T) { return func(t *testing.T) { @@ -282,6 +334,8 @@ func testSecondAgentCanInstall(ctx context.Context, fixture *atesting.Fixture, b require.NoError(t, installtest.CheckSuccess(ctx, fixture, topPath, &installtest.CheckOpts{ Privileged: installOpts.Privileged, Namespace: installOpts.Namespace, + Username: installOpts.Username, + Group: installOpts.Group, })) } } diff --git a/testing/integration/switch_unprivileged_test.go b/testing/integration/switch_unprivileged_test.go index b13a459bb36..0d95e78e7e1 100644 --- a/testing/integration/switch_unprivileged_test.go +++ b/testing/integration/switch_unprivileged_test.go @@ -14,6 +14,9 @@ import ( "testing" "time" + "github.com/schollz/progressbar/v3" + + "github.com/elastic/elastic-agent/internal/pkg/agent/install" atesting "github.com/elastic/elastic-agent/pkg/testing" "github.com/elastic/elastic-agent/pkg/testing/define" "github.com/elastic/elastic-agent/pkg/testing/tools/testcontext" @@ -23,6 +26,7 @@ import ( ) func TestSwitchUnprivilegedWithoutBasePath(t *testing.T) { + define.Require(t, define.Requirements{ Group: Default, // We require sudo for this test to run @@ -32,6 +36,13 @@ func TestSwitchUnprivilegedWithoutBasePath(t *testing.T) { // It's not safe to run this test locally as it // installs Elastic Agent. Local: false, + OS: []define.OS{ + { + Type: define.Darwin, + }, { + Type: define.Linux, + }, + }, }) // Get path to Elastic Agent executable @@ -44,7 +55,42 @@ func TestSwitchUnprivilegedWithoutBasePath(t *testing.T) { // Prepare the Elastic Agent so the binary is extracted and ready to use. err = fixture.Prepare(ctx) require.NoError(t, err) + testSwitchUnprivilegedWithoutBasePathCustomUser(ctx, t, fixture, "", "") +} + +func TestSwitchUnprivilegedWithoutBasePathCustomUser(t *testing.T) { + define.Require(t, define.Requirements{ + Group: Default, + // We require sudo for this test to run + // `elastic-agent install`. + Sudo: true, + + // It's not safe to run this test locally as it + // installs Elastic Agent. + Local: false, + OS: []define.OS{ + { + Type: define.Darwin, + }, { + Type: define.Linux, + }, + }, + }) + + // Get path to Elastic Agent executable + fixture, err := define.NewFixtureFromLocalBuild(t, define.Version()) + require.NoError(t, err) + ctx, cancel := testcontext.WithDeadline(t, context.Background(), time.Now().Add(10*time.Minute)) + defer cancel() + + // Prepare the Elastic Agent so the binary is extracted and ready to use. + err = fixture.Prepare(ctx) + require.NoError(t, err) + testSwitchUnprivilegedWithoutBasePathCustomUser(ctx, t, fixture, "tester", "testing") +} + +func testSwitchUnprivilegedWithoutBasePathCustomUser(ctx context.Context, t *testing.T, fixture *atesting.Fixture, customUsername, customGroup string) { // Run `elastic-agent install`. We use `--force` to prevent interactive // execution. opts := &atesting.InstallOpts{Force: true, Privileged: true} @@ -54,18 +100,39 @@ func TestSwitchUnprivilegedWithoutBasePath(t *testing.T) { require.NoError(t, err) } + // setup user + if customUsername != "" { + pt := progressbar.NewOptions(-1) + _, err = install.EnsureUserAndGroup(customUsername, customGroup, pt, true) + require.NoError(t, err) + } + // Check that Agent was installed in default base path in privileged mode require.NoError(t, installtest.CheckSuccess(ctx, fixture, opts.BasePath, &installtest.CheckOpts{Privileged: true})) // Switch to unprivileged mode - out, err = fixture.Exec(ctx, []string{"unprivileged", "-f"}) + args := []string{"unprivileged", "-f"} + if customUsername != "" { + args = append(args, "--user", customUsername) + } + + if customGroup != "" { + args = append(args, "--group", customGroup) + } + + out, err = fixture.Exec(ctx, args) if err != nil { t.Logf("unprivileged output: %s", out) require.NoError(t, err) } // Check that Agent is running in default base path in unprivileged mode - require.NoError(t, installtest.CheckSuccess(ctx, fixture, opts.BasePath, &installtest.CheckOpts{Privileged: false})) + checks := &installtest.CheckOpts{ + Privileged: false, + Username: customUsername, + Group: customGroup, + } + require.NoError(t, installtest.CheckSuccess(ctx, fixture, opts.BasePath, checks)) } func TestSwitchUnprivilegedWithBasePath(t *testing.T) { From be28ba115c244e8d39395a66c968f2d23170c0d4 Mon Sep 17 00:00:00 2001 From: Michal Pristas Date: Wed, 4 Dec 2024 10:02:21 +0100 Subject: [PATCH 2/2] lint, remove G115 directives --- internal/pkg/agent/install/user_windows.go | 2 +- testing/installtest/checks_unix.go | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/pkg/agent/install/user_windows.go b/internal/pkg/agent/install/user_windows.go index 9b63d96ba8c..3627ba799f3 100644 --- a/internal/pkg/agent/install/user_windows.go +++ b/internal/pkg/agent/install/user_windows.go @@ -188,7 +188,7 @@ func AddUserToGroup(username string, groupName string) error { uintptr(unsafe.Pointer(groupNamePtr)), uintptr(uint32(0)), uintptr(unsafe.Pointer(&entries[0])), - uintptr(uint32(len(entries))), //nolint:gosec // G115 max 1 + uintptr(uint32(len(entries))), ) if ret != 0 { return fmt.Errorf("call to NetLocalGroupAddMembers failed: status=%d", ret) diff --git a/testing/installtest/checks_unix.go b/testing/installtest/checks_unix.go index b8030f0bf6a..95100d792b6 100644 --- a/testing/installtest/checks_unix.go +++ b/testing/installtest/checks_unix.go @@ -47,14 +47,12 @@ func checkPlatform(ctx context.Context, _ *atesting.Fixture, topPath string, opt if uid > math.MaxUint32 { return fmt.Errorf("provided UID %d does is higher than %d", uid, math.MaxInt32) } - //nolint:gosec // G115 false positive on conversion uid32 = uint32(uid) var gid32 uint32 if gid > math.MaxUint32 { return fmt.Errorf("provided GID %d does is higher than %d", gid, math.MaxInt32) } - //nolint:gosec // G115 false positive on conversion gid32 = uint32(gid) // Ensure entire installation tree has the correct permissions.