5 ms·

Isn't this a feature..? No, I'm not serious. But if everyone wakes up for a second, checks if they are vulnerable and maybe, accidentally stumbles upon 'Why yo

Isn't this a feature..? No, I'm not serious. But if everyone wakes up for a second, checks if they are vulnerable and maybe, accidentally stumbles upon 'Why you don't want to use MD5 for your authentication' posts.. Wouldn't the world be a better place tomorrow?

dramaticus3 15y ago
Get ready for some WTF-ery Crypt takes an optional salt. If that value is an MD5 hash it is prefixed with the chars $1$ to tell the underlying crypt(3) function to use Modular Crypt Format[1]. MCF is an ad-hoc cruft because the orginal crypt() is weak. Anyway guess who did it : "let's use strlcpy/strlcat instead for these static string copies" - Rasmus I guess that's Lerdorf himself Whoever it was also didn't check the return values for error. Strlcat returns the length of the new string which might not be the same as strlen(dst) + strlen(src). "I'm not a real programmer. I throw together things until it works then I move on." - Rasmus Lerdorf Here's where he broke it : Sun Aug 7 16:10:34 2011 UTC http://svn.php.net/viewvc/php/php-src/trunk/ext/standard/php_crypt_r.c?sortdir=down&r1=313615&r2=314434&sortby=date http://svn.php.net/viewvc/php/php-src/trunk/ext/standard/php... Here's it being fixed : Fri Aug 19 22:49:18 2011 UTC http://svn.php.net/viewvc/php/php-src/trunk/ext/standard/php_crypt_r.c?sortdir=down&r1=314438&r2=315218&sortby=date http://svn.php.net/viewvc/php/php-src/trunk/ext/standard/php... [1] http://packages.python.org/passlib/modular_crypt_format.html http://packages.python.org/passlib/modular_crypt_format.html
- rll 15y ago
  If you have never broken anything you have probably never built anything.
  - dramaticus3 15y ago
    The project leader. Tests, I'm in charge, I don't test.
    - dangrossman 15y ago
      This code had a unit test, and it failed after the change as expected: http://gcov.php.net/viewer.php?version=PHP_5_3&func=tests&file=ext%2Fstandard%2Ftests%2Fstrings%2Fcrypt.phpt http://gcov.php.net/viewer.php?version=PHP_5_3&func=test...
kragen 15y ago
crypt()'s MD5 is salted and iterated and therefore perfectly adequate to use for your authentication. Just not in PHP 5.3.7. bcrypt() or scrypt() might be better, but crypt() with MD5 is perfectly adequate.
- daeken 15y ago
  Are you sure it's iterated? I just checked the manual for crypt() and it just says it's MD5 with a salt, but it could be a failure of the documentation.
  - kragen 15y ago
    Yes, see http://www.freebsd.org/cgi/cvsweb.cgi/src/lib/libcrypt/crypt-md5.c?rev=1.13.36.1;content-type=text%2Fx-cvsweb-markup http://www.freebsd.org/cgi/cvsweb.cgi/src/lib/libcrypt/crypt... where it says, "for(i = 0; i < 1000; i++)".
    - antihero 15y ago
      1000 isn't very many at all in this day and age. And because MD5 has been actually broken I'd avoid it entirely. Use scrypt.
      - nbpoole 15y ago
        Or bcrypt, which is actually implemented in PHP as part of the crypt function ;-)
      - kragen 15y ago
        This is tricky to answer. Of course I don't want to advocate using MD5, even iterated a thousand times, for both of the reasons you state. There are better alternatives. scrypt appears to be one of them, and if it stands up to analysis, it's better than PBKDF2 and bcrypt, which in turn are better than MD5-crypt. However, MD5 iterated 1000 times is still 1000 times better than MD5 iterated once (which an alarming number of codebases still use!) and the vulnerabilities that have been published in MD5 are not sufficient to speed up an attack on an MD5-crypted password file. So MD5-crypt is still vastly preferable to many alternatives, including traditional Unix crypt(), even though MD5 has been broken and DES hasn't.

dramaticus3 15y ago

Get ready for some WTF-ery Crypt takes an optional salt. If that value is an MD5 hash it is prefixed with the chars $1$ to tell the underlying crypt(3) function to use Modular Crypt Format[1]. MCF is an ad-hoc cruft because the orginal crypt() is weak. Anyway guess who did it : "let's use strlcpy/strlcat instead for these static string copies" - Rasmus I guess that's Lerdorf himself Whoever it was also didn't check the return values for error. Strlcat returns the length of the new string which might not be the same as strlen(dst) + strlen(src). "I'm not a real programmer. I throw together things until it works then I move on." - Rasmus Lerdorf Here's where he broke it : Sun Aug 7 16:10:34 2011 UTC http://svn.php.net/viewvc/php/php-src/trunk/ext/standard/php_crypt_r.c?sortdir=down&r1=313615&r2=314434&sortby=date http://svn.php.net/viewvc/php/php-src/trunk/ext/standard/php... Here's it being fixed : Fri Aug 19 22:49:18 2011 UTC http://svn.php.net/viewvc/php/php-src/trunk/ext/standard/php_crypt_r.c?sortdir=down&r1=314438&r2=315218&sortby=date http://svn.php.net/viewvc/php/php-src/trunk/ext/standard/php... [1] http://packages.python.org/passlib/modular_crypt_format.html http://packages.python.org/passlib/modular_crypt_format.html

rll 15y ago
If you have never broken anything you have probably never built anything.
- dramaticus3 15y ago
  The project leader. Tests, I'm in charge, I don't test.
  - dangrossman 15y ago
    This code had a unit test, and it failed after the change as expected: http://gcov.php.net/viewer.php?version=PHP_5_3&func=tests&file=ext%2Fstandard%2Ftests%2Fstrings%2Fcrypt.phpt http://gcov.php.net/viewer.php?version=PHP_5_3&func=test...

kragen 15y ago

crypt()'s MD5 is salted and iterated and therefore perfectly adequate to use for your authentication. Just not in PHP 5.3.7. bcrypt() or scrypt() might be better, but crypt() with MD5 is perfectly adequate.

daeken 15y ago
Are you sure it's iterated? I just checked the manual for crypt() and it just says it's MD5 with a salt, but it could be a failure of the documentation.
- kragen 15y ago
  Yes, see http://www.freebsd.org/cgi/cvsweb.cgi/src/lib/libcrypt/crypt-md5.c?rev=1.13.36.1;content-type=text%2Fx-cvsweb-markup http://www.freebsd.org/cgi/cvsweb.cgi/src/lib/libcrypt/crypt... where it says, "for(i = 0; i < 1000; i++)".
  - antihero 15y ago
    1000 isn't very many at all in this day and age. And because MD5 has been actually broken I'd avoid it entirely. Use scrypt.
    - nbpoole 15y ago
      Or bcrypt, which is actually implemented in PHP as part of the crypt function ;-)
    - kragen 15y ago
      This is tricky to answer. Of course I don't want to advocate using MD5, even iterated a thousand times, for both of the reasons you state. There are better alternatives. scrypt appears to be one of them, and if it stands up to analysis, it's better than PBKDF2 and bcrypt, which in turn are better than MD5-crypt. However, MD5 iterated 1000 times is still 1000 times better than MD5 iterated once (which an alarming number of codebases still use!) and the vulnerabilities that have been published in MD5 are not sufficient to speed up an attack on an MD5-crypted password file. So MD5-crypt is still vastly preferable to many alternatives, including traditional Unix crypt(), even though MD5 has been broken and DES hasn't.