Authentication errors: try to avoid user enumeration

Fixes #395

dataprovider/admin.go

@@ -125,7 +125,7 @@ func (a *Admin) validate() error {
 func (a *Admin) CheckPassword(password string) (bool, error) {
 	if strings.HasPrefix(a.Password, bcryptPwdPrefix) {
 		if err := bcrypt.CompareHashAndPassword([]byte(a.Password), []byte(password)); err != nil {
-			return false, err
+			return false, ErrInvalidCredentials
 		}
 		return true, nil
 	}

dataprovider/bolt.go

@@ -111,7 +111,7 @@ func (p *BoltProvider) validateUserAndTLSCert(username, protocol string, tlsCert
 	user, err := p.userExists(username)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating user %#v: %v", username, err)
-		return user, err
+		return user, ErrInvalidCredentials
 	}
 	return checkUserAndTLSCertificate(&user, protocol, tlsCert)
 }
@@ -124,7 +124,7 @@ func (p *BoltProvider) validateUserAndPass(username, password, ip, protocol stri
 	user, err := p.userExists(username)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating user %#v: %v", username, err)
-		return user, err
+		return user, ErrInvalidCredentials
 	}
 	return checkUserAndPass(&user, password, ip, protocol)
 }
@@ -133,7 +133,7 @@ func (p *BoltProvider) validateAdminAndPass(username, password, ip string) (Admi
 	admin, err := p.adminExists(username)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating admin %#v: %v", username, err)
-		return admin, err
+		return admin, ErrInvalidCredentials
 	}
 	err = admin.checkUserAndPass(password, ip)
 	return admin, err
@@ -147,7 +147,7 @@ func (p *BoltProvider) validateUserAndPubKey(username string, pubKey []byte) (Us
 	user, err := p.userExists(username)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating user %#v: %v", username, err)
-		return user, "", err
+		return user, "", ErrInvalidCredentials
 	}
 	return checkUserAndPubKey(&user, pubKey)
 }

dataprovider/dataprovider.go

@@ -1636,7 +1636,7 @@ func isPasswordOK(user *User, password string) (bool, error) {
 	} else if strings.HasPrefix(user.Password, bcryptPwdPrefix) {
 		if err = bcrypt.CompareHashAndPassword([]byte(user.Password), []byte(password)); err != nil {
 			providerLog(logger.LevelWarn, "error comparing password with bcrypt hash: %v", err)
-			return match, err
+			return match, ErrInvalidCredentials
 		}
 		match = true
 	} else if utils.IsStringPrefixInSlice(user.Password, pbkdfPwdPrefixes) {

dataprovider/memory.go

@@ -96,7 +96,7 @@ func (p *MemoryProvider) validateUserAndTLSCert(username, protocol string, tlsCe
 	user, err := p.userExists(username)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating user %#v: %v", username, err)
-		return user, err
+		return user, ErrInvalidCredentials
 	}
 	return checkUserAndTLSCertificate(&user, protocol, tlsCert)
 }
@@ -109,7 +109,7 @@ func (p *MemoryProvider) validateUserAndPass(username, password, ip, protocol st
 	user, err := p.userExists(username)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating user %#v: %v", username, err)
-		return user, err
+		return user, ErrInvalidCredentials
 	}
 	return checkUserAndPass(&user, password, ip, protocol)
 }
@@ -122,7 +122,7 @@ func (p *MemoryProvider) validateUserAndPubKey(username string, pubKey []byte) (
 	user, err := p.userExists(username)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating user %#v: %v", username, err)
-		return user, "", err
+		return user, "", ErrInvalidCredentials
 	}
 	return checkUserAndPubKey(&user, pubKey)
 }
@@ -131,7 +131,7 @@ func (p *MemoryProvider) validateAdminAndPass(username, password, ip string) (Ad
 	admin, err := p.adminExists(username)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating admin %#v: %v", username, err)
-		return admin, err
+		return admin, ErrInvalidCredentials
 	}
 	err = admin.checkUserAndPass(password, ip)
 	return admin, err

dataprovider/sqlcommon.go

@@ -53,7 +53,7 @@ func sqlCommonValidateAdminAndPass(username, password, ip string, dbHandle *sql.
 	admin, err := sqlCommonGetAdminByUsername(username, dbHandle)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating admin %#v: %v", username, err)
-		return admin, err
+		return admin, ErrInvalidCredentials
 	}
 	err = admin.checkUserAndPass(password, ip)
 	return admin, err
