0

my algorithm looks like this:

$new_password = sha1($salt . $password . $email);

it works good, but Im trying to change to sha1 since ive heard its better but it wont work. Why is that?

register:

//generate a strong unique salt
$salt = uniqid(mt_rand());

$new_password = sha1($salt . $password . $email);

and then i rehash it when i log in

tomek wojtek
  • 1
  • What doesn't work? Are there any error messages – Yacoby Oct 18 '09 at 13:05
  • 2
    Please don't roll your own password hashing code and read this answer: http://stackoverflow.com/questions/1581610/help-me-make-my-password-storage-safe/1581919#1581919 – Inshallah Oct 18 '09 at 13:06
  • Do you already have MD5 hash values in your database? Or do you just want to use `sha1` instead of `md5` right from start? – Gumbo Oct 19 '09 at 17:02

3 Answers3

3

Are you storing the salt with the hashed password? You need to use the same salt when checking the hash - each user should have their email address, salt and hash stored.

Greg
  • 316,276
  • 54
  • 369
  • 333
  • Greg is correct. You will either need to store the salt in a separate db field or include the salt in the hash with a delimiter like this: $hash = $salt . '$' . sha1($salt . $password . $email); – Mark Oct 18 '09 at 14:13
0

Yes, sha1 is a better hash than md5. Much of the time tho, md5 is good enough... Really, how "insecure" your site is is determined much more by other factors.

dicroce
  • 45,396
  • 28
  • 101
  • 140
  • 1
    I suppose it depends a bit on how determined an adversary is to break you security. Finding MD5 collisions _is_ practical with a certain amount of computing power, and computing power is not that difficult to come by. – Inshallah Oct 18 '09 at 16:39
  • 1
    just because some other part is insecure is not an excuse for using an algorithm that is cryptographically broken and unsuitable for further use. – Jacco Oct 18 '09 at 20:42
  • I was referring to non primarily security focused uses of MD5... Sure, use sha1 for your password hashes... But there are plenty of uses for hashes (for example, file modification detection) that work perfectly well with md5... I'm not trying to argue that someone should use an insecure hash, just that I bet his site securing energy would be better spent on form validation or protecting against xss than the security difference between sha1 and md5... – dicroce Oct 18 '09 at 22:10
-1

You want a constant string for your salt, eg 

$salt = "iodized sea salt"

EDIT: The commenters and downvoters here seem not to understand the concept of a salt. Using a constant salt certainly would not defeat the purpose of the salt, which is to make your choice of hashing algorithm ("SHA1 with such-and-such a salt") private. This salt can be long and its choice has more entropy than, say, uniqid(mt_rand()). You can't brute force a long salt any time this side of never. A per-user salt might make you feel better if you're not clear on computer security, but doesn't provide any other benefit.

Grumdrig
  • 16,588
  • 14
  • 58
  • 69
  • Not completely - it's still better than no salt. It just means an attacker would have to generate their own rainbow table rather than using an existing one. – Greg Oct 18 '09 at 13:29
  • 2
    It's not much better than using no salt at all. Using a constant salt will allow an adversary to do a brute force search against the whole set of acquired password hashes all at once, instead of only a single one. – Inshallah Oct 18 '09 at 13:32
  • Oh _ I see where you guys are being tripped up. Yes, hash($constant_salt, $password) would be less secure than hash(user_salt($username), $password). But user_salt($username) is $constant_salt . $username which is no more knowable to an attacker than one generated some other way. $constant_salt is secret information. – Grumdrig Oct 19 '09 at 17:48
  • @Grumdrig, salts are not supposed to be private information. That's why we're using password hashing algorithms, so that we don't have to store secrets anywhere. Storing an additional secret in your codebase can only be considered obfuscation, not real security. (Both, the database and the codebase must be readable by many people, and an adversary who can gain access to one, may well gain access to the other). – Inshallah Oct 20 '09 at 14:50
  • Let's look at it another way. Let's say that using the SHA-256 crypt algorithm makes brute forcing a single password 10000 times more difficult than using a plain hash. However, if an adversary has gained access to 10000 password hashes computed with a constant salt, and he only needs to break a single one to be successful, then his task has become about 9999 times easier, and that would void any benefit gained from the more costly crypt algorithm. – Inshallah Oct 20 '09 at 14:55
  • Also, don't sweat the downvotes. The fact that there have been several comments makes this answer a good reference for the constant salt issue. – Inshallah Oct 20 '09 at 15:09
  • Thanks for the words of encouragement, @Inshallah. :) We should probably have been more careful about defining our threat model; yours as I read it is an attacker with full access to code & db trying to recover passwords. In other cases, however, keeping a secret is real security, not just obfuscation. Public key encryption, e.g., depends on a secret private key. Regardless, while, as you say, a universal constant salt would be bad, username+constantsalt isn't a universal constant salt; it's a per-user salt. – Grumdrig Oct 20 '09 at 19:29
  • And to clarify, we probably agree that keeping a constant salt secret has equal utility to keeping per-user salts secret. At least I hope we do. – Grumdrig Oct 20 '09 at 19:32
  • @Grumdrig, ah, I get what you mean; rather than computing the salt from a RNG generated value, or the current time, or some other external factors, you suggest that the salt be computed from *constant* information about the user; that way the OP's original problem about salt storage would be solved. I don't really see a problem with that, though I should note that you probably want to run the salt through a hashing function once; I don't really know why, but it would make me feel safer about using this approach :-). – Inshallah Oct 20 '09 at 20:28
  • I *do* think that hashing-in a secret value would be ideal, whether that value is per-user or global. In fact, having a global secret is probably a better idea, since it only needs to be generated once and can then be stashed away somewhere; if it's easier to handle, it's easier to keep secret. – Inshallah Oct 20 '09 at 20:39