# RFC proposal to deprecate crypt() - Thread: https://externals.io/message/117070 - Original archive: https://news-web.php.net/php.internals/117070 - Started by steve@tobtu.com on 2022-02-20T01:03:35+00:00 - Messages: 22 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · RFCs at https://externals.io/rfc.md · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #117070 — steve@tobtu.com — 2022-02-20T01:03:35+00:00 URL: https://externals.io/message/117070#117070 crypt() should be deprecate because it can be used to create bad password hashes: * descrypt: 12 bits of salt is too small and it's ~100x faster to crack than md5crypt. Which itself is too fast for password crackers (see CVE-2012-3287). * Extended DES: 24 bits of salt is too small. * md5crypt is too fast for password crackers (see CVE-2012-3287). * sha256crypt and sha512crypt are dangerous (see CVE-2016-20013). Since `password_verify()` and `password_needs_rehash()` already supports hashes created with `crypt()`, the only thing needed to do is remove `crypt()`. --- ## #117071 — Vasilii Shpilchin — 2022-02-20T01:16:04+00:00 URL: https://externals.io/message/117070#117071 · In reply to: #117070 Hashes are not for passwords only. For instance, hashes usually in use in sharding and to calculate checksums. I suggest to add a warning to the documentation, something like: if you need to hash a password, use `password_hash()`. Best regards, Vasilii. > `crypt()` should be deprecate because it can be used to create bad password > hashes: > > * descrypt: 12 bits of salt is too small and it's ~100x faster to crack > than md5crypt. Which itself is too fast for password crackers (see > CVE-2012-3287). > * Extended DES: 24 bits of salt is too small. > * md5crypt is too fast for password crackers (see CVE-2012-3287). > * sha256crypt and sha512crypt are dangerous (see CVE-2016-20013). > > Since `password_verify()` and `password_needs_rehash()` already supports > hashes created with `crypt()`, the only thing needed to do is remove `crypt()`. > > -- > > To unsubscribe, visit: https://www.php.net/unsub.php --- ## #117072 — steve@tobtu.com — 2022-02-20T01:57:47+00:00 URL: https://externals.io/message/117070#117072 · In reply to: #117071 hash() is for cryptographic hashes and checksums. `crypt()` only supports password hashing algorithms which should not be used as a cryptographic hash or checksum because they are purposefully slow. > On 02/19/2022 7:16 PM Vasilii Shpilchin wrote: > > > Hashes are not for passwords only. For instance, hashes usually in use in > sharding and to calculate checksums. I suggest to add a warning to the > documentation, something like: if you need to hash a password, use > `password_hash()`. > > Best regards, > Vasilii. > > > > > […] --- ## #117073 — Stanislav Malyshev — 2022-02-20T07:10:57+00:00 URL: https://externals.io/message/117070#117073 · In reply to: #117070 Hi! > `crypt()` should be deprecate because it can be used to create bad password hashes: I don't think it's a good reason for deprecating functions. A lot of functions, if used incorrectly, could produce bad results, it's not the reason to not use them correctly. > Since `password_verify()` and `password_needs_rehash()` already supports hashes created with `crypt()`, the only thing needed to do is remove `crypt()`. Removing it would cause serious BC issues with no practical gain. -- Stas Malyshev smalyshev@gmail.com --- ## #117074 — steve@tobtu.com — 2022-02-20T11:44:04+00:00 URL: https://externals.io/message/117070#117074 · In reply to: #117073 > On 02/20/2022 1:10 AM Stanislav Malyshev wrote: > > > Hi! > > > > […] > > I don't think it's a good reason for deprecating functions. A lot of > functions, if used incorrectly, could produce bad results, it's not the > reason to not use them correctly. > Sorry, I really meant to say it can *only* create bad password hashes... and bcrypt which `password_hash()` is the preferred function to create a bcrypt hash. `crypt()` should be deprecated, unless there's a reason to let users write: $hash = crypt($password, "$2y$" . str_pad($cost, 2, "0", STR_PAD_LEFT) . strtr(base64_encode(random_bytes(16)), '+', '.')); // hi from 9 years ago https://www.php.net/manual/en/function.crypt.php#111086 (oof... ignore the minor errors and the update with `random_bytes()` because mcrypt was deprecated then removed) vs $hash = password_hash($password, PASSWORD_BCRYPT); Note the `crypt()` documentation: "password_hash() uses a strong hash, generates a strong salt, and applies proper rounds automatically. `password_hash()` is a simple `crypt()` wrapper and [password_verify() is] compatible with existing password hashes. Use of `password_hash()` is encouraged." That is the exact reason why mcrypt was deprecated: hard to use correctly and most uses of it were broken. Also ~99% of implementations of `crypt()` that use sha256crypt and/or sha512crypt password hashing algorithms are vulnerable to a long password DoS attack. Since they don't know they need to limit the password length because the runtime is O(pwLen^2+pwLen*rounds). Note a 14000 byte password takes ~1 second, 28000 is ~3 seconds, 56000 is ~11 seconds (results may vary depending on CPU and sha256crypt vs sha512crypt). Ignoring bcrypt: sha256crypt and sha512crypt are the only other algos that are not horrible and they are still very bad. > > […] > > Removing it would cause serious BC issues with no practical gain. What are "BC issues"?... Backward compatibility issues? If that's the case, you may not know that `password_verify()` can verify all password hashes created by `crypt()`. The whole point of deprecating and finally removing `crypt()` is that users can no longer create bad password hashes. This is a massive gain in security. It's like removing mcrypt which removed people's ability to ECB encrypt data. Sure there are very limited uses that are secure but 99.9999% are crypto101 errors. Also the *ONLY* non-broken password hash function that `crypt()` can do is bcrypt and `password_hash()`/password_verify() is a better alternative for bcrypt hashes. Basically `crypt()` serves no purpose besides as a legacy footgun. --- ## #117075 — tim@bastelstu.be — 2022-02-20T13:17:14+00:00 URL: https://externals.io/message/117070#117075 · In reply to: #117074 Hi Steve, > Also ~99% of implementations of `crypt()` that use sha256crypt and/or sha512crypt password hashing algorithms are vulnerable to a long password DoS attack. Since they don't know they need to limit the password length because the runtime is O(pwLen^2+pwLen*rounds). Note a 14000 byte password takes ~1 second, 28000 is ~3 seconds, 56000 is ~11 seconds (results may vary depending on CPU and sha256crypt vs sha512crypt). Ignoring bcrypt: sha256crypt and sha512crypt are the only other algos that are not horrible and they are still very bad. > > > […] > > What are "BC issues"?... Backward compatibility issues? Yes. > If that's the case, you may not know that `password_verify()` can verify all password hashes created by `crypt()`. The whole point of deprecating and finally removing `crypt()` is that users can no longer create bad password hashes. This is a massive gain in security. It's like removing mcrypt which removed people's ability to ECB encrypt data. Sure there are very limited uses that are secure but 99.9999% are crypto101 errors. I am maintaining a software that supports a *legacy* password hashing algorithm that, for reasons that are not relevant to this discussion, performs two passes of BCrypt hashing with the *same* salt: crypt(crypt('password', '$2a$08$salt'), '$2a$08$salt'); This is not something that can be replicated with password_hash and password_verify, because password_hash does not accept an explicit salt starting with PHP 8.0 and password_verify does not know about this double hashing. Even though this hashing algorithm is legacy, we need to maintain compatibility with that for the foreseeable future to be able to upgrade users into the current (password_hash() based) hashes, without them needing to reset their passwords. > Also the *ONLY* non-broken password hash function that `crypt()` can do is bcrypt and `password_hash()`/password_verify() is a better alternative for bcrypt hashes. > > Basically `crypt()` serves no purpose besides as a legacy footgun. > I disagree. Apart from my real world use case above, there might also be other real world use-cases where interoperability with other software is required. As an example: Older versions of Dovecot (< 2.3) do not necessarily support BCrypt hashes, but they support SHA512-CRYPT. The existence of the `crypt()` function allows a PHP-based management portal to generate password hashes that are understood by those Dovecot versions. Best regards Tim Düsterhus --- ## #117076 — Marco Pivetta — 2022-02-20T13:24:16+00:00 URL: https://externals.io/message/117070#117076 · In reply to: #117075 > Hi Steve, > > > > > > […] > all password hashes created by `crypt()`. The whole point of deprecating and > finally removing `crypt()` is that users can no longer create bad password > hashes. This is a massive gain in security. It's like removing mcrypt which > removed people's ability to ECB encrypt data. Sure there are very limited > uses that are secure but 99.9999% are crypto101 errors. > > I am maintaining a software that supports a *legacy* password hashing > algorithm that, for reasons that are not relevant to this discussion, > performs two passes of BCrypt hashing with the *same* salt: > > crypt(crypt('password', '$2a$08$salt'), '$2a$08$salt'); > > This is not something that can be replicated with password_hash and > password_verify, because password_hash does not accept an explicit salt > starting with PHP 8.0 and password_verify does not know about this > double hashing. > > Even though this hashing algorithm is legacy, we need to maintain > compatibility with that for the foreseeable future to be able to upgrade > users into the current (password_hash() based) hashes, without them > needing to reset their passwords. > The RFC is about deprecation, not removal. Set a deadline for your customer (few years?): * Enable rehashing (you already do) * Deprecate the old algo internally * When the deadline is past, drop the old algo: users with an old hash will have to reset their password Note that PHP 9 is still far away, so you have time to rehash. --- ## #117077 — Kamil Tekiela — 2022-02-20T14:35:00+00:00 URL: https://externals.io/message/117070#117077 · In reply to: #117076 I think it's a great idea to deprecate `crypt()`. I wouldn't want anyone to use it in a new code. For legacy applications, we are giving them enough time to upgrade their password storing policy. Also, it's not like we are removing support for hashing in general from PHP, we are just deprecating a function that should not be used for password hashing. There's a suitable replacement and it's also possible to create a shim for the `crypt()` function if one really needs it. I also recommend reading this article https://www.michalspacek.com/upgrading-existing-password-hashes --- ## #117086 — Stanislav Malyshev — 2022-02-21T09:06:15+00:00 URL: https://externals.io/message/117070#117086 · In reply to: #117076 Hi! > The RFC is about deprecation, not removal. If it's not going to be removed, what's the point of annoying people with deprecation warnings (that they would patch out/silence anyway)? If we want to document which functions are recommended to be used in which case, we have the manual for that. I don't think deprecation should be used for such things. -- Stas Malyshev smalyshev@gmail.com --- ## #117088 — Marco Pivetta — 2022-02-21T09:15:29+00:00 URL: https://externals.io/message/117070#117088 · In reply to: #117086 On Mon, Feb 21, 2022 at 10:06 AM Stanislav Malyshev wrote: > Hi! > > > […] > > If it's not going to be removed, what's the point of annoying people > with deprecation warnings (that they would patch out/silence anyway)? > Probably to be removed in `9.0` or `10.0`? Yes, it should be removed at some point. Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.io/ --- ## #117089 — tim@bastelstu.be — 2022-02-21T10:10:56+00:00 URL: https://externals.io/message/117070#117089 · In reply to: #117088 Hi Marco > > […] > > Probably to be removed in `9.0` or `10.0`? Yes, it should be removed at > some point. In contrast to other deprecations (e.g. the utf8_encode/decode currently discussed), deprecating and ultimately removing `crypt()` results in an actual loss of functionality. Even if we leave out that home-grown nested BCrypt hashing out of the picture, `crypt()` allows one to interoperate with non-PHP-software that does not support BCrypt, but supports the SHA-X variants. I already mentioned Dovecot as an example, but BCrypt support in glibc in general is something that was added only somewhat recently (and I'm not even sure if that's only for Debian-based systems or generally available [1]). Yes, users should just use `password_hash()` if they need to hash passwords. Yes, the documentation for `crypt()` should more prominently point to `password_hash()` as the better alternative. But if `crypt()`'s features are what you need, then alternatives to `crypt()` (e.g. a userland implementation or FFI) certainly are going to be even worse. Best regards Tim Düsterhus [1] https://sourceware.org/bugzilla/show_bug.cgi?id=16814 --- ## #117091 — Marco Pivetta — 2022-02-21T11:12:08+00:00 URL: https://externals.io/message/117070#117091 · In reply to: #117089 > Hi Marco > > > > […] > > In contrast to other deprecations (e.g. the utf8_encode/decode currently > discussed), deprecating and ultimately removing `crypt()` results in an > actual loss of functionality. > ...yes? That's the point? Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.io/ --- ## #117092 — tim@bastelstu.be — 2022-02-21T11:39:34+00:00 URL: https://externals.io/message/117070#117092 · In reply to: #117091 Hi > > […] > > ...yes? That's the point? > I understand that that's the point. However I agree with Stas that this would be a BC break with no practical gain and I articulated the reasons why I believe that: If `crypt()` is what you need, then `crypt()` is what you need and being told that your use-case is invalid is not helping you. The function already exists and I assume that it does not require (relevant) maintenance effort for PHP maintainers keeping it. With the same arguments one could deprecate and remove `md5()` (and possibly `sha1()` as well). It should not be used for passwords, it should not be used for signatures and any new use should require *careful* review. Nonetheless there are cases where you still need an implementation of `md5()` and then not having `md5()` is an issue. If someone proposed the removal of `md5()` I'd disagree the same. Best regards Tim Düsterhus --- ## #117093 — Marco Pivetta — 2022-02-21T11:49:01+00:00 URL: https://externals.io/message/117070#117093 · In reply to: #117092 > Hi > > > > […] > > I understand that that's the point. However I agree with Stas that this > would be a BC break with no practical gain and I articulated the reasons > why I believe that: If `crypt()` is what you need, then `crypt()` is what > you need and being told that your use-case is invalid is not helping > you. The function already exists and I assume that it does not require > (relevant) maintenance effort for PHP maintainers keeping it. > Not a maintenance effort exercise, but a user education exercise. Users need to stop using `crypt()`, because it just has to stop, period. They have a period of time to migrate away from it, and then they will get a hard crash when it's gone-gone-gone, which is **OK**. In addition to that, end-users of PHP-SRC are not paying customers to the php foundation, and their systems will keep rotting until they will finally have to touch them: it would be a different story if there was a LTS contract in place, but that's not how OSS works. They can also polyfill it with whatever crazy and broken contraption works for them, as long as they take the irresponsible security decision upon themselves. Calibrated code and feature removal is part of the software maintenance process, and it has more effects than just code size increase. > With the same arguments one could deprecate and remove `md5()` (and > possibly `sha1()` as well). It should not be used for passwords, it should > not be used for signatures and any new use should require *careful* > review. Nonetheless there are cases where you still need an > implementation of `md5()` and then not having `md5()` is an issue. > > If someone proposed the removal of `md5()` I'd disagree the same. > As mentioned elsewhere in the mail thread, `crypt()` is not designed for fast hashing, and is in fact slow by design. MD5 and SHA1 still have a place, compared to that, as they are not designed solely for password hashing. This is part of "calibrated code and feature removal" from above. Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.io/ --- ## #117094 — Hans Henrik Bergan — 2022-02-21T12:03:16+00:00 URL: https://externals.io/message/117070#117094 · In reply to: #117093 fwiw i recall a real-world script modifying a linux system's /etc/passwd / /etc/shadow using `crypt()` because `password_hash()` couldn't create passwd/shadow-compatible hashes while `crypt()` could > > > > […] > anyway)? > > […] > at > > […] > currently > > […] > > Not a maintenance effort exercise, but a user education exercise. > Users need to stop using `crypt()`, because it just has to stop, period. > They have a period of time to migrate away from it, and then they will get > a hard crash when it's gone-gone-gone, which is **OK**. > > In addition to that, end-users of PHP-SRC are not paying customers to the > php foundation, and their systems will keep rotting until they will finally > have to touch them: it would be a different story if there was a LTS > contract in place, but that's not how OSS works. > > They can also polyfill it with whatever crazy and broken contraption works > for them, as long as they take the irresponsible security decision upon > themselves. > > Calibrated code and feature removal is part of the software maintenance > process, and it has more effects than just code size increase. > > > > > […] > > As mentioned elsewhere in the mail thread, `crypt()` is not designed for > fast hashing, and is in fact slow by design. > MD5 and SHA1 still have a place, compared to that, as they are not designed > solely for password hashing. > > This is part of "calibrated code and feature removal" from above. > > Marco Pivetta > > http://twitter.com/Ocramius > > http://ocramius.github.io/ --- ## #117096 — Jakub Zelenka — 2022-02-21T12:28:28+00:00 URL: https://externals.io/message/117070#117096 · In reply to: #117093 > > As mentioned elsewhere in the mail thread, `crypt()` is not designed for > fast hashing, and is in fact slow by design. > What do you mean by slow? Are you aware that password_hash and password_verify for bcrypt are just wrappers around crypt? Just to note that by removing that, we would still need to keep php_crypt in the core and would just get rid of couple of lines for the actual function definitions and arguments so there's absolutely no benefit in terms of maintanance. We have got plenty of functions that are quite low level but offer some additional functionality that can be used by specialised libraries / apps. I think that it's much better to educate users by documentation rather than removing the actual functionality that has got some valid users. We can see that there are some valid use case for using crypt directly and we can also see that it's offered by other languages as well - e.g. Python: https://docs.python.org/3/library/crypt.html . Jakub --- ## #117098 — Rowan Tommins — 2022-02-21T12:45:04+00:00 URL: https://externals.io/message/117070#117098 · In reply to: #117096 > We can see > that there are some valid use case for using crypt directly and we can also > see that it's offered by other languages as well - e.g. Python: > https://docs.python.org/3/library/crypt.html . I think this is quite an important point: if `crypt()` worked with some wacky homebrew format that only PHP understood, then planning to remove it would make sense. But since we don't have control over applications *outside* PHP, providing the low-level function that interoperates with them, and is hard to implement in userland, seems useful. If updating the manual isn't enough, we could make more aggressive changes short of removal, such as renaming "CRYPT_MD5" to "CRYPT_INSECURE_MD5" and so on. Incidentally, does the function now support Argon hashes, or are they implemented separately in the password functions? Regards, -- Rowan Tommins [IMSoP] --- ## #117099 — Christoph M. Becker — 2022-02-21T12:56:34+00:00 URL: https://externals.io/message/117070#117099 · In reply to: #117098 > > > > […] > > Incidentally, does the function now support Argon hashes, or are they > implemented separately in the password functions? Argon hashing is provided by libargon or libsodium; `crypt()` does not support these directly. -- Christoph M. Becker --- ## #117105 — steve@tobtu.com — 2022-02-21T16:43:55+00:00 URL: https://externals.io/message/117070#117105 · In reply to: #117089 If `crypt()` is removed, you can still use `password_verify()` to verify all the password hashes created by `crypt()`. The only thing you lose is creating those bad password hashes. Which can be done in userland because most people aren't changing their passwords daily. So it will run that slow userland code infrequently. This will not help the nested bcrypt example. Well besides the last bcrypt call can be `password_verify()`: password_verify(crypt($password, $hash), $hash); > On 02/21/2022 4:10 AM Tim Düsterhus wrote: > > > Hi Marco > > > > […] > > In contrast to other deprecations (e.g. the utf8_encode/decode currently > discussed), deprecating and ultimately removing `crypt()` results in an > actual loss of functionality. > > Even if we leave out that home-grown nested BCrypt hashing out of the > picture, `crypt()` allows one to interoperate with non-PHP-software that > does not support BCrypt, but supports the SHA-X variants. I already > mentioned Dovecot as an example, but BCrypt support in glibc in general > is something that was added only somewhat recently (and I'm not even > sure if that's only for Debian-based systems or generally available [1]). > > Yes, users should just use `password_hash()` if they need to hash > passwords. Yes, the documentation for `crypt()` should more prominently > point to `password_hash()` as the better alternative. But if `crypt()`'s > features are what you need, then alternatives to `crypt()` (e.g. a > userland implementation or FFI) certainly are going to be even worse. > > Best regards > Tim Düsterhus > > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=16814 > > -- > > To unsubscribe, visit: https://www.php.net/unsub.php --- ## #117107 — Craig Francis — 2022-02-21T17:35:33+00:00 URL: https://externals.io/message/117070#117107 · In reply to: #117105 > If `crypt()` is removed [...] The only thing you lose is creating those bad > password hashes. That's not exactly fair, as noted by Tim, `crypt()` can be used for other software (e.g. Dovecot); and by Hans for modifying `/etc/shadow`. While I would warn most developers away from using `crypt()`, because it is dangerous, it can still be useful (dare I say it, md5, terrible idea, but sometimes you need it when integrating with other systems). Craig > `crypt()` allows one to interoperate with non-PHP-software that does not > support BCrypt, but supports the SHA-X variants. I already mentioned > Dovecot as an example. On Mon, 21 Feb 2022 at 12:04, Hans Henrik Bergan wrote: > script modifying a linux system's /etc/passwd / /etc/shadow using `crypt()` > because `password_hash()` couldn't create passwd/shadow-compatible hashes > while `crypt()` could --- ## #117108 — Rowan Tommins — 2022-02-21T17:37:25+00:00 URL: https://externals.io/message/117070#117108 · In reply to: #117105 > If `crypt()` is removed, you can still use `password_verify()` to verify all the password hashes created by `crypt()`. The only thing you lose is creating those bad password hashes. Which can be done in userland because most people aren't changing their passwords daily. So it will run that slow userland code infrequently. What "slow userland code"? Is there an implementation of the legacy crypt hashing function in pure PHP out there somewhere? I certainly wouldn't be confident writing one. Regards, -- Rowan Tommins [IMSoP] --- ## #117109 — tim@bastelstu.be — 2022-02-21T17:51:59+00:00 URL: https://externals.io/message/117070#117109 · In reply to: #117105 Hi Steve, I wanted to stay away from this thread, as I believe I've made my opinion clear and there's not much more signal for me to add, but as you've replied to me directly: > If `crypt()` is removed, you can still use `password_verify()` to verify all the password hashes created by `crypt()`. Yes, this is true. It also appears to be tested by ext/standard/tests/password/password_verify.phpt for the DES style hashes. However it does not appear to be documented at `password_verify()` (https://www.php.net/manual/en/function.password-verify.php), so that likely should be adjusted first. > The only thing you lose is creating those bad password hashes. Which can be done in userland because most people aren't changing their passwords daily. So it will run that slow userland code infrequently. This will not help the nested bcrypt example. Well besides the last bcrypt call can be `password_verify()`: > Please ignore that "nested BCrypt" example for a moment, that wasn't my main point, it's a legacy implementation (for good reason), any new hashes in that software are regular BCrypt hashes based on `password_hash()` and any old passwords are rehashed on login. If that legacy hashing breaks, then so-be-it [1]. The larger point I've attempted to make is that `crypt()` is the lowest common denominator for interoperability with other programming languages and software (specifically libc), as also acknowledged by the Rowan and Jakub. Yes, in the ideal world everyone would use just BCrypt (or Argon2 [2]), but unfortunately that isn't a reality. Removing the functionality from PHP is not likely to achieve this goal faster, as users will either not upgrade PHP or defer to a userland implementation. While the latter certainly is an option, implementing a hashing algorithm definitely is something that is highly non-trivial to do in userland. If the deprecation of `crypt()` leads to a dozen (semi-)broken and insecure userland implementations to keep compatibility with whatever software that uses `crypt()` then I consider that a net-negative as well. So: - Improving the docs page for `crypt()`: Yes please. - Deprecating `CRYPT_STD_DES` which is easy to accidentally use by not passing a valid algorithm indicator: That's also a yes. - Rejecting long inputs (e.g. > 512 Bytes) for CRYPT_SHA256 / CRYPT_SHA512: Sure, that also makes sense. - Completely removing support for `crypt()`: I'd rather not, because see above. Best regards Tim Düsterhus [1] Even if I'd prefer it didn't as users sometimes come back to their account after several years. They still know their passwords, but in many cases their email address no longer is valid, thus they can't reset their password. [2] Which incidentally might not be more secure that BCrypt for common configurations: https://twitter.com/TerahashCorp/status/1155129705034653698