Problem/Motivation

The MCrypt function is deprecated from PHP 7.1. and is removed from PHP 7.2.

The the recommended version of this module at the time of this writing (7.x-2.x), has MCrypt as the default.

It is proposed to move MCrypt to a separate project in 7.x-3.x at #2554093: Roadmap to Encrypt 7.x-3.0. In the meantime, it might be better to throw an exception instead of die outright if MCrypt is not available.

Proposed resolution

  1. Make openssl (AES) an built-in encryption method.
  2. Throw an exception if MCrypt is required and not available.
  3. Create fallback to decrypt legacy data encrypted with mcrypt_aes_cbc using openssl.
  4. Do not set MCrypt as the default. (Do not have any default encryption method.)
  5. Set status of mcrypt_aes_cbc to deprecated i hook_FILENAME_encrypt_encryption_methods().
  6. Fix tests.
  7. Provide upgrade path to convert legacy data.

Remaining tasks

See list above. Strikeout indicate that a task is completed.

User interface changes

Not having any defaults require users to set up a default profile. Prior versions did not require this step.

API changes

None.

Data model changes

None.

Comments

alberto56 created an issue. See original summary.

alberto56’s picture

Status: Active » Needs review
StatusFileSize
new1.54 KB

Here is a patch which does this.

Status: Needs review » Needs work

The last submitted patch, 2: 2942898-2-encrypt-7.x-2.x-mcrypt-not-found.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

manuel garcia’s picture

Triggered the tests again to see where we stand with this.

manuel garcia’s picture

Issue tags: +PHP 7.2
manuel garcia’s picture

Status: Needs work » Needs review
StatusFileSize
new722 bytes
new1.51 KB

This should get rid of the test failures I hope

manuel garcia’s picture

mcrypt is gone on PHP 7.2, and support for PHP 7.1 ends on December 1st of this year, so this is not going to be enough pretty soon...

manuel garcia’s picture

Status: Needs review » Needs work
manuel garcia’s picture

Here's a log from our travis build on PHP 7.2:

FILE: .../all/modules/contrib/encrypt/plugins/encryption_methods/mcrypt_aes_cbc.inc
--------------------------------------------------------------------------------
FOUND 24 ERRORS AFFECTING 11 LINES
--------------------------------------------------------------------------------
  33 | ERROR | Function mcrypt_module_open() is deprecated since PHP 7.1 and
     |       | removed since PHP 7.2; Use OpenSSL instead
  33 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
     |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
     |       | instead
  33 | ERROR | The constant "MCRYPT_RIJNDAEL_128" is deprecated since PHP 7.1
     |       | and removed since PHP 7.2
  33 | ERROR | The constant "MCRYPT_MODE_CBC" is deprecated since PHP 7.1 and
     |       | removed since PHP 7.2
  34 | ERROR | Function mcrypt_enc_get_iv_size() is deprecated since PHP 7.1
     |       | and removed since PHP 7.2; Use OpenSSL instead
  34 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
     |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
     |       | instead
  35 | ERROR | Function mcrypt_enc_get_block_size() is deprecated since PHP 7.1
     |       | and removed since PHP 7.2; Use OpenSSL instead
  35 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
     |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
     |       | instead
  92 | ERROR | Function mcrypt_generic_init() is deprecated since PHP 7.1 and
     |       | removed since PHP 7.2; Use OpenSSL instead
  92 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
     |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
     |       | instead
  95 | ERROR | Function mdecrypt_generic() is deprecated since PHP 7.1 and
     |       | removed since PHP 7.2; Use OpenSSL instead
  98 | ERROR | Function mcrypt_generic_deinit() is deprecated since PHP 7.1 and
     |       | removed since PHP 7.2; Use OpenSSL instead
  98 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
     |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
     |       | instead
 105 | ERROR | Function mcrypt_module_close() is deprecated since PHP 7.1 and
     |       | removed since PHP 7.2; Use OpenSSL instead
 105 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
     |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
     |       | instead
 109 | ERROR | Function mcrypt_create_iv() is deprecated since PHP 7.1 and
     |       | removed since PHP 7.2; Use random_bytes() or OpenSSL instead
 109 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
     |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
     |       | instead
 109 | ERROR | The constant "MCRYPT_DEV_URANDOM" is deprecated since PHP 7.1
     |       | and removed since PHP 7.2
 121 | ERROR | Function mcrypt_generic_init() is deprecated since PHP 7.1 and
     |       | removed since PHP 7.2; Use OpenSSL instead
 121 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
     |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
     |       | instead
 127 | ERROR | Function mcrypt_generic() is deprecated since PHP 7.1 and
     |       | removed since PHP 7.2; Use OpenSSL instead
 127 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
     |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
     |       | instead
 145 | ERROR | Function mcrypt_module_close() is deprecated since PHP 7.1 and
     |       | removed since PHP 7.2; Use OpenSSL instead
 145 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
     |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
     |       | instead
