PASSWORD_HASHERS attempt to look up empty algorithm in certain cases

[Walter Doekes]

Hi,

the Django 1.4+ PASSWORD_HASHERS lookup conflicts with $1$<salt>$<password> (md5crypt) style hashes.

I'm using Modoboa which uses it's own auth first and falls back to Django auth if passwords don't match. For the aforementioned hash the algorithm detected becomes '' (the empty string).

1.4:

    if len(encoded) == 32 and '$' not in encoded:
        hasher = get_hasher('unsalted_md5')
    else:
        algorithm = encoded.split('$', 1)[0]
        hasher = get_hasher(algorithm)

master:

    if ((len(encoded) == 32 and '$' not in encoded) or
            (len(encoded) == 37 and encoded.startswith('md5$$'))):
        algorithm = 'unsalted_md5'
    else:
        algorithm = encoded.split('$', 1)[0]

That yields:

ValueError: Unknown password hashing algorithm ''. Did you specify it in the PASSWORD_HASHERS setting?

I thought I could fix that by adding a custom hasher which always returns False, but that fails because I'm not allowed to use the empty name for algorithm.

ImproperlyConfigured: hasher doesn't specify an algorithm name: modoboa.auth.hashers.NoAlgorithmHasher

To fix this, I either need it to refuse the empty algorithm (fix1.patch), or allow the empty algorithm (fix2.patch, where I'll have my bogus hasher return false later on).

In both cases the fixes are trivial.

As an aside, the current if not getattr(hasher, 'algorithm') looks like a typo. I'd go for either if not hasher.algorithm or if not getattr(hasher, 'algorithm', None).

        if not getattr(hasher, 'algorithm'):
            raise ImproperlyConfigured("hasher doesn't specify an "
                                       "algorithm name: %s" % backend)

Kind regards,
Walter Doekes
OSSO B.V.

[Walter Doekes]

Example to go along with fix2.

[Claude Paroz]

As explained here [1], Django currently only supports hashes in the form <algorithm>$<iterations>$<salt>$<hash>. There is also a note about bcrypt [2] that explicitely tells about incompatibility of such hashes (starting with $), and even suggests updating hashes in the database.

In 1.5, you could create a custom user model and customize check_password to pass a modified hash to the hashers.check_password function.

What Django could do in the future (need to be confirmed by a security-savvy developer) is to pass hashes to the hashers who could determine themselves if they support the hash format (in identify_hashers). Marking as DDN for this idea.

[1] ​https://docs.djangoproject.com/en/dev/topics/auth/passwords/
[2] ​https://docs.djangoproject.com/en/dev/topics/auth/passwords/#using-bcrypt-with-django

[Claude Paroz]

Proof of concept about identify_hasher asking hashers

[Walter Doekes]

Thank you for the links. In my case I cannot switch passwords at will since the mail auth daemon uses them too.

I like the idea of your identify_hasher fix. That would be an even better solution indeed.

Regards,
Walter

[Jacob]

Now that we have custom user models I don't really see the need to support custom schemes in this way; marking wontfix.

[Claude Paroz]

As for me, I think that custom user model is orthogonal to custom hashers functionality. The idea here is to give more flexibility to the password hashers system, and not hard code the hash starting with $algorithm$....
I even think that asking people having to cope with custom password hashes to rewrite the password checking mechanism instead of simply creating their own hasher has security implications, as they might easily introduce flaws in this sensible part of their code.