Removing mcrypt compat and replacing it with OpenSSL lib #40053

jakwinkler · 2025-07-08T19:46:34Z

Description (*)

Replacing mcrypt with OpenSSL lib to use up to date libraries.

Fixed Issues (if relevant)

Fixes phpseclib Mcrypt performance slow #30556

Manual testing scenarios (*)

...
...

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
All automated tests passed successfully (all builds are green)

This commit replaces the deprecated mcrypt extension usage with OpenSSL for encryption and decryption operations within the Magento framework. A new OpenSsl adapter has been introduced, and the Encryptor class has been updated to utilize it. Composer dependencies have been updated to reflect these changes.

m2-assistant · 2025-07-08T19:46:37Z

Hi @jakwinkler. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

hostep · 2025-07-08T20:14:29Z

I always wondered about mcrypt in Magento, because it's considered deprecated since many many years.

Is it actually still being used in Magento, I mean for real in practice, not in code? If yes, where or in which circumstances?
If no, we should just remove all code that references it and don't replace it in my opinion.

Copilot

Pull Request Overview

This PR replaces the deprecated mcrypt-based encryption with an OpenSSL-backed implementation and removes the mcrypt compatibility library.

Swaps out the Mcrypt adapter for a new OpenSsl adapter in the core Encryptor
Adds explicit cipher-mode constants, updates method signatures with return types, and refines type hints
Removes the phpseclib/mcrypt_compat dependency from composer

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File	Description
lib/internal/Magento/Framework/Encryption/Encryptor.php	Replaced `Mcrypt` usage with `OpenSsl`, added MODE constants, and tightened type hints/return types
lib/internal/Magento/Framework/Encryption/Adapter/OpenSsl.php	Introduced new `OpenSsl` adapter implementing `EncryptionAdapterInterface`
composer.json	Removed the `phpseclib/mcrypt_compat` package from dependencies

Comments suppressed due to low confidence (2)

lib/internal/Magento/Framework/Encryption/Adapter/OpenSsl.php:100

The decrypt() method can throw RuntimeException on failure but lacks a @throws annotation in the docblock; please document thrown exceptions for clarity.

    public function decrypt(string $data): string

lib/internal/Magento/Framework/Encryption/Adapter/OpenSsl.php:16

This new OpenSsl adapter is not covered by unit tests; consider adding tests to validate encrypt/decrypt flows for different ciphers and modes.

class OpenSsl implements EncryptionAdapterInterface

Copilot · 2025-07-09T06:00:42Z

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


    /**
     * Array of encryption keys
     *
     * @var string[]
     */
-    protected $keys = [];
+    protected array|false $keys = [];


The property $keys is declared as array|false but initialized to an empty array; consider refining the type to string[] or just array to reflect actual usage and improve clarity.

Suggested change

protected array|false $keys = [];

protected string[] $keys = [];

Copilot · 2025-07-09T06:00:42Z

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


    /**
     * @var KeyValidator
     */
-    private $keyValidator;
+    private mixed $keyValidator;


Typing $keyValidator as mixed reduces type safety; consider using the explicit KeyValidator type or an interface to maintain strong typing.

Suggested change

private mixed $keyValidator;

private KeyValidator $keyValidator;

Copilot · 2025-07-09T06:00:43Z

lib/internal/Magento/Framework/Encryption/Adapter/OpenSsl.php

