diff --git a/api/tasks.go b/api/tasks.go index 3c18269ea..f357c78db 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -158,6 +158,7 @@ type TaskState struct { const ( TaskDriverFailure = "Driver Failure" TaskReceived = "Received" + TaskFailedValidation = "Failed Validation" TaskStarted = "Started" TaskTerminated = "Terminated" TaskKilled = "Killed" @@ -170,13 +171,14 @@ const ( // TaskEvent is an event that effects the state of a task and contains meta-data // appropriate to the events type. type TaskEvent struct { - Type string - Time int64 - DriverError string - ExitCode int - Signal int - Message string - KillError string - StartDelay int64 - DownloadError string + Type string + Time int64 + DriverError string + ExitCode int + Signal int + Message string + KillError string + StartDelay int64 + DownloadError string + ValidationError string } diff --git a/client/config/config.go b/client/config/config.go index 702d9a5e2..f5a0a6b0b 100644 --- a/client/config/config.go +++ b/client/config/config.go @@ -20,6 +20,22 @@ var ( "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", "AWS_SESSION_TOKEN", "GOOGLE_APPLICATION_CREDENTIALS", }, ",") + + // DefaulUserBlacklist is the default set of users that tasks are not + // allowed to run as when using a driver in "user.checked_drivers" + DefaultUserBlacklist = strings.Join([]string{ + "root", + "Administrator", + }, ",") + + // DefaultUserCheckedDrivers is the set of drivers we apply the user + // blacklist onto. For virtualized drivers it often doesn't make sense to + // make this stipulation so by default they are ignored. + DefaultUserCheckedDrivers = strings.Join([]string{ + "exec", + "qemu", + "java", + }, ",") ) // RPCHandler can be provided to the Client if there is a local server @@ -105,21 +121,17 @@ func (c *Config) Copy() *Config { // Read returns the specified configuration value or "". func (c *Config) Read(id string) string { - val, ok := c.Options[id] - if !ok { - return "" - } - return val + return c.Options[id] } // ReadDefault returns the specified configuration value, or the specified // default value if none is set. func (c *Config) ReadDefault(id string, defaultValue string) string { - val := c.Read(id) - if val != "" { - return val + val, ok := c.Options[id] + if !ok { + return defaultValue } - return defaultValue + return val } // ReadBool parses the specified option as a boolean. @@ -158,3 +170,21 @@ func (c *Config) ReadStringListToMap(key string) map[string]struct{} { } return list } + +// ReadStringListToMap tries to parse the specified option as a comma seperated list. +// If there is an error in parsing, an empty list is returned. +func (c *Config) ReadStringListToMapDefault(key, defaultValue string) map[string]struct{} { + val, ok := c.Options[key] + if !ok { + val = defaultValue + } + + list := make(map[string]struct{}) + if val != "" { + for _, e := range strings.Split(val, ",") { + trimmed := strings.TrimSpace(e) + list[trimmed] = struct{}{} + } + } + return list +} diff --git a/client/task_runner.go b/client/task_runner.go index a2d25ad22..6689c7903 100644 --- a/client/task_runner.go +++ b/client/task_runner.go @@ -214,10 +214,49 @@ func (r *TaskRunner) Run() { r.logger.Printf("[DEBUG] client: starting task context for '%s' (alloc '%s')", r.task.Name, r.alloc.ID) + if err := r.validateTask(); err != nil { + r.setState( + structs.TaskStateDead, + structs.NewTaskEvent(structs.TaskFailedValidation).SetValidationError(err)) + return + } + r.run() return } +// validateTask validates the fields of the task and returns an error if the +// task is invalid. +func (r *TaskRunner) validateTask() error { + var mErr multierror.Error + + // Validate the user. + unallowedUsers := r.config.ReadStringListToMapDefault("user.blacklist", config.DefaultUserBlacklist) + checkDrivers := r.config.ReadStringListToMapDefault("user.checked_drivers", config.DefaultUserCheckedDrivers) + if _, driverMatch := checkDrivers[r.task.Driver]; driverMatch { + if _, unallowed := unallowedUsers[r.task.User]; unallowed { + mErr.Errors = append(mErr.Errors, fmt.Errorf("running as user %q is disallowed", r.task.User)) + } + } + + // Validate the artifacts + for i, artifact := range r.task.Artifacts { + // Verify the artifact doesn't escape the task directory. + if err := artifact.Validate(); err != nil { + // If this error occurs there is potentially a server bug or + // mallicious, server spoofing. + r.logger.Printf("[ERR] client: allocation %q, task %v, artifact %#v (%v) fails validation: %v", + r.alloc.ID, r.task.Name, artifact, i, err) + mErr.Errors = append(mErr.Errors, fmt.Errorf("artifact (%d) failed validation: %v", i, err)) + } + } + + if len(mErr.Errors) == 1 { + return mErr.Errors[0] + } + return mErr.ErrorOrNil() +} + func (r *TaskRunner) run() { // Predeclare things so we an jump to the RESTART var handleEmpty bool @@ -236,18 +275,7 @@ func (r *TaskRunner) run() { return } - for i, artifact := range r.task.Artifacts { - // Verify the artifact doesn't escape the task directory. - if err := artifact.Validate(); err != nil { - // If this error occurs there is potentially a server bug or - // mallicious, server spoofing. - r.setState(structs.TaskStateDead, - structs.NewTaskEvent(structs.TaskArtifactDownloadFailed).SetDownloadError(err)) - r.logger.Printf("[ERR] client: allocation %q, task %v, artifact %#v (%v) fails validation: %v", - r.alloc.ID, r.task.Name, artifact, i, err) - return - } - + for _, artifact := range r.task.Artifacts { if err := getter.GetArtifact(artifact, taskDir, r.logger); err != nil { r.setState(structs.TaskStateDead, structs.NewTaskEvent(structs.TaskArtifactDownloadFailed).SetDownloadError(err)) diff --git a/client/task_runner_test.go b/client/task_runner_test.go index 309d77daf..908834fcc 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -363,3 +363,29 @@ func TestTaskRunner_Download_Retries(t *testing.T) { t.Fatalf("Seventh Event was %v; want %v", upd.events[6].Type, structs.TaskNotRestarting) } } + +func TestTaskRunner_Validate_UserEnforcement(t *testing.T) { + _, tr := testTaskRunner(false) + + // Try to run as root with exec. + tr.task.Driver = "exec" + tr.task.User = "root" + if err := tr.validateTask(); err == nil { + t.Fatalf("expected error running as root with exec") + } + + // Try to run a non-blacklisted user with exec. + tr.task.Driver = "exec" + tr.task.User = "foobar" + if err := tr.validateTask(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Try to run as root with docker. + tr.task.Driver = "docker" + tr.task.User = "root" + if err := tr.validateTask(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + +} diff --git a/command/alloc_status.go b/command/alloc_status.go index c41eb3c05..ead7ab454 100644 --- a/command/alloc_status.go +++ b/command/alloc_status.go @@ -201,6 +201,12 @@ func (c *AllocStatusCommand) taskStatus(alloc *api.Allocation) { desc = "Task started by client" case api.TaskReceived: desc = "Task received by client" + case api.TaskFailedValidation: + if event.ValidationError != "" { + desc = event.ValidationError + } else { + desc = "Validation of task failed" + } case api.TaskDriverFailure: if event.DriverError != "" { desc = event.DriverError diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 1b8c67018..0904afcdb 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1794,26 +1794,35 @@ func (ts *TaskState) Failed() bool { return false } - return ts.Events[l-1].Type == TaskNotRestarting + switch ts.Events[l-1].Type { + case TaskNotRestarting, TaskArtifactDownloadFailed, TaskFailedValidation: + return true + default: + return false + } } const ( - // A Driver failure indicates that the task could not be started due to a + // TaskDriveFailure indicates that the task could not be started due to a // failure in the driver. TaskDriverFailure = "Driver Failure" - // Task Received signals that the task has been pulled by the client at the + // TaskReceived signals that the task has been pulled by the client at the // given timestamp. TaskReceived = "Received" - // Task Started signals that the task was started and its timestamp can be + // TaskFailedValidation indicates the task was invalid and as such was not + // run. + TaskFailedValidation = "Failed Validation" + + // TaskStarted signals that the task was started and its timestamp can be // used to determine the running length of the task. TaskStarted = "Started" - // Task terminated indicates that the task was started and exited. + // TaskTerminated indicates that the task was started and exited. TaskTerminated = "Terminated" - // Task Killed indicates a user has killed the task. + // TaskKilled indicates a user has killed the task. TaskKilled = "Killed" // TaskRestarting indicates that task terminated and is being restarted. @@ -1823,7 +1832,7 @@ const ( // restarted because it has exceeded its restart policy. TaskNotRestarting = "Restarts Exceeded" - // Task Downloading Artifacts means the task is downloading the artifacts + // TaskDownloadingArtifacts means the task is downloading the artifacts // specified in the task. TaskDownloadingArtifacts = "Downloading Artifacts" @@ -1854,6 +1863,9 @@ type TaskEvent struct { // Artifact Download fields DownloadError string // Error downloading artifacts + + // Validation fields + ValidationError string // Validation error } func (te *TaskEvent) GoString() string { @@ -1919,6 +1931,13 @@ func (e *TaskEvent) SetDownloadError(err error) *TaskEvent { return e } +func (e *TaskEvent) SetValidationError(err error) *TaskEvent { + if err != nil { + e.ValidationError = err.Error() + } + return e +} + // TaskArtifact is an artifact to download before running the task. type TaskArtifact struct { // GetterSource is the source to download an artifact using go-getter diff --git a/website/source/docs/agent/config.html.md b/website/source/docs/agent/config.html.md index 9ceed03fe..041bf31c8 100644 --- a/website/source/docs/agent/config.html.md +++ b/website/source/docs/agent/config.html.md @@ -381,6 +381,21 @@ documentation [here](/docs/drivers/index.html) * `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, `AWS_SESSION_TOKEN` * `GOOGLE_APPLICATION_CREDENTIALS` +* `user.blacklist`: An operator specifiable blacklist of users which a task is + not allowed to run as when using a driver in `user.checked_drivers`. + Defaults to: + + * `root` + * `Administrator` + +* `user.checked_drivers`: An operator specifiable list of drivers to enforce + the the `user.blacklist`. For drivers using containers, this enforcement often + doesn't make sense and as such the default is set to: + + * `exec` + * `qemu` + * `java` + * `fingerprint.whitelist`: A comma separated list of whitelisted fingerprinters. If specified, fingerprinters not in the whitelist will be disabled. If the whitelist is empty, all fingerprinters are used.