Reconsider harden_verify

Issue #83 resolved
Donald Stufft created an issue

I was looking over the 1.7 changelog and I noticed harden_verify (and the deprecated min_verify_time) and I would like to implore you to reconsider these options all together. They do not appear to meaningfully increase the security of a particular application.

I get that the idea is that if you have some users using a weak hash (say sha1) and some using a strong hash (say argon2) this would let you hide which users are still using a weak hash and which are not. However I don't think hiding that is actually all that useful. Using a slow password hash is there to protect against offline attacks from people who have gained a copy of the database. For this people, they can already tell who has a strong or a weak hash simply by looking at the data they possess.

So then, this only protects people who can submit password attempts but can't otherwise access the underlying data, presumably through something like an online login form. My guess is the goal here is to make it so that an attacker can't go "oh, I'll focus on the users that have a sha1 hash because I can submit more of those quicker". However, I think a better way to defend against this is to recommend people put rate limiting on their authentication (by IP, user, or whatever makes sense for them). This will provide much stronger protection not just against people with weak hashes, but also against people with weak passwords in general.

All in all, at best case all this tells you is if person A has a strong hash or not, but that information does not appear to overly useful. The only real data leakage I can think of beyond that is that it possibly lets you figure out when the last time they logged in was, but even that doesn't appear very useful to me. The dummy_verify is maybe useful, but I feel like it would be better implemented by just hashing a random (or empty string) password using the default hasher and then just throwing the result away before returning.

Comments (4)

  1. Eli Collins repo owner

    I agree with pretty much all those points... I never felt it was very effective, and didn't try to draw very much attention to it. Your analysis really pins down why pretty concretely too :)

    It got added mainly for parity with Django, which has a similar feature. I wanted parity with Django because the 'passlib.ext.django' module tries to provide a replacement for Django's hasher framework. But by the end of the 1.7 development cycle, I'm questioning the utility of that module -- I plan to split it out concurrent with the 1.8 release, and see what the actual userbase is for it, as a separate project. Which will probably give a good opportunity to reconsider harden_verify, as it's not providing much utility outside of that module.

    I'm very much in the mind of trimming the fat, so when 1.8.0's release comes around, and I haven't come up with any good counter-arguments, your vote tips me into the "kill it off" crowd.

    Regarding the dummy_verify() portion -- that part I probably will keep, because I've been using it in my own applications to fake a login delay for non-existent user accounts. But I deliberately have it using sleep(), because that only wastes the client app's time ... whereas hashing a dummy password wastes server cycles too.

    • Eli
  2. Eli Collins repo owner

    As a final nail in the coffin of this feature, I performed some timing tests on the currently implementation of dummy_verify(), and decided that it's just not a workable approach. Since the only other one I've seen is Django's, which (IMO) isn't the right approach either, I think this feature is dead architecturally; independent of arguments about it's utility.

    The newly released 1.7.1 turns harden_verify into a NOOP, and dummy_verify() just hashes a random string. The 1.8 branch has already had the harden_verify code stripped out.

  3. Log in to comment