diff options
-rw-r--r-- | .travis.yml | 4 | ||||
-rw-r--r-- | dev-requirements.txt | 8 | ||||
-rw-r--r-- | paramiko/_winapi.py | 6 | ||||
-rw-r--r-- | paramiko/client.py | 2 | ||||
-rw-r--r-- | paramiko/hostkeys.py | 6 | ||||
-rw-r--r-- | paramiko/message.py | 45 | ||||
-rw-r--r-- | paramiko/packet.py | 45 | ||||
-rw-r--r-- | paramiko/ssh_exception.py | 6 | ||||
-rw-r--r-- | paramiko/transport.py | 27 | ||||
-rw-r--r-- | sites/www/changelog.rst | 19 | ||||
-rw-r--r-- | tasks.py | 29 | ||||
-rw-r--r-- | tests/test_hostkeys.py | 1 | ||||
-rw-r--r-- | tests/test_message.py | 8 | ||||
-rw-r--r-- | tests/test_transport.py | 17 |
14 files changed, 146 insertions, 77 deletions
diff --git a/.travis.yml b/.travis.yml index 9a55dbb6..45fb1722 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,8 +13,8 @@ install: - pip install coveralls # For coveralls.io specifically - pip install -r dev-requirements.txt script: - # Main tests, with coverage! - - inv test --coverage + # Main tests, w/ coverage! (but skip coverage on 3.2, coverage.py dropped it) + - "[[ $TRAVIS_PYTHON_VERSION != 3.2 ]] && inv test --coverage || inv test" # Ensure documentation & invoke pipeline run OK. # Run 'docs' first since its objects.inv is referred to by 'www'. # Also force warnings to be errors since most of them tend to be actual diff --git a/dev-requirements.txt b/dev-requirements.txt index 7a0ccbc5..059572cf 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,9 +1,11 @@ # Older junk tox>=1.4,<1.5 # For newer tasks like building Sphinx docs. -invoke>=0.7.0,<0.8 -invocations>=0.5.0 +invoke>=0.10 +invocations>=0.9.2 sphinx>=1.1.3 alabaster>=0.6.1 releases>=0.5.2 -wheel==0.23.0 +semantic_version>=2.4,<2.5 +wheel==0.24 +twine==1.5 diff --git a/paramiko/_winapi.py b/paramiko/_winapi.py index f48e1890..cf4d68e5 100644 --- a/paramiko/_winapi.py +++ b/paramiko/_winapi.py @@ -106,7 +106,7 @@ MapViewOfFile.restype = ctypes.wintypes.HANDLE class MemoryMap(object): """ - A memory map object which can have security attributes overrideden. + A memory map object which can have security attributes overridden. """ def __init__(self, name, length, security_attributes=None): self.name = name @@ -219,8 +219,8 @@ class SECURITY_ATTRIBUTES(ctypes.Structure): @descriptor.setter def descriptor(self, value): - self._descriptor = descriptor - self.lpSecurityDescriptor = ctypes.addressof(descriptor) + self._descriptor = value + self.lpSecurityDescriptor = ctypes.addressof(value) def GetTokenInformation(token, information_class): """ diff --git a/paramiko/client.py b/paramiko/client.py index 393e3e09..9ee30287 100644 --- a/paramiko/client.py +++ b/paramiko/client.py @@ -338,7 +338,7 @@ class SSHClient (ClosingContextManager): :raises SSHException: if the server fails to execute the command """ - chan = self._transport.open_session() + chan = self._transport.open_session(timeout=timeout) if get_pty: chan.get_pty() chan.settimeout(timeout) diff --git a/paramiko/hostkeys.py b/paramiko/hostkeys.py index 84868875..c7e1f72e 100644 --- a/paramiko/hostkeys.py +++ b/paramiko/hostkeys.py @@ -35,6 +35,7 @@ from paramiko.dsskey import DSSKey from paramiko.rsakey import RSAKey from paramiko.util import get_logger, constant_time_bytes_eq from paramiko.ecdsakey import ECDSAKey +from paramiko.ssh_exception import SSHException class HostKeys (MutableMapping): @@ -96,7 +97,10 @@ class HostKeys (MutableMapping): line = line.strip() if (len(line) == 0) or (line[0] == '#'): continue - e = HostKeyEntry.from_line(line, lineno) + try: + e = HostKeyEntry.from_line(line, lineno) + except SSHException: + continue if e is not None: _hostnames = e.hostnames for h in _hostnames: diff --git a/paramiko/message.py b/paramiko/message.py index b893e76d..bf4c6b95 100644 --- a/paramiko/message.py +++ b/paramiko/message.py @@ -129,7 +129,7 @@ class Message (object): b = self.get_bytes(1) return b != zero_byte - def get_int(self): + def get_adaptive_int(self): """ Fetch an int from the stream. @@ -141,20 +141,7 @@ class Message (object): byte += self.get_bytes(3) return struct.unpack('>I', byte)[0] - def get_size(self): - """ - Fetch an int from the stream. - - @return: a 32-bit unsigned integer. - @rtype: int - """ - byte = self.get_bytes(1) - if byte == max_byte: - return util.inflate_long(self.get_binary()) - byte += self.get_bytes(3) - return struct.unpack('>I', byte)[0] - - def get_size(self): + def get_int(self): """ Fetch an int from the stream. @@ -185,7 +172,7 @@ class Message (object): contain unprintable characters. (It's not unheard of for a string to contain another byte-stream message.) """ - return self.get_bytes(self.get_size()) + return self.get_bytes(self.get_int()) def get_text(self): """ @@ -196,7 +183,7 @@ class Message (object): @return: a string. @rtype: string """ - return u(self.get_bytes(self.get_size())) + return u(self.get_bytes(self.get_int())) #return self.get_bytes(self.get_size()) def get_binary(self): @@ -208,7 +195,7 @@ class Message (object): @return: a string. @rtype: string """ - return self.get_bytes(self.get_size()) + return self.get_bytes(self.get_int()) def get_list(self): """ @@ -248,7 +235,7 @@ class Message (object): self.packet.write(zero_byte) return self - def add_size(self, n): + def add_int(self, n): """ Add an integer to the stream. @@ -257,7 +244,7 @@ class Message (object): self.packet.write(struct.pack('>I', n)) return self - def add_int(self, n): + def add_adaptive_int(self, n): """ Add an integer to the stream. @@ -270,20 +257,6 @@ class Message (object): self.packet.write(struct.pack('>I', n)) return self - def add_int(self, n): - """ - Add an integer to the stream. - - @param n: integer to add - @type n: int - """ - if n >= Message.big_int: - self.packet.write(max_byte) - self.add_string(util.deflate_long(n)) - else: - self.packet.write(struct.pack('>I', n)) - return self - def add_int64(self, n): """ Add a 64-bit int to the stream. @@ -310,7 +283,7 @@ class Message (object): :param str s: string to add """ s = asbytes(s) - self.add_size(len(s)) + self.add_int(len(s)) self.packet.write(s) return self @@ -329,7 +302,7 @@ class Message (object): if type(i) is bool: return self.add_boolean(i) elif isinstance(i, integer_types): - return self.add_int(i) + return self.add_adaptive_int(i) elif type(i) is list: return self.add_list(i) else: diff --git a/paramiko/packet.py b/paramiko/packet.py index f516ff9b..b922000c 100644 --- a/paramiko/packet.py +++ b/paramiko/packet.py @@ -99,6 +99,10 @@ class Packetizer (object): self.__keepalive_last = time.time() self.__keepalive_callback = None + self.__timer = None + self.__handshake_complete = False + self.__timer_expired = False + def set_log(self, log): """ Set the Python log object to use for logging. @@ -182,6 +186,45 @@ class Packetizer (object): self.__keepalive_callback = callback self.__keepalive_last = time.time() + def read_timer(self): + self.__timer_expired = True + + def start_handshake(self, timeout): + """ + Tells `Packetizer` that the handshake process started. + Starts a book keeping timer that can signal a timeout in the + handshake process. + + :param float timeout: amount of seconds to wait before timing out + """ + if not self.__timer: + self.__timer = threading.Timer(float(timeout), self.read_timer) + self.__timer.start() + + def handshake_timed_out(self): + """ + Checks if the handshake has timed out. + If `start_handshake` wasn't called before the call to this function + the return value will always be `False`. + If the handshake completed before a time out was reached the return value will be `False` + + :return: handshake time out status, as a `bool` + """ + if not self.__timer: + return False + if self.__handshake_complete: + return False + return self.__timer_expired + + def complete_handshake(self): + """ + Tells `Packetizer` that the handshake has completed. + """ + if self.__timer: + self.__timer.cancel() + self.__timer_expired = False + self.__handshake_complete = True + def read_all(self, n, check_rekey=False): """ Read as close to N bytes as possible, blocking as long as necessary. @@ -200,6 +243,8 @@ class Packetizer (object): n -= len(out) while n > 0: got_timeout = False + if self.handshake_timed_out(): + raise EOFError() try: x = self.__socket.recv(n) if len(x) == 0: diff --git a/paramiko/ssh_exception.py b/paramiko/ssh_exception.py index b99e42b3..e120a45e 100644 --- a/paramiko/ssh_exception.py +++ b/paramiko/ssh_exception.py @@ -105,7 +105,11 @@ class BadHostKeyException (SSHException): .. versionadded:: 1.6 """ def __init__(self, hostname, got_key, expected_key): - SSHException.__init__(self, 'Host key for server %s does not match!' % hostname) + SSHException.__init__(self, + 'Host key for server %s does not match : got %s expected %s' % ( + hostname, + got_key.get_base64(), + expected_key.get_base64())) self.hostname = hostname self.key = got_key self.expected_key = expected_key diff --git a/paramiko/transport.py b/paramiko/transport.py index 36da3043..31c27a2f 100644 --- a/paramiko/transport.py +++ b/paramiko/transport.py @@ -295,6 +295,8 @@ class Transport (threading.Thread, ClosingContextManager): self.global_response = None # response Message from an arbitrary global request self.completion_event = None # user-defined event callbacks self.banner_timeout = 15 # how long (seconds) to wait for the SSH banner + self.handshake_timeout = 15 # how long (seconds) to wait for the handshake to finish after SSH banner sent. + # server mode: self.server_mode = False @@ -587,7 +589,7 @@ class Transport (threading.Thread, ClosingContextManager): """ return self.active - def open_session(self, window_size=None, max_packet_size=None): + def open_session(self, window_size=None, max_packet_size=None, timeout=None): """ Request a new channel to the server, of type ``"session"``. This is just an alias for calling `open_channel` with an argument of @@ -612,7 +614,8 @@ class Transport (threading.Thread, ClosingContextManager): """ return self.open_channel('session', window_size=window_size, - max_packet_size=max_packet_size) + max_packet_size=max_packet_size, + timeout=timeout) def open_x11_channel(self, src_addr=None): """ @@ -659,7 +662,8 @@ class Transport (threading.Thread, ClosingContextManager): dest_addr=None, src_addr=None, window_size=None, - max_packet_size=None): + max_packet_size=None, + timeout=None): """ Request a new channel to the server. `Channels <.Channel>` are socket-like objects used for the actual transfer of data across the @@ -683,17 +687,20 @@ class Transport (threading.Thread, ClosingContextManager): optional window size for this session. :param int max_packet_size: optional max packet size for this session. + :param float timeout: + optional timeout opening a channel, default 3600s (1h) :return: a new `.Channel` on success - :raises SSHException: if the request is rejected or the session ends - prematurely + :raises SSHException: if the request is rejected, the session ends + prematurely or there is a timeout openning a channel .. versionchanged:: 1.15 Added the ``window_size`` and ``max_packet_size`` arguments. """ if not self.active: raise SSHException('SSH session not active') + timeout = 3600 if timeout is None else timeout self.lock.acquire() try: window_size = self._sanitize_window_size(window_size) @@ -722,6 +729,7 @@ class Transport (threading.Thread, ClosingContextManager): finally: self.lock.release() self._send_user_message(m) + start_ts = time.time() while True: event.wait(0.1) if not self.active: @@ -731,6 +739,8 @@ class Transport (threading.Thread, ClosingContextManager): raise e if event.is_set(): break + elif start_ts + timeout < time.time(): + raise SSHException('Timeout openning channel.') chan = self._channels.get(chanid) if chan is not None: return chan @@ -1582,6 +1592,12 @@ class Transport (threading.Thread, ClosingContextManager): try: self.packetizer.write_all(b(self.local_version + '\r\n')) self._check_banner() + # The above is actually very much part of the handshake, but sometimes the banner can be read + # but the machine is not responding, for example when the remote ssh daemon is loaded in to memory + # but we can not read from the disk/spawn a new shell. + # Make sure we can specify a timeout for the initial handshake. + # Re-use the banner timeout for now. + self.packetizer.start_handshake(self.handshake_timeout) self._send_kex_init() self._expect_packet(MSG_KEXINIT) @@ -1631,6 +1647,7 @@ class Transport (threading.Thread, ClosingContextManager): msg.add_byte(cMSG_UNIMPLEMENTED) msg.add_int(m.seqno) self._send_message(msg) + self.packetizer.complete_handshake() except SSHException as e: self._log(ERROR, 'Exception: ' + str(e)) self._log(ERROR, util.tb_strings()) diff --git a/sites/www/changelog.rst b/sites/www/changelog.rst index 6520dde4..764c8801 100644 --- a/sites/www/changelog.rst +++ b/sites/www/changelog.rst @@ -2,6 +2,25 @@ Changelog ========= +* :bug:`491` (combines :issue:`62` and :issue:`439`) Implement timeout + functionality to address hangs from dropped network connections and/or failed + handshakes. Credit to ``@vazir`` and ``@dacut`` for the original patches and + to Olle Lundberg for reimplementation. +* :bug:`490` Skip invalid/unparseable lines in ``known_hosts`` files, instead + of raising `SSHException`. This brings Paramiko's behavior more in line with + OpenSSH, which silently ignores such input. Catch & patch courtesy of Martin + Topholm. +* :bug:`404` Print details when displaying `BadHostKeyException` objects + (expected vs received data) instead of just "hey shit broke". Patch credit: + Loic Dachary. +* :bug:`469` (also :issue:`488`, :issue:`461` and like a dozen others) Fix a + typo introduced in the 1.15 release which broke WinPageant support. Thanks to + everyone who submitted patches, and to Steve Cohen who was the lucky winner + of the cherry-pick lottery. +* :bug:`353` (via :issue:`482`) Fix a bug introduced in the Python 3 port + which caused ``OverFlowError`` (and other symptoms) in SFTP functionality. + Thanks to ``@dboreham`` for leading the troubleshooting charge, and to + Scott Maxwell for the final patch. * :bug:`402` Check to see if an SSH agent is actually present before trying to forward it to the remote end. This replaces what was usually a useless ``TypeError`` with a human-readable ``AuthenticationError``. Credit to Ken @@ -3,28 +3,10 @@ from os.path import join from shutil import rmtree, copytree from invoke import Collection, ctask as task -from invocations import docs as _docs +from invocations.docs import docs, www from invocations.packaging import publish -d = 'sites' - -# Usage doc/API site (published as docs.paramiko.org) -docs_path = join(d, 'docs') -docs_build = join(docs_path, '_build') -docs = Collection.from_module(_docs, name='docs', config={ - 'sphinx.source': docs_path, - 'sphinx.target': docs_build, -}) - -# Main/about/changelog site ((www.)?paramiko.org) -www_path = join(d, 'www') -www = Collection.from_module(_docs, name='www', config={ - 'sphinx.source': www_path, - 'sphinx.target': join(www_path, '_build'), -}) - - # Until we move to spec-based testing @task def test(ctx, coverage=False): @@ -35,6 +17,11 @@ def test(ctx, coverage=False): ctx.run("{0} test.py {1}".format(runner, flags), pty=True) +@task +def coverage(ctx): + ctx.run("coverage run --source=paramiko test.py --verbose") + + # Until we stop bundling docs w/ releases. Need to discover use cases first. @task def release(ctx): @@ -45,9 +32,9 @@ def release(ctx): rmtree(target, ignore_errors=True) copytree(docs_build, target) # Publish - publish(ctx, wheel=True) + publish(ctx) # Remind print("\n\nDon't forget to update RTD's versions page for new minor releases!") -ns = Collection(test, release, docs=docs, www=www) +ns = Collection(test, coverage, release, docs, www) diff --git a/tests/test_hostkeys.py b/tests/test_hostkeys.py index 0ee1bbf0..2bdcad9c 100644 --- a/tests/test_hostkeys.py +++ b/tests/test_hostkeys.py @@ -31,6 +31,7 @@ test_hosts_file = """\ secure.example.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAIEA1PD6U2/TVxET6lkpKhOk5r\ 9q/kAYG6sP9f5zuUYP8i7FOFp/6ncCEbbtg/lB+A3iidyxoSWl+9jtoyyDOOVX4UIDV9G11Ml8om3\ D+jrpI9cycZHqilK0HmxDeCuxbwyMuaCygU9gS2qoRvNLWZk70OpIKSSpBo0Wl3/XUmz9uhc= +broken.example.com ssh-rsa AAAA happy.example.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAIEA8bP1ZA7DCZDB9J0s50l31M\ BGQ3GQ/Fc7SX6gkpXkwcZryoi4kNFhHu5LvHcZPdxXV1D+uTMfGS1eyd2Yz/DoNWXNAl8TI0cAsW\ 5ymME3bQ4J/k1IKxCtz/bAlAqFgKoc+EolMziDYqWIATtW0rYTJvzGAzTmMj80/QpsFH+Pc2M= diff --git a/tests/test_message.py b/tests/test_message.py index f308c037..f18cae90 100644 --- a/tests/test_message.py +++ b/tests/test_message.py @@ -92,12 +92,12 @@ class MessageTest (unittest.TestCase): def test_4_misc(self): msg = Message(self.__d) - self.assertEqual(msg.get_int(), 5) - self.assertEqual(msg.get_int(), 0x1122334455) - self.assertEqual(msg.get_int(), 0xf00000000000000000) + self.assertEqual(msg.get_adaptive_int(), 5) + self.assertEqual(msg.get_adaptive_int(), 0x1122334455) + self.assertEqual(msg.get_adaptive_int(), 0xf00000000000000000) self.assertEqual(msg.get_so_far(), self.__d[:29]) self.assertEqual(msg.get_remainder(), self.__d[29:]) msg.rewind() - self.assertEqual(msg.get_int(), 5) + self.assertEqual(msg.get_adaptive_int(), 5) self.assertEqual(msg.get_so_far(), self.__d[:4]) self.assertEqual(msg.get_remainder(), self.__d[4:]) diff --git a/tests/test_transport.py b/tests/test_transport.py index 5cf9a867..3c8ad81e 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -792,3 +792,20 @@ class TransportTest(unittest.TestCase): (None, DEFAULT_WINDOW_SIZE), (2**32, MAX_WINDOW_SIZE)]: self.assertEqual(self.tc._sanitize_window_size(val), correct) + + def test_L_handshake_timeout(self): + """ + verify that we can get a hanshake timeout. + """ + host_key = RSAKey.from_private_key_file(test_path('test_rsa.key')) + public_host_key = RSAKey(data=host_key.asbytes()) + self.ts.add_server_key(host_key) + event = threading.Event() + server = NullServer() + self.assertTrue(not event.is_set()) + self.tc.handshake_timeout = 0.000000000001 + self.ts.start_server(event, server) + self.assertRaises(EOFError, self.tc.connect, + hostkey=public_host_key, + username='slowdive', + password='pygmalion') |