From bb2f693560079182132dda91546277ceff8ff981 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Tue, 2 Mar 2021 04:10:15 -0800 Subject: [PATCH] Upgrader (5.34) - Handle unsavable characters Overview -------- In php-mysqli with utf8mb4, the escaping rules do not handle 8-bit characters (`chr(128)`+). ([Demo](https://gist.github.com/totten/4083741b920113ffc569d40053ce849d)) Here's a situation reported by @agileware-justin which provokes this: > 1. SMTP credentials (mailing_backend) were saved and had been encrypted using mcrypt, prior to PHP 7.1 > 2. SMTP outbound email was NOT enabled, but the SMTP credentials are in the database > 3. Active PHP version was PHP 7.3, without mcrypt module > 4. CiviCRM 5.34 upgrade triggers the database error Before ------ The behavior can be viewed in two variables: * Depending on whether `CIVICRM_CRED_KEYS` is set, the upgrader may be writing passwords as plain-text or as `^CTK?` tokens. * Depending on what value is in `$setting['smtpPassword']`, what value is in `CIVICRM_SITE_KEY`, and whether `mcrypt` is active, we may or may not get 8-bit characters when reading the password (`CRM_Utils_Crypt::decrypt($setting['smtpPassword'])`). The fatal combination arises when using plain-text with 8-bit characters. But other combinations (encrypted tokens and/or 7-bit plain-text) seem fine. After ----- As before, combinations involving encrypted tokens and/or 7-bit plain-text are fine. We don't have a head-on soultion for escaping 8-bit plain-text for use with php-mysqli-utf8mb4. (Which is insane, right?) But now we manage the symptoms better: * If you aren't even using SMTP (like in Justin's example), then this is not legit. We show a warning and simply discard the unneeded/corrupt value of `smtpPassword`. * If you are using SMTP, then this might theoretically be legit. (We haven't confirmed, but it seems plausible in other locales.) We show a different warning and encourage the sysadmin to setup `CIVICRM_CRED_KEYS` (which will enable the more permissive `^CTK?` format.) --- .../Incremental/php/FiveThirtyFour.php | 75 +++++++++++++++++-- 1 file changed, 69 insertions(+), 6 deletions(-) diff --git a/CRM/Upgrade/Incremental/php/FiveThirtyFour.php b/CRM/Upgrade/Incremental/php/FiveThirtyFour.php index 7023a3ae27..ef777762be 100644 --- a/CRM/Upgrade/Incremental/php/FiveThirtyFour.php +++ b/CRM/Upgrade/Incremental/php/FiveThirtyFour.php @@ -35,13 +35,42 @@ class CRM_Upgrade_Incremental_php_FiveThirtyFour extends CRM_Upgrade_Incremental } if ($rev === '5.34.alpha1') { - if (extension_loaded('mcrypt') && !empty(self::findSmtpPasswords())) { + $smtpPasswords = self::findSmtpPasswords(); + $cryptoRegistry = \Civi\Crypto\CryptoRegistry::createDefaultRegistry(); + if (extension_loaded('mcrypt') && !empty($smtpPasswords)) { // NOTE: We don't re-encrypt automatically because the old "civicrm.settings.php" lacks a good key, and we don't keep the old encryption because the format is ambiguous. // The admin may forget to re-enable. That's OK -- this only affects 1 field, this is a secondary defense, and (in the future) we can remind the admin via status-checks. $preUpgradeMessage .= '

' . ts('This system has an encrypted SMTP password. Encryption in v5.34+ requires CIVICRM_CRED_KEYS. You may setup CIVICRM_CRED_KEYS before or after upgrading. If you choose to wait, then the SMTP password will be stored as plain-text until you setup CIVICRM_CRED_KEYS.', [ 1 => 'https://docs.civicrm.org/sysadmin/en/latest/upgrade/version-specific/#smtp-password', ]) . '

'; } + foreach ($smtpPasswords as $setting) { + $settingValue = unserialize($setting['value']); + $canBeStored = self::canBeStored($settingValue['smtpPassword'], $cryptoRegistry); + $isSmtpActive = (((int) $settingValue['outBound_option'] ?? -1) == CRM_Mailing_Config::OUTBOUND_OPTION_SMTP); + + $msg = NULL; + if (!$canBeStored && !$isSmtpActive) { + $prose = ts('The SMTP password (%1) was previously stored in the database. It has unexpected content which cannot be migrated automatically. However, it appears to be inactive, so the upgrader will drop this value.', [ + 1 => "setting#" . $setting['id'], + ]); + $preUpgradeMessage .= '

' . $prose . '

'; + } + elseif (!$canBeStored && $isSmtpActive) { + // "Misconfiguration" ==> Ex: 'mcrypt' was enabled, used to setup the password, and then disabled, leaving us with unreadable ciphertext. + // "Misconfiguration" ==> Ex: 'mcrypt' was enabled, used to setup the password, but the SITE_KEY was changed, leaving us with unreadable ciphertext. + // "Unrecognized character" ==> Ex: When using plain-text and utf8mb4, it fails to write some characters (chr(128)-chr(255)). + // To date, we have only seen reports where this arose from misconfiguration. + $prose = ts('The SMTP password (%1) has unusual content that cannot be stored as plain-text. It may be unreadable due to a previous misconfiguration, or it may rely on an unclear character-set. Your options are:', [ + 1 => "setting#" . $setting['id'], + ]); + $option_1 = ts('Setup CIVICRM_CRED_KEYS before upgrading. This warning will go away, and the current SMTP password will ultimately be preserved during upgrade.', [ + 1 => 'https://docs.civicrm.org/sysadmin/en/latest/upgrade/version-specific/#smtp-password', + ]); + $option_2 = ts('Proceed with upgrading in the current configuration. The SMTP password will be lost, but you may re-configure it after upgrade.'); + $preUpgradeMessage .= "

$prose

"; + } + } foreach ($GLOBALS['civicrm_setting'] ?? [] as $entity => $overrides) { if (extension_loaded('mcrypt') && !empty($overrides['mailing_backend']['smtpPassword']) && $overrides['mailing_backend']['outBound_option'] == 0) { // This is a fairly unlikely situation. I'm sure it's *useful* to set smtpPassword via $civicrm_setting (eg for dev or multitenant). @@ -147,20 +176,54 @@ class CRM_Upgrade_Incremental_php_FiveThirtyFour extends CRM_Upgrade_Incremental */ public static function migrateSmtpPasswords(CRM_Queue_TaskContext $ctx) { $settings = self::findSmtpPasswords(); - $cryptoToken = new \Civi\Crypto\CryptoToken(\Civi\Crypto\CryptoRegistry::createDefaultRegistry()); + $cryptoRegistry = \Civi\Crypto\CryptoRegistry::createDefaultRegistry(); + $cryptoToken = new \Civi\Crypto\CryptoToken($cryptoRegistry); foreach ($settings as $setting) { - $value = unserialize($setting['value']); - $plain = CRM_Utils_Crypt::decrypt($value['smtpPassword']); - $value['smtpPassword'] = $cryptoToken->encrypt($plain, 'CRED'); + $new = $old = unserialize($setting['value']); + if (!self::canBeStored($old['smtpPassword'], $cryptoRegistry)) { + $new['smtpPassword'] = ''; + } + else { + $plain = CRM_Utils_Crypt::decrypt($old['smtpPassword']); + $new['smtpPassword'] = $cryptoToken->encrypt($plain, 'CRED'); + } CRM_Core_DAO::executeQuery('UPDATE civicrm_setting SET value = %2 WHERE id = %1', [ 1 => [$setting['id'], 'Positive'], - 2 => [serialize($value), 'String'], + 2 => [serialize($new), 'String'], ]); } return TRUE; } + /** + * If you decode an old value of smtpPassword, will it be possible to store that + * password in the updated format? + * + * If you actually have encryption enabled, then it's straight-yes. But if you + * have to write in plain-text, then you're working within the constraints + * of php-mysqli-utf8mb4, and it does not accept anything > chr(128). + * + * Note: This could potentially change in the future if we updated `CryptToken` + * to put difficult strings into `^CTK?k=plain&t={base64}` format. + * + * @param string $oldCipherText + * @param \Civi\Crypto\CryptoRegistry $registry + * @return bool + * @throws \Civi\Crypto\Exception\CryptoException + */ + protected static function canBeStored($oldCipherText, \Civi\Crypto\CryptoRegistry $registry) { + $plainText = CRM_Utils_Crypt::decrypt($oldCipherText); + $activeKey = $registry->findKey('CRED'); + $isPrintable = ctype_print($plainText); + if ($activeKey['suite'] === 'plain' && !$isPrintable) { + return FALSE; + } + else { + return TRUE; + } + } + /** * Update financial type table to reflect recent schema changes. * -- 2.25.1