#29289: Clarify comment regarding what data is hashed to generate
PasswordResetTokenGenerator tokens
------------------------------------------------+------------------------
Reporter: Tim Graham | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
Robert Picard reported this to the security mailing list:
The code for the password generator has
[https://github.com/django/django/blob/be6ca89396c031619947921c81b8795d816e3285/django/contrib/auth/tokens.py#L60
a comment] (there since it was written 10 years ago in
fcd837cd0f9b2c706bc49af509628778d442bb3f) that mentions that the salt is
included in the hashed values so that the token can only be used once.
[https://github.com/django/django/blob/be6ca89396c031619947921c81b8795d816e3285/django/contrib/auth/tokens.py#L76
The code] actually uses the password hash (`user.password`) instead of the
salt.
While this is a hash, it seems like making a value from the password hash
was not the original intention and is an unnecessary security risk (though
it may be a small risk).
The security team couldn't identify a problem with the implementation. The
suggestion to change the implementation to use only the salt was raised,
but Luke Plant said:
Using just the password salt for the reset token hash, rather than the
whole field, could reduce the entropy available to the reset token.
possibly quite significantly if the salt is short (some old/custom
password hash algorithms?), so I think that would be a bad idea. \\\\
From what I'm aware, there is no known danger from using the full
password field (i.e. including hash) to create the token, providing that
we are hashing it (and in our case we are adding various other bits that
the attacker does not know to it as well). Of course, *theoretically*
there is a danger - we are inevitably leaking *some* information about the
password when we do this, in the information theoretic sense - if you have
a hash of a password, you have more information about the password than if
you had nothing, and similarly for a hash of a hash of a password. The
issue for an attack is turning this theory into practice, which is very
hard, which is why we use password hashing etc.. And in this case, we are
sending that 'leaked' information only to the user themselves. \\\\
AFAICS, the hash function would have to be *very* broken for this to be a
problem in practice. The benefit of using it (the extra entropy it
provides to the reset token) seems to far outweigh any danger, IMO.
--
Ticket URL: <https://code.djangoproject.com/ticket/29289>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/052.1a8ff4aaad6703de95dc7ef122041948%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
