Opened 7 years ago

Closed 6 years ago

#12417 closed New feature (fixed)

Add signing and signed cookies to Django

Reported by: simon Owned by: simon
Component: Core (Other) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX:

Attachments (7)

ticket12417.diff (28.1 KB) - added by simon 7 years ago.
ticket12417-v2.diff (28.0 KB) - added by simon 7 years ago.
Updated based on feedback on the mailing list
ticket12417-v3.diff (28.0 KB) - added by simon 7 years ago.
ticket12417-v4.diff (29.3 KB) - added by Jannis Leidel 6 years ago.
Updated ticket with changes discussed in mailing list and ticket comments
ticket12417-v5.diff (29.0 KB) - added by steph 6 years ago.
Updated patch to use salted_hmac from django.utils.crypto
ticket12417-v6.diff (28.9 KB) - added by Jannis Leidel 6 years ago.
Updated method argument signature as discussed on the mailing list
ticket12417-v7.diff (35.2 KB) - added by Jannis Leidel 6 years ago.
Fixed baseconv.

Download all attachments as: .zip

Change History (22)

Changed 7 years ago by simon

Attachment: ticket12417.diff added

Changed 7 years ago by simon

Attachment: ticket12417-v2.diff added

Updated based on feedback on the mailing list

Changed 7 years ago by simon

Attachment: ticket12417-v3.diff added

comment:2 Changed 7 years ago by simon

Has patch: set
milestone: 1.2
Version: 1.1SVN

Patch is basically ready, but we're going to hold off committing it until we've had it reviewed by a real cryptographer. The plan is to get it in before the 1.2 beta feature freeze.

comment:3 Changed 7 years ago by anonymous

milestone: 1.21.3
Owner: changed from nobody to anonymous
Status: newassigned
Triage Stage: UnreviewedAccepted

comment:4 Changed 7 years ago by anonymous

milestone: 1.31.2
Owner: changed from anonymous to simom
Status: assignednew
Triage Stage: AcceptedUnreviewed

comment:5 Changed 7 years ago by Russell Keith-Magee

milestone: 1.21.3

We're past the feature freeze, this will have to wait until 1.3.

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

Owner: changed from simom to simon

Also, no idea who simom is :-)

comment:7 Changed 7 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

comment:8 Changed 6 years ago by mr.kamu@…

Did this get reviewed by a cryptographer?

comment:9 Changed 6 years ago by Luke Plant

A few comments on the patch, while I remember:

I think that get_cookie_signer should return a Signer that is unique for cookies e.g. uses "django.http.cookies" + settings.SECRET_KEY as the key to pass to Signer. This reduces the likelihood of someone being able to forge cookies by using another Signer in the project that happens to be exposed and allows attackers to generate signatures for arbitrary content.

Method Signer.unsign should replace if sig != expected with code that is not vulnerable to timing attacks.

Also there are some PEP8 fixes needed - double spaces between top level definitions in a module, no spaces around '=' for keyword arguments.

comment:10 Changed 6 years ago by EmilStenstrom

@mr.kamu: The mailing list thread above concluded that a many-eyes approach was better than getting a single cryptographer to review the code. So the review was done in this mail thread: http://groups.google.com/group/django-developers/browse_thread/thread/297e8b22006f7f3a - I'm not sure how to deem a crowd sourced mail thread "complete" so I leave that up to you.

comment:11 Changed 6 years ago by tsuraan

There are no signs of this work in django's svn repository. Are signed cookies planned for 1.3?

comment:12 Changed 6 years ago by Jannis Leidel

milestone: 1.3

The deadline for new features has passed, removing 1.3 milestone.

comment:13 Changed 6 years ago by Matt McClanahan

Severity: Normal
Type: New feature

comment:14 Changed 6 years ago by Jannis Leidel

Easy pickings: unset
Patch needs improvement: set

Changed 6 years ago by Jannis Leidel

Attachment: ticket12417-v4.diff added

Updated ticket with changes discussed in mailing list and ticket comments

Changed 6 years ago by steph

Attachment: ticket12417-v5.diff added

Updated patch to use salted_hmac from django.utils.crypto

Changed 6 years ago by Jannis Leidel

Attachment: ticket12417-v6.diff added

Updated method argument signature as discussed on the mailing list

Changed 6 years ago by Jannis Leidel

Attachment: ticket12417-v7.diff added

Fixed baseconv.

comment:15 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

In [16253]:

Fixed #12417 -- Added signing functionality, including signing cookies. Many thanks to Simon, Stephan, Paul and everyone else involved.

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