No Code Attached Yet
avatar nikosdion
nikosdion
4 Sep 2021

Steps to reproduce the issue

  • Set up a site on PHP 7
  • Enable Two Factor Authentication
  • Take a backup (do it manually, so nobody can pretend this is an issue with a third party extension, even though I will present code evidence to the contrary)
  • Restore the backup on a PHP 8 host
  • Try to log in

Expected result

You log in.

Actual result

You get a fatal error about the constant MCRYPT_RIJNDAEL_128 not being defined.

System information (as much as possible)

See the steps above. Apart from the PHP version everything else is irrelevant.

Additional comments

The problem occurs because of the following lines https://github.com/joomla/joomla-cms/blob/3.10-dev/libraries/fof/encrypt/aes.php#L171-L176

The way we had addressed mCrypt being discontinued in PHP 7 was to first try and see if mCrypt is available, then fall back to OpenSSL.

This did indeed work on PHP 7. The mCrypt adapter was created, it reported itself as not supported and we'd fall back to OpenSSL just fine. Caveat: there was a deprecated notice about using an undefined constant.

Come PHP 8 using an undefined constant is a PHP Fatal Error per PHP 8
s backwards incompatible changes page
:

A number of warnings have been converted into Error exceptions:
...
Attempting to access unqualified constants which are undefined. Previously, unqualified constant accesses resulted in a warning and were interpreted as strings.

As a result the order of operations in this OLD AND UNSUPPORTED version of FOF Joomla is still distributing causes a fatal error.

Proposed fix

There are two ways to address this issue.

  1. Remove the mcrypt adapter completely. This may break compatibility with older PHP 5 hosts which have the mcrypt enabled but not the PHP OpenSSL extension enabled. Do note that mcrypt is End of Life for nearly 15 years now...
  2. Change the order of operations in the lines I highlighted above. First try OpenSSL, then try mCrypt.

We can NOT change the mCrypt adapter in any meaningful way. The properties get initialised by constants which are not set which is what is throwing the Error on PHP 8.

Since Joomla 3 is going to be supported until August 2023 this either needs to be fixed or Joomla 3 should be explicitly marked incompatible with PHP 8.

I am pinging @zero-24 and @wilsonge since you are the most likely persons to understand this issue and do something about it.

Hat tip to Robert S. for reporting an issue with a site he restored and who provided enough information for me to unravel the mystery.

avatar nikosdion nikosdion - open - 4 Sep 2021
avatar joomla-cms-bot joomla-cms-bot - change - 4 Sep 2021
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 4 Sep 2021
avatar zero-24
zero-24 - comment - 4 Sep 2021

Change the order of operations in the lines I highlighted above. First try OpenSSL, then try mCrypt.

Just to be sure we do not get a b/c issue when using this solution for older hosts like you descibed for your first proposal right?

avatar nikosdion
nikosdion - comment - 4 Sep 2021

@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 :)

avatar zero-24
zero-24 - comment - 4 Sep 2021

Good, so I would propose to go this path for 3.10 to keep it as b/c as possible but also compatible with PHP8. Can you send a PR with the proposed change against 3.10-dev?

avatar PhilETaylor
PhilETaylor - comment - 5 Sep 2021

Does this work? #35485

avatar alikon alikon - close - 6 Sep 2021
avatar alikon
alikon - comment - 6 Sep 2021

please test #35485

avatar alikon alikon - change - 6 Sep 2021
Status New Closed
Closed_Date 0000-00-00 00:00:00 2021-09-06 06:20:05
Closed_By alikon
avatar PhilETaylor
PhilETaylor - comment - 12 Sep 2021

Please reopen.

avatar richard67 richard67 - change - 12 Sep 2021
Status Closed New
Closed_Date 2021-09-06 06:20:05
Closed_By alikon
avatar richard67
richard67 - comment - 12 Sep 2021

Re-opening since the PR has been closed.

avatar richard67 richard67 - reopen - 12 Sep 2021
avatar brianteeman
brianteeman - comment - 15 Nov 2021

@zero-24 as there is still 2 years life in j3 and this problem will only get bigger as more people switch to php 8 this really needs to be resolved with a high priority

avatar richard67
richard67 - comment - 15 Nov 2021

The PR has meanwhile been re-opened.

avatar Quy Quy - change - 25 Nov 2021
Status New Closed
Closed_Date 0000-00-00 00:00:00 2021-11-25 03:16:32
Closed_By Quy
avatar Quy Quy - close - 25 Nov 2021
avatar Quy
Quy - comment - 25 Nov 2021

Merged #35485

Add a Comment

Login with GitHub to post a comment