--------------------------------------------------------------------------------
FILE: .../all/modules/contrib/encrypt/plugins/encryption_methods/mcrypt_rij_256.inc
--------------------------------------------------------------------------------
FOUND 15 ERRORS AFFECTING 4 LINES
--------------------------------------------------------------------------------
 41 | ERROR | Function mcrypt_get_iv_size() is deprecated since PHP 7.1 and
    |       | removed since PHP 7.2; Use OpenSSL instead
 41 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
    |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
    |       | instead
 41 | ERROR | The constant "MCRYPT_RIJNDAEL_256" is deprecated since PHP 7.1
    |       | and removed since PHP 7.2
 41 | ERROR | The constant "MCRYPT_MODE_ECB" is deprecated since PHP 7.1 and
    |       | removed since PHP 7.2
 42 | ERROR | Function mcrypt_create_iv() is deprecated since PHP 7.1 and
    |       | removed since PHP 7.2; Use random_bytes() or OpenSSL instead
 42 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
    |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
    |       | instead
 42 | ERROR | The constant "MCRYPT_RAND" is deprecated since PHP 7.1 and
    |       | removed since PHP 7.2
 53 | ERROR | Function mcrypt_decrypt() is deprecated since PHP 7.1 and removed
    |       | since PHP 7.2; Use OpenSSL instead
 53 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
    |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
    |       | instead
 53 | ERROR | The constant "MCRYPT_RIJNDAEL_256" is deprecated since PHP 7.1
    |       | and removed since PHP 7.2
 53 | ERROR | The constant "MCRYPT_MODE_ECB" is deprecated since PHP 7.1 and
    |       | removed since PHP 7.2
 57 | ERROR | Function mcrypt_encrypt() is deprecated since PHP 7.1 and removed
    |       | since PHP 7.2; Use OpenSSL instead
 57 | ERROR | Extension 'mcrypt' is deprecated since PHP 7.1 and removed since
    |       | PHP 7.2; Use openssl (preferred) or pecl/mcrypt once available
    |       | instead
 57 | ERROR | The constant "MCRYPT_RIJNDAEL_256" is deprecated since PHP 7.1
    |       | and removed since PHP 7.2
 57 | ERROR | The constant "MCRYPT_MODE_ECB" is deprecated since PHP 7.1 and
    |       | removed since PHP 7.2
--------------------------------------------------------------------------------
manuel garcia’s picture

So the tests are failing on PHP 7.2 because we're using mcrypt_aes_cbc as the default encryption profile.

PHP 7.2 recommends using libsodium moving forward: https://secure.php.net/manual/en/book.sodium.php

So we should probably:

  1. Create an encryption method plugin for sodium.
  2. Change the default encryption method to the new plugin.
  3. Write upgrade path.
  4. Figure out if we should maintain BC and if so, how.
manuel garcia’s picture

StatusFileSize
new59.32 KB

Currently on PHP7.2 the module installs without errors, however the default configuration is still using mcrypt:

rlhawk’s picture

An encryption method plugin for Sodium already exists: https://www.drupal.org/project/sodium/

manuel garcia’s picture

Thanks @rlhawk that's great news. How do you think we should handle the default enryption profile going forward?

I also wanted to point out that mcrypt isn't actually gone, just moved to pecl. People could still install it, for PHP 7.2:

sudo apt install gcc make autoconf libc-dev pkg-config -y
sudo apt install php7.2-dev -y
sudo apt install libmcrypt-dev -y
sudo pecl install mcrypt-1.0.1
gisle’s picture

Category: Feature request » Plan

Isn't throwing an exception if it does not exist a bit heavy-handed?

We've already have support for alternatives (e.g. OpenSSL and Sodium).
And as pointed out in #13 - people can still install it from PECL (for example to convert legacy data to a newer encryption method).

There should be no requirement for mcrypt to exist or to be available, as future use of mcrypt is depreciated. But removing it altogether as an encryption method would be disruptive to legacy data.

I think the best plan for moving forwards with this is just to keep the plugin, but no longer have it pre-configured as the default encryption method.

We also need to adjust the tests to account for this.

Comments please.

manuel garcia’s picture

