Opened 15 years ago
Closed 14 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)
by , 15 years ago
| Attachment: | ticket12417.diff added |
|---|
comment:1 by , 15 years ago
by , 15 years ago
| Attachment: | ticket12417-v2.diff added |
|---|
Updated based on feedback on the mailing list
by , 15 years ago
| Attachment: | ticket12417-v3.diff added |
|---|
comment:2 by , 15 years ago
| 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 by , 15 years ago
| milestone: | 1.2 → 1.3 |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
| Triage Stage: | Unreviewed → Accepted |
comment:4 by , 15 years ago
| milestone: | 1.3 → 1.2 |
|---|---|
| Owner: | changed from to |
| Status: | assigned → new |
| Triage Stage: | Accepted → Unreviewed |
comment:5 by , 15 years ago
| milestone: | 1.2 → 1.3 |
|---|
We're past the feature freeze, this will have to wait until 1.3.
comment:7 by , 15 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:9 by , 14 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 , 14 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 , 14 years ago
There are no signs of this work in django's svn repository. Are signed cookies planned for 1.3?
comment:12 by , 14 years ago
| milestone: | 1.3 |
|---|
The deadline for new features has passed, removing 1.3 milestone.
comment:13 by , 14 years ago
| Severity: | → Normal |
|---|---|
| Type: | → New feature |
comment:14 by , 14 years ago
| Easy pickings: | unset |
|---|---|
| Patch needs improvement: | set |
by , 14 years ago
| Attachment: | ticket12417-v4.diff added |
|---|
Updated ticket with changes discussed in mailing list and ticket comments
by , 14 years ago
| Attachment: | ticket12417-v5.diff added |
|---|
Updated patch to use salted_hmac from django.utils.crypto
by , 14 years ago
| 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