From 53276ba4a119163d6d6ab18e93c49fb19081ecbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hans-J=C3=BCrgen=20Petrich?= Date: Sat, 1 Jun 2013 08:00:04 +0700 Subject: [PATCH] AES: inconsistencey with 160 / 224-bits keys re: https://github.com/phpseclib/phpseclib/issues/110 --- phpseclib/Crypt/AES.php | 68 +++++++++++++++++++++++------------- phpseclib/Crypt/Rijndael.php | 53 +++++++++++++++------------- 2 files changed, 73 insertions(+), 48 deletions(-) diff --git a/phpseclib/Crypt/AES.php b/phpseclib/Crypt/AES.php index cf329f58..19c72d9e 100644 --- a/phpseclib/Crypt/AES.php +++ b/phpseclib/Crypt/AES.php @@ -193,6 +193,39 @@ class Crypt_AES extends Crypt_Rijndael { return; } + /** + * 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. + * + * Note: phpseclib extends AES for using 160- and 224-bit keys but they are officially not defined in AES + * and the most (if not all) implementations of AES are not able using 160/224-bit keys but round/pad + * them up to 192/256 bits as, for example, mcrypt will do. + * + * That said, if you want be compatible with other AES implementations, + * you should not setKeyLength(160) or setKeyLength(224). + * + * Additional: In case of 160- and 224-bit keys, phpseclib will/can, for that reason, not use + * the mcrypt php extention, even if available. This results then in slower encryption. + * + * @access public + * @param Integer $length + */ + function setKeyLength($length) + { + parent::setKeyLength($length); + + switch ($this->key_size) { + case 20: // 160-bits + case 28: // 224-bits + $this->engine = CRYPT_AES_MODE_INTERNAL; // because mcrypt is not able to use (real) 160/224-bit keys + break; // we force using our internal AES engine instead of mcrypt. + default: + $this->engine = CRYPT_AES_MODE; + } + } + /** * Setup the CRYPT_MODE_MCRYPT $engine * @@ -204,33 +237,20 @@ class Crypt_AES extends Crypt_Rijndael { function _setupMcrypt() { if (!$this->explicit_key_length) { - // this just copied from Crypt_Rijndael::_setup() - $length = strlen($this->key) >> 2; - if ($length > 8) { - $length = 8; - } else if ($length < 4) { - $length = 4; + $length = strlen($this->key); + switch (true) { + case $length <= 16: + $this->key_size = 16; + break; + case $length <= 24: + $this->key_size = 24; + break; + default: + $this->key_size = 32; } - $this->Nk = $length; - $this->key_size = $length << 2; } - switch ($this->Nk) { - case 4: // 128 - $this->key_size = 16; - break; - case 5: // 160 - case 6: // 192 - $this->key_size = 24; - break; - case 7: // 224 - case 8: // 256 - $this->key_size = 32; - } - - $this->password_key_size = $this->key_size; - $this->key = str_pad(substr($this->key, 0, $this->key_size), $this->key_size, chr(0)); - + $this->key = str_pad(substr($this->key, 0, $this->key_size), $this->key_size, "\0"); parent::_setupMcrypt(); } } diff --git a/phpseclib/Crypt/Rijndael.php b/phpseclib/Crypt/Rijndael.php index 02b292d3..6b09ff82 100644 --- a/phpseclib/Crypt/Rijndael.php +++ b/phpseclib/Crypt/Rijndael.php @@ -218,7 +218,7 @@ class Crypt_Rijndael extends Crypt_Base { * @see setKeyLength() * @var Integer * @access private - * @internal The max value is 256 / 8 = 32, the min value is 128 / 8 = 16. Exists in conjunction with $key_size + * @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. @@ -704,10 +704,6 @@ class Crypt_Rijndael extends Crypt_Base { */ function setKey($key) { - if (strlen($key) > 32) { - $key = substr($key, 0, 32); - } - parent::setKey($key); } @@ -722,14 +718,22 @@ class Crypt_Rijndael extends Crypt_Base { */ function setKeyLength($length) { - $length >>= 5; - if ($length > 8) { - $length = 8; - } else if ($length < 4) { - $length = 4; + 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 <= 192: + $this->key_size = 24; + break; + default: + $this->key_size = 32; } - $this->Nk = $length; - $this->key_size = $this->password_key_size = $length << 2; $this->explicit_key_length = true; $this->changed = true; @@ -970,26 +974,27 @@ class Crypt_Rijndael extends Crypt_Base { ); if (!$this->explicit_key_length) { - // we do >> 2, here, and not >> 5, as we do above, since strlen($this->key) tells us the number of bytes - not bits - $length = strlen($this->key) >> 2; - if ($length > 8) { - $length = 8; - } else if ($length < 4) { - $length = 4; + $length = strlen($this->key); + switch (true) { + case $length <= 16: + $this->key_size = 16; + break; + case $length <= 24: + $this->key_size = 24; + break; + default: + $this->key_size = 32; } - $this->Nk = $length; - $this->key_size = $this->password_key_size = $length << 2; } - - $key = str_pad(substr($this->key, 0, $this->key_size), $this->key_size, chr(0)); + $key = str_pad(substr($this->key, 0, $this->key_size), $this->key_size, "\0"); if (isset($this->kl['key']) && $key === $this->kl['key'] && $this->key_size === $this->kl['key_size'] && $this->block_size === $this->kl['block_size']) { // already expanded return; } - $this->kl = array('key' => $key, 'block_size' => $this->block_size, 'key_size' => $this->key_size); - + $this->kl = array('key' => $key, 'key_size' => $this->key_size, 'block_size' => $this->block_size); + $this->Nk = $this->key_size >> 2; // see Rijndael-ammended.pdf#page=44 $this->Nr = max($this->Nk, $this->Nb) + 6;