Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#15627 closed (fixed)

check_password should use constant_time_compare instead of == to check passwords

Reported by: Harro 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 Harro 6 years ago.
Patch to use constant_time_compare

Download all attachments as: .zip

Change History (6)

Changed 6 years ago by Harro

Attachment: 15627.diff added

Patch to use constant_time_compare

comment:1 Changed 6 years ago by Russell Keith-Magee

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedReady for checkin

comment:2 Changed 6 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

In [15870]:

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

comment:3 Changed 6 years ago by Luke Plant

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 6 years ago by Russell Keith-Magee

@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 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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