Skip to content

Commit

Permalink
EventManager: allow to define the allowed system commands
Browse files Browse the repository at this point in the history
Signed-off-by: Nicola Murino <[email protected]>
  • Loading branch information
drakkan committed Nov 1, 2024
1 parent 60558de commit 88b1850
Show file tree
Hide file tree
Showing 10 changed files with 259 additions and 17 deletions.
25 changes: 24 additions & 1 deletion internal/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ func Initialize(c Configuration, isShared int) error {
if err := c.initializeProxyProtocol(); err != nil {
return err
}
if err := c.EventManager.validate(); err != nil {
return err
}
vfs.SetTempPath(c.TempPath)
dataprovider.SetTempPath(c.TempPath)
vfs.SetAllowSelfConnections(c.AllowSelfConnections)
Expand All @@ -247,6 +250,7 @@ func Initialize(c Configuration, isShared int) error {
vfs.SetResumeMaxSize(c.ResumeMaxSize)
vfs.SetUploadMode(c.UploadMode)
dataprovider.SetAllowSelfConnections(c.AllowSelfConnections)
dataprovider.EnabledActionCommands = c.EventManager.EnabledCommands
transfersChecker = getTransfersChecker(isShared)
return nil
}
Expand Down Expand Up @@ -512,6 +516,23 @@ type ConnectionTransfer struct {
DLSize int64 `json:"-"`
}

// EventManagerConfig defines the configuration for the EventManager
type EventManagerConfig struct {
// EnabledCommands defines the system commands that can be executed via EventManager,
// an empty list means that any command is allowed to be executed.
// Commands must be set as an absolute path
EnabledCommands []string `json:"enabled_commands" mapstructure:"enabled_commands"`
}

func (c *EventManagerConfig) validate() error {
for _, c := range c.EnabledCommands {
if !filepath.IsAbs(c) {
return fmt.Errorf("invalid command %q: it must be an absolute path", c)
}
}
return nil
}

// MetadataConfig defines how to handle metadata for cloud storage backends
type MetadataConfig struct {
// If not zero the metadata will be read before downloads and will be
Expand Down Expand Up @@ -621,7 +642,9 @@ type Configuration struct {
// server's local time, otherwise UTC will be used.
TZ string `json:"tz" mapstructure:"tz"`
// Metadata configuration
Metadata MetadataConfig `json:"metadata" mapstructure:"metadata"`
Metadata MetadataConfig `json:"metadata" mapstructure:"metadata"`
// EventManager configuration
EventManager EventManagerConfig `json:"event_manager" mapstructure:"event_manager"`
idleTimeoutAsDuration time.Duration
idleLoginTimeout time.Duration
defender Defender
Expand Down
27 changes: 27 additions & 0 deletions internal/common/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,33 @@ func TestConnections(t *testing.T) {
Connections.RUnlock()
}

func TestEventManagerCommandsInitialization(t *testing.T) {
configCopy := Config

c := Configuration{
EventManager: EventManagerConfig{
EnabledCommands: []string{"ls"}, // not an absolute path
},
}
err := Initialize(c, 0)
assert.ErrorContains(t, err, "invalid command")

var commands []string
if runtime.GOOS == osWindows {
commands = []string{"C:\\command"}
} else {
commands = []string{"/bin/ls"}
}

c.EventManager.EnabledCommands = commands
err = Initialize(c, 0)
assert.NoError(t, err)
assert.Equal(t, commands, dataprovider.EnabledActionCommands)

dataprovider.EnabledActionCommands = configCopy.EventManager.EnabledCommands
Config = configCopy
}

func TestInitializationProxyErrors(t *testing.T) {
configCopy := Config

Expand Down
3 changes: 3 additions & 0 deletions internal/common/eventmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -1484,6 +1484,9 @@ func executeHTTPRuleAction(c dataprovider.EventActionHTTPConfig, params *EventPa
}

func executeCommandRuleAction(c dataprovider.EventActionCommandConfig, params *EventParams) error {
if !dataprovider.IsActionCommandAllowed(c.Cmd) {
return fmt.Errorf("command %q is not allowed", c.Cmd)
}
addObjectData := false
if params.Object != nil {
for _, k := range c.EnvVars {
Expand Down
142 changes: 142 additions & 0 deletions internal/common/protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4208,6 +4208,148 @@ func TestEventRuleStatues(t *testing.T) {
require.NoError(t, err)
}

func TestEventRuleDisabledCommand(t *testing.T) {
if runtime.GOOS == osWindows {
t.Skip("this test is not available on Windows")
}
smtpCfg := smtp.Config{
Host: "127.0.0.1",
Port: 2525,
From: "[email protected]",
TemplatesPath: "templates",
}
err := smtpCfg.Initialize(configDir, true)
require.NoError(t, err)

saveObjectScriptPath := filepath.Join(os.TempDir(), "provider.sh")
outPath := filepath.Join(os.TempDir(), "provider_out.json")
err = os.WriteFile(saveObjectScriptPath, getSaveProviderObjectScriptContent(outPath, 0), 0755)
assert.NoError(t, err)

a1 := dataprovider.BaseEventAction{
Name: "a1",
Type: dataprovider.ActionTypeCommand,
Options: dataprovider.BaseEventActionOptions{
CmdConfig: dataprovider.EventActionCommandConfig{
Cmd: saveObjectScriptPath,
Timeout: 10,
EnvVars: []dataprovider.KeyValue{
{
Key: "SFTPGO_OBJECT_DATA",
Value: "{{ObjectData}}",
},
},
},
},
}
a2 := dataprovider.BaseEventAction{
Name: "a2",
Type: dataprovider.ActionTypeEmail,
Options: dataprovider.BaseEventActionOptions{
EmailConfig: dataprovider.EventActionEmailConfig{
Recipients: []string{"[email protected]"},
Subject: `New "{{Event}}" from "{{Name}}"`,
Body: "Object name: {{ObjectName}} object type: {{ObjectType}} Data: {{ObjectData}}",
},
},
}

a3 := dataprovider.BaseEventAction{
Name: "a3",
Type: dataprovider.ActionTypeEmail,
Options: dataprovider.BaseEventActionOptions{
EmailConfig: dataprovider.EventActionEmailConfig{
Recipients: []string{"[email protected]"},
Subject: `Failed "{{Event}}" from "{{Name}}"`,
Body: "Object name: {{ObjectName}} object type: {{ObjectType}}, IP: {{IP}}",
},
},
}
action1, _, err := httpdtest.AddEventAction(a1, http.StatusCreated)
assert.NoError(t, err)
action2, _, err := httpdtest.AddEventAction(a2, http.StatusCreated)
assert.NoError(t, err)
action3, _, err := httpdtest.AddEventAction(a3, http.StatusCreated)
assert.NoError(t, err)

r := dataprovider.EventRule{
Name: "rule",
Status: 1,
Trigger: dataprovider.EventTriggerProviderEvent,
Conditions: dataprovider.EventConditions{
ProviderEvents: []string{"add"},
Options: dataprovider.ConditionOptions{
ProviderObjects: []string{"folder"},
},
},
Actions: []dataprovider.EventAction{
{
BaseEventAction: dataprovider.BaseEventAction{
Name: action1.Name,
},
Order: 1,
Options: dataprovider.EventActionOptions{
StopOnFailure: true,
},
},
{
BaseEventAction: dataprovider.BaseEventAction{
Name: action2.Name,
},
Order: 2,
},
{
BaseEventAction: dataprovider.BaseEventAction{
Name: action3.Name,
},
Order: 3,
Options: dataprovider.EventActionOptions{
IsFailureAction: true,
StopOnFailure: true,
},
},
},
}
rule, _, err := httpdtest.AddEventRule(r, http.StatusCreated)
assert.NoError(t, err)
// restrit command execution
dataprovider.EnabledActionCommands = []string{"/bin/ls"}

lastReceivedEmail.reset()
// create a folder to trigger the rule
folder := vfs.BaseVirtualFolder{
Name: "ftest failed command",
MappedPath: filepath.Join(os.TempDir(), "p"),
}
folder, _, err = httpdtest.AddFolder(folder, http.StatusCreated)
assert.NoError(t, err)

assert.NoFileExists(t, outPath)
assert.Eventually(t, func() bool {
return lastReceivedEmail.get().From != ""
}, 3000*time.Millisecond, 100*time.Millisecond)
email := lastReceivedEmail.get()
assert.Len(t, email.To, 1)
assert.True(t, slices.Contains(email.To, "[email protected]"))
assert.Contains(t, email.Data, `Subject: Failed "add" from "admin"`)
assert.Contains(t, email.Data, fmt.Sprintf("Object name: %s object type: folder", folder.Name))
lastReceivedEmail.reset()

dataprovider.EnabledActionCommands = nil

_, err = httpdtest.RemoveFolder(folder, http.StatusOK)
assert.NoError(t, err)

_, err = httpdtest.RemoveEventRule(rule, http.StatusOK)
assert.NoError(t, err)
_, err = httpdtest.RemoveEventAction(action1, http.StatusOK)
assert.NoError(t, err)
_, err = httpdtest.RemoveEventAction(action2, http.StatusOK)
assert.NoError(t, err)
_, err = httpdtest.RemoveEventAction(action3, http.StatusOK)
assert.NoError(t, err)
}

func TestEventRuleProviderEvents(t *testing.T) {
if runtime.GOOS == osWindows {
t.Skip("this test is not available on Windows")
Expand Down
4 changes: 4 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ func Init() {
Metadata: common.MetadataConfig{
Read: 0,
},
EventManager: common.EventManagerConfig{
EnabledCommands: []string{},
},
},
ACME: acme.Configuration{
Email: "",
Expand Down Expand Up @@ -2032,6 +2035,7 @@ func setViperDefaults() {
viper.SetDefault("common.server_version", globalConf.Common.ServerVersion)
viper.SetDefault("common.tz", globalConf.Common.TZ)
viper.SetDefault("common.metadata.read", globalConf.Common.Metadata.Read)
viper.SetDefault("common.event_manager.enabled_commands", globalConf.Common.EventManager.EnabledCommands)
viper.SetDefault("acme.email", globalConf.ACME.Email)
viper.SetDefault("acme.key_type", globalConf.ACME.KeyType)
viper.SetDefault("acme.certs_path", globalConf.ACME.CertsPath)
Expand Down
14 changes: 14 additions & 0 deletions internal/dataprovider/eventrule.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ var (
ActionTypeBackup, ActionTypeUserQuotaReset, ActionTypeFolderQuotaReset, ActionTypeTransferQuotaReset,
ActionTypeDataRetentionCheck, ActionTypePasswordExpirationCheck, ActionTypeUserExpirationCheck,
ActionTypeUserInactivityCheck, ActionTypeIDPAccountCheck, ActionTypeRotateLogs}
// EnabledActionCommands defines the system commands that can be executed via EventManager,
// an empty list means that any command is allowed to be executed.
EnabledActionCommands []string
)

func isActionTypeValid(action int) bool {
Expand Down Expand Up @@ -450,6 +453,14 @@ func (c *EventActionHTTPConfig) GetHTTPClient() *http.Client {
return client
}

// IsActionCommandAllowed returns true if the specified command is allowed
func IsActionCommandAllowed(cmd string) bool {
if len(EnabledActionCommands) == 0 {
return true
}
return slices.Contains(EnabledActionCommands, cmd)
}

// EventActionCommandConfig defines the configuration for a command event target
type EventActionCommandConfig struct {
Cmd string `json:"cmd,omitempty"`
Expand All @@ -462,6 +473,9 @@ func (c *EventActionCommandConfig) validate() error {
if c.Cmd == "" {
return util.NewI18nError(util.NewValidationError("command is required"), util.I18nErrorCommandRequired)
}
if !IsActionCommandAllowed(c.Cmd) {
return util.NewValidationError(fmt.Sprintf("command %q is not allowed", c.Cmd))
}
if !filepath.IsAbs(c.Cmd) {
return util.NewI18nError(
util.NewValidationError("invalid command, it must be an absolute path"),
Expand Down
11 changes: 11 additions & 0 deletions internal/httpd/httpd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2395,6 +2395,17 @@ func TestEventActionValidation(t *testing.T) {
_, resp, err = httpdtest.AddEventAction(action, http.StatusBadRequest)
assert.NoError(t, err)
assert.Contains(t, string(resp), "invalid command args")
action.Options.CmdConfig.Args = nil
// restrict commands
if runtime.GOOS == osWindows {
dataprovider.EnabledActionCommands = []string{"C:\\cmd.exe"}
} else {
dataprovider.EnabledActionCommands = []string{"/bin/sh"}
}
_, resp, err = httpdtest.AddEventAction(action, http.StatusBadRequest)
assert.NoError(t, err)
assert.Contains(t, string(resp), "is not allowed")
dataprovider.EnabledActionCommands = nil

action.Type = dataprovider.ActionTypeEmail
_, resp, err = httpdtest.AddEventAction(action, http.StatusBadRequest)
Expand Down
32 changes: 17 additions & 15 deletions internal/httpd/webadmin.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,13 +297,14 @@ type rolePage struct {

type eventActionPage struct {
basePage
Action dataprovider.BaseEventAction
ActionTypes []dataprovider.EnumMapping
FsActions []dataprovider.EnumMapping
HTTPMethods []string
RedactedSecret string
Error *util.I18nError
Mode genericPageMode
Action dataprovider.BaseEventAction
ActionTypes []dataprovider.EnumMapping
FsActions []dataprovider.EnumMapping
HTTPMethods []string
EnabledCommands []string
RedactedSecret string
Error *util.I18nError
Mode genericPageMode
}

type eventRulePage struct {
Expand Down Expand Up @@ -1088,14 +1089,15 @@ func (s *httpdServer) renderEventActionPage(w http.ResponseWriter, r *http.Reque
}

data := eventActionPage{
basePage: s.getBasePageData(title, currentURL, w, r),
Action: action,
ActionTypes: dataprovider.EventActionTypes,
FsActions: dataprovider.FsActionTypes,
HTTPMethods: dataprovider.SupportedHTTPActionMethods,
RedactedSecret: redactedSecret,
Error: getI18nError(err),
Mode: mode,
basePage: s.getBasePageData(title, currentURL, w, r),
Action: action,
ActionTypes: dataprovider.EventActionTypes,
FsActions: dataprovider.FsActionTypes,
HTTPMethods: dataprovider.SupportedHTTPActionMethods,
EnabledCommands: dataprovider.EnabledActionCommands,
RedactedSecret: redactedSecret,
Error: getI18nError(err),
Mode: mode,
}
renderAdminTemplate(w, templateEventAction, data)
}
Expand Down
5 changes: 4 additions & 1 deletion sftpgo.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@
"entries_soft_limit": 100,
"entries_hard_limit": 150
}
]
],
"event_manager": {
"enabled_commands": []
}
},
"acme": {
"domains": [],
Expand Down
Loading

0 comments on commit 88b1850

Please sign in to comment.