Thanks @gisle for taking a look here, some of my thoughts:

  • I'd be fine with keeping the plugin in the project, though I think t we should consider moving it to a third party module say encrypt_mcrypt at some point, perhaps on 7.x-3.x if we ever get there.
  • We're going to have to fail gracefully on installations where they upgraded to PHP 7.2, are still using mcrypt plugins and have not installed it from pecl. Else they will get WSOD.
  • Pre-configuring the module to use a different plugin by default would be a great start, agreed. Which plugin would we use by default? Another option would be to stop setting up a default encryption profile on install. We don't do so on 8.x as far as I can tell.
gisle’s picture

Manuel Garcia wrote:

I'd be fine with keeping the plugin in the project, though I think we should consider moving it to a third party module say encrypt_mcrypt at some point, perhaps on 7.x-3.x if we ever get there.

I shall keep it as a plugin for now (with a depreciation warning). There need to be a stable 7.x-2.4-release with an upgrade path before we can consider removing it. I think that if we're going to move it to a third party module, it should happen in the 7.x-3.x branch. There is a separate issue for this: #2554093: Roadmap to Encrypt 7.x-3.0, so the discussion of what to do with mcrypt in that release should take place there.

We're going to have to fail gracefully on installations where they upgraded to PHP 7.2, are still using mcrypt plugins and have not installed it from pecl. Else they will get WSOD.

I'm testing now on a site with PHP 7.2 and no mcrypt. I am not getting a WSOD, just the same display as the one you posted in comment #11. The radiobutton for any method (including mcrypt) is already unselectable unless preticked. So I believe removing it as the default selection should suffice. If you think there is a situation where we will get a WSOD with the library missing, please provide the steps required to reproduce.

Pre-configuring the module to use a different plugin by default would be a great start, agreed. Which plugin would we use by default? Another option would be to stop setting up a default encryption profile on install. We don't do so on 8.x as far as I can tell.

I am committed to make the Drupal 7 branch as similar to the Drupal 8 branch as possible, so I think it makes sense to follow the lead from the Drupal 8 version.

manuel garcia’s picture

Thanks @gisle for the detailed responses,

I shall keep it as a plugin for now (with a depreciation warning). There need to be a stable 7.x-2.4-release with an upgrade path before we can consider removing it. I think that if we're going to move it to a third party module, it should happen in the 7.x-3.x branch. There is a separate issue for this: #2554093: Roadmap to Encrypt 7.x-3.0, so the discussion of what to do with mcrypt in that release should take place there.

+1

I'm testing now on a site with PHP 7.2 and no mcrypt. I am not getting a WSOD, just the same display as the one you posted in comment #11. The radiobutton for any method (including mcrypt) is already unselectable unless preticked. So I believe removing it as the default selection should suffice. If you think there is a situation where we will get a WSOD with the library missing, please provide the steps required to reproduce.

I've not directly tested this, but I assume that an existing site, which is using an mcrypt encrytion method, say using webform_encrypt or something similar, and they upgrade to PHP 7.2, then when they try to decrypt a value or encrypt a value (making a new webform submission for example), then it probably will throw a fatal error I think.

I am committed to make the Drupal 7 branch as similar to the Drupal 8 branch as possible, so I think it makes sense to follow the lead from the Drupal 8 version.

Sounds good to me :)

cellar door’s picture

I think the best way to handle this is making sure there's a try/catch on encrypt so that if there's an exception thrown from not having mcrypt then we can gracefully fail with an error message.

There will still need to be a way to notify people of the deprecation of mcrypt and likely some support steps on how to decrypt/re-encrypt with a new encryption profile setup using openssl.

gisle’s picture

Assigned: Unassigned » gisle

I'll try to get this one resolved.

gisle’s picture

Added #2421921: Update all encryption plugins to use base64 so data can be saved to database as a related issue. It looks like all non-depreciated methos default to base64 now, but should make sure as part of resolving this one.

  • gisle committed 8c6269e on 7.x-2.x authored by Manuel Garcia
    Issue #2942898 by Manuel Garcia, alberto56: Check if mcrypt exists...
gisle’s picture

Cellar Door wrote:

I think the best way to handle this is making sure there's a try/catch on encrypt so that if there's an exception thrown from not having mcrypt then we can gracefully fail with an error message.

I agree. I've verified that there is a WSOD if the module is handed legacy data encrypted with mcrypt_aes_cbc when using plain vanilla PHP 7.2, and that the patch in #6 fixes that. I've just pushed this patch to 7.x-2.x-dev.

There will still need to be a way to notify people of the deprecation of mcrypt and likely some support steps on how to decrypt/re-encrypt with a new encryption profile setup using openssl.

True. I am leaving status set to "Needs work" for this reason.

