Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#14445 closed (fixed)

Use HMAC and constant-time comparison functions where needed in Django

Reported by: lukeplant Owned by: nobody
Component: Uncategorized Version: 1.2
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Various places in Django use MD5/SHA1 based methods as a kind of MAC (Message Authentication Code). HMAC is a much better alternative.

In most cases, we need to maintain backwards compatibility with existing signed values, by falling back to the existing MD5/SHA1 method. This means we will continue to have potential weaknesses, but by only generating HMACs from now on, we can phase out support for these old values over time, using the normal deprecation method. This should give plenty of time for the old hashes to expire naturally anyway.

In addition, in all these places, and some others, the MACs are tested for validity using simple string equality, which potentially opens us up to timing-based attacks to discover the tokens.

Attachments (3)

14445.diff (44.0 KB) - added by lukeplant 4 years ago.
14445.2.diff (45.2 KB) - added by lukeplant 4 years ago.
Updated to ensure we do SHA1 on the key
14445.2.2.diff (46.5 KB) - added by lukeplant 4 years ago.
Updated to ensure we do SHA1 on the key

Download all attachments as: .zip

Change History (7)

Changed 4 years ago by lukeplant

comment:1 Changed 4 years ago by lukeplant

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

Notes on patch:

  • In most cases actually raising PendingDeprecationWarning wouldn't be helpful, so I haven't done that.
  • In every case there are tests for the hashes produced by Django 1.2 still being accepted.
  • To make better tests for FormWizard I had to move the tests.py to a tests/__init__.py in order to create a tests/templates directory. The new tests are:
    • FormHmacTests
    • PreviewTests: test_form_submit_django12_hash, test_form_submit_django12_hash_custom_hash
    • WizardTests: test_bad_hash, test_good_hash_django12, test_good_hash_django12_subclass, test_good_hash_django13

There are other uses of MD5/SHA1 that I haven't changed, because they are not an application of MAC (for example, the generation of unique keys in django/template/loaders/cached.py)

comment:2 Changed 4 years ago by gabrielhurley

  • Has patch set
  • milestone set to 1.3
  • Triage Stage changed from Unreviewed to Accepted

Changed 4 years ago by lukeplant

Updated to ensure we do SHA1 on the key

Changed 4 years ago by lukeplant

Updated to ensure we do SHA1 on the key

comment:3 Changed 4 years ago by lukeplant

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

(In [14218]) Fixed #14445 - Use HMAC and constant-time comparison functions where needed.

All adhoc MAC applications have been updated to use HMAC, using SHA1 to
generate unique keys for each application based on the SECRET_KEY, which is
common practice for this situation. In all cases, backwards compatibility
with existing hashes has been maintained, aiming to phase this out as per
the normal deprecation process. In this way, under most normal
circumstances the old hashes will have expired (e.g. by session expiration
etc.) before they become invalid.

In the case of the messages framework and the cookie backend, which was
already using HMAC, there is the possibility of a backwards incompatibility
if the SECRET_KEY is shorter than the default 50 bytes, but the low
likelihood and low impact meant compatibility code was not worth it.

All known instances where tokens/hashes were compared using simple string
equality, which could potentially open timing based attacks, have also been
fixed using a constant-time comparison function.

There are no known practical attacks against the existing implementations,
so these security improvements will not be backported.

comment:4 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.