[Django] #28285: Updating Password Hasher Invalidates Users First Login After Upgrade
Django
-------------------------------------------+------------------------
Reporter: Simon Blanchard | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------------+------------------------
The code below, from contrib.auth.base_user.py, says "hash upgrade is not
a password change"
{{{
def check_password(self, raw_password):
"""
Return a boolean of whether the raw_password was correct. Handles
hashing formats behind the scenes.
"""
def setter(raw_password):
self.set_password(raw_password)
# Password hash upgrades shouldn't be considered password
changes.
self._password = None
self.save(update_fields=["password"])
return check_password(raw_password, self.password, setter)
}}}
but the code in contrib.auth.__init__.py/get_user()
verifies the session by comparing a salted hmac of the password hash to
the same thing stored in the session.
If we update the password hash the salted hmac will change. Therefore, the
session will no longer be considered valid and will be flushed. We can
keep the session alive by updating the HASH_SESSION_KEY in request.session
e.g. via the update_session_auth_hash() function. But this is not called
by the setter function when the hasher is updated.
So the first time the user logs (with the correct password) following an
upgrade, their (valid) session will be flushed and they will be logged out
again.
--
Ticket URL: <https://code.djangoproject.com/ticket/28285>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Simon Blanchard | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.11
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Old description:
> The code below, from contrib.auth.base_user.py, says "hash upgrade is not
> a password change"
>
> {{{
> def check_password(self, raw_password):
> """
> Return a boolean of whether the raw_password was correct. Handles
> hashing formats behind the scenes.
> """
> def setter(raw_password):
> self.set_password(raw_password)
> # Password hash upgrades shouldn't be considered password
> changes.
> self._password = None
> self.save(update_fields=["password"])
> return check_password(raw_password, self.password, setter)
> }}}
>
> but the code in contrib.auth.__init__.py/get_user()
>
> verifies the session by comparing a salted hmac of the password hash to
> the same thing stored in the session.
>
> If we update the password hash the salted hmac will change. Therefore,
> the session will no longer be considered valid and will be flushed. We
> can keep the session alive by updating the HASH_SESSION_KEY in
> request.session e.g. via the update_session_auth_hash() function. But
> this is not called by the setter function when the hasher is updated.
>
> So the first time the user logs (with the correct password) following an
> upgrade, their (valid) session will be flushed and they will be logged
> out again.
New description:
The code below, from `contrib.auth.base_user.py`, says "hash upgrade is
not a password change"
{{{
def check_password(self, raw_password):
"""
Return a boolean of whether the raw_password was correct. Handles
hashing formats behind the scenes.
"""
def setter(raw_password):
self.set_password(raw_password)
# Password hash upgrades shouldn't be considered password
changes.
self._password = None
self.save(update_fields=["password"])
return check_password(raw_password, self.password, setter)
}}}
but the code in `contrib.auth.get_user()` verifies the session by
comparing a salted hmac of the password hash to the same thing stored in
the session.
If we update the password hash the salted hmac will change. Therefore, the
session will no longer be considered valid and will be flushed. We can
keep the session alive by updating the `HASH_SESSION_KEY` in
`request.session` e.g. via the `update_session_auth_hash()` function. But
this is not called by the setter function when the hasher is updated.
So the first time the user logs (with the correct password) following an
upgrade, their (valid) session will be flushed and they will be logged out
again.
--
Comment (by Tim Graham):
Here's my understanding of the scenario:
1. User X logs in and creates session A
2. Number of hash iterations changes in Django due to a software upgrade
3. Session A is still valid at this point
4. For whatever reason (using a different browser perhaps), user X logs in
and creates session B, triggering a password hash upgrade and an
invalidation of session A
I don't think there's a way to keep session A valid is this scenario. Am I
misunderstanding something?
--
Ticket URL: <https://code.djangoproject.com/ticket/28285#comment:1>
Django
Reporter: Simon Blanchard | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.11
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 Simon Blanchard):
Replying to [comment:1 Tim Graham]:
> Here's my understanding of the scenario:
> 1. User X logs in and creates session A
> 2. Number of hash iterations changes in Django due to a software upgrade
> 3. Session A is still valid at this point
I don't think so. Hash changes invalidate the session. Hence the need for
the update_session_auth_hash() function
> 4. For whatever reason (using a different browser perhaps), user X logs
in and creates session B, triggering a password hash upgrade and an
invalidation of session A
>
I think the hash upgrade already happened in your point 2 above
--
Ticket URL: <https://code.djangoproject.com/ticket/28285#comment:2>
Django
Reporter: Simon Blanchard | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.11
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 Tim Graham):
I'm not following your scenario. Could you please give your steps to
reproduce?
--
Ticket URL: <https://code.djangoproject.com/ticket/28285#comment:3>
Django
Reporter: Simon Blanchard | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.11
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 Simon Blanchard):
Replying to [comment:3 Tim Graham]:
> I'm not following your scenario. Could you please give your steps to
reproduce?
{{{
def get_user(request):
"""
Returns the user model instance associated with the given request
session.
If no user is retrieved an instance of `AnonymousUser` is returned.
"""
from .models import AnonymousUser
user = None
try:
user_id = _get_user_session_key(request)
backend_path = request.session[BACKEND_SESSION_KEY]
except KeyError:
pass
else:
if backend_path in settings.AUTHENTICATION_BACKENDS:
backend = load_backend(backend_path)
user = backend.get_user(user_id)
# Verify the session
if hasattr(user, 'get_session_auth_hash'):
session_hash = request.session.get(HASH_SESSION_KEY)
session_hash_verified = session_hash and
constant_time_compare(
session_hash,
user.get_session_auth_hash()
)
if not session_hash_verified:
request.session.flush()
user = None
return user or AnonymousUser()
}}}
get_user() verifies the user's session hash is the same as the one in the
session. If the users session hash (calculated from the password hash) is
different from that stored in the session, the session is flushed.
So the scenario is:
1. Upgrade Django
2. User logs in
3. Session hash is set from original password hash before the upgrade
4. Django updates password hash but does not update the session hash
5. On next request, get_user() notices the session hash and password hash
are different
6. Session is flushed
7. User is logged out
At least that's what I think is happening. But looking at the code I'm not
so sure. authenticate() should be callled before login(), authenticate()
updates the hash, then login() should be setting the session key with the
new hash, oh I may be passing an old user object - with former password
hash to login. Sorry my mistake! Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/28285#comment:4>
Django
Reporter: Simon Blanchard | Owner: nobody
Component: contrib.auth | Version: 1.11
Severity: Normal | Resolution: worksforme
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => worksforme
Comment:
I can't reproduce a problem with the steps provided, so I'll assume the
problem is in your application unless you provide more details.
--
Ticket URL: <https://code.djangoproject.com/ticket/28285#comment:5>