AES: inconsistencey with 160 / 224-bits keys

re: https://github.com/phpseclib/phpseclib/issues/110

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);
-            $length = strlen($this->key) >> 2;
+            switch (true) {
-            if ($length > 8) {
+                case $length <= 16:
-                $length = 8;
+                    $this->key_size = 16;
-            } else if ($length < 4) {
+                    break;
-                $length = 4;
+                case $length <= 24:
+                    $this->key_size = 24;
+                    break;
+                default:
+                    $this->key_size = 32;
             }
-            $this->Nk = $length;
-            $this->key_size = $length << 2;
         }
 
-        switch ($this->Nk) {
+        $this->key = str_pad(substr($this->key, 0, $this->key_size), $this->key_size, "\0");
-            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));
-
         parent::_setupMcrypt();
     }
 }

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;
+        switch (true) {
-        if ($length > 8) {
+            case $length == 160:
-            $length = 8;
+                $this->key_size = 20;
-        } else if ($length < 4) {
+                break;
-            $length = 4;
+            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);
-            $length = strlen($this->key) >> 2;
+            switch (true) {
-            if ($length > 8) {
+                case $length <= 16:
-                $length = 8;
+                    $this->key_size = 16;
-            } else if ($length < 4) {
+                    break;
-                $length = 4;
+                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, "\0");
-        $key = str_pad(substr($this->key, 0, $this->key_size), $this->key_size, chr(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;