Opened 12 years ago

Closed 11 years ago

#12417 closed New feature (fixed)

Add signing and signed cookies to Django

Reported by: simon Owned by: simon
Component: Core (Other) Version: dev
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 12 years ago.
ticket12417-v2.diff (28.0 KB) - added by simon 12 years ago.
Updated based on feedback on the mailing list
ticket12417-v3.diff (28.0 KB) - added by simon 12 years ago.
ticket12417-v4.diff (29.3 KB) - added by Jannis Leidel 11 years ago.
Updated ticket with changes discussed in mailing list and ticket comments
ticket12417-v5.diff (29.0 KB) - added by steph 11 years ago.
Updated patch to use salted_hmac from django.utils.crypto
ticket12417-v6.diff (28.9 KB) - added by Jannis Leidel 11 years ago.
Updated method argument signature as discussed on the mailing list
ticket12417-v7.diff (35.2 KB) - added by Jannis Leidel 11 years ago.
Fixed baseconv.

Download all attachments as: .zip

Change History (22)

Changed 12 years ago by simon

Attachment: ticket12417.diff added

Changed 12 years ago by simon

Attachment: ticket12417-v2.diff added

Updated based on feedback on the mailing list

Changed 12 years ago by simon

Attachment: ticket12417-v3.diff added

comment:2 Changed 12 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 12 years ago by anonymous

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

comment:4 Changed 12 years ago by anonymous

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

comment:5 Changed 12 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 12 years ago by Russell Keith-Magee

Owner: changed from simom to simon

Also, no idea who simom is :-)

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

Triage Stage: UnreviewedAccepted

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

Did this get reviewed by a cryptographer?

comment:9 Changed 11 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 11 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 11 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 11 years ago by Jannis Leidel

milestone: 1.3

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

comment:13 Changed 11 years ago by Matt McClanahan

Severity: Normal
Type: New feature

comment:14 Changed 11 years ago by Jannis Leidel

Easy pickings: unset
Patch needs improvement: set

Changed 11 years ago by Jannis Leidel

Attachment: ticket12417-v4.diff added

Updated ticket with changes discussed in mailing list and ticket comments

Changed 11 years ago by steph

Attachment: ticket12417-v5.diff added

Updated patch to use salted_hmac from django.utils.crypto

Changed 11 years ago by Jannis Leidel

Attachment: ticket12417-v6.diff added

Updated method argument signature as discussed on the mailing list

Changed 11 years ago by Jannis Leidel

Attachment: ticket12417-v7.diff added

Fixed baseconv.

comment:15 Changed 11 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