Just noticed you're using a single pass of SHA1 (salted) for password storage. Would you be opposed to a patch using a safer password storage mechanism? If not, I'll throw one your way in a couple hours.
if what you are thinking about is to use blowfish or other algorithm with a slow key scheduling step, what about if we just reiterate N times SHA1? Should be exactly as secure, like in:
SHA1(SHA1(SHA1(0|pass)|1)|2) and so forth.
This way there is no requirement for an additional library.
I would strongly recommend that rather than doing that, go with standard PBKDF2. In essence, HMAC(HMAC(HMAC(...(password)))) with a per-user salt. I generally recommend 10k+ rounds with PBKDF2 (each one is cheap). This wouldn't give you an additional dependency and is super easy to put in place -- I'll do it, if you want.
If you concatenate your password and a salt, then iterate a cryptographic hash like SHA256 a few thousand times, this is almost exactly PBKDF1. It's a good, respectable password hashing scheme. The main advantage that PBKDF2 offers is the ability to produce arbitrary output sizes.
(The difference between this and PBKDF1 is that PBKDF1 requires using either MD2 or SHA1 as the hash function, and hasn't been updated to reflect the availability of SHA-256 and SHA-512.)
Note that PBKDF1 is exactly that, vanilla chaining of the same hash function. And AFAIK is not believed to have known attacks, with the only drawback being the fixed output size.
I guess that the xor approach used in PBKDF2 is useful since you want to compute T1, T2, T3 ..., Tc that are multiple output blocks all starting from the same input, so this gives more information to the attacker and the schema is designed to avoid showing some "state" that is possible to more easily analyze. Not sure, but the point is, I don't think PBKDF1 is unsafe either, and it is just chaining.
It's very much up for debate how much bcrypt, scrypt, and friends buy you over iterated HMACs like PBKDF2, which can be implemented in just a few lines of code. While I'd recommend the use of bcrypt or scrypt for most purposes, I personally have no qualms recommending something like PBKDF2 with a large number of iterations -- it's pretty damn sound. Iterated SHA1 isn't much worse than any of them, but there are a couple optimizations you can make while brute-forcing them, which you can't make with iterated HMACs, so I'd recommend going with that route.
I agree, but we could do worse than to have "bcrypt" be the lay developer's default answer. All that is is annoying, whereas "secret salt construction using Whirlpool as the hash" or- whatever- craziness is actually dangerous.
Your code is neat. I don't care what hash you use. I'm just saying.
If iterated SHA1 and bcrypt are (all other things equal) equally secure, but you're more likely to mess up while implementing your own iterated SHA1, then bcrypt is more secure.
With perhaps five thousand iterations of SHA256, if you really don't want to depend on bcrypt. Or just use bcrypt; it's a good library, and removes the temptation to get creative with password hashing.
"PBKDF1 is recommended only for compatibility with existing applications since the keys it produces may not be large enough for some applications." — RFC 2898, September 2000
The keys it produces are big enough for this application. PBKDF2 can produce output keys of arbitrary size, which is why PBKDF1 got deprecated, but it's not always necessary.
