Code

Opened 4 years ago

Closed 3 years ago

#13969 closed Cleanup/optimization (fixed)

auth module should use longer salt for hashing

Reported by: cyounkins 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 cyounkins 4 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 4 years ago by cyounkins

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:3 Changed 4 years ago by cyounkins

  • 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 4 years ago by cyounkins

comment:4 Changed 4 years ago by gabrielhurley

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

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 3 years ago by gabrielhurley

  • Component changed from Contrib apps to contrib.auth

comment:6 Changed 3 years ago by graham_king

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:7 Changed 3 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

In [16453]:

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.