From 6aff8c2f5edc780b45c559d2c13ef97477ab9794 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 29 Dec 2019 07:43:59 +0100 Subject: [PATCH] add support for checking passwords in md5crypt ($1$) format this is an old and unsafe schema but it is still useful to import users from legacy systems --- README.md | 2 +- dataprovider/dataprovider.go | 55 ++++++++++++++++++++++++++---------- sftpd/handler.go | 26 ++++++++--------- sftpd/sftpd_test.go | 33 ++++++++++++++++++++++ 4 files changed, 86 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 9e341cc4..b252e197 100644 --- a/README.md +++ b/README.md @@ -364,7 +364,7 @@ Here is an example of the advertised service including credentials as seen using For each account the following properties can be configured: - `username` -- `password` used for password authentication. For users created using SFTPGo REST API if the password has no known hashing algo prefix it will be stored using argon2id. SFTPGo supports checking passwords stored with bcrypt, pbkdf2 and sha512crypt too. For pbkdf2 the supported format is `$$$$`, where algo is `pbkdf2-sha1` or `pbkdf2-sha256` or `pbkdf2-sha512`. For example the `pbkdf2-sha256` of the word `password` using 150000 iterations and `E86a9YMX3zC7` as salt must be stored as `$pbkdf2-sha256$150000$E86a9YMX3zC7$R5J62hsSq+pYw00hLLPKBbcGXmq7fj5+/M0IFoYtZbo=`. For bcrypt the format must be the one supported by golang's [crypto/bcrypt](https://godoc.org/golang.org/x/crypto/bcrypt) package, for example the password `secret` with cost `14` must be stored as `$2a$14$ajq8Q7fbtFRQvXpdCq7Jcuy.Rx1h/L4J60Otx.gyNLbAYctGMJ9tK`. For sha512crypt we support the format used in `/etc/shadow` with the `$6$` prefix, this is useful if you are migrating from Unix system user accounts. Using the REST API you can send a password hashed as bcrypt, pbkdf2 or sha512crypt and it will be stored as is. +- `password` used for password authentication. For users created using SFTPGo REST API if the password has no known hashing algo prefix it will be stored using argon2id. SFTPGo supports checking passwords stored with bcrypt, pbkdf2, md5crypt and sha512crypt too. For pbkdf2 the supported format is `$$$$`, where algo is `pbkdf2-sha1` or `pbkdf2-sha256` or `pbkdf2-sha512`. For example the `pbkdf2-sha256` of the word `password` using 150000 iterations and `E86a9YMX3zC7` as salt must be stored as `$pbkdf2-sha256$150000$E86a9YMX3zC7$R5J62hsSq+pYw00hLLPKBbcGXmq7fj5+/M0IFoYtZbo=`. For bcrypt the format must be the one supported by golang's [crypto/bcrypt](https://godoc.org/golang.org/x/crypto/bcrypt) package, for example the password `secret` with cost `14` must be stored as `$2a$14$ajq8Q7fbtFRQvXpdCq7Jcuy.Rx1h/L4J60Otx.gyNLbAYctGMJ9tK`. For md5crypt and sha512crypt we support the format used in `/etc/shadow` with the `$1$` and `$6$` prefix, this is useful if you are migrating from Unix system user accounts. Using the REST API you can send a password hashed as bcrypt, pbkdf2, md5crypt or sha512crypt and it will be stored as is. - `public_keys` array of public keys. At least one public key or the password is mandatory. - `status` 1 means "active", 0 "inactive". An inactive account cannot login. - `expiration_date` expiration date as unix timestamp in milliseconds. An expired account cannot login. 0 means no expiration. diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go index afe691aa..7e32910a 100644 --- a/dataprovider/dataprovider.go +++ b/dataprovider/dataprovider.go @@ -32,7 +32,7 @@ import ( "github.com/drakkan/sftpgo/logger" "github.com/drakkan/sftpgo/metrics" "github.com/drakkan/sftpgo/utils" - sha512crypt "github.com/nathanaelle/password" + unixcrypt "github.com/nathanaelle/password" ) const ( @@ -52,6 +52,7 @@ const ( pbkdf2SHA1Prefix = "$pbkdf2-sha1$" pbkdf2SHA256Prefix = "$pbkdf2-sha256$" pbkdf2SHA512Prefix = "$pbkdf2-sha512$" + md5cryptPwdPrefix = "$1$" sha512cryptPwdPrefix = "$6$" manageUsersDisabledError = "please set manage_users to 1 in your configuration to enable this method" trackQuotaDisabledError = "please enable track_quota in your configuration to use this method" @@ -71,11 +72,13 @@ var ( provider Provider sqlPlaceholders []string hashPwdPrefixes = []string{argonPwdPrefix, bcryptPwdPrefix, pbkdf2SHA1Prefix, pbkdf2SHA256Prefix, - pbkdf2SHA512Prefix, sha512cryptPwdPrefix} + pbkdf2SHA512Prefix, md5cryptPwdPrefix, sha512cryptPwdPrefix} pbkdfPwdPrefixes = []string{pbkdf2SHA1Prefix, pbkdf2SHA256Prefix, pbkdf2SHA512Prefix} + unixPwdPrefixes = []string{md5cryptPwdPrefix, sha512cryptPwdPrefix} logSender = "dataProvider" availabilityTicker *time.Ticker availabilityTickerDone chan bool + errWrongPassword = errors.New("password does not match") ) // Actions to execute on user create, update, delete. @@ -435,7 +438,7 @@ func checkUserAndPass(user User, password string) (User, error) { if len(user.Password) == 0 { return user, errors.New("Credentials cannot be null or empty") } - var match bool + match := false if strings.HasPrefix(user.Password, argonPwdPrefix) { match, err = argon2id.ComparePasswordAndHash(password, user.Password) if err != nil { @@ -451,22 +454,13 @@ func checkUserAndPass(user User, password string) (User, error) { } else if utils.IsStringPrefixInSlice(user.Password, pbkdfPwdPrefixes) { match, err = comparePbkdf2PasswordAndHash(password, user.Password) if err != nil { - providerLog(logger.LevelWarn, "error comparing password with pbkdf2 sha256 hash: %v", err) return user, err } - } else if strings.HasPrefix(user.Password, sha512cryptPwdPrefix) { - crypter, ok := sha512crypt.SHA512.CrypterFound(user.Password) - if !ok { - err = errors.New("cannot found matching SHA512 crypter") - providerLog(logger.LevelWarn, "error comparing password with SHA512 hash: %v", err) + } else if utils.IsStringPrefixInSlice(user.Password, unixPwdPrefixes) { + match, err = compareUnixPasswordAndHash(user, password) + if err != nil { return user, err } - if !crypter.Verify([]byte(password)) { - err = errors.New("password does not match") - providerLog(logger.LevelWarn, "error comparing password with SHA512 hash: %v", err) - return user, err - } - match = true } if !match { err = errors.New("Invalid credentials") @@ -496,6 +490,37 @@ func checkUserAndPubKey(user User, pubKey string) (User, string, error) { return user, "", errors.New("Invalid credentials") } +func compareUnixPasswordAndHash(user User, password string) (bool, error) { + match := false + var err error + if strings.HasPrefix(user.Password, sha512cryptPwdPrefix) { + crypter, ok := unixcrypt.SHA512.CrypterFound(user.Password) + if !ok { + err = errors.New("cannot found matching SHA512 crypter") + providerLog(logger.LevelWarn, "error comparing password with SHA512 crypt hash: %v", err) + return match, err + } + if !crypter.Verify([]byte(password)) { + return match, errWrongPassword + } + match = true + } else if strings.HasPrefix(user.Password, md5cryptPwdPrefix) { + crypter, ok := unixcrypt.MD5.CrypterFound(user.Password) + if !ok { + err = errors.New("cannot found matching MD5 crypter") + providerLog(logger.LevelWarn, "error comparing password with MD5 crypt hash: %v", err) + return match, err + } + if !crypter.Verify([]byte(password)) { + return match, errWrongPassword + } + match = true + } else { + err = errors.New("unix crypt: invalid or unsupported hash format") + } + return match, err +} + func comparePbkdf2PasswordAndHash(password, hashedPassword string) (bool, error) { vals := strings.Split(hashedPassword, "$") if len(vals) != 5 { diff --git a/sftpd/handler.go b/sftpd/handler.go index 4e21afe1..a72b0ef3 100644 --- a/sftpd/handler.go +++ b/sftpd/handler.go @@ -150,8 +150,6 @@ func (c Connection) Filecmd(request *sftp.Request) error { return err } - isHomeDir := c.User.GetRelativePath(p) == "/" - c.Log(logger.LevelDebug, logSender, "new cmd, method: %v, sourcePath: %#v, targetPath: %#v", request.Method, p, target) @@ -159,19 +157,11 @@ func (c Connection) Filecmd(request *sftp.Request) error { case "Setstat": return c.handleSFTPSetstat(p, request) case "Rename": - if isHomeDir { - c.Log(logger.LevelWarn, logSender, "renaming root dir is not allowed") - return sftp.ErrSSHFxPermissionDenied - } if err = c.handleSFTPRename(p, target); err != nil { return err } break case "Rmdir": - if isHomeDir { - c.Log(logger.LevelWarn, logSender, "removing root dir is not allowed") - return sftp.ErrSSHFxPermissionDenied - } return c.handleSFTPRmdir(p) case "Mkdir": @@ -181,10 +171,6 @@ func (c Connection) Filecmd(request *sftp.Request) error { } break case "Symlink": - if isHomeDir { - c.Log(logger.LevelWarn, logSender, "symlinking root dir is not allowed") - return sftp.ErrSSHFxPermissionDenied - } if err = c.handleSFTPSymlink(p, target); err != nil { return err } @@ -324,6 +310,10 @@ func (c Connection) handleSFTPSetstat(path string, request *sftp.Request) error } func (c Connection) handleSFTPRename(sourcePath string, targetPath string) error { + if c.User.GetRelativePath(sourcePath) == "/" { + c.Log(logger.LevelWarn, logSender, "renaming root dir is not allowed") + return sftp.ErrSSHFxPermissionDenied + } if !c.User.HasPerm(dataprovider.PermRename, filepath.Dir(targetPath)) { return sftp.ErrSSHFxPermissionDenied } @@ -337,6 +327,10 @@ func (c Connection) handleSFTPRename(sourcePath string, targetPath string) error } func (c Connection) handleSFTPRmdir(path string) error { + if c.User.GetRelativePath(path) == "/" { + c.Log(logger.LevelWarn, logSender, "removing root dir is not allowed") + return sftp.ErrSSHFxPermissionDenied + } if !c.User.HasPerm(dataprovider.PermDelete, filepath.Dir(path)) { return sftp.ErrSSHFxPermissionDenied } @@ -362,6 +356,10 @@ func (c Connection) handleSFTPRmdir(path string) error { } func (c Connection) handleSFTPSymlink(sourcePath string, targetPath string) error { + if c.User.GetRelativePath(sourcePath) == "/" { + c.Log(logger.LevelWarn, logSender, "symlinking root dir is not allowed") + return sftp.ErrSSHFxPermissionDenied + } if !c.User.HasPerm(dataprovider.PermCreateSymlinks, filepath.Dir(targetPath)) { return sftp.ErrSSHFxPermissionDenied } diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go index a615c195..ae6eb943 100644 --- a/sftpd/sftpd_test.go +++ b/sftpd/sftpd_test.go @@ -1694,6 +1694,39 @@ func TestPasswordsHashSHA512Crypt(t *testing.T) { os.RemoveAll(user.GetHomeDir()) } +func TestPasswordsHashMD5Crypt(t *testing.T) { + md5CryptPwd := "$1$b5caebda$VODr/nyhGWgZaY8sJ4x05." + clearPwd := "password" + usePubKey := false + u := getTestUser(usePubKey) + u.Password = md5CryptPwd + user, _, err := httpd.AddUser(u, http.StatusOK) + if err != nil { + t.Errorf("unable to add user: %v", err) + } + user.Password = clearPwd + client, err := getSftpClient(user, usePubKey) + if err != nil { + t.Errorf("unable to login with md5 crypt password: %v", err) + } else { + defer client.Close() + _, err = client.Getwd() + if err != nil { + t.Errorf("unable to get working dir with md5 crypt password: %v", err) + } + } + user.Password = md5CryptPwd + _, err = getSftpClient(user, usePubKey) + if err == nil { + t.Errorf("login with wrong password must fail") + } + _, err = httpd.RemoveUser(user, http.StatusOK) + if err != nil { + t.Errorf("unable to remove user: %v", err) + } + os.RemoveAll(user.GetHomeDir()) +} + func TestPermList(t *testing.T) { usePubKey := true u := getTestUser(usePubKey)