phpseclib Mcrypt performance slow

[MichaelThessel]

Description (*)

Support OpenSSL for encryption.

Expected behavior (*)

While profiling a M2 site I noticed that phpseclib is always the single slowest component. In some cases I see 700ms of my page load time attributed to it. At first I thought that this is an issue with my profiler but I have since confirmed that with (New Relic, xdebug and xhprof).

I had a look at the code and in Magento\Framework\Encryption\Encryptor::getCrypt I see that Mcrypt is hardcoded.

  | #0  Magento\Framework\Encryption\Encryptor->getCrypt() called at [/data/src/m2/vendor/magento/framework/Encryption/Encryptor.php:450]
  | #1  Magento\Framework\Encryption\Encryptor->decrypt() called at [/data/src/m2/vendor/magento/module-config/App/Config/Type/System.php:252]
  | #2  Magento\Config\App\Config\Type\System->Magento\Config\App\Config\Type\{closure}() called at [/data/src/m2/vendor/magento/framework/Cache/LockGuardedCacheLoader.php:84]
  | #3  Magento\Framework\Cache\LockGuardedCacheLoader->lockedLoadData() called at [/data/src/m2/vendor/magento/module-config/App/Config/Type/System.php:261]
  | #4  Magento\Config\App\Config\Type\System->loadDefaultScopeData() called at [/data/src/m2/vendor/magento/module-config/App/Config/Type/System.php:195]
  | #5  Magento\Config\App\Config\Type\System->getWithParts() called at [/data/src/m2/vendor/magento/module-config/App/Config/Type/System.php:169]
  | #6  Magento\Config\App\Config\Type\System->get() called at [/data/src/m2/vendor/magento/framework/App/Config.php:132]
  | #7  Magento\Framework\App\Config->get() called at [/data/src/m2/vendor/magento/module-backend/App/Config.php:51]
  | #8  Magento\Backend\App\Config->getValue() called at [/data/src/m2/vendor/magento/module-backend/App/Area/FrontNameResolver.php:109]
  | #9  Magento\Backend\App\Area\FrontNameResolver->getFrontName() called at [/data/src/m2/vendor/magento/module-backend/Helper/Data.php:209]
  | #10 Magento\Backend\Helper\Data->getAreaFrontName() called at [/data/src/m2/vendor/magento/module-backend/App/Request/PathInfoProcessor.php:50]
  | #11 Magento\Backend\App\Request\PathInfoProcessor->process() called at [/data/src/m2/generated/code/Magento/Backend/App/Request/PathInfoProcessor/Proxy.php:95]
  | #12 Magento\Backend\App\Request\PathInfoProcessor\Proxy->process() called at [/data/src/m2/vendor/magento/framework/App/Request/Http.php:147]
  | #13 Magento\Framework\App\Request\Http->getOriginalPathInfo() called at [/data/src/m2/vendor/magento/framework/App/Request/Http.php:162]
  | #14 Magento\Framework\App\Request\Http->getPathInfo() called at [/data/src/m2/vendor/magento/framework/App/Request/Http.php:217]
  | #15 Magento\Framework\App\Request\Http->getFrontName() called at [/data/src/m2/vendor/magento/framework/App/Http.php:111]
  | #16 Magento\Framework\App\Http->launch() called at [/data/src/m2/vendor/magento/framework/App/Bootstrap.php:261]
  | #17 Magento\Framework\App\Bootstrap->run() called at [/data/src/m2/pub/index.php:40]

This leads to the problem that if you don't have https://pecl.php.net/package/mcrypt installed this is super slow because it does the en/decryption in PHP using phpseclib. The mcrypt PHP lib is deprecated though and not supposed to be used anymore. I installed it for testing purposes and sure enough the decryption is more than 100x faster.

As OpenSSL is a system requirement for M2 (https://devdocs.magento.com/guides/v2.4/install-gde/system-requirements-tech.html) why don't we use it instead of Mcrypt?

Benefits

Make M2 much faster

[m2-assistant]

Hi @MichaelThessel. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

• Summary of the issue
• Information on your environment
• Steps to reproduce
• Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

For more details, please, review the Magento Contributor Assistant documentation.

Please, add a comment to assign the issue: @magento I am working on this

---

• Join Magento Community Engineering Slack and ask your questions in #github channel.

⚠️ According to the Magento Contribution requirements, all issues must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[ihor-sviziev]

Hi @MichaelThessel,
I see in the requirements page that you just shown there is additional requirement for sodium php extension

As magento 2.4.x doesn’t support php 7.2 and lower - seems like we can add this as a requirement to composer.json and update docs that sodium is one of required php extensions.
Are you interested in creating Pull request?

[m2-assistant]

Hi @ihor-sviziev. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

• 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).

  Details

  If the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.

