From 9a63d6103d2449e34ba4b50ab11bfe27fcec441d Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Wed, 18 Jul 2018 16:51:50 -0700 Subject: [PATCH] tr: add validate task hook --- .../taskrunner/task_runner_test.go | 1 + .../taskrunner/task_runner_hooks.go | 1 + .../allocrunnerv2/taskrunner/validate_hook.go | 66 +++++++++++++++++++ .../taskrunner/validate_hook_test.go | 63 ++++++++++++++++++ client/driver/env/env.go | 3 +- 5 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 client/allocrunnerv2/taskrunner/validate_hook.go create mode 100644 client/allocrunnerv2/taskrunner/validate_hook_test.go diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index 3503d3ee5..68e082616 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -664,6 +664,7 @@ func TestTaskRunner_UnregisterConsul_Retries(t *testing.T) { } } +//XXX Ported to allocrunnerv2/task_runner/validate_hook_test.go func TestTaskRunner_Validate_UserEnforcement(t *testing.T) { t.Parallel() ctx := testTaskRunner(t, false) diff --git a/client/allocrunnerv2/taskrunner/task_runner_hooks.go b/client/allocrunnerv2/taskrunner/task_runner_hooks.go index 9affd2231..35ebfd474 100644 --- a/client/allocrunnerv2/taskrunner/task_runner_hooks.go +++ b/client/allocrunnerv2/taskrunner/task_runner_hooks.go @@ -18,6 +18,7 @@ func (tr *TaskRunner) initHooks() { // Create the task directory hook. This is run first to ensure the // directoy path exists for other hooks. tr.runnerHooks = []interfaces.TaskHook{ + newValidateHook(tr.clientConfig, hookLogger), newTaskDirHook(tr, hookLogger), newArtifactHook(tr, hookLogger), newShutdownDelayHook(task.ShutdownDelay, hookLogger), diff --git a/client/allocrunnerv2/taskrunner/validate_hook.go b/client/allocrunnerv2/taskrunner/validate_hook.go new file mode 100644 index 000000000..c17544875 --- /dev/null +++ b/client/allocrunnerv2/taskrunner/validate_hook.go @@ -0,0 +1,66 @@ +package taskrunner + +import ( + "context" + "fmt" + + log "github.com/hashicorp/go-hclog" + multierror "github.com/hashicorp/go-multierror" + "github.com/hashicorp/nomad/client/allocrunnerv2/interfaces" + "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/client/driver/env" + "github.com/hashicorp/nomad/nomad/structs" +) + +// validateHook validates the task is able to be run. +type validateHook struct { + config *config.Config + logger log.Logger +} + +func newValidateHook(config *config.Config, logger log.Logger) *validateHook { + h := &validateHook{ + config: config, + } + h.logger = logger.Named(h.Name()) + return h +} + +func (*validateHook) Name() string { + return "validate" +} + +func (h *validateHook) Prestart(ctx context.Context, req *interfaces.TaskPrestartRequest, resp *interfaces.TaskPrestartResponse) error { + if err := validateTask(req.Task, req.TaskEnv, h.config); err != nil { + return err + } + + resp.Done = true + return nil +} + +func validateTask(task *structs.Task, taskEnv *env.TaskEnv, conf *config.Config) error { + var mErr multierror.Error + + // Validate the user + unallowedUsers := conf.ReadStringListToMapDefault("user.blacklist", config.DefaultUserBlacklist) + checkDrivers := conf.ReadStringListToMapDefault("user.checked_drivers", config.DefaultUserCheckedDrivers) + if _, driverMatch := checkDrivers[task.Driver]; driverMatch { + if _, unallowed := unallowedUsers[task.User]; unallowed { + mErr.Errors = append(mErr.Errors, fmt.Errorf("running as user %q is disallowed", task.User)) + } + } + + // Validate the Service names once they're interpolated + for i, service := range task.Services { + name := taskEnv.ReplaceEnv(service.Name) + if err := service.ValidateName(name); err != nil { + mErr.Errors = append(mErr.Errors, fmt.Errorf("service (%d) failed validation: %v", i, err)) + } + } + + if len(mErr.Errors) == 1 { + return mErr.Errors[0] + } + return mErr.ErrorOrNil() +} diff --git a/client/allocrunnerv2/taskrunner/validate_hook_test.go b/client/allocrunnerv2/taskrunner/validate_hook_test.go new file mode 100644 index 000000000..823f6cdcb --- /dev/null +++ b/client/allocrunnerv2/taskrunner/validate_hook_test.go @@ -0,0 +1,63 @@ +package taskrunner + +import ( + "testing" + + "github.com/hashicorp/nomad/client/config" + "github.com/hashicorp/nomad/client/driver/env" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/stretchr/testify/require" +) + +func TestTaskRunner_Validate_UserEnforcement(t *testing.T) { + t.Parallel() + + taskEnv := env.NewEmptyBuilder().Build() + conf := config.DefaultConfig() + + // Try to run as root with exec. + task := &structs.Task{ + Driver: "exec", + User: "root", + } + if err := validateTask(task, taskEnv, conf); err == nil { + t.Fatalf("expected error running as root with exec") + } + + // Try to run a non-blacklisted user with exec. + task.User = "foobar" + require.NoError(t, validateTask(task, taskEnv, conf)) + + // Try to run as root with docker. + task.Driver = "docker" + task.User = "root" + require.NoError(t, validateTask(task, taskEnv, conf)) +} + +func TestTaskRunner_Validate_ServiceName(t *testing.T) { + t.Parallel() + + builder := env.NewEmptyBuilder() + conf := config.DefaultConfig() + + // Create a task with a service for validation + task := &structs.Task{ + Services: []*structs.Service{ + { + Name: "ok", + }, + }, + } + + require.NoError(t, validateTask(task, builder.Build(), conf)) + + // Add an env var that should validate + builder.SetGenericEnv(map[string]string{"FOO": "bar"}) + task.Services[0].Name = "${FOO}" + require.NoError(t, validateTask(task, builder.Build(), conf)) + + // Add an env var that should *not* validate + builder.SetGenericEnv(map[string]string{"BAD": "invalid/in/consul"}) + task.Services[0].Name = "${BAD}" + require.Error(t, validateTask(task, builder.Build(), conf)) +} diff --git a/client/driver/env/env.go b/client/driver/env/env.go index ecd4ea2c8..013623168 100644 --- a/client/driver/env/env.go +++ b/client/driver/env/env.go @@ -247,7 +247,8 @@ func NewBuilder(node *structs.Node, alloc *structs.Allocation, task *structs.Tas // NewEmptyBuilder creates a new environment builder. func NewEmptyBuilder() *Builder { return &Builder{ - mu: &sync.RWMutex{}, + mu: &sync.RWMutex{}, + envvars: make(map[string]string), } }