diff --git a/config/config.go b/config/config.go index 0e1f2434..7acf528d 100644 --- a/config/config.go +++ b/config/config.go @@ -72,6 +72,7 @@ func init() { LoginBannerFile: "", EnabledSSHCommands: sftpd.GetDefaultSSHCommands(), KeyboardInteractiveHook: "", + PasswordAuthentication: true, }, FTPD: ftpd.Configuration{ BindPort: 0, diff --git a/dataprovider/user.go b/dataprovider/user.go index 7260dd23..b4757917 100644 --- a/dataprovider/user.go +++ b/dataprovider/user.go @@ -366,7 +366,7 @@ func (u *User) IsLoginMethodAllowed(loginMethod string, partialSuccessMethods [] return true } if len(partialSuccessMethods) == 1 { - for _, method := range u.GetNextAuthMethods(partialSuccessMethods) { + for _, method := range u.GetNextAuthMethods(partialSuccessMethods, true) { if method == loginMethod { return true } @@ -380,7 +380,7 @@ func (u *User) IsLoginMethodAllowed(loginMethod string, partialSuccessMethods [] // GetNextAuthMethods returns the list of authentications methods that // can continue for multi-step authentication -func (u *User) GetNextAuthMethods(partialSuccessMethods []string) []string { +func (u *User) GetNextAuthMethods(partialSuccessMethods []string, isPasswordAuthEnabled bool) []string { var methods []string if len(partialSuccessMethods) != 1 { return methods @@ -389,7 +389,7 @@ func (u *User) GetNextAuthMethods(partialSuccessMethods []string) []string { return methods } for _, method := range u.GetAllowedLoginMethods() { - if method == SSHLoginMethodKeyAndPassword { + if method == SSHLoginMethodKeyAndPassword && isPasswordAuthEnabled { methods = append(methods, LoginMethodPassword) } if method == SSHLoginMethodKeyAndKeyboardInt { diff --git a/docs/full-configuration.md b/docs/full-configuration.md index a00d4025..ddf7458d 100644 --- a/docs/full-configuration.md +++ b/docs/full-configuration.md @@ -63,7 +63,6 @@ The configuration file contains the following sections: - `bind_address`, string. Leave blank to listen on all available network interfaces. Default: "" - `idle_timeout`, integer. Deprecated, please use the same key in `common` section. - `max_auth_tries` integer. Maximum number of authentication attempts permitted per connection. If set to a negative number, the number of attempts is unlimited. If set to zero, the number of attempts are limited to 6. - - `password_disabled`, boolean. Set to false to forbid password authentication (for example in a pubkey-only setup). - `banner`, string. Identification string used by the server. Leave empty to use the default banner. Default `SFTPGo_`, for example `SSH-2.0-SFTPGo_0.9.5` - `upload_mode` integer. Deprecated, please use the same key in `common` section. - `actions`, struct. Deprecated, please use the same key in `common` section. @@ -78,6 +77,7 @@ The configuration file contains the following sections: - `setstat_mode`, integer. Deprecated, please use the same key in `common` section. - `enabled_ssh_commands`, list of enabled SSH commands. `*` enables all supported commands. More information can be found [here](./ssh-commands.md). - `keyboard_interactive_auth_hook`, string. Absolute path to an external program or an HTTP URL to invoke for keyboard interactive authentication. See [Keyboard Interactive Authentication](./keyboard-interactive.md) for more details. + - `password_authentication`, boolean. Set to false to disable password authentication. This setting will disable multi-step authentication method using public key + password too. It is useful for public key only configurations if you need to manage old clients that will not attempt to authenticate with public keys if the password login method is advertised. Default: true. - `proxy_protocol`, integer. Deprecated, please use the same key in `common` section. - `proxy_allowed`, list of strings. Deprecated, please use the same key in `common` section. - **"ftpd"**, the configuration for the FTP server diff --git a/sftpd/server.go b/sftpd/server.go index 43e8a895..60f450a7 100644 --- a/sftpd/server.go +++ b/sftpd/server.go @@ -97,11 +97,11 @@ type Configuration struct { // The following SSH commands are enabled by default: "md5sum", "sha1sum", "cd", "pwd". // "*" enables all supported SSH commands. EnabledSSHCommands []string `json:"enabled_ssh_commands" mapstructure:"enabled_ssh_commands"` - // PasswordDisabled specifies whether to forbid password authentication, for example in a publickey-only setup. - PasswordDisabled bool `json:"password_disabled" mapstructure:"password_disabled"` // Absolute path to an external program or an HTTP URL to invoke for keyboard interactive authentication. // Leave empty to disable this authentication mode. KeyboardInteractiveHook string `json:"keyboard_interactive_auth_hook" mapstructure:"keyboard_interactive_auth_hook"` + // PasswordAuthentication specifies whether password authentication is allowed. + PasswordAuthentication bool `json:"password_authentication" mapstructure:"password_authentication"` // Deprecated: please use the same key in common configuration ProxyProtocol int `json:"proxy_protocol" mapstructure:"proxy_protocol"` // Deprecated: please use the same key in common configuration @@ -145,14 +145,14 @@ func (c Configuration) Initialize(configDir string) error { var nextMethods []string user, err := dataprovider.UserExists(conn.User()) if err == nil { - nextMethods = user.GetNextAuthMethods(conn.PartialSuccessMethods()) + nextMethods = user.GetNextAuthMethods(conn.PartialSuccessMethods(), c.PasswordAuthentication) } return nextMethods }, ServerVersion: fmt.Sprintf("SSH-2.0-%v", c.Banner), } - if !c.PasswordDisabled { + if c.PasswordAuthentication { serverConfig.PasswordCallback = func(conn ssh.ConnMetadata, pass []byte) (*ssh.Permissions, error) { sp, err := c.validatePasswordCredentials(conn, pass) if err != nil { diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go index eca51d35..2d3a7bc2 100644 --- a/sftpd/sftpd_test.go +++ b/sftpd/sftpd_test.go @@ -49,6 +49,7 @@ import ( const ( logSender = "sftpdTesting" sftpServerAddr = "127.0.0.1:2022" + sftpSrvAddr2222 = "127.0.0.1:2222" defaultUsername = "test_user_sftp" defaultPassword = "test_password" testPubKey = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC03jj0D+djk7pxIf/0OhrxrchJTRZklofJ1NoIu4752Sq02mdXmarMVsqJ1cAjV5LBVy3D1F5U6XW4rppkXeVtd04Pxb09ehtH0pRRPaoHHlALiJt8CoMpbKYMA8b3KXPPriGxgGomvtU2T2RMURSwOZbMtpsugfjYSWenyYX+VORYhylWnSXL961LTyC21ehd6d6QnW9G7E5hYMITMY9TuQZz3bROYzXiTsgN0+g6Hn7exFQp50p45StUMfV/SftCMdCxlxuyGny2CrN/vfjO7xxOo2uv7q1qm10Q46KPWJQv+pgZ/OfL+EDjy07n5QVSKHlbx+2nT4Q0EgOSQaCTYwn3YjtABfIxWwgAFdyj6YlPulCL22qU4MYhDcA6PSBwDdf8hvxBfvsiHdM+JcSHvv8/VeJhk6CmnZxGY0fxBupov27z3yEO8nAg8k+6PaUiW1MSUfuGMF/ktB8LOstXsEPXSszuyXiOv4DaryOXUiSn7bmRqKcEFlJusO6aZP0= nicola@p1" @@ -243,6 +244,7 @@ func TestMain(m *testing.M) { waitTCPListening(fmt.Sprintf("%s:%d", httpdConf.BindAddress, httpdConf.BindPort)) sftpdConf.BindPort = 2222 + sftpdConf.PasswordAuthentication = false common.Config.ProxyProtocol = 1 go func() { logger.Debug(logSender, "", "initializing SFTP server with config %+v and proxy protocol %v", @@ -256,6 +258,7 @@ func TestMain(m *testing.M) { waitTCPListening(fmt.Sprintf("%s:%d", sftpdConf.BindAddress, sftpdConf.BindPort)) sftpdConf.BindPort = 2224 + sftpdConf.PasswordAuthentication = true common.Config.ProxyProtocol = 2 go func() { logger.Debug(logSender, "", "initializing SFTP server with config %+v and proxy protocol %v", @@ -524,13 +527,13 @@ func TestConcurrency(t *testing.T) { } func TestProxyProtocol(t *testing.T) { - usePubKey := false + usePubKey := true user, _, err := httpd.AddUser(getTestUser(usePubKey), http.StatusOK) assert.NoError(t, err) // remove the home dir to test auto creation err = os.RemoveAll(user.HomeDir) assert.NoError(t, err) - client, err := getSftpClientWithAddr(user, usePubKey, "127.0.0.1:2222") + client, err := getSftpClientWithAddr(user, usePubKey, sftpSrvAddr2222) if assert.NoError(t, err) { defer client.Close() assert.NoError(t, checkBasicSFTP(client)) @@ -995,7 +998,7 @@ func TestLoginUserCert(t *testing.T) { // try login using a cert signed from a trusted CA signer, err := getSignerForUserCert([]byte(testCertValid)) assert.NoError(t, err) - client, err := getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}) + client, err := getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}, "") if assert.NoError(t, err) { defer client.Close() assert.NoError(t, checkBasicSFTP(client)) @@ -1003,28 +1006,28 @@ func TestLoginUserCert(t *testing.T) { // try login using a cert signed from an untrusted CA signer, err = getSignerForUserCert([]byte(testCertUntrustedCA)) assert.NoError(t, err) - client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}) + client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}, "") if !assert.Error(t, err) { client.Close() } // try login using an host certificate instead of an user certificate signer, err = getSignerForUserCert([]byte(testHostCert)) assert.NoError(t, err) - client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}) + client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}, "") if !assert.Error(t, err) { client.Close() } // try login using a user certificate with an authorized source address different from localhost signer, err = getSignerForUserCert([]byte(testCertOtherSourceAddress)) assert.NoError(t, err) - client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}) + client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}, "") if !assert.Error(t, err) { client.Close() } // try login using an expired certificate signer, err = getSignerForUserCert([]byte(testCertExpired)) assert.NoError(t, err) - client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}) + client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}, "") if !assert.Error(t, err) { client.Close() } @@ -1040,7 +1043,7 @@ func TestLoginUserCert(t *testing.T) { signer, err = getSignerForUserCert([]byte(testCertValid)) assert.NoError(t, err) - client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}) + client, err = getCustomAuthSftpClient(user, []ssh.AuthMethod{ssh.PublicKeys(signer)}, "") if !assert.Error(t, err) { client.Close() } @@ -1075,16 +1078,20 @@ func TestMultiStepLoginKeyAndPwd(t *testing.T) { ssh.PublicKeys(signer), ssh.Password(defaultPassword), } - client, err = getCustomAuthSftpClient(user, authMethods) + client, err = getCustomAuthSftpClient(user, authMethods, "") if assert.NoError(t, err) { defer client.Close() assert.NoError(t, checkBasicSFTP(client)) } + client, err = getCustomAuthSftpClient(user, authMethods, sftpSrvAddr2222) + if !assert.Error(t, err, "password auth is disabled on port 2222, multi-step auth must fail") { + client.Close() + } authMethods = []ssh.AuthMethod{ ssh.Password(defaultPassword), ssh.PublicKeys(signer), } - _, err = getCustomAuthSftpClient(user, authMethods) + _, err = getCustomAuthSftpClient(user, authMethods, "") assert.Error(t, err, "multi step auth login with wrong order must fail") _, err = httpd.RemoveUser(user, http.StatusOK) assert.NoError(t, err) @@ -1120,7 +1127,12 @@ func TestMultiStepLoginKeyAndKeyInt(t *testing.T) { return []string{"1", "2"}, nil }), } - client, err = getCustomAuthSftpClient(user, authMethods) + client, err = getCustomAuthSftpClient(user, authMethods, "") + if assert.NoError(t, err) { + defer client.Close() + assert.NoError(t, checkBasicSFTP(client)) + } + client, err = getCustomAuthSftpClient(user, authMethods, sftpSrvAddr2222) if assert.NoError(t, err) { defer client.Close() assert.NoError(t, checkBasicSFTP(client)) @@ -1131,16 +1143,33 @@ func TestMultiStepLoginKeyAndKeyInt(t *testing.T) { }), ssh.PublicKeys(signer), } - _, err = getCustomAuthSftpClient(user, authMethods) + _, err = getCustomAuthSftpClient(user, authMethods, "") assert.Error(t, err, "multi step auth login with wrong order must fail") authMethods = []ssh.AuthMethod{ ssh.PublicKeys(signer), ssh.Password(defaultPassword), } - _, err = getCustomAuthSftpClient(user, authMethods) + _, err = getCustomAuthSftpClient(user, authMethods, "") assert.Error(t, err, "multi step auth login with wrong method must fail") + user.Filters.DeniedLoginMethods = nil + user.Filters.DeniedLoginMethods = append(user.Filters.DeniedLoginMethods, []string{ + dataprovider.SSHLoginMethodKeyAndKeyboardInt, + dataprovider.SSHLoginMethodPublicKey, + dataprovider.LoginMethodPassword, + dataprovider.SSHLoginMethodKeyboardInteractive, + }...) + user, _, err = httpd.UpdateUser(user, http.StatusOK, "") + assert.NoError(t, err) + _, err = getCustomAuthSftpClient(user, authMethods, sftpSrvAddr2222) + assert.Error(t, err) + client, err = getCustomAuthSftpClient(user, authMethods, "") + if assert.NoError(t, err) { + assert.NoError(t, checkBasicSFTP(client)) + client.Close() + } + _, err = httpd.RemoveUser(user, http.StatusOK) assert.NoError(t, err) err = os.RemoveAll(user.GetHomeDir()) @@ -1165,7 +1194,7 @@ func TestMultiStepLoginCertAndPwd(t *testing.T) { ssh.PublicKeys(signer), ssh.Password(defaultPassword), } - client, err := getCustomAuthSftpClient(user, authMethods) + client, err := getCustomAuthSftpClient(user, authMethods, "") if assert.NoError(t, err) { defer client.Close() assert.NoError(t, checkBasicSFTP(client)) @@ -1177,7 +1206,7 @@ func TestMultiStepLoginCertAndPwd(t *testing.T) { ssh.PublicKeys(signer), ssh.Password(defaultPassword), } - client, err = getCustomAuthSftpClient(user, authMethods) + client, err = getCustomAuthSftpClient(user, authMethods, "") if !assert.Error(t, err) { client.Close() } @@ -5671,25 +5700,28 @@ func TestUserGetNextAuthMethods(t *testing.T) { dataprovider.SSHLoginMethodPublicKey, dataprovider.SSHLoginMethodKeyboardInteractive, } - methods := user.GetNextAuthMethods(nil) + methods := user.GetNextAuthMethods(nil, true) assert.Equal(t, 0, len(methods)) - methods = user.GetNextAuthMethods([]string{dataprovider.LoginMethodPassword}) + methods = user.GetNextAuthMethods([]string{dataprovider.LoginMethodPassword}, true) assert.Equal(t, 0, len(methods)) - methods = user.GetNextAuthMethods([]string{dataprovider.SSHLoginMethodKeyboardInteractive}) + methods = user.GetNextAuthMethods([]string{dataprovider.SSHLoginMethodKeyboardInteractive}, true) assert.Equal(t, 0, len(methods)) methods = user.GetNextAuthMethods([]string{ dataprovider.SSHLoginMethodPublicKey, dataprovider.SSHLoginMethodKeyboardInteractive, - }) + }, true) assert.Equal(t, 0, len(methods)) - methods = user.GetNextAuthMethods([]string{dataprovider.SSHLoginMethodPublicKey}) + methods = user.GetNextAuthMethods([]string{dataprovider.SSHLoginMethodPublicKey}, true) assert.Equal(t, 2, len(methods)) assert.True(t, utils.IsStringInSlice(dataprovider.LoginMethodPassword, methods)) assert.True(t, utils.IsStringInSlice(dataprovider.SSHLoginMethodKeyboardInteractive, methods)) + methods = user.GetNextAuthMethods([]string{dataprovider.SSHLoginMethodPublicKey}, false) + assert.Equal(t, 1, len(methods)) + assert.True(t, utils.IsStringInSlice(dataprovider.SSHLoginMethodKeyboardInteractive, methods)) user.Filters.DeniedLoginMethods = []string{ dataprovider.LoginMethodPassword, @@ -5697,7 +5729,7 @@ func TestUserGetNextAuthMethods(t *testing.T) { dataprovider.SSHLoginMethodKeyboardInteractive, dataprovider.SSHLoginMethodKeyAndKeyboardInt, } - methods = user.GetNextAuthMethods([]string{dataprovider.SSHLoginMethodPublicKey}) + methods = user.GetNextAuthMethods([]string{dataprovider.SSHLoginMethodPublicKey}, true) assert.Equal(t, 1, len(methods)) assert.True(t, utils.IsStringInSlice(dataprovider.LoginMethodPassword, methods)) @@ -5707,7 +5739,7 @@ func TestUserGetNextAuthMethods(t *testing.T) { dataprovider.SSHLoginMethodKeyboardInteractive, dataprovider.SSHLoginMethodKeyAndPassword, } - methods = user.GetNextAuthMethods([]string{dataprovider.SSHLoginMethodPublicKey}) + methods = user.GetNextAuthMethods([]string{dataprovider.SSHLoginMethodPublicKey}, true) assert.Equal(t, 1, len(methods)) assert.True(t, utils.IsStringInSlice(dataprovider.SSHLoginMethodKeyboardInteractive, methods)) } @@ -5720,8 +5752,11 @@ func TestUserIsLoginMethodAllowed(t *testing.T) { dataprovider.SSHLoginMethodKeyboardInteractive, } assert.False(t, user.IsLoginMethodAllowed(dataprovider.LoginMethodPassword, nil)) + assert.False(t, user.IsLoginMethodAllowed(dataprovider.SSHLoginMethodPublicKey, nil)) + assert.False(t, user.IsLoginMethodAllowed(dataprovider.SSHLoginMethodKeyboardInteractive, nil)) assert.True(t, user.IsLoginMethodAllowed(dataprovider.LoginMethodPassword, []string{dataprovider.SSHLoginMethodPublicKey})) assert.True(t, user.IsLoginMethodAllowed(dataprovider.SSHLoginMethodKeyboardInteractive, []string{dataprovider.SSHLoginMethodPublicKey})) + assert.True(t, user.IsLoginMethodAllowed(dataprovider.SSHLoginMethodKeyAndPassword, []string{dataprovider.SSHLoginMethodPublicKey})) user.Filters.DeniedLoginMethods = []string{ dataprovider.SSHLoginMethodPublicKey, @@ -7536,7 +7571,7 @@ func getKeyboardInteractiveSftpClient(user dataprovider.User, answers []string) return sftpClient, err } -func getCustomAuthSftpClient(user dataprovider.User, authMethods []ssh.AuthMethod) (*sftp.Client, error) { +func getCustomAuthSftpClient(user dataprovider.User, authMethods []ssh.AuthMethod, addr string) (*sftp.Client, error) { var sftpClient *sftp.Client config := &ssh.ClientConfig{ User: user.Username, @@ -7545,7 +7580,13 @@ func getCustomAuthSftpClient(user dataprovider.User, authMethods []ssh.AuthMetho }, Auth: authMethods, } - conn, err := ssh.Dial("tcp", sftpServerAddr, config) + var err error + var conn *ssh.Client + if len(addr) > 0 { + conn, err = ssh.Dial("tcp", addr, config) + } else { + conn, err = ssh.Dial("tcp", sftpServerAddr, config) + } if err != nil { return sftpClient, err } diff --git a/sftpgo.json b/sftpgo.json index b8c97481..d4b22c0f 100644 --- a/sftpgo.json +++ b/sftpgo.json @@ -29,7 +29,8 @@ "pwd", "scp" ], - "keyboard_interactive_auth_hook": "" + "keyboard_interactive_auth_hook": "", + "password_authentication": true }, "ftpd": { "bind_port": 0,