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: ircmaxell@php.net 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 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... Previous Comments: ------------------------------------------------------------------------ [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. ------------------------------------------------------------------------ [2014-01-24 00:06:21] ss23 at ss23 dot geek dot nz Description: ------------ It seems there is a limit to the input length of the password/str parameter to crypt(), however this is not documented anywhere. This has profound security implications, and all users should be aware of the issue. My preference would be a warning/notice triggered when you exceed the length, as well as documentation on this. Test script: --------------- $long = str_repeat('a', 100); var_dump(crypt($long . "1", '$2y$04$saltysaltysaltysaltytt')); var_dump(crypt($long . "2", '$2y$04$saltysaltysaltysaltytt')); var_dump(crypt($long . "12", '$2y$04$saltysaltysaltysaltytt')); Expected result: ---------------- A different hash in each case Actual result: -------------- string(60) "$2y$04$saltysaltysaltysaltyte9usMwh4/IIx0al18sl5oEFVM2Z/XJ7q" string(60) "$2y$04$saltysaltysaltysaltyte9usMwh4/IIx0al18sl5oEFVM2Z/XJ7q" string(60) "$2y$04$saltysaltysaltysaltyte9usMwh4/IIx0al18sl5oEFVM2Z/XJ7q" ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=66564&edit=1