On 11/05/11 20:20, Jannis Leidel wrote: > Hi all, > > This is in continuation to Simon's previous efforts about adding tools > for easy signing, including secure cookies ([1], [2]). > > Stephan (who is working on #9200 [3] -- improving the wizard in > contrib.formtools) and me just updated the patch attached to ticket > #12417 [4] with the recommended changes according to the mailing list > threads and the Trac ticket: > > http://code.djangoproject.com/attachment/ticket/12417/ticket12417-v4.diff > > The complete changes (as noted) are: > > - Moved from django.utils.signed to django.core.signing > - Removed the seperator argument to the Signer.sign and Signer.unsign > methods and moved it to a class attribute > - Added key prefix ('django.http.cookies') to Signer instantiation > - Changed key ordering from `"signer" + key + salt` to `salt + "signer" + key` > to lower the chance for brute force attacks > - PEP8 and other style changes > - Use constant_time_compare from django.utils.crypto for timing attack > proof signature verification > - Updated and fixed docs > - Changed setting name from COOKIE_SIGNER_BACKEND to SIGNING_BACKEND > > We'd appreciate any further comments before I commit this patch, given > the formtools wizard being dependent on it for its cookie storage backend.
We already have one way of using hmac with unique keys for each application - django.utils.crypto.salted_hmac. I looked at the code, and can't see any reason we couldn't make Signer use that code path, rather than having two very similar code paths. I see no reason for the new code to directly use either settings.SECRET_KEY or hmac even once, and doing so makes our code much harder to audit for security problems - every place where we use SECRET_KEY or hmac directly is a place we have to worry about. Currently we have 4 uses of SECRET_KEY (one of them is in a deprecated code path), and 1 use of hmac.new. So I feel quite strongly that we should fix this code to use salted_hmac. (Or fix salted_hmac if there is some problem with it, but remembering that there is lots of data that depends on it). I haven't had time to further review, but thought I'd get that out straight away. Regards, Luke -- Parenthetical remarks (however relevant) are unnecessary Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
