[6.0] Removing MCrypt class

[Hackwar]

Summary of Changes

I'm currently checking our codebase for validity against PHP 7.2 and there are a few things which have come up. One of them is that the mcrypt library has been removed in PHP 7.2.0 completely.
php.net/manual/en/function.mcrypt-get-iv-size.php
This library does not exist for any version of PHP that Joomla supports and thus this class has not been possible to be used with MCrypt instead of OpenSSL. Since that means no one has been using this, we can remove this now.

This is the updated version of #38527.

[wilsonge]

It's worth noting that mcrypt isn't installed by default in PHP - but it wasn't deleted - just relegated to a pecl install https://pecl.php.net/package/mcrypt so it's still possible to use it in Joomla - it will just depend on your host setup

We will need to understand the impact this has on old 2FA users before removing this who might have had keys utilising mcrypt from a 3.x version.

[HLeithner]

@wilsonge @Hackwar I would like to run a motion to remove mcrypt in 5.0 and not in 6.0 is there any reason why we should keep it? it's unmaintained more or less since 2007.
It's hard to find out if Joomla it self uses mcrypt or what was the last version it uses it for encoding things. Also beside the removal done by hannes here, the libraries/php-encryption folder could be removed too because it holds only Crypto which is basically a wrapper around mcrypt and I can't find any usage in the CMS for it.

[wilsonge]

@wilsonge @Hackwar I would like to run a motion to remove mcrypt in 5.0 and not in 6.0 is there any reason why we should keep it? it's unmaintained more or less since 2007.

I'm unclear on whether we would have issues with old 2FA setups. If you can validate that old 2FA keys continue to work/are migrated correctly without it then I think we are all good. Else should be for v6 (or even later) given this isn't even a developer b/c break but an website user b/c break (in the sense you'd potentially loose ability to auth)

the libraries/php-encryption folder could be removed too because it holds only Crypto which is basically a wrapper around mcrypt and I can't find any usage in the CMS for it.

https://github.com/joomla/joomla-cms/blob/4.2-dev/libraries/src/Crypt/Cipher/CryptoCipher.php#L47 This entire cipher uses it - which we called the only secure implementation for a large stretch of j3.x development

[Hackwar]

Do you know in which versions we used MCrypt for 2FA?

[HLeithner]

@wilsonge @Hackwar do you found out which version of joomla is relevant? and could we test if old 2fa keys would be effected?

[HLeithner]

What's gone happen here? can we find consent? beta1 comes soon and it's time to merge a removal or it will stay till 6.0

[HLeithner]

This pull request has been automatically rebased to 5.1-dev.

[Hackwar]

Ok, I updated the PR and it would be ready to be merged into 6.0.

[HLeithner]

do we have a removal documentation pr?

[wilsonge]

Have we tested older 2fa keys. Unless we can document the workarounds we still shouldn’t be merging this

[Hackwar]

So wherever I look where we use the AES library, we are using only new AES($key, 256) and don't define an encryption format. We also didn't use something else in the past and the AES library was also never changed to change the encryption order. I think it is safe to say that we never used this. I would say this is ready to be merged.

[brianteeman]

I think it is safe to say that we never used this. I would say this is ready to be merged.

Just because core may not have used something doesnt mean that the library was not used in the ecosystem

[Hackwar]

Yes, which is why we want to remove it in the next major release, after it has been deprecated for at least a complete major release. If someone still needs this in 6.0, they have to copy the class to their own code.

[HLeithner]

ok for me, mcrypt should really not used anymore

[HLeithner]

@Hackwar can you please create a PR for Manual at https://github.com/joomla/Manual/blob/main/migrations/54-60/removed-backward-incompatibility.md

[wilsonge]

https://github.com/joomla/joomla-cms/blob/3.2.7/libraries/fof/encrypt/aes.php

When AES was introduced with fof (which we used directly - and got permission to migrate into core in 4.0) and as you can see was hardcoded to use mcrypt back then

The PR called “2fa handeling for mcrypt and openssl” #12497 added it. Google really isn’t hard.

So yes we really did use it for 2fa between 3.2.0 and 3.6.4. So users who have logged in since 3.6.4 should be migrated those from before need testing and a recovery mechanism placed in core just like we need for the old password formats.

[Hackwar]

The thing is, that this is broken since at least 4.2.0, where any switch to mcrypt was removed as far as I can see. I would even say that it is broken for a lot longer. If there is old 2fa keys like that out there, they aren't working right now either. I don't know how a migration of the potential old data could look like, especially considering that we can't assume to have the mcrypt library available. Didn't we need to convert all old keys in 4.2.0 anyway?

[laoneo]

@wilsonge when you want to support users which haven't been logged in with a release which was shipped 7 years ago, I doubt we shouldn't support that.

[wilsonge]

@wilsonge when you want to support users which haven't been logged in with a release which was shipped 7 years ago, I doubt we shouldn't support that.

I want to make sure they have a way to restore their accounts and not just making it seem like things have broken and they have to guess their way into their accounts. If that means we have to say the sites encryption means they need to reset up mfa sobeit.

FWIW i have lots of things in my 1password vault i've not logged into for much more than 7 years :)

[laoneo]

Can we detect and remove that during the update if such a mfa code is used?

[wilsonge]

Can we detect and remove that during the update if such a mfa code is used?

I'm not sure - but I think that's an acceptable solution. Force a reset of codes and show users a message like "to increase our site security we've upgraded our encryption library" or something that sounds good to them.

[laoneo]

@Hackwar can you implement the suggestion from @wilsonge ?