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)
Change History (22)
Changed 14 years ago by
Attachment: | ticket12417.diff added |
---|
comment:1 Changed 14 years ago by
Changed 14 years ago by
Attachment: | ticket12417-v2.diff added |
---|
Updated based on feedback on the mailing list
Changed 14 years ago by
Attachment: | ticket12417-v3.diff added |
---|
comment:2 Changed 14 years ago by
Has patch: | set |
---|---|
milestone: | → 1.2 |
Version: | 1.1 → SVN |
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
milestone: | 1.2 → 1.3 |
---|---|
Owner: | changed from nobody to anonymous |
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
comment:4 Changed 14 years ago by
milestone: | 1.3 → 1.2 |
---|---|
Owner: | changed from anonymous to simom |
Status: | assigned → new |
Triage Stage: | Accepted → Unreviewed |
comment:5 Changed 14 years ago by
milestone: | 1.2 → 1.3 |
---|
We're past the feature freeze, this will have to wait until 1.3.
comment:7 Changed 14 years ago by
Triage Stage: | Unreviewed → Accepted |
---|
comment:9 Changed 13 years ago by
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
@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
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
milestone: | 1.3 |
---|
The deadline for new features has passed, removing 1.3 milestone.
comment:13 Changed 13 years ago by
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:14 Changed 13 years ago by
Easy pickings: | unset |
---|---|
Patch needs improvement: | set |
Changed 13 years ago by
Attachment: | ticket12417-v4.diff added |
---|
Updated ticket with changes discussed in mailing list and ticket comments
Changed 13 years ago by
Attachment: | ticket12417-v5.diff added |
---|
Updated patch to use salted_hmac from django.utils.crypto
Changed 13 years ago by
Attachment: | ticket12417-v6.diff added |
---|
Updated method argument signature as discussed on the mailing list
Under discussion here: http://groups.google.com/group/django-developers/browse_thread/thread/d9d635afb6d1820f