From bfba3db1a70dc234b87fbca7d382607ae378fed1 Mon Sep 17 00:00:00 2001 From: terrafrost Date: Thu, 17 Sep 2015 10:46:45 -0500 Subject: [PATCH] Crypt: make it so the key length can be explicitly set for all ciphers --- phpseclib/Crypt/Base.php | 55 +++++++++++++++++- phpseclib/Crypt/Blowfish.php | 52 ++++++++--------- phpseclib/Crypt/DES.php | 17 +----- phpseclib/Crypt/RC2.php | 32 +++++++++-- phpseclib/Crypt/RC4.php | 27 +++++---- phpseclib/Crypt/Rijndael.php | 92 +++---------------------------- phpseclib/Crypt/TripleDES.php | 36 ++++++++++-- phpseclib/Crypt/Twofish.php | 32 ++++------- tests/Unit/Crypt/AES/TestCase.php | 35 ++++++++++++ 9 files changed, 208 insertions(+), 170 deletions(-) diff --git a/phpseclib/Crypt/Base.php b/phpseclib/Crypt/Base.php index 9566f288..e3840e00 100644 --- a/phpseclib/Crypt/Base.php +++ b/phpseclib/Crypt/Base.php @@ -470,6 +470,24 @@ class Crypt_Base */ var $openssl_options; + /** + * Has the key length explicitly been set or should it be derived from the key, itself? + * + * @see setKeyLength() + * @var bool + * @access private + */ + var $explicit_key_length = false; + + /** + * Don't truncate / null pad key + * + * @see Crypt_Base::_clearBuffers + * @var bool + * @access private + */ + var $skip_key_adjustment = false; + /** * Default Constructor. * @@ -542,6 +560,32 @@ class Crypt_Base $this->changed = true; } + /** + * 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 = true; + $this->changed = true; + $this->_setEngine(); + } + + /** + * Returns the current key length + * + * @access public + * @return int + */ + function getKeyLength() + { + return $this->key_size << 3; + } + /** * Sets the key. * @@ -558,6 +602,11 @@ class Crypt_Base */ function setKey($key) { + if (!$this->explicit_key_length) { + $this->setKeyLength(strlen($key) << 3); + $this->explicit_key_length = false; + } + $this->key = $key; $this->changed = true; $this->_setEngine(); @@ -601,7 +650,7 @@ class Crypt_Base if (isset($func_args[5])) { $dkLen = $func_args[5]; } else { - $dkLen = $method == 'pbkdf1' ? 2 * $this->password_key_size : $this->password_key_size; + $dkLen = $method == 'pbkdf1' ? 2 * $this->key_size : $this->key_size; } switch (true) { @@ -1873,6 +1922,10 @@ class Crypt_Base // 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_size), $this->key_size, "\0"); + } } /** diff --git a/phpseclib/Crypt/Blowfish.php b/phpseclib/Crypt/Blowfish.php index 943e4476..c5db8484 100644 --- a/phpseclib/Crypt/Blowfish.php +++ b/phpseclib/Crypt/Blowfish.php @@ -120,16 +120,6 @@ class Crypt_Blowfish extends Crypt_Base */ var $block_size = 8; - /** - * The default password key_size used by setPassword() - * - * @see Crypt_Base::password_key_size - * @see Crypt_Base::setPassword() - * @var int - * @access private - */ - var $password_key_size = 56; - /** * The namespace used by the cipher for its constants. * @@ -354,31 +344,37 @@ class Crypt_Blowfish extends Crypt_Base var $kl; /** - * Sets the key. + * The Key Length * - * Keys can be of any length. Blowfish, itself, requires the use of a key between 32 and max. 448-bits long. - * If the key is less than 32-bits we NOT fill the key to 32bit but let the key as it is to be compatible - * with mcrypt because mcrypt act this way with blowfish key's < 32 bits. + * @see setKeyLength() + * @var int + * @access private + * @internal The max value is 256 / 8 = 32, the min value is 128 / 8 = 16. Exists in conjunction with $Nk + * because the encryption / decryption / key schedule creation requires this number and not $key_size. We could + * derive this from $key_size or vice versa, but that'd mean we'd have to do multiple shift operations, so in lieu + * of that, we'll just precompute it once. + */ + var $key_size = 16; + + /** + * Sets the key length. * - * If the key is more than 448-bits, we trim the excess bits. - * - * If the key is not explicitly set, or empty, it'll be assumed a 128 bits key to be all null bytes. + * Key lengths can be between 32 and 448 bits. * * @access public - * @see Crypt_Base::setKey() - * @param string $key + * @param int $length */ - function setKey($key) + function setKeyLength($length) { - $keylength = strlen($key); - - if (!$keylength) { - $key = "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"; - } elseif ($keylength > 56) { - $key = substr($key, 0, 56); + if ($length < 32) { + $this->key_size = 7; + } elseif ($length > 448) { + $this->key_size = 56; + } else { + $this->key_size = $length >> 3; } - parent::setKey($key); + parent::setKeyLength($length); } /** @@ -394,7 +390,7 @@ class Crypt_Blowfish extends Crypt_Base function isValidEngine($engine) { if ($engine == CRYPT_ENGINE_OPENSSL) { - if (strlen($this->key) != 16) { + if ($this->key_size != 16) { return false; } $this->cipher_name_openssl_ecb = 'bf-ecb'; diff --git a/phpseclib/Crypt/DES.php b/phpseclib/Crypt/DES.php index 8ac1fe6b..98fd9394 100644 --- a/phpseclib/Crypt/DES.php +++ b/phpseclib/Crypt/DES.php @@ -140,24 +140,13 @@ class Crypt_DES extends Crypt_Base var $block_size = 8; /** - * The Key + * Key Length * - * @see Crypt_Base::key - * @see setKey() - * @var string - * @access private - */ - var $key = "\0\0\0\0\0\0\0\0"; - - /** - * The default password key_size used by setPassword() - * - * @see Crypt_Base::password_key_size - * @see Crypt_Base::setPassword() + * @see Crypt_Base::setKeyLength() * @var int * @access private */ - var $password_key_size = 8; + var $key_size = 8; /** * The namespace used by the cipher for its constants. diff --git a/phpseclib/Crypt/RC2.php b/phpseclib/Crypt/RC2.php index eb41fe8f..cda4f0d7 100644 --- a/phpseclib/Crypt/RC2.php +++ b/phpseclib/Crypt/RC2.php @@ -139,14 +139,22 @@ class Crypt_RC2 extends Crypt_Base var $orig_key; /** - * The default password key_size used by setPassword() + * Don't truncate / null pad key * - * @see Crypt_Base::password_key_size - * @see Crypt_Base::setPassword() + * @see Crypt_Base::_clearBuffers + * @var bool + * @access private + */ + var $skip_key_adjustment = true; + + /** + * Key Length + * + * @see Crypt_RC2::setKeyLength() * @var int * @access private */ - var $password_key_size = 16; // = 128 bits + var $key_size = 16; // = 128 bits /** * The namespace used by the cipher for its constants. @@ -366,7 +374,7 @@ class Crypt_RC2 extends Crypt_Base { switch ($engine) { case CRYPT_ENGINE_OPENSSL: - if ($this->current_key_length != 128 || strlen($this->orig_key) != 16) { + if ($this->current_key_length != 128 || strlen($this->orig_key) < 16) { return false; } $this->cipher_name_openssl_ecb = 'rc2-ecb'; @@ -377,7 +385,7 @@ class Crypt_RC2 extends Crypt_Base } /** - * Sets the key length + * Sets the key length. * * Valid key lengths are 1 to 1024. * Calling this function after setting the key has no effect until the next @@ -393,6 +401,17 @@ class Crypt_RC2 extends Crypt_Base } } + /** + * Returns the current key length + * + * @access public + * @return int + */ + function getKeyLength() + { + return $this->current_key_length; + } + /** * Sets the key. * @@ -448,6 +467,7 @@ class Crypt_RC2 extends Crypt_Base // Prepare the key for mcrypt. $l[0] = $this->invpitable[$l[0]]; array_unshift($l, 'C*'); + parent::setKey(call_user_func_array('pack', $l)); } diff --git a/phpseclib/Crypt/RC4.php b/phpseclib/Crypt/RC4.php index 8f21cff0..3ad16836 100644 --- a/phpseclib/Crypt/RC4.php +++ b/phpseclib/Crypt/RC4.php @@ -99,14 +99,13 @@ class Crypt_RC4 extends Crypt_Base var $block_size = 0; /** - * The default password key_size used by setPassword() + * Key Length * - * @see Crypt_Base::password_key_size - * @see Crypt_Base::setPassword() + * @see Crypt_RC4::setKeyLength() * @var int * @access private */ - var $password_key_size = 128; // = 1024 bits + var $key_size = 128; // = 1024 bits /** * The namespace used by the cipher for its constants. @@ -223,18 +222,24 @@ class Crypt_RC4 extends Crypt_Base } /** - * Sets the key. + * Sets the key length * - * Keys can be between 1 and 256 bytes long. If they are longer then 256 bytes, the first 256 bytes will - * be used. If no key is explicitly set, it'll be assumed to be a single null byte. + * Keys can be between 1 and 256 bytes long. * * @access public - * @see Crypt_Base::setKey() - * @param string $key + * @param int $length */ - function setKey($key) + function setKeyLength($length) { - parent::setKey(substr($key, 0, 256)); + if ($length < 8) { + $this->key_size = 1; + } elseif ($length > 2048) { + $this->key_size = 248; + } else { + $this->key_size = $length >> 3; + } + + parent::setKeyLength($length); } /** diff --git a/phpseclib/Crypt/Rijndael.php b/phpseclib/Crypt/Rijndael.php index 17bbb864..7bd01903 100644 --- a/phpseclib/Crypt/Rijndael.php +++ b/phpseclib/Crypt/Rijndael.php @@ -127,16 +127,6 @@ define('CRYPT_RIJNDAEL_MODE_OFB', CRYPT_MODE_OFB); */ class Crypt_Rijndael extends Crypt_Base { - /** - * The default password key_size used by setPassword() - * - * @see Crypt_Base::password_key_size - * @see Crypt_Base::setPassword() - * @var int - * @access private - */ - var $password_key_size = 16; - /** * The namespace used by the cipher for its constants. * @@ -172,15 +162,6 @@ class Crypt_Rijndael extends Crypt_Base */ var $password_default_salt = 'phpseclib'; - /** - * Has the key length explicitly been set or should it be derived from the key, itself? - * - * @see setKeyLength() - * @var bool - * @access private - */ - var $explicit_key_length = false; - /** * The Key Schedule * @@ -289,48 +270,7 @@ class Crypt_Rijndael extends Crypt_Base } /** - * Sets the key. - * - * Keys can be of any length. Rijndael, itself, requires the use of a key that's between 128-bits and 256-bits long and - * whose length is a multiple of 32. If the key is less than 256-bits and the key length isn't set, we round the length - * up to the closest valid key length, padding $key with null bytes. If the key is more than 256-bits, we trim the - * excess bits. - * - * If the key is not explicitly set, it'll be assumed to be all null bytes. - * - * Note: 160/224-bit keys must explicitly set by setKeyLength(), otherwise they will be round/pad up to 192/256 bits. - * - * @see Crypt_Base:setKey() - * @see setKeyLength() - * @access public - * @param string $key - */ - function setKey($key) - { - if (!$this->explicit_key_length) { - $length = strlen($key); - switch (true) { - case $length <= 16: - $this->key_size = 16; - break; - case $length <= 20: - $this->key_size = 20; - break; - case $length <= 24: - $this->key_size = 24; - break; - case $length <= 28: - $this->key_size = 28; - break; - default: - $this->key_size = 32; - } - } - parent::setKey($key); - } - - /** - * Sets the key length + * 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. @@ -352,25 +292,23 @@ class Crypt_Rijndael extends Crypt_Base function setKeyLength($length) { switch (true) { - case $length == 160: - $this->key_size = 20; - break; - case $length == 224: - $this->key_size = 28; - break; case $length <= 128: $this->key_size = 16; break; + case $length <= 160: + $this->key_size = 20; + break; case $length <= 192: $this->key_size = 24; break; + case $length <= 224: + $this->key_size = 28; + break; default: $this->key_size = 32; } - $this->explicit_key_length = true; - $this->changed = true; - $this->_setEngine(); + parent::setKeyLength($length); } /** @@ -427,18 +365,6 @@ class Crypt_Rijndael extends Crypt_Base return parent::isValidEngine($engine); } - /** - * Setup the CRYPT_ENGINE_MCRYPT $engine - * - * @see Crypt_Base::_setupMcrypt() - * @access private - */ - function _setupMcrypt() - { - $this->key = str_pad(substr($this->key, 0, $this->key_size), $this->key_size, "\0"); - parent::_setupMcrypt(); - } - /** * Encrypts a block * @@ -649,8 +575,6 @@ class Crypt_Rijndael extends Crypt_Base 0x7D000000, 0xFA000000, 0xEF000000, 0xC5000000, 0x91000000 ); - $this->key = str_pad(substr($this->key, 0, $this->key_size), $this->key_size, "\0"); - if (isset($this->kl['key']) && $this->key === $this->kl['key'] && $this->key_size === $this->kl['key_size'] && $this->block_size === $this->kl['block_size']) { // already expanded return; diff --git a/phpseclib/Crypt/TripleDES.php b/phpseclib/Crypt/TripleDES.php index 99a0a5b7..0bd68361 100644 --- a/phpseclib/Crypt/TripleDES.php +++ b/phpseclib/Crypt/TripleDES.php @@ -95,15 +95,13 @@ define('CRYPT_DES_MODE_CBC3', CRYPT_MODE_CBC3); class Crypt_TripleDES extends Crypt_DES { /** - * The default password key_size used by setPassword() + * Key Length * - * @see Crypt_DES::password_key_size - * @see Crypt_Base::password_key_size - * @see Crypt_Base::setPassword() + * @see Crypt_TripleDES::setKeyLength() * @var int * @access private */ - var $password_key_size = 24; + var $key_size = 24; /** * The default salt used by setPassword() @@ -266,6 +264,32 @@ class Crypt_TripleDES extends Crypt_DES } } + /** + * Sets the key length. + * + * Valid key lengths are 64, 128 and 192 + * + * @see Crypt_Rijndael:setKeyLength() + * @access public + * @param int $length + */ + function setKeyLength($length) + { + $length >>= 3; + switch (true) { + case $length <= 8: + $this->key_size = 8; + break; + case $length <= 16: + $this->key_size = 16; + break; + default: + $this->key_size = 24; + } + + parent::setKeyLength($length); + } + /** * Sets the key. * @@ -283,7 +307,7 @@ class Crypt_TripleDES extends Crypt_DES */ function setKey($key) { - $length = strlen($key); + $length = $this->explicit_key_length ? $this->key_size : 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: diff --git a/phpseclib/Crypt/Twofish.php b/phpseclib/Crypt/Twofish.php index 07b43ed5..c332a3e5 100644 --- a/phpseclib/Crypt/Twofish.php +++ b/phpseclib/Crypt/Twofish.php @@ -433,35 +433,27 @@ class Crypt_Twofish extends Crypt_Base var $kl; /** - * Sets the key. + * Sets the key length. * - * Keys can be of any length. Twofish, itself, requires the use of a key that's 128, 192 or 256-bits long. - * If the key is less than 256-bits we round the length up to the closest valid key length, - * padding $key with null bytes. If the key is more than 256-bits, we trim the excess bits. - * - * If the key is not explicitly set, it'll be assumed a 128 bits key to be all null bytes. + * Valid key lengths are 128, 192 or 256 bits * * @access public - * @see Crypt_Base::setKey() - * @param string $key + * @param int $length */ - function setKey($key) + function setKeyLength($length) { - $keylength = strlen($key); switch (true) { - case $keylength <= 16: - $key = str_pad($key, 16, "\0"); + case $length <= 128: + $this->key_size = 16; break; - case $keylength <= 24: - $key = str_pad($key, 24, "\0"); + case $length <= 192: + $this->key_size = 24; break; - case $keylength < 32: - $key = str_pad($key, 32, "\0"); - break; - case $keylength > 32: - $key = substr($key, 0, 32); + default: + $this->key_size = 32; } - parent::setKey($key); + + parent::setKeyLength($length); } /** diff --git a/tests/Unit/Crypt/AES/TestCase.php b/tests/Unit/Crypt/AES/TestCase.php index 5a277c8b..c55e0bd5 100644 --- a/tests/Unit/Crypt/AES/TestCase.php +++ b/tests/Unit/Crypt/AES/TestCase.php @@ -334,4 +334,39 @@ abstract class Unit_Crypt_AES_TestCase extends PhpseclibTestCase $result = bin2hex($aes->encrypt(pack('H*', '91fbef2d15a97816060bee1feaa49afe'))); $this->assertSame($result, '1bc704f1bce135ceb810341b216d7abe'); } + + public function testGetKeyLengthDefault() + { + $aes = new Crypt_AES(); + $this->assertSame($aes->getKeyLength(), 128); + } + + public function testGetKeyLengthWith192BitKey() + { + $aes = new Crypt_AES(); + $aes->setKey(str_repeat('a', 24)); + $this->assertSame($aes->getKeyLength(), 192); + } + + public function testSetKeyLengthWithLargerKey() + { + $aes = new Crypt_AES(); + $aes->setKeyLength(128); + $aes->setKey(str_repeat('a', 24)); + $this->assertSame($aes->getKeyLength(), 128); + $ciphertext = bin2hex($aes->encrypt('a')); + $this->assertSame($ciphertext, '82b7b068dfc60ed2a46893b69fecd6c2'); + $this->assertSame($aes->getKeyLength(), 128); + } + + public function testSetKeyLengthWithSmallerKey() + { + $aes = new Crypt_AES(); + $aes->setKeyLength(256); + $aes->setKey(str_repeat('a', 16)); + $this->assertSame($aes->getKeyLength(), 256); + $ciphertext = bin2hex($aes->encrypt('a')); + $this->assertSame($ciphertext, 'fd4250c0d234aa7e1aa592820aa8406b'); + $this->assertSame($aes->getKeyLength(), 256); + } }