From 54046a1d28f32e76cb1a0f96336c255b7f5943c1 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Fri, 7 Oct 2022 15:43:37 -0400 Subject: [PATCH] exec: allow running commands from host volume The exec driver and other drivers derived from the shared executor check the path of the command before handing off to libcontainer to ensure that the command doesn't escape the sandbox. But we don't check any host volume mounts, which should be safe to use as a source for executables if we're letting the user mount them to the container in the first place. Check the mount config to verify the executable lives in the mount's host path, but then return an absolute path for the mount's task path so that we can hand that off to libcontainer to run. --- drivers/shared/executor/executor_linux.go | 53 +++++++++++-------- .../shared/executor/executor_linux_test.go | 6 +-- 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index fc91fc367280..877455ca6c1f 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -120,30 +120,30 @@ func (l *LibcontainerExecutor) Launch(command *ExecCommand) (*ProcessState, erro l.container = container // Look up the binary path and make it executable - absPath, err := lookupTaskBin(command) + absPath, fromMount, err := lookupTaskBin(command) if err != nil { return nil, err } - - if err := makeExecutable(absPath); err != nil { - return nil, err - } - path := absPath - // 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) - } + if !fromMount { + if err := makeExecutable(absPath); err != nil { + return nil, err + } - // Turn relative-to-chroot path into absolute path to avoid - // libcontainer trying to resolve the binary using $PATH. - // Do *not* use filepath.Join as it will translate ".."s returned by - // filepath.Rel. Prepending "/" will cause the path to be rooted in the - // chroot which is the desired behavior. - path = "/" + rel + // 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) + } + // Turn relative-to-chroot path into absolute path to avoid + // libcontainer trying to resolve the binary using $PATH. + // Do *not* use filepath.Join as it will translate ".."s returned by + // filepath.Rel. Prepending "/" will cause the path to be rooted in the + // chroot which is the desired behavior. + path = "/" + rel + } combined := append([]string{path}, command.Args...) stdout, err := command.Stdout() @@ -807,7 +807,7 @@ 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(command *ExecCommand) (string, error) { +func lookupTaskBin(command *ExecCommand) (string, bool, error) { taskDir := command.TaskDir bin := command.Cmd @@ -815,22 +815,31 @@ func lookupTaskBin(command *ExecCommand) (string, error) { localDir := filepath.Join(taskDir, allocdir.TaskLocal) local := filepath.Join(localDir, bin) if _, err := os.Stat(local); err == nil { - return local, nil + return local, false, 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 root, false, nil + } + + // Check in our mounts + for _, mount := range command.Mounts { + inMount := filepath.Join(mount.HostPath, bin) + if _, err := os.Stat(inMount); err == nil { + return filepath.Join(mount.TaskPath, bin), true, nil + } } if strings.Contains(bin, "/") { - return "", fmt.Errorf("file %s not found under path %s", bin, taskDir) + return "", false, fmt.Errorf("file %s not found under path %s", bin, taskDir) } path := "/usr/local/bin:/usr/bin:/bin" - return lookPathIn(path, taskDir, bin) + pathFromPath, err := lookPathIn(path, taskDir, bin) + return pathFromPath, false, err } // lookPathIn looks for a file with PATH inside the directory root. Like exec.LookPath diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index d6b2d137fa50..2ed8ab64c43d 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -430,7 +430,7 @@ func TestUniversalExecutor_LookupTaskBin(t *testing.T) { // Lookout with an absolute path to the binary cmd.Cmd = "/foo/tmp.txt" - _, err = lookupTaskBin(cmd) + _, _, err = lookupTaskBin(cmd) require.NoError(err) // Write a file under local subdir @@ -440,12 +440,12 @@ func TestUniversalExecutor_LookupTaskBin(t *testing.T) { // Lookup with file name, should find the one we wrote above cmd.Cmd = "tmp.txt" - _, err = lookupTaskBin(cmd) + _, _, err = lookupTaskBin(cmd) require.NoError(err) // Lookup a host absolute path cmd.Cmd = "/bin/sh" - _, err = lookupTaskBin(cmd) + _, _, err = lookupTaskBin(cmd) require.Error(err) }