However:

  1. The project already supports deprecation of encryption methods. One just need to set 'deprecated' => TRUE in hook_FILENAME_encrypt_encryption_methods().
  2. As far as I am able to tell, AES (OpenSSL) + HMAC-SHA256 (now built-in for PHP 7.2) decrypts data encrypted with MCrypt AES CBC just fine. So it is trivial to create a fallback to use openssl to decrypt legacy data encrypted with mcrypt_aes_cbc. Such a fallback will be pushed in the next snapshot.

(I think legacy data should be converted by means of an upgrade path, but the fallback may be used as a stopgap.)

gisle’s picture

Title: Check if mcrypt exists before using, throw Exception otherwise » MCrypt is deprecated in PHP 7.2
Issue summary: View changes

Updated summary with current status.

gisle’s picture

Issue summary: View changes

  • gisle committed 1b5eefc on 7.x-2.x
    Issue #2942898 by gisle: Use fallback for MCrypt AES.
    
gisle’s picture

Issue summary: View changes

Fallback to decrypt legacy data encrypted with mcrypt_aes_cbc using openssl (AES) just pushed to latest snapshot of 7-x.2.x-dev.

q2_faith’s picture

Fallback to decrypt legacy data encrypted with mcrypt_aes_cbc using openssl (AES) just pushed to latest snapshot of 7-x.2.x-dev.

I don't think that hard-coded fallback from mcrypt_aes_cbc to openssl it's a flexible solution. Because site owner can use another AES compatible plugin.

gisle’s picture

I don't think that hard-coded fallback from mcrypt_aes_cbc to openssl it's a flexible solution. Because site owner can use another AES compatible plugin.

It is not supposed to be a flexible solution. It supposed to offer a solution that will work in the majority of cases (because the openssl method is included in any vanilla PHP configuration - and even if openssl is missing, it will fail with a meaningful error message.)

This issue isn't done (see issue summary). We will eventually offer an upgrade path where the site owner can chose another AES compatible plugin (or whatever). We're not just there yet - this will not be done until step 7.

tom.smith’s picture

I may be able to help out a little with part 7. I am currently using this in conjunction with the Field encrypt module and am writing a patch to help migrate our data. We have just shy of 100k encrypted entries so I am currently evaluating 3 migration paths.

Upgrade while on php 7.1
1. drush command to decrypt via mcrypt and encrypt via sodium. (Would require sodium.so added to server. this is my current favorite)

Upgrade while on php 7.2
2. drush command to decrypt mcrypt data via openssl and encrypt via sodium.
3. Write fallback to do lazy upgrade. (This is my least favorite option)

I understand my migration is very specific to the the Field encrypt module and moving to sodium but hopefully my patch will be helpful in some way. Even if it is just a good laugh :)

gisle’s picture

Issue summary: View changes

@tom.smith,
great!

I am thankful for any help.

If I understand things correctly, your first two options (drush commands) can be implemented without altering Encrypt. If this is correct, just go ahead and we'll package the scripted drush commands with the project when they are ready.

If this is not correct, just say what you need from me, and I'll do my best.

tom.smith’s picture

@gisle,

That is correct. It should not require any changes to Encrypt.

steven jones’s picture

Title: MCrypt is deprecated in PHP 7.2 » mcrypt extension is deprecated in PHP 7.1 and removed in PHP 7.2

Just changing the issue title to be a little more accurate.

As for providing an upgrade path, am I right in thinking that really since encrypt module is pretty much an API module that provides two main functions (encrypt/decrypt) that actually the upgrade path for legacy data needs to go in the modules using encrypt, and that it would probably look a lot like https://git.drupalcode.org/project/webform_encrypt/blob/7.x-1.x/webform_... for example, but doing an immediate encrypt on the data too?

gisle’s picture

Steven Jones wrote:

As for providing an upgrade path, am I right in thinking that really since encrypt module is pretty much an API module that provides two main functions (encrypt/decrypt) that actually the upgrade path for legacy data needs to go in the modules using encrypt

Well, there is nothing stopping the maintainers of modules that build on Encrypt from doing so, following the example you posted (Webform encrypt).

However, I still think that Encrypt should come with some sort of upgrade path. This may be drush-scripts to decrypt and then re-encrypt as suggested in comment #29, or it may just be instructions describing how to go about decrypting all data, and then re-encrypting with the method of your choice.

divyesh19’s picture

@gisle and @tom.smith, any update on the scripted drush commands as mentioned in comments #29 and #30? (or a rough idea on when would we be reaching to step 7 of providing an upgrade path?)

steven jones’s picture

