On 12.05.2011, at 14:17, Paul McMillan wrote: > 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.
Yeah, SHA256 and SHA512 have the downsides of being rather long for the type of application the signatures are going to be used for, e.g. one-time URLs. Simon discussed this a year ago, with the hint that using SHA1 together with HMAC isn't prone to the collision issue: "I chose sha-1 over sha-256 for reasons of signature length. A base64 encoded signature generated with hmac/sha1 is 27 characters long. The same thing using hmac/sha256 is 43 characters long. If you're planning on using signatures in cookies and URLs that's quite a big difference (43 characters is more than half of the maximum 80 characters needed to safely transmit URLs in plain text e-mails, e.g. for account recovery links)." http://groups.google.com/group/django-developers/msg/2ceedd2dcb4feed7 In other words, if we go with SHA256 or even SHA512 we'll limit the usefulness of this feature -- with only theoretical gain in security per the discussion a year ago. > 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. If I read the draft spec correctly it's basically the same algorithm as used in the current patch but SHA256 instead of SHA1. [1] > 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. Fair enough. Jannis 1: http://tools.ietf.org/html/draft-jones-json-web-signature-02#section-7 > > 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. > -- 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.
