From a08dd85efd2b65bbe170b116ae42230a651143da Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 16 May 2020 23:26:44 +0200 Subject: [PATCH] sftpd: deprecate keys and add a new host_keys config param host_key defines the private host keys as plain list of strings. Remove the other deprecated config params from the default config too. Signed-off-by: Nicola Murino --- config/config.go | 14 +++++++++++++- config/config_test.go | 32 ++++++++++++++++++++++++++++++++ docs/full-configuration.md | 3 ++- sftpd/internal_test.go | 15 ++------------- sftpd/server.go | 30 ++++++++++++++++++------------ sftpd/sftpd_test.go | 6 +----- sftpgo.json | 7 ++----- 7 files changed, 70 insertions(+), 37 deletions(-) diff --git a/config/config.go b/config/config.go index 6a6d9004..df617c05 100644 --- a/config/config.go +++ b/config/config.go @@ -57,7 +57,7 @@ func init() { Command: "", HTTPNotificationURL: "", }, - Keys: []sftpd.Key{}, + HostKeys: []string{}, KexAlgorithms: []string{}, Ciphers: []string{}, MACs: []string{}, @@ -218,6 +218,7 @@ func LoadConfig(configDir, configName string) error { logger.WarnToConsole("Configuration error: %v", err) } checkHooksCompatibility() + checkHostKeyCompatibility() logger.Debug(logSender, "", "config file used: '%#v', config loaded: %+v", viper.ConfigFileUsed(), getRedactedGlobalConf()) return err } @@ -240,3 +241,14 @@ func checkHooksCompatibility() { globalConf.SFTPD.KeyboardInteractiveHook = globalConf.SFTPD.KeyboardInteractiveProgram //nolint:staticcheck } } + +func checkHostKeyCompatibility() { + // we copy deprecated fields to new ones to keep backward compatibility so lint is disabled + if len(globalConf.SFTPD.Keys) > 0 && len(globalConf.SFTPD.HostKeys) == 0 { //nolint:staticcheck + logger.Warn(logSender, "", "keys is deprecated, please use host_keys") + logger.WarnToConsole("keys is deprecated, please use host_keys") + for _, k := range globalConf.SFTPD.Keys { //nolint:staticcheck + globalConf.SFTPD.HostKeys = append(globalConf.SFTPD.HostKeys, k.PrivateKey) + } + } +} diff --git a/config/config_test.go b/config/config_test.go index b5d7719f..ae2bdacd 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -15,6 +15,7 @@ import ( "github.com/drakkan/sftpgo/httpclient" "github.com/drakkan/sftpgo/httpd" "github.com/drakkan/sftpgo/sftpd" + "github.com/drakkan/sftpgo/utils" ) const ( @@ -205,6 +206,37 @@ func TestHookCompatibity(t *testing.T) { assert.NoError(t, err) } +func TestHostKeyCompatibility(t *testing.T) { + configDir := ".." + confName := tempConfigName + ".json" + configFilePath := filepath.Join(configDir, confName) + err := config.LoadConfig(configDir, configName) + assert.NoError(t, err) + sftpdConf := config.GetSFTPDConfig() + sftpdConf.Keys = []sftpd.Key{ //nolint:staticcheck + { + PrivateKey: "rsa", + }, + { + PrivateKey: "ecdsa", + }, + } + c := make(map[string]sftpd.Configuration) + c["sftpd"] = sftpdConf + jsonConf, err := json.Marshal(c) + assert.NoError(t, err) + err = ioutil.WriteFile(configFilePath, jsonConf, 0666) + assert.NoError(t, err) + err = config.LoadConfig(configDir, tempConfigName) + assert.NoError(t, err) + sftpdConf = config.GetSFTPDConfig() + assert.Equal(t, 2, len(sftpdConf.HostKeys)) + assert.True(t, utils.IsStringInSlice("rsa", sftpdConf.HostKeys)) + assert.True(t, utils.IsStringInSlice("ecdsa", sftpdConf.HostKeys)) + err = os.Remove(configFilePath) + assert.NoError(t, err) +} + func TestSetGetConfig(t *testing.T) { sftpdConf := config.GetSFTPDConfig() sftpdConf.IdleTimeout = 3 diff --git a/docs/full-configuration.md b/docs/full-configuration.md index 802184fc..91586ec2 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -50,8 +50,9 @@ The configuration file contains the following sections: - `execute_on`, list of strings. Valid values are `download`, `upload`, `delete`, `rename`, `ssh_cmd`. Leave empty to disable actions. - `command`, string. Absolute path to the command to execute. Leave empty to disable. - `http_notification_url`, a valid URL. An HTTP GET request will be executed to this URL. Leave empty to disable. - - `keys`, struct array. It contains the daemon's private keys. If empty or missing, the daemon will search or try to generate `id_rsa` and `id_ecdsa` keys in the configuration directory. + - `keys`, struct array. Deprecated, please use `host_keys`. - `private_key`, path to the private key file. It can be a path relative to the config dir or an absolute one. + - `host_keys`, list of strings. It contains the daemon's private host keys. Each host key can be defined as a path relative to the configuration directory or an absolute one. If empty or missing, the daemon will search or try to generate `id_rsa` and `id_ecdsa` keys inside the configuration directory. - `kex_algorithms`, list of strings. Available KEX (Key Exchange) algorithms in preference order. Leave empty to use default values. The supported values can be found here: [`crypto/ssh`](https://github.com/golang/crypto/blob/master/ssh/common.go#L46 "Supported kex algos") - `ciphers`, list of strings. Allowed ciphers. Leave empty to use default values. The supported values can be found here: [`crypto/ssh`](https://github.com/golang/crypto/blob/master/ssh/common.go#L28 "Supported ciphers") - `macs`, list of strings. Available MAC (message authentication code) algorithms in preference order. Leave empty to use default values. The supported values can be found here: [`crypto/ssh`](https://github.com/golang/crypto/blob/master/ssh/common.go#L84 "Supported MACs") diff --git a/sftpd/internal_test.go b/sftpd/internal_test.go index 76246c27..3a603dd7 100644 --- a/sftpd/internal_test.go +++ b/sftpd/internal_test.go @@ -1738,24 +1738,13 @@ func TestProxyProtocolVersion(t *testing.T) { func TestLoadHostKeys(t *testing.T) { c := Configuration{} - c.Keys = []Key{ - { - PrivateKey: ".", - }, - { - PrivateKey: "missing file", - }, - } + c.HostKeys = []string{".", "missing file"} err := c.checkAndLoadHostKeys("..", &ssh.ServerConfig{}) assert.Error(t, err) testfile := filepath.Join(os.TempDir(), "invalidkey") err = ioutil.WriteFile(testfile, []byte("some bytes"), 0666) assert.NoError(t, err) - c.Keys = []Key{ - { - PrivateKey: testfile, - }, - } + c.HostKeys = []string{testfile} err = c.checkAndLoadHostKeys("..", &ssh.ServerConfig{}) assert.Error(t, err) err = os.Remove(testfile) diff --git a/sftpd/server.go b/sftpd/server.go index 10bf8a71..0a249f6d 100644 --- a/sftpd/server.go +++ b/sftpd/server.go @@ -63,8 +63,13 @@ type Configuration struct { UploadMode int `json:"upload_mode" mapstructure:"upload_mode"` // Actions to execute on SFTP create, download, delete and rename Actions Actions `json:"actions" mapstructure:"actions"` - // Keys are a list of host keys + // Deprecated: please use HostKeys Keys []Key `json:"keys" mapstructure:"keys"` + // HostKeys define the daemon's private host keys. + // Each host key can be defined as a path relative to the configuration directory or an absolute one. + // If empty or missing, the daemon will search or try to generate "id_rsa" and "id_ecdsa" host keys + // inside the configuration directory. + HostKeys []string `json:"host_keys" mapstructure:"host_keys"` // KexAlgorithms specifies the available KEX (Key Exchange) algorithms in // preference order. KexAlgorithms []string `json:"kex_algorithms" mapstructure:"kex_algorithms"` @@ -131,6 +136,7 @@ type Configuration struct { } // Key contains information about host keys +// Deprecated: please use HostKeys type Key struct { // The private key path as absolute path or relative to the configuration directory PrivateKey string `json:"private_key" mapstructure:"private_key"` @@ -509,7 +515,7 @@ func (c *Configuration) checkSSHCommands() { // If no host keys are defined we try to use or generate the default ones. func (c *Configuration) checkAndLoadHostKeys(configDir string, serverConfig *ssh.ServerConfig) error { - if len(c.Keys) == 0 { + if len(c.HostKeys) == 0 { defaultKeys := []string{defaultPrivateRSAKeyName, defaultPrivateECDSAKeyName} for _, k := range defaultKeys { autoFile := filepath.Join(configDir, k) @@ -525,22 +531,22 @@ func (c *Configuration) checkAndLoadHostKeys(configDir string, serverConfig *ssh return err } } - c.Keys = append(c.Keys, Key{PrivateKey: k}) + c.HostKeys = append(c.HostKeys, k) } } - for _, k := range c.Keys { - privateFile := k.PrivateKey - if !utils.IsFileInputValid(privateFile) { - logger.Warn(logSender, "", "unable to load invalid host key: %#v", privateFile) - logger.WarnToConsole("unable to load invalid host key: %#v", privateFile) + for _, k := range c.HostKeys { + hostKey := k + if !utils.IsFileInputValid(hostKey) { + logger.Warn(logSender, "", "unable to load invalid host key: %#v", hostKey) + logger.WarnToConsole("unable to load invalid host key: %#v", hostKey) continue } - if !filepath.IsAbs(privateFile) { - privateFile = filepath.Join(configDir, privateFile) + if !filepath.IsAbs(hostKey) { + hostKey = filepath.Join(configDir, hostKey) } - logger.Info(logSender, "", "Loading private key: %s", privateFile) + logger.Info(logSender, "", "Loading private host key: %s", hostKey) - privateBytes, err := ioutil.ReadFile(privateFile) + privateBytes, err := ioutil.ReadFile(hostKey) if err != nil { return err } diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go index f97866b8..89b7daf0 100644 --- a/sftpd/sftpd_test.go +++ b/sftpd/sftpd_test.go @@ -293,11 +293,7 @@ func TestInitialization(t *testing.T) { sftpdConf.ProxyAllowed = []string{"1270.0.0.1"} err = sftpdConf.Initialize(configDir) assert.Error(t, err) - sftpdConf.Keys = []sftpd.Key{ - { - PrivateKey: "missing file", - }, - } + sftpdConf.HostKeys = []string{"missing file"} err = sftpdConf.Initialize(configDir) assert.Error(t, err) sftpdConf.Keys = nil diff --git a/sftpgo.json b/sftpgo.json index 1ce129b7..d3d2703d 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -12,7 +12,7 @@ "command": "", "http_notification_url": "" }, - "keys": [], + "host_keys": [], "kex_algorithms": [], "ciphers": [], "macs": [], @@ -26,7 +26,6 @@ "pwd", "scp" ], - "keyboard_interactive_auth_program": "", "keyboard_interactive_auth_hook": "", "proxy_protocol": 0, "proxy_allowed": [] @@ -50,12 +49,10 @@ "command": "", "http_notification_url": "" }, - "external_auth_program": "", "external_auth_hook": "", "external_auth_scope": 0, "credentials_path": "credentials", - "pre_login_hook": "", - "pre_login_program": "" + "pre_login_hook": "" }, "httpd": { "bind_port": 8080,