What I went with in the end was this:

  • An update hook that made sure that various encrypt configurations were present and correct, so leaving the older one intact was key and then configuring a new one to replace it and setting is as the default method.
  • I ended up writing a hook_cron implementation that is currently re-encrypting the millions of rows of data I have from the older config to the new one. An update hook simply would have taken too long. It could have been a custom Drush command, since the implementation of the hook is about as simple as it gets. Project constraints forced me to use a hook_cron implementation. Note that this is webform_encrypt's data, encrypt module doesn't manage any actual encrypted data in the database as such.
  • I've got a hook_requirements that checks to see if there's any more data to convert and pops a warning onto admin/reports/status if there is.
  • Once the conversion process is done, I'll remove the old encrypt configuration.

So, I reckon the upgrade path needs to be a bunch of documentation and some useful drush/cron commands for webform_encrypt and any other modules that use the data that re-encrypt everything at the right point in the process.

Here's my hook_cron implementation for those that are interested.
To be general purpose it would need to support identifying the legacy data in a more configurable way, and probably setting an amount to do per invocation, but otherwise it's pretty straightforward:

/**
 * Implements hook_cron().
 */
function hook_cron() {
  // Find legacy data that needs encrypting.
  $data = db_query('SELECT nid, sid, cid, data FROM {webform_submitted_data} WHERE data LIKE :data LIMIT 0, 1000', array(':data' => '%' . db_like('s:6:"method";s:14:"mcrypt_rij_256"') . '%'))->fetchAll();
  if (!empty($data)) {
    $components = array();
    $results = db_query('SELECT nid, cid, extra FROM {webform_component}')->fetchAll();
    foreach ($results as $row) {
      $components[$row->nid . ':' . $row->cid] = unserialize($row->extra);
    }

    foreach ($data as $row) {
      $key = $row->nid . ':' . $row->cid;
      if (!empty($components[$key]['encrypt']) && is_array(@unserialize($row->data))) {
        db_update('webform_submitted_data')
          ->fields(array(
            'data' => encrypt(decrypt($row->data, array('base64' => TRUE)), array('base64' => TRUE)),
          ))
          ->condition('nid', $row->nid)
          ->condition('sid', $row->sid)
          ->condition('cid', $row->cid)
          ->execute();
      }
    }
  }
}

/**
 * Implements hook_requirements().
 */
function hook_requirements($phase) {
  $requirements = array();

  if ($phase == 'runtime') {
    $rows_remaining = db_query('SELECT count(*) FROM {webform_submitted_data} WHERE data LIKE :data', array(':data' => '%' . db_like('s:6:"method";s:14:"mcrypt_rij_256"') . '%'))->fetchField();
    if ($rows_remaining > 0) {
      $requirements['webform_reencrypt'] = array(
        'title' => t('Webform data re-encryption'),
        'value' => format_plural($rows_remaining, '1 row remaining to re-encrypt.', '@count rows remaining to re-encrypt.'),
        'severity' => REQUIREMENT_INFO,
        'description' => t('Encrypt module has been upgraded and configured but older webform data has not been fully re-encrypted.<br>Re-encryption will happen on cron. We will need this process to complete before moving to PHP 7.2. See <a href="https://www.drupal.org/project/encrypt/issues/2942898" target="_blank">#2942898</a> for further information.'),
      );
    }
  }

  return $requirements;
}

moshe weitzman’s picture

Thanks @steven. If one only uses Encrypt for TFA, is a data migration needed when switching from Mcrypt to OpenSSL? Maybe backup codes become invalid without a migration.

steven jones’s picture

@moshe weitzman I'm not sure how Two-factor Authentication (TFA) module uses mcrypt, but yeah, if it's simply encrypting/decrypting short lived data, then you could probably get away with moving the encryption and invalidating any logins in progress (if that's what it's doing).

jenna.tollerson’s picture

I also wanted to point out that mcrypt isn't actually gone, just moved to pecl. People could still install it, for PHP 7.2

Just FYI for any Acquia-hosted folks looking at this issue, Acquia Security has rejected the use of PECL mcrypt on their platform. Their recommendation is to move to OpenSSL. That is the entirety of the guidance we received.

powrsurg’s picture

There is an issue with proposed resolution item #3. While this module does attempt to decrypt data that was encrypted with mycrypt, the openssl decrypt method fails to decrypt the data with errors like:

error:03000082:digital envelope routines::invalid key length

I have attempted this in multiple environments and this seems pretty consistent.

ptmkenny’s picture

Status: Needs work » Closed (outdated)

Development of Encrypt for Drupal 7 has ended.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.