From d5a868ed4f07624bf9df76fd877ace54da85c093 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Sat, 16 Jan 2016 23:34:57 -0600 Subject: [PATCH 1/2] Crypt/Base: throw an exception if an IV is required but not defined --- phpseclib/Crypt/Base.php | 9 +++++++-- phpseclib/Crypt/RSA/PuTTY.php | 2 ++ tests/Unit/Crypt/AES/TestCase.php | 4 ++++ tests/Unit/Crypt/BlowfishTest.php | 1 + tests/Unit/Crypt/DESTest.php | 2 +- tests/Unit/Crypt/RC2Test.php | 1 + tests/Unit/Crypt/TripleDESTest.php | 2 ++ tests/Unit/Crypt/TwofishTest.php | 1 + 8 files changed, 19 insertions(+), 3 deletions(-) diff --git a/phpseclib/Crypt/Base.php b/phpseclib/Crypt/Base.php index cbc2b0cf..4c708a84 100644 --- a/phpseclib/Crypt/Base.php +++ b/phpseclib/Crypt/Base.php @@ -150,7 +150,7 @@ abstract class Base * @var string * @access private */ - var $iv; + var $iv = false; /** * A "sliding" Initialization Vector @@ -1871,13 +1871,18 @@ abstract class Base * after disableContinuousBuffer() or on cipher $engine (re)init * ie after setKey() or setIV() * - * @access public + * @access private * @internal Could, but not must, extend by the child Crypt_* class + * @throws \UnexpectedValueException when an IV is required but not defined */ function _clearBuffers() { $this->enbuffer = $this->debuffer = array('ciphertext' => '', 'xor' => '', 'pos' => 0, 'enmcrypt_init' => true); + if ($this->iv === false && !in_array($this->mode, array(self::MODE_STREAM, self::MODE_ECB))) { + throw new \UnexpectedValueException('No IV has been defined'); + } + // mcrypt's handling of invalid's $iv: // $this->encryptIV = $this->decryptIV = strlen($this->iv) == $this->block_size ? $this->iv : str_repeat("\0", $this->block_size); $this->encryptIV = $this->decryptIV = str_pad(substr($this->iv, 0, $this->block_size), $this->block_size, "\0"); diff --git a/phpseclib/Crypt/RSA/PuTTY.php b/phpseclib/Crypt/RSA/PuTTY.php index 5e8596db..38894a84 100644 --- a/phpseclib/Crypt/RSA/PuTTY.php +++ b/phpseclib/Crypt/RSA/PuTTY.php @@ -132,6 +132,7 @@ class PuTTY if ($encryption != 'none') { $crypto->setKey($symkey); + $crypto->setIV(str_repeat("\0", $crypto->getBlockLength() >> 3)); $crypto->disablePadding(); $private = $crypto->decrypt($private); if ($private === false) { @@ -263,6 +264,7 @@ class PuTTY $crypto = new AES(); $crypto->setKey(static::generateSymmetricKey($password, 32)); + $crypto->setIV(str_repeat("\0", $crypto->getBlockLength() >> 3)); $crypto->disablePadding(); $private = $crypto->encrypt($private); $hashkey = 'putty-private-key-file-mac-key' . $password; diff --git a/tests/Unit/Crypt/AES/TestCase.php b/tests/Unit/Crypt/AES/TestCase.php index ba890c78..20b161d3 100644 --- a/tests/Unit/Crypt/AES/TestCase.php +++ b/tests/Unit/Crypt/AES/TestCase.php @@ -104,6 +104,7 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase $aes->setPreferredEngine($this->engine); $aes->disablePadding(); $aes->setKey(pack('H*', '2b7e151628aed2a6abf7158809cf4f3c762e7160')); // 160-bit key. Valid in Rijndael. + $aes->setIV(str_repeat("\0", 16)); //$this->_checkEngine($aes); // should only work in internal mode $ciphertext = $aes->encrypt(pack('H*', '3243f6a8885a308d313198a2e0370734')); $this->assertEquals($ciphertext, pack('H*', '231d844639b31b412211cfe93712b880')); @@ -120,6 +121,7 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase $aes->setPreferredEngine($this->engine); $aes->disablePadding(); $aes->setKey(pack('H*', '2b7e151628aed2a6abf7158809cf4f3c762e7160')); // 160-bit key. AES should null pad to 192-bits + $aes->setIV(str_repeat("\0", 16)); $this->_checkEngine($aes); $ciphertext = $aes->encrypt(pack('H*', '3243f6a8885a308d313198a2e0370734')); $this->assertEquals($ciphertext, pack('H*', 'c109292b173f841b88e0ee49f13db8c0')); @@ -355,6 +357,7 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase $aes = new AES(); $aes->setKeyLength(128); $aes->setKey(str_repeat('a', 24)); + $aes->setIV(str_repeat("\0", 16)); $this->assertSame($aes->getKeyLength(), 128); $ciphertext = bin2hex($aes->encrypt('a')); $this->assertSame($ciphertext, '82b7b068dfc60ed2a46893b69fecd6c2'); @@ -366,6 +369,7 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase $aes = new AES(); $aes->setKeyLength(256); $aes->setKey(str_repeat('a', 16)); + $aes->setIV(str_repeat("\0", 16)); $this->assertSame($aes->getKeyLength(), 256); $ciphertext = bin2hex($aes->encrypt('a')); $this->assertSame($ciphertext, 'fd4250c0d234aa7e1aa592820aa8406b'); diff --git a/tests/Unit/Crypt/BlowfishTest.php b/tests/Unit/Crypt/BlowfishTest.php index bf15feb8..f5b45a42 100644 --- a/tests/Unit/Crypt/BlowfishTest.php +++ b/tests/Unit/Crypt/BlowfishTest.php @@ -75,6 +75,7 @@ class Unit_Crypt_BlowfishTest extends PhpseclibTestCase { $bf = new Blowfish(); $bf->setKey($key); + $bf->setIV(str_repeat("\0", $bf->getBlockLength() >> 3)); if (!$bf->isValidEngine($engine)) { self::markTestSkipped('Unable to initialize ' . $engineName . ' engine'); } diff --git a/tests/Unit/Crypt/DESTest.php b/tests/Unit/Crypt/DESTest.php index a1fbea6f..1f0eda9f 100644 --- a/tests/Unit/Crypt/DESTest.php +++ b/tests/Unit/Crypt/DESTest.php @@ -51,7 +51,7 @@ class Unit_Crypt_DESTest extends PhpseclibTestCase $des->disablePadding(); // when the key and iv are not specified they should be null padded //$des->setKey(); - //$des->setIV(); + $des->setIV(''); $des->setPreferredEngine(Base::ENGINE_INTERNAL); $internal = $des->decrypt('d'); diff --git a/tests/Unit/Crypt/RC2Test.php b/tests/Unit/Crypt/RC2Test.php index 04171ffc..f3a85725 100644 --- a/tests/Unit/Crypt/RC2Test.php +++ b/tests/Unit/Crypt/RC2Test.php @@ -114,6 +114,7 @@ class Unit_Crypt_RC2Test extends PhpseclibTestCase $rc2->disablePadding(); $rc2->setKeyLength($keyLen); $rc2->setKey(pack('H*', $key)); // could also do $rc2->setKey(pack('H*', $key), $keyLen) + $rc2->setIV(str_repeat("\0", $rc2->getBlockLength() >> 3)); if (!$rc2->isValidEngine($engine)) { self::markTestSkipped('Unable to initialize ' . $engineName . ' engine'); } diff --git a/tests/Unit/Crypt/TripleDESTest.php b/tests/Unit/Crypt/TripleDESTest.php index e41c4386..e2de3f2f 100644 --- a/tests/Unit/Crypt/TripleDESTest.php +++ b/tests/Unit/Crypt/TripleDESTest.php @@ -110,6 +110,7 @@ class Unit_Crypt_TripleDESTest extends PhpseclibTestCase } $des->setPreferredEngine($engine); $des->setKey($key); + $des->setIV(str_repeat("\0", $des->getBlockLength() >> 3)); $des->disablePadding(); $result = $des->encrypt($plaintext); $plaintext = bin2hex($plaintext); @@ -176,6 +177,7 @@ class Unit_Crypt_TripleDESTest extends PhpseclibTestCase $des = new TripleDES(TripleDES::MODE_3CBC); $des->setKey('abcdefghijklmnopqrstuvwx'); + $des->setIV(str_repeat("\0", $des->getBlockLength() >> 3)); foreach ($this->engines as $engine => $engineName) { $des->setPreferredEngine($engine); diff --git a/tests/Unit/Crypt/TwofishTest.php b/tests/Unit/Crypt/TwofishTest.php index f47443e9..6ebd84b1 100644 --- a/tests/Unit/Crypt/TwofishTest.php +++ b/tests/Unit/Crypt/TwofishTest.php @@ -20,6 +20,7 @@ class Unit_Crypt_TwofishTest extends PhpseclibTestCase foreach ($engines as $engine => $name) { $tf = new Twofish(); + $tf->setIV(str_repeat("\0", $tf->getBlockLength() >> 3)); $tf->disablePadding(); // tests from https://www.schneier.com/code/ecb_ival.txt From cea059e9dd9b290952bcd333221d705ae2cebccf Mon Sep 17 00:00:00 2001 From: terrafrost Date: Wed, 20 Jan 2016 10:09:06 -0600 Subject: [PATCH 2/2] Crypt/Base: updates to IV handling per Joey3000 --- phpseclib/Crypt/Base.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/phpseclib/Crypt/Base.php b/phpseclib/Crypt/Base.php index 4c708a84..63f2a5e4 100644 --- a/phpseclib/Crypt/Base.php +++ b/phpseclib/Crypt/Base.php @@ -500,10 +500,9 @@ abstract class Base } /** - * Sets the initialization vector. (optional) + * Sets the initialization vector. * - * SetIV is not required when self::MODE_ECB (or ie for AES: \phpseclib\Crypt\AES::MODE_ECB) is being used. If not explicitly set, it'll be assumed - * to be all zero's. + * setIV() is not required when self::MODE_ECB (or ie for AES: \phpseclib\Crypt\AES::MODE_ECB) is being used. * * @access public * @param string $iv @@ -511,8 +510,10 @@ abstract class Base */ function setIV($iv) { - if ($this->mode == self::MODE_ECB) { - return; + switch ($this->mode) { + case self::MODE_ECB: + case self::MODE_STREAM: + return; } $this->iv = $iv;