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().
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()andpassword_needs_rehash()already supports
hashes created withcrypt(), the only thing needed to do is removecrypt().--
To unsubscribe, visit: https://www.php.net/unsub.php
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 vasilii.b.shpilchin@gmail.com 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.
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()andpassword_needs_rehash()already supports
hashes created withcrypt(), the only thing needed to do is removecrypt().--
To unsubscribe, visit: https://www.php.net/unsub.php
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()andpassword_needs_rehash()already supports hashes created withcrypt(), the only thing needed to do is removecrypt().
Removing it would cause serious BC issues with no practical gain.
--
Stas Malyshev
smalyshev@gmail.com
On 02/20/2022 1:10 AM Stanislav Malyshev smalyshev@gmail.com wrote:
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.
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.
Since
password_verify()andpassword_needs_rehash()already supports hashes created withcrypt(), the only thing needed to do is removecrypt().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.
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.Since
password_verify()andpassword_needs_rehash()already supports hashes created withcrypt(), the only thing needed to do is removecrypt().Removing it would cause serious BC issues with no practical gain.
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 bycrypt(). The whole point of deprecating and finally removingcrypt()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 andpassword_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
Hi Steve,
If that's the case, you may not know that
password_verify()can verify
all password hashes created bycrypt(). The whole point of deprecating and
finally removingcrypt()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.
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
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
On Mon, Feb 21, 2022 at 10:06 AM Stanislav Malyshev smalyshev@gmail.com
wrote:
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)?
Probably to be removed in 9.0 or 10.0? Yes, it should be removed at
some point.
Marco Pivetta
Hi Marco
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.0or10.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
Hi Marco
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.0or10.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 removingcrypt()results in an
actual loss of functionality.
...yes? That's the point?
Marco Pivetta
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.0or10.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 removingcrypt()results in an
actual loss of functionality....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
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.0or10.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 removingcrypt()results in an
actual loss of functionality....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: Ifcrypt()is what you need, thencrypt()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
possiblysha1()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 ofmd5()and then not havingmd5()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
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
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.0or10.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 removingcrypt()results in an
actual loss of functionality....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: Ifcrypt()is what you need, thencrypt()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 usingcrypt(), 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
possiblysha1()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 ofmd5()and then not havingmd5()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
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
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]
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 .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
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 tim@bastelstu.be wrote:
Hi Marco
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.0or10.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 removingcrypt()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 forcrypt()should more prominently
point topassword_hash()as the better alternative. But ifcrypt()'s
features are what you need, then alternatives tocrypt()(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
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 divinity76@gmail.com
wrote:
script modifying a linux system's /etc/passwd / /etc/shadow using
crypt()
becausepassword_hash()couldn't create passwd/shadow-compatible hashes
whilecrypt()could
If
crypt()is removed, you can still usepassword_verify()to verify all the password hashes created bycrypt(). 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]
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 usepassword_verify()to verify all the password hashes created bycrypt().
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_DESwhich 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