ircmaxell / password_compat

Compatibility with the password_* functions that ship with PHP 5.5
MIT License
2.15k stars 421 forks source link

Storing "half" of the salt inside the .ini? #29

Closed Lucas-Malor closed 11 years ago

Lucas-Malor commented 11 years ago

This API is really useful, and I'm using it with PHP. I would propose here a little improvement.

This is a sort of pepper. I want to premise I know that pepper techniques are already proposed in past ( https://wiki.php.net/rfc/password_hash#the_api_does_not_support_pepper ), but I think my propose is better, because:

  1. there's no need to pass any additional pepper parameter
  2. you have not to use PHP variables
  3. it's (IMHO) better than using a block cypher

I think it's a good idea to have the possibility to add x characters as a variable of your ini file, an "ini salt". When password_hash() is invoked, by default it will generate a salt with length (y - x), when y is the length of the salt the algorithm requires. password_hash() will complete it with the ini salt and generate the hash. The output will have only the salt generated by password_hash(), not the ini salt. Of course password_verify() must also complete the salt using the ini one.

This way you have not to store the ini salt to a PHP variable, you can read the ini each time. Of course if you'll change the ini salt you have to rehash all the db passwords. This should be well documented inside the ini. Furthermore this doesn't protect you if all the site, not only the db, is compromised.

Of course ini salt should be ignored if the salt is specified as passwordhash option, and if the salt stored inside the hash is complete, password verify() (and password_needs_rehash() ) should not read the ini salt. Furthermore if the resulting salt length is incompatible with the algorithm, the functions should raise an error.

Let me know what do you think about.

ircmaxell commented 11 years ago

The biggest and strongest argument against this is that it would require a modification of the bcrypt algorithm (and in fact the crypt() library itself) to implement. Which is completely off the table.

Now, you mention "it's better than using a block cipher". Why would you say that? Block ciphers are designed for this exact use. So why would modifying a set of function to do something outside their design spec, be better than using two tools both inside their design spec?

ircmaxell commented 11 years ago

Additionally, check this StackOverflow answer for more reasons why the concept that you're looking for is a bad (less than optimal) one, and why block ciphers should be used instead...

Lucas-Malor commented 11 years ago

There's no need to change the bcrypt algorithm, as I described my proposal. I have not suggested to add another parameter, I have suggested to divide the salt into two parts, a "db" salt part generated by the API and stored inside the db, and an "ini" salt part, pregenerated by yourself ans stored inside the API ini file. It's the API that will divide and recollapse the salt; the standard hash functions will accept only one parameter as usual, the salt.

About your Stack Overflow answer, I find it very interesting and I agree with the most. Anyway I'm not completely convinced about the last part, because I don't think a block cipher will be a real problem for an hacker. I mean, if the hacker see my db and I have no salt columns in the user table and a VARCHAR(255) or a CHAR(60) as password column, there's an high probability that I'm using your password API. So if the column data are not formatted as expected (your API inserts also informations about the algorithm and its options), he could guess I'm using also a cipher and try some two-way decryptors that are part of the PHP standard. And this is more fast than guessing a pepper, since to find a pepper you must use brute force or violate the entire site.

About the other objections to peppers in your stack overflow answer, I think they do not apply to my proposal, since it doesn't feed another hash to the hashing function, and it's not complex at all.

ircmaxell commented 11 years ago

Well, the current "salt" is 128 bits wide. Which means that to include a secret component you'd have to sacrifice some of those bits. There are only a few ways to do this:

So why do I still say that we shouldn't do it? Well, the problem is not what you gain if the pepper part is kept secret, but what you lose if it's not.

If your system is compromised and the secret is revealed, you've handed the attacker a database that uses small enough salts that (s)he may gain an advantage in brute forcing (even a handful of collisions can be significantly valuable to an attacker).

Portability Concerns

Another problem with this is that it would change the output of the password_hash() function. Currently it uses the crypt(3) format. There are bindings for libcrypt and the like for almost every single programming language out there. Which means that passwords generated with password_hash() can be verified by other programming languages natively, and vice-versa.

So to be pedantic (or not), this type of a request should go to libcrypt instead (and then would be flushed downstream).

Encryption

You stated:

So if the column data are not formatted as expected (your API inserts also informations about the algorithm and its options), he could guess I'm using also a cipher and try some two-way decryptors that are part of the PHP standard. And this is more fast than guessing a pepper, since to find a pepper you must use brute force or violate the entire site.

Considering that AES-128 uses a 128 bit key, a proper encryption implementation would require approximately 2^127 decryption attempts to have a 50% chance of guessing the encryption key. To put that number into perspective, if you tried 1 trillion candidate keys per second, it would take you around 1 trillion times the age of the universe to brute force with a 50% chance of finding the key. If that's "more fast than guessing a pepper", the pepper isn't adding anything beyond that.

Additionally, what happens in that case if the encryption key is discovered? Not Much since the algorithm is still strong. So if the attacker gets the key, they still need to brute force every hash separately since we're using a large enough salt to be statistically unique.

About Algorithms

You mentioned:

I mean, if the hacker see my db and I have no salt columns in the user table and a VARCHAR(255) or a CHAR(60) as password column, there's an high probability that I'm using your password API.

The whole point of this is that that's by design. If your algorithm requires itself to be secret for it to be secure, it's not secure. That's one definition of security-through-obscurity. Instead bcrypt (and hence password_hash) are designed so that the salt and the algorithm can be completely public without compromising any degree of practical security.

Just use the algorithms as-designed, as opposed to trying to invent or modify them for your use. And please don't roll your own crypto.

Lucas-Malor commented 11 years ago

If your system is compromised and the secret is revealed, you've handed the attacker a database that uses small enough salts that (s)he may gain an advantage in brute forcing"

:+1: It's a good point :-)

Considering that AES-128 uses a 128 bit key, a proper encryption implementation would require approximately 2^127 decryption attempts to have a 50% chance of guessing the encryption key

Sorry for the newbie statement :-P

ircmaxell commented 11 years ago

Not at all. Having these discussions is very healthy. Don't feel bad about posting it. It's how we all learn, and it's how we all check each other's work. Without the openness and free flow of this information, we'd be in a worse spot then we are today... So thank you!