diff --git a/builtin/credential/approle/backend_test.go b/builtin/credential/approle/backend_test.go index 044f02d2a2..212fe36f0f 100644 --- a/builtin/credential/approle/backend_test.go +++ b/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() { diff --git a/builtin/credential/approle/path_login.go b/builtin/credential/approle/path_login.go index 5822519b1c..2ad3924ba9 100644 --- a/builtin/credential/approle/path_login.go +++ b/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 diff --git a/builtin/credential/approle/path_role_test.go b/builtin/credential/approle/path_role_test.go index 8ab3bfc666..06706dda5f 100644 --- a/builtin/credential/approle/path_role_test.go +++ b/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() { diff --git a/builtin/credential/ldap/backend.go b/builtin/credential/ldap/backend.go index d318fe4b4c..35e0f102c3 100644 --- a/builtin/credential/ldap/backend.go +++ b/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") diff --git a/builtin/credential/userpass/path_login.go b/builtin/credential/userpass/path_login.go index 95d63f28c6..8e3f42ea4f 100644 --- a/builtin/credential/userpass/path_login.go +++ b/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 { - return logical.ErrorResponse("invalid username or password"), 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 + } + return logical.ErrorResponse("invalid username or password"), logical.ErrInvalidCredentials } - } else { + default: if subtle.ConstantTimeCompare(userPassword, passwordBytes) != 1 { - return logical.ErrorResponse("invalid username or password"), 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 + } + return logical.ErrorResponse("invalid username or password"), logical.ErrInvalidCredentials } + } if userError != nil { diff --git a/changelog/17104.txt b/changelog/17104.txt new file mode 100644 index 0000000000..00c5eebf39 --- /dev/null +++ b/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. +``` \ No newline at end of file diff --git a/sdk/logical/error.go b/sdk/logical/error.go index 02f68dd918..68c8e13732 100644 --- a/sdk/logical/error.go +++ b/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") diff --git a/sdk/logical/response_util.go b/sdk/logical/response_util.go index 7454189f1d..a269fc639b 100644 --- a/sdk/logical/response_util.go +++ b/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 } } diff --git a/sdk/logical/response_util_test.go b/sdk/logical/response_util_test.go index 823c4afbdc..00d70a5c4a 100644 --- a/sdk/logical/response_util_test.go +++ b/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 {