Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117109
Message-ID: <b948f796-21e7-fa9a-351b-ea9214aea93d@bastelstu.be>
Date: Mon, 21 Feb 2022 18:51:59 +0100
MIME-Version: 1.0
Content-Language: en-US
To: steve@tobtu.com, Marco Pivetta <ocramius@gmail.com>,
 Stanislav Malyshev <smalyshev@gmail.com>
Cc: PHP Internals List <internals@lists.php.net>
References: <5983302.2649742.1645319015766@email.ionos.com>
 <6238bf00-011e-35cc-d84b-4082b4f05099@gmail.com>
 <497325306.1564942.1645357444018@email.ionos.com>
 <3c6871ca-589d-6812-800c-a3b9ad6bb575@bastelstu.be>
 <CADyq6sLWRfD2DJwRNWmX=s7rbA2Rs+Agu9tsW9mLQbCeJ27vwA@mail.gmail.com>
 <40015164-ac0c-336d-c7d6-c4766d6caff8@gmail.com>
 <CADyq6s+e_Jkwg1UoGS_e23=6FciRB2-d8q_NxkREMt45f=vs8g@mail.gmail.com>
 <b52c5f52-3295-7792-1897-c170bb132331@bastelstu.be>
 <1846031176.2904638.1645461835527@email.ionos.com>
In-Reply-To: <1846031176.2904638.1645461835527@email.ionos.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] RFC proposal to deprecate crypt()
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=c3=bcsterhus?=)

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:

On 2/21/22 17:43, steve@tobtu.com wrote:
> 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