Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:117074
Date: Sun, 20 Feb 2022 05:44:04 -0600 (CST)
To: Stanislav Malyshev <smalyshev@gmail.com>, internals@lists.php.net
Message-ID: <497325306.1564942.1645357444018@email.ionos.com>
In-Reply-To: <6238bf00-011e-35cc-d84b-4082b4f05099@gmail.com>
References: <5983302.2649742.1645319015766@email.ionos.com>
 <6238bf00-011e-35cc-d84b-4082b4f05099@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Importance: Normal
Subject: Re: [PHP-DEV] RFC proposal to deprecate crypt()
From: steve@tobtu.com

> On 02/20/2022 1:10 AM Stanislav Malyshev <smalyshev@gmail.com> wrote:
> 
>  
> Hi!
> 
> On 2/19/22 6:03 PM, steve@tobtu.com wrote:
> > 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() 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.

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.