From f541f2e59b00f645218c824c798a9437653eefa3 Mon Sep 17 00:00:00 2001 From: the-nando <89834752+the-nando@users.noreply.github.com> Date: Wed, 5 Apr 2023 18:47:51 +0200 Subject: [PATCH] Do not set attributes when spawning the getter child (#16791) * Do not set attributes when spawning the getter child * Cleanup * Cleanup --------- Co-authored-by: the-nando --- .changelog/16791.txt | 3 ++ .../allocrunner/taskrunner/getter/testing.go | 7 +--- client/allocrunner/taskrunner/getter/util.go | 1 - .../taskrunner/getter/util_default.go | 11 ------- .../taskrunner/getter/util_linux.go | 33 ------------------- .../taskrunner/getter/util_windows.go | 11 ------- 6 files changed, 4 insertions(+), 62 deletions(-) create mode 100644 .changelog/16791.txt diff --git a/.changelog/16791.txt b/.changelog/16791.txt new file mode 100644 index 000000000..66d503a59 --- /dev/null +++ b/.changelog/16791.txt @@ -0,0 +1,3 @@ +```release-note:bug +client: Remove setting attributes when spawning the getter child +``` diff --git a/client/allocrunner/taskrunner/getter/testing.go b/client/allocrunner/taskrunner/getter/testing.go index e44c61559..4daf4d056 100644 --- a/client/allocrunner/taskrunner/getter/testing.go +++ b/client/allocrunner/taskrunner/getter/testing.go @@ -24,24 +24,19 @@ func TestSandbox(t *testing.T) *Sandbox { } // SetupDir creates a directory suitable for testing artifact - i.e. it is -// owned by the nobody user as would be the case in a normal client operation. +// owned by the user under which nomad runs. // // returns alloc_dir, task_dir func SetupDir(t *testing.T) (string, string) { - uid, gid := credentials() - allocDir := t.TempDir() taskDir := filepath.Join(allocDir, "local") topDir := filepath.Dir(allocDir) - must.NoError(t, os.Chown(topDir, int(uid), int(gid))) must.NoError(t, os.Chmod(topDir, 0o755)) - must.NoError(t, os.Chown(allocDir, int(uid), int(gid))) must.NoError(t, os.Chmod(allocDir, 0o755)) must.NoError(t, os.Mkdir(taskDir, 0o755)) - must.NoError(t, os.Chown(taskDir, int(uid), int(gid))) must.NoError(t, os.Chmod(taskDir, 0o755)) return allocDir, taskDir } diff --git a/client/allocrunner/taskrunner/getter/util.go b/client/allocrunner/taskrunner/getter/util.go index 1a71017f3..6422c95c2 100644 --- a/client/allocrunner/taskrunner/getter/util.go +++ b/client/allocrunner/taskrunner/getter/util.go @@ -137,7 +137,6 @@ func (s *Sandbox) runCmd(env *parameters) error { cmd.Stdin = env.reader() cmd.Stdout = output cmd.Stderr = output - cmd.SysProcAttr = attributes() // start & wait for the subprocess to terminate if err := cmd.Run(); err != nil { diff --git a/client/allocrunner/taskrunner/getter/util_default.go b/client/allocrunner/taskrunner/getter/util_default.go index c32effcad..be1facdb4 100644 --- a/client/allocrunner/taskrunner/getter/util_default.go +++ b/client/allocrunner/taskrunner/getter/util_default.go @@ -4,19 +4,8 @@ package getter import ( "path/filepath" - "syscall" ) -// attributes is not implemented by default -func attributes() *syscall.SysProcAttr { - return nil -} - -// credentials is not implemented by default -func credentials() (uint32, uint32) { - return 0, 0 -} - // lockdown is not implemented by default func lockdown(string, string) error { return nil diff --git a/client/allocrunner/taskrunner/getter/util_linux.go b/client/allocrunner/taskrunner/getter/util_linux.go index e2c52146c..c034a08ac 100644 --- a/client/allocrunner/taskrunner/getter/util_linux.go +++ b/client/allocrunner/taskrunner/getter/util_linux.go @@ -5,45 +5,12 @@ package getter import ( "os" "path/filepath" - "syscall" "github.com/mitchellh/go-homedir" "github.com/shoenig/go-landlock" "golang.org/x/sys/unix" ) -var ( - // userUID is the current user's uid - userUID uint32 - - // userGID is the current user's gid - userGID uint32 -) - -func init() { - userUID = uint32(syscall.Getuid()) - userGID = uint32(syscall.Getgid()) -} - -// attributes returns the system process attributes to run -// the sandbox process with -func attributes() *syscall.SysProcAttr { - uid, gid := credentials() - return &syscall.SysProcAttr{ - Credential: &syscall.Credential{ - Uid: uid, - Gid: gid, - }, - } -} - -// credentials returns the UID and GID of the user the child process -// will run as - for now this is always the same user the Nomad agent is -// running as. -func credentials() (uint32, uint32) { - return userUID, userGID -} - // findHomeDir returns the home directory as provided by os.UserHomeDir. In case // os.UserHomeDir returns an error, we return /root if the current process is being // run by root, or /dev/null otherwise. diff --git a/client/allocrunner/taskrunner/getter/util_windows.go b/client/allocrunner/taskrunner/getter/util_windows.go index b78cc9117..f22b58e91 100644 --- a/client/allocrunner/taskrunner/getter/util_windows.go +++ b/client/allocrunner/taskrunner/getter/util_windows.go @@ -5,19 +5,8 @@ package getter import ( "os" "path/filepath" - "syscall" ) -// attributes is not implemented on Windows -func attributes() *syscall.SysProcAttr { - return nil -} - -// credentials is not implemented on Windows -func credentials() (uint32, uint32) { - return 0, 0 -} - // lockdown is not implemented on Windows func lockdown(string, string) error { return nil