Skip to content

Commit

Permalink
Merge pull request #978 from hashicorp/f-enforce-user
Browse files Browse the repository at this point in the history
Operator specifiable blacklist for task's using certain users
  • Loading branch information
dadgar committed Mar 25, 2016
2 parents 9e7d0d9 + a1ff98a commit aaff02c
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 37 deletions.
20 changes: 11 additions & 9 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ type TaskState struct {
const (
TaskDriverFailure = "Driver Failure"
TaskReceived = "Received"
TaskFailedValidation = "Failed Validation"
TaskStarted = "Started"
TaskTerminated = "Terminated"
TaskKilled = "Killed"
Expand All @@ -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
}
48 changes: 39 additions & 9 deletions client/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
52 changes: 40 additions & 12 deletions client/task_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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))
Expand Down
26 changes: 26 additions & 0 deletions client/task_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

}
6 changes: 6 additions & 0 deletions command/alloc_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 26 additions & 7 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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"

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions website/source/docs/agent/config.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit aaff02c

Please sign in to comment.