Skip to content

Commit

Permalink
Add new connect_timeout connection option
Browse files Browse the repository at this point in the history 
This commit adds a new connect_timeout SSH connection option. It
applies to both client and server connections, whenever an outbound
TCP connection is being made. It also applies when opening a tunnel
which is then used to set up a client or server listener, and when
requesting SSH server host keys.

As part of this change, the existing ConnectTimeout config file option
now sets this new connect_timeout value instead of login_timeout,
which only applies after a TCP connection is established. The behavior
of this new option is more consistent with OpenSSH's ConnectTimeout
behavior.

A note has also been added to the documentation for login_timeout to
recommend the use of this new connect_timeout option when wanting
the timeout to include the time to establish the TCP connection.
  • Loading branch information
@ronf
ronf committed Oct 30, 2021
1 parent 16becd8 commit b2cadab
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 24 deletions.
92 changes: 68 additions & 24 deletions asyncssh/connection.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,6 @@ async def _connect(options, passphrase, loop, flags, conn_factory, msg):
family=family, flags=flags,
local_addr=local_addr)

# pylint: disable=broad-except
try:
await conn.wait_established()
free_conn = False
Expand Down Expand Up @@ -5602,8 +5601,8 @@ def prepare(self, config, protocol_factory, version, host, port, tunnel,
encryption_algs, mac_algs, compression_algs, signature_algs,
host_based_auth, public_key_auth, kbdint_auth, password_auth,
x509_trusted_certs, x509_trusted_cert_paths, x509_purposes,
rekey_bytes, rekey_seconds, login_timeout, keepalive_interval,
keepalive_count_max):
rekey_bytes, rekey_seconds, connect_timeout, login_timeout,
keepalive_interval, keepalive_count_max):
"""Prepare common connection configuration options"""

