Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SSHClient: request host key type from known_hosts (backport) #945

Merged
merged 4 commits into from Sep 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 21 additions & 17 deletions paramiko/client.py
Expand Up @@ -334,34 +334,38 @@ def connect(
t.set_log_channel(self._log_channel)
if banner_timeout is not None:
t.banner_timeout = banner_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

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 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.gss_kex_used:
our_server_key = self._system_host_keys.get(server_hostkey_name,
{}).get(keytype, None)
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:
server_key = t.get_remote_server_key()
if our_server_keys 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)
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()
Expand Down
3 changes: 3 additions & 0 deletions paramiko/transport.py
Expand Up @@ -1995,6 +1995,9 @@ def _parse_kex_init(self, m):
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)')
self._log_agreement(
'HostKey', agreed_keys[0], agreed_keys[0]
)

if self.server_mode:
agreed_local_ciphers = list(filter(self._preferred_ciphers.__contains__,
Expand Down
4 changes: 4 additions & 0 deletions sites/www/changelog.rst
Expand Up @@ -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 the case where an ecdsa key is in known_hosts and
the server also has an rsa host key. Thanks to Pierce Lopez.
* :bug:`1055` (also :issue:`1056`, :issue:`1057`, :issue:`1058`, :issue:`1059`)
Fix up host-key checking in our GSSAPI support, which was previously using an
incorrect API call. Thanks to Anselm Kruis for the patches.
Expand Down
52 changes: 51 additions & 1 deletion tests/test_client.py
Expand Up @@ -117,7 +117,11 @@ def _run(self, allowed_keys=None, delay=0):
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:
Expand Down Expand Up @@ -440,6 +444,52 @@ def test_12_reject_policy_gsskex(self):
**self.connect_kwargs
)

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.
Expand Down
5 changes: 5 additions & 0 deletions tests/test_ecdsa_256.key
@@ -0,0 +1,5 @@
-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIKB6ty3yVyKEnfF/zprx0qwC76MsMlHY4HXCnqho2eKioAoGCCqGSM49
AwEHoUQDQgAElI9mbdlaS+T9nHxY/59lFnn80EEecZDBHq4gLpccY8Mge5ZTMiMD
ADRvOqQ5R98Sxst765CAqXmRtz8vwoD96g==
-----END EC PRIVATE KEY-----