From 4538014cc3dec50e818f2551ea2efd417d513197 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Thu, 2 May 2019 13:34:41 -0400 Subject: [PATCH 01/15] executor split up lookupBin --- drivers/shared/executor/executor.go | 31 +++++++++++++++++++---------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/drivers/shared/executor/executor.go b/drivers/shared/executor/executor.go index 699d3247a..40eb3c88f 100644 --- a/drivers/shared/executor/executor.go +++ b/drivers/shared/executor/executor.go @@ -546,19 +546,12 @@ func (e *UniversalExecutor) handleStats(ch chan *cstructs.TaskResourceUsage, ctx } // lookupBin looks for path to the binary to run by looking for the binary in -// the following locations, in-order: task/local/, task/, based on host $PATH. +// the following locations, in-order: +// task/local/, task/, on the host file system, in host $PATH // The return path is absolute. func lookupBin(taskDir string, bin string) (string, error) { - // Check in the local directory - local := filepath.Join(taskDir, allocdir.TaskLocal, bin) - if _, err := os.Stat(local); err == nil { - return local, nil - } - - // Check at the root of the task's directory - root := filepath.Join(taskDir, bin) - if _, err := os.Stat(root); err == nil { - return root, nil + if p, err := lookupTaskBin(taskDir, bin); err == nil { + return p, nil } // when checking host paths, check with Stat first if path is absolute @@ -577,6 +570,22 @@ func lookupBin(taskDir string, bin string) (string, error) { return "", fmt.Errorf("binary %q could not be found", bin) } +// LookTaskBin finds the file in taskDir/local or taskDir and returns an absolute path +func lookupTaskBin(taskDir string, bin string) (string, error) { + // Check in the local directory + local := filepath.Join(taskDir, allocdir.TaskLocal, bin) + if _, err := os.Stat(local); err == nil { + return local, nil + } + + // Check at the root of the task's directory + root := filepath.Join(taskDir, bin) + if _, err := os.Stat(root); err == nil { + return root, nil + } + return "", fmt.Errorf("file %s not found under path %s", bin, taskDir) +} + // makeExecutable makes the given file executable for root,group,others. func makeExecutable(binPath string) error { if runtime.GOOS == "windows" { From 83930dcb9c0f894b4497f98fbdb619bdf8c0410d Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Thu, 2 May 2019 13:35:24 -0400 Subject: [PATCH 02/15] executor_linux call new lookupTaskBin --- drivers/shared/executor/executor_linux.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index b007da139..a4fd87e48 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -142,7 +142,8 @@ func (l *LibcontainerExecutor) Launch(command *ExecCommand) (*ProcessState, erro l.container = container // Look up the binary path and make it executable - absPath, err := lookupBin(command.TaskDir, command.Cmd) + absPath, err := lookupTaskBin(command.TaskDir, command.Cmd) + if err != nil { return nil, err } @@ -153,7 +154,7 @@ func (l *LibcontainerExecutor) Launch(command *ExecCommand) (*ProcessState, erro path := absPath - // Determine the path to run as it may have to be relative to the chroot. + // Ensure that the path is contained in the chroot, and find it relative to the container rel, err := filepath.Rel(command.TaskDir, path) if err != nil { return nil, fmt.Errorf("failed to determine relative path base=%q target=%q: %v", command.TaskDir, path, err) From c16f82bb9a5d2c98e8d77472449c5bbcc232d994 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Thu, 2 May 2019 13:35:42 -0400 Subject: [PATCH 03/15] executor_test test for more edges of lookupBin behavior --- drivers/shared/executor/executor_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/shared/executor/executor_test.go b/drivers/shared/executor/executor_test.go index 3da4c7b37..91d21e78c 100644 --- a/drivers/shared/executor/executor_test.go +++ b/drivers/shared/executor/executor_test.go @@ -23,6 +23,7 @@ import ( "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/plugins/drivers" tu "github.com/hashicorp/nomad/testutil" + "github.com/kr/pretty" ps "github.com/mitchellh/go-ps" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -415,10 +416,17 @@ func TestUniversalExecutor_LookupPath(t *testing.T) { err = ioutil.WriteFile(filePath, []byte{1, 2}, os.ModeAppend) require.Nil(err) + pretty.Log(tmpDir) + pretty.Log(filePath) + // Lookup with full path to binary _, err = lookupBin("dummy", filePath) require.Nil(err) + // Lookout with an absolute path to the binary + _, err = lookupBin(tmpDir, "/foo/tmp.txt") + require.Nil(err) + // Write a file under local subdir os.MkdirAll(filepath.Join(tmpDir, "local"), 0700) filePath2 := filepath.Join(tmpDir, "local", "tmp.txt") @@ -436,6 +444,13 @@ func TestUniversalExecutor_LookupPath(t *testing.T) { _, err = lookupBin(tmpDir, "tmp.txt") require.Nil(err) + // Lookup a host path + _, err = lookupBin(tmpDir, "/bin/sh") + require.Error(err) + + // Lookup a host path via $PATH + _, err = lookupBin(tmpDir, "sh") + require.Error(err) } // setupRoootfs setups the rootfs for libcontainer executor From 103d37d4f96134ad02d9855b502e61b0891c540f Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Thu, 2 May 2019 13:36:34 -0400 Subject: [PATCH 04/15] executor_linux_test new TestExecutor_EscapeContainer --- .../shared/executor/executor_linux_test.go | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index 727d2ea60..8c0129165 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -160,6 +160,27 @@ ld.so.conf.d/` }, func(err error) { t.Error(err) }) } +// Exec Launch looks for the binary only inside the chroot +func TestExecutor_EscapeContainer(t *testing.T) { + t.Parallel() + require := require.New(t) + testutil.ExecCompatible(t) + + testExecCmd := testExecutorCommandWithChroot(t) + execCmd, allocDir := testExecCmd.command, testExecCmd.allocDir + execCmd.Cmd = "/bin/kill" // missing from the chroot container + defer allocDir.Destroy() + + execCmd.ResourceLimits = true + + executor := NewExecutorWithIsolation(testlog.HCLogger(t)) + defer executor.Shutdown("SIGKILL", 0) + + _, err := executor.Launch(execCmd) + require.Error(err) + require.Regexp("^file /bin/kill not found under path", err) +} + func TestExecutor_ClientCleanup(t *testing.T) { t.Parallel() testutil.ExecCompatible(t) From 7a2fdf7a2dbbb93d851fe93cf55b98108094cf4f Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Fri, 3 May 2019 14:42:57 -0400 Subject: [PATCH 05/15] executor and executor_linux debug launch prep and process start --- drivers/shared/executor/executor.go | 3 ++- drivers/shared/executor/executor_linux.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/shared/executor/executor.go b/drivers/shared/executor/executor.go index 40eb3c88f..a4abd864c 100644 --- a/drivers/shared/executor/executor.go +++ b/drivers/shared/executor/executor.go @@ -248,7 +248,7 @@ func (e *UniversalExecutor) Version() (*ExecutorVersion, error) { // Launch launches the main process and returns its state. It also // configures an applies isolation on certain platforms. func (e *UniversalExecutor) Launch(command *ExecCommand) (*ProcessState, error) { - e.logger.Debug("launching command", "command", command.Cmd, "args", strings.Join(command.Args, " ")) + e.logger.Debug("launch prep", "command", command.Cmd, "args", strings.Join(command.Args, " ")) e.commandCfg = command @@ -303,6 +303,7 @@ func (e *UniversalExecutor) Launch(command *ExecCommand) (*ProcessState, error) e.childCmd.Env = e.commandCfg.Env // Start the process + e.logger.Debug("launching", "command", command.Cmd, "args", strings.Join(command.Args, " ")) if err := e.childCmd.Start(); err != nil { return nil, fmt.Errorf("failed to start command path=%q --- args=%q: %v", path, e.childCmd.Args, err) } diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index a4fd87e48..91ec6ad98 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -97,7 +97,7 @@ func NewExecutorWithIsolation(logger hclog.Logger) Executor { // Launch creates a new container in libcontainer and starts a new process with it func (l *LibcontainerExecutor) Launch(command *ExecCommand) (*ProcessState, error) { - l.logger.Debug("launching command", "command", command.Cmd, "args", strings.Join(command.Args, " ")) + l.logger.Debug("launch prep", "command", command.Cmd, "args", strings.Join(command.Args, " ")) if command.Resources == nil { command.Resources = &drivers.Resources{ @@ -177,6 +177,8 @@ func (l *LibcontainerExecutor) Launch(command *ExecCommand) (*ProcessState, erro return nil, err } + l.logger.Debug("launching", "command", command.Cmd, "args", strings.Join(command.Args, " ")) + // the task process will be started by the container process := &libcontainer.Process{ Args: combined, From 512bc52af5e229a5fce2b73b1510aed0b9799cbc Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Fri, 3 May 2019 16:20:05 -0400 Subject: [PATCH 06/15] executor_linux_test test PATH lookup inside the container --- drivers/shared/executor/executor_linux_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index 8c0129165..04799b2df 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -179,6 +179,22 @@ func TestExecutor_EscapeContainer(t *testing.T) { _, err := executor.Launch(execCmd) require.Error(err) require.Regexp("^file /bin/kill not found under path", err) + + // Bare files are looked up using the system path, inside the container + allocDir.Destroy() + testExecCmd = testExecutorCommandWithChroot(t) + execCmd, allocDir = testExecCmd.command, testExecCmd.allocDir + execCmd.Cmd = "kill" + _, err = executor.Launch(execCmd) + require.Error(err) + require.Regexp("^file kill not found under path", err) + + allocDir.Destroy() + testExecCmd = testExecutorCommandWithChroot(t) + execCmd, allocDir = testExecCmd.command, testExecCmd.allocDir + execCmd.Cmd = "echo" + _, err = executor.Launch(execCmd) + require.NoError(err) } func TestExecutor_ClientCleanup(t *testing.T) { From 8a7b5c68309d38c59568abb7ddb26209e0799940 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Fri, 3 May 2019 16:22:09 -0400 Subject: [PATCH 07/15] executor lookupTaskBin also does PATH expansion, anchored in taskDIR --- drivers/shared/executor/executor.go | 41 ++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/drivers/shared/executor/executor.go b/drivers/shared/executor/executor.go index a4abd864c..cb7acfa65 100644 --- a/drivers/shared/executor/executor.go +++ b/drivers/shared/executor/executor.go @@ -551,8 +551,16 @@ func (e *UniversalExecutor) handleStats(ch chan *cstructs.TaskResourceUsage, ctx // task/local/, task/, on the host file system, in host $PATH // The return path is absolute. func lookupBin(taskDir string, bin string) (string, error) { - if p, err := lookupTaskBin(taskDir, bin); err == nil { - return p, nil + // Check in the local directory + local := filepath.Join(taskDir, allocdir.TaskLocal, bin) + if _, err := os.Stat(local); err == nil { + return local, nil + } + + // Check at the root of the task's directory + root := filepath.Join(taskDir, bin) + if _, err := os.Stat(root); err == nil { + return root, nil } // when checking host paths, check with Stat first if path is absolute @@ -571,10 +579,13 @@ func lookupBin(taskDir string, bin string) (string, error) { return "", fmt.Errorf("binary %q could not be found", bin) } -// LookTaskBin finds the file in taskDir/local or taskDir and returns an absolute path +// lookupTaskBin finds the file in: +// taskDir/local, taskDir, PATH search inside taskDir +// and returns an absolute path func lookupTaskBin(taskDir string, bin string) (string, error) { // Check in the local directory - local := filepath.Join(taskDir, allocdir.TaskLocal, bin) + localDir := filepath.Join(taskDir, allocdir.TaskLocal) + local := filepath.Join(localDir, bin) if _, err := os.Stat(local); err == nil { return local, nil } @@ -584,6 +595,28 @@ func lookupTaskBin(taskDir string, bin string) (string, error) { if _, err := os.Stat(root); err == nil { return root, nil } + + // Check the host PATH anchored inside the taskDir + if !strings.Contains(bin, "/") { + envPath := os.Getenv("PATH") + for _, dir := range filepath.SplitList(envPath) { + if dir == "" { + // match unix shell behavior, empty path element == . + dir = "." + } + for _, root := range []string{localDir, taskDir} { + path := filepath.Join(root, dir, bin) + f, err := os.Stat(path) + if err != nil { + continue + } + if m := f.Mode(); !m.IsDir() { + return path, nil + } + } + } + } + return "", fmt.Errorf("file %s not found under path %s", bin, taskDir) } From 8c638ada5aa1d34b6a004ec62271b4f90b1ceee8 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Fri, 3 May 2019 16:34:31 -0400 Subject: [PATCH 08/15] driver_test StartWait task calls a program inside it's chroot --- drivers/exec/driver_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/exec/driver_test.go b/drivers/exec/driver_test.go index 2c0a565bc..cb56747a2 100644 --- a/drivers/exec/driver_test.go +++ b/drivers/exec/driver_test.go @@ -102,7 +102,7 @@ func TestExecDriver_StartWait(t *testing.T) { } tc := &TaskConfig{ - Command: "cat", + Command: "echo", Args: []string{"/proc/self/cgroup"}, } require.NoError(task.EncodeConcreteDriverConfig(&tc)) From bc5eaf6cdbac8c63b7952318d4dbaa26bd4fb8dc Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Sat, 4 May 2019 10:21:59 -0400 Subject: [PATCH 09/15] executor_test cleanup old lookupBin tests --- drivers/shared/executor/executor_test.go | 70 ++++++++++++++++++------ 1 file changed, 52 insertions(+), 18 deletions(-) diff --git a/drivers/shared/executor/executor_test.go b/drivers/shared/executor/executor_test.go index 91d21e78c..9db7689db 100644 --- a/drivers/shared/executor/executor_test.go +++ b/drivers/shared/executor/executor_test.go @@ -23,7 +23,6 @@ import ( "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/plugins/drivers" tu "github.com/hashicorp/nomad/testutil" - "github.com/kr/pretty" ps "github.com/mitchellh/go-ps" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -402,9 +401,9 @@ func TestUniversalExecutor_MakeExecutable(t *testing.T) { func TestUniversalExecutor_LookupPath(t *testing.T) { t.Parallel() + require := require.New(t) // Create a temp dir tmpDir, err := ioutil.TempDir("", "") - require := require.New(t) require.Nil(err) defer os.Remove(tmpDir) @@ -416,40 +415,75 @@ func TestUniversalExecutor_LookupPath(t *testing.T) { err = ioutil.WriteFile(filePath, []byte{1, 2}, os.ModeAppend) require.Nil(err) - pretty.Log(tmpDir) - pretty.Log(filePath) - - // Lookup with full path to binary - _, err = lookupBin("dummy", filePath) + // Lookup with full path on host to binary + path, err := lookupBin("not_tmpDir", filePath) require.Nil(err) + require.Equal(filePath, path) // Lookout with an absolute path to the binary _, err = lookupBin(tmpDir, "/foo/tmp.txt") require.Nil(err) - // Write a file under local subdir - os.MkdirAll(filepath.Join(tmpDir, "local"), 0700) - filePath2 := filepath.Join(tmpDir, "local", "tmp.txt") - ioutil.WriteFile(filePath2, []byte{1, 2}, os.ModeAppend) - - // Lookup with file name, should find the one we wrote above - _, err = lookupBin(tmpDir, "tmp.txt") - require.Nil(err) - // Write a file under task dir filePath3 := filepath.Join(tmpDir, "tmp.txt") ioutil.WriteFile(filePath3, []byte{1, 2}, os.ModeAppend) // Lookup with file name, should find the one we wrote above - _, err = lookupBin(tmpDir, "tmp.txt") + path, err = lookupBin(tmpDir, "tmp.txt") require.Nil(err) + require.Equal(filepath.Join(tmpDir, "tmp.txt"), path) + + // Write a file under local subdir + os.MkdirAll(filepath.Join(tmpDir, "local"), 0700) + filePath2 := filepath.Join(tmpDir, "local", "tmp.txt") + ioutil.WriteFile(filePath2, []byte{1, 2}, os.ModeAppend) + + // Lookup with file name, should find the one we wrote above + path, err = lookupBin(tmpDir, "tmp.txt") + require.Nil(err) + require.Equal(filepath.Join(tmpDir, "local", "tmp.txt"), path) // Lookup a host path _, err = lookupBin(tmpDir, "/bin/sh") - require.Error(err) + require.NoError(err) // Lookup a host path via $PATH _, err = lookupBin(tmpDir, "sh") + require.NoError(err) +} + +func TestUniversalExecutor_LookupTaskBin(t *testing.T) { + t.Parallel() + require := require.New(t) + + // Create a temp dir + tmpDir, err := ioutil.TempDir("", "") + require.Nil(err) + defer os.Remove(tmpDir) + + // Make a foo subdir + os.MkdirAll(filepath.Join(tmpDir, "foo"), 0700) + + // Write a file under foo + filePath := filepath.Join(tmpDir, "foo", "tmp.txt") + err = ioutil.WriteFile(filePath, []byte{1, 2}, os.ModeAppend) + require.NoError(err) + + // Lookout with an absolute path to the binary + _, err = lookupTaskBin(tmpDir, "/foo/tmp.txt") + require.NoError(err) + + // Write a file under local subdir + os.MkdirAll(filepath.Join(tmpDir, "local"), 0700) + filePath2 := filepath.Join(tmpDir, "local", "tmp.txt") + ioutil.WriteFile(filePath2, []byte{1, 2}, os.ModeAppend) + + // Lookup with file name, should find the one we wrote above + _, err = lookupTaskBin(tmpDir, "tmp.txt") + require.NoError(err) + + // Lookup a host absolute path + _, err = lookupTaskBin(tmpDir, "/bin/sh") require.Error(err) } From 2ea860a0877896a5dbde485227f02d961f683c49 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Tue, 7 May 2019 16:13:38 -0400 Subject: [PATCH 10/15] driver_test leave cat in the test, but add cat to the chroot --- drivers/exec/driver_test.go | 2 +- drivers/shared/executor/executor_linux_test.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/exec/driver_test.go b/drivers/exec/driver_test.go index cb56747a2..2c0a565bc 100644 --- a/drivers/exec/driver_test.go +++ b/drivers/exec/driver_test.go @@ -102,7 +102,7 @@ func TestExecDriver_StartWait(t *testing.T) { } tc := &TaskConfig{ - Command: "echo", + Command: "cat", Args: []string{"/proc/self/cgroup"}, } require.NoError(task.EncodeConcreteDriverConfig(&tc)) diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index 04799b2df..a90e1d200 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -48,6 +48,7 @@ func testExecutorCommandWithChroot(t *testing.T) *testExecCmd { "/lib64": "/lib64", "/usr/lib": "/usr/lib", "/bin/ls": "/bin/ls", + "/bin/cat": "/bin/cat", "/bin/echo": "/bin/echo", "/bin/bash": "/bin/bash", "/bin/sleep": "/bin/sleep", From 538f387d8d4e0c21c483b39a982417e2cb8698c6 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Tue, 7 May 2019 16:58:27 -0400 Subject: [PATCH 11/15] move lookupTaskBin to executor_linux, for os dependency clarity --- drivers/shared/executor/executor.go | 41 ------------------- drivers/shared/executor/executor_linux.go | 41 +++++++++++++++++++ .../shared/executor/executor_linux_test.go | 35 ++++++++++++++++ drivers/shared/executor/executor_test.go | 35 ---------------- 4 files changed, 76 insertions(+), 76 deletions(-) diff --git a/drivers/shared/executor/executor.go b/drivers/shared/executor/executor.go index cb7acfa65..301deb45b 100644 --- a/drivers/shared/executor/executor.go +++ b/drivers/shared/executor/executor.go @@ -579,47 +579,6 @@ func lookupBin(taskDir string, bin string) (string, error) { return "", fmt.Errorf("binary %q could not be found", bin) } -// lookupTaskBin finds the file in: -// taskDir/local, taskDir, PATH search inside taskDir -// and returns an absolute path -func lookupTaskBin(taskDir string, bin string) (string, error) { - // Check in the local directory - localDir := filepath.Join(taskDir, allocdir.TaskLocal) - local := filepath.Join(localDir, bin) - if _, err := os.Stat(local); err == nil { - return local, nil - } - - // Check at the root of the task's directory - root := filepath.Join(taskDir, bin) - if _, err := os.Stat(root); err == nil { - return root, nil - } - - // Check the host PATH anchored inside the taskDir - if !strings.Contains(bin, "/") { - envPath := os.Getenv("PATH") - for _, dir := range filepath.SplitList(envPath) { - if dir == "" { - // match unix shell behavior, empty path element == . - dir = "." - } - for _, root := range []string{localDir, taskDir} { - path := filepath.Join(root, dir, bin) - f, err := os.Stat(path) - if err != nil { - continue - } - if m := f.Mode(); !m.IsDir() { - return path, nil - } - } - } - } - - return "", fmt.Errorf("file %s not found under path %s", bin, taskDir) -} - // makeExecutable makes the given file executable for root,group,others. func makeExecutable(binPath string) error { if runtime.GOOS == "windows" { diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index 91ec6ad98..3757cd746 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/consul-template/signals" hclog "github.com/hashicorp/go-hclog" multierror "github.com/hashicorp/go-multierror" + "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/stats" cstructs "github.com/hashicorp/nomad/client/structs" shelpers "github.com/hashicorp/nomad/helper/stats" @@ -770,3 +771,43 @@ func cmdMounts(mounts []*drivers.MountConfig) []*lconfigs.Mount { return r } + +// lookupTaskBin finds the file `bin` in taskDir/local, taskDir in that order, then performs +// a PATH search inside taskDir. It returns an absolute path. See also executor.lookupBin +func lookupTaskBin(taskDir string, bin string) (string, error) { + // Check in the local directory + localDir := filepath.Join(taskDir, allocdir.TaskLocal) + local := filepath.Join(localDir, bin) + if _, err := os.Stat(local); err == nil { + return local, nil + } + + // Check at the root of the task's directory + root := filepath.Join(taskDir, bin) + if _, err := os.Stat(root); err == nil { + return root, nil + } + + // Check the host PATH anchored inside the taskDir + if !strings.Contains(bin, "/") { + envPath := os.Getenv("PATH") + for _, dir := range filepath.SplitList(envPath) { + if dir == "" { + // match unix shell behavior, empty path element == . + dir = "." + } + for _, root := range []string{localDir, taskDir} { + path := filepath.Join(root, dir, bin) + f, err := os.Stat(path) + if err != nil { + continue + } + if m := f.Mode(); !m.IsDir() { + return path, nil + } + } + } + } + + return "", fmt.Errorf("file %s not found under path %s", bin, taskDir) +} diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index a90e1d200..1b9318775 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -161,6 +161,41 @@ ld.so.conf.d/` }, func(err error) { t.Error(err) }) } +func TestUniversalExecutor_LookupTaskBin(t *testing.T) { + t.Parallel() + require := require.New(t) + + // Create a temp dir + tmpDir, err := ioutil.TempDir("", "") + require.Nil(err) + defer os.Remove(tmpDir) + + // Make a foo subdir + os.MkdirAll(filepath.Join(tmpDir, "foo"), 0700) + + // Write a file under foo + filePath := filepath.Join(tmpDir, "foo", "tmp.txt") + err = ioutil.WriteFile(filePath, []byte{1, 2}, os.ModeAppend) + require.NoError(err) + + // Lookout with an absolute path to the binary + _, err = lookupTaskBin(tmpDir, "/foo/tmp.txt") + require.NoError(err) + + // Write a file under local subdir + os.MkdirAll(filepath.Join(tmpDir, "local"), 0700) + filePath2 := filepath.Join(tmpDir, "local", "tmp.txt") + ioutil.WriteFile(filePath2, []byte{1, 2}, os.ModeAppend) + + // Lookup with file name, should find the one we wrote above + _, err = lookupTaskBin(tmpDir, "tmp.txt") + require.NoError(err) + + // Lookup a host absolute path + _, err = lookupTaskBin(tmpDir, "/bin/sh") + require.Error(err) +} + // Exec Launch looks for the binary only inside the chroot func TestExecutor_EscapeContainer(t *testing.T) { t.Parallel() diff --git a/drivers/shared/executor/executor_test.go b/drivers/shared/executor/executor_test.go index 9db7689db..65af774a5 100644 --- a/drivers/shared/executor/executor_test.go +++ b/drivers/shared/executor/executor_test.go @@ -452,41 +452,6 @@ func TestUniversalExecutor_LookupPath(t *testing.T) { require.NoError(err) } -func TestUniversalExecutor_LookupTaskBin(t *testing.T) { - t.Parallel() - require := require.New(t) - - // Create a temp dir - tmpDir, err := ioutil.TempDir("", "") - require.Nil(err) - defer os.Remove(tmpDir) - - // Make a foo subdir - os.MkdirAll(filepath.Join(tmpDir, "foo"), 0700) - - // Write a file under foo - filePath := filepath.Join(tmpDir, "foo", "tmp.txt") - err = ioutil.WriteFile(filePath, []byte{1, 2}, os.ModeAppend) - require.NoError(err) - - // Lookout with an absolute path to the binary - _, err = lookupTaskBin(tmpDir, "/foo/tmp.txt") - require.NoError(err) - - // Write a file under local subdir - os.MkdirAll(filepath.Join(tmpDir, "local"), 0700) - filePath2 := filepath.Join(tmpDir, "local", "tmp.txt") - ioutil.WriteFile(filePath2, []byte{1, 2}, os.ModeAppend) - - // Lookup with file name, should find the one we wrote above - _, err = lookupTaskBin(tmpDir, "tmp.txt") - require.NoError(err) - - // Lookup a host absolute path - _, err = lookupTaskBin(tmpDir, "/bin/sh") - require.Error(err) -} - // setupRoootfs setups the rootfs for libcontainer executor // It uses busybox to make some binaries available - somewhat cheaper // than mounting the underlying host filesystem From 9688710c1052985d4752daf626dcfcf8107daf48 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Tue, 7 May 2019 17:01:05 -0400 Subject: [PATCH 12/15] executor/* Launch log at top of Launch is more explicit, trace --- drivers/shared/executor/executor.go | 2 +- drivers/shared/executor/executor_linux.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/shared/executor/executor.go b/drivers/shared/executor/executor.go index 301deb45b..59a890fa1 100644 --- a/drivers/shared/executor/executor.go +++ b/drivers/shared/executor/executor.go @@ -248,7 +248,7 @@ func (e *UniversalExecutor) Version() (*ExecutorVersion, error) { // Launch launches the main process and returns its state. It also // configures an applies isolation on certain platforms. func (e *UniversalExecutor) Launch(command *ExecCommand) (*ProcessState, error) { - e.logger.Debug("launch prep", "command", command.Cmd, "args", strings.Join(command.Args, " ")) + e.logger.Trace("preparing to launch command", "command", command.Cmd, "args", strings.Join(command.Args, " ")) e.commandCfg = command diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index 3757cd746..3c235206b 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -98,7 +98,7 @@ func NewExecutorWithIsolation(logger hclog.Logger) Executor { // Launch creates a new container in libcontainer and starts a new process with it func (l *LibcontainerExecutor) Launch(command *ExecCommand) (*ProcessState, error) { - l.logger.Debug("launch prep", "command", command.Cmd, "args", strings.Join(command.Args, " ")) + l.logger.Trace("preparing to launch command", "command", command.Cmd, "args", strings.Join(command.Args, " ")) if command.Resources == nil { command.Resources = &drivers.Resources{ From 0c4a45fa16183a56336cc73720c444b30281ac91 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Wed, 8 May 2019 10:01:20 -0400 Subject: [PATCH 13/15] executor_linux pass the command to lookupTaskBin to get path --- drivers/shared/executor/executor_linux.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index 3c235206b..77304ec0b 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -143,7 +143,7 @@ func (l *LibcontainerExecutor) Launch(command *ExecCommand) (*ProcessState, erro l.container = container // Look up the binary path and make it executable - absPath, err := lookupTaskBin(command.TaskDir, command.Cmd) + absPath, err := lookupTaskBin(command) if err != nil { return nil, err @@ -774,7 +774,10 @@ func cmdMounts(mounts []*drivers.MountConfig) []*lconfigs.Mount { // lookupTaskBin finds the file `bin` in taskDir/local, taskDir in that order, then performs // a PATH search inside taskDir. It returns an absolute path. See also executor.lookupBin -func lookupTaskBin(taskDir string, bin string) (string, error) { +func lookupTaskBin(command *ExecCommand) (string, error) { + taskDir := command.TaskDir + bin := command.Cmd + // Check in the local directory localDir := filepath.Join(taskDir, allocdir.TaskLocal) local := filepath.Join(localDir, bin) @@ -788,10 +791,17 @@ func lookupTaskBin(taskDir string, bin string) (string, error) { return root, nil } + // Find the PATH + path := "/usr/local/bin:/usr/bin:/bin" + for _, e := range command.Env { + if strings.HasPrefix("PATH=", e) { + path = e[5:] + } + } + // Check the host PATH anchored inside the taskDir if !strings.Contains(bin, "/") { - envPath := os.Getenv("PATH") - for _, dir := range filepath.SplitList(envPath) { + for _, dir := range filepath.SplitList(path) { if dir == "" { // match unix shell behavior, empty path element == . dir = "." From 60735deb7b81f40ec85c526ede7554d203d76838 Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Wed, 8 May 2019 10:01:51 -0400 Subject: [PATCH 14/15] executor_linux_test call lookupTaskBin with an ExecCommand --- drivers/shared/executor/executor_linux_test.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index 1b9318775..9d2a1e41e 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -170,6 +170,9 @@ func TestUniversalExecutor_LookupTaskBin(t *testing.T) { require.Nil(err) defer os.Remove(tmpDir) + // Create the command + cmd := &ExecCommand{Env: []string{"PATH=/bin"}, TaskDir: tmpDir} + // Make a foo subdir os.MkdirAll(filepath.Join(tmpDir, "foo"), 0700) @@ -179,7 +182,8 @@ func TestUniversalExecutor_LookupTaskBin(t *testing.T) { require.NoError(err) // Lookout with an absolute path to the binary - _, err = lookupTaskBin(tmpDir, "/foo/tmp.txt") + cmd.Cmd = "/foo/tmp.txt" + _, err = lookupTaskBin(cmd) require.NoError(err) // Write a file under local subdir @@ -188,11 +192,13 @@ func TestUniversalExecutor_LookupTaskBin(t *testing.T) { ioutil.WriteFile(filePath2, []byte{1, 2}, os.ModeAppend) // Lookup with file name, should find the one we wrote above - _, err = lookupTaskBin(tmpDir, "tmp.txt") + cmd.Cmd = "tmp.txt" + _, err = lookupTaskBin(cmd) require.NoError(err) // Lookup a host absolute path - _, err = lookupTaskBin(tmpDir, "/bin/sh") + cmd.Cmd = "/bin/sh" + _, err = lookupTaskBin(cmd) require.Error(err) } From a9da4bac111cf93c169979f5a5edfae588ddc4ec Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Fri, 10 May 2019 11:33:35 -0400 Subject: [PATCH 15/15] executor_linux only do path resolution in the taskDir, not local split out lookPathIn to show it's similarity to exec.LookPath --- drivers/shared/executor/executor_linux.go | 42 +++++++++++++---------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index 77304ec0b..e3e851127 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -791,6 +791,10 @@ func lookupTaskBin(command *ExecCommand) (string, error) { return root, nil } + if strings.Contains(bin, "/") { + return "", fmt.Errorf("file %s not found under path %s", bin, taskDir) + } + // Find the PATH path := "/usr/local/bin:/usr/bin:/bin" for _, e := range command.Env { @@ -799,25 +803,25 @@ func lookupTaskBin(command *ExecCommand) (string, error) { } } - // Check the host PATH anchored inside the taskDir - if !strings.Contains(bin, "/") { - for _, dir := range filepath.SplitList(path) { - if dir == "" { - // match unix shell behavior, empty path element == . - dir = "." - } - for _, root := range []string{localDir, taskDir} { - path := filepath.Join(root, dir, bin) - f, err := os.Stat(path) - if err != nil { - continue - } - if m := f.Mode(); !m.IsDir() { - return path, nil - } - } + return lookPathIn(path, taskDir, bin) +} + +// lookPathIn looks for a file with PATH inside the directory root. Like exec.LookPath +func lookPathIn(path string, root string, bin string) (string, error) { + // exec.LookPath(file string) + for _, dir := range filepath.SplitList(path) { + if dir == "" { + // match unix shell behavior, empty path element == . + dir = "." + } + path := filepath.Join(root, dir, bin) + f, err := os.Stat(path) + if err != nil { + continue + } + if m := f.Mode(); !m.IsDir() { + return path, nil } } - - return "", fmt.Errorf("file %s not found under path %s", bin, taskDir) + return "", fmt.Errorf("file %s not found under path %s", bin, root) }