On Mon, Jan 4, 2010 at 8:47 PM, Simon Willison <si...@simonwillison.net> wrote:
> I'm pursing expert feedback on the crypto used in Django signing /
> signed cookies. Here's the e-mail I'm sending out to various mailing
> lists. I'll also link to this post from a number of places and see if
> I can get some smart eyes on it that way.
>
> ====================================
>
> Hello,
>
> We've been working to add string signing and signed cookies to the
> core of the Django web framework <http://www.djangoproject.com/>. We
> have a design and an implementation but we've decided we should get
> some feedback from security experts before adding it to the project.
>
> I'd really appreciate any feedback people can give us on the approach
> we are using. We're planning on adding two APIs to Django - a low-
> level API for signing and checking signatures on strings, and a high-
> level API for setting and reading signed cookies.
>
> The low-level API can be seen here:
>
> http://github.com/simonw/django/blob/signed/django/utils/signed.py
>
> The core signing logic lives in the Signer class on line 89:
>
> http://github.com/simonw/django/blob/signed/django/utils/signed.py#L89
>
> Here are the corresponding unit tests:
>
> http://github.com/simonw/django/blob/signed/tests/regressiontests/utils/signed.py
>
> To summarise, the low-level API is used like this:
>
>>>> signer = Signer('my-top-secret-key')
>>>> value = u'Hello world'
>>>> signed_value = signer.sign(value)
>>>> signed_value
> 'Hello world:DeYFglqKoK8DLYD0nQijugnZaTc'
>>>> unsigned_value = signer.unsign(signed_value)
>>>> unsigned_value
> u'Hello world'
>
> A few notes on how the code works:
>
> Signing strings
> ---------------
>
> Actual signatures are generated using hmac/sha1. We encode the
> resulting signature with base64 (rather than the more common
> hexdigest). We do this because the signatures are expected to be used
> in both URLs and cookies, where every character counts. The relevant
> functions are:
>
> def b64_encode(s):
> return base64.urlsafe_b64encode(s).strip('=')
>
> def base64_hmac(value, key):
> return b64_encode(
> (hmac.new(key, value, sha_constructor).digest())
> )
>
> Picking a key to use for the signature
> --------------------------------------
>
> We've been (I think) pretty paranoid about picking the key used for
> each signature. Every Django installation has a SECRET_KEY setting
> which is designed to be used for this kind of purpose. Rather than use
> the SECRET_KEY directly as the key for signatures, we derive a key for
> each signature based on the SECRET_KEY plus a salt.
>
> We do this because we're worried about attackers abusing a component
> of a Django application that allows them to control what is being
> signed and hence increase their information about the key. I don't
> know if hmac/sha1 can be attacked in this way, but I'm pretty sure
> this is a good idea. If you have any tips as to how I can explain the
> benefits of this in the documentation for the feature they would be
> greatly appreciated!
>
> By default, the key we actually use for the signature (and hence pass
> to the base64_hmac function above) is sha1('signer' + SECRET_KEY +
> salt). 'salt' defaults to the empty string but users will be
> encouraged to provide a salt every time they use the low-level signing
> API - Django functionality that calls it (such as the signed cookie
> implementation) will always use a salt.
>
> Here's the signature method from the Signer class in full:
>
> def signature(self, value, salt=''):
> # Derive a new key from the SECRET_KEY, using the optional salt
> key = sha_constructor('signer' + self.key + salt).hexdigest()
> return base64_hmac(value, key)
>
> Appending the signature to a string
> -----------------------------------
>
> API users are not expected to call that signature() method directly
> though. Instead, we provide two methods - sign() and unsign() - which
> handle appending the signature to the string.
>
> Here they are in full:
>
> def sign(self, value, salt='', sep=':'):
> value = smart_str(value)
> return '%s%s%s' % (
> value, sep, self.signature(value, salt=salt)
> )
>
> def unsign(self, signed_value, salt='', sep=':'):
> signed_value = smart_str(signed_value)
> if not sep in signed_value:
> raise BadSignature, "No '%s' found in value" % sep
> value, sig = signed_value.rsplit(sep, 1)
> expected = self.signature(value, salt=salt)
> if sig != expected:
> # Important: do NOT include the expected sig in the exception
> # message, since it might leak up to an attacker!
> raise BadSignature, 'Signature "%s" does not match' % sig
> else:
> return force_unicode(value)
>
> The smart_str method simply ensures that any Python unicode strings
> are converted to UTF8 bytestrings before being signed. force_unicode
> converts utf8 bytestrings back to Python unicode strings.
>
> As you can see, the separator between the signature and the value
> defaults to being a ':'. I plan to move it from being an argument on
> the sign and unsign methods to being an argument to the Signer class
> constructor.
>
> Including a timestamp with the signature
> ----------------------------------------
>
> The second class in the signed.py module is a subclass of Signer that
> appends a unix timestamp to the string before it is signed. This
> allows the unsign() method to specify a max_age - if the signed value
> is older than that max_age, it is discarded.
>
> Here's the code:
>
> class TimestampSigner(Signer):
> def timestamp(self):
> return baseconv.base62.from_int(int(time.time()))
>
> def sign(self, value, salt='', sep=':'):
> value = smart_str('%s%s%s' % (value, sep, self.timestamp()))
> return '%s%s%s' % (
> value, sep, self.signature(value, salt=salt)
> )
>
> def unsign(self, value, salt='', sep=':', max_age=None):
> value, timestamp = super(TimestampSigner, self).unsign(
> value, salt=salt, sep=sep
> ).rsplit(sep, 1)
> timestamp = baseconv.base62.to_int(timestamp)
> if max_age is not None:
> # Check timestamp is not older than max_age
> age = time.time() - timestamp
> if age > max_age:
> raise SignatureExpired, 'Signature age %s > %s
> seconds' % (
> age, max_age
> )
> return value
>
> As you can see, the timestamp is appended to the value before it is
> calculated, and a max_age can be passed to the unsign() method and
> will be checked before the string is returned.
>
> Again, as a space saving the unix timestamp is a base62 encoded - this
> shrinks it and makes it suitable for inclusion in a URL (for example).
>
> Signing cookies
> ---------------
>
> Signed cookies are provided using two new methods on core Django
> objects: a get_signed_cookie() method on the Django request object and
> a set_signed_cookie() method on the Django response. Here's what a
> very simple Django view might look like that reads the name from a
> signed cookie and sets that cookie if a new name has been provided in
> a POST parameter:
>
> def index(request):
> name = request.get_signed_cookie('name')
> set_cookie = False
> if name in request.POST:
> name = request.POST['name']
> set_cookie = True
> response = render_to_response('index.html', {
> 'name': name,
> })
> if set_cookie:
> response.set_signed_cookie('name', name)
> return response
>
> The get_signed_cookie() method is implemented here:
>
> http://github.com/simonw/django/blob/signed/django/http/__init__.py#L66
>
> And set_signed_cookie() is here:
>
> http://github.com/simonw/django/blob/signed/django/http/__init__.py#L388
>
> Both of these methods take an optional 'salt' argument - but even if
> you don't provide a salt, the name of the cookie will be used as the
> salt. If you DO provide a salt the actual salt used will be
> cookie_name + your_salt.
>
> SECRET_KEY rotation
> -------------------
>
> I'm still working out the details for this, but the final feature we
> want to provide is a mechanism for rolling out a new SECRET_KEY
> without breaking everything that has been signed with an old one. I
> believe this is best practice, but I'm eager to hear if it isn't.
>
> The plan is to support an optional OLD_SECRET_KEYS setting which is a
> list of old secret keys which should still work for unsigning but
> should not be used for signing.
>
> The UpgradingSigner class here is a start at this code:
>
> http://github.com/simonw/django/blob/signed/django/utils/signed.py#L142
>
> A piece of Django middleware will be provided that quietly "upgrades"
> any signed cookies that have been signed using one of the older keys,
> replacing them with a cookie signed with the current SECRET_KEY. This
> middleware will need to know the names of the cookies that should be
> upgraded and what salt was used to generate them.
>
> The process for rolling out a new SECRET_KEY then will be this:
>
> 1. Put the current secret key in OLD_SECRET_KEYS:
>
> OLD_SECRET_KEYS = ['your-current-secret-key']
>
> 2. Put the NEW secret key in SECRET_KEY:
>
> SECRET_KEYS = 'your-new-secret-key'
I presume this is a typo that should be SECRET_KEY?
> 3. Turn on the Django signed cookie upgrading middleware:
>
> MIDDLEWARE_CLASSES += (
> 'django.middleware.signedcookies.UpgradeOldSignedCookies',
> )
>
> 4. Tell that middleware which cookies to upgrade:
>
> SIGNED_COOKIES_TO_UPGRADE = (
> ('name', 'salt-for-name'),
> )
>
> Now wait a week while a bunch of cookies get upgraded, then remove the
> key from OLD_SECRET_KEYS.
My concern here is whether this last step (SIGNED_COOKIES_TO_UPGRADE)
will be sufficient. Specifically, I can see two use cases that will
fall through the cracks:
1) Cookies that don't use Django's signing infrastructure
2) Uses of signing that don't use cookies - e.g., login tokens.
Dealing with (1) is a legacy problem - we can't really be expected to
migrate cookie signing techniques that aren't ours. However, it will
be a legacy problem that we will have, since our existing session
signing doesn't currently use the new signing library, and any
existing signed sessions will need to be resigned. Strictly, this
isn't a problem with keychange - it's a problem with a change of
signing method.
(2) is a problem that will exist long term. For example, our
login/password reset tokens will be affected if you change your secret
key, and the failure mode won't be particularly friendly to users.
I suspect the answer to both is another upgrade middleware (e.g., a
LegacySessionCookieUpgrade middleware, and a PasswordResetTokenUpgrade
middleware), but it's worth putting on the todo list.
Yours,
Russ Magee %-)
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-develop...@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.
