Skip to content
This repository was archived by the owner on Apr 29, 2019. It is now read-only.

HLD Removing mcrypt and adding libsodium

Greg Goldsmith edited this page Feb 23, 2018 · 3 revisions

1. Goals

Three (3) specific goals that have to be accomplished in order for our code base to be considered cryptographically secure

  1. Fix all encryption weaknesses.
  2. Improve overall hashing, including password storing mechanism.
  3. Use strong Cryptographically Secure Pseudo-Random Number Generator (CSPRNG), and replace current ones.

2. Good features of a security library

  • It should have high-level (not low-level) authentication functions, so that easily deployable/used by developers.
  • It should have authenticated encryption. (like AES-OCB or AES-GCM or HMAC)
  • It should use AES-256 or better algorithm, not any variant of this algorithms (currently Magento uses a variant of AES-256).
  • It should use CBC (layered, not divided) cipher mode by default (not ECB at all), and of course with a strong and unpredictable initialization vector (IV). IV is a block of bits that is used by several modes to randomize the encryption and hence to produce distinct ciphertexts even if the same plaintext is encrypted multiple times, without the need for a slower re-keying process.
  • No weak entropy (random number generation). The library should have strong Cryptographically Secure Pseudo-Random Number Generator (CSPRNG).

3. Which library that covers our goals? – libsodium

