TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-08-12 03:47:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: prevent error reason leakage in case of IgnoreUnknownUsernames (#8372)

Browse Source 
# Which Problems Are Solved

ZITADEL administrators can enable a setting called "Ignoring unknown
usernames" which helps mitigate attacks that try to guess/enumerate
usernames. If enabled, ZITADEL will show the password prompt even if the
user doesn't exist and report "Username or Password invalid".
Due to a implementation change to prevent deadlocks calling the
database, the flag would not be correctly respected in all cases and an
attacker would gain information if an account exist within ZITADEL,
since the error message shows "object not found" instead of the generic
error message.

# How the Problems Are Solved

- Proper check of the error using an error function / type and
`errors.Is`

# Additional Changes

None.

# Additional Context

- raised in a support request

Co-authored-by: Silvan <silvan.reusser@gmail.com>
This commit is contained in:
Livio Spring
2024-07-31 14:23:57 +02:00
committed by GitHub
parent 189505c80f
commit a1d24353db
6 changed files with 207 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
internal/auth/repository/eventsourcing/eventstore/auth_request.go
View File

@@ -2,6 +2,7 @@ package eventstore
import (
"context"
"errors"
"slices"
"strings"
"time"
@@ -29,6 +30,12 @@ import (
const unknownUserID = "UNKNOWN"
var (
ErrUserNotFound = func(err error) error {
return zerrors.ThrowNotFound(err, "EVENT-hodc6", "Errors.User.NotFound")
}
)
type AuthRequestRepo struct {
Command *command.Commands
Query *query.Queries
@@ -53,6 +60,7 @@ type AuthRequestRepo struct {
ApplicationProvider applicationProvider
CustomTextProvider customTextProvider
PasswordReset passwordReset
PasswordChecker passwordChecker
IdGenerator id.Generator
}
@@ -72,7 +80,7 @@ type userSessionViewProvider interface {
}
type userViewProvider interface {
UserByID(string, string) (*user_view_model.UserView, error)
UserByID(context.Context, string, string) (*user_view_model.UserView, error)
}
type loginPolicyViewProvider interface {
@@ -131,6 +139,10 @@ type passwordReset interface {
RequestSetPassword(ctx context.Context, userID, resourceOwner string, notifyType domain.NotificationType, authRequestID string) (objectDetails *domain.ObjectDetails, err error)
}
type passwordChecker interface {
HumanCheckPassword(ctx context.Context, resourceOwner, userID, password string, authReq *domain.AuthRequest) error
}
func (repo *AuthRequestRepo) Health(ctx context.Context) error {
return repo.AuthRequests.Health(ctx)
}
@@ -347,23 +359,25 @@ func (repo *AuthRequestRepo) VerifyPassword(ctx context.Context, authReqID, user
request, err := repo.getAuthRequestEnsureUser(ctx, authReqID, userAgentID, userID)
if err != nil {
if isIgnoreUserNotFoundError(err, request) {
// use the same errorID as below (otherwise it would expose the error reason)
return zerrors.ThrowInvalidArgument(nil, "EVENT-SDe2f", "Errors.User.UsernameOrPassword.Invalid")
}
return err
}
err = repo.Command.HumanCheckPassword(ctx, resourceOwner, userID, password, request.WithCurrentInfo(info))
err = repo.PasswordChecker.HumanCheckPassword(ctx, resourceOwner, userID, password, request.WithCurrentInfo(info))
if isIgnoreUserInvalidPasswordError(err, request) {
return zerrors.ThrowInvalidArgument(nil, "EVENT-Jsf32", "Errors.User.UsernameOrPassword.Invalid")
// use the same errorID as above (otherwise it would expose the error reason)
return zerrors.ThrowInvalidArgument(nil, "EVENT-SDe2f", "Errors.User.UsernameOrPassword.Invalid")
}
return err
}
func isIgnoreUserNotFoundError(err error, request *domain.AuthRequest) bool {
return request != nil && request.LoginPolicy != nil && request.LoginPolicy.IgnoreUnknownUsernames && zerrors.IsNotFound(err) && zerrors.Contains(err, "Errors.User.NotFound")
return request != nil && request.LoginPolicy != nil && request.LoginPolicy.IgnoreUnknownUsernames && errors.Is(err, ErrUserNotFound(nil))
}
func isIgnoreUserInvalidPasswordError(err error, request *domain.AuthRequest) bool {
return request != nil && request.LoginPolicy != nil && request.LoginPolicy.IgnoreUnknownUsernames && zerrors.IsErrorInvalidArgument(err) && zerrors.Contains(err, "Errors.User.Password.Invalid")
return request != nil && request.LoginPolicy != nil && request.LoginPolicy.IgnoreUnknownUsernames && errors.Is(err, command.ErrPasswordInvalid(nil))
}
func lockoutPolicyToDomain(policy *query.LockoutPolicy) *domain.LockoutPolicy {
@@ -1646,7 +1660,7 @@ func userByID(ctx context.Context, viewProvider userViewProvider, eventProvider
ctx, span := tracing.NewSpan(ctx)
defer func() { span.EndWithError(err) }()
user, viewErr := viewProvider.UserByID(userID, authz.GetInstance(ctx).InstanceID())
user, viewErr := viewProvider.UserByID(ctx, userID, authz.GetInstance(ctx).InstanceID())
if viewErr != nil && !zerrors.IsNotFound(viewErr) {
return nil, viewErr
} else if user == nil {
@@ -1659,9 +1673,10 @@ func userByID(ctx context.Context, viewProvider userViewProvider, eventProvider
}
if len(events) == 0 {
if viewErr != nil {
return nil, viewErr
// We already returned all errors apart from not found, but need to make sure that can be checked in case IgnoreUnknownUsernames option is active.
return nil, ErrUserNotFound(viewErr)
}
return user_view_model.UserToModel(user), viewErr
return user_view_model.UserToModel(user), nil
}
userCopy := *user
for _, event := range events {