From e74841477fb39fd1e7d9449eab0e3204cef63713 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Mon, 25 Jan 2016 13:03:02 -0600 Subject: [PATCH] Crypt: become a lot less tolerant of bad parameters --- phpseclib/Crypt/AES.php | 42 +++++---- phpseclib/Crypt/Base.php | 131 +++++++++++++++++------------ phpseclib/Crypt/Blowfish.php | 26 ++++-- phpseclib/Crypt/DES.php | 28 ++++-- phpseclib/Crypt/RC2.php | 35 ++++++-- phpseclib/Crypt/RC4.php | 52 ++++++------ phpseclib/Crypt/RSA/PKCS.php | 2 +- phpseclib/Crypt/RSA/PKCS1.php | 2 +- phpseclib/Crypt/RSA/PKCS8.php | 2 +- phpseclib/Crypt/RSA/PuTTY.php | 2 +- phpseclib/Crypt/Rijndael.php | 89 ++++++++++++++------ phpseclib/Crypt/TripleDES.php | 68 ++++++++------- phpseclib/Crypt/Twofish.php | 52 ++++++++++-- phpseclib/Net/SSH1.php | 6 +- phpseclib/Net/SSH2.php | 28 +++--- tests/Unit/Crypt/AES/TestCase.php | 37 ++++---- tests/Unit/Crypt/BlowfishTest.php | 2 +- tests/Unit/Crypt/DESTest.php | 78 ----------------- tests/Unit/Crypt/RC2Test.php | 2 +- tests/Unit/Crypt/TripleDESTest.php | 4 +- tests/Unit/Crypt/TwofishTest.php | 2 +- 21 files changed, 389 insertions(+), 301 deletions(-) delete mode 100644 tests/Unit/Crypt/DESTest.php diff --git a/phpseclib/Crypt/AES.php b/phpseclib/Crypt/AES.php index 2bb4d5e8..87bae702 100644 --- a/phpseclib/Crypt/AES.php +++ b/phpseclib/Crypt/AES.php @@ -67,31 +67,32 @@ class AES extends Rijndael * * @see \phpseclib\Crypt\Rijndael::setBlockLength() * @access public - * @param int $length + * @throws \BadMethodCallException anytime it's called */ function setBlockLength($length) { - return; + throw new \BadMethodCallException('The block length cannot be set for AES.'); } /** * Sets the key length * - * Valid key lengths are 128, 192, and 256. If the length is less than 128, it will be rounded up to - * 128. If the length is greater than 128 and invalid, it will be rounded down to the closest valid amount. + * Valid key lengths are 128, 192, and 256. Set the link to bool(false) to disable a fixed key length * * @see \phpseclib\Crypt\Rijndael:setKeyLength() * @access public * @param int $length + * @throws \LengthException if the key length isn't supported */ function setKeyLength($length) { switch ($length) { - case 160: - $length = 192; + case 128: + case 192: + case 256: break; - case 224: - $length = 256; + default: + throw new \LengthException('Key of size ' . strlen($key) . ' not supported by this algorithm. Only keys of sizes 16, 24 or 32 supported'); } parent::setKeyLength($length); } @@ -105,24 +106,19 @@ class AES extends Rijndael * @see setKeyLength() * @access public * @param string $key + * @throws \LengthException if the key length isn't supported */ function setKey($key) { - parent::setKey($key); - - if (!$this->explicit_key_length) { - $length = strlen($key); - switch (true) { - case $length <= 16: - $this->key_length = 16; - break; - case $length <= 24: - $this->key_length = 24; - break; - default: - $this->key_length = 32; - } - $this->_setEngine(); + switch (strlen($key)) { + case 16: + case 24: + case 32: + break; + default: + throw new \LengthException('Key of size ' . strlen($key) . ' not supported by this algorithm. Only keys of sizes 16, 24 or 32 supported'); } + + parent::setKey($key); } } diff --git a/phpseclib/Crypt/Base.php b/phpseclib/Crypt/Base.php index 63f2a5e4..b134cd13 100644 --- a/phpseclib/Crypt/Base.php +++ b/phpseclib/Crypt/Base.php @@ -141,7 +141,7 @@ abstract class Base * @var string * @access private */ - var $key = "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"; + var $key = false; /** * The Initialization Vector @@ -431,15 +431,6 @@ abstract class Base */ var $openssl_options; - /** - * Has the key length explicitly been set or should it be derived from the key, itself? - * - * @see self::setKeyLength() - * @var bool - * @access private - */ - var $explicit_key_length = false; - /** * Don't truncate / null pad key * @@ -450,9 +441,16 @@ abstract class Base var $skip_key_adjustment = false; /** - * Default Constructor. + * Has the key length explicitly been set or should it be derived from the key, itself? * - * Determines whether or not the mcrypt extension should be used. + * @see self::setKeyLength() + * @var bool + * @access private + */ + var $explicit_key_length = false; + + /** + * Default Constructor. * * $mode could be: * @@ -466,32 +464,29 @@ abstract class Base * * - self::MODE_OFB * - * If not explicitly set, self::MODE_CBC will be used. - * * @param int $mode * @access public + * @throws \InvalidArgumentException if an invalid / unsupported mode is provided */ - function __construct($mode = self::MODE_CBC) + function __construct($mode) { // $mode dependent settings switch ($mode) { case self::MODE_ECB: + case self::MODE_CBC: $this->paddable = true; - $this->mode = self::MODE_ECB; break; case self::MODE_CTR: case self::MODE_CFB: case self::MODE_OFB: case self::MODE_STREAM: - $this->mode = $mode; + $this->paddable = false; break; - case self::MODE_CBC: default: - $this->paddable = true; - $this->mode = self::MODE_CBC; + throw new \InvalidArgumentException('No valid mode has been specified'); } - $this->_setEngine(); + $this->mode = $mode; // Determining whether inline crypting can be used by the cipher if ($this->use_inline_crypt !== false && function_exists('create_function')) { @@ -506,14 +501,22 @@ abstract class Base * * @access public * @param string $iv + * @throws \LengthException if the IV length isn't equal to the block size + * @throws \InvalidArgumentException if an IV is provided when one shouldn't be * @internal Can be overwritten by a sub class, but does not have to be */ function setIV($iv) { - switch ($this->mode) { - case self::MODE_ECB: - case self::MODE_STREAM: - return; + if ($this->mode == self::MODE_ECB) { + throw new \InvalidArgumentException('This mode does not require an IV.'); + } + + if ($this->mode == self::MODE_STREAM && $this->usesIV()) { + throw new \InvalidArgumentException('This algorithm does not use an IV.'); + } + + if (strlen($iv) != $this->block_size) { + throw new \LengthException('Received initialization vector of size ' . strlen($iv) . ', but size ' . $this->block_size . ' is required'); } $this->iv = $iv; @@ -521,18 +524,14 @@ abstract class Base } /** - * Sets the key length. - * - * Keys with explicitly set lengths need to be treated accordingly + * Returns whether or not the algorithm uses an IV * * @access public - * @param int $length + * @return bool */ - function setKeyLength($length) + function usesIV() { - $this->explicit_key_length = true; - $this->changed = true; - $this->_setEngine(); + return true; } /** @@ -557,6 +556,24 @@ abstract class Base return $this->block_size << 3; } + /** + * Sets the key length. + * + * Keys with explicitly set lengths need to be treated accordingly + * + * @access public + * @param int $length + */ + function setKeyLength($length) + { + $this->explicit_key_length = $length >> 3; + + if (is_string($this->key) && strlen($this->key) != $this->explicit_key_length) { + $this->key = false; + throw new \LengthException('Key has already been set and is not ' .$this->explicit_key_length . ' bytes long'); + } + } + /** * Sets the key. * @@ -573,12 +590,12 @@ abstract class Base */ function setKey($key) { - if (!$this->explicit_key_length) { - $this->setKeyLength(strlen($key) << 3); - $this->explicit_key_length = false; + if ($this->explicit_key_length !== false && strlen($key) != $this->explicit_key_length) { + throw new \LengthException('Key length has already been set to ' . $this->explicit_key_length . ' bytes and this key is ' . strlen($key) . ' bytes'); } $this->key = $key; + $this->key_length = strlen($key); $this->changed = true; $this->_setEngine(); } @@ -622,7 +639,8 @@ abstract class Base if (isset($func_args[5])) { $dkLen = $func_args[5]; } else { - $dkLen = $method == 'pbkdf1' ? 2 * $this->key_length : $this->key_length; + $key_length = $this->explicit_key_length !== false ? $this->explicit_key_length : $this->key_length; + $dkLen = $method == 'pbkdf1' ? 2 * $key_length : $key_length; } switch (true) { @@ -774,7 +792,7 @@ abstract class Base $this->changed = false; } if ($this->enchanged) { - mcrypt_generic_init($this->enmcrypt, $this->key, $this->encryptIV); + mcrypt_generic_init($this->enmcrypt, $this->key, $this->_getIV($this->encryptIV)); $this->enchanged = false; } @@ -837,7 +855,7 @@ abstract class Base $ciphertext = mcrypt_generic($this->enmcrypt, $plaintext); if (!$this->continuousBuffer) { - mcrypt_generic_init($this->enmcrypt, $this->key, $this->encryptIV); + mcrypt_generic_init($this->enmcrypt, $this->key, $this->_getIV($this->encryptIV)); } return $ciphertext; @@ -986,14 +1004,13 @@ abstract class Base * @access public * @param string $ciphertext * @return string $plaintext + * @throws \LengthException if we're inside a block cipher and the ciphertext length is not a multiple of the block size * @internal Could, but not must, extend by the child Crypt_* class */ function decrypt($ciphertext) { - if ($this->paddable) { - // we pad with chr(0) since that's what mcrypt_generic does. to quote from {@link http://www.php.net/function.mcrypt-generic}: - // "The data is padded with "\0" to make sure the length of the data is n * blocksize." - $ciphertext = str_pad($ciphertext, strlen($ciphertext) + ($this->block_size - strlen($ciphertext) % $this->block_size) % $this->block_size, chr(0)); + if ($this->paddable && strlen($ciphertext) % $this->block_size) { + throw new \LengthException('The ciphertext length (' . strlen($ciphertext) . ') needs to be a multiple of the block size (' . $this->block_size . ')'); } if ($this->engine === self::ENGINE_OPENSSL) { @@ -1083,7 +1100,7 @@ abstract class Base $this->changed = false; } if ($this->dechanged) { - mcrypt_generic_init($this->demcrypt, $this->key, $this->decryptIV); + mcrypt_generic_init($this->demcrypt, $this->key, $this->_getIV($this->decryptIV)); $this->dechanged = false; } @@ -1128,7 +1145,7 @@ abstract class Base $plaintext = mdecrypt_generic($this->demcrypt, $ciphertext); if (!$this->continuousBuffer) { - mcrypt_generic_init($this->demcrypt, $this->key, $this->decryptIV); + mcrypt_generic_init($this->demcrypt, $this->key, $this->_getIV($this->decryptIV)); } return $this->paddable ? $this->_unpad($plaintext) : $plaintext; @@ -1265,6 +1282,22 @@ abstract class Base return $this->paddable ? $this->_unpad($plaintext) : $plaintext; } + /** + * Get the IV + * + * mcrypt requires an IV even if ECB is used + * + * @see self::encrypt() + * @see self::decrypt() + * @param string $iv + * @return string + * @access private + */ + function _getIV($iv) + { + return $this->mode == self::MODE_ECB ? str_repeat("\0", $this->block_size) : $iv; + } + /** * OpenSSL CTR Processor * @@ -1884,13 +1917,7 @@ abstract class Base 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"); - - if (!$this->skip_key_adjustment) { - $this->key = str_pad(substr($this->key, 0, $this->key_length), $this->key_length, "\0"); - } + $this->encryptIV = $this->decryptIV = $this->iv; } /** diff --git a/phpseclib/Crypt/Blowfish.php b/phpseclib/Crypt/Blowfish.php index 138afe53..bb49dac2 100644 --- a/phpseclib/Crypt/Blowfish.php +++ b/phpseclib/Crypt/Blowfish.php @@ -285,6 +285,22 @@ class Blowfish extends Base */ var $key_length = 16; + /** + * Default Constructor. + * + * @param int $mode + * @access public + * @throws \InvalidArgumentException if an invalid / unsupported mode is provided + */ + function __construct($mode) + { + if ($mode == self::MODE_STREAM) { + throw new \InvalidArgumentException('Block ciphers cannot be ran in stream mode'); + } + + parent::__construct($mode); + } + /** * Sets the key length. * @@ -295,14 +311,12 @@ class Blowfish extends Base */ function setKeyLength($length) { - if ($length < 32) { - $this->key_length = 7; - } elseif ($length > 448) { - $this->key_length = 56; - } else { - $this->key_length = $length >> 3; + if ($length < 32 || $length > 448) { + throw new \LengthException('Key size of ' . $length . ' bits is not supported by this algorithm. Only keys of sizes between 32 and 448 bits are supported'); } + $this->key_length = $length >> 3; + parent::setKeyLength($length); } diff --git a/phpseclib/Crypt/DES.php b/phpseclib/Crypt/DES.php index 93e583a7..3397d1b1 100644 --- a/phpseclib/Crypt/DES.php +++ b/phpseclib/Crypt/DES.php @@ -580,6 +580,22 @@ class DES extends Base 0x00000820, 0x00020020, 0x08000000, 0x08020800 ); + /** + * Default Constructor. + * + * @param int $mode + * @access public + * @throws \InvalidArgumentException if an invalid / unsupported mode is provided + */ + function __construct($mode) + { + if ($mode == self::MODE_STREAM) { + throw new \InvalidArgumentException('Block ciphers cannot be ran in stream mode'); + } + + parent::__construct($mode); + } + /** * Test for engine validity * @@ -605,24 +621,18 @@ class DES extends Base /** * Sets the key. * - * Keys can be of any length. DES, itself, uses 64-bit keys (eg. strlen($key) == 8), however, we - * only use the first eight, if $key has more then eight characters in it, and pad $key with the - * null byte if it is less then eight characters long. + * Keys must be 64-bits long or 8 bytes long. * * DES also requires that every eighth bit be a parity bit, however, we'll ignore that. * - * If the key is not explicitly set, it'll be assumed to be all zero's. - * * @see \phpseclib\Crypt\Base::setKey() * @access public * @param string $key */ function setKey($key) { - // We check/cut here only up to max length of the key. - // Key padding to the proper length will be done in _setupKey() - if (strlen($key) > $this->key_length_max) { - $key = substr($key, 0, $this->key_length_max); + if (!($this instanceof TripleDES) && strlen($key) != 8) { + throw new \LengthException('Key of size ' . strlen($key) . ' not supported by this algorithm. Only keys of size 8 are supported'); } // Sets the key diff --git a/phpseclib/Crypt/RC2.php b/phpseclib/Crypt/RC2.php index bfe3bca4..13aeae4b 100644 --- a/phpseclib/Crypt/RC2.php +++ b/phpseclib/Crypt/RC2.php @@ -261,6 +261,22 @@ class RC2 extends Base 0x70, 0x02, 0xC2, 0x1E, 0xB8, 0x0A, 0xFC, 0xE6 ); + /** + * Default Constructor. + * + * @param int $mode + * @access public + * @throws \InvalidArgumentException if an invalid / unsupported mode is provided + */ + function __construct($mode) + { + if ($mode == self::MODE_STREAM) { + throw new \InvalidArgumentException('Block ciphers cannot be ran in stream mode'); + } + + parent::__construct($mode); + } + /** * Test for engine validity * @@ -294,12 +310,15 @@ class RC2 extends Base * * @access public * @param int $length in bits + * @throws \LengthException if the key length isn't supported */ function setKeyLength($length) { - if ($length >= 1 && $length <= 1024) { - $this->default_key_length = $length; + if ($length < 1 || $length > 1024) { + throw new \LengthException('Key size of ' . $length . ' bits is not supported by this algorithm. Only keys between 1 and 1024 bits, inclusive, are supported'); } + + $this->default_key_length = $length; } /** @@ -328,16 +347,20 @@ class RC2 extends Base * @access public * @param string $key * @param int $t1 optional Effective key length in bits. + * @throws \LengthException if the key length isn't supported */ - function setKey($key, $t1 = 0) + function setKey($key, $t1 = false) { $this->orig_key = $key; - if ($t1 <= 0) { + if ($t1 === false) { $t1 = $this->default_key_length; - } elseif ($t1 > 1024) { - $t1 = 1024; } + + if ($t1 < 1 || $t1 > 1024) { + throw new \LengthException('Key size of ' . $length . ' bits is not supported by this algorithm. Only keys between 1 and 1024 bits, inclusive, are supported'); + } + $this->current_key_length = $t1; // Key byte count should be 1..128. $key = strlen($key) ? substr($key, 0, 128) : "\x00"; diff --git a/phpseclib/Crypt/RC4.php b/phpseclib/Crypt/RC4.php index b24129d2..51625aae 100644 --- a/phpseclib/Crypt/RC4.php +++ b/phpseclib/Crypt/RC4.php @@ -123,8 +123,6 @@ class RC4 extends Base /** * Default Constructor. * - * Determines whether or not the mcrypt extension should be used. - * * @see \phpseclib\Crypt\Base::__construct() * @return \phpseclib\Crypt\RC4 * @access public @@ -167,26 +165,14 @@ class RC4 extends Base } /** - * Dummy function. + * RC4 does not use an IV * - * Some protocols, such as WEP, prepend an "initialization vector" to the key, effectively creating a new key [1]. - * If you need to use an initialization vector in this manner, feel free to prepend it to the key, yourself, before - * calling setKey(). - * - * [1] WEP's initialization vectors (IV's) are used in a somewhat insecure way. Since, in that protocol, - * the IV's are relatively easy to predict, an attack described by - * {@link http://www.drizzle.com/~aboba/IEEE/rc4_ksaproc.pdf Scott Fluhrer, Itsik Mantin, and Adi Shamir} - * can be used to quickly guess at the rest of the key. The following links elaborate: - * - * {@link http://www.rsa.com/rsalabs/node.asp?id=2009 http://www.rsa.com/rsalabs/node.asp?id=2009} - * {@link http://en.wikipedia.org/wiki/Related_key_attack http://en.wikipedia.org/wiki/Related_key_attack} - * - * @param string $iv - * @see self::setKey() * @access public + * @return bool */ - function setIV($iv) + function usesIV() { + return false; } /** @@ -196,20 +182,38 @@ class RC4 extends Base * * @access public * @param int $length + * @throws \LengthException if the key length is invalid */ function setKeyLength($length) { - if ($length < 8) { - $this->key_length = 1; - } elseif ($length > 2048) { - $this->key_length = 248; - } else { - $this->key_length = $length >> 3; + if ($length < 8 || $length > 2048) { + throw new \LengthException('Key size of ' . $length . ' bits is not supported by this algorithm. Only keys between 1 and 256 bytes are supported'); } + $this->key_length = $length >> 3; + parent::setKeyLength($length); } + /** + * Sets the key length + * + * Keys can be between 1 and 256 bytes long. + * + * @access public + * @param int $length + * @throws \LengthException if the key length is invalid + */ + function setKey($key) + { + $length = strlen($key); + if ($length < 1 || $length > 256) { + throw new \LengthException('Key size of ' . $length . ' bytes is not supported by RC4. Keys must be between 1 and 256 bytes long'); + } + + parent::setKey($key); + } + /** * Encrypts a message. * diff --git a/phpseclib/Crypt/RSA/PKCS.php b/phpseclib/Crypt/RSA/PKCS.php index 6f5ba33b..96c36d0b 100644 --- a/phpseclib/Crypt/RSA/PKCS.php +++ b/phpseclib/Crypt/RSA/PKCS.php @@ -264,7 +264,7 @@ abstract class PKCS return false; } - $crypto = new DES(); + $crypto = new DES(DES::MODE_CBC); $crypto->setPassword($password, 'pbkdf1', 'md5', $salt, $iterationCount); $key = $crypto->decrypt($key); if ($key === false) { diff --git a/phpseclib/Crypt/RSA/PKCS1.php b/phpseclib/Crypt/RSA/PKCS1.php index 8ce8b115..0bb9b8b4 100644 --- a/phpseclib/Crypt/RSA/PKCS1.php +++ b/phpseclib/Crypt/RSA/PKCS1.php @@ -115,7 +115,7 @@ class PKCS1 extends PKCS if (!empty($password) || is_string($password)) { $cipher = self::getEncryptionObject(self::$defaultEncryptionAlgorithm); $iv = Random::string($cipher->getBlockLength() >> 3); - $cipher->setKey(self::generateSymmetricKey($password, $iv, $cipher->getKeyLength())); + $cipher->setKey(self::generateSymmetricKey($password, $iv, $cipher->getKeyLength() >> 3)); $cipher->setIV($iv); $iv = strtoupper(bin2hex($iv)); $RSAPrivateKey = "-----BEGIN RSA PRIVATE KEY-----\r\n" . diff --git a/phpseclib/Crypt/RSA/PKCS8.php b/phpseclib/Crypt/RSA/PKCS8.php index 84a473d3..f94b81d4 100644 --- a/phpseclib/Crypt/RSA/PKCS8.php +++ b/phpseclib/Crypt/RSA/PKCS8.php @@ -108,7 +108,7 @@ class PKCS8 extends PKCS $salt = Random::string(8); $iterationCount = 2048; - $crypto = new DES(); + $crypto = new DES(DES::MODE_CBC); $crypto->setPassword($password, 'pbkdf1', 'md5', $salt, $iterationCount); $RSAPrivateKey = $crypto->encrypt($RSAPrivateKey); diff --git a/phpseclib/Crypt/RSA/PuTTY.php b/phpseclib/Crypt/RSA/PuTTY.php index 38894a84..6628846a 100644 --- a/phpseclib/Crypt/RSA/PuTTY.php +++ b/phpseclib/Crypt/RSA/PuTTY.php @@ -127,7 +127,7 @@ class PuTTY switch ($encryption) { case 'aes256-cbc': $symkey = static::generateSymmetricKey($password, 32); - $crypto = new AES(); + $crypto = new AES(AES::MODE_CBC); } if ($encryption != 'none') { diff --git a/phpseclib/Crypt/Rijndael.php b/phpseclib/Crypt/Rijndael.php index e9723932..ba656cd6 100644 --- a/phpseclib/Crypt/Rijndael.php +++ b/phpseclib/Crypt/Rijndael.php @@ -170,11 +170,26 @@ class Rijndael extends Base */ var $kl; + /** + * Default Constructor. + * + * @param int $mode + * @access public + * @throws \InvalidArgumentException if an invalid / unsupported mode is provided + */ + function __construct($mode) + { + if ($mode == self::MODE_STREAM) { + throw new \InvalidArgumentException('Block ciphers cannot be ran in stream mode'); + } + + parent::__construct($mode); + } + /** * Sets the key length. * - * Valid key lengths are 128, 160, 192, 224, and 256. If the length is less than 128, it will be rounded up to - * 128. If the length is greater than 128 and invalid, it will be rounded down to the closest valid amount. + * Valid key lengths are 128, 160, 192, 224, and 256. * * Note: phpseclib extends Rijndael (and AES) for using 160- and 224-bit keys but they are officially not defined * and the most (if not all) implementations are not able using 160/224-bit keys but round/pad them up to @@ -188,49 +203,75 @@ class Rijndael extends Base * This results then in slower encryption. * * @access public + * @throws \LengthException if the key length is invalid * @param int $length */ function setKeyLength($length) { - switch (true) { - case $length <= 128: - $this->key_length = 16; - break; - case $length <= 160: - $this->key_length = 20; - break; - case $length <= 192: - $this->key_length = 24; - break; - case $length <= 224: - $this->key_length = 28; + switch ($length) { + case 128: + case 160: + case 192: + case 224: + case 256: + $this->key_length = $length >> 3; break; default: - $this->key_length = 32; + throw new \LengthException('Key size of ' . $length . ' bits is not supported by this algorithm. Only keys of sizes 128, 160, 192, 224 or 256 bits are supported'); } parent::setKeyLength($length); } + /** + * Sets the key. + * + * Rijndael supports five different key lengths + * + * @see setKeyLength() + * @access public + * @param string $key + * @throws \LengthException if the key length isn't supported + */ + function setKey($key) + { + switch (strlen($key)) { + case 16: + case 20: + case 24: + case 28: + case 32: + break; + default: + throw new \LengthException('Key of size ' . strlen($key) . ' not supported by this algorithm. Only keys of sizes 16, 20, 24, 28 or 32 are supported'); + } + + parent::setKey($key); + } + /** * Sets the block length * - * Valid block lengths are 128, 160, 192, 224, and 256. If the length is less than 128, it will be rounded up to - * 128. If the length is greater than 128 and invalid, it will be rounded down to the closest valid amount. + * Valid block lengths are 128, 160, 192, 224, and 256. * * @access public * @param int $length */ function setBlockLength($length) { - $length >>= 5; - if ($length > 8) { - $length = 8; - } elseif ($length < 4) { - $length = 4; + switch ($length) { + case 128: + case 160: + case 192: + case 224: + case 256: + break; + default: + throw new \LengthException('Key size of ' . $length . ' bits is not supported by this algorithm. Only keys of sizes 128, 160, 192, 224 or 256 bits are supported'); } - $this->Nb = $length; - $this->block_size = $length << 2; + + $this->Nb = $length >> 5; + $this->block_size = $length >> 3; $this->changed = true; $this->_setEngine(); } diff --git a/phpseclib/Crypt/TripleDES.php b/phpseclib/Crypt/TripleDES.php index 91c5bc62..b0070114 100644 --- a/phpseclib/Crypt/TripleDES.php +++ b/phpseclib/Crypt/TripleDES.php @@ -131,7 +131,7 @@ class TripleDES extends DES /** * Default Constructor. * - * Determines whether or not the mcrypt extension should be used. + * Determines whether or not the mcrypt or OpenSSL extensions should be used. * * $mode could be: * @@ -145,16 +145,14 @@ class TripleDES extends DES * * - \phpseclib\Crypt\Base::MODE_OFB * - * - \phpseclib\Crypt\TripleDES::MODE_3CBC - * - * If not explicitly set, \phpseclib\Crypt\Base::MODE_CBC will be used. + * - \phpseclib\Crypt\TripleDES::MODE_3CB * * @see \phpseclib\Crypt\DES::__construct() * @see \phpseclib\Crypt\Base::__construct() * @param int $mode * @access public */ - function __construct($mode = Base::MODE_CBC) + function __construct($mode) { switch ($mode) { // In case of self::MODE_3CBC, we init as CRYPT_DES_MODE_CBC @@ -203,10 +201,9 @@ class TripleDES extends DES } /** - * Sets the initialization vector. (optional) + * Sets the initialization vector. * - * SetIV is not required when \phpseclib\Crypt\Base::MODE_ECB is being used. If not explicitly set, it'll be assumed - * to be all zero's. + * SetIV is not required when \phpseclib\Crypt\Base::MODE_ECB is being used. * * @see \phpseclib\Crypt\Base::setIV() * @access public @@ -225,24 +222,23 @@ class TripleDES extends DES /** * Sets the key length. * - * Valid key lengths are 64, 128 and 192 + * Valid key lengths are 128 and 192 bits. + * + * If you want to use a 64-bit key use DES.php * * @see \phpseclib\Crypt\Base:setKeyLength() * @access public + * @throws \LengthException if the key length is invalid * @param int $length */ function setKeyLength($length) { - $length >>= 3; - switch (true) { - case $length <= 8: - $this->key_length = 8; - break; - case $length <= 16: - $this->key_length = 16; + switch ($length) { + case 128: + case 192: break; default: - $this->key_length = 24; + throw new \LengthException('Key size of ' . $length . ' bits is not supported by this algorithm. Only keys of sizes 128 or 192 bits are supported'); } parent::setKeyLength($length); @@ -251,36 +247,38 @@ class TripleDES extends DES /** * Sets the key. * - * Keys can be of any length. Triple DES, itself, can use 128-bit (eg. strlen($key) == 16) or - * 192-bit (eg. strlen($key) == 24) keys. This function pads and truncates $key as appropriate. + * Triple DES can use 128-bit (eg. strlen($key) == 16) or 192-bit (eg. strlen($key) == 24) keys. * * DES also requires that every eighth bit be a parity bit, however, we'll ignore that. * - * If the key is not explicitly set, it'll be assumed to be all null bytes. - * * @access public * @see \phpseclib\Crypt\DES::setKey() * @see \phpseclib\Crypt\Base::setKey() + * @throws \LengthException if the key length is invalid * @param string $key */ function setKey($key) { - $length = $this->explicit_key_length ? $this->key_length : strlen($key); - if ($length > 8) { - $key = str_pad(substr($key, 0, 24), 24, chr(0)); - // if $key is between 64 and 128-bits, use the first 64-bits as the last, per this: - // http://php.net/function.mcrypt-encrypt#47973 - $key = $length <= 16 ? substr_replace($key, substr($key, 0, 8), 16) : substr($key, 0, 24); - } else { - $key = str_pad($key, 8, chr(0)); + if ($this->explicit_key_length !== false && strlen($key) != $this->explicit_key_length) { + throw new \LengthException('Key length has already been set to ' . $this->explicit_key_length . ' bytes and this key is ' . strlen($key) . ' bytes'); } - parent::setKey($key); - // And in case of self::MODE_3CBC: - // if key <= 64bits we not need the 3 $des to work, - // because we will then act as regular DES-CBC with just a <= 64bit key. - // So only if the key > 64bits (> 8 bytes) we will call setKey() for the 3 $des. - if ($this->mode_3cbc && $length > 8) { + switch (strlen($key)) { + case 16: + $key.= substr($key, 0, 8); + case 24: + break; + default: + throw new \LengthException('Key of size ' . strlen($key) . ' not supported by this algorithm. Only keys of sizes 16 or 24 are supported'); + } + + // copied from Base::setKey() + $this->key = $key; + $this->key_length = strlen($key); + $this->changed = true; + $this->_setEngine(); + + if ($this->mode_3cbc) { $this->des[0]->setKey(substr($key, 0, 8)); $this->des[1]->setKey(substr($key, 8, 8)); $this->des[2]->setKey(substr($key, 16, 8)); diff --git a/phpseclib/Crypt/Twofish.php b/phpseclib/Crypt/Twofish.php index 623e9a6a..4aad1a4d 100644 --- a/phpseclib/Crypt/Twofish.php +++ b/phpseclib/Crypt/Twofish.php @@ -370,6 +370,22 @@ class Twofish extends Base */ var $key_length = 16; + /** + * Default Constructor. + * + * @param int $mode + * @access public + * @throws \InvalidArgumentException if an invalid / unsupported mode is provided + */ + function __construct($mode) + { + if ($mode == self::MODE_STREAM) { + throw new \InvalidArgumentException('Block ciphers cannot be ran in stream mode'); + } + + parent::__construct($mode); + } + /** * Sets the key length. * @@ -380,20 +396,42 @@ class Twofish extends Base */ function setKeyLength($length) { - switch (true) { - case $length <= 128: - $this->key_length = 16; - break; - case $length <= 192: - $this->key_length = 24; + switch ($length) { + case 128: + case 192: + case 256: break; default: - $this->key_length = 32; + throw new \LengthException('Key of size ' . strlen($key) . ' not supported by this algorithm. Only keys of sizes 16, 24 or 32 supported'); } parent::setKeyLength($length); } + /** + * Sets the key. + * + * Rijndael supports five different key lengths + * + * @see setKeyLength() + * @access public + * @param string $key + * @throws \LengthException if the key length isn't supported + */ + function setKey($key) + { + switch (strlen($key)) { + case 16: + case 24: + case 32: + break; + default: + throw new \LengthException('Key of size ' . strlen($key) . ' not supported by this algorithm. Only keys of sizes 16, 24 or 32 supported'); + } + + parent::setKey($key); + } + /** * Setup the key (expansion) * diff --git a/phpseclib/Net/SSH1.php b/phpseclib/Net/SSH1.php index f1c53492..6f2dcadc 100644 --- a/phpseclib/Net/SSH1.php +++ b/phpseclib/Net/SSH1.php @@ -658,16 +658,20 @@ class SSH1 // $this->crypto = new \phpseclib\Crypt\Null(); // break; case self::CIPHER_DES: - $this->crypto = new DES(); + $this->crypto = new DES(DES::MODE_CBC); $this->crypto->disablePadding(); $this->crypto->enableContinuousBuffer(); $this->crypto->setKey(substr($session_key, 0, 8)); + // "The iv (initialization vector) is initialized to all zeroes." + $this->crypto->setIV(str_repeat("\0", 8)); break; case self::CIPHER_3DES: $this->crypto = new TripleDES(TripleDES::MODE_3CBC); $this->crypto->disablePadding(); $this->crypto->enableContinuousBuffer(); $this->crypto->setKey(substr($session_key, 0, 24)); + // "All three initialization vectors are initialized to zero." + $this->crypto->setIV(str_repeat("\0", 8)); break; //case self::CIPHER_RC4: // $this->crypto = new RC4(); diff --git a/phpseclib/Net/SSH2.php b/phpseclib/Net/SSH2.php index 59ce16e0..e809b385 100644 --- a/phpseclib/Net/SSH2.php +++ b/phpseclib/Net/SSH2.php @@ -1616,11 +1616,13 @@ class SSH2 $this->encrypt->enableContinuousBuffer(); $this->encrypt->disablePadding(); - $iv = $kexHash->hash($keyBytes . $this->exchange_hash . 'A' . $this->session_id); - while ($this->encrypt_block_size > strlen($iv)) { - $iv.= $kexHash->hash($keyBytes . $this->exchange_hash . $iv); + if ($this->encrypt->usesIV()) { + $iv = $kexHash->hash($keyBytes . $this->exchange_hash . 'A' . $this->session_id); + while ($this->encrypt_block_size > strlen($iv)) { + $iv.= $kexHash->hash($keyBytes . $this->exchange_hash . $iv); + } + $this->encrypt->setIV(substr($iv, 0, $this->encrypt_block_size)); } - $this->encrypt->setIV(substr($iv, 0, $this->encrypt_block_size)); $key = $kexHash->hash($keyBytes . $this->exchange_hash . 'C' . $this->session_id); while ($encryptKeyLength > strlen($key)) { @@ -1640,11 +1642,13 @@ class SSH2 $this->decrypt->enableContinuousBuffer(); $this->decrypt->disablePadding(); - $iv = $kexHash->hash($keyBytes . $this->exchange_hash . 'B' . $this->session_id); - while ($this->decrypt_block_size > strlen($iv)) { - $iv.= $kexHash->hash($keyBytes . $this->exchange_hash . $iv); + if ($this->decrypt->usesIV()) { + $iv = $kexHash->hash($keyBytes . $this->exchange_hash . 'B' . $this->session_id); + while ($this->decrypt_block_size > strlen($iv)) { + $iv.= $kexHash->hash($keyBytes . $this->exchange_hash . $iv); + } + $this->decrypt->setIV(substr($iv, 0, $this->decrypt_block_size)); } - $this->decrypt->setIV(substr($iv, 0, $this->decrypt_block_size)); $key = $kexHash->hash($keyBytes . $this->exchange_hash . 'D' . $this->session_id); while ($decryptKeyLength > strlen($key)) { @@ -1811,26 +1815,26 @@ class SSH2 { switch ($algorithm) { case '3des-cbc': - return new TripleDES(); + return new TripleDES(Base::MODE_CBC); case '3des-ctr': return new TripleDES(Base::MODE_CTR); case 'aes256-cbc': case 'aes192-cbc': case 'aes128-cbc': - return new Rijndael(); + return new Rijndael(Base::MODE_CBC); case 'aes256-ctr': case 'aes192-ctr': case 'aes128-ctr': return new Rijndael(Base::MODE_CTR); case 'blowfish-cbc': - return new Blowfish(); + return new Blowfish(Base::MODE_CBC); case 'blowfish-ctr': return new Blowfish(Base::MODE_CTR); case 'twofish128-cbc': case 'twofish192-cbc': case 'twofish256-cbc': case 'twofish-cbc': - return new Twofish(); + return new Twofish(Base::MODE_CBC); case 'twofish128-ctr': case 'twofish192-ctr': case 'twofish256-ctr': diff --git a/tests/Unit/Crypt/AES/TestCase.php b/tests/Unit/Crypt/AES/TestCase.php index 20b161d3..426ad7d9 100644 --- a/tests/Unit/Crypt/AES/TestCase.php +++ b/tests/Unit/Crypt/AES/TestCase.php @@ -47,13 +47,13 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase ':-):-):-):-):-):-)', // https://github.com/phpseclib/phpseclib/pull/43 ); $ivs = array( - '', - 'test123', + str_repeat("\0", 16), + str_pad('test123', 16, "\0"), ); $keys = array( - '', - ':-8', // https://github.com/phpseclib/phpseclib/pull/43 - 'FOOBARZ', + str_repeat("\0", 16), + str_pad(':-8', 16, "\0"), // https://github.com/phpseclib/phpseclib/pull/43 + str_pad('FOOBARZ', 16, "\0"), ); $result = array(); @@ -100,7 +100,7 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase // this test case is from the following URL: // https://web.archive.org/web/20070209120224/http://fp.gladman.plus.com/cryptography_technology/rijndael/aesdvec.zip - $aes = new Rijndael(); + $aes = new Rijndael(Base::MODE_CBC); $aes->setPreferredEngine($this->engine); $aes->disablePadding(); $aes->setKey(pack('H*', '2b7e151628aed2a6abf7158809cf4f3c762e7160')); // 160-bit key. Valid in Rijndael. @@ -112,15 +112,16 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase /** * @group github451 + * @expectedException \LengthException */ public function testKeyPaddingAES() { // same as the above - just with a different ciphertext - $aes = new AES(); + $aes = new AES(Base::MODE_CBC); $aes->setPreferredEngine($this->engine); $aes->disablePadding(); - $aes->setKey(pack('H*', '2b7e151628aed2a6abf7158809cf4f3c762e7160')); // 160-bit key. AES should null pad to 192-bits + $aes->setKey(pack('H*', '2b7e151628aed2a6abf7158809cf4f3c762e7160')); // 160-bit key. supported by Rijndael - not AES $aes->setIV(str_repeat("\0", 16)); $this->_checkEngine($aes); $ciphertext = $aes->encrypt(pack('H*', '3243f6a8885a308d313198a2e0370734')); @@ -266,7 +267,7 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase // from http://csrc.nist.gov/groups/STM/cavp/documents/aes/AESAVS.pdf#page=16 public function testGFSBox128() { - $aes = new AES(); + $aes = new AES(Base::MODE_CBC); $aes->setKey(pack('H*', '00000000000000000000000000000000')); $aes->setIV(pack('H*', '00000000000000000000000000000000')); @@ -293,7 +294,7 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase public function testGFSBox192() { - $aes = new AES(); + $aes = new AES(Base::MODE_CBC); $aes->setKey(pack('H*', '000000000000000000000000000000000000000000000000')); $aes->setIV(pack('H*', '00000000000000000000000000000000')); @@ -318,7 +319,7 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase public function testGFSBox256() { - $aes = new AES(); + $aes = new AES(Base::MODE_CBC); $aes->setKey(pack('H*', '00000000000000000000000000000000' . '00000000000000000000000000000000')); $aes->setIV(pack('H*', '00000000000000000000000000000000')); @@ -341,20 +342,23 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase public function testGetKeyLengthDefault() { - $aes = new AES(); + $aes = new AES(Base::MODE_CBC); $this->assertSame($aes->getKeyLength(), 128); } public function testGetKeyLengthWith192BitKey() { - $aes = new AES(); + $aes = new AES(Base::MODE_CBC); $aes->setKey(str_repeat('a', 24)); $this->assertSame($aes->getKeyLength(), 192); } + /** + * @expectedException \LengthException + */ public function testSetKeyLengthWithLargerKey() { - $aes = new AES(); + $aes = new AES(Base::MODE_CBC); $aes->setKeyLength(128); $aes->setKey(str_repeat('a', 24)); $aes->setIV(str_repeat("\0", 16)); @@ -364,9 +368,12 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase $this->assertSame($aes->getKeyLength(), 128); } + /** + * @expectedException \LengthException + */ public function testSetKeyLengthWithSmallerKey() { - $aes = new AES(); + $aes = new AES(Base::MODE_CBC); $aes->setKeyLength(256); $aes->setKey(str_repeat('a', 16)); $aes->setIV(str_repeat("\0", 16)); diff --git a/tests/Unit/Crypt/BlowfishTest.php b/tests/Unit/Crypt/BlowfishTest.php index f5b45a42..4d095664 100644 --- a/tests/Unit/Crypt/BlowfishTest.php +++ b/tests/Unit/Crypt/BlowfishTest.php @@ -73,7 +73,7 @@ class Unit_Crypt_BlowfishTest extends PhpseclibTestCase */ public function testVectors($engine, $engineName, $key, $plaintext, $expected) { - $bf = new Blowfish(); + $bf = new Blowfish(Blowfish::MODE_CBC); $bf->setKey($key); $bf->setIV(str_repeat("\0", $bf->getBlockLength() >> 3)); if (!$bf->isValidEngine($engine)) { diff --git a/tests/Unit/Crypt/DESTest.php b/tests/Unit/Crypt/DESTest.php deleted file mode 100644 index 1f0eda9f..00000000 --- a/tests/Unit/Crypt/DESTest.php +++ /dev/null @@ -1,78 +0,0 @@ - - * @copyright MMXIII Andreas Fischer - * @license http://www.opensource.org/licenses/mit-license.html MIT License - */ - -use phpseclib\Crypt\Base; -use phpseclib\Crypt\DES; - -// the AES tests establish the correctness of the modes of operation. this test is inteded to establish the consistency of -// key and iv padding between the multiple engines -class Unit_Crypt_DESTest extends PhpseclibTestCase -{ - public function testEncryptPadding() - { - $des = new DES(Base::MODE_CBC); - $des->setKey('d'); - $des->setIV('d'); - - $des->setPreferredEngine(Base::ENGINE_INTERNAL); - - $result = pack('H*', '3e7613642049af1e'); - - $internal = $des->encrypt('d'); - $this->assertEquals($result, $internal, 'Failed asserting that the internal engine produced the correct result'); - - $des->setPreferredEngine(Base::ENGINE_MCRYPT); - if ($des->getEngine() == Base::ENGINE_MCRYPT) { - $mcrypt = $des->encrypt('d'); - $this->assertEquals($result, $mcrypt, 'Failed asserting that the mcrypt engine produced the correct result'); - } else { - self::markTestSkipped('Unable to initialize mcrypt engine'); - } - - $des->setPreferredEngine(Base::ENGINE_OPENSSL); - if ($des->getEngine() == Base::ENGINE_OPENSSL) { - $openssl = $des->encrypt('d'); - $this->assertEquals($result, $openssl, 'Failed asserting that the OpenSSL engine produced the correct result'); - } else { - self::markTestSkipped('Unable to initialize OpenSSL engine'); - } - } - - // phpseclib null pads ciphertext's if they're not long enough and you're in ecb / cbc mode. this silent failure mode is consistent - // with mcrypt's behavior. maybe throwing an exception would be better but whatever. this test is more intended to establish consistent - // behavior between the various engine's - public function testDecryptPadding() - { - $des = new DES(Base::MODE_CBC); - $des->disablePadding(); - // when the key and iv are not specified they should be null padded - //$des->setKey(); - $des->setIV(''); - - $des->setPreferredEngine(Base::ENGINE_INTERNAL); - $internal = $des->decrypt('d'); - - $result = pack('H*', '79b305d1ce555221'); - $this->assertEquals($result, $internal, 'Failed asserting that the internal engine produced the correct result'); - - $des->setPreferredEngine(Base::ENGINE_MCRYPT); - if ($des->getEngine() == Base::ENGINE_MCRYPT) { - $mcrypt = $des->decrypt('d'); - $this->assertEquals($result, $mcrypt, 'Failed asserting that the mcrypt engine produced the correct result'); - } else { - self::markTestSkipped('Unable to initialize mcrypt engine'); - } - - $des->setPreferredEngine(Base::ENGINE_OPENSSL); - if ($des->getEngine() == Base::ENGINE_OPENSSL) { - $openssl = $des->decrypt('d'); - $this->assertEquals($result, $openssl, 'Failed asserting that the OpenSSL engine produced the correct result'); - } else { - self::markTestSkipped('Unable to initialize OpenSSL engine'); - } - } -} diff --git a/tests/Unit/Crypt/RC2Test.php b/tests/Unit/Crypt/RC2Test.php index f3a85725..e8606404 100644 --- a/tests/Unit/Crypt/RC2Test.php +++ b/tests/Unit/Crypt/RC2Test.php @@ -110,7 +110,7 @@ class Unit_Crypt_RC2Test extends PhpseclibTestCase */ public function testVectors($engine, $engineName, $key, $keyLen, $plaintext, $ciphertext) { - $rc2 = new RC2(); + $rc2 = new RC2(RC2::MODE_CBC); $rc2->disablePadding(); $rc2->setKeyLength($keyLen); $rc2->setKey(pack('H*', $key)); // could also do $rc2->setKey(pack('H*', $key), $keyLen) diff --git a/tests/Unit/Crypt/TripleDESTest.php b/tests/Unit/Crypt/TripleDESTest.php index e2de3f2f..2ed8f321 100644 --- a/tests/Unit/Crypt/TripleDESTest.php +++ b/tests/Unit/Crypt/TripleDESTest.php @@ -104,7 +104,7 @@ class Unit_Crypt_TripleDESTest extends PhpseclibTestCase */ public function testVectors($engine, $engineName, $key, $plaintext, $expected) { - $des = new TripleDES(); + $des = new TripleDES(TripleDES::MODE_CBC); if (!$des->isValidEngine($engine)) { self::markTestSkipped('Unable to initialize ' . $engineName . ' engine'); } @@ -156,7 +156,7 @@ class Unit_Crypt_TripleDESTest extends PhpseclibTestCase */ public function testVectorsWithIV($engine, $engineName, $key, $iv, $plaintext, $expected) { - $des = new TripleDES(); + $des = new TripleDES(TripleDES::MODE_CBC); if (!$des->isValidEngine($engine)) { self::markTestSkipped('Unable to initialize ' . $engineName . ' engine'); } diff --git a/tests/Unit/Crypt/TwofishTest.php b/tests/Unit/Crypt/TwofishTest.php index 6ebd84b1..d678c9e1 100644 --- a/tests/Unit/Crypt/TwofishTest.php +++ b/tests/Unit/Crypt/TwofishTest.php @@ -19,7 +19,7 @@ class Unit_Crypt_TwofishTest extends PhpseclibTestCase ); foreach ($engines as $engine => $name) { - $tf = new Twofish(); + $tf = new Twofish(Twofish::MODE_CBC); $tf->setIV(str_repeat("\0", $tf->getBlockLength() >> 3)); $tf->disablePadding();