I'd also like to review this in depth before it gets committed, but won't have a chance to do that for a day or two.
I agree with Luke re: salted hmac. A couple of initial thoughts: Since we aren't supporting Python 2.4 anymore, we have access to hashlib. We should use SHA256 or 512. Since these functions are for signing JSON objects, let's use the existing format which is on the standards track - JSON Web Signature (JWS)[1] for signing (and in the future, JWE for encryption). They (along with JWT, the token format) are part of OAuth2.0, but are getting standardized separately so they get done sooner. It's mostly a formatting change to produce output that is compatible. I'll be happy to write the change it if nobody complains about the idea. I like the idea of using a standard for this. I REALLY like the idea that other people have been reviewing this standard with cryptographic security in mind. -Paul [1] http://tools.ietf.org/html/draft-jones-json-web-signature On Wed, May 11, 2011 at 4:05 PM, Luke Plant <l.plant...@cantab.net> 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). > > 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. > > -- 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.
