Code

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15627 closed (fixed)

check_password should use constant_time_compare instead of == to check passwords

Reported by: hvdklauw Owned by: nobody
Component: contrib.auth Version: 1.3-rc
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

I just noticed django doesn't use the constant_time_compare function in the check_password function in contrib.auth.models.

I'll add a patch that changes it, would be nice to have this little bit extra security in the 1.3 release.

Attachments (1)

15627.diff (808 bytes) - added by hvdklauw 3 years ago.
Patch to use constant_time_compare

Download all attachments as: .zip

Change History (6)

Changed 3 years ago by hvdklauw

Patch to use constant_time_compare

comment:1 Changed 3 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Ready for checkin

comment:2 Changed 3 years ago by russellm

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

In [15870]:

Fixed #15627 -- Use constant time comparison for password checks. Thanks to hvdklauw for the report and patch.

comment:3 Changed 3 years ago by lukeplant

For the record, this code is not actually vulnerable to a timing-based attack, as neither of the strings compared by the '==' is user supplied, or can be controlled easily by the user.

To actually attack this code using a timing-attacks, you would have to:

  1. Know the salt that was stored
  2. Generate a password that, when passed through get_hexdigest(algo, salt, raw_password) would produce a given prefix, e.g. 'a', 'aa', 'ab', 'aab' etc. This is Hard - we store password hashes precisely because going from the hash (or part of the hash) to the string that generated it is difficult.
  3. In this way, control the RHS of the comparison, and by measuring timings eventually extract the hash (and therefore the password which you generated in step 2).

But anyway.

comment:4 Changed 3 years ago by russellm

@luke - Yeah - I knew it would be extremely hard to turn this into a functional attack, but it cost nothing to make the change, on the off chance that anyone ever found a way to construct a hash-based timing attack, we're pre-emptively protected.

comment:5 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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.