self.config = config
Expand Down Expand Up @@ -5689,6 +5688,12 @@ def prepare(self, config, protocol_factory, version, host, port, tunnel,
if rekey_seconds and rekey_seconds <= 0:
raise ValueError('Rekey seconds cannot be negative or zero')

if isinstance(connect_timeout, str):
connect_timeout = parse_time_interval(connect_timeout)

if connect_timeout and connect_timeout < 0:
raise ValueError('Connect timeout cannot be negative')

if isinstance(login_timeout, str):
login_timeout = parse_time_interval(login_timeout)

Expand All @@ -5706,6 +5711,7 @@ def prepare(self, config, protocol_factory, version, host, port, tunnel,

self.rekey_bytes = int(rekey_bytes)
self.rekey_seconds = rekey_seconds
self.connect_timeout = connect_timeout or None
self.login_timeout = login_timeout
self.keepalive_interval = keepalive_interval
self.keepalive_count_max = keepalive_count_max
Expand Down Expand Up @@ -5955,10 +5961,22 @@ class SSHClientConnectionOptions(SSHConnectionOptions):
:param rekey_seconds: (optional)
The maximum time in seconds before the SSH session key is
renegotiated. This defaults to 1 hour.
:param connect_timeout: (optional)
The maximum time in seconds allowed to complete an outbound
SSH connection. This includes the time to establish the TCP
connection and the time to perform the initial SSH protocol
handshake, key exchange, and authentication. This is disabled
by default, relying on the system's default TCP connect timeout
and AsyncSSH's login timeout.
:param login_timeout: (optional)
The maximum time in seconds allowed for authentication to
complete, defaulting to 2 minutes. Setting this to 0 will
disable the login timeout.
.. note:: This timeout only applies after the SSH TCP
connection is established. To set a timeout
which includes establishing the TCP connection,
use the `connect_timeout` argument above.
:param keepalive_interval: (optional)
The time in seconds to wait before sending a keepalive message
if no data has been received from the server. This defaults to
Expand Down Expand Up @@ -6101,6 +6119,7 @@ class SSHClientConnectionOptions(SSHConnectionOptions):
:type signature_algs: `str` or `list` of `str`
:type rekey_bytes: *see* :ref:`SpecifyingByteCounts`
:type rekey_seconds: *see* :ref:`SpecifyingTimeIntervals`
:type connect_timeout: *see* :ref:`SpecifyingTimeIntervals`
:type login_timeout: *see* :ref:`SpecifyingTimeIntervals`
:type keepalive_interval: *see* :ref:`SpecifyingTimeIntervals`
:type keepalive_count_max: `int`
Expand Down Expand Up @@ -6134,7 +6153,8 @@ def prepare(self, last_config=None, config=(), reload=False,
public_key_auth=(), kbdint_auth=(), password_auth=(),
x509_trusted_certs=(), x509_trusted_cert_paths=(),
x509_purposes='secureShellServer', rekey_bytes=(),
rekey_seconds=(), login_timeout=(), keepalive_interval=(),
rekey_seconds=(), connect_timeout=(),
login_timeout=_DEFAULT_LOGIN_TIMEOUT, keepalive_interval=(),
keepalive_count_max=(), known_hosts=(), host_key_alias=None,
server_host_key_algs=(), username=(), password=None,
client_host_keysign=(), client_host_keys=None,
Expand Down Expand Up @@ -6179,9 +6199,8 @@ def prepare(self, last_config=None, config=(), reload=False,
if default_x509_cert_path.is_dir():
x509_trusted_cert_paths = [str(default_x509_cert_path)]

if login_timeout == ():
login_timeout = config.get('ConnectTimeout',
_DEFAULT_LOGIN_TIMEOUT)
if connect_timeout == ():
connect_timeout = config.get('ConnectTimeout', None)

if keepalive_interval == ():
keepalive_interval = config.get('ServerAliveInterval',
Expand All @@ -6198,7 +6217,8 @@ def prepare(self, last_config=None, config=(), reload=False,
public_key_auth, kbdint_auth, password_auth,
x509_trusted_certs, x509_trusted_cert_paths,
x509_purposes, rekey_bytes, rekey_seconds,
login_timeout, keepalive_interval, keepalive_count_max)
connect_timeout, login_timeout, keepalive_interval,
keepalive_count_max)

if known_hosts == ():
known_hosts = (config.get('UserKnownHostsFile', []) + \
Expand Down Expand Up @@ -6550,10 +6570,22 @@ class SSHServerConnectionOptions(SSHConnectionOptions):
:param rekey_seconds: (optional)
The maximum time in seconds before the SSH session key is
renegotiated, defaulting to 1 hour
:param connect_timeout: (optional)
The maximum time in seconds allowed to complete an outbound
SSH connection. This includes the time to establish the TCP
connection and the time to perform the initial SSH protocol
handshake, key exchange, and authentication. This is disabled
by default, relying on the system's default TCP connect timeout
and AsyncSSH's login timeout.
:param login_timeout: (optional)
The maximum time in seconds allowed for authentication to
complete, defaulting to 2 minutes. Setting this to 0
will disable the login timeout.
.. note:: This timeout only applies after the SSH TCP
connection is established. To set a timeout
which includes establishing the TCP connection,
use the `connect_timeout` argument above.
:param keepalive_interval: (optional)
The time in seconds to wait before sending a keepalive message
if no data has been received from the client. This defaults to
Expand Down Expand Up @@ -6631,6 +6663,7 @@ class SSHServerConnectionOptions(SSHConnectionOptions):
:type signature_algs: `str` or `list` of `str`
:type rekey_bytes: *see* :ref:`SpecifyingByteCounts`
:type rekey_seconds: *see* :ref:`SpecifyingTimeIntervals`
:type connect_timeout: *see* :ref:`SpecifyingTimeIntervals`
:type login_timeout: *see* :ref:`SpecifyingTimeIntervals`
:type keepalive_interval: *see* :ref:`SpecifyingTimeIntervals`
:type keepalive_count_max: `int`
Expand All @@ -6649,9 +6682,10 @@ def prepare(self, last_config=None, config=(), reload=False,
signature_algs=(), host_based_auth=(), public_key_auth=(),
kbdint_auth=(), password_auth=(), x509_trusted_certs=(),
x509_trusted_cert_paths=(), x509_purposes='secureShellClient',
rekey_bytes=(), rekey_seconds=(), login_timeout=(),
keepalive_interval=(), keepalive_count_max=(),
server_host_keys=(), server_host_certs=(), passphrase=None,
rekey_bytes=(), rekey_seconds=(), connect_timeout=None,
login_timeout=(), keepalive_interval=(),
keepalive_count_max=(), server_host_keys=(),
server_host_certs=(), passphrase=None,
known_client_hosts=None, trust_client_host=False,
authorized_client_keys=(), gss_host=(), gss_kex=(),
gss_auth=(), allow_pty=(), line_editor=True,
Expand Down Expand Up @@ -6686,7 +6720,8 @@ def prepare(self, last_config=None, config=(), reload=False,
public_key_auth, kbdint_auth, password_auth,
x509_trusted_certs, x509_trusted_cert_paths,
x509_purposes, rekey_bytes, rekey_seconds,
login_timeout, keepalive_interval, keepalive_count_max)
connect_timeout, login_timeout, keepalive_interval,
keepalive_count_max)

if server_host_keys == ():
server_host_keys = config.get('HostKey')
Expand Down Expand Up @@ -6854,8 +6889,10 @@ def conn_factory():
family=family, local_addr=local_addr,
**kwargs)

return await _connect(options, passphrase, loop, flags, conn_factory,
'Opening SSH connection to')
return await asyncio.wait_for(
_connect(options, passphrase, loop, flags, conn_factory,
'Opening SSH connection to'),
timeout=options.connect_timeout)


@async_context_manager
Expand Down Expand Up @@ -6931,8 +6968,10 @@ def conn_factory():
family=family, local_addr=local_addr,
**kwargs)

return await _connect(options, passphrase, loop, flags, conn_factory,
'Opening reverse SSH connection to')
return await asyncio.wait_for(
_connect(options, passphrase, loop, flags, conn_factory,
'Opening reverse SSH connection to'),
timeout=options.connect_timeout)


@async_context_manager
Expand Down Expand Up @@ -7029,9 +7068,10 @@ def conn_factory():
# pylint: disable=attribute-defined-outside-init
options.proxy_command = None

return await _listen(options, passphrase, loop, flags, backlog,
reuse_address, reuse_port, conn_factory,
'Creating SSH listener on')
return await asyncio.wait_for(
_listen(options, passphrase, loop, flags, backlog, reuse_address,
reuse_port, conn_factory, 'Creating SSH listener on'),
timeout=options.connect_timeout)


@async_context_manager
Expand Down Expand Up @@ -7141,9 +7181,11 @@ def conn_factory():
# pylint: disable=attribute-defined-outside-init
options.proxy_command = None

return await _listen(options, passphrase, loop, flags, backlog,
reuse_address, reuse_port, conn_factory,
'Creating reverse direction SSH listener on')
return await asyncio.wait_for(
_listen(options, passphrase, loop, flags, backlog,
reuse_address, reuse_port, conn_factory,
'Creating reverse direction SSH listener on'),
timeout=options.connect_timeout)


async def create_connection(client_factory, host, port=(), **kwargs):
Expand Down Expand Up @@ -7287,8 +7329,10 @@ def conn_factory():
x509_purposes='any', gss_host=None, kex_algs=kex_algs,
client_version=client_version)

conn = await _connect(options, passphrase, loop, flags, conn_factory,
'Fetching server host key from')
conn = await asyncio.wait_for(
_connect(options, passphrase, loop, flags, conn_factory,
'Fetching server host key from'),
timeout=options.connect_timeout)

server_host_key = conn.get_server_host_key()

Expand Down
29 changes: 29 additions & 0 deletions tests/test_connection.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,35 @@ async def test_connect_failure_without_agent(self):
with self.assertRaises(OSError):
await asyncssh.connect('0.0.0.1', agent_path=None)

@asynctest
async def test_connect_timeout_exceeded(self):
"""Test connect timeout exceeded"""

with self.assertRaises(asyncio.TimeoutError):
await asyncssh.connect('223.255.255.254', connect_timeout=1)

@asynctest
async def test_connect_timeout_exceeded_string(self):
"""Test connect timeout exceeded with string value"""

with self.assertRaises(asyncio.TimeoutError):
await asyncssh.connect('223.255.255.254', connect_timeout='0m1s')

@asynctest
async def test_connect_timeout_exceeded_tunnel(self):
"""Test connect timeout exceeded"""

with self.assertRaises(asyncio.TimeoutError):
await asyncssh.listen(server_host_keys=['skey'],
tunnel='223.255.255.254', connect_timeout=1)

@asynctest
async def test_invalid_connect_timeout(self):
"""Test invalid connect timeout"""

with self.assertRaises(ValueError):
await self.connect(connect_timeout=-1)

@asynctest
async def test_connect_tcp_keepalive_off(self):
"""Test connecting with TCP keepalive disabled"""
Expand Down

0 comments on commit b2cadab

Please sign in to comment.