+        // For CBC mode, if IV is not provided for encryption, generate one.
+        // For decryption, the IV is expected to be part of the data or provided.
+        if ($this->mode === Encryptor::MODE_CBC && $this->initVector === null) {
+            $this->initVector = openssl_random_pseudo_bytes(openssl_cipher_iv_length($this->cipherMethod));


openssl_random_pseudo_bytes can return false on failure; handle the false case to avoid unexpected null or boolean values in IV generation.

Suggested change

$this->initVector = openssl_random_pseudo_bytes(openssl_cipher_iv_length($this->cipherMethod));

$ivLength = openssl_cipher_iv_length($this->cipherMethod);

$generatedIv = openssl_random_pseudo_bytes($ivLength);

if ($generatedIv === false) {

throw new \RuntimeException('Failed to generate initialization vector using OpenSSL.');

}

$this->initVector = $generatedIv;

ihor-sviziev · 2025-07-09T16:21:47Z

@jakwinkler, please review the comments from Copilot and let us know if you would like anything to be fixed based on them. If no, I'll review the pull request

ihor-sviziev · 2025-07-10T10:01:49Z

@magento run all tests

Jakub Winkler added 3 commits July 8, 2025 21:29

Remove PhpSecLib adapter

48bf2a8

code refactoring for mcrypt removal

cd5e9c1

Merge branch '2.4-develop' into mcrypt_removal

7249b59

jakwinkler mentioned this pull request Jul 8, 2025

phpseclib Mcrypt performance slow #30556

Open

Merge branch '2.4-develop' into mcrypt_removal

dd68b56

ihor-sviziev requested a review from Copilot July 9, 2025 05:59

ct-prd-projects-boards-automation bot added the Progress: review label Jul 9, 2025

Copilot AI reviewed Jul 9, 2025

View reviewed changes

Merge branch '2.4-develop' into mcrypt_removal

1b2e10d

engcom-Charlie added the Project: Community Picked PRs upvoted by the community label Jul 14, 2025

engcom-Charlie added this to Community Dashboard Jul 14, 2025

engcom-Charlie moved this to Pending Review in Community Dashboard Jul 14, 2025

ct-prd-projects-boards-automation bot added the Progress: pending review label Jul 14, 2025

engcom-Bravo added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Jul 14, 2025

engcom-Charlie added feature request Priority: P2 A defect with this priority could have functionality issues which are not to expectations. labels Jul 15, 2025

ct-prd-projects-boards-automation bot added this to Pull Requests Dashboard Jul 15, 2025

github-project-automation bot moved this to Pending Review in Pull Requests Dashboard Jul 15, 2025

ct-prd-projects-boards-automation bot removed this from Community Dashboard Jul 15, 2025

engcom-Charlie added this to Community Dashboard Jul 16, 2025

engcom-Charlie removed this from Pull Requests Dashboard Jul 16, 2025

engcom-Charlie moved this to Pending Review in Community Dashboard Jul 16, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Removing mcrypt compat and replacing it with OpenSSL lib #40053

Removing mcrypt compat and replacing it with OpenSSL lib #40053

jakwinkler commented Jul 8, 2025

Uh oh!

m2-assistant bot commented Jul 8, 2025

Uh oh!

hostep commented Jul 8, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jul 9, 2025

Uh oh!

Copilot AI Jul 9, 2025

Uh oh!

Copilot AI Jul 9, 2025

Uh oh!

ihor-sviziev commented Jul 9, 2025

Uh oh!

ihor-sviziev commented Jul 10, 2025

Uh oh!

Uh oh!

	protected array\|false $keys = [];
	protected string[] $keys = [];

	private mixed $keyValidator;
	private KeyValidator $keyValidator;

-            $this->initVector = openssl_random_pseudo_bytes(openssl_cipher_iv_length($this->cipherMethod));
+            $ivLength = openssl_cipher_iv_length($this->cipherMethod);
+            $generatedIv = openssl_random_pseudo_bytes($ivLength);
+            if ($generatedIv === false) {
+                throw new \RuntimeException('Failed to generate initialization vector using OpenSSL.');
+            }
+            $this->initVector = $generatedIv;

Removing mcrypt compat and replacing it with OpenSSL lib #40053

Are you sure you want to change the base?

Removing mcrypt compat and replacing it with OpenSSL lib #40053

Conversation

jakwinkler commented Jul 8, 2025

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Jul 8, 2025

Uh oh!

hostep commented Jul 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

ihor-sviziev commented Jul 9, 2025

Uh oh!

ihor-sviziev commented Jul 10, 2025

Uh oh!

Uh oh!