Opened 13 years ago

Closed 13 years ago

Last modified 12 years ago

#14445 closed (fixed)

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

Reported by: Luke Plant 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: no UI/UX: no

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 Luke Plant 13 years ago.
14445.2.diff (45.2 KB ) - added by Luke Plant 13 years ago.
Updated to ensure we do SHA1 on the key
14445.2.2.diff (46.5 KB ) - added by Luke Plant 13 years ago.
Updated to ensure we do SHA1 on the key

Download all attachments as: .zip

Change History (7)

by Luke Plant, 13 years ago

Attachment: 14445.diff added

comment:1 by Luke Plant, 13 years ago

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 by Gabriel Hurley, 13 years ago

Has patch: set
milestone: 1.3
Triage Stage: UnreviewedAccepted

by Luke Plant, 13 years ago

Attachment: 14445.2.diff added

Updated to ensure we do SHA1 on the key

by Luke Plant, 13 years ago

Attachment: 14445.2.2.diff added

Updated to ensure we do SHA1 on the key

comment:3 by Luke Plant, 13 years ago

Resolution: fixed
Status: newclosed

(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 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

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