update to defuse/php-encryption 2.1

CHANGELOG.md

@@ -22,6 +22,7 @@ NOTE: Mail related Workflow methods changed signature, see #263
 - Use filename-normalizer for attachment filenames extracted from emails (@glensc, #356)
 - Fix Open Redirect vulnerability found by NetSparker (https://www.netsparker.com/, @balsdorf)
 - Add experimental markdown rendering (@glensc, #291)
+- Update to defuse/php-encryption 2.1 (@glensc, #358)
 
 [3.4.0]: https://github.com/eventum/eventum/compare/v3.3.5...master
 

composer.json

@@ -65,7 +65,7 @@
 		"ext-session": "*",
 		"ext-spl": "*",
 		"cebe/markdown": "~1.1.1",
-		"defuse/php-encryption": "~1.2.1",
+		"defuse/php-encryption": "^2.1",
 		"doctrine/dbal": "^2.5",
 		"doctrine/orm": "^2.5",
 		"enrise/urihelper": "^1.0",

db/migrations/20180316174428_eventum_crypto2_upgrade.php

@@ -0,0 +1,39 @@
+<?php
+
+/*
+ * This file is part of the Eventum (Issue Tracking System) package.
+ *
+ * @copyright (c) Eventum Team
+ * @license GNU General Public License, version 2 or later (GPL-2+)
+ *
+ * For the full copyright and license information,
+ * please see the COPYING and AUTHORS files
+ * that were distributed with this source code.
+ */
+
+use Eventum\Crypto\CryptoManager;
+use Eventum\Crypto\CryptoUpgradeManager;
+use Eventum\Db\AbstractMigration;
+
+class EventumCrypto2Upgrade extends AbstractMigration
+{
+    public function up()
+    {
+        if (CryptoManager::encryptionEnabled()) {
+            $cm = new CryptoUpgradeManager();
+            $cm->regenerateKey();
+        }
+    }
+
+    /**
+     * We can not migrate back to old key format,
+     * so just to keep installation usable, disable encryption.
+     */
+    public function down()
+    {
+        if (CryptoManager::encryptionEnabled()) {
+            $cm = new CryptoUpgradeManager();
+            $cm->disable();
+        }
+    }
+}

src/Crypto/CryptoException.php

@@ -13,8 +13,8 @@
 
 namespace Eventum\Crypto;
 
-use RuntimeException;
+use Defuse\Crypto\Exception\CryptoException as DefuseCryptoException;
 
-class CryptoException extends RuntimeException
+class CryptoException extends DefuseCryptoException
 {
 }

src/Crypto/CryptoKeyManager.php

@@ -13,8 +13,7 @@
 
 namespace Eventum\Crypto;
 
-use Crypto;
-use RandomLib;
+use Defuse\Crypto\Key;
 use Symfony\Component\Filesystem\Exception\IOException;
 use Symfony\Component\Filesystem\Filesystem;
 
@@ -26,6 +25,7 @@ final class CryptoKeyManager
     /** @var string */
     private $keyfile;
 
+    /** @var Key */
     private $key;
 
     public function __construct()
@@ -40,6 +40,7 @@ public function regen()
 
     /**
      * Checks if key file can be updated
+     * @throws CryptoException
      */
     public function canUpdate()
     {
@@ -51,7 +52,8 @@ public function canUpdate()
     /**
      * Load or generate secret key used for crypt
      *
-     * @return string
+     * @throws CryptoException
+     * @return Key
      */
     public function getKey()
     {
@@ -66,16 +68,23 @@ public function getKey()
         return $this->key;
     }
 
+    /**
+     * @throws CryptoException
+     */
     private function generateKey()
     {
-        // use RandomLib to get most compatible implementation
-        // Crypto uses mcrypt *ONLY* without any fallback
-        $factory = new RandomLib\Factory();
-        $generator = $factory->getMediumStrengthGenerator();
-        $this->key = $generator->generate(Crypto::KEY_BYTE_SIZE);
-        $this->storePrivateKey();
+        try {
+            $this->key = Key::createNewRandomKey();
+            $this->storePrivateKey();
+        } catch (CryptoException $e) {
+            throw new CryptoException('Cannot perform operation: ' . $e->getMessage());
+        }
     }
 
+    /**
+     * @throws CryptoException
+     * @return bool|null
+     */
     private function loadPrivateKey()
     {
         if (!file_exists($this->keyfile) || !filesize($this->keyfile)) {
@@ -84,22 +93,46 @@ private function loadPrivateKey()
         if (!is_readable($this->keyfile)) {
             throw new CryptoException("Secret file '{$this->keyfile}' not readable");
         }
-        $key = trim(file_get_contents($this->keyfile));
+
+        // load first to see that it's php script
+        // this would avoid printing secret key to output if in old or invalid format
+        $key = file_get_contents($this->keyfile);
         if (!$key) {
             throw new CryptoException("Unable to read secret file '{$this->keyfile}");
         }
-        $this->key = $key;
+
+        // support legacy key format
+        if (substr($key, 0, 5) !== '<?php') {
+            $this->key = $key;
+
+            return true;
+        }
+
+        $key = require $this->keyfile;
+        if (!$key) {
+            throw new CryptoException("Secret file corrupted: {$this->keyfile}");
+        }
+
+        try {
+            $this->key = Key::loadFromAsciiSafeString($key);
+        } catch (CryptoException $e) {
+            throw new CryptoException('Cannot perform operation: ' . $e->getMessage());
+        }
 
         return true;
     }
 
+    /**
+     * @throws CryptoException
+     */
     private function storePrivateKey()
     {
         $this->canUpdate();
 
         try {
             $fs = new Filesystem();
-            $fs->dumpFile($this->keyfile, $this->key);
+            $content = sprintf('<' . '?php return %s;', var_export($this->key->saveToAsciiSafeString(), 1));
+            $fs->dumpFile($this->keyfile, $content);
         } catch (IOException $e) {
             throw new CryptoException("Unable to store secret file '{$this->keyfile}': {$e->getMessage()}");
         }

src/Crypto/CryptoManager.php

@@ -13,11 +13,9 @@
 
 namespace Eventum\Crypto;
 
-use CannotPerformOperationException;
-use Crypto;
-use CryptoTestFailedException;
-use InvalidArgumentException;
-use InvalidCiphertextException;
+use Defuse\Crypto\Crypto;
+use Defuse\Crypto\Key;
+use Defuse\Crypto\RuntimeTests;
 use Setup;
 
 /**
@@ -33,14 +31,16 @@ final class CryptoManager
      */
     public static function encryptionEnabled()
     {
-        return Setup::get()->encryption == 'enabled';
+        return Setup::get()->encryption === 'enabled';
     }
 
     /**
      * Checks if system can perform encryption:
-     * - has mcrypt extension
+     * - has openssl extension
      * - some other tests performed by Crypto library
      *
+     * Checks for extensions presence because defuse/php-encryption is very cryptic about it errors.
+     *
      * @throws CryptoException if it can't be enabled
      * @return bool
      */
@@ -49,12 +49,9 @@ public static function canEncrypt()
         if (!function_exists('openssl_encrypt')) {
             throw new CryptoException('openssl extension not enabled');
         }
-        if (!function_exists('mcrypt_create_iv')) {
-            throw new CryptoException('mcrypt extension not enabled');
-        }
         try {
-            Crypto::RuntimeTest();
-        } catch (CryptoTestFailedException $e) {
+            RuntimeTests::runtimeTest();
+        } catch (CryptoException $e) {
             throw new CryptoException($e->getMessage(), $e->getCode(), $e);
         }
 
@@ -74,19 +71,17 @@ public static function canEncrypt()
     public static function encrypt($plaintext, $key = null)
     {
         if ($plaintext === null || $plaintext === false) {
-            throw new InvalidArgumentException('Refusing to encrypt empty value');
+            throw new CryptoException('Refusing to encrypt empty value');
         }
 
         if (!self::encryptionEnabled()) {
             return $plaintext;
         }
 
         try {
-            $ciphertext = Crypto::Encrypt($plaintext, $key ?: self::getKey());
-        } catch (CryptoTestFailedException $e) {
-            throw new CryptoException('Cannot safely perform encryption: Crypto test failed');
-        } catch (CannotPerformOperationException $e) {
-            throw new CryptoException('Cannot safely perform encryption: Cannot perform operation: ' . $e->getMessage());
+            $ciphertext = Crypto::encrypt($plaintext, $key ?: self::getKey(), true);
+        } catch (CryptoException $e) {
+            throw new CryptoException('Cannot perform operation: ' . $e->getMessage());
         }
 
         return rtrim(base64_encode($ciphertext), '=');
@@ -106,23 +101,23 @@ public static function decrypt($ciphertext)
             return $ciphertext;
         }
 
-        try {
-            $decrypted = Crypto::Decrypt(base64_decode($ciphertext), self::getKey());
-        } catch (InvalidCiphertextException $e) {
-            // VERY IMPORTANT
-            // Either:
-            //   1. The ciphertext was modified by the attacker,
-            //   2. The key is wrong, or
-            //   3. $ciphertext is not a valid ciphertext or was corrupted.
-            // Assume the worst.
-            throw new CryptoException('The ciphertext has been tampered with');
-        } catch (CryptoTestFailedException $e) {
-            throw new CryptoException('Cannot safely perform encryption: Crypto test failed');
-        } catch (CannotPerformOperationException $e) {
-            throw new CryptoException('Cannot safely perform encryption: Cannot perform operation: ' . $e->getMessage());
+        $ciphertext = base64_decode($ciphertext);
+        if (!$ciphertext) {
+            throw new CryptoException('Unable to decode ciphertext');
         }
 
-        return $decrypted;
+        try {
+            $key = self::getKey();
+
+            if ($key instanceof Key) {
+                return Crypto::decrypt($ciphertext, $key, true);
+            }
+
+            // support legacy decrypt
+            return Crypto::legacyDecrypt($ciphertext, $key);
+        } catch (CryptoException $e) {
+            throw new CryptoException('Cannot perform operation: ' . $e->getMessage());
+        }
     }
 
     private static function getKey()

src/Crypto/CryptoUpgradeManager.php

@@ -32,11 +32,12 @@ public function __construct()
      * Perform few checks that enable/disable can be performed
      *
      * @param bool $enable TRUE if action is to enable, FALSE if action is to disable
+     * @throws CryptoException
      */
     private function canPerform($enable)
     {
         $enabled = CryptoManager::encryptionEnabled();
-        if ($enabled && $enable || (!$enabled && !$enable)) {
+        if (($enabled && $enable) || (!$enabled && !$enable)) {
             $state = $enabled ? 'enabled' : 'disabled';
             throw new CryptoException("Can not perform, already $state");
         }