Return errInvalidCredentials when wrong credentials is provided for existent users (#17104)

* adding errInvalidCredentials

* fixing tests

* add changelog

* fixing fmt errors

* test if routeErr is seen externally and fixing error comment

* adding fmt changes

* adding comments

builtin/credential/approle/backend_test.go

@@ -174,7 +174,7 @@ func TestAppRole_RoleNameCaseSensitivity(t *testing.T) {
 			},
 			Storage: s,
 		})
-		if err != nil {
+		if err != nil && err != logical.ErrInvalidCredentials {
 			t.Fatal(err)
 		}
 		if resp == nil || !resp.IsError() {
@@ -233,7 +233,7 @@ func TestAppRole_RoleNameCaseSensitivity(t *testing.T) {
 			},
 			Storage: s,
 		})
-		if err != nil {
+		if err != nil && err != logical.ErrInvalidCredentials {
 			t.Fatal(err)
 		}
 		if resp == nil || !resp.IsError() {
@@ -292,7 +292,7 @@ func TestAppRole_RoleNameCaseSensitivity(t *testing.T) {
 			},
 			Storage: s,
 		})
-		if err != nil {
+		if err != nil && err != logical.ErrInvalidCredentials {
 			t.Fatal(err)
 		}
 		if resp == nil || !resp.IsError() {
@@ -351,7 +351,7 @@ func TestAppRole_RoleNameCaseSensitivity(t *testing.T) {
 			},
 			Storage: s,
 		})
-		if err != nil {
+		if err != nil && err != logical.ErrInvalidCredentials {
 			t.Fatal(err)
 		}
 		if resp == nil || !resp.IsError() {
@@ -410,7 +410,7 @@ func TestAppRole_RoleNameCaseSensitivity(t *testing.T) {
 			},
 			Storage: s,
 		})
-		if err != nil {
+		if err != nil && err != logical.ErrInvalidCredentials {
 			t.Fatal(err)
 		}
 		if resp == nil || !resp.IsError() {

builtin/credential/approle/path_login.go

@@ -155,7 +155,7 @@ func (b *backend) pathLoginUpdate(ctx context.Context, req *logical.Request, dat
 			return nil, err
 		}
 		if entry == nil {
-			return logical.ErrorResponse("invalid secret id"), nil
+			return logical.ErrorResponse("invalid secret id"), logical.ErrInvalidCredentials
 		}
 
 		// If a secret ID entry does not have a corresponding accessor

builtin/credential/approle/path_role_test.go

@@ -381,7 +381,7 @@ func TestAppRole_RoleNameLowerCasing(t *testing.T) {
 			"secret_id": secretID,
 		},
 	})
-	if err != nil {
+	if err != nil && err != logical.ErrInvalidCredentials {
 		t.Fatal(err)
 	}
 	if resp == nil || !resp.IsError() {

builtin/credential/ldap/backend.go

@@ -107,7 +107,7 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
 		if b.Logger().IsDebug() {
 			b.Logger().Debug("ldap bind failed", "error", err)
 		}
-		return "", nil, logical.ErrorResponse(errUserBindFailed), nil, nil
+		return "", nil, logical.ErrorResponse(errUserBindFailed), nil, logical.ErrInvalidCredentials
 	}
 
 	// We re-bind to the BindDN if it's defined because we assume
@@ -117,7 +117,7 @@ func (b *backend) Login(ctx context.Context, req *logical.Request, username stri
 			if b.Logger().IsDebug() {
 				b.Logger().Debug("error while attempting to re-bind with the BindDN User", "error", err)
 			}
-			return "", nil, logical.ErrorResponse("ldap operation failed: failed to re-bind with the BindDN user"), nil, nil
+			return "", nil, logical.ErrorResponse("ldap operation failed: failed to re-bind with the BindDN user"), nil, logical.ErrInvalidCredentials
 		}
 		if b.Logger().IsDebug() {
 			b.Logger().Debug("re-bound to original binddn")

builtin/credential/userpass/path_login.go

@@ -86,14 +86,26 @@ func (b *backend) pathLogin(ctx context.Context, req *logical.Request, d *framew
 	// Check for a password match. Check for a hash collision for Vault 0.2+,
 	// but handle the older legacy passwords with a constant time comparison.
 	passwordBytes := []byte(password)
-	if !legacyPassword {
+	switch {
+	case !legacyPassword:
 		if err := bcrypt.CompareHashAndPassword(userPassword, passwordBytes); err != nil {
+			// The failed login info of existing users alone are tracked as only
+			// existing user's failed login information is stored in storage for optimization
+			if user == nil || userError != nil {
 				return logical.ErrorResponse("invalid username or password"), nil
 			}
-	} else {
+			return logical.ErrorResponse("invalid username or password"), logical.ErrInvalidCredentials
+		}
+	default:
 		if subtle.ConstantTimeCompare(userPassword, passwordBytes) != 1 {
+			// The failed login info of existing users alone are tracked as only
+			// existing user's failed login information is stored in storage for optimization
+			if user == nil || userError != nil {
 				return logical.ErrorResponse("invalid username or password"), nil
 			}
+			return logical.ErrorResponse("invalid username or password"), logical.ErrInvalidCredentials
+		}
+
 	}
 
 	if userError != nil {

changelog/17104.txt

@@ -0,0 +1,4 @@
+```release-note:change
+auth: Returns invalid credentials for ldap, userpass and approle when wrong credentials are provided for existent users.
+This will only be used internally for implementing user lockout. 
+```

sdk/logical/error.go

@@ -17,6 +17,11 @@ var (
 	// ErrPermissionDenied is returned if the client is not authorized
 	ErrPermissionDenied = errors.New("permission denied")
 
+	// ErrInvalidCredentials is returned when the provided credentials are incorrect
+	// This is used internally for user lockout purposes. This is not seen externally.
+	// The status code returned does not change because of this error
+	ErrInvalidCredentials = errors.New("invalid credentials")
+
 	// ErrMultiAuthzPending is returned if the the request needs more
 	// authorizations
 	ErrMultiAuthzPending = errors.New("request needs further approval")

sdk/logical/response_util.go

@@ -122,6 +122,8 @@ func RespondErrorCommon(req *Request, resp *Response, err error) (int, error) {
 			statusCode = http.StatusNotFound
 		case errwrap.Contains(err, ErrRelativePath.Error()):
 			statusCode = http.StatusBadRequest
+		case errwrap.Contains(err, ErrInvalidCredentials.Error()):
+			statusCode = http.StatusBadRequest
 		}
 	}
 

sdk/logical/response_util_test.go

@@ -1,6 +1,7 @@
 package logical
 
 import (
+	"errors"
 	"strings"
 	"testing"
 )
@@ -60,6 +61,17 @@ func TestResponseUtil_RespondErrorCommon_basic(t *testing.T) {
 			respErr:        nil,
 			expectedStatus: 0,
 		},
+		{
+			title:   "Invalid Credentials error ",
+			respErr: ErrInvalidCredentials,
+			resp: &Response{
+				Data: map[string]interface{}{
+					"error": "error due to wrong credentials",
+				},
+			},
+			expectedErr:    errors.New("error due to wrong credentials"),
+			expectedStatus: 400,
+		},
 	}
 
 	for _, tc := range testCases {