diff --git a/internal/acl/acl.go b/internal/acl/acl.go index a1f0d92..d1b1bdb 100644 --- a/internal/acl/acl.go +++ b/internal/acl/acl.go @@ -86,7 +86,7 @@ func UserFromContext(ctx context.Context, cfg *config.Config) *AuthenticatedUser buildUserAcls(cfg, ret) log.WithFields(log.Fields{ - "username": ret.Username, + "username": ret.Username, "usergroup": ret.Usergroup, }).Infof("UserFromContext") diff --git a/internal/config/config.go b/internal/config/config.go index 6079ca3..3bd5daf 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -15,12 +15,12 @@ type Action struct { // ActionArgument objects appear on Actions. type ActionArgument struct { - Name string - Title string + Name string + Title string Description string - Type string - Default string - Choices []ActionArgumentChoice + Type string + Default string + Choices []ActionArgumentChoice } // ActionArgumentChoice represents a predefined choice for an argument. diff --git a/internal/executor/arguments.go b/internal/executor/arguments.go new file mode 100644 index 0000000..da97ebd --- /dev/null +++ b/internal/executor/arguments.go @@ -0,0 +1,121 @@ +package executor + +import ( + config "github.com/OliveTin/OliveTin/internal/config" + log "github.com/sirupsen/logrus" + + "errors" + "net/url" + "regexp" + "strings" +) + +var ( + typecheckRegex = map[string]string{ + "very_dangerous_raw_string": "", + "int": "^[\\d]+$", + "ascii": "^[a-zA-Z0-9]+$", + "ascii_identifier": "^[a-zA-Z0-9\\-\\.\\_]+$", + "ascii_sentence": "^[a-zA-Z0-9 \\,\\.]+$", + } +) + +func parseActionArguments(rawShellCommand string, values map[string]string, action *config.Action) (string, error) { + log.WithFields(log.Fields{ + "cmd": rawShellCommand, + }).Infof("Before Parse Args") + + r := regexp.MustCompile("{{ *?([a-zA-Z0-9_]+?) *?}}") + matches := r.FindAllStringSubmatch(rawShellCommand, -1) + + for _, match := range matches { + argValue, argProvided := values[match[1]] + + if !argProvided { + log.Infof("%v", values) + return "", errors.New("Required arg not provided: " + match[1]) + } + + err := typecheckActionArgument(match[1], argValue, action) + + if err != nil { + return "", err + } + + log.WithFields(log.Fields{ + "name": match[1], + "value": argValue, + }).Debugf("Arg assigned") + + rawShellCommand = strings.ReplaceAll(rawShellCommand, match[0], argValue) + } + + log.WithFields(log.Fields{ + "cmd": rawShellCommand, + }).Infof("After Parse Args") + + return rawShellCommand, nil +} + +func typecheckActionArgument(name string, value string, action *config.Action) error { + arg := action.FindArg(name) + + if arg == nil { + return errors.New("Action arg not defined: " + name) + } + + if len(arg.Choices) > 0 { + return typecheckChoice(value, arg) + } + + return TypeSafetyCheck(name, value, arg.Type) +} + +func typecheckChoice(value string, arg *config.ActionArgument) error { + for _, choice := range arg.Choices { + if value == choice.Value { + return nil + } + } + + return errors.New("argument value is not one of the predefined choices") +} + +// TypeSafetyCheck checks argument values match a specific type. The types are +// defined in typecheckRegex, and, you guessed it, uses regex to check for allowed +// characters. +func TypeSafetyCheck(name string, value string, argumentType string) error { + if argumentType == "url" { + return typeSafetyCheckUrl(name, value) + } + + return typeSafetyCheckRegex(name, value, argumentType) +} + +func typeSafetyCheckRegex(name string, value string, argumentType string) error { + pattern, found := typecheckRegex[argumentType] + + if !found { + return errors.New("argument type not implemented " + argumentType) + } + + matches, _ := regexp.MatchString(pattern, value) + + if !matches { + log.WithFields(log.Fields{ + "name": name, + "value": value, + "type": argumentType, + }).Warn("Arg type check safety failure") + + return errors.New("invalid argument, doesn't match " + argumentType) + } + + return nil +} + +func typeSafetyCheckUrl(name string, value string) error { + _, err := url.ParseRequestURI(value) + + return err +} diff --git a/internal/executor/arguments_test.go b/internal/executor/arguments_test.go new file mode 100644 index 0000000..7e2f8a6 --- /dev/null +++ b/internal/executor/arguments_test.go @@ -0,0 +1,69 @@ +package executor + +import ( + config "github.com/OliveTin/OliveTin/internal/config" + + "github.com/stretchr/testify/assert" + "testing" +) + +func TestSanitizeUnsafe(t *testing.T) { + assert.Nil(t, TypeSafetyCheck("", "_zomg_ c:/ haxxor ' bobby tables && rm -rf ", "very_dangerous_raw_string")) +} + +func TestSanitizeUnimplemented(t *testing.T) { + err := TypeSafetyCheck("", "I am a happy little argument", "greeting_type") + + assert.NotNil(t, err, "Test an argument type that does not exist") +} + +func TestArgumentNameNumbers(t *testing.T) { + a1 := config.Action{ + Title: "Do some tickles", + Shell: "echo 'Tickling {{ person1name }}'", + Arguments: []config.ActionArgument{ + { + Name: "person1name", + Type: "ascii", + }, + }, + } + + values := map[string]string{ + "person1name": "Fred", + } + + out, err := parseActionArguments(a1.Shell, values, &a1) + + assert.Equal(t, "echo 'Tickling Fred'", out) + assert.Nil(t, err) +} + +func TestArgumentNotProvided(t *testing.T) { + a1 := config.Action{ + Title: "Do some tickles", + Shell: "echo 'Tickling {{ personName }}'", + Arguments: []config.ActionArgument{ + { + Name: "person", + Type: "ascii", + }, + }, + } + + values := map[string]string{} + + out, err := parseActionArguments(a1.Shell, values, &a1) + + assert.Equal(t, "", out) + assert.Equal(t, err.Error(), "Required arg not provided: personName") +} + +func TestTypeSafetyCheckUrl(t *testing.T) { + assert.Nil(t, TypeSafetyCheck("test1", "http://google.com", "url"), "Test URL: google.com") + assert.Nil(t, TypeSafetyCheck("test2", "http://technowax.net:80?foo=bar", "url"), "Test URL: technowax.net with query arguments") + assert.Nil(t, TypeSafetyCheck("test3", "http://localhost:80?foo=bar", "url"), "Test URL: localhost with query arguments") + assert.NotNil(t, TypeSafetyCheck("test4", "http://lo host:80", "url"), "Test a badly formed URL") + assert.NotNil(t, TypeSafetyCheck("test5", "12345", "url"), "Test a badly formed URL") + assert.NotNil(t, TypeSafetyCheck("test6", "_!23;", "url"), "Test a badly formed URL") +} diff --git a/internal/executor/executor.go b/internal/executor/executor.go index 02df5ea..a233a11 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -8,23 +8,21 @@ import ( "bytes" "context" - "errors" "os/exec" - "net/url" - "regexp" - "strings" "time" ) -var ( - typecheckRegex = map[string]string{ - "very_dangerous_raw_string": "", - "int": "^[\\d]+$", - "ascii": "^[a-zA-Z0-9]+$", - "ascii_identifier": "^[a-zA-Z0-9\\-\\.\\_]+$", - "ascii_sentence": "^[a-zA-Z0-9 \\,\\.]+$", - } -) +// ExecutionRequest is a request to execute an action. It's passed to an +// Executor. They're created from the grpcapi. +type ExecutionRequest struct { + ActionName string + Arguments map[string]string + action *config.Action + Cfg *config.Config + AuthenticatedUser *acl.AuthenticatedUser + logEntry *InternalLogEntry + finalParsedCommand string +} // InternalLogEntry objects are created by an Executor, and represent the final // state of execution (even if the command is not executed). It's designed to be @@ -45,71 +43,14 @@ type InternalLogEntry struct { ActionIcon string } -// ExecutionRequest is a request to execute an action. It's passed to an -// Executor. They're created from the grpcapi. -type ExecutionRequest struct { - ActionName string - Arguments map[string]string - action *config.Action - Cfg *config.Config - AuthenticatedUser *acl.AuthenticatedUser - logEntry *InternalLogEntry - finalParsedCommand string -} - -type executorStep interface { - Exec(*ExecutionRequest) bool -} +type executorStepFunc func(*ExecutionRequest) bool // Executor represents a helper class for executing commands. It's main method // is ExecRequest type Executor struct { Logs []InternalLogEntry - chainOfCommand []executorStep -} - -// DefaultExecutor returns an Executor, with a sensible "chain of command" for -// executing actions. -func DefaultExecutor() *Executor { - e := Executor{} - e.chainOfCommand = []executorStep{ - stepFindAction{}, - stepACLCheck{}, - stepParseArgs{}, - stepLogStart{}, - stepExec{}, - stepLogFinish{}, - } - - return &e -} - -type stepFindAction struct{} - -func (s stepFindAction) Exec(req *ExecutionRequest) bool { - actualAction := req.Cfg.FindAction(req.ActionName) - - if actualAction == nil { - log.WithFields(log.Fields{ - "actionName": req.ActionName, - }).Warnf("Action not found") - - req.logEntry.Stderr = "Action not found" - - return false - } - - req.action = actualAction - req.logEntry.ActionIcon = actualAction.Icon - - return true -} - -type stepACLCheck struct{} - -func (s stepACLCheck) Exec(req *ExecutionRequest) bool { - return acl.IsAllowedExec(req.Cfg, req.AuthenticatedUser, req.action) + chainOfCommand []executorStepFunc } // ExecRequest processes an ExecutionRequest @@ -123,7 +64,7 @@ func (e *Executor) ExecRequest(req *ExecutionRequest) *pb.StartActionResponse { } for _, step := range e.chainOfCommand { - if !step.Exec(req) { + if !step(req) { break } } @@ -143,34 +84,46 @@ func (e *Executor) ExecRequest(req *ExecutionRequest) *pb.StartActionResponse { } } -type stepLogStart struct{} +// DefaultExecutor returns an Executor, with a sensible "chain of command" for +// executing actions. +func DefaultExecutor() *Executor { + e := Executor{} + e.chainOfCommand = []executorStepFunc{ + stepFindAction, + stepACLCheck, + stepParseArgs, + stepLogStart, + stepExec, + stepLogFinish, + } -func (e stepLogStart) Exec(req *ExecutionRequest) bool { - log.WithFields(log.Fields{ - "title": req.action.Title, - "timeout": req.action.Timeout, - }).Infof("Action starting") + return &e +} + +func stepFindAction(req *ExecutionRequest) bool { + actualAction := req.Cfg.FindAction(req.ActionName) + + if actualAction == nil { + log.WithFields(log.Fields{ + "actionName": req.ActionName, + }).Warnf("Action not found") + + req.logEntry.Stderr = "Action not found" + + return false + } + + req.action = actualAction + req.logEntry.ActionIcon = actualAction.Icon return true } -type stepLogFinish struct{} - -func (e stepLogFinish) Exec(req *ExecutionRequest) bool { - log.WithFields(log.Fields{ - "title": req.action.Title, - "stdout": req.logEntry.Stdout, - "stderr": req.logEntry.Stderr, - "timedOut": req.logEntry.TimedOut, - "exit": req.logEntry.ExitCode, - }).Infof("Action finished") - - return true +func stepACLCheck(req *ExecutionRequest) bool { + return acl.IsAllowedExec(req.Cfg, req.AuthenticatedUser, req.action) } -type stepParseArgs struct{} - -func (e stepParseArgs) Exec(req *ExecutionRequest) bool { +func stepParseArgs(req *ExecutionRequest) bool { var err error req.finalParsedCommand, err = parseActionArguments(req.action.Shell, req.Arguments, req.action) @@ -186,9 +139,28 @@ func (e stepParseArgs) Exec(req *ExecutionRequest) bool { return true } -type stepExec struct{} +func stepLogStart(req *ExecutionRequest) bool { + log.WithFields(log.Fields{ + "title": req.action.Title, + "timeout": req.action.Timeout, + }).Infof("Action starting") -func (e stepExec) Exec(req *ExecutionRequest) bool { + return true +} + +func stepLogFinish(req *ExecutionRequest) bool { + log.WithFields(log.Fields{ + "title": req.action.Title, + "stdout": req.logEntry.Stdout, + "stderr": req.logEntry.Stderr, + "timedOut": req.logEntry.TimedOut, + "exit": req.logEntry.ExitCode, + }).Infof("Action finished") + + return true +} + +func stepExec(req *ExecutionRequest) bool { ctx, cancel := context.WithTimeout(context.Background(), time.Duration(req.action.Timeout)*time.Second) defer cancel() @@ -215,103 +187,3 @@ func (e stepExec) Exec(req *ExecutionRequest) bool { return true } - -func parseActionArguments(rawShellCommand string, values map[string]string, action *config.Action) (string, error) { - log.WithFields(log.Fields{ - "cmd": rawShellCommand, - }).Infof("Before Parse Args") - - r := regexp.MustCompile("{{ *?([a-zA-Z0-9_]+?) *?}}") - matches := r.FindAllStringSubmatch(rawShellCommand, -1) - - for _, match := range matches { - argValue, argProvided := values[match[1]] - - if !argProvided { - log.Infof("%v", values) - return "", errors.New("Required arg not provided: " + match[1]) - } - - err := typecheckActionArgument(match[1], argValue, action) - - if err != nil { - return "", err - } - - log.WithFields(log.Fields{ - "name": match[1], - "value": argValue, - }).Debugf("Arg assigned") - - rawShellCommand = strings.ReplaceAll(rawShellCommand, match[0], argValue) - } - - log.WithFields(log.Fields{ - "cmd": rawShellCommand, - }).Infof("After Parse Args") - - return rawShellCommand, nil -} - -func typecheckActionArgument(name string, value string, action *config.Action) error { - arg := action.FindArg(name) - - if arg == nil { - return errors.New("Action arg not defined: " + name) - } - - if len(arg.Choices) > 0 { - return typecheckChoice(value, arg) - } - - return TypeSafetyCheck(name, value, arg.Type) -} - -func typecheckChoice(value string, arg *config.ActionArgument) error { - for _, choice := range arg.Choices { - if value == choice.Value { - return nil - } - } - - return errors.New("argument value is not one of the predefined choices") -} - -// TypeSafetyCheck checks argument values match a specific type. The types are -// defined in typecheckRegex, and, you guessed it, uses regex to check for allowed -// characters. -func TypeSafetyCheck(name string, value string, argumentType string) error { - if argumentType == "url" { - return typeSafetyCheckUrl(name, value) - } - - return typeSafetyCheckRegex(name, value, argumentType) -} - -func typeSafetyCheckRegex(name string, value string, argumentType string) error { - pattern, found := typecheckRegex[argumentType] - - if !found { - return errors.New("argument type not implemented " + argumentType) - } - - matches, _ := regexp.MatchString(pattern, value) - - if !matches { - log.WithFields(log.Fields{ - "name": name, - "value": value, - "type": argumentType, - }).Warn("Arg type check safety failure") - - return errors.New("invalid argument, doesn't match " + argumentType) - } - - return nil -} - -func typeSafetyCheckUrl(name string, value string) error { - _, err := url.ParseRequestURI(value) - - return err -} diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go index b9e76d6..e850d79 100644 --- a/internal/executor/executor_test.go +++ b/internal/executor/executor_test.go @@ -8,17 +8,6 @@ import ( config "github.com/OliveTin/OliveTin/internal/config" ) -func TestSanitizeUnsafe(t *testing.T) { - assert.Nil(t, TypeSafetyCheck("", "_zomg_ c:/ haxxor ' bobby tables && rm -rf ", "very_dangerous_raw_string")) -} - -func TestSanitizeUnimplemented(t *testing.T) { - err := TypeSafetyCheck("", "I am a happy little argument", "greeting_type") - - assert.NotNil(t, err, "Test an argument type that does not exist") -} - - func testingExecutor() (*Executor, *config.Config) { e := DefaultExecutor() @@ -119,54 +108,3 @@ func TestArgumentNameSnakeCase(t *testing.T) { assert.Equal(t, "echo 'Tickling Fred'", out) assert.Nil(t, err) } - -func TestArgumentNameNumbers(t *testing.T) { - a1 := config.Action{ - Title: "Do some tickles", - Shell: "echo 'Tickling {{ person1name }}'", - Arguments: []config.ActionArgument{ - { - Name: "person1name", - Type: "ascii", - }, - }, - } - - values := map[string]string{ - "person1name": "Fred", - } - - out, err := parseActionArguments(a1.Shell, values, &a1) - - assert.Equal(t, "echo 'Tickling Fred'", out) - assert.Nil(t, err) -} - -func TestArgumentNotProvided(t *testing.T) { - a1 := config.Action{ - Title: "Do some tickles", - Shell: "echo 'Tickling {{ personName }}'", - Arguments: []config.ActionArgument{ - { - Name: "person", - Type: "ascii", - }, - }, - } - - values := map[string]string{} - - out, err := parseActionArguments(a1.Shell, values, &a1) - - assert.Equal(t, "", out) - assert.Equal(t, err.Error(), "Required arg not provided: personName") -} - -func TestTypeSafetyCheckUrl(t *testing.T) { - assert.Nil(t, TypeSafetyCheck("test1", "http://google.com", "url"), "Test URL: google.com") - assert.Nil(t, TypeSafetyCheck("test2", "http://technowax.net:80?foo=bar", "url"), "Test URL: technowax.net with query arguments") - assert.Nil(t, TypeSafetyCheck("test3", "http://localhost:80?foo=bar", "url"), "Test URL: localhost with query arguments") - assert.NotNil(t, TypeSafetyCheck("test4", "http://lo host:80", "url"), "Test a badly formed URL") - assert.NotNil(t, TypeSafetyCheck("test5", "12345", "url"), "Test a badly formed URL") - assert.NotNil(t, TypeSafetyCheck("test6", "_!23;", "url"), "Test a badly formed URL") -}