I'm obviously going to weigh in here, having authored the signed cookies app you mentioned below, but be aware I'm not a cryptographer, nor do I have any personal use for signed cookies at all. I can appreciate their value, but I'm not even using my own app in anything, so if there are problems with it, I haven't experienced them first-hand. I won't comment on everything, because I certainly trust your thoughts on the matter. (Your comment on the Django book is what prompted me to write the app in the first place!) I do have a couple thoughts, though. So, anything I don't comment on, I agree with.
On Thu, Sep 24, 2009 at 1:18 PM, Simon Willison <[email protected]> wrote: > It's also something that's hard to do correctly. At the sprints Armin > pointed out that I should be using hmac, not straight sha1, for > generating signatures (something Django itself gets wrong in the few > places that implement signing already). Having a cryptographer- > approved implementation will save a lot of people from making the same > mistakes. I have no idea how hmac differs from straight sha1, but this was raised in a django-signedcookies issue as well, and i've since integrated it. I'm with you on this; if somebody who knows better recommends something, I'm inclined to listen. > I think signed cookies should either be a separate API from > response.set_cookie or should be provided as an additional argument to > that method. I'm not a fan of signing using middleware (as seen in > http://code.google.com/p/django-signedcookies/ ) since that approach > signs everything - some cookies, such as those used by Google > Analytics, need to remain unsigned. I admit, I hadn't considered third-party cookies when I put it together as a decorator. Client-side cookie access is likely problematic as well, but that'll always be questionable anyway. You can't validate a cookie in the client without divulging your secret key and you can't just ignore the signature, because that defeats the whole purpose. My app also provides a decorator, which might help in some rare situations, but most of the time things like analytics cookies will be in a base template and will always be included in whatever view happens to have the decorator. I'm very surprised that in all this time, nobody submitted a bug about the analytics problem. > So the API could either be: > > response.set_signed_cookie(key, value) > > Or... > > response.set_cookie(key, value, signed=True) > > (I prefer the latter option) I prefer the latter as well, for an added reason. I'd personally like to invest some time in seeing if there are any other interesting pitfalls in set_cookie() based on it deferring to Python's SimpleCookie implementation. When writing Pro Django, I realized that SimpleCookie expects everything to be strings, so #6657 came up with secure=False resulting in a secure cookie after all. I don't know if there are other such issues, but it might be worth looking at in detail if we already have to add in signed cookie support. And before anyone asks, no I don't think tying the signing behavior into secure=True would be a good idea. Secure is meant to tell the browser it should only send the cookie back over a secure channel, such as HTTPS. While people who need secure cookies may also want signed cookies, they're two separate ideas that I don't think would do well mixed together. Of course, that leaves us with a potential response.set_cookie(key, value, secure=True, signed=True), but I think it's worth it to be explicit. But I do have one other concern with either of these APIs that is at least worth discussing: the disparity between setting a signed cookie and retrieving one. response.set_cookie(key, value, signed=True) value = signed.unsign(request.COOKIES[key]) Mainly, unsigning a cookie requires importing and directly using the signing module anyway, so the benefit of having set_cookie() handle it transparently is fairly weak (unless, of course, I'm missing something obvious). I'd rather just see the signing code made available and well-documented, so that at least the change in code when adding signed cookies is equivalent on both sides. Another option would be to have request.COOKIES be a custom dictionary, with an extra .get_unsigned(key) method that would work like .get(), but validates the signature along the way. That behavior can't be added straight to __getitem__() though, because that has no way of knowing whether the cookie was signed to begin with. These are the types of issues that led me to just implement it as a middleware, so the API could be as dead simple as possible. With these new issues in mind, I don't think dead simplicity is really an option, so I'd rather fall back to being explicit. >>>> signed.unsign('hello.9asVJn9dfv6qLJ_BYObzF7mmH8c') > 'hello' >>>> signed.unsign('hello.badsignature') > Traceback (most recent call last): > ... > BadSignature: Signature failed: badsignature > > BadSignature is a subclass of ValueError, meaning lazy developers > (like myself) can do the following rather than importing the exception > itself: > > try: > value = signed.unsign(signed_value) > except ValueError: > return tamper_error_view(request) I'm not a big fan of this, personally. Yes, it does make things slightly easier, by not requiring a separate import, but we already have django.core.exceptions.SuspiciousOperation exists for a reason, and a missing or invalid signature would certainly qualify, in my eyes. Yeah, I suppose we could perhaps make that a subclass of ValueError or TypeError, but I would worry about people wrapping it up into something else that might cause problems. try: value = signed.unsign(signed_value).decode('utf-8') except ValueError: # Whoops! UnicodeDecodeError winds up here as well! I don't know how likely this is to happen, since the only examples I could come up with offhand are: * Unicode errors, which could possibly be handled by automatic Unicode translating in the signing code * Using int() for things like user IDs, in which case a non-integer would be evidence of tampering anyway. Of course, you could just as easily argue that people shouldn't be doing that, or if they do, they should import BadSignature (or whatever) and give it its own except block ahead of ValueError so the two can be distinguished. But of course, that expects a non-lazy programmer, and if we're trying to make it easier for lazy programmers, I don't think the behavior of non-lazy ones matters much. > Potential uses > ============== > > Lots of stuff: > > - Signed cookies (obviously) > - Generating CSRF tokens > - Secure /logout/ and /change-language/ links > - Securing /login/?next=/some/path/ > - Securing hidden fields in form wizards > - Recover-your-account links in e-mails I think this is a big win for including signing as a separate piece from signed cookies, because you have a much bigger list of use cases, which can make it easier for people to understand the value. One of the most common questions I got on my app was, "why would I want to sign my cookies?" I think expanding the available uses signing will help answer that question. > So... what do people think? Is this a feature suitable for Django > (obviously I think so)? Is this as simple as getting a cryptographer's > input and dropping signed.py in to django.utils or are there other > design factors we should consider? All in all, I understand and appreciate the benefits of signing things, and I'd like that behavior to be made available reliably and easily, and it looks like that would require them being in core. I'm a little worried about ease of use, though, because it seems like most of the ways to make things easy can also make things either more confusing or less flexible. I'm a little scattered on the moment, but I think it's definitely worth refining into something that can go into core. -Gul --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
