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: mail at michalspacek dot cz Reported by: ss23 at ss23 dot geek dot nz Summary: crypt() seems to silently discard input after a certain length Status: Closed Type: Documentation Problem Package: Documentation problem PHP Version: Irrelevant Assigned To: googleguy Block user comment: N Private report: N New Comment: "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." Previous Comments: ------------------------------------------------------------------------ [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. ------------------------------------------------------------------------ [2014-01-28 13:10:22] googleguy@php.net Automatic comment from SVN on behalf of googleguy Revision: http://svn.php.net/viewvc/?view=revision&revision=332747 Log: Add cautionary statement about truncation for crypt and password_hash using BCRYPT. Fixes Bug #66564. This includes a cautionary statement that the CRYPT_BLOWFISH algorithm in crypt/password_hash functions will truncate the input string at a maxmimum length of 72 characters. Typically not a problem for the average use case since this is only likely used for passwords and assuming each hash has a unique salt. However, it's still a good idea to document this behavior so that users are aware of the side effect. ------------------------------------------------------------------------ [2014-01-28 10:29:09] googleguy@php.net It seems that input from the $str argument of crypt will truncate at exactly 73 characters when using CRYPT_BLOWFISH $salt. password_hash, is obviously also affected in the same way. Tested on release bracnhes of 5.3.0 through 5.6.0alpha1 with 3v4l.org and independently on my own system. Reproducible results for crypt: http://3v4l.org/3icqi Reproducible results for password_hash: http://3v4l.org/GbOo4 Reference to php-src http://lxr.php.net/xref/PHP_5_5/ext/standard/crypt_blowfish.c#819 Will update the documentation to reflect this limit more clearly in the documentation since it is defined behavior. ------------------------------------------------------------------------ [2014-01-24 00:07:51] googleguy@php.net Will assign to myself for now. ------------------------------------------------------------------------ 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