-
Notifications
You must be signed in to change notification settings - Fork 153
Mcrypt to Sodium migration #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
da8d43d
Mcrypt to Sodium migration
buskamuza dae382c
Updated reference to the image
buskamuza aa9996d
Fixed image reference
buskamuza 7cf3cc2
Fixed explanation for public API for encryption
buskamuza 8776ef7
Update mcrypt-to-sodium-migration.md
buskamuza File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
## 1. Goals and Requirements | ||
|
||
1. Target Magento version is 2.3 | ||
1. Possibly, 2.3.x patch version. The implementation should be fully backward compatible | ||
2. Use Sodium library for encryption, as this is the latest encryption library supported natively by the latest PHP version (PHP 7.2) | ||
3. Ensure encryption is possible on PHP 7.1, which is also supported by Magneto 2.3 | ||
4. Data is migrated to be compatible with the new algorithm if necessary | ||
1. On-the-fly migration (data is re-encrypted when being read/written during application run) is acceptable | ||
2. Upgrade time should not increase significantly on large stores | ||
|
||
## 2. Strategy | ||
|
||
### PHP 7.1 and 7.2 Support | ||
|
||
Magento 2.3 supports both PHP 7.1 and 7.2. This leads to necessity to have a solution for both versions of PHP. In the same time, | ||
1. Php 7.1 ships with mcrypt but doesn’t include sodium. | ||
2. Php 7.2 ships with sodium but doesn’t include mcrypt. | ||
|
||
To solve the problem, we can use polyfill library [paragonie/sodium_compat](https://github.com/paragonie/sodium_compat), which provides Sodium support to PHP installations that don't have Sodium support. It uses the PHP extension if it exists, and it's more performant in this case. | ||
|
||
As we still need to decrypt old data encrypted with mcrypt, and Sodium doesn't support same algorithms, another polyfill library [phpseclib/mcrypt_compat](https://github.com/phpseclib/mcrypt_compat) can be used to decrypt data on PHP 7.2. | ||
|
||
### Implementation | ||
|
||
Include both `phpseclib/mcrypt_compat` and `paragonie/sodium_compat` as Composer dependencies. | ||
|
||
Create adapters for Mcrypt and Sodium: | ||
|
||
 | ||
|
||
`Mcrypt` implementation uses `phpseclib/mcrypt_compat`. | ||
* Old `\Magento\Framework\Encryption\Crypt` class is deprecated, and reuses the new implementation for avoiding code duplication. | ||
`Sodium` implementation uses `paragonie/sodium_compat`. | ||
* Use `crypto_aead_xchacha20poly1305_ietf*` methods for encryption/decryption. See [recommendations](https://paragonie.com/blog/2017/06/libsodium-quick-reference-quick-comparison-similar-functions-and-which-one-use). | ||
|
||
`\Magento\Framework\Encryption\Encryptor` is a public API (`@api` annotation should be added) for encryption, which uses `EncryptionAdapterInterface` under the hood. | ||
|
||
Please, see [Implementation](https://github.com/magento-engcom/php-7.2-support/pull/135) for details. | ||
|
||
## 3. Data migration | ||
|
||
* Limited or expected-to-be small amount of data to be converted during upgrade process | ||
* Large amount of data to be migrated on the fly: the data is re-encrypted when read and stored again during application work. Currently used encryption algorithms are secure enough to allow the data stay. | ||
* Additionally, a Magento CLI command can be implemented that converts the data after the application is upgraded. This should not cause issues as both old and new data is supported by the application. The command is implemented for potentially large amounts of data - https://github.com/magento-engcom/php-7.2-support/pull/135/files#diff-9da6c367f822ceff09c7dd810c6bfb85 | ||
|
||
## 4. What does this mean for extension developers | ||
|
||
* Extension developers should use `\Magento\Framework\Encryption\Encryptor` for encryption. | ||
* They may also implement a DB patch to re-encrypt the data, if amount of data is not expected to be large. | ||
|
||
## 5. Resources | ||
|
||
* [Epic](https://github.com/magento-engcom/php-7.2-support/issues/127) | ||
* [Initial Design Document](https://github.com/magento-engcom/php-7.2-support/wiki/HLD---Removing-mcrypt-and-adding-libsodium) | ||
* [Discussion](https://github.com/magento-engcom/php-7.2-support/wiki/Discussion:-Encryption-with-Libsodium) | ||
* [Implementation](https://github.com/magento-engcom/php-7.2-support/pull/135) |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should specify a procedure of converting all the remaining data at some point. Because not all data will be accessed over the specific period of time.
The time of the upgrade was a concerns for SIs during migration to Magento 2.2 because of
serialize
tojson_encode
conversion, we need to make sure to prevent similar issues this time.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if we don't migrate the data, the application will work fine, we will still support old algorithms. While, for
serialize
->json_encode
task, we had to migrate all the data during upgrade because we had to eliminate usages ofserialize
.I'll add to the document (as a separate PR) as we investigate options for data migration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added example of CLI implementation.