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:

Description

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)

by simon, 14 years ago

Attachment: ticket12417.diff added

by simon, 14 years ago

Attachment: ticket12417-v2.diff added

Updated based on feedback on the mailing list

by simon, 14 years ago

Attachment: ticket12417-v3.diff added

comment:2 by simon, 14 years ago

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 by anonymous, 14 years ago

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

comment:4 by anonymous, 14 years ago

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

comment:5 by Russell Keith-Magee, 14 years ago

milestone: 1.21.3

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

comment:6 by Russell Keith-Magee, 14 years ago

Owner: changed from simom to simon

Also, no idea who simom is :-)

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

Triage Stage: UnreviewedAccepted

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

Did this get reviewed by a cryptographer?

comment:9 by Luke Plant, 13 years ago

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

@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 by tsuraan, 13 years ago

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

comment:12 by Jannis Leidel, 13 years ago

milestone: 1.3

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

comment:13 by Matt McClanahan, 13 years ago

Severity: Normal
Type: New feature

comment:14 by Jannis Leidel, 13 years ago

Easy pickings: unset
Patch needs improvement: set

by Jannis Leidel, 13 years ago

Attachment: ticket12417-v4.diff added

Updated ticket with changes discussed in mailing list and ticket comments

by steph, 13 years ago

Attachment: ticket12417-v5.diff added

Updated patch to use salted_hmac from django.utils.crypto

by Jannis Leidel, 13 years ago

Attachment: ticket12417-v6.diff added

Updated method argument signature as discussed on the mailing list

by Jannis Leidel, 13 years ago

Attachment: ticket12417-v7.diff added

Fixed baseconv.

comment:15 by Jannis Leidel, 13 years ago

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