In PHP 7.2 Sodium replaced Mcrypt as their inherent crypto security library. Not only is that a major factor in the decision to use the library, it fulfills all the Magento requirements as a trusted cryptographic security solution. Below are the reasons why it is the clear solution for our security needs.

  • All details about this library: https://paragonie.com/book/pecl-libsodium
  • Pros: uses authenticated encryption.
  • Pros: uses AES-GCM.
  • Pros: good for both symmetric and asymmetric encryption.
  • Pros: good for cryptographic hash functions.
  • Pros: super easy to use.
  • Pros: constantly updating/releasing (https://pecl.php.net/package/libsodium)
  • Cons: probably not good for SSL/TLS (which actually we don’t require in our case for Magento)

4. Strategy

When PHP 7.2 came out the mcrypt library was removed as a bundled extension because the code is no longer being maintained by its creators. This has caused it to fall behind from a security standpoint and is no longer considered cryptographically secure. Due to this problem PHP decided to bundle libsodium into PHP as their encryption and hashing library.

In order for Magento to support PHP 7.2 changes have to be made to our crypt security. One school of thought would be to leave the current code in place for PHP 7.1 installations and acknowledge this is a less secure platform choice. If a client decides to upgrade their environment to PHP 7.2 then they would receive the benefit of newer, more secure dependencies that provide a cryptographically secure data utility.

The problems with this theory are we ship a half baked security update. If you decide to upgrade your environment to PHP 7.2 your data is protected. If you don’t then we can’t guarantee our code will secure your data.

The school of thought should be that if you upgrade your Magento instance we will fill the security gaps exposed by previous versions. By using dependency injection for the nonce we won’t need to modify the interface. This will keep us from causing BIC which is the goal of any solution at Magento.

Two problems exist that have to be resolved in order for us to deliver an effective upgrade strategy:

  1. Php 7.1 ships with mcrypt but doesn’t include sodium.
  2. Php 7.2 ships with sodium but doesn’t include mcrypt.

By tying Magento’s dependencies to PHP’s dependencies we break most rules defined by the SOLID principals. The requirements are take what has been cryptographically modified using legacy techniques, validate it, and cryptographically modify it using cryptographically secure techniques. If we rely on PHP’s dependency choices the solution becomes convoluted and highly susceptible to backwards incompatible changes.

Instead of relying on the bundled php libraries we should use composer to load both libraries and inject them accordingly. Then you don’t have to worry about what version of PHP is running on the web instance. Removing the dependency on the PHP extensions allows us to give the same amount of security to clients running PHP 7.1 and 7.2.

Below are the Composer repositories for the libraries we need.

**paragonie/sodium_compat ** Pure PHP implementation of libsodium; uses the PHP extension if it exists phpseclib/mcrypt_compat PHP 7.1 polyfill for the mcrypt extension from PHP

5. What does this mean for 3rd party modules

  1. If a 3rd party developer wants their code to be compatible with PHP 7.2 they already know their crypto security code has to be refactored now that mcrypt has been removed.
  2. 3rd party developers should follow the vision described by this document for converting data secured by inferior cryptographic techniques to cryptographically secure techniques.
  3. If a client wants to upgrade to PHP 7.2 and their 3rd party component dependencies haven't released a compatible version they are pinned to their current version.
  4. The inclusion of modules that haven't been updated to use Sodium are not cryptographically secure.
  5. If a module hasn't been updated to use Sodium they will continue to use the bundled mcrypt library which will not cause backward incompatible changes.

6. How to implement libsodium in Magento

For encryption and hashing, this is the current interface in Magento we need to modify the method bodies of: lib/internal/Magento/Framework/Encryption/EncryptorInterface. We also need to inject a new interface into the classes that implement EncryptorInterface for the nonce that is required by Sodium. For overall random number generation and hashing, those are calling Magento FW's hash functions currently, we need to refactor them throughout Magento code base to leverage libsodium's corresponding functions.

6.1. Encryption

6.1.1. Authenticated Encryption

Authenticated encryption is a form of encryption which simultaneously provides confidentiality, integrity, and authenticity assurances on the data that is sent from a client to a server. Currently Magento doesn't have a secure authenticated encryption protocol and it needs to be added to make sure we provide our clients a mechanism to secure themselves from replay attacks and semantic url attacks. The good news is we don't have to modify the existing EncryptorInterface methods. We simply need to create a two new methods for generating a nonce and validating the nonce once the message is received from the client.

The following is a diagram of a typical authenticated encryption workflow.

Nonce Workflow

In order for us to support this workflow the AuthenticatedEncryptionInterface needs to be created with the following two new methods.

<?php
/**
 * Copyright © Magento, Inc. All rights reserved.
 * See COPYING.txt for license details.
 */
namespace Magento\Framework\Encryption;

/**
 * Authenticated Encryption Interface
 *
 * @api
 */
interface AuthenticatedEncryptionInterface
{
    /**
     * Create a unique nonce and store it for future validation
     *
     * @return string
     * @throws \Exception
     */
    public function getNonce();

    /**
     * Validate encrypted text using a nonce to make sure the ciphertext hasn't been tampered with. 
     * The method returns the decrypted text.
     * If the nonce validation fails a SecurityViolationException will be thrown.			      
     *
     * @param string $cipherText
     * @param string $nonce
     * @return string
     * @throws SecurityViolationException
     */
    public function validateEncryption($cipherText, $nonce);

}

An example implementation of the AuthenticatedEncryptionInterface would be the following:

<?php
/**
 * Copyright © Magento, Inc. All rights reserved.
 * See COPYING.txt for license details.
 */
namespace Magento\Framework\Encryption;

/**
 * Authenticated Encryption
 *
 * @api
 */
class AuthenticatedEncryption implements AuthenticatedEncryptionInterface
{
    /**
     * Cryptographic key for instance
     *
     * @var string
     */
    private string $key;
 
    /**
     * @param DeploymentConfig $deploymentConfig
     */
    public function __construct(
        DeploymentConfig $deploymentConfig
    ) {
        // Load the cryptographic key from env.php
        $this->key = $deploymentConfig->get('crypt/key')));
    }

    /**
     * Create a unique nonce and store it for future validation
     *
     * @return string
     */
    public function getNonce()
    {
	unsigned char $nonce[crypto_secretbox_NONCEBYTES];
	randombytes_buf($nonce, sizeof $nonce);
	activateNonce($nonce);
	return $nonce;
    }

    /**
     * Validate encrypted text using a nonce to make sure the ciphertext hasn't been tampered with. 
     * The method returns the decrypted text.
     * If the nonce validation fails a SecurityViolationException will be thrown.			      
     *
     * @param string $cipherText
     * @param string $nonce
     * @return string
     * @throws SecurityViolationException
     */
    public function validateEncryption(
        string $cipherText, 
        string $nonce
    ) {
        unsigned char $decrypted[MESSAGE_LEN];
	
        if (crypto_secretbox_open_easy(
            $decrypted, 
            $ciphertext, 
            sizeof $cipherText, 
            $nonce, 
            $key
        ) != 0) {
    	    /* message forged! */
            return new \Magento\Framework\Exception\SecurityViolationException(
                __("This message has been tampered with.")
            );
	}

        deactivateNonce($nonce);
        return $decrypted;
    }
 
    /**
     * Store the nonce so that it can be referenced later for validation			      
     *
     * @param string $nonce
     */
    private function activateNonce(
	string $nonce
    ) {
        /* Store the nonce in persistent storage
	...
    }
 
    /**
     * Remove the nonce so that it can't be used again			      
     *
     * @param string $nonce
     */
    private function deactivateNonce(
        string $nonce
    ) {
        /* Remove the nonce from storage to indicate it has been used
	...
    }
}

For clients that want to use this added security benefit they will need to refactor their code to get the nonce from the Magento API and then use the nonce, cryptographic key and payload to generate the ciphertext that will be sent to the server API along with the nonce. Then when the server validates the ciphertext with the nonce the action will be allowed to continue. If the nonce has been used or the cipher text cannot be decrypted with the nonce and key the action will fail.

6.1.2 Normal Encryption

For data that needs to be stored in persistent storage the nonce is needed as an argument to the encryption function, however, we don't want to make the caller keep track of the data because it will force them to refactor the encryption code to store the nonce. If the nonce is prepended to the ciphertext the decryption code can extract the nonce from the stored data and successfully decrypt the stored data. This will enable us to remove mcrypt and implement the advanced security features of libsodium without causing backwards incompatible changes. This means we can upgrade Magento cryptographic security without making 3rd party modules modify their code.

Below is an example of how we can refactor the encrypt and decrypt methods on the EncryptorInterface without causing backwards incompatible changes:

<?php
/**
 * Copyright © Magento, Inc. All rights reserved.
 * See COPYING.txt for license details.
 */
namespace Magento\Framework\Encryption;

use Magento\Framework\App\DeploymentConfig;
use Magento\Framework\Encryption\Helper\Security;
use Magento\Framework\Math\Random;

/**
 * Class Encryptor provides basic logic for hashing strings and encrypting/decrypting misc data
 */
class Encryptor implements EncryptorInterface
{
    /**
     * Cryptographic key for instance
     *
     * @var string
     */
    private string $key;
 
    /**
     * @param DeploymentConfig $deploymentConfig
     */
    public function __construct(
        DeploymentConfig $deploymentConfig
    ) {
        // Load the cryptographic key from env.php
        $this->key = $deploymentConfig->get('crypt/key')));
    }
    /**
     * Encrypt a message
     * 
     * @param string $message - message to encrypt
     * @return string
     */
    function encrypt(
        string $message
    ) {
        $nonce = \Sodium\randombytes_buf(
            \Sodium\CRYPTO_SECRETBOX_NONCEBYTES
    	);

        $cipher = base64_encode(
            $nonce.
            \Sodium\crypto_secretbox(
                $message,
                $nonce,
                $key
            )
        );
 
        \Sodium\memzero($message);
        \Sodium\memzero($key);
        return $cipher;
    }

    /**
     * Decrypt a message
     * 
     * @param string $encrypted - message encrypted with encrypt()
     * @return string
     * @throws SecurityViolationException
     */
    function decrypt(
        string $encrypted, 
    ) {   
        $decoded = base64_decode($encrypted);
        if ($decoded === false) {
            throw new \Exception('Scream bloody murder, the encoding failed');
        }
 
    	if (mb_strlen($decoded, '8bit') < (\Sodium\CRYPTO_SECRETBOX_NONCEBYTES + \Sodium\CRYPTO_SECRETBOX_MACBYTES)) {
        	throw new \Exception('Scream bloody murder, the message was truncated');
    	}
    	$nonce = mb_substr($decoded, 0, \Sodium\CRYPTO_SECRETBOX_NONCEBYTES, '8bit');
    	$ciphertext = mb_substr($decoded, \Sodium\CRYPTO_SECRETBOX_NONCEBYTES, null, '8bit');

    	$plain = \Sodium\crypto_secretbox_open(
        	$ciphertext,
        	$nonce,
        	$key
    	);
 
    	if ($plain === false) {
         	throw new \SecurityViolationException('This message has been tampered with.');
    	}
    	\Sodium\memzero($ciphertext);
    	\Sodium\memzero($key);
    	return $plain;
    }
}

