Doc #66564 [Com]: crypt() seems to silently discard input after a certain length

Edit report at https://bugs.php.net/bug.php?id=66564&edit=1 ID: 66564 Comment by: narf at devilix dot net Reported by: ss23 at ss23 dot geek dot nz Summary: crypt() seems to silently discard input after a certain length Status: Re-Opened Type: Documentation Problem Package: Documentation problem PHP Version: Irrelevant Assigned To: googleguy Block user comment: N Private report: N New Comment: The currently used "Caution" block to describe this does indeed make it look like other options are better than BCrypt. And it's also kind of embarassing in the case of password_hash(), because it doesn't actually support anything else at this time. IMO, a reference to a nice description of BCrypt would be a better option and if anything should be noted in a Caution block, it is DES for trimming passwords to 9 characters. Previous Comments: ------------------------------------------------------------------------ [2014-02-24 16:24:17] googleguy@php.net @ mail at michalspacek dot cz Your concerns are valid and duly noted. I will be updating this for clarity and bumping it down to a note instead of a caution in the param list. ------------------------------------------------------------------------ [2014-02-24 16:22:07] googleguy@php.net @ircmaxell You make a good point about misinterpretations in the documentation, but in retrospect people can misinterpret almost anything and we have very little control over how information is interpreted. The one thing I do know is that people rely on the manual to find documented behavior. I, myself, have to rely on it because I can't always remember how everything works, off the top of my head. So I'm a strong believer in documenting what it does and not how to use it. As for resolving your concerns over security, I propose we use your SO answer as a reference point for the security page in the manual and include a link to it from cryp/password_hash pages. It's informative and I think necessary to explain at length what's going on. ------------------------------------------------------------------------ [2014-02-23 14:00:35] mail at michalspacek dot cz "This is only a concern if are using the same salt to hash strings with this algorithm that are over 72 bytes in length, as this will result in those hashes being identical." This is slightly misleading. It sounds like all strings longer than 72 bytes will produce identical hashes, which is not true. The crucial part here is that the leading 72 bytes must be identical as the password parameter is truncated as per the first sentence of the warning. So it should be changed to something like: "This is only a concern if you are using the same salt to hash strings with this algorithm that are over 72 bytes in length and have the first 72 bytes identical, as this will result in those hashes being identical, too." ------------------------------------------------------------------------ [2014-02-22 16:22:24] ircmaxell@php.net This is a double-edged sword. On one hand, it should be documented. On the other, it shouldn't. Why do I say that it shouldn't be documented? Mainly because you shouldn't do anything about it. Pretty much any technique that you use to "get around" this will actually open up significantly worse security issues. For an in-depth analysis, check this post out: http://stackoverflow.com/questions/16594613/how-to-hash-long-passwords-72-characters-with-blowfish/16597402#16597402 So IMHO, it would be better for users to not know, so they would not be tempted to "fix it" (and make things far worse). Practically, bcrypt is sufficiently strong for *all* passwords >= 72 characters. Could it be better? Absolutely. But it's not something people should "guess at" because they think it's weak. Instead, cryptographers should create a new algorithm (already being worked on) to replace bcrypt in the future. This will not happen for several years, but it's being worked on. In the mean time, bcrypt is plenty strong enough for even the most sensitive use-cases. It can be strengthened by encrypting after hashing as well. So, considering that any attempt to "rectify" the 72 character limit will decrease security, the only way I think this should be documented would be *at most* a note with explanation that this is still secure and you shouldn't try to "work around it". Under no circumstances should a warning or notice be triggered when the length is exceeded. Also, you should **never** be using a static salt in the first place (except for testing), so in practice you should never see a duplicate hash. The use of the static salt is the security vulnerability... As far as documenting it, I've thought about this for a long time, and I firmly believe that it should stay out of the documentation. There are a few reasons for this: 1. It will scare developers: it will make it seam like bcrypt is weaker than it is, when it's actually the best option at present. This may lead them to pick other, weaker algorithms instead. 2. It will lead developers to try to "work around it" themselves: they will try pre-hashing, or raising errors if long passwords are used. These will reduce the effective security of their implementation. So the best thing to do for security is to omit it. It's a hard line, but one that I think is important. It's less omitting information, and more preventing misleading information from being distributed. My $0.02... ------------------------------------------------------------------------ [2014-01-28 13:14:53] googleguy@php.net This bug has been fixed in the documentation's XML sources. Since the online and downloadable versions of the documentation need some time to get updated, we would like to ask you to be a bit patient. Thank you for the report, and for helping us make our documentation better. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=66564 -- Edit this bug report at https://bugs.php.net/bug.php?id=66564&edit=1