On 12.05.2011, at 01:05, Luke Plant wrote: > 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).
Agreed, see the patch Stephan just updated. Jannis -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
