From e3c1d0e3cab836546659b83d3ccad6bd02a6f7ab Mon Sep 17 00:00:00 2001 From: Jared Hancock <jared@osticket.com> Date: Fri, 2 Aug 2013 09:51:36 -0500 Subject: [PATCH] Add a test suite for the crypto libraries Only use 8 bytes (64-bits) for the IV data (salt) for the encryption keys, except for Mcrypt, which requires you to query the specific length and use it specifically. --- include/class.crypto.php | 37 +++++++------ setup/test/tests/class.test.php | 26 +++++++++ setup/test/tests/test.crypto.php | 93 ++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 15 deletions(-) create mode 100644 setup/test/tests/test.crypto.php diff --git a/include/class.crypto.php b/include/class.crypto.php index 4b5a8754b..6ca0b2b08 100644 --- a/include/class.crypto.php +++ b/include/class.crypto.php @@ -268,6 +268,7 @@ endif; Class CryptoMcrypt extends CryptoAlgo { + # WARNING: Change and you will lose your passwords ... var $ciphers = array( CRYPTO_CIPHER_MCRYPT_RIJNDAEL_128 => array( 'name' => MCRYPT_RIJNDAEL_128, @@ -365,11 +366,9 @@ Class CryptoMcrypt extends CryptoAlgo { $keysize = mcrypt_enc_get_key_size($td); $ivsize = mcrypt_enc_get_iv_size($td); - if(strlen($ciphertext) <= $ivsize) - return false; - $iv = substr($ciphertext, 0, $ivsize); - $ciphertext = substr($ciphertext, $ivsize); + if (!($ciphertext = substr($ciphertext, $ivsize))) + return false; // Do the decryption. mcrypt_generic_init($td, $this->getKeyHash($iv, $ivsize), $iv); @@ -404,9 +403,11 @@ define('CRYPTO_CIPHER_OPENSSL_AES_128_CBC', 1); class CryptoOpenSSL extends CryptoAlgo { + # WARNING: Change and you will lose your passwords ... var $ciphers = array( CRYPTO_CIPHER_OPENSSL_AES_128_CBC => array( 'method' => 'aes-128-cbc', + 'seed' => 8 ), ); @@ -452,10 +453,12 @@ class CryptoOpenSSL extends CryptoAlgo { return false; $ivlen = openssl_cipher_iv_length($cipher['method']); - $iv = openssl_random_pseudo_bytes($ivlen); + $iv = openssl_random_pseudo_bytes($cipher['seed']); $key = $this->getKeyHash($iv, $ivlen); - if(!($ciphertext = openssl_encrypt($text, $cipher['method'], $key, 0, $iv))) + $options = (defined('OPENSSL_RAW_DATA')) ? OPENSSL_RAW_DATA : true; + if(!($ciphertext = openssl_encrypt($text, $cipher['method'], $key, + $options, $iv))) return false; return sprintf('$%s$%s%s', $cipher['cid'], $iv, $ciphertext); @@ -479,15 +482,17 @@ class CryptoOpenSSL extends CryptoAlgo { if(!$cid || !$ciphertext - || !($method=$this->getMethod($cid))) + || !($cipher=$this->getCipher($cid))) return false; - $ivlen = openssl_cipher_iv_length($method); - $iv = substr($ciphertext, 0, $ivlen); - $ciphertext = substr($ciphertext, $ivlen); + $ivlen = openssl_cipher_iv_length($cipher['method']); + $iv = substr($ciphertext, 0, $cipher['seed']); + $ciphertext = substr($ciphertext, $cipher['seed']); $key = $this->getKeyHash($iv, $ivlen); - $plaintext = openssl_decrypt($ciphertext, $method, $key, 0, $iv); + $options = (defined('OPENSSL_RAW_DATA')) ? OPENSSL_RAW_DATA : true; + $plaintext = openssl_decrypt($ciphertext, $cipher['method'], $key, + $options, $iv); return $plaintext; } @@ -518,7 +523,7 @@ class CryptoPHPSecLib extends CryptoAlgo { var $ciphers = array( //Replace with interface class CRYPTO_CIPHER_PHPSECLIB_AES_CBC => array( 'mode' => CRYPT_AES_MODE_CBC, - 'ivlen' => 16, + 'seed' => 8, 'class' => 'Crypt_AES', ), ); @@ -562,7 +567,7 @@ class CryptoPHPSecLib extends CryptoAlgo { ) return false; - $ivlen = $cipher['ivlen']; + $ivlen = $cipher['seed']; $iv = Crypto::randcode($ivlen); $crypto->setKey($this->getKeyHash($iv, $ivlen)); $crypto->setIV($iv); @@ -583,9 +588,11 @@ class CryptoPHPSecLib extends CryptoAlgo { ) return false; - $ivlen = $cipher['ivlen']; + $ivlen = $cipher['seed']; $iv = substr($ciphertext, 0, $ivlen); - $ciphertext = substr($ciphertext, $ivlen); + if (!($ciphertext = substr($ciphertext, $ivlen))) + return false; + $crypto->setKey($this->getKeyHash($iv, $ivlen)); $crypto->setIV($iv); diff --git a/setup/test/tests/class.test.php b/setup/test/tests/class.test.php index 4a82354cf..c7e2fb3fe 100644 --- a/setup/test/tests/class.test.php +++ b/setup/test/tests/class.test.php @@ -55,6 +55,32 @@ class Test { fputs(STDOUT, "."); } + function warn($message) { + $this->fails[] = array(get_class($this), '', '', 'WARNING: '.$message); + fputs(STDOUT, 'w'); + } + + function assert($expr, $message) { + if ($expr) + $this->pass(); + elseif ($message) + $this->fail('', '', $message); + else + $this->fail('', '', "assertion: {$a} != {$b}"); + } + + function assertEqual($a, $b, $message=false) { + if (!$message) + $message = "Assertion: {$a} != {$b}"; + return $this->assert($a == $b, $message); + } + + function assertNotEqual($a, $b, $message=false) { + if (!$message) + $message = "Assertion: {$a} == {$b}"; + return $this->assert($a != $b, $message); + } + function run() { $rc = new ReflectionClass(get_class($this)); foreach ($rc->getMethods() as $m) { diff --git a/setup/test/tests/test.crypto.php b/setup/test/tests/test.crypto.php new file mode 100644 index 000000000..f406e3097 --- /dev/null +++ b/setup/test/tests/test.crypto.php @@ -0,0 +1,93 @@ +<?php +require_once "class.test.php"; +define('INCLUDE_DIR', realpath(dirname(__file__).'/../../../include').'/'); +define('PEAR_DIR', INCLUDE_DIR.'/pear/'); +require_once INCLUDE_DIR."class.crypto.php"; + +class TestCrypto extends Test { + var $name = "Crypto library tests"; + + var $test_data = 'supercalifragilisticexpialidocious'; # notrans + var $master = 'master'; # notrans + + var $passwords = array( + 'english-password', + 'CaseSensitive Password', + '«ταБЬℓσ»', + '٩(-̮̮̃-̃)۶ ٩(●̮̮̃•̃)۶ ٩(͡๏̯͡๏)۶ ٩(-̮̮̃•̃).', + '发同讲说宅电的手机告的世全所回广讲说跟', + ); + + function testSimple() { + $tests = array_merge(array($this->test_data), $this->passwords); + foreach ($tests as $subject) { + $enc = Crypto::encrypt($subject, $this->master, 'simple'); + $dec = Crypto::decrypt($enc, $this->master, 'simple'); + $this->assertEqual($dec, $subject, + "{$subject}: Encryption failed closed loop"); + $this->assertNotEqual($enc, $subject, + 'Data was not encrypted'); + $this->assertNotEqual($enc, false, 'Encryption failed'); + $this->assertNotEqual($dec, false, 'Decryption failed'); + + $dec = Crypto::decrypt($enc, $this->master, 'wrong'); + $this->assertNotEqual($dec, $this->test_data, 'Subkeys are broken'); + } + } + + function _testLibrary($c, $tests) { + $name = get_class($c); + foreach ($tests as $id => $subject) { + $dec = $c->decrypt(base64_decode($subject)); + $this->assertEqual($dec, $this->test_data, "$name: decryption incorrect"); + $this->assertNotEqual($dec, false, "$name: decryption FAILED"); + } + $enc = $c->encrypt($this->test_data); + $this->assertNotEqual($enc, $this->test_data, + "$name: encryption cheaped out"); + $this->assertNotEqual($enc, false, "$name: encryption failed"); + + $c->setKeys($this->master, 'wrong'); + $dec = $c->decrypt(base64_decode($subject)); + $this->assertEqual($dec, false, "$name: Subkeys are broken"); + + } + + function testMcrypt() { + $tests = array( + 1 => 'JDEkIEDoeaSiOUEGE5KQ3UmJpQ5+pUaX91HyLMG58GmNU9pZXAdiXXJsfl+7TSDlLczGD98UCD6tLuDIwI9XJLEwew==', + ); + if (!CryptoMcrypt::exists()) + return $this->warn('Not testing mcrypt encryption'); + + $c = new CryptoMcrypt(0); + $c->setKeys($this->master, 'simple'); + $this->_testLibrary($c, $tests); + } + + function testOpenSSL() { + $tests = array( + 1 => 'JDEk4Wt4jRG460XnEIzKhTCKE9I0xfU3UadzF4rvlx++uCAOz0cQXDnRFX+VzHtgvfdabZ0FJ8T3e+M=', + ); + if (!CryptoOpenSSL::exists()) + return $this->warn('Not testing openssl encryption'); + + $c = new CryptoOpenSSL(0); + $c->setKeys($this->master, 'simple'); + $this->_testLibrary($c, $tests); + } + + function testPHPSecLib() { + $tests = array( + 1 => 'JDEkj42jvo2ADNoAGCvtbKoZfsVvFPGNNPDQrlcHOxQV9pjNRTJocsJhguJtjqajFTJX6rMuEVmMgrE=', + ); + if (!CryptoPHPSecLib::exists()) + return $this->warn('Not testing PHPSecLib encryption'); + + $c = new CryptoPHPSecLib(0); + $c->setKeys($this->master, 'simple'); + $this->_testLibrary($c, $tests); + } +} +return 'TestCrypto'; +?> -- GitLab