6.1.3 Related Refactoring

  • lib/internal/Magento/Framework/Encryption/Encryptor should be there to implement the interface. Modify the code to remove mcrypt functionality and add libsodium functionality.
  • lib/internal/Magento/Framework/Encryption/Crypt should be deprecated.
  • lib/internal/Magento/Framework/Encryption/Helper/Security::compareStrings() should be replaced with Constant-Time String Comparison in https://paragonie.com/book/pecl-libsodium/read/03-utilities-helpers.md
  • lib/internal/Magento/Framework/Encryption/UrlCoder should not be touched. Keep it as it is.
  • Introduce a test to make sure in future everyone uses only FW's encryption.

6.1.4 Migration Tool

Now encryption is complete, work on a migration tool that will convert all encrypted data from old format to new format for an existing application.

  • The tool should take the old $key and decrypt a cipher text to its corresponding plain text.
  • Now convert the old $key to a valid $key according to new format (see Section 6.1. #1(d)). Update that info to user if user provided the $key.
  • Using the new $key and system generated new $nonce (see Section 6.1. #1.b), encrypt the plain text again, and store it in proper places.

6.2 Hashing

Delete the all four hash functions from lib/internal/Magento/Framework/Encryption/EncryptorInterface and corresponding lib/internal/Magento/Framework/Encryption/Encryptor and introduce a new interface in lib/internal/Magento/Framework/Encryption/HashInterface with following three functions:

1. passwordHash($password, $opslimit, $memlimit) (equivalent to old getHash()) which will implement \Sodium\crypto_pwhash_str() a. Note that, you don’t need to provide $salt as before. In modern password hash functions, it is recommended not to provide any salt which will be automatically generated by the algorithm. b. Refactoring required in whole Magento code base, wherever we are using salt now (no more salt). c. It's highly recommended that you use the provided constants for $opslimit and $memlimit. You can use them as default values in the parameters. d. See here for more details. 2. validatePasswordHash($hash_str, $password) (equivalent to old validateHash() and isValidHash() - don’t know why there are 2 identical functions in this interface. Team, investigate this please), which will implement \Sodium\crypto_pwhash_str_verify(). 3. genericHash($msg, $key = null, $length = 32) (equivalent to old hash()) which will implement \Sodium\crypto_generichash(). a. Recommend to use $key (our encryption key, see Section 6.1) for authenticated hashing. This is a strong modern security feature.

6.2.1 Related Refactoring

  • Write lib/internal/Magento/Framework/Encryption/hash which will implement lib/internal/Magento/Framework/Encryption/HashInterface.
  • All hashing (besides password hashing) throughout the code should use lib/internal/Magento/Framework/Encryption/hash::genericHash() only.
    • It ensures consistent and secure hashing everywhere in Magento.
    • In some cases, Magento does hashing only to generate some ID's where it is not necessary to use a secure hashing function. However, there is no harm to use a secuyre one too. Therefore, leverage our FW everywhere as suggested in previous bullet.
    • This approach makes it centrally changeable in future with ease, in case.
  • Introduce a test to make sure in future everyone uses only FW's hashing.

6.1.2 Migration Tool

This is business inCompatible (BiC) change, because it is impossible to convert old hashed data to new ones using the new methods, as it is impossible to reverse hashed data to the plain text and hash again.

Team needs to investigate all use cases, and how we can support backward compatibility in each use cases. Few examples follow:

  1. Password hashing: after the upgrade, whenever a user will input his password, Magento will apply old hash function first to see if it is correct. Then, it will apply the new hash function on the password and update it in the database. Moving forward, Magento will only use the new hash functions.
  2. If we use generic hash function only once to create an ID and never store it, this change will not make any affect on this.
  3. If we use generic hash function to create some ID from user input and store it, we will follow the same approach as explained in #1 above for password hashing. Instead of password hashing methods, we will use generic hash functions here.
  4. If we use generic hash function to create some ID, not from user input but by the application itself, and store it, we need to investigate if we can regenerate all such hashed values with new functions directly without any impact. In case there is any impact, we need to see how application is generating the values, from which we can probably follow the same approach as mentioned in last bullet (#3).

6.3 Cryptographically Secure Pseudo-Random Number Generator (CSPRNG)

Random number generation is very important in encryption. We need to use a Cryptographically Secure Pseudo-Random Number Generator (CSPRNG). A general purpose random number generator will NOT suffice. Introduce a new interface in lib/internal/Magento/Framework/Encryption/randNumGenInterface with following functions:

1. randomBytes($number) which will implement \Sodium\randombytes_buf(). 2. randomInteger($range) which will implement \Sodium\randombytes_uniform(). 3. randomInteger16Bit() which will implement \Sodium\randombytes_random16().

Team needs to investigate if we really need both #2 and #3 above - if possible, better to use only one to make it easier for the developers.

6.3.1 Related Refactoring

  • Write lib/internal/Magento/Framework/Encryption/randNumGen to implement lib/internal/Magento/Framework/Encryption/randNumGenInterface.
  • All random number generation (few examples include mt_rand(), rand(), openssl_random_pseudo_bytes etc.) throughout Magento code base should be refactored to use our FW only (lib/internal/Magento/Framework/Encryption/randNumGenIterface).
    • It ensures consistent and secure random number generation everywhere in Magento.
    • This approach makes it centrally changeable in future with ease, in case.
  • Introduce a test to make sure in future everyone uses only FW's random number generator.

6.3.2 Migration Tool

I don't see any use cases at this point why we will need a migration tool for this refactoring. However, we may store a random number directly in some cases, or we may use a random number in conjunction with another value to generate something which is stored in the database. Team needs to investigate all such cases, and needs to come up with a decision if we really need a migration tool or not. In case, it is required, it is recommended to follow the same approach as in Section 6.2.2.