From a05627156a46c83886b6f8c654d373f8d1646bc6 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 24 Nov 2020 13:35:11 -0500 Subject: [PATCH 1/4] add a test for testing user --- drivers/rawexec/driver_unix_test.go | 39 +++++++++++++++++++++++ plugins/drivers/testutils/exec_testing.go | 5 +++ 2 files changed, 44 insertions(+) diff --git a/drivers/rawexec/driver_unix_test.go b/drivers/rawexec/driver_unix_test.go index 5b72a274a87..b35902297fa 100644 --- a/drivers/rawexec/driver_unix_test.go +++ b/drivers/rawexec/driver_unix_test.go @@ -339,6 +339,45 @@ func TestRawExec_ExecTaskStreaming(t *testing.T) { } +func TestRawExec_ExecTaskStreaming_User(t *testing.T) { + t.Parallel() + if runtime.GOOS != "linux" { + t.Skip("skip, requires running on Linux for testing custom user setting") + } + + d := newEnabledRawExecDriver(t) + harness := dtestutil.NewDriverHarness(t, d) + defer harness.Kill() + + task := &drivers.TaskConfig{ + ID: uuid.Generate(), + Name: "sleep", + User: "nobody", + } + + cleanup := harness.MkAllocDir(task, false) + defer cleanup() + + err := os.Chmod(task.AllocDir, 0777) + require.NoError(t, err) + + tc := &TaskConfig{ + Command: "/bin/sleep", + Args: []string{"9000"}, + } + require.NoError(t, task.EncodeConcreteDriverConfig(&tc)) + testtask.SetTaskConfigEnv(task) + + _, _, err = harness.StartTask(task) + require.NoError(t, err) + defer d.DestroyTask(task.ID, true) + + code, stdout, stderr := dtestutil.ExecTask(t, harness, task.ID, "whoami", false, "") + require.Zero(t, code) + require.Empty(t, stderr) + require.Contains(t, stdout, "nobody") +} + func TestRawExecDriver_NoCgroup(t *testing.T) { t.Parallel() if runtime.GOOS != "linux" { diff --git a/plugins/drivers/testutils/exec_testing.go b/plugins/drivers/testutils/exec_testing.go index 7a79025704c..4be1f28904b 100644 --- a/plugins/drivers/testutils/exec_testing.go +++ b/plugins/drivers/testutils/exec_testing.go @@ -208,6 +208,11 @@ func TestExecFSIsolation(t *testing.T, driver *DriverHarness, taskID string) { }) } +func ExecTask(t *testing.T, driver *DriverHarness, taskID string, cmd string, tty bool, stdin string) (exitCode int, stdout, stderr string) { + r := execTask(t, driver, taskID, cmd, tty, stdin) + return r.exitCode, r.stdout, r.stderr +} + func execTask(t *testing.T, driver *DriverHarness, taskID string, cmd string, tty bool, stdin string) execResult { stream := newTestExecStream(t, tty, stdin) From 53422b8ac8c468d5dbd29fe4846a91d75f733b86 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 24 Nov 2020 12:59:31 -0500 Subject: [PATCH 2/4] executor: Refactor runAs to accept cmd Pure refactor command so that we can configure command user. --- drivers/shared/executor/executor.go | 2 +- drivers/shared/executor/executor_basic.go | 6 ++++-- .../executor/executor_universal_linux.go | 21 +++++++++---------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/shared/executor/executor.go b/drivers/shared/executor/executor.go index 370a0cea13e..091ee223b45 100644 --- a/drivers/shared/executor/executor.go +++ b/drivers/shared/executor/executor.go @@ -266,7 +266,7 @@ func (e *UniversalExecutor) Launch(command *ExecCommand) (*ProcessState, error) // setting the user of the process if command.User != "" { e.logger.Debug("running command as user", "user", command.User) - if err := e.runAs(command.User); err != nil { + if err := setCmdUser(&e.childCmd, command.User); err != nil { return nil, err } } diff --git a/drivers/shared/executor/executor_basic.go b/drivers/shared/executor/executor_basic.go index b1e6f9b1399..7326ad172d3 100644 --- a/drivers/shared/executor/executor_basic.go +++ b/drivers/shared/executor/executor_basic.go @@ -3,6 +3,8 @@ package executor import ( + "os/exec" + hclog "github.com/hashicorp/go-hclog" "github.com/hashicorp/nomad/plugins/drivers" ) @@ -15,8 +17,6 @@ func NewExecutorWithIsolation(logger hclog.Logger) Executor { func (e *UniversalExecutor) configureResourceContainer(_ int) error { return nil } -func (e *UniversalExecutor) runAs(_ string) error { return nil } - func (e *UniversalExecutor) getAllPids() (map[int]*nomadPid, error) { return getAllPidsByScanning() } @@ -28,3 +28,5 @@ func (e *UniversalExecutor) start(command *ExecCommand) error { func withNetworkIsolation(f func() error, _ *drivers.NetworkIsolationSpec) error { return f() } + +func setCmdUser(*exec.Cmd, string) error { return nil } diff --git a/drivers/shared/executor/executor_universal_linux.go b/drivers/shared/executor/executor_universal_linux.go index 431e8b471e1..e52ea55b713 100644 --- a/drivers/shared/executor/executor_universal_linux.go +++ b/drivers/shared/executor/executor_universal_linux.go @@ -3,6 +3,7 @@ package executor import ( "fmt" "os" + "os/exec" "os/user" "strconv" "syscall" @@ -16,9 +17,9 @@ import ( "github.com/opencontainers/runc/libcontainer/specconv" ) -// runAs takes a user id as a string and looks up the user, and sets the command +// setCmdUser takes a user id as a string and looks up the user, and sets the command // to execute as that user. -func (e *UniversalExecutor) runAs(userid string) error { +func setCmdUser(cmd *exec.Cmd, userid string) error { u, err := user.Lookup(userid) if err != nil { return fmt.Errorf("Failed to identify user %v: %v", userid, err) @@ -51,17 +52,15 @@ func (e *UniversalExecutor) runAs(userid string) error { } // Set the command to run as that user and group. - if e.childCmd.SysProcAttr == nil { - e.childCmd.SysProcAttr = &syscall.SysProcAttr{} + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} } - if e.childCmd.SysProcAttr.Credential == nil { - e.childCmd.SysProcAttr.Credential = &syscall.Credential{} + if cmd.SysProcAttr.Credential == nil { + cmd.SysProcAttr.Credential = &syscall.Credential{} } - e.childCmd.SysProcAttr.Credential.Uid = uint32(uid) - e.childCmd.SysProcAttr.Credential.Gid = uint32(gid) - e.childCmd.SysProcAttr.Credential.Groups = gids - - e.logger.Debug("setting process user", "user", uid, "group", gid, "additional_groups", gids) + cmd.SysProcAttr.Credential.Uid = uint32(uid) + cmd.SysProcAttr.Credential.Gid = uint32(gid) + cmd.SysProcAttr.Credential.Groups = gids return nil } From 361b2c97cbdd9bc628dfee0bf1a8213993da3510 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 24 Nov 2020 13:57:54 -0500 Subject: [PATCH 3/4] executor: set user for exec --- drivers/shared/executor/executor.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/shared/executor/executor.go b/drivers/shared/executor/executor.go index 091ee223b45..c83f42163e0 100644 --- a/drivers/shared/executor/executor.go +++ b/drivers/shared/executor/executor.go @@ -408,6 +408,12 @@ func (e *UniversalExecutor) ExecStreaming(ctx context.Context, command []string, return nil }, processStart: func() error { + if u := e.commandCfg.User; u != "" { + if err := setCmdUser(cmd, u); err != nil { + return err + } + } + return withNetworkIsolation(cmd.Start, e.commandCfg.NetworkIsolation) }, processWait: func() (*os.ProcessState, error) { From f906ae6cea3b3217d667f3f9110ea8515041a140 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Wed, 25 Nov 2020 09:21:01 -0500 Subject: [PATCH 4/4] changelog update [ci skip] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 972eca65f8a..980e4379e68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ IMPROVEMENTS: * csi: Relaxed validation requirements when checking volume capabilities with controller plugins, to accommodate existing plugin behaviors. [[GH-9049](https://github.com/hashicorp/nomad/issues/9049)] * driver/docker: Upgrade pause container and detect architecture [[GH-8957](https://github.com/hashicorp/nomad/pull/8957)] * driver/docker: Support pinning tasks to specific CPUs with `cpuset_cpus` option. [[GH-8291](https://github.com/hashicorp/nomad/pull/8291)] + * driver/raw_exec: Honor the task user setting when a user runs `nomad alloc exec` [[GH-9439](https://github.com/hashicorp/nomad/pull/9439)] * jobspec: Lowered minimum CPU allowed from 10 to 1. [[GH-8996](https://github.com/hashicorp/nomad/issues/8996)] * jobspec: Added support for `headers` option in `artifact` stanza [[GH-9306](https://github.com/hashicorp/nomad/issues/9306)]