Re: [Django] #36651: Brute-force password attack against inactive users returns distinct error message

[Django]

#36651: Brute-force password attack against inactive users returns distinct error
message
-------------------------------------+-------------------------------------
Reporter: heindrickdumdum0217 | Owner: (none)
Type: Bug | Status: closed
Component: contrib.auth | Version: 5.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by heindrickdumdum0217):

Replying to [comment:2 Jacob Walls]:
> As mentioned in the ticket submission form, security-related reports are
not to be submitted here. They should be sent to
secu...@djangoproject.com instead.
>
> That said, we do not consider this a security issue. If the user is
active, brute-forcing the password results in successful authentication.
The techniques to protect against this are well-known, including requiring
strong passwords and rate-limiting requests to authentication endpoints.
>
> Here you have raised the case where the user is inactive and
authentication does not succeed, but the correctness of the password can
be inferred from the variance in the error. But in the active user case,
it's already "game over" if the password can be brute-forced. We wouldn't
add complexity to treat the inactive user case differently. Moreover,
reversing the order of conditions could cause an account enumeration
attack, see #20760.

Hi Jacob
Thanks for your comment.

Brute-forcing doesn't work for active user, becuase after 3 consecutive
failed login attempts, account will be locked.
Of course we have implemented rate-limit too.

But even though we have rate-limit, it's still less secure, because
hackers can try with different passwords as many as times until they reach
rate-limit for inactive user.
--
Ticket URL: <https://code.djangoproject.com/ticket/36651#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36651: Brute-force password attack against inactive users returns distinct error
message
-------------------------------------+-------------------------------------
Reporter: heindrickdumdum0217 | Owner: (none)
Type: Bug | Status: closed
Component: contrib.auth | Version: 5.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by heindrickdumdum0217:

Old description:

> https://github.com/django/django/blob/main/django/contrib/auth/backends.py#L71
>

> {{{
> def authenticate(self, request, username=None, password=None,
> **kwargs):
> if username is None:
> username = kwargs.get(UserModel.USERNAME_FIELD)
> if username is None or password is None:
> return
> try:
> user =
> UserModel._default_manager.get_by_natural_key(username)
> except UserModel.DoesNotExist:
> # Run the default password hasher once to reduce the timing
> # difference between an existing and a nonexistent user
> (#20760).
> UserModel().set_password(password)
> else:
> if user.check_password(password) and
> self.user_can_authenticate(user):
> return user
> }}}
>

> We have implemented user account lock after 3 consecutive failed login
> attempts.
> When user try to login in 4-th item we have to show correct error message
> about user account is locked, but for now it's impossible without
> rewriting "authenticate" function again.
>
> But the current code checks password first, then check user can
> authenticate.
> It means if user receives different error message, user can sure at least
> username and password are correct.
> It may allow hackers can try with different password as many as times
> until they receive different error message.
>
> For example, when password is not match, it returns error message
> ''unable to login with provided credentials''.
> But when acount is locked, it returns error message "your account is
> locked".
>
> This is just an example.
> What I'm going to say is we should check if user can authenticate first
> after get user from username or email.
> Then compare password, otherwise it may allow hackers guess password.
>
> Of course I can inherit ''ModelBackend'' class and update "authenticate"
> function, but I don't think it's a good approach.
> When we inhert, we should use at least super class's function not
> override, because the "authenticate" function can be updated later in
> next Django releases.

New description:

https://github.com/django/django/blob/main/django/contrib/auth/backends.py#L71


{{{
def authenticate(self, request, username=None, password=None,
**kwargs):
if username is None:
username = kwargs.get(UserModel.USERNAME_FIELD)
if username is None or password is None:
return
try:
user = UserModel._default_manager.get_by_natural_key(username)
except UserModel.DoesNotExist:
# Run the default password hasher once to reduce the timing
# difference between an existing and a nonexistent user
(#20760).
UserModel().set_password(password)
else:
if user.check_password(password) and
self.user_can_authenticate(user):
return user
}}}


We have implemented user account lock after 3 consecutive failed login
attempts.
When user try to login in 4-th item we have to show correct error message
about account is locked, but for now it's impossible without rewriting
"authenticate" function again.

The current code checks password first, then check user can authenticate.
It means if user receives different error message, attacker can sure at
least username and password are correct.
It may allow hackers can try with different password as many as times
until they receive different error message.

For example, when password is not match, it returns error message ''unable
to login with provided credentials''.
But when acount is locked, it returns error message "your account is
locked".

This is just an example.
What I'm going to say is we should check if user can authenticate first
after get user from username or email.
Then compare password, otherwise it may allow hackers guess password.

Of course I can inherit ''ModelBackend'' class and update "authenticate"
function, but I don't think it's a good approach.
When we inhert, we should use at least super class's function not
override, because the "authenticate" function can be updated later in next
Django releases.

--
--
Ticket URL: <https://code.djangoproject.com/ticket/36651#comment:5>

[Django]

#36651: Brute-force password attack against inactive users returns distinct error
message
-------------------------------------+-------------------------------------
Reporter: heindrickdumdum0217 | Owner: (none)
Type: Bug | Status: closed
Component: contrib.auth | Version: 5.2
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jake Howard):

The return value for `authenticate` is `User | None` - how would you
imagine distinguishing between "There's a user here, but they're not
active" and "There's no user with these credentials"?

My suggestion would be to not set `is_active`, and instead handle inactive
users in the login view yourselves. [https://github.com/jazzband/django-
axes/ django-axes] is a fairly popular library which implements the
behaviour you're after. If you're not able to use it directly, perhaps it
can aid in your implementation.

Checking whether the user is active before their password would allow for
user enumeration of inactive accounts. A similar vulnerability was fixed
in [https://www.djangoproject.com/weblog/2024/jul/09/security-releases/
CVE-2024-38875]. As Jacob mentions above, brute forcing username and
password is always a vulnerability - all changing the error message does
is identify whether the previous attempts were valid, which yields no
additional information.
--
Ticket URL: <https://code.djangoproject.com/ticket/36651#comment:6>