Opened 6 years ago

Closed 5 years ago

#13969 closed Cleanup/optimization (fixed)

auth module should use longer salt for hashing

Reported by: Craig Younkins Owned by: nobody
Component: contrib.auth Version: 1.2
Severity: Normal Keywords: security
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

As noted here - http://www.pythonsecurity.org/wiki/django/#authentication - the current auth module uses 5 hexadecimal characters as a salt. This is equivalent to 20 bits (log base 2 of 165). See http://code.djangoproject.com/browser/django/tags/releases/1.2.1/django/contrib/auth/models.py#L240

PKCS5 v2.1 draft (http://www.rsa.com/rsalabs/node.asp?id=2127) recommends that a salt of at least 64 bits be used. This will strengthen the password scheme by increasing the time needed for dictionary attacks.

Attachments (1)

better_salting.diff (1.5 KB) - added by Craig Younkins 6 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 6 years ago by Craig Younkins

comment:3 Changed 6 years ago by Craig Younkins

Has patch: set

Added patch that adds better salt generation with 12 random characters from a-z, A-Z, 0-9, equivalent to 71-bits of entropy.

It tries to use random.SystemRandom as the entropy pool, falling back to random if SystemRandom raises NotImplementedError, which will occur on operating systems without /dev/urandom or CryptGenRandom on Windows.

I ran the 1700 Django unit tests but did not test it in an application. Adding a regression test is not possible.

Craig Younkins

Changed 6 years ago by Craig Younkins

Attachment: better_salting.diff added

comment:4 Changed 6 years ago by Gabriel Hurley

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

This is a valid concern, and the code works for me, however I'm not a security expert so it could use another set of eyes.

It's a minor concern, but the example salt in the documentation probably ought to be updated as well, lest people think we're still only using a five-character salt: http://docs.djangoproject.com/en/dev/topics/auth/#passwords

comment:5 Changed 6 years ago by Gabriel Hurley

Component: Contrib appscontrib.auth

comment:6 Changed 6 years ago by Graham King

Severity: Normal
Type: Cleanup/optimization

comment:7 Changed 5 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

In [16453]:

Fixed #13969 -- Extended length of salt used when setting the password. Thanks to cyounkins for the initial patch.

Note: See TracTickets for help on using tickets.
Back to Top