From 1473da35e674bd8be10671aa97e981d6a368cd43 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Fri, 17 Jul 2015 18:20:42 +0200 Subject: [PATCH] SSH2: Introduce _array_intersect_first function. + No more empty for-loop bodies + No more counting variables $i leaked into outer context + No more unintuitive $i == count(...) comparisons + No more array / hash table access of the form $kex_algorithms[$i] - Function call overhead; not in the performance critical path, though. --- phpseclib/Net/SSH2.php | 95 ++++++++++++++++++++++-------------------- 1 file changed, 50 insertions(+), 45 deletions(-) diff --git a/phpseclib/Net/SSH2.php b/phpseclib/Net/SSH2.php index 28d7bfb9..46df01dc 100644 --- a/phpseclib/Net/SSH2.php +++ b/phpseclib/Net/SSH2.php @@ -1357,16 +1357,9 @@ class Net_SSH2 // here ends the second place. // we need to decide upon the symmetric encryption algorithms before we do the diffie-hellman key exchange - for ($i = 0; $i < count($encryption_algorithms) && !in_array($encryption_algorithms[$i], $this->encryption_algorithms_server_to_client); $i++) { - } - if ($i == count($encryption_algorithms)) { - user_error('No compatible server to client encryption algorithms found'); - return $this->_disconnect(NET_SSH2_DISCONNECT_KEY_EXCHANGE_FAILED); - } - // we don't initialize any crypto-objects, yet - we do that, later. for now, we need the lengths to make the // diffie-hellman key exchange as fast as possible - $decrypt = $encryption_algorithms[$i]; + $decrypt = $this->_array_intersect_first($encryption_algorithms, $this->encryption_algorithms_server_to_client); switch ($decrypt) { case '3des-cbc': case '3des-ctr': @@ -1402,16 +1395,13 @@ class Net_SSH2 break; case 'none': $decryptKeyLength = 0; + break; + default: + user_error('No compatible server to client encryption algorithms found'); + return $this->_disconnect(NET_SSH2_DISCONNECT_KEY_EXCHANGE_FAILED); } - for ($i = 0; $i < count($encryption_algorithms) && !in_array($encryption_algorithms[$i], $this->encryption_algorithms_client_to_server); $i++) { - } - if ($i == count($encryption_algorithms)) { - user_error('No compatible client to server encryption algorithms found'); - return $this->_disconnect(NET_SSH2_DISCONNECT_KEY_EXCHANGE_FAILED); - } - - $encrypt = $encryption_algorithms[$i]; + $encrypt = $this->_array_intersect_first($encryption_algorithms, $this->encryption_algorithms_client_to_server); switch ($encrypt) { case '3des-cbc': case '3des-ctr': @@ -1447,20 +1437,21 @@ class Net_SSH2 break; case 'none': $encryptKeyLength = 0; + break; + default: + user_error('No compatible client to server encryption algorithms found'); + return $this->_disconnect(NET_SSH2_DISCONNECT_KEY_EXCHANGE_FAILED); } $keyLength = $decryptKeyLength > $encryptKeyLength ? $decryptKeyLength : $encryptKeyLength; // through diffie-hellman key exchange a symmetric key is obtained - for ($i = 0; $i < count($kex_algorithms) && !in_array($kex_algorithms[$i], $this->kex_algorithms); - $i++) { - } - if ($i == count($kex_algorithms)) { + $kex_algorithm = $this->_array_intersect_first($kex_algorithms, $this->kex_algorithms); + if ($kex_algorithm === false) { user_error('No compatible key exchange algorithms found'); return $this->_disconnect(NET_SSH2_DISCONNECT_KEY_EXCHANGE_FAILED); } - - if (strpos($kex_algorithms[$i], 'diffie-hellman-group-exchange') === 0) { + if (strpos($kex_algorithm, 'diffie-hellman-group-exchange') === 0) { $dh_group_sizes_packed = pack( 'NNN', $this->kex_dh_group_size_min, @@ -1507,7 +1498,7 @@ class Net_SSH2 $clientKexInitMessage = NET_SSH2_MSG_KEXDH_GEX_INIT; $serverKexReplyMessage = NET_SSH2_MSG_KEXDH_GEX_REPLY; } else { - switch ($kex_algorithms[$i]) { + switch ($kex_algorithm) { // see http://tools.ietf.org/html/rfc2409#section-6.2 and // http://tools.ietf.org/html/rfc2412, appendex E case 'diffie-hellman-group1-sha1': @@ -1537,7 +1528,7 @@ class Net_SSH2 $serverKexReplyMessage = NET_SSH2_MSG_KEXDH_REPLY; } - switch ($kex_algorithms[$i]) { + switch ($kex_algorithm) { case 'diffie-hellman-group-exchange-sha256': $kexHash = new Crypt_Hash('sha256'); break; @@ -1626,14 +1617,13 @@ class Net_SSH2 $this->session_id = $this->exchange_hash; } - for ($i = 0; $i < count($server_host_key_algorithms) && !in_array($server_host_key_algorithms[$i], $this->server_host_key_algorithms); $i++) { - } - if ($i == count($server_host_key_algorithms)) { + $server_host_key_algorithm = $this->_array_intersect_first($server_host_key_algorithms, $this->server_host_key_algorithms); + if ($server_host_key_algorithm === false) { user_error('No compatible server host key algorithms found'); return $this->_disconnect(NET_SSH2_DISCONNECT_KEY_EXCHANGE_FAILED); } - if ($public_key_format != $server_host_key_algorithms[$i] || $this->signature_format != $server_host_key_algorithms[$i]) { + if ($public_key_format != $server_host_key_algorithm || $this->signature_format != $server_host_key_algorithm) { user_error('Server Host Key Algorithm Mismatch'); return $this->_disconnect(NET_SSH2_DISCONNECT_KEY_EXCHANGE_FAILED); } @@ -1871,15 +1861,14 @@ class Net_SSH2 $this->decrypt->decrypt(str_repeat("\0", 1536)); } - for ($i = 0; $i < count($mac_algorithms) && !in_array($mac_algorithms[$i], $this->mac_algorithms_client_to_server); $i++) { - } - if ($i == count($mac_algorithms)) { + $mac_algorithm = $this->_array_intersect_first($mac_algorithms, $this->mac_algorithms_client_to_server); + if ($mac_algorithm === false) { user_error('No compatible client to server message authentication algorithms found'); return $this->_disconnect(NET_SSH2_DISCONNECT_KEY_EXCHANGE_FAILED); } - $createKeyLength = 0; // ie. $mac_algorithms[$i] == 'none' - switch ($mac_algorithms[$i]) { + $createKeyLength = 0; // ie. $mac_algorithm == 'none' + switch ($mac_algorithm) { case 'hmac-sha2-256': $this->hmac_create = new Crypt_Hash('sha256'); $createKeyLength = 32; @@ -1901,16 +1890,15 @@ class Net_SSH2 $createKeyLength = 16; } - for ($i = 0; $i < count($mac_algorithms) && !in_array($mac_algorithms[$i], $this->mac_algorithms_server_to_client); $i++) { - } - if ($i == count($mac_algorithms)) { + $mac_algorithm = $this->_array_intersect_first($mac_algorithms, $this->mac_algorithms_server_to_client); + if ($mac_algorithm === false) { user_error('No compatible server to client message authentication algorithms found'); return $this->_disconnect(NET_SSH2_DISCONNECT_KEY_EXCHANGE_FAILED); } $checkKeyLength = 0; $this->hmac_size = 0; - switch ($mac_algorithms[$i]) { + switch ($mac_algorithm) { case 'hmac-sha2-256': $this->hmac_check = new Crypt_Hash('sha256'); $checkKeyLength = 32; @@ -1949,21 +1937,19 @@ class Net_SSH2 } $this->hmac_check->setKey(substr($key, 0, $checkKeyLength)); - for ($i = 0; $i < count($compression_algorithms) && !in_array($compression_algorithms[$i], $this->compression_algorithms_server_to_client); $i++) { - } - if ($i == count($compression_algorithms)) { + $compression_algorithm = $this->_array_intersect_first($compression_algorithms, $this->compression_algorithms_server_to_client); + if ($compression_algorithm === false) { user_error('No compatible server to client compression algorithms found'); return $this->_disconnect(NET_SSH2_DISCONNECT_KEY_EXCHANGE_FAILED); } - $this->decompress = $compression_algorithms[$i] == 'zlib'; + $this->decompress = $compression_algorithm == 'zlib'; - for ($i = 0; $i < count($compression_algorithms) && !in_array($compression_algorithms[$i], $this->compression_algorithms_client_to_server); $i++) { - } - if ($i == count($compression_algorithms)) { + $compression_algorithm = $this->_array_intersect_first($compression_algorithms, $this->compression_algorithms_client_to_server); + if ($compression_algorithm === false) { user_error('No compatible client to server compression algorithms found'); return $this->_disconnect(NET_SSH2_DISCONNECT_KEY_EXCHANGE_FAILED); } - $this->compress = $compression_algorithms[$i] == 'zlib'; + $this->compress = $compression_algorithm == 'zlib'; return true; } @@ -3877,6 +3863,25 @@ class Net_SSH2 } } + /** + * Returns the first value of the intersection of two arrays or false if + * the intersection is empty. The order is defined by the first parameter. + * + * @param Array $array1 + * @param Array $array2 + * @return Mixed False if intersection is empty, else intersected value. + * @access private + */ + function _array_intersect_first($array1, $array2) + { + foreach ($array1 as $value) { + if (in_array($value, $array2)) { + return $value; + } + } + return false; + } + /** * Returns all errors *