Design and code review requested for Django string signing / signed cookies
Simon Willison
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'
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.
Sending feedback
----------------
Does this look sane? Have we overlooked anything? Is there anything we
can do to make this more secure by default?
I'll read any replies here, or you can e-mail feedback to simon AT
simonwillison.net. Please say if you don't want stuff sent to that
address to be shared in public.
Thanks,
Simon Willison
Simon Willison
"@simonw why sha-1 instead of sha-256? NIST has recommended not using
SHA-1 in new systems: http://bit.ly/6bIf5h"
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).
My understanding is that the collision weaknesses discovered in SHA-1
are countered by the use of HMAC. Here's Bruce Schneier on the matter:
http://www.schneier.com/blog/archives/2005/02/sha1_broken.html
"It pretty much puts a bullet into SHA-1 as a hash function for
digital signatures (although it doesn't affect applications such as
HMAC where collisions aren't important)."
Despite the confusing API name, we're doing HMAC here, not digital
signatures - so I think we're OK. If I'm wrong I'm sure a crypto geek
will set me straight pretty quickly.
Cheers,
Simon
Simon Willison
you can follow the threads here:
http://news.ycombinator.com/item?id=1030290
http://www.reddit.com/r/programming/comments/ald1m/calling_crypto_security_experts_help_review_the/
tptacek on news.ycombinator pointed out a timing attack based on our
use of an insecure string comparison (an attack which affected Rails a
while ago). We can fix that using a constant time string comparison
such as this one:
ascii on programming.reddit has convinced me to ditch the sep=":"
argument and hard code the separator. Customising that doesn't feel
like a feature anyone will ever need. They also repeated the advice to
use SHA-256 - I think I'll almost certainly have to give up my quest
for shorter signatures :(
Jordan Christensen
http://csrc.nist.gov/groups/ST/hash/statement.html
"There are many applications of hash functions, and many do not
require strong collision resistance; for example, keyed hash
applications, such as the Hash-based Message Authentication Code
(HMAC) or key derivation applications of hash functions do not seem to
be affected."
Their plan in the same article doesn't mention transitioning off of
SHA-1 for HMAC related applications.
They also mention that SHA-1 is allowable for HMAC related use after
the 2010 switch over:
http://csrc.nist.gov/groups/ST/hash/policy.html
"After 2010, Federal agencies may use SHA-1 only for the following
applications: hash-based message authentication codes (HMACs); key
derivation functions (KDFs); and random number generators (RNGs)."
However it does say:
"Regardless of use, NIST encourages application and protocol designers
to use the SHA-2 family of hash functions for all new applications and
protocols."
Is there a good way to make it forward upgradeable? Allow the
developer to decide on the shorter SHA-1 hash or the (theoretically)
more secure SHA-256?
Jordan
Daniel Pope
probably be more than optional - always issued, and checked by
default. The danger is that users might think "signing" is a
bulletproof guarantee that the cookie as received is exactly the
latest cookie issued to that unique user and use them where a replay
attack would be unacceptable for security. There is still a max_age
window for replay attacks, which I don't know of any way to close.
Encrypting the cookies as well would make replay attacks a little
trickier, forcing the attacker to use trial-and-error rather than
identifying targets by inspection. Security through obscurity only,
though cookies being opaque to the user can be useful in its own
right.
The API could accept a user-supplied "check" string so that if the
user can supply some other details - maybe an IP - and have the API
verify this for them (thus preventing replay attacks for different
IPs).
> unsigned_value = signer.unsign(signed_value)
On an API level, 'unsign' isn't an obvious choice. The operation you
are doing is verifying and stripping a signature, so I'd expect
'verify' in the name. You also might want access to the signature
data.
So for example, something like this:
>>> s = SignedString(signed_value)
>>> s
'Hello world'
>>> s.timestamp
1262612904
>>> s.is_key_current()
True
>>> s = SignedString(old_signed_value, max_age=3600)
...
SignatureExpired: signature older than max_age
Dan
Simon Willison
> Is there a good way to make it forward upgradeable? Allow the
> developer to decide on the shorter SHA-1 hash or the (theoretically)
> more secure SHA-256?
There is - we can expand the BACKEND setting which is already in place
for signed cookies (but not for other clients of the Signer class). I
think we should do this. For one thing, it would mean we could provide
a backend which uses the Google keyczar Python library instead of the
code that we write. keyczar is properly audited, but depends on
PyCrypto so isn't appropriate as a required dependency of Django.
Another thing we can do is use SHA-256 but truncate the HMAC to 128
characters. Apparently it's perfectly fine to do this - it's being
discussed in the programming.reddit thread at the moment.
Michael Malone
could always truncate the result. If the original hash is
cryptographically secure then a truncated version is too. It just
increases the likelihood of a collision.
Mike
On Jan 4, 2010, at 6:49 AM, Simon Willison <si...@simonwillison.net>
wrote:
> --
>
> You received this message because you are subscribed to the Google
> Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com
> .
> To unsubscribe from this group, send email to django-develop...@googlegroups.com
> .
> For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
> .
>
>
marknca
some of the issues being discussed and hopefully provide a little
insight into the _why_ of it all.
SHA-1 is perfectly fine for HMAC (as verified by Bruce Schneier, NIST,
and several others). Here's why:
SHA-1
==========
plaintext -> SHA-1 = hash
HMAC-SHA-1
============
plaintext + key -> SHA-1 = hash
The key is --excuse the pun-- the key. The receiver of the message
will use the key in the decryption process and will then be able to
verify the authenticity and _integrity_ of the message.
Assuming that the key exchange is safe (a completely different
discussion), a collision in the hash space will not affect security in
HMAC.
A collision (in a simple hash) results when random plaintext creates
the same hash as the "real" plaintext. An attacker can exploit this to
open an avenue into your application.
Because each party has the key, this avenue extremely unlikely to be
open. If the attack is tried the validation of the hash fails because
the receiver already has the key for the message.
That being said, I would still use SHA-256...why use a weaker
algorithm when a stronger one is available at almost no cost?
I don't have the mathematical proof that a truncated hash is still
valid (someone jump in if they do) but the logic holds if a smaller
hash is required.
Because the SHA algorithm diffuses changes in the plaintext throughout
the resulting hash, truncating the SHA-256 hash simply reduces the
possible hash space, thus increasing the possibility of a collision.
There may be other ramifications but it's definitely worth exploring.
(I'm checking out the other threads right now).
Is the short URL requirement really required by the large community?
(honest question, not trying to be an @$$) Don't most email clients
(including mobile) handle multi line URL's properly now?
Also if you're planning on expanding the API in the future, please
setup version-ing now. It's a lot easier to handle when it's built in
from the start.
Keep up the great work!
Mark
@marknca
James Bennett
need to be made to start bringing it up to snuff, make me feel far too
nervous about this being ready in time to make 1.2 at all. I know
you've put in the effort to shepherd this along, but I'm starting to
think it's time to push this to the 1.3 release cycle (especially
since 1.2 alpha freeze is tomorrow, and I don't think there's any way
it'll be even alpha-ready by then).
--
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."
Simon Willison
> Simon, the amount of pushback this is getting, and the changes which
> need to be made to start bringing it up to snuff, make me feel far too
> nervous about this being ready in time to make 1.2 at all. I know
> you've put in the effort to shepherd this along, but I'm starting to
> think it's time to push this to the 1.3 release cycle (especially
> since 1.2 alpha freeze is tomorrow, and I don't think there's any way
> it'll be even alpha-ready by then).
I certainly don't think we should check this in for the alpha freeze.
We do however need to consider the places in Django that are already
using hmac / md5 / sha1 (contrib.formtools and middleware.csrf for
example). Even if we don't add the signed cookies feature to 1.2,
fixing any problems with our existing use of crypto should not be
affected by the feature freeze. There's not much point in implementing
this logic in several different places, so I think we should keep
targeting the django.utils.signed module for 1.2.
Cheers,
Simon
Jacob Kaplan-Moss
> We do however need to consider the places in Django that are already
> using hmac / md5 / sha1 (contrib.formtools and middleware.csrf for
> example). Even if we don't add the signed cookies feature to 1.2,
> fixing any problems with our existing use of crypto should not be
> affected by the feature freeze. There's not much point in implementing
> this logic in several different places, so I think we should keep
> targeting the django.utils.signed module for 1.2.
Agreed - I see no issues with targeted it for the beta (but using it
for signed cookies probably has to slip to 1.3). It's certainly an
improvement to have a single place where this sensitive code lives.
From a worst-case-scenario standpoint, it'd make a security fix much
easier to only have a single place to fix it.
Jacob
jcampbell1
On Jan 4, 7:47 am, Simon Willison <si...@simonwillison.net> wrote:
> And set_signed_cookie() is here:
>
> http://github.com/simonw/django/blob/signed/django/http/__init__.py#L388
>
I am not that familiar with your framework, but I think a signed
cookie should use http only cookies by default. There is no valid
reason for a script to read a cookie that it can't verify. http only
cookies significantly decrease the surface area of XSS attacks.
Regards,
John Campbell
Luke Plant
> I am not that familiar with your framework, but I think a signed
> cookie should use http only cookies by default. There is no valid
> reason for a script to read a cookie that it can't verify. http
> only cookies significantly decrease the surface area of XSS
> attacks.
I can think of various circumstances where it might be useful and
harmless. For example, if the signed cookie stored the user's login
name, and some client side javascript used the login name for some
convenience feature, like highlighting the login name wherever it
appeared on the page.
To generalise, the issue of using HttpOnly cookies is orthogonal to
whether they are signed or not, because the value of the cookie can be
used in multiple ways, not all of which will depend on the value being
verified.
Luke
--
"Love is like an hourglass, with the heart filling up as the brain
empties."
Luke Plant || http://lukeplant.me.uk/
Alex Gaynor
>
> You received this message because you are subscribed to the Google Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to django-develop...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
>
>
>
So, thinking out loud here, I know the DSF has a policy of hands of in
the development of Django, but I was thinking (out loud) that perhaps
it would be sensible for the DSF to hire someone to do a security
audit of some of this stuff. I have 0 clue about the particulars of
how anything like that works, but it was just a thought that occurred
to me.
Alex
--
"I disapprove of what you say, but I will defend to the death your
right to say it." -- Voltaire
"The people's good is the highest law." -- Cicero
"Code can always be simpler than you think, but never as simple as you
want" -- Me
Tobias McNulty
the development of Django, but I was thinking (out loud) that perhapsSo, thinking out loud here, I know the DSF has a policy of hands of in
it would be sensible for the DSF to hire someone to do a security
audit of some of this stuff. I have 0 clue about the particulars of
how anything like that works, but it was just a thought that occurred
to me.
Cheers
Tobias
--
Tobias McNulty
Caktus Consulting Group, LLC
P.O. Box 1454
Carrboro, NC 27510
(919) 951-0052
http://www.caktusgroup.com
Jacob Kaplan-Moss
> So, thinking out loud here, I know the DSF has a policy of hands of in
> the development of Django, but I was thinking (out loud) that perhaps
> it would be sensible for the DSF to hire someone to do a security
> audit of some of this stuff. I have 0 clue about the particulars of
> how anything like that works, but it was just a thought that occurred
> to me.
The policy isn't exactly "hands off;" it's just that the DSF has no
interest in driving the development of Django. But the DSF certain can
(and in my opinion should) pay for services like this *if* the
development team and community thinks it's necessary.
That said, I think the idea's probably a non-starter. First, like
Tobias (see below) I tend to highly believe in the mantra of many
eyeballs, so I'd argue that an expert review in no way absolves of the
need for open peer review. However, it would tend to discourage
community review -- paid work almost always discourages volunteers --
and would thus be a net loss. Second, from a more pragmatic point of
view, my impression is that anyone who's actually worth paying would
cost well beyond what the DSF could actually afford.
Don't mean to shoot you down, though, and I *really* like the
precedent you're setting by bringing this up. The main reason I wanted
to have a DSF in the first place was so that we could have options
like this. Even if we don't use the option this time (and I think we
shouldn't) it's good for the community to know that it's available.
Jacob
richar...@gmail.com
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)
may I suggest putting the salt before the constant string "signer",
like this:
def signature(self, value, salt=''):
# Derive a new key from the SECRET_KEY, using the optional salt
key = sha_constructor(salt + 'signer' + self.key).hexdigest
() ## THIS LINE IS DIFFERENT
return base64_hmac(value, key)
If a constant string comes first, an attacker can calculate the
partial sha hash of 'signer' and cache the state, making this prefix
pretty much useless. By putting the salt first, they'd have to do a
lot more calculation.
There are obviously other corresponding changes when you check the
signature.
Russell Keith-Magee
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 %-)
Elias Torres
I'm not a security expert by any means, but I really the fact that
you're making use of HMACs in your design. I will ask a good friend
(Ben Adida) who's really an expert on the subject to see if your
paranoia on adding a salt and hashing the key helps you in any way. My
intuition says if the salt will be stored and made available to anyone
with access to the DB, I'm not sure it will make much of a difference.
HMAC takes care of the prefix/suffix attacks already.
Anyway, the main point of my email is to remind folks here that
password management in Django is still not using HMAC-based hashing.
Please read [1] on the matter. I'm currently using Tornado so I used a
couple of methods from Django auth [2] blindly thinking that Django
was doing the right thing and after a friendly code review I was
caught with simple hashing. The good thing is that it was very easy
for me to make a backwards-compatible change to the get_hexdigest()
function:
if algorithm == 'hmac':
return hmac.new(raw_password, salt, hashlib.sha1).hexdigest()
The nice property of the authentication code path, is that the secret
key is owned by the user logging in and no-one else. Therefore, the
chance of discovering people's passwords with a dump from a Django
application is really small.
[1] http://benlog.com/articles/2008/06/19/dont-hash-secrets/
[2] http://code.djangoproject.com/svn/django/trunk/django/contrib/auth/models.py
Regards,
Elias Torres
On Jan 4, 7:47 am, 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/uti...
Karen Tracey
I'm not a security expert by any means, but I really the fact that
you're making use of HMACs in your design.
There seems to be a key word missing here: really <?> the fact...
(One might guess based on the rest of the message, but still it might be nice to know what was intended to go there.)
Karen
Luke Plant
> Simon,
>
> I'm not a security expert by any means, but I really the fact that
> you're making use of HMACs in your design. I will ask a good friend
> (Ben Adida) who's really an expert on the subject to see if your
> paranoia on adding a salt and hashing the key helps you in any way.
> My intuition says if the salt will be stored and made available to
> anyone with access to the DB, I'm not sure it will make much of a
> difference. HMAC takes care of the prefix/suffix attacks already.
The point of the 'salt' is to make it easy to use unique keys.
Otherwise, one use of HMAC with SECRET_KEY in a web app could be
trivially used to compromise another usage.
For example, suppose that, on a social network, the user can specify
the username of someone that they nominate as a 'best friend' . This
value might be stored in a signed cookie. If we use SECRET_KEY
without salt as the HMAC key, the user then has access to the value
HMAC(SECRET_KEY, some_username). But suppose another signed cookie is
used to store the username when a user logs in (as opposed to using a
session). The value will be HMAC(SECRET_KEY, users_username). Since
an attacker can trivially get hold of HMAC(SECRET_KEY,
somone_elses_username), they can log in as anyone they like.
'Salt' in the key is to protect against that. The signed cookie
implementation, for example, uses the cookie *name* as part of the
salt, so that all cookies have their own key. The salt is not a
secret, so doesn't provide any additional protection against brute
force attacks, but it isn't meant to.
Luke
--
"Making it up? Why should I want to make anything up? Life's bad
enough as it is without wanting to invent any more of it." (Marvin
the paranoid android)
Luke Plant || http://lukeplant.me.uk/
Elias Torres
Elias Torres
Thanks Luke for your explanation. I think I have learned something
here in terms of my own application security independent of Django's
multi-app environment. Basically, you're reminding me that as an
application, I must be careful to not sign any random string for a
user, because it can be re-used for another purpose. Therefore, we
include the 'purpose', salt, or namespace in the process. One thing I
would like to ask though, is whether the salt needs to be part of the
key or the value? If you look at TimestampSigner, the timestamp goes
as part of the value. I think the same can be done with the name of
the cookie. In other words you can always use a method like
_cookie_signature as in Tornado's implementation [1] and always pass
two parts: cookie name and cookie value. Technically, as long as your
SECRET_KEY is protected, there should not be a need to creating
multiple signing keys, especially if the data is not secret.
def _cookie_signature(self, *parts):
hash = hmac.new(SECRET_KEY, digestmod=hashlib.sha1)
for part in parts: hash.update(part)
return hash.hexdigest()
Any thoughts on Django's auth using HMAC besides md5 and sha1 hashing
alone?
Thanks,
-Elias
[1] http://github.com/facebook/tornado/blob/master/tornado/web.py#L251
Luke Plant
> Thanks Luke for your explanation. I think I have learned something
> here in terms of my own application security independent of
> Django's multi-app environment. Basically, you're reminding me
> that as an application, I must be careful to not sign any random
> string for a user, because it can be re-used for another purpose.
> Therefore, we include the 'purpose', salt, or namespace in the
> process. One thing I would like to ask though, is whether the salt
> needs to be part of the key or the value? If you look at
> TimestampSigner, the timestamp goes as part of the value. I think
> the same can be done with the name of the cookie. In other words
> you can always use a method like _cookie_signature as in Tornado's
> implementation [1] and always pass two parts: cookie name and
> cookie value. Technically, as long as your SECRET_KEY is
> protected, there should not be a need to creating multiple signing
> keys, especially if the data is not secret.
>
> def _cookie_signature(self, *parts):
> hash = hmac.new(SECRET_KEY, digestmod=hashlib.sha1)
> for part in parts: hash.update(part)
> return hash.hexdigest()
This is equivalent to:
hmac.new(SECRET_KEY,
digestmod=hashlib.sha1).update("".join(parts)).hexdigest()
With this, one problem is that accidental collisions between different
parts of code are easier. Consider two cookies:
Cookie 1, name = "use", value = arbitrary string set by user
Cookie 2, name = "user_id", value = server assigned once they've
logged in correctly.
By supplying "r_id123" as the value for cookie 1, I can forge a
user_id=123 cookie. If some validation is imposed on cookie 1, it
might still be possible to manipulate the system such that "r_id123"
is a valid choice, and the exploit would still work.
Actually, the implementation in Tornado does not include the cookie
name in the signature at all, making it even more vulnerable to this
kind of attack.
So that would be my defence of why it's better to put the "purpose"
namespace into the key, rather than the value, in the context of HMAC.
I'm not an expert though.
This is one of the tricky things with security - it's never just a
case of "use this API" - what you feed into the API is always
critical.
> Any thoughts on Django's auth using HMAC besides md5 and sha1
> hashing alone?
It sounds like a good idea, I'm not aware of any particular problems
with our current implementation that would make this a priority
change.
Luke
--
"Mediocrity: It takes a lot less time, and most people don't
realise until it's too late." (despair.com)
Luke Plant || http://lukeplant.me.uk/
Elias Torres
Agreed.
>
> Actually, the implementation in Tornado does not include the cookie
> name in the signature at all, making it even more vulnerable to this
> kind of attack.
Absolutely. I mailed the Tornado mailing list last night as well on
the matter.
>
> So that would be my defence of why it's better to put the "purpose"
> namespace into the key, rather than the value, in the context of HMAC.
> I'm not an expert though.
Can a separator solve that issue?
>
> This is one of the tricky things with security - it's never just a
> case of "use this API" - what you feed into the API is always
> critical.
Absolutely. It can become a very powerful black box.
>
> > Any thoughts on Django's auth using HMAC besides md5 and sha1
> > hashing alone?
>
> It sounds like a good idea, I'm not aware of any particular problems
> with our current implementation that would make this a priority
> change.
>
I think you are right and the 'being able to sign other messages'
attack that HMAC protects us against is not the main issue with
passwords. However, one thing I see some folks moving towards are
slower hashing functions to better protect stolen databases against
dictionary attacks. My friend enlightened me that computing SHA1 HMACs
or hashes it's way too fast so even with a salt, you can still match
many passwords from a stolen database. bcrypt on the other hand is
much slower. But that's just an FYI, not really suggesting to use it.
Thanks for the answers. Every time the subject comes up, I learn
something else about security and passwords.
[1] http://www.mindrot.org/projects/py-bcrypt/
Luke Plant
> > So that would be my defence of why it's better to put the
> > "purpose" namespace into the key, rather than the value, in the
> > context of HMAC. I'm not an expert though.
>
> Can a separator solve that issue?
In that instance, yes. I'm wary of other applications of HMAC
producing loopholes in which the user provides the separator as part
as the value being signed, and is able to generate the same string. In
Tornado, they are suggesting have a separate key for signing cookies,
in which case just signing "name=value" should be enough (provided the
developer doesn't do something silly like make "=" part of the name of
the cookie).
Julian
> 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.
Here's my $.02 based on a cursory review. I like the idea of putting
this logic into django. I find myself implementing something similar
often.
This all looks ok from a security perspective, but it seems there are
a couple of things that could be simplified:
1) sep. Why use a delimiter at all? The fields you are encoding
(hash or hash+date) are of a determinate length. Put those fields
first, and expect them to have the correct length when unsigning.
2) salt + 'signer'. I don't see the security advantage to this. If
the django secret key is compromised, you are dead. If not, then
everything should be good. If someone can brute-force the key from a
large sample of signatures, and I agree that is a small but real risk
(at least I thought so with MD5), then they will much more easily
reverse the salt+'signer'+secret from that.
Unless you picture the salt changing frequently. In which case, how
does the unsigner know the salt?
I suspect there may even be a security risk to letting people use
salt. They may use something that is known to the attacker, and it
seems like this might carry some risk in itself.
One other thought - I know you expect this to be used for texty
protocols like http cookies, but should the signer class be producing
already-encoded data? It seems like the protocol using the signed
cookie should be in charge of encoding it. i.e. produce an 8-bit
chunk of data. Let the set-cookie logic decide what encoding it is
passed that binary chunk. This would make the base signer class more
useful for applications where encoding is not necessary. And set-
cookie should be robust enough to know when the data passed to it
needs encoding.
-=Julian=-