Skip to content

Commit

Permalink
Merge branch 'main' into cleanup/fleet_gateway
Browse files Browse the repository at this point in the history
  • Loading branch information
aleksmaus authored Jul 23, 2024
2 parents 29ad891 + 2ff1cd6 commit cda113c
Show file tree
Hide file tree
Showing 14 changed files with 124 additions and 31 deletions.
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: Fix issue with Windows install when user has no home directory

# 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:

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

# 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/5019
5 changes: 4 additions & 1 deletion internal/pkg/agent/cmd/enroll_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ func getFileOwnerFromCmd(cmd *cobra.Command) (utils.FileOwner, error) {
if err != nil {
return utils.FileOwner{}, err
}
ownership := utils.CurrentFileOwner()
ownership, err := utils.CurrentFileOwner()
if err != nil {
return utils.FileOwner{}, err
}
if uid != -1 {
ownership.UID = uid
}
Expand Down
6 changes: 5 additions & 1 deletion internal/pkg/agent/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,11 @@ func ensureInstallMarkerPresent() error {
// Otherwise, we're being upgraded from a version of an installed Agent
// that didn't use an installation marker file (that is, before v8.8.0).
// So create the file now.
if err := install.CreateInstallMarker(paths.Top(), utils.CurrentFileOwner()); err != nil {
ownership, err := utils.CurrentFileOwner()
if err != nil {
return fmt.Errorf("failed to get current file owner: %w", err)
}
if err := install.CreateInstallMarker(paths.Top(), ownership); err != nil {
return fmt.Errorf("unable to create installation marker file during upgrade: %w", err)
}

Expand Down
4 changes: 3 additions & 1 deletion internal/pkg/agent/install/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,9 @@ func TestCopyFiles(t *testing.T) {

func TestSetupInstallPath(t *testing.T) {
tmpdir := t.TempDir()
err := setupInstallPath(tmpdir, utils.CurrentFileOwner())
ownership, err := utils.CurrentFileOwner()
require.NoError(t, err)
err = setupInstallPath(tmpdir, ownership)
require.NoError(t, err)
require.FileExists(t, filepath.Join(tmpdir, paths.MarkerFileName))
}
5 changes: 4 additions & 1 deletion internal/pkg/agent/install/uninstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ func EnsureStoppedService(topPath string, pt *progressbar.ProgressBar) (service.
func checkForUnprivilegedVault(ctx context.Context, opts ...vault.OptionFunc) (bool, error) {
// check if we have a file vault to detect if we have to use it for reading config
opts = append(opts, vault.WithReadonly(true))
vaultOpts := vault.ApplyOptions(opts...)
vaultOpts, err := vault.ApplyOptions(opts...)
if err != nil {
return false, err
}
fileVault, fileVaultErr := vault.NewFileVault(ctx, vaultOpts)
if fileVaultErr == nil {
ok, keyErr := fileVault.Exists(ctx, secret.AgentSecretKey)
Expand Down
4 changes: 3 additions & 1 deletion internal/pkg/agent/install/uninstall_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ func Test_checkForUnprivilegedVault(t *testing.T) {
}

func initFileVault(t *testing.T, ctx context.Context, testVaultPath string, keys map[string][]byte) {
newFileVault, err := vault.NewFileVault(ctx, vault.ApplyOptions(vault.WithVaultPath(testVaultPath)))
opts, err := vault.ApplyOptions(vault.WithVaultPath(testVaultPath))
require.NoError(t, err)
newFileVault, err := vault.NewFileVault(ctx, opts)
require.NoError(t, err, "setting up test file vault store")
defer func(newFileVault *vault.FileVault) {
err := newFileVault.Close()
Expand Down
11 changes: 8 additions & 3 deletions internal/pkg/agent/perms/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package perms

import (
"fmt"
"os"

"github.com/elastic/elastic-agent/pkg/utils"
Expand Down Expand Up @@ -35,13 +36,17 @@ func WithOwnership(ownership utils.FileOwner) OptFunc {
}
}

func newOpts(optFuncs ...OptFunc) *opts {
func newOpts(optFuncs ...OptFunc) (*opts, error) {
ownership, err := utils.CurrentFileOwner()
if err != nil {
return nil, fmt.Errorf("failed to get current file owner: %w", err)
}
o := &opts{
mask: defaultMask,
ownership: utils.CurrentFileOwner(),
ownership: ownership,
}
for _, f := range optFuncs {
f(o)
}
return o
return o, nil
}
5 changes: 4 additions & 1 deletion internal/pkg/agent/perms/unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ import (

// FixPermissions fixes the permissions so only root:root is the owner and no world read-able permissions
func FixPermissions(topPath string, opts ...OptFunc) error {
o := newOpts(opts...)
o, err := newOpts(opts...)
if err != nil {
return err
}
return filepath.Walk(topPath, func(name string, info fs.FileInfo, err error) error {
if errors.Is(err, fs.ErrNotExist) {
return nil
Expand Down
5 changes: 4 additions & 1 deletion internal/pkg/agent/perms/windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import (

// FixPermissions fixes the permissions so only SYSTEM and Administrators have access to the files in the install path
func FixPermissions(topPath string, opts ...OptFunc) error {
o := newOpts(opts...)
o, err := newOpts(opts...)
if err != nil {
return err
}

// SYSTEM and Administrators always get permissions
// https://support.microsoft.com/en-us/help/243330/well-known-security-identifiers-in-windows-operating-systems
Expand Down
5 changes: 4 additions & 1 deletion internal/pkg/agent/vault/vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ type Vault interface {
}

func New(ctx context.Context, opts ...OptionFunc) (Vault, error) {
options := ApplyOptions(opts...)
options, err := ApplyOptions(opts...)
if err != nil {
return nil, err
}

if runtime.GOOS == "darwin" && !options.unprivileged {
return NewDarwinKeyChainVault(ctx, options)
Expand Down
15 changes: 12 additions & 3 deletions internal/pkg/agent/vault/vault_file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ func TestFileVaultRekey(t *testing.T) {
defer cn()

vaultPath := getTestFileVaultPath(t)
options := ApplyOptions(WithVaultPath(vaultPath))
options, err := ApplyOptions(WithVaultPath(vaultPath))
if err != nil {
t.Fatal(err)
}
v, err := NewFileVault(ctx, options)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -83,7 +86,10 @@ func TestFileVault(t *testing.T) {

ctx, cn := context.WithCancel(context.Background())
defer cn()
options := ApplyOptions(WithVaultPath(vaultPath))
options, err := ApplyOptions(WithVaultPath(vaultPath))
if err != nil {
t.Fatal(err)
}
v, err := NewFileVault(ctx, options)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -209,7 +215,10 @@ func TestFileVaultConcurrent(t *testing.T) {
}

func doCrud(t *testing.T, ctx context.Context, vaultPath, key string) error {
options := ApplyOptions(WithVaultPath(vaultPath))
options, err := ApplyOptions(WithVaultPath(vaultPath))
if err != nil {
return err
}
v, err := NewFileVault(ctx, options)
if err != nil {
return fmt.Errorf("could not create new vault: %w", err)
Expand Down
11 changes: 8 additions & 3 deletions internal/pkg/agent/vault/vault_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package vault

import (
"fmt"
"time"

"github.com/elastic/elastic-agent/internal/pkg/agent/application/paths"
Expand Down Expand Up @@ -70,7 +71,11 @@ func WithUnprivileged(unprivileged bool) OptionFunc {
}

// ApplyOptions applies options for Windows, Linux and Mac, not all the options may be used
func ApplyOptions(opts ...OptionFunc) Options {
func ApplyOptions(opts ...OptionFunc) (Options, error) {
ownership, err := utils.CurrentFileOwner()
if err != nil {
return Options{}, fmt.Errorf("failed to get current file owner: %w", err)
}
o := Options{
CommonVaultOptions: CommonVaultOptions{
readonly: false,
Expand All @@ -79,7 +84,7 @@ func ApplyOptions(opts ...OptionFunc) Options {
FileVaultOptions: FileVaultOptions{
vaultPath: paths.AgentVaultPath(),
lockRetryDelay: defaultFlockRetryDelay,
ownership: utils.CurrentFileOwner(),
ownership: ownership,
},
KeychainVaultOptions: KeychainVaultOptions{
entryName: paths.AgentKeychainName(),
Expand All @@ -89,5 +94,5 @@ func ApplyOptions(opts ...OptionFunc) Options {
for _, opt := range opts {
opt(&o)
}
return o
return o, nil
}
4 changes: 2 additions & 2 deletions pkg/utils/perm_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ type FileOwner struct {
}

// CurrentFileOwner returns the executing UID and GID of the current process.
func CurrentFileOwner() FileOwner {
func CurrentFileOwner() (FileOwner, error) {
return FileOwner{
UID: os.Getuid(),
GID: os.Getgid(),
}
}, nil
}

// HasStrictExecPerms ensures that the path is executable by the owner, cannot be written by anyone other than the
Expand Down
43 changes: 31 additions & 12 deletions pkg/utils/perm_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
package utils

import (
"os/user"
"fmt"
"syscall"
)

const (
Expand All @@ -26,20 +27,38 @@ type FileOwner struct {
}

// CurrentFileOwner returns the executing UID and GID of the current process.
//
// Note: Very unlikely for this to panic if this function is unable to get the current
// user. Not being able to get the current user, is a critical problem and nothing
// can continue so a panic is appropriate.
func CurrentFileOwner() FileOwner {
u, err := user.Current()
func CurrentFileOwner() (FileOwner, error) {
// os/user.Current() is not used here, because it tries to access the users home
// directory. It is possible during installation that the users home directory
// is not created yet. See issue https://github.com/elastic/elastic-agent/issues/5019
// for more information.
t, err := syscall.OpenCurrentProcessToken()
if err != nil {
// should not fail; if it does then there is a big problem
panic(err)
return FileOwner{}, fmt.Errorf("failed to open current process token: %w", err)
}
return FileOwner{
UID: u.Uid,
GID: u.Gid,
defer func() {
_ = t.Close()
}()
u, err := t.GetTokenUser()
if err != nil {
return FileOwner{}, fmt.Errorf("failed to get token user: %w", err)
}
pg, err := t.GetTokenPrimaryGroup()
if err != nil {
return FileOwner{}, fmt.Errorf("failed to get token primary group: %w", err)
}
uid, err := u.User.Sid.String()
if err != nil {
return FileOwner{}, fmt.Errorf("failed to convert token user sid to string: %w", err)
}
gid, err := pg.PrimaryGroup.String()
if err != nil {
return FileOwner{}, fmt.Errorf("failed to convert token primary group sid to string: %w", err)
}
return FileOwner{
UID: uid,
GID: gid,
}, nil
}

// HasStrictExecPerms ensures that the path is executable by the owner and that the owner of the file
Expand Down

0 comments on commit cda113c

Please sign in to comment.