diff options
| -rw-r--r-- | paramiko/client.py | 44 | ||||
| -rw-r--r-- | paramiko/transport.py | 3 | ||||
| -rw-r--r-- | sites/www/changelog.rst | 4 | ||||
| -rw-r--r-- | tests/test_client.py | 60 |
4 files changed, 88 insertions, 23 deletions
diff --git a/paramiko/client.py b/paramiko/client.py index b2c21798..936693fc 100644 --- a/paramiko/client.py +++ b/paramiko/client.py @@ -343,35 +343,41 @@ class SSHClient (ClosingContextManager): t.banner_timeout = banner_timeout if auth_timeout is not None: t.auth_timeout = auth_timeout - t.start_client(timeout=timeout) - - server_key = t.get_remote_server_key() - keytype = server_key.get_name() if port == SSH_PORT: server_hostkey_name = hostname else: server_hostkey_name = "[%s]:%d" % (hostname, port) + our_server_keys = None # If GSS-API Key Exchange is performed we are not required to check the # host key, because the host is authenticated via GSS-API / SSPI as # well as our client. if not self._transport.use_gss_kex: - our_server_key = self._system_host_keys.get( - server_hostkey_name, {}).get(keytype) - if our_server_key is None: - our_server_key = self._host_keys.get(server_hostkey_name, - {}).get(keytype, None) - if our_server_key is None: - # will raise exception if the key is rejected; - # let that fall out - self._policy.missing_host_key(self, server_hostkey_name, - server_key) - # if the callback returns, assume the key is ok - our_server_key = server_key - - if server_key != our_server_key: - raise BadHostKeyException(hostname, server_key, our_server_key) + our_server_keys = self._system_host_keys.get(server_hostkey_name) + if our_server_keys is None: + our_server_keys = self._host_keys.get(server_hostkey_name) + if our_server_keys is not None: + keytype = our_server_keys.keys()[0] + sec_opts = t.get_security_options() + other_types = [x for x in sec_opts.key_types if x != keytype] + sec_opts.key_types = [keytype] + other_types + + t.start_client(timeout=timeout) + + if not self._transport.use_gss_kex: + server_key = t.get_remote_server_key() + if our_server_keys is None: + # will raise exception if the key is rejected + self._policy.missing_host_key( + self, server_hostkey_name, server_key + ) + else: + our_key = our_server_keys.get(server_key.get_name()) + if our_key != server_key: + if our_key is None: + our_key = list(our_server_keys.values())[0] + raise BadHostKeyException(hostname, server_key, our_key) if username is None: username = getpass.getuser() diff --git a/paramiko/transport.py b/paramiko/transport.py index 174e0bf4..bab23fa1 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -2100,6 +2100,9 @@ class Transport(threading.Thread, ClosingContextManager): self.host_key_type = agreed_keys[0] if self.server_mode and (self.get_server_key() is None): raise SSHException('Incompatible ssh peer (can\'t match requested host key type)') # noqa + self._log_agreement( + 'HostKey', agreed_keys[0], agreed_keys[0] + ) if self.server_mode: agreed_local_ciphers = list(filter( diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 1d299119..2fa80c0a 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,10 @@ Changelog ========= +* :bug:`865` SSHClient requests the type of host key it has (e.g. from + known_hosts) and does not consider a different type to be a "Missing" host + key. This fixes a common case where an ECDSA key is in known_hosts and the + server also has an RSA host key. Thanks to Pierce Lopez. * :support:`906 (1.18+)` Clean up a handful of outdated imports and related tweaks. Thanks to Pierce Lopez. * :bug:`984` Enhance default cipher preference order such that diff --git a/tests/test_client.py b/tests/test_client.py index bddaf4bc..e912d5b2 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -122,7 +122,11 @@ class SSHClientTest (unittest.TestCase): allowed_keys = FINGERPRINTS.keys() self.socks, addr = self.sockl.accept() self.ts = paramiko.Transport(self.socks) - host_key = paramiko.RSAKey.from_private_key_file(test_path('test_rsa.key')) + keypath = test_path('test_rsa.key') + host_key = paramiko.RSAKey.from_private_key_file(keypath) + self.ts.add_server_key(host_key) + keypath = test_path('test_ecdsa_256.key') + host_key = paramiko.ECDSAKey.from_private_key_file(keypath) self.ts.add_server_key(host_key) server = NullServer(allowed_keys=allowed_keys) if delay: @@ -249,8 +253,9 @@ class SSHClientTest (unittest.TestCase): verify that SSHClient's AutoAddPolicy works. """ threading.Thread(target=self._run).start() - host_key = paramiko.RSAKey.from_private_key_file(test_path('test_rsa.key')) - public_host_key = paramiko.RSAKey(data=host_key.asbytes()) + hostname = '[%s]:%d' % (self.addr, self.port) + key_file = test_path('test_ecdsa_256.key') + public_host_key = paramiko.ECDSAKey.from_private_key_file(key_file) self.tc = paramiko.SSHClient() self.tc.set_missing_host_key_policy(paramiko.AutoAddPolicy()) @@ -263,7 +268,8 @@ class SSHClientTest (unittest.TestCase): self.assertEqual('slowdive', self.ts.get_username()) self.assertEqual(True, self.ts.is_authenticated()) self.assertEqual(1, len(self.tc.get_host_keys())) - self.assertEqual(public_host_key, self.tc.get_host_keys()['[%s]:%d' % (self.addr, self.port)]['ssh-rsa']) + new_host_key = list(self.tc.get_host_keys()[hostname].values())[0] + self.assertEqual(public_host_key, new_host_key) def test_5_save_host_keys(self): """ @@ -396,6 +402,52 @@ class SSHClientTest (unittest.TestCase): auth_timeout=0.5, ) + def _client_host_key_bad(self, host_key): + threading.Thread(target=self._run).start() + hostname = '[%s]:%d' % (self.addr, self.port) + + self.tc = paramiko.SSHClient() + self.tc.set_missing_host_key_policy(paramiko.WarningPolicy()) + known_hosts = self.tc.get_host_keys() + known_hosts.add(hostname, host_key.get_name(), host_key) + + self.assertRaises( + paramiko.BadHostKeyException, + self.tc.connect, + password='pygmalion', + **self.connect_kwargs + ) + + def _client_host_key_good(self, ktype, kfile): + threading.Thread(target=self._run).start() + hostname = '[%s]:%d' % (self.addr, self.port) + + self.tc = paramiko.SSHClient() + self.tc.set_missing_host_key_policy(paramiko.RejectPolicy()) + host_key = ktype.from_private_key_file(test_path(kfile)) + known_hosts = self.tc.get_host_keys() + known_hosts.add(hostname, host_key.get_name(), host_key) + + self.tc.connect(password='pygmalion', **self.connect_kwargs) + self.event.wait(1.0) + self.assertTrue(self.event.is_set()) + self.assertTrue(self.ts.is_active()) + self.assertEqual(True, self.ts.is_authenticated()) + + def test_host_key_negotiation_1(self): + host_key = paramiko.ECDSAKey.generate() + self._client_host_key_bad(host_key) + + def test_host_key_negotiation_2(self): + host_key = paramiko.RSAKey.generate(2048) + self._client_host_key_bad(host_key) + + def test_host_key_negotiation_3(self): + self._client_host_key_good(paramiko.ECDSAKey, 'test_ecdsa_256.key') + + def test_host_key_negotiation_4(self): + self._client_host_key_good(paramiko.RSAKey, 'test_rsa.key') + def test_update_environment(self): """ Verify that environment variables can be set by the client. |