#31358: Increase default password salt size in BasePasswordHasher.
--------------------------------------+------------------------------------
Reporter: Jon Moroney | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Jon Moroney):
Replying to [comment:19 Florian Apolloner]:
> Ha, sorry for not being clearer. What I wanted to say is that I don't
have a good answer for you. In a perfect world (ie if you are up to the
challenge :D) I would suggest adding a `decode` function to the hashers
that basically does the reverse of `encode`. `safe_summary` could then use
the decoded values and mask them as needed.
>
> Adding a `decode` function seems to make sense since
`Argon2PasswordHasher` already has a `_decode` and others manually repeat
(the simpler logic) ala `algorithm, empty, algostr, rounds, data =
encoded.split('$', 4)` over multiple functions.
I'm not opposed to implementing a decode function, but I'm not sure I
understand how it would differ from the `safe_summary` function. Further
if decode functionality is require/desired then is the scope of concern
just for the hashers shipped with django or do we need to consider third
party hashers? I have a preference for not considering them and creating a
clear breaking change (but I'm also lazy :p).
On a potential decode function; This comment on the encode function
worries me a bit
> The result is normally formatted as "algorithm$salt$hash" and must be
fewer than 128 characters.
It makes me think that the encoded result could be truncated and if we
consider third party hashers then we must consider truncated DB entries.
I'm not sure if this is a real issue or not, but the 128 character limit
does raise an eye brow to me.
> This new `decode` functionality could be in a new PR and your current PR
would be blocked by it and the use that. Interested to hear your and
Mariusz' thoughts
Sounds reasonable, but what does the decode function look like? It it a
stub in the `BasePasswordHasher` which requires that derived classes
implement it with an implementation for each of the included hashers? Let
me know if that sounds good and I can make a second PR to implement that
tomorrow. Else lets keep this conversation going :)
--
Ticket URL: <https://code.djangoproject.com/ticket/31358#comment:20>
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 [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.1b43cfb47a929ee76ad116863c9fece1%40djangoproject.com.
