I have tested this item ✅ successfully on c696557

Works good to me. I could reproduce the issue, and it got fixed by the chagnes here as well as still works on PHP 7 :)

If php8 hard fails on mcrypt can we add a php version check to the mcrypt isSupported method to return false on php8 so in case OpenSSL doesn’t work for whatever reason we don’t fatal but cleanly handle the lack of support?

If OpenSSL is not available you get a soft fail. The check returns false and no encryption is used.

The only problem you might ever have is if Two Factor Authentication was already enabled with encryption and now you don't have it because you moved your site, changed the PHP version but forgot to enable OpenSSL etc.

I personally believe that the PHP OpenSSL extension should be a requirement for Joomla 4. Any sensitive information stored in the DB should always be encrypted with a key derived from $secret (which is stored in the filesystem). If Joomla does that in the core it is very much simpler to a. train 3PDs to encrypt sensitive info even if it's just monkey-copying the core and b. allow 3PDs to always encrypt sensitive info instead of having to offer a cop-out like we do today. Something something something Joomla4Security ;)

I’m code reviewing from phone (flying back from holiday later today) so might be really stupid but isn’t Phil’s change if OpenSSL supported check returns false - then it tried to fallback to mcrypt?

Message heard on the OpenSSL requirement. I’m missing the production meeting today as I’m flying but will try and ensure it gets added to agenda for discussion in two weeks. Technically as it breaks b/c i guess it’s for the 4.1/4.2 release or something so we can scream in advance about it

Besides adding an openssql requirement later i would still go and merge this into 3.10.2 to make sure the issue on correctly setup servers is gone for now.

@nikosdion
Off Topic, please see https://forum.joomla.org/viewtopic.php?f=54&t=860989&p=3638915#p3638915

@infograf768 It's indeed off–topic. I have not been the Greek language maintainer for nearly 3 years. Contact the people at CrowdIn. If they need my input they know where to find me (they can definitely DM me on Twitter, Facebook or here one GitHub).

@nikosdion
The file concerned is the English en-GB original file in your pack, not any translation. The double quotes are NOT escaped
example:
COM_AKEEBABACKUP_CLI_PROFILE_CREATE_OPT_DESCRIPTION="Description for the new backup profile. Default: "New backup profile"."
Sorry for being off topic again. Don't know how to contact you elsewhere.

@infograf768 Please use https://www.akeeba.com/contact-us.html

@nikosdion Please could you test this change you suggested and then mark it as tested when you get a moment, that would be the second test and enough to get it RTC. Thanks.

Then its just left with @wilsonge then with:

ensure it gets added to agenda for discussion in two weeks. Technically as it breaks b/c i guess it’s for the 4.1/4.2 release or something so we can scream in advance about it

@PhilETaylor I am reluctant to provide a successful test since I reported this issue and my use case is a two part issue which requires a change in my own software as well. Inasmuch Joomla TFA is concerned I can confirm that after the PR it does work on PHP 8 just fine but since this was not my original reproduction of the issue I don't consider it a valid test on my part.

Okay, I did a lot more testing. I have tested this PR UNSUCCESSFULLY.

The code in FOF is just one instance where Mcrypt is given priority. Trying to log into the site with Two Factor Authentication enabled on PHP 8 yielded the same error with the following backtrace (partial):

Undefined constant "MCRYPT_RIJNDAEL_128" 
....../libraries/fof/encrypt/aes.php:60

Call stack
--
# | Function | Location
1 | () | JROOT/libraries/fof/encrypt/aes.php:60
2 | FOFEncryptAes->__construct() | JROOT/administrator/components/com_users/models/user.php:1003
3 | UsersModelUser->getOtpConfig() | JROOT/plugins/authentication/joomla/joomla.php:113

The problem is that the user helper sees that the OTP information is encrypted and tries to decrypt it with either OpenSSL or mCrypt. Since creating the mCrypt adapter triggers the PHP 8 fatal error I cannot log into the site.

For the same reason I cannot edit the user in the backend either. I get the Undefined constant "MCRYPT_RIJNDAEL_128" error.

The only way to fix that is delete the otep / otpKey values from the database. This lets me edit the user AND since Joomla 3.6.4 and later saves these settings unencrypted in the database it finally allows me to log into the site.

So, the problem is deeper and only apparent if you have a user account set up with TFA before Joomla 3.6.3. That would explain why I haven't seen it in the wild the past year that PHP 8 has been generally available. It would only hit you if you have a site which has been continuously upgraded since before Joomla 3.6.4 and you had set up TFA on the user account before Joomla 3.6.4 and never changed it again.

The question is, should we even bother with it?