@@ -224,7 +224,7 @@ func sqlCommonValidateUserAndPass(username, password, ip, protocol string, dbHan
 	user, err := sqlCommonGetUserByUsername(username, dbHandle)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating user %#v: %v", username, err)
-		return user, err
+		return user, ErrInvalidCredentials
 	}
 	return checkUserAndPass(&user, password, ip, protocol)
 }
@@ -237,7 +237,7 @@ func sqlCommonValidateUserAndTLSCertificate(username, protocol string, tlsCert *
 	user, err := sqlCommonGetUserByUsername(username, dbHandle)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating user %#v: %v", username, err)
-		return user, err
+		return user, ErrInvalidCredentials
 	}
 	return checkUserAndTLSCertificate(&user, protocol, tlsCert)
 }
@@ -250,7 +250,7 @@ func sqlCommonValidateUserAndPubKey(username string, pubKey []byte, dbHandle *sq
 	user, err := sqlCommonGetUserByUsername(username, dbHandle)
 	if err != nil {
 		providerLog(logger.LevelWarn, "error authenticating user %#v: %v", username, err)
-		return user, "", err
+		return user, "", ErrInvalidCredentials
 	}
 	return checkUserAndPubKey(&user, pubKey)
 }

ftpd/ftpd_test.go

@@ -562,13 +562,21 @@ func TestBasicFTPHandling(t *testing.T) {
 	assert.Eventually(t, func() bool { return len(common.Connections.GetStats()) == 0 }, 1*time.Second, 50*time.Millisecond)
 }
 
-func TestLoginInvalidPwd(t *testing.T) {
+func TestLoginInvalidCredetials(t *testing.T) {
 	u := getTestUser()
 	user, _, err := httpdtest.AddUser(u, http.StatusCreated)
 	assert.NoError(t, err)
-	user.Password = "wrong"
+	user.Username = "wrong username"
 	_, err = getFTPClient(user, false, nil)
-	assert.Error(t, err)
+	if assert.Error(t, err) {
+		assert.Contains(t, err.Error(), dataprovider.ErrInvalidCredentials.Error())
+	}
+	user.Username = u.Username
+	user.Password = "wrong pwd"
+	_, err = getFTPClient(user, false, nil)
+	if assert.Error(t, err) {
+		assert.Contains(t, err.Error(), dataprovider.ErrInvalidCredentials.Error())
+	}
 	_, err = httpdtest.RemoveUser(user, http.StatusOK)
 	assert.NoError(t, err)
 }

httpd/httpd_test.go

@@ -52,6 +52,7 @@ const (
 	altAdminUsername          = "newTestAdmin"
 	altAdminPassword          = "password1"
 	csrfFormToken             = "_form_token"
+	tokenPath                 = "/api/v2/token"
 	userPath                  = "/api/v2/users"
 	adminPath                 = "/api/v2/admins"
 	adminPwdPath              = "/api/v2/changepwd/admin"
@@ -427,6 +428,39 @@ func TestAdminPasswordHashing(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestAdminInvalidCredentials(t *testing.T) {
+	req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%v%v", httpBaseURL, tokenPath), nil)
+	assert.NoError(t, err)
+	req.SetBasicAuth(defaultTokenAuthUser, defaultTokenAuthPass)
+	resp, err := httpclient.GetHTTPClient().Do(req)
+	assert.NoError(t, err)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+	err = resp.Body.Close()
+	assert.NoError(t, err)
+	// wrong password
+	req.SetBasicAuth(defaultTokenAuthUser, "wrong pwd")
+	resp, err = httpclient.GetHTTPClient().Do(req)
+	assert.NoError(t, err)
+	assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
+	responseHolder := make(map[string]interface{})
+	err = render.DecodeJSON(resp.Body, &responseHolder)
+	assert.NoError(t, err)
+	err = resp.Body.Close()
+	assert.NoError(t, err)
+	assert.Equal(t, dataprovider.ErrInvalidCredentials.Error(), responseHolder["error"].(string))
+	// wrong username
+	req.SetBasicAuth("wrong username", defaultTokenAuthPass)
+	resp, err = httpclient.GetHTTPClient().Do(req)
+	assert.NoError(t, err)
+	assert.Equal(t, http.StatusUnauthorized, resp.StatusCode)
+	responseHolder = make(map[string]interface{})
+	err = render.DecodeJSON(resp.Body, &responseHolder)
+	assert.NoError(t, err)
+	err = resp.Body.Close()
+	assert.NoError(t, err)
+	assert.Equal(t, dataprovider.ErrInvalidCredentials.Error(), responseHolder["error"].(string))
+}
+
 func TestAdminAllowList(t *testing.T) {
 	a := getTestAdmin()
 	a.Username = altAdminUsername