Fixed support for PHP 7.2 so that Argon2 works too

[Toflar]

With the release of PHP 7.2, there's a new password hashing algorithm Argon2 available. Our Encryption::test() currently does not recognize that because it's built the wrong way around. It should not check for the ones it knows are password api compatible but check for the ones it knows are not. That ensures that we do not have to extend that check every time a new algorithm is introduced. The password api does that itself internally.
I also removed all the legacy code which is handled way better by the password-compat polyfill so we know it behaves the same as does the core of php.

I also removed the bcrypt cost configuration. PHP itself defaults to a sane cost, I see no reason why we should not go with what the responsible crypto experts recommend.

This should also go to Contao 4.4 but without the polyfill as we require a php version >= 5.5 where the password api was introduced in php.

[Toflar]

TODO:

• Use password_get_info()
• Make sure the password fields are varchar(255)

[Toflar]

Ping reviewers, PR updated.

[Toflar]

There is no constant for unknown password algorithms :(

[leofeyer]

Why not $info['algo'] > 0?

[Toflar]

It's the same? I don't really care :D

[ausi]

Should we add a notice somewhere that \Config::get('bcryptCost') is no longer used?

[Toflar]

Should we add a notice somewhere that \Config::get('bcryptCost') is no longer used?

I thought about that but to be honest, I think 99.9% of all users never changed that value. And adding a deprecation notice would mean we still need to support it but only for bcrypt and not the other algorithms (and it has to be done everywhere I now use password_hash() directly) so I thought it's not worth it and over-complicates things.

[ausi]

K, agreed 🙂

[leofeyer]

In order to being able to deprecate the Encryption::test() method, we could use

$info = password_get_info($this->password);

if ($info['algo'] > 0)
{

here. WDYT?

[Toflar]

Ah yes, wie could do that indeed. Then we can probably deprecate the whole Encryption class, can't we?

[leofeyer]

Nope, just the hashing part. Encrypting and decrypting remains untouched.

[Toflar]

Talking about Contao 4.

[leofeyer]

We do not (yet) have an alternative for encrypting and decrypting data in Contao 4.

[Toflar]

Of course we have, it's already documented? What are you talking about exactly?

[leofeyer]

Ah, I see. The methods are deprecated without a concrete alternative. 😄

[leofeyer]

Then yes, we can deprecate the whole class.

[Toflar]

Using Encryption::encrypt() has been deprecated and will no longer work in Contao 5.0. Use a third-party library such as OpenSSL or phpseclib instead. is a pretty well described alternative 😄

So everything ready here or what do you need?

[leofeyer]

Implemented in Contao 4.4 in contao/core-bundle@815b384.

While porting the changes, I noticed that we are not using password_needs_rehash() yet. Since we are using PASSWORD_DEFAULT, it is likely that the algorithm will change in the future and then we should ask the users to re-enter their passwords, shouldn't we?

[Toflar]

You're right, we should check and update the hash in those cases.

[leofeyer]

Added in Contao 4.4 in contao/core-bundle@1e2096d.

[leofeyer]

Backported in 7d1f0f1.

[fritzmg]

This backport increased the minimum PHP version for Contao 3.5 from PHP 5.4.0 to PHP 5.5.0 because the password_verify function is only available in PHP >=5.5.0.

See also https://community.contao.org/de/showthread.php?69433-Nach-Update-Fatal-error-Call-to-undefined-function-Contao-password_verify()

[Toflar]

We might add https://github.com/ircmaxell/password_compat.

[leofeyer]

My bad, I forgot to update the composer.json accordingly. 😢

[fritzmg]

If the new PHP minimum version stays, then the README, docs.contao.org and the Contao Check need to be updated too. This should may be also be announced somewhere.

Ah you meant to add ircmaxell/password_compat :)

[leofeyer]

The issue has been fixed in Contao 3.5.33 (see 60893ac).