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
2019-12-29 07:43:59 +01:00 · 2019-12-29 07:43:59 +01:00 · 6aff8c2f5e
commit 6aff8c2f5e
parent e5770af2fa
4 changed files with 86 additions and 30 deletions
--- 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 `$<algo>$<iterations>$<salt>$<hashed pwd base64 encoded>`, 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 `$<algo>$<iterations>$<salt>$<hashed pwd base64 encoded>`, 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.
--- 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 {
--- 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
 	}
--- 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)