Adding `crypt' to list of password hashes for legacy apps.

Patch to add crypt support to auth hashes

Hey, thanks for contributing. I have a few notes on your patch:

• The crypt library is only available under Unix. I'm not sure about the policy regarding this.
• Please don't add your login into the patch. It would make the code unreadable if every patch author did this.

Update to patch as per mir's comments.

I'm probably in favour of this, since it does help with porting existing apps across to Django (you don't need to recreate all passwords). Feels like we should document why the functionality is there, though; just not sure where to do that (in the code or in a doc file somewhere).

Also, for anybody else trying to work out where this applies (since the patch is ambiguous), it is a patch against django/contrib/auth/models.py.

When things have quietened down a bit on the developer's list, we should bring this up for discussion (including the reasoning why it is useful). It is a change in direction, but that is probably just because we haven't thought of it before.

for what it's worth, I'm +1 on this too - the whole point of the auth framework hashing is that it's extendible to other hash formats (this is why we have md5 and sha1). Crypt is a fairly common hash, and I'm sure a lot of people would find it useful.

However, I'm not sure if the if..elif..elif.. approach is best. It's a shame that the hash function in crypt isn't standardised (with "new()" and hexdigest(), or we could make this entirely future proof by doing something like:

try:
   hasher = __import__(algo)
   return hsh == hasher.new(salt+raw_password).hexdigest()

except ImportError:
   raise ValueError, "Got unknown password algorithm type (%s) in password." % algo

This way the you could use any hashing method you want.

That does have the disadvantage of importing something from the database...

Yes, I was really just thinking out loud, but I don't think there'll be any security issues - If someone tries to smuggle in some code in the "algo" slot, then import will fail (e.g. my hashing algorithm is called 'md5;print "fudge"') and raise an ImportError?.

An attacker could create a new module (with new() doing something malicious), but this would require them to get the module into an importable location, and get the hashing algo to match that - I *think* this is impossible.

I could be wrong (shared hosting situations come to mind).

If we did want to go down this path, then you could always have a SAFE_HASHERS list, which could be added to in settings.py

Simon, your dynamic import approach is dangerous for just the reasons you suggest: you are trusting externally supplied data as part of the security path. History suggests that as smart as programmers are, debugging is twice as hard and we will miss some sneaky way of smuggling in a bad import or having the bad import used in combination with a compromised machine to capture passwords (I have no idea how, but that's the point -- we can't think of everything). White-listing the permitted methods, whether through if...elif blocks or a list is the safer way.

I don't think the if...elif sequence is that bad. We need to have slightly different actions in each case and it's only around a dozen lines for the whole block. The alternative would be to look up the method in a table and dispatch to a separate function each time, which will end up with even more lines of code and harder to read. If that if-block gets to twice the size, we think of splitting it up. For now, I'm not unhappy with this.

I'm pretty happy with the patch. I'll apply it when I get a chance.

I've found some pure Python implementation of crypt here and there. Althought I have never tried it, it may be worth including (in case import crypt fails with an ImportError) so that would work on Windows as well. Or maybe it's not that important - who uses crypt on Windows anyway? - but being platform independent as far as possible always seemed to me like a good idea.

I don't think it's worth adding the software fallbacks. Both of the modules you point to admit to having problems in their code (and the one in the so-called public domain makes me nervous about the license -- it's not actually a good idea to release stuff into the public domain because it doesn't provide any legal protections at all).

Crypt is a Unix-based feature. So it's not really going to be useful on Windows setups and it will mean we're maintaining even more encryption methods: real crypt, Python crypt, etc... Fitting in with the existing server makes some sense, since password strings can be shared. Trying to port it to platforms where it isn't a used format, not so much.

The change in docs.

(In [5073]) Fixed #3316 -- Added support for crypt hashing of passwords, mostly to support easy porting from existing Unix-based legacy apps. Thanks, axiak@mit.edu.