From 587031f14d1a828a08874c9bdc37946aabbf9338 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 4 Nov 2015 17:20:52 -0800 Subject: [PATCH] Get rid of exec.cmd struct and setuid file --- client/driver/executor/exec.go | 16 +----- client/driver/executor/exec_basic.go | 14 ++--- client/driver/executor/exec_linux.go | 68 +++++++++++++----------- client/driver/executor/setuid.go | 41 -------------- client/driver/executor/setuid_windows.go | 13 ----- 5 files changed, 44 insertions(+), 108 deletions(-) delete mode 100644 client/driver/executor/setuid.go delete mode 100644 client/driver/executor/setuid_windows.go diff --git a/client/driver/executor/exec.go b/client/driver/executor/exec.go index ca104ca3e..b3878ba0c 100644 --- a/client/driver/executor/exec.go +++ b/client/driver/executor/exec.go @@ -70,7 +70,7 @@ type Executor interface { // Command provides access the underlying Cmd struct in case the Executor // interface doesn't expose the functionality you need. - Command() *cmd + Command() *exec.Cmd } // Command is a mirror of exec.Command that returns a platform-specific Executor @@ -100,17 +100,3 @@ func OpenId(id string) (Executor, error) { } return executor, nil } - -// Cmd is an extension of exec.Cmd that incorporates functionality for -// re-attaching to processes, dropping priviledges, etc., based on platform- -// specific implementations. -type cmd struct { - exec.Cmd - - // Resources is used to limit CPU and RAM used by the process, by way of - // cgroups or a similar mechanism. - Resources structs.Resources - - // RunAs may be a username or Uid. The implementation will decide how to use it. - RunAs string -} diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index 81f17d414..b9bdebd7d 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -3,6 +3,7 @@ package executor import ( "fmt" "os" + "os/exec" "strconv" "strings" @@ -15,10 +16,9 @@ import ( // BasicExecutor should work everywhere, and as a result does not include // any resource restrictions or runas capabilities. type BasicExecutor struct { - cmd + cmd exec.Cmd } -// TODO: Update to use the Spawner. // TODO: Have raw_exec use this as well. func NewBasicExecutor() Executor { return &BasicExecutor{} @@ -36,7 +36,7 @@ func (e *BasicExecutor) ConfigureTaskDir(taskName string, alloc *allocdir.AllocD if !ok { return fmt.Errorf("Error finding task dir for (%s)", taskName) } - e.Dir = taskDir + e.cmd.Dir = taskDir return nil } @@ -61,7 +61,7 @@ func (e *BasicExecutor) Start() error { if err != nil { return err } - e.Cmd.Args = parsed + e.cmd.Args = parsed // We don't want to call ourself. We want to call Start on our embedded Cmd return e.cmd.Start() @@ -77,7 +77,7 @@ func (e *BasicExecutor) Open(pid string) error { if err != nil { return fmt.Errorf("Failed to reopen pid %d: %v", pidNum, err) } - e.Process = process + e.cmd.Process = process return nil } @@ -99,9 +99,9 @@ func (e *BasicExecutor) Shutdown() error { } func (e *BasicExecutor) ForceStop() error { - return e.Process.Kill() + return e.cmd.Process.Kill() } -func (e *BasicExecutor) Command() *cmd { +func (e *BasicExecutor) Command() *exec.Cmd { return &e.cmd } diff --git a/client/driver/executor/exec_linux.go b/client/driver/executor/exec_linux.go index 1b4b312bf..a75440ba8 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -6,8 +6,10 @@ import ( "errors" "fmt" "os" + "os/exec" "os/user" "path/filepath" + "strconv" "strings" "syscall" @@ -44,7 +46,7 @@ func NewExecutor() Executor { // Linux executor is designed to run on linux kernel 2.8+. type LinuxExecutor struct { - cmd + cmd exec.Cmd user *user.User // Isolation configurations. @@ -57,7 +59,7 @@ type LinuxExecutor struct { spawn *spawn.Spawner } -func (e *LinuxExecutor) Command() *cmd { +func (e *LinuxExecutor) Command() *exec.Cmd { return &e.cmd } @@ -114,45 +116,47 @@ func (e *LinuxExecutor) ID() (string, error) { return buffer.String(), nil } -// runAs takes a user id as a string and looks up the user. It stores the -// results in the executor and returns an error if the user could not be found. +// runAs takes a user id as a string and looks up the user, and sets the command +// to execute as that user. func (e *LinuxExecutor) runAs(userid string) error { - errs := new(multierror.Error) - - // First, try to lookup the user by uid - u, err := user.LookupId(userid) - if err == nil { - e.user = u - return nil - } else { - errs = multierror.Append(errs, err) + u, err := user.Lookup(userid) + if err != nil { + return fmt.Errorf("Failed to identify user %v: %v", userid, err) } - // Lookup failed, so try by username instead - u, err = user.Lookup(userid) - if err == nil { - e.user = u - return nil - } else { - errs = multierror.Append(errs, err) + // Convert the uid and gid + uid, err := strconv.ParseUint(u.Uid, 10, 32) + if err != nil { + return fmt.Errorf("Unable to convert userid to uint32: %s", err) + } + gid, err := strconv.ParseUint(u.Gid, 10, 32) + if err != nil { + return fmt.Errorf("Unable to convert groupid to uint32: %s", err) } - // If we got here we failed to lookup based on id and username, so we'll - // return those errors. - return fmt.Errorf("Failed to identify user to run as: %s", errs) + // Set the command to run as that user and group. + if e.cmd.SysProcAttr == nil { + e.cmd.SysProcAttr = &syscall.SysProcAttr{} + } + if e.cmd.SysProcAttr.Credential == nil { + e.cmd.SysProcAttr.Credential = &syscall.Credential{} + } + e.cmd.SysProcAttr.Credential.Uid = uint32(uid) + e.cmd.SysProcAttr.Credential.Gid = uint32(gid) + + return nil } func (e *LinuxExecutor) Start() error { // Run as "nobody" user so we don't leak root privilege to the spawned // process. - if err := e.runAs("nobody"); err == nil && e.user != nil { - e.cmd.SetUID(e.user.Uid) - e.cmd.SetGID(e.user.Gid) + if err := e.runAs("nobody"); err != nil { + return err } // Parse the commands arguments and replace instances of Nomad environment // variables. - envVars, err := environment.ParseFromList(e.Cmd.Env) + envVars, err := environment.ParseFromList(e.cmd.Env) if err != nil { return err } @@ -165,16 +169,16 @@ func (e *LinuxExecutor) Start() error { } e.cmd.Path = parsedPath[0] - combined := strings.Join(e.Cmd.Args, " ") + combined := strings.Join(e.cmd.Args, " ") parsed, err := args.ParseAndReplace(combined, envVars.Map()) if err != nil { return err } - e.Cmd.Args = parsed + e.cmd.Args = parsed spawnState := filepath.Join(e.allocDir, fmt.Sprintf("%s_%s", e.taskName, "exit_status")) e.spawn = spawn.NewSpawner(spawnState) - e.spawn.SetCommand(&e.cmd.Cmd) + e.spawn.SetCommand(&e.cmd) e.spawn.SetChroot(e.taskDir) e.spawn.SetLogs(&spawn.Logs{ Stdout: filepath.Join(e.taskDir, allocdir.TaskLocal, fmt.Sprintf("%v.stdout", e.taskName)), @@ -283,13 +287,13 @@ func (e *LinuxExecutor) ConfigureTaskDir(taskName string, alloc *allocdir.AllocD } // Set the tasks AllocDir environment variable. - env, err := environment.ParseFromList(e.Cmd.Env) + env, err := environment.ParseFromList(e.cmd.Env) if err != nil { return err } env.SetAllocDir(filepath.Join("/", allocdir.SharedAllocName)) env.SetTaskLocalDir(filepath.Join("/", allocdir.TaskLocal)) - e.Cmd.Env = env.List() + e.cmd.Env = env.List() return nil } diff --git a/client/driver/executor/setuid.go b/client/driver/executor/setuid.go deleted file mode 100644 index 4793f8e2c..000000000 --- a/client/driver/executor/setuid.go +++ /dev/null @@ -1,41 +0,0 @@ -// +build !windows - -package executor - -import ( - "fmt" - "strconv" - "syscall" -) - -// SetUID changes the Uid for this command (must be set before starting) -func (c *cmd) SetUID(userid string) error { - uid, err := strconv.ParseUint(userid, 10, 32) - if err != nil { - return fmt.Errorf("Unable to convert userid to uint32: %s", err) - } - if c.SysProcAttr == nil { - c.SysProcAttr = &syscall.SysProcAttr{} - } - if c.SysProcAttr.Credential == nil { - c.SysProcAttr.Credential = &syscall.Credential{} - } - c.SysProcAttr.Credential.Uid = uint32(uid) - return nil -} - -// SetGID changes the Gid for this command (must be set before starting) -func (c *cmd) SetGID(groupid string) error { - gid, err := strconv.ParseUint(groupid, 10, 32) - if err != nil { - return fmt.Errorf("Unable to convert groupid to uint32: %s", err) - } - if c.SysProcAttr == nil { - c.SysProcAttr = &syscall.SysProcAttr{} - } - if c.SysProcAttr.Credential == nil { - c.SysProcAttr.Credential = &syscall.Credential{} - } - c.SysProcAttr.Credential.Gid = uint32(gid) - return nil -} diff --git a/client/driver/executor/setuid_windows.go b/client/driver/executor/setuid_windows.go deleted file mode 100644 index 9c18bed53..000000000 --- a/client/driver/executor/setuid_windows.go +++ /dev/null @@ -1,13 +0,0 @@ -package executor - -// SetUID changes the Uid for this command (must be set before starting) -func (c *cmd) SetUID(userid string) error { - // TODO implement something for windows - return nil -} - -// SetGID changes the Gid for this command (must be set before starting) -func (c *cmd) SetGID(groupid string) error { - // TODO implement something for windows - return nil -}