diff --git a/CHANGELOG.md b/CHANGELOG.md index 39eec0187165..77351547cc49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.10.9 (November 19, 2020) + +BUG FIXES: + * client: _Backport from v0.12.9_ - Fixed a regression where `NOMAD_{ALLOC,TASK,SECRETS}_DIR` variables would cause an error when interpolated into `template.source` stanzas. [[GH-9405](https://github.com/hashicorp/nomad/issues/9405)] + ## 0.10.8 (November 10, 2020) SECURITY: diff --git a/client/allocrunner/taskrunner/template/template.go b/client/allocrunner/taskrunner/template/template.go index 3107b8cecf9f..2661b340b474 100644 --- a/client/allocrunner/taskrunner/template/template.go +++ b/client/allocrunner/taskrunner/template/template.go @@ -2,6 +2,7 @@ package template import ( "context" + "errors" "fmt" "math/rand" "os" @@ -17,6 +18,7 @@ import ( "github.com/hashicorp/consul-template/signals" envparse "github.com/hashicorp/go-envparse" multierror "github.com/hashicorp/go-multierror" + "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/allocrunner/taskrunner/interfaces" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/taskenv" @@ -38,6 +40,11 @@ const ( DefaultMaxTemplateEventRate = 3 * time.Second ) +var ( + sourceEscapesErr = errors.New("template source path escapes alloc directory") + destEscapesErr = errors.New("template destination path escapes alloc directory") +) + // TaskTemplateManager is used to run a set of templates for a given task type TaskTemplateManager struct { // config holds the template managers configuration @@ -545,6 +552,18 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa sandboxEnabled := !config.ClientConfig.TemplateConfig.DisableSandbox taskEnv := config.EnvBuilder.Build() + // Make NOMAD_{ALLOC,TASK,SECRETS}_DIR relative paths to avoid treating + // them as sandbox escapes when using containers. + if taskEnv.EnvMap[taskenv.AllocDir] == allocdir.SharedAllocContainerPath { + taskEnv.EnvMap[taskenv.AllocDir] = allocdir.SharedAllocName + } + if taskEnv.EnvMap[taskenv.TaskLocalDir] == allocdir.TaskLocalContainerPath { + taskEnv.EnvMap[taskenv.TaskLocalDir] = allocdir.TaskLocal + } + if taskEnv.EnvMap[taskenv.SecretsDir] == allocdir.TaskSecretsContainerPath { + taskEnv.EnvMap[taskenv.SecretsDir] = allocdir.TaskSecrets + } + ctmpls := make(map[*ctconf.TemplateConfig]*structs.Template, len(config.Templates)) for _, tmpl := range config.Templates { var src, dest string @@ -557,7 +576,7 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa } escapes := helper.PathEscapesSandbox(config.TaskDir, src) if escapes && sandboxEnabled { - return nil, fmt.Errorf("template source path escapes alloc directory") + return nil, sourceEscapesErr } } @@ -569,7 +588,7 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa dest = filepath.Join(config.TaskDir, dest) escapes := helper.PathEscapesSandbox(config.TaskDir, dest) if escapes && sandboxEnabled { - return nil, fmt.Errorf("template destination path escapes alloc directory") + return nil, destEscapesErr } } diff --git a/client/allocrunner/taskrunner/template/template_test.go b/client/allocrunner/taskrunner/template/template_test.go index 35baebefb481..e1cc3947f398 100644 --- a/client/allocrunner/taskrunner/template/template_test.go +++ b/client/allocrunner/taskrunner/template/template_test.go @@ -13,9 +13,11 @@ import ( "time" ctestutil "github.com/hashicorp/consul/testutil" + "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" "github.com/hashicorp/nomad/client/taskenv" "github.com/hashicorp/nomad/helper" + "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" @@ -1455,6 +1457,255 @@ func TestTaskTemplateManager_Config_VaultNamespace(t *testing.T) { assert.Equal(testNS, *ctconf.Vault.Namespace, "Vault Namespace Value") } +// TestTaskTemplateManager_Escapes asserts that when sandboxing is enabled +// interpolated paths are not incorrectly treated as escaping the alloc dir. +func TestTaskTemplateManager_Escapes(t *testing.T) { + t.Parallel() + + clientConf := config.DefaultConfig() + require.False(t, clientConf.TemplateConfig.DisableSandbox, "expected sandbox to be disabled") + + // Set a fake alloc dir to make test output more realistic + clientConf.AllocDir = "/fake/allocdir" + + clientConf.Node = mock.Node() + alloc := mock.Alloc() + task := alloc.Job.TaskGroups[0].Tasks[0] + logger := testlog.HCLogger(t) + allocDir := allocdir.NewAllocDir(logger, filepath.Join(clientConf.AllocDir, alloc.ID)) + taskDir := allocDir.NewTaskDir(task.Name) + + containerEnv := func() *taskenv.Builder { + // To emulate a Docker or exec tasks we must copy the + // Set{Alloc,Task,Secrets}Dir logic in taskrunner/task_dir_hook.go + b := taskenv.NewBuilder(clientConf.Node, alloc, task, clientConf.Region) + b.SetAllocDir(allocdir.SharedAllocContainerPath) + b.SetTaskLocalDir(allocdir.TaskLocalContainerPath) + b.SetSecretsDir(allocdir.TaskSecretsContainerPath) + return b + } + + rawExecEnv := func() *taskenv.Builder { + // To emulate a unisolated tasks we must copy the + // Set{Alloc,Task,Secrets}Dir logic in taskrunner/task_dir_hook.go + b := taskenv.NewBuilder(clientConf.Node, alloc, task, clientConf.Region) + b.SetAllocDir(taskDir.SharedAllocDir) + b.SetTaskLocalDir(taskDir.LocalDir) + b.SetSecretsDir(taskDir.SecretsDir) + return b + } + + cases := []struct { + Name string + Config func() *TaskTemplateManagerConfig + + // Set to skip a test; remove once bugs are fixed + Skip bool + + // Expected paths to be returned if Err is nil + SourcePath string + DestPath string + + // Err is the expected error to be returned or nil + Err error + }{ + { + Name: "ContainerOk", + Config: func() *TaskTemplateManagerConfig { + return &TaskTemplateManagerConfig{ + ClientConfig: clientConf, + TaskDir: taskDir.Dir, + EnvBuilder: containerEnv(), + Templates: []*structs.Template{ + { + SourcePath: "${NOMAD_TASK_DIR}/src", + DestPath: "${NOMAD_SECRETS_DIR}/dst", + }, + }, + } + }, + SourcePath: filepath.Join(taskDir.Dir, "local/src"), + DestPath: filepath.Join(taskDir.Dir, "secrets/dst"), + }, + { + Name: "ContainerSrcEscapesErr", + Config: func() *TaskTemplateManagerConfig { + return &TaskTemplateManagerConfig{ + ClientConfig: clientConf, + TaskDir: taskDir.Dir, + EnvBuilder: containerEnv(), + Templates: []*structs.Template{ + { + SourcePath: "/etc/src_escapes", + DestPath: "${NOMAD_SECRETS_DIR}/dst", + }, + }, + } + }, + Err: sourceEscapesErr, + }, + { + Name: "ContainerSrcEscapesOk", + Config: func() *TaskTemplateManagerConfig { + unsafeConf := clientConf.Copy() + unsafeConf.TemplateConfig.DisableSandbox = true + return &TaskTemplateManagerConfig{ + ClientConfig: unsafeConf, + TaskDir: taskDir.Dir, + EnvBuilder: containerEnv(), + Templates: []*structs.Template{ + { + SourcePath: "/etc/src_escapes_ok", + DestPath: "${NOMAD_SECRETS_DIR}/dst", + }, + }, + } + }, + SourcePath: "/etc/src_escapes_ok", + DestPath: filepath.Join(taskDir.Dir, "secrets/dst"), + }, + { + Name: "ContainerDstAbsoluteOk", + Config: func() *TaskTemplateManagerConfig { + return &TaskTemplateManagerConfig{ + ClientConfig: clientConf, + TaskDir: taskDir.Dir, + EnvBuilder: containerEnv(), + Templates: []*structs.Template{ + { + SourcePath: "${NOMAD_TASK_DIR}/src", + DestPath: "/etc/absolutely_relative", + }, + }, + } + }, + SourcePath: filepath.Join(taskDir.Dir, "local/src"), + DestPath: filepath.Join(taskDir.Dir, "etc/absolutely_relative"), + }, + { + Name: "ContainerDstAbsoluteEscapesErr", + Config: func() *TaskTemplateManagerConfig { + return &TaskTemplateManagerConfig{ + ClientConfig: clientConf, + TaskDir: taskDir.Dir, + EnvBuilder: containerEnv(), + Templates: []*structs.Template{ + { + SourcePath: "${NOMAD_TASK_DIR}/src", + DestPath: "../escapes", + }, + }, + } + }, + Err: destEscapesErr, + }, + { + Name: "ContainerDstAbsoluteEscapesOk", + Config: func() *TaskTemplateManagerConfig { + unsafeConf := clientConf.Copy() + unsafeConf.TemplateConfig.DisableSandbox = true + return &TaskTemplateManagerConfig{ + ClientConfig: unsafeConf, + TaskDir: taskDir.Dir, + EnvBuilder: containerEnv(), + Templates: []*structs.Template{ + { + SourcePath: "${NOMAD_TASK_DIR}/src", + DestPath: "../escapes", + }, + }, + } + }, + SourcePath: filepath.Join(taskDir.Dir, "local/src"), + DestPath: filepath.Join(taskDir.Dir, "..", "escapes"), + }, + //TODO: Fix this test. I *think* it should pass. The double + // joining of the task dir onto the destination seems like + // a bug. https://github.com/hashicorp/nomad/issues/9389 + { + Skip: true, + Name: "RawExecOk", + Config: func() *TaskTemplateManagerConfig { + return &TaskTemplateManagerConfig{ + ClientConfig: clientConf, + TaskDir: taskDir.Dir, + EnvBuilder: rawExecEnv(), + Templates: []*structs.Template{ + { + SourcePath: "${NOMAD_TASK_DIR}/src", + DestPath: "${NOMAD_SECRETS_DIR}/dst", + }, + }, + } + }, + SourcePath: filepath.Join(taskDir.Dir, "local/src"), + DestPath: filepath.Join(taskDir.Dir, "secrets/dst"), + }, + { + Name: "RawExecSrcEscapesErr", + Config: func() *TaskTemplateManagerConfig { + return &TaskTemplateManagerConfig{ + ClientConfig: clientConf, + TaskDir: taskDir.Dir, + EnvBuilder: rawExecEnv(), + Templates: []*structs.Template{ + { + SourcePath: "/etc/src_escapes", + DestPath: "${NOMAD_SECRETS_DIR}/dst", + }, + }, + } + }, + Err: sourceEscapesErr, + }, + { + Name: "RawExecDstAbsoluteOk", + Config: func() *TaskTemplateManagerConfig { + return &TaskTemplateManagerConfig{ + ClientConfig: clientConf, + TaskDir: taskDir.Dir, + EnvBuilder: rawExecEnv(), + Templates: []*structs.Template{ + { + SourcePath: "${NOMAD_TASK_DIR}/src", + DestPath: "/etc/absolutely_relative", + }, + }, + } + }, + SourcePath: filepath.Join(taskDir.Dir, "local/src"), + DestPath: filepath.Join(taskDir.Dir, "etc/absolutely_relative"), + }, + } + + for i := range cases { + tc := cases[i] + t.Run(tc.Name, func(t *testing.T) { + if tc.Skip { + t.Skip("FIXME: Skipping broken test") + } + config := tc.Config() + mapping, err := parseTemplateConfigs(config) + if tc.Err == nil { + // Ok path + require.NoError(t, err) + require.NotNil(t, mapping) + require.Len(t, mapping, 1) + for k := range mapping { + require.Equal(t, tc.SourcePath, *k.Source) + require.Equal(t, tc.DestPath, *k.Destination) + t.Logf("Rendering %s => %s", *k.Source, *k.Destination) + } + } else { + // Err path + assert.EqualError(t, err, tc.Err.Error()) + require.Nil(t, mapping) + } + + }) + } +} + func TestTaskTemplateManager_BlockedEvents(t *testing.T) { t.Parallel() require := require.New(t)