• 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.

• 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

• 4. Verify that the issue is reproducible on 2.4-develop branch

  Details

  - Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
  - If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
  - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

• 5. Add label Issue: Confirmed once verification is complete.

• 6. Make sure that automatic system confirms that report has been added to the backlog.

[MichaelThessel]

Hi @ihor-sviziev I had a look at this and by the looks of it Sodium wouldn't just work as a drop in replacement. Rewriting the Magento encryption stack is unfortunately beyond what I can commit to time wise.

[ihor-sviziev]

@MichaelThessel,
Basically right now Magento using sodium if the extension is installed with a fallback to mbstring. You can see it here:

magento2/lib/internal/Magento/Framework/Encryption/Encryptor.php

Lines 512 to 550 in 68c903f

	private function getCrypt(
	string $key = null,
	int $cipherVersion = null,
	string $initVector = null
	): ?EncryptionAdapterInterface {
	if (null === $key && null === $cipherVersion) {
	$cipherVersion = $this->getCipherVersion();
	}
	if (null === $key) {
	$key = $this->keys[$this->keyVersion];
	}
	if (!$key) {
	return null;
	}
	if (null === $cipherVersion) {
	$cipherVersion = $this->cipher;
	}
	$cipherVersion = $this->validateCipher($cipherVersion);
	if ($cipherVersion >= self::CIPHER_AEAD_CHACHA20POLY1305) {
	return new SodiumChachaIetf($key);
	}
	if ($cipherVersion === self::CIPHER_RIJNDAEL_128) {
	$cipher = MCRYPT_RIJNDAEL_128;
	$mode = MCRYPT_MODE_ECB;
	} elseif ($cipherVersion === self::CIPHER_RIJNDAEL_256) {
	$cipher = MCRYPT_RIJNDAEL_256;
	$mode = MCRYPT_MODE_CBC;
	} else {
	$cipher = MCRYPT_BLOWFISH;
	$mode = MCRYPT_MODE_ECB;
	}
	return new Mcrypt($key, $cipher, $mode, $initVector);
	}

magento2/lib/internal/Magento/Framework/Encryption/Encryptor.php

Line 165 in 68c903f

if (extension_loaded('sodium') && defined('SODIUM_CRYPTO_PWHASH_ALG_ARGON2ID13')) {

According to php docs - sodium extension is bundled with php by default with php 7.2 and above while mcrypt was bundled with php by default in earlier versions, but still could be installed via pecl.

Looks like in your case both extensions were missing and that's why you got such behavior.
In this case Magento tries to use compatibility fallback implemented in php as result you're getting terribly slow performance:

magento2/composer.json

Line 71 in a55b880

"paragonie/sodium_compat": "^1.6",

magento2/composer.json

Line 74 in a55b880

"phpseclib/mcrypt_compat": "1.0.8",

Could you confirm that?

[MichaelThessel]

@ihor-sviziev thanks for pointing me in that direction. I did some further testing and the sodium extension works fine for me now. I must have configured something wrong during my initial tests with sodium as it was still falling back to the mcrypt php lib.

I agree with you though that ext-sodium should be a hard requirement for Magento enforced by composer. The performance of the mcrypt fallback is too poor to be an acceptable workaround. I'm pretty sure a lot of people are running installs without sodium and are not even aware of the performance impact.

If you can give me a list of what is required for this (composer update, doc update, ...) I can do a PR.

[ihor-sviziev]

Basically I see here the only two variants to solve this issue:

1. Add ext-sodium to composer.json. As result we'll not be able to install magento without it. Unfortunately this is backward incompatible change and have to be approved.
   Unfortunately seems like there is additional check should be done - that some specific algorithm is present, seems like it was missing in older versions of lib sodium and we need to find a way to address it.
   Also just checked that we can't fully remove using mcrypt, as it will be required for data migration from old magento versions or hash comparing for old passwords

2. Just add info that sodium or mcrypt is required in all places where php extensions mentioned in the https://github.com/magento/devdocs repo. Technically it's not affecting magento at all, just the docs.

I believe 2nd option we can do without any issues.

@sidolov @gabrieldagama what do you think about 1st option? I'm really prefer going with 1st option.

[hostep]

If the first option is chosen (which I think i the right move here, Magento shops really need to be forced to switch to argon2id password hashing, sha256 for password hashes is completely outdated, it was so even before magento 2 launched), then it needs to be added to the composer.json file from the particular component which requires it, which in this case would be the magento framework's composer.json file, not to the global composer.json file.

For option 2, I happen to know how devdocs compiles a list of required php extensions, and it happens by scanning the composer requirements, so if it's not required using composer, they will not add it to the documentation.