Opened 14 years ago

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

Download all attachments as: .zip

Change History (22)

Changed 14 years ago by simon

Attachment: ticket12417.diff added

Changed 14 years ago by simon

Attachment: ticket12417-v2.diff added

Updated based on feedback on the mailing list

Changed 14 years ago by simon

Attachment: ticket12417-v3.diff added

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

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

comment:4 Changed 14 years ago by anonymous

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

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

Owner: changed from simom to simon

Also, no idea who simom is :-)

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

Triage Stage: UnreviewedAccepted

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

Did this get reviewed by a cryptographer?

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

milestone: 1.3

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

comment:13 Changed 13 years ago by Matt McClanahan

Severity: Normal
Type: New feature

comment:14 Changed 13 years ago by Jannis Leidel

Easy pickings: unset
Patch needs improvement: set

Changed 13 years ago by Jannis Leidel

Attachment: ticket12417-v4.diff added

Updated ticket with changes discussed in mailing list and ticket comments

Changed 13 years ago by steph

Attachment: ticket12417-v5.diff added

Updated patch to use salted_hmac from django.utils.crypto

Changed 13 years ago by Jannis Leidel

Attachment: ticket12417-v6.diff added

Updated method argument signature as discussed on the mailing list

Changed 13 years ago by Jannis Leidel

Attachment: ticket12417-v7.diff added

Fixed baseconv.

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