The other question is, why the heck does Joomla 4 still ship the mCrypt support files when it's PHP 7.2+ only and mCrypt is not available on PHP 7.0 and later? I see someone reviewed the code and made comments but they missed the fact that the only supported platforms for Joomla 4 don't even support mCrypt. Ah, well.

The other question is, why the heck does Joomla 4 still ship the mCrypt support files when it's PHP 7.2+ only and mCrypt is not available on PHP 7.0 and later? I see someone reviewed the code and made comments but they missed the fact that the only supported platforms for Joomla 4 don't even support mCrypt. Ah, well.

Cough #29830 Officially reported to the JSST in 2020

So, the problem is deeper and only apparent if you have a user account set up with TFA before Joomla 3.6.3. That would explain why I haven't seen it in the wild the past year that PHP 8 has been generally available. It would only hit you if you have a site which has been continuously upgraded since before Joomla 3.6.4 and you had set up TFA on the user account before Joomla 3.6.4 and never changed it again.

Ok i have just reproduced the issue following this path:

• installed 3.6.0 (on PHP 5.6)
• created another test user with 2fa
• logged out
• logged in with 2fa
• works
• login with the non-2fa user
• update that site up to 3.10.1
• apply the patch
• take a manuall backup
• restored the backup on php8
• login with the 2fa user fails

Its important that its a new user which you do not login with again after starting the upgrade train, thats also the reason it worked for me later as I used my main SU for that test which i used to login between the updates.

I have just found this line: https://github.com/joomla/joomla-cms/blob/3.10-dev/administrator/components/com_users/models/user.php#L1000-L1005

Here the core forces the mcrypt adapter to be created (and is ignoring everything changed in here) and that resulted into the login issue on php8.

But didnt you mention that openssl can decrypt mcrypt and vise versa too?

@zero-24 Correct. The second option is perfectly backwards compatible. Both OpenSSL and mCrypt can decrypt the data encrypted by the other. This was the first thing I had checked when I wrote the OpenSSL adapter all those years ago :)

When that is the case we should be able to decrypt the mcrypt data with the openssl class (here) and all should be fine on PHP8 too right?

I have found this here: https://stackoverflow.com/questions/31520311/decrypt-mcrypt-with-openssl But I'm not that deep into that stuff to fully understand whether and how that can be applied to Joomla.

When its possible to decrypt mcrypt data with openssl it should be possible to replace the code (here) with such openssl decryption method and that should work good on PHP7 and PHP8.

When openssl can not decrypt our mcrypt data we would have to execute the migration of mcrypt to openssl with the next update using the script.php for all affected users so every user that has been created before but not used since than has the new openssl encrypted data too and will be able to login on PHP8 too.

Or do you see other solutions to this issue?

OpenSSL and mCrypt are cross–compatible: https://github.com/akeeba/fof/blob/3.5.0/Tests/Encrypt/AesTest.php#L260 It's just that mCrypt NULL-pads the ciphertext. If you right trim NULL bytes before decrypting it can be decrypted by OpenSSL just fine. Instead of doing something complicated which might fail you can just rtrim($ciphertext, "\000") and only use the OpenSSL adapter.

I think this is the clue we had missed but when we were rushing to remove the hard mCrypt dependency for Joomla and why we have the complicated code which doesn't quite work.

This discussion in the context of TFA is pretty much moot considering that TFA information is stored unencrypted since Joomla 3.6.4. Not ideal for security but a practical requirement because of how Joomla works. When you transfer your site you need to change the $secret in configuration.php to prevent session hijacking. Doing that you make information encrypted with this secret impossible to decrypt. If TFA is encrypted you are locked out of your site. Bummer.

This brings us back to a discussion we had about a year ago about the need for two different secrets, one for encryption (which never changes) and one for hashing (which does change every time you move your site). Until this is implemented you can't encrypt the TFA settings.

Meanwhile, Joomla should just drop mCrypt support altogether and have the Encrypt package require OpenSSL because a. mCrypt is no longer available on PHP 7 and 8 (just because it's a PECL package doesn't mean it will be installed and enabled on any server); b. mCrypt has not been maintained for well over a decade now, making it unsuitable for production; c. the mCrypt adapter class has a hard requirement on the mCrypt constants which causes a fatal error on PHP 8, making the adapter impossible to use on PHP 8.

For b/c reasons the mCrypt adapter in both Joomla 3.10 and 4.0 could be marked @deprecated 4.1, extend from the OpenSSL adapter and simply override the decrypt method:

public function decrypt($cipherText, $key)
{
  $cipherText = rtrim($cipherText, "\000");
  
  return parent::decrypt($cipherText, $key);
}

