exec: allow running commands from host volume (#14851)

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 within the mount's task path so that we can hand
that off to libcontainer to run.

Includes a good bit of refactoring here because the anchoring of the final task
path has different code paths for inside the task dir vs inside a mount. But
I've fleshed out the test coverage of this a good bit to ensure we haven't
created any regressions in the process.
This commit is contained in:
Tim Gross 2022-11-11 09:51:15 -05:00 committed by GitHub
parent 01a3a29e51
commit eabbcebdd4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 241 additions and 80 deletions

3
.changelog/14851.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
exec: Allow running commands from mounted host volumes
```

View File

@ -120,32 +120,15 @@ func (l *LibcontainerExecutor) Launch(command *ExecCommand) (*ProcessState, erro
l.container = container
// Look up the binary path and make it executable
absPath, err := lookupTaskBin(command)
taskPath, hostPath, err := lookupTaskBin(command)
if err != nil {
return nil, err
}
if err := makeExecutable(absPath); err != nil {
if err := makeExecutable(hostPath); 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)
}
// 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...)
combined := append([]string{taskPath}, command.Args...)
stdout, err := command.Stdout()
if err != nil {
return nil, err
@ -805,52 +788,135 @@ 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(command *ExecCommand) (string, error) {
// lookupTaskBin finds the file `bin`, searching in order:
// - taskDir/local
// - taskDir
// - each mount, in order listed in the jobspec
// - a PATH-like search of usr/local/bin/, usr/bin/, and bin/ inside the taskDir
//
// Returns an absolute path inside the container that will get passed as arg[0]
// to the launched process, and the absolute path to that binary as seen by the
// host (these will be identical for binaries that don't come from mounts).
//
// See also executor.lookupBin for a version used by non-isolated drivers.
func lookupTaskBin(command *ExecCommand) (string, string, error) {
taskDir := command.TaskDir
bin := command.Cmd
// 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
taskPath, hostPath, err := getPathInTaskDir(command.TaskDir, localDir, bin)
if err == nil {
return taskPath, hostPath, 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
taskPath, hostPath, err = getPathInTaskDir(command.TaskDir, command.TaskDir, bin)
if err == nil {
return taskPath, hostPath, nil
}
// Check in our mounts
for _, mount := range command.Mounts {
taskPath, hostPath, err = getPathInMount(mount.HostPath, mount.TaskPath, bin)
if err == nil {
return taskPath, hostPath, nil
}
}
// If there's a / in the binary's path, we can't fallback to a PATH search
if strings.Contains(bin, "/") {
return "", fmt.Errorf("file %s not found under path %s", bin, taskDir)
return "", "", fmt.Errorf("file %s not found under path %s", bin, taskDir)
}
path := "/usr/local/bin:/usr/bin:/bin"
// look for a file using a PATH-style lookup inside the directory
// root. Similar to the stdlib's exec.LookPath except:
// - uses a restricted lookup PATH rather than the agent process's PATH env var.
// - does not require that the file is already executable (this will be ensured
// by the caller)
// - does not prevent using relative path as added to exec.LookPath in go1.19
// (this gets fixed-up in the caller)
return lookPathIn(path, taskDir, bin)
// This is a fake PATH so that we're not using the agent's PATH
restrictedPaths := []string{"/usr/local/bin", "/usr/bin", "/bin"}
for _, dir := range restrictedPaths {
pathDir := filepath.Join(command.TaskDir, dir)
taskPath, hostPath, err = getPathInTaskDir(command.TaskDir, pathDir, bin)
if err == nil {
return taskPath, hostPath, nil
}
}
return "", "", fmt.Errorf("file %s not found under path", 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
}
// getPathInTaskDir searches for the binary in the task directory and nested
// search directory. It returns the absolute path rooted inside the container
// and the absolute path on the host.
func getPathInTaskDir(taskDir, searchDir, bin string) (string, string, error) {
hostPath := filepath.Join(searchDir, bin)
err := filepathIsRegular(hostPath)
if err != nil {
return "", "", err
}
return "", fmt.Errorf("file %s not found under path %s", bin, root)
// Find the path relative to the task directory
rel, err := filepath.Rel(taskDir, hostPath)
if rel == "" || err != nil {
return "", "", fmt.Errorf(
"failed to determine relative path base=%q target=%q: %v",
taskDir, hostPath, err)
}
// Turn relative-to-taskdir path into re-rooted 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.
return filepath.Clean("/" + rel), hostPath, nil
}
// getPathInMount for the binary in the mount's host path, constructing the path
// considering that the bin path is rooted in the mount's task path and not its
// host path. It returns the absolute path rooted inside the container and the
// absolute path on the host.
func getPathInMount(mountHostPath, mountTaskPath, bin string) (string, string, error) {
// Find the path relative to the mount point in the task so that we can
// trim off any shared prefix when we search on the host path
mountRel, err := filepath.Rel(mountTaskPath, bin)
if mountRel == "" || err != nil {
return "", "", fmt.Errorf("path was not relative to the mount task path")
}
hostPath := filepath.Join(mountHostPath, mountRel)
err = filepathIsRegular(hostPath)
if err != nil {
return "", "", err
}
// Turn relative-to-taskdir path into re-rooted 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.
return filepath.Clean("/" + bin), hostPath, nil
}
// filepathIsRegular verifies that a filepath is a regular file (i.e. not a
// directory, socket, device, etc.)
func filepathIsRegular(path string) error {
f, err := os.Stat(path)
if err != nil {
return err
}
if !f.Mode().Type().IsRegular() {
return fmt.Errorf("path was not a regular file")
}
return nil
}
func newSetCPUSetCgroupHook(cgroupPath string) lconfigs.Hook {

View File

@ -25,6 +25,8 @@ import (
"github.com/opencontainers/runc/libcontainer/cgroups"
lconfigs "github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/devices"
"github.com/shoenig/test"
"github.com/shoenig/test/must"
"github.com/stretchr/testify/require"
"golang.org/x/sys/unix"
)
@ -410,43 +412,121 @@ func TestExecutor_CgroupPathsAreDestroyed(t *testing.T) {
}
}
func TestUniversalExecutor_LookupTaskBin(t *testing.T) {
func TestExecutor_LookupTaskBin(t *testing.T) {
ci.Parallel(t)
require := require.New(t)
// Create a temp dir
tmpDir := t.TempDir()
taskDir := t.TempDir()
mountDir := t.TempDir()
// Create the command
cmd := &ExecCommand{Env: []string{"PATH=/bin"}, TaskDir: tmpDir}
// Create the command with mounts
cmd := &ExecCommand{
Env: []string{"PATH=/bin"},
TaskDir: taskDir,
Mounts: []*drivers.MountConfig{{TaskPath: "/srv", HostPath: mountDir}},
}
// Make a foo subdir
os.MkdirAll(filepath.Join(tmpDir, "foo"), 0700)
// Make a /foo /local/foo and /usr/local/bin subdirs under task dir
// and /bar under mountdir
must.NoError(t, os.MkdirAll(filepath.Join(taskDir, "foo"), 0700))
must.NoError(t, os.MkdirAll(filepath.Join(taskDir, "local/foo"), 0700))
must.NoError(t, os.MkdirAll(filepath.Join(taskDir, "usr/local/bin"), 0700))
must.NoError(t, os.MkdirAll(filepath.Join(mountDir, "bar"), 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)
writeFile := func(paths ...string) {
t.Helper()
path := filepath.Join(paths...)
must.NoError(t, os.WriteFile(path, []byte("hello"), 0o700))
}
// Lookout with an absolute path to the binary
cmd.Cmd = "/foo/tmp.txt"
_, err = lookupTaskBin(cmd)
require.NoError(err)
// Write some files
writeFile(taskDir, "usr/local/bin", "tmp0.txt") // under /usr/local/bin in taskdir
writeFile(taskDir, "foo", "tmp1.txt") // under foo in taskdir
writeFile(taskDir, "local", "tmp2.txt") // under root of task-local dir
writeFile(taskDir, "local/foo", "tmp3.txt") // under foo in task-local dir
writeFile(mountDir, "tmp4.txt") // under root of mount dir
writeFile(mountDir, "bar/tmp5.txt") // under bar in mount dir
// 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)
testCases := []struct {
name string
cmd string
expectErr string
expectTaskPath string
expectHostPath string
}{
{
name: "lookup with file name in PATH",
cmd: "tmp0.txt",
expectTaskPath: "/usr/local/bin/tmp0.txt",
expectHostPath: filepath.Join(taskDir, "usr/local/bin/tmp0.txt"),
},
{
name: "lookup with absolute path to binary",
cmd: "/foo/tmp1.txt",
expectTaskPath: "/foo/tmp1.txt",
expectHostPath: filepath.Join(taskDir, "foo/tmp1.txt"),
},
{
name: "lookup in task local dir with absolute path to binary",
cmd: "/local/tmp2.txt",
expectTaskPath: "/local/tmp2.txt",
expectHostPath: filepath.Join(taskDir, "local/tmp2.txt"),
},
{
name: "lookup in task local dir with relative path to binary",
cmd: "local/tmp2.txt",
expectTaskPath: "/local/tmp2.txt",
expectHostPath: filepath.Join(taskDir, "local/tmp2.txt"),
},
{
name: "lookup in task local dir with file name",
cmd: "tmp2.txt",
expectTaskPath: "/local/tmp2.txt",
expectHostPath: filepath.Join(taskDir, "local/tmp2.txt"),
},
{
name: "lookup in task local subdir with absolute path to binary",
cmd: "/local/foo/tmp3.txt",
expectTaskPath: "/local/foo/tmp3.txt",
expectHostPath: filepath.Join(taskDir, "local/foo/tmp3.txt"),
},
{
name: "lookup host absolute path outside taskdir",
cmd: "/bin/sh",
expectErr: "file /bin/sh not found under path " + taskDir,
},
{
name: "lookup file from mount with absolute path",
cmd: "/srv/tmp4.txt",
expectTaskPath: "/srv/tmp4.txt",
expectHostPath: filepath.Join(mountDir, "tmp4.txt"),
},
{
name: "lookup file from mount with file name fails",
cmd: "tmp4.txt",
expectErr: "file tmp4.txt not found under path",
},
{
name: "lookup file from mount with subdir",
cmd: "/srv/bar/tmp5.txt",
expectTaskPath: "/srv/bar/tmp5.txt",
expectHostPath: filepath.Join(mountDir, "bar/tmp5.txt"),
},
}
// Lookup with file name, should find the one we wrote above
cmd.Cmd = "tmp.txt"
_, err = lookupTaskBin(cmd)
require.NoError(err)
// Lookup a host absolute path
cmd.Cmd = "/bin/sh"
_, err = lookupTaskBin(cmd)
require.Error(err)
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
cmd.Cmd = tc.cmd
taskPath, hostPath, err := lookupTaskBin(cmd)
if tc.expectErr == "" {
must.NoError(t, err)
test.Eq(t, tc.expectTaskPath, taskPath)
test.Eq(t, tc.expectHostPath, hostPath)
} else {
test.EqError(t, err, tc.expectErr)
}
})
}
}
// Exec Launch looks for the binary only inside the chroot

View File

@ -31,9 +31,19 @@ The `exec` driver supports the following configuration in the job spec:
- `command` - The command to execute. Must be provided. If executing a binary
that exists on the host, the path must be absolute and within the task's
[chroot](#chroot). If executing a binary that is downloaded from
an [`artifact`](/docs/job-specification/artifact), the path can be
relative from the allocations's root directory.
[chroot](#chroot) or in a [host volume][] mounted with a
[`volume_mount`][volume_mount] block. The driver will make the binary
executable and will search, in order:
- The `local` directory with the task directory.
- The task directory.
- Any mounts, in the order listed in the job specification.
- The `usr/local/bin`, `usr/bin` and `bin` directories inside the task
directory.
If executing a binary that is downloaded
from an [`artifact`](/docs/job-specification/artifact), the path can be
relative from the allocation's root directory.
- `args` - (Optional) A list of arguments to the `command`. References
to environment variables or any [interpretable Nomad
@ -243,3 +253,5 @@ This list is configurable through the agent client
[no_net_raw]: /docs/upgrade/upgrade-specific#nomad-1-1-0-rc1-1-0-5-0-12-12
[allow_caps]: /docs/drivers/exec#allow_caps
[docker_caps]: https://docs.docker.com/engine/reference/run/#runtime-privilege-and-linux-capabilities
[host volume]: /docs/configuration/client#host_volume-stanza
[volume_mount]: /docs/job-specification/volume_mount