From 4b4edef0adfc1d88ff7f102337edf2b837ba1a35 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 12 Oct 2022 18:12:12 +0200 Subject: [PATCH] disable self connections by default now that the event manager can create files, self connections may create even more issues than before Signed-off-by: Nicola Murino --- docs/full-configuration.md | 1 + internal/common/common.go | 5 +++++ internal/common/protocol_test.go | 1 + internal/config/config.go | 2 ++ internal/dataprovider/dataprovider.go | 6 ++++++ internal/dataprovider/user.go | 3 +++ internal/ftpd/ftpd_test.go | 1 + internal/httpd/httpd_test.go | 1 + internal/sftpd/sftpd_test.go | 1 + internal/vfs/sftpfs.go | 6 +++++- internal/vfs/vfs.go | 12 +++++++++--- internal/webdavd/webdavd_test.go | 1 + sftpgo.json | 1 + 13 files changed, 37 insertions(+), 4 deletions(-) diff --git a/docs/full-configuration.md b/docs/full-configuration.md index 26abdafd..4710f3f4 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -78,6 +78,7 @@ The configuration file contains the following sections: - `max_total_connections`, integer. Maximum number of concurrent client connections. 0 means unlimited. Default: 0. - `max_per_host_connections`, integer. Maximum number of concurrent client connections from the same host (IP). If the defender is enabled, exceeding this limit will generate `score_limit_exceeded` events and thus hosts that repeatedly exceed the max allowed connections can be automatically blocked. 0 means unlimited. Default: 20. - `whitelist_file`, string. Path to a file containing a list of IP addresses and/or networks to allow. Only the listed IPs/networks can access the configured services, all other client connections will be dropped before they even try to authenticate. The whitelist must be a JSON file with the same structure documented for the [defenders's list](./defender.md). The whitelist can be reloaded on demand sending a `SIGHUP` signal on Unix based systems and a `paramchange` request to the running service on Windows. Default: "". + - `allow_self_connections`, integer. Allow users on this instance to use other users/virtual folders on this instance as storage backend. Enable this setting if you know what you are doing. Set to `1` to enable. Default: `0`. - `defender`, struct containing the defender configuration. See [Defender](./defender.md) for more details. - `enabled`, boolean. Default `false`. - `driver`, string. Supported drivers are `memory` and `provider`. The `provider` driver will use the configured data provider to store defender events and it is supported for `MySQL`, `PostgreSQL` and `CockroachDB` data providers. Using the `provider` driver you can share the defender events among multiple SFTPGO instances. For a single instance the `memory` driver will be much faster. Default: `memory`. diff --git a/internal/common/common.go b/internal/common/common.go index 17347b2a..f0acaf9e 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -212,6 +212,8 @@ func Initialize(c Configuration, isShared int) error { } vfs.SetTempPath(c.TempPath) dataprovider.SetTempPath(c.TempPath) + vfs.SetAllowSelfConnections(c.AllowSelfConnections) + dataprovider.SetAllowSelfConnections(c.AllowSelfConnections) transfersChecker = getTransfersChecker(isShared) return nil } @@ -504,6 +506,9 @@ type Configuration struct { // Only the listed IPs/networks can access the configured services, all other client connections // will be dropped before they even try to authenticate. WhiteListFile string `json:"whitelist_file" mapstructure:"whitelist_file"` + // Allow users on this instance to use other users/virtual folders on this instance as storage backend. + // Enable this setting if you know what you are doing. + AllowSelfConnections int `json:"allow_self_connections" mapstructure:"allow_self_connections"` // Defender configuration DefenderConfig DefenderConfig `json:"defender" mapstructure:"defender"` // Rate limiter configurations diff --git a/internal/common/protocol_test.go b/internal/common/protocol_test.go index 49c5ff0f..d5ff7c8b 100644 --- a/internal/common/protocol_test.go +++ b/internal/common/protocol_test.go @@ -93,6 +93,7 @@ func TestMain(m *testing.M) { logger.InitLogger(logFilePath, 5, 1, 28, false, false, zerolog.DebugLevel) os.Setenv("SFTPGO_DATA_PROVIDER__CREATE_DEFAULT_ADMIN", "1") + os.Setenv("SFTPGO_COMMON__ALLOW_SELF_CONNECTIONS", "1") os.Setenv("SFTPGO_DEFAULT_ADMIN_USERNAME", "admin") os.Setenv("SFTPGO_DEFAULT_ADMIN_PASSWORD", "password") err := config.LoadConfig(configDir, "") diff --git a/internal/config/config.go b/internal/config/config.go index 595434f8..e7f6fbbe 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -208,6 +208,7 @@ func Init() { MaxTotalConnections: 0, MaxPerHostConnections: 20, WhiteListFile: "", + AllowSelfConnections: 0, DefenderConfig: common.DefenderConfig{ Enabled: false, Driver: common.DefenderDriverMemory, @@ -1862,6 +1863,7 @@ func setViperDefaults() { viper.SetDefault("common.max_total_connections", globalConf.Common.MaxTotalConnections) viper.SetDefault("common.max_per_host_connections", globalConf.Common.MaxPerHostConnections) viper.SetDefault("common.whitelist_file", globalConf.Common.WhiteListFile) + viper.SetDefault("common.allow_self_connections", globalConf.Common.AllowSelfConnections) viper.SetDefault("common.defender.enabled", globalConf.Common.DefenderConfig.Enabled) viper.SetDefault("common.defender.driver", globalConf.Common.DefenderConfig.Driver) viper.SetDefault("common.defender.ban_time", globalConf.Common.DefenderConfig.BanTime) diff --git a/internal/dataprovider/dataprovider.go b/internal/dataprovider/dataprovider.go index b5281777..2d0f5863 100644 --- a/internal/dataprovider/dataprovider.go +++ b/internal/dataprovider/dataprovider.go @@ -195,6 +195,7 @@ var ( lastLoginMinDelay = 10 * time.Minute usernameRegex = regexp.MustCompile("^[a-zA-Z0-9-_.~]+$") tempPath string + allowSelfConnections int fnReloadRules FnReloadRules fnRemoveRule FnRemoveRule fnHandleRuleForProviderEvent FnHandleRuleForProviderEvent @@ -804,6 +805,11 @@ type Provider interface { resetDatabase() error } +// SetAllowSelfConnections sets the desired behaviour for self connections +func SetAllowSelfConnections(value int) { + allowSelfConnections = value +} + // SetTempPath sets the path for temporary files func SetTempPath(fsPath string) { tempPath = fsPath diff --git a/internal/dataprovider/user.go b/internal/dataprovider/user.go index 4b74ce80..a95a4ddc 100644 --- a/internal/dataprovider/user.go +++ b/internal/dataprovider/user.go @@ -497,6 +497,9 @@ func (u *User) GetPermissionsForPath(p string) []string { } func (u *User) getForbiddenSFTPSelfUsers(username string) ([]string, error) { + if allowSelfConnections == 0 { + return nil, nil + } sftpUser, err := UserExists(username) if err == nil { err = sftpUser.LoadAndApplyGroupSettings() diff --git a/internal/ftpd/ftpd_test.go b/internal/ftpd/ftpd_test.go index 0e94fe2d..79b49302 100644 --- a/internal/ftpd/ftpd_test.go +++ b/internal/ftpd/ftpd_test.go @@ -277,6 +277,7 @@ func TestMain(m *testing.M) { // work in non atomic mode too os.Setenv("SFTPGO_COMMON__UPLOAD_MODE", "2") os.Setenv("SFTPGO_DATA_PROVIDER__CREATE_DEFAULT_ADMIN", "1") + os.Setenv("SFTPGO_COMMON__ALLOW_SELF_CONNECTIONS", "1") os.Setenv("SFTPGO_DEFAULT_ADMIN_USERNAME", "admin") os.Setenv("SFTPGO_DEFAULT_ADMIN_PASSWORD", "password") err = config.LoadConfig(configDir, "") diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index 2f36bdb7..c1265cd0 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -296,6 +296,7 @@ func TestMain(m *testing.M) { logger.InitLogger(logfilePath, 5, 1, 28, false, false, zerolog.DebugLevel) os.Setenv("SFTPGO_COMMON__UPLOAD_MODE", "2") os.Setenv("SFTPGO_DATA_PROVIDER__CREATE_DEFAULT_ADMIN", "1") + os.Setenv("SFTPGO_COMMON__ALLOW_SELF_CONNECTIONS", "1") os.Setenv("SFTPGO_DATA_PROVIDER__NAMING_RULES", "0") os.Setenv("SFTPGO_DEFAULT_ADMIN_USERNAME", "admin") os.Setenv("SFTPGO_DEFAULT_ADMIN_PASSWORD", "password") diff --git a/internal/sftpd/sftpd_test.go b/internal/sftpd/sftpd_test.go index ae32c411..33d26e0f 100644 --- a/internal/sftpd/sftpd_test.go +++ b/internal/sftpd/sftpd_test.go @@ -189,6 +189,7 @@ func TestMain(m *testing.M) { } os.Setenv("SFTPGO_COMMON__UPLOAD_MODE", "2") os.Setenv("SFTPGO_DATA_PROVIDER__CREATE_DEFAULT_ADMIN", "1") + os.Setenv("SFTPGO_COMMON__ALLOW_SELF_CONNECTIONS", "1") os.Setenv("SFTPGO_DEFAULT_ADMIN_USERNAME", "admin") os.Setenv("SFTPGO_DEFAULT_ADMIN_PASSWORD", "password") err = config.LoadConfig(configDir, "") diff --git a/internal/vfs/sftpfs.go b/internal/vfs/sftpfs.go index 469caa63..38555583 100644 --- a/internal/vfs/sftpfs.go +++ b/internal/vfs/sftpfs.go @@ -843,8 +843,12 @@ func (fs *SFTPFs) createConnection() error { HostKeyCallback: func(_ string, _ net.Addr, key ssh.PublicKey) error { fp := ssh.FingerprintSHA256(key) if util.Contains(sftpFingerprints, fp) { + if allowSelfConnections == 0 { + fsLog(fs, logger.LevelError, "SFTP self connections not allowed") + return ErrSFTPLoop + } if util.Contains(fs.config.forbiddenSelfUsernames, fs.config.Username) { - fsLog(fs, logger.LevelError, "SFTP loop or nested local SFTP folders detected, mount path %#v, username %#v, forbidden usernames: %+v", + fsLog(fs, logger.LevelError, "SFTP loop or nested local SFTP folders detected, mount path %q, username %q, forbidden usernames: %+v", fs.mountPath, fs.config.Username, fs.config.forbiddenSelfUsernames) return ErrSFTPLoop } diff --git a/internal/vfs/vfs.go b/internal/vfs/vfs.go index 82bb5e25..82612391 100644 --- a/internal/vfs/vfs.go +++ b/internal/vfs/vfs.go @@ -45,11 +45,17 @@ var ( // ErrStorageSizeUnavailable is returned if the storage backend does not support getting the size ErrStorageSizeUnavailable = errors.New("unable to get available size for this storage backend") // ErrVfsUnsupported defines the error for an unsupported VFS operation - ErrVfsUnsupported = errors.New("not supported") - tempPath string - sftpFingerprints []string + ErrVfsUnsupported = errors.New("not supported") + tempPath string + sftpFingerprints []string + allowSelfConnections int ) +// SetAllowSelfConnections sets the desired behaviour for self connections +func SetAllowSelfConnections(value int) { + allowSelfConnections = value +} + // SetTempPath sets the path for temporary files func SetTempPath(fsPath string) { tempPath = fsPath diff --git a/internal/webdavd/webdavd_test.go b/internal/webdavd/webdavd_test.go index f1100837..785080c1 100644 --- a/internal/webdavd/webdavd_test.go +++ b/internal/webdavd/webdavd_test.go @@ -268,6 +268,7 @@ func TestMain(m *testing.M) { logFilePath = filepath.Join(configDir, "sftpgo_webdavd_test.log") logger.InitLogger(logFilePath, 5, 1, 28, false, false, zerolog.DebugLevel) os.Setenv("SFTPGO_DATA_PROVIDER__CREATE_DEFAULT_ADMIN", "1") + os.Setenv("SFTPGO_COMMON__ALLOW_SELF_CONNECTIONS", "1") os.Setenv("SFTPGO_DEFAULT_ADMIN_USERNAME", "admin") os.Setenv("SFTPGO_DEFAULT_ADMIN_PASSWORD", "password") err := config.LoadConfig(configDir, "") diff --git a/sftpgo.json b/sftpgo.json index e9516fe8..1782ecb1 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -18,6 +18,7 @@ "max_total_connections": 0, "max_per_host_connections": 20, "whitelist_file": "", + "allow_self_connections": 0, "defender": { "enabled": false, "driver": "memory",