You could adapt the unit test I linked to in this comment to make sure nothing breaks.

As this PR, in its initial and current state, doesnt fix the problem and the conversation has moved on from this exact proposed fix, I will close this PR now.

The original issue can be reopened at #35480 or another PR can be proposed to solve this issue correctly.

Feel free though to keep chatting here (or there #35480)... but there is no reason this PR should be open, it doesnt have any chance of being merged.

From my understanding this PR has to be merged too as its the same issue while it might not fix this exact issue?

@zero-24 Yes, this should be merged anyway AND ported to Joomla 4's Encrypt package. Even though it's not used by Joomla itself, any 3PD trying to use either package on PHP 8 will have nasty fatal errors as things stand right now. This is fairly self–evident and doesn't require any thought or decision. It's even fully b/c!

Step 2 (not in this PR) is deprecating mCrypt support and make it into a shim for the OpenSSL adapter, finally removing it altogether sometime in 4.1 or 5.0 — that requires a leadership decision since it does break b/c, even though we're talking about b/c to a very specific subset of people running Joomla on a PHP 7 or 8 server with the mcrypt extension separately compiled and installed through PECL which I can't imagine is anywhere near a significant number of sites.

Yes, this should be merged anyway

Reopening...

For b/c reasons the mCrypt adapter in both Joomla 3.10 and 4.0 could be marked @deprecated 4.1, extend from the OpenSSL adapter and simply override the decrypt method:

I have just tried that code and it seems still to fail on my system?

<?php
/**
 * @package     FrameworkOnFramework
 * @subpackage  utils
 * @copyright   Copyright (C) 2010-2016 Nicholas K. Dionysopoulos / Akeeba Ltd. All rights reserved.
 * @license     GNU General Public License version 2 or later; see LICENSE.txt
 */

// Protect from unauthorized access
defined('FOF_INCLUDED') or die;

class FOFEncryptAesMcrypt extends FOFEncryptAesOpenssl implements FOFEncryptAesInterface
{
	public function decrypt($cipherText, $key)
	{
		$cipherText = rtrim($cipherText, "\000");

		return parent::decrypt($cipherText, $key);
	}
}

Thats the error I get when triying to login with said user:

I'm sure i have just did something wrong?

I think that Mcrypt was also null–padding the plaintext. I don't have access to a system with Mcrypt right now; I only have my MacBook Pro with me which only has PHP 7.2+ and the PECL Mcrypt extension won't compile (it looks like it's missing a library).

Can you try replacing the second line with return rtim(parent::decrypt($cipherText, $key), "\000"); and see if that works?

Can you try replacing the second line with return rtim(parent::decrypt($cipherText, $key), "\000"); and see if that works?

That errors with the same error message and stacktrace. (after fixing the typo in rtrim ;))

I think that Mcrypt was also null–padding the plaintext. I don't have access to a system with Mcrypt right now; I only have my MacBook Pro with me which only has PHP 7.2+ and the PECL Mcrypt extension won't compile (it looks like it's missing a library).

I have send you a backup taken on Joomla 3.6.0 and PHP 5.6 with 2fa setup via mail. That site has been updated to the latest 3.10 version too so besides that issue here it should run on PHP8.

Thank you, Tobias! I got your email. I will definitely take a look a bit later or at least tomorrow.

So, I got a look at the site, debugged it and I can tell you where the problem is.

Before Joomla 3.6.4 we were storing the encrypted data base64-encoded. However, since at least Joomla 3.7.0 there is no code to handle decoding this data. In other words all Joomla versions since at least 3.7.0 up to and including 4.0.3 can NOT migrate your TFA settings if they are encrypted, regardless of whether you have mCrypt installed on your server or not. For the past 5 years Joomla has been unable to migrate these user accounts.

Therefore this whole discussion is about a moot point. If I only ever got the one client with ancient data and you got zero people complaining about the loss of TFA settings in the last 5 years it means that we should NOT even try to decrypt old, encrypted TFA settings. In fact, the entire logic should be removed by Joomla and replaced with code to disable TFA on these user accounts. These are user accounts which have not been logged into the last 5 years. It's unlikely their users will still even have the authenticator set up for that account — remember that Google Authenticator does not back up your data and once you move to a different phone or reset your phone you lose all of your TFA settings.

I'm sorry @nikosdion that it took that long to come back to you here. I agree on your points regarding 2fa. It means that we can keep the PR here as it is right? So the only thing thats missing would be tests that other stuff still works as expected.

@zero-24 Correct. This PR does solve the encryption problem under PHP 8, exactly as it's supposed to.

Merging this now. Thanks for your patience :)