Allow SSH connection callers to not permit agent usage
While debugging the ``rescue`` test functionality with ironic's tempest plugin, we discovered that if the environment suggests the agent is available, then we may enter a situation where the test can fail because paramiko prefers ssh over password authentication. This is important, because for rescue functionality in particular, it is password authentication based without the use of SSH keys, as a temporary password is generated by the services and provided to the user requesting to rescue the instance/node. Instead of trying to make an assumption that password being present means we should just disable the agent, explicitly allow the caller to specify it. Change-Id: Iefb6cb5cb80eb2b9a4307912c4d6d07c684ed70a
This commit is contained in:
parent
7416b91cf7
commit
a6614d38dd
10
releasenotes/notes/add-ssh-allow-agent-2dee6448fd250e50.yaml
Normal file
10
releasenotes/notes/add-ssh-allow-agent-2dee6448fd250e50.yaml
Normal file
@ -0,0 +1,10 @@
|
|||||||
|
---
|
||||||
|
features:
|
||||||
|
- |
|
||||||
|
Adds a ``ssh_allow_agent`` parameter to the ``RemoteClient`` class
|
||||||
|
wrapper and the direct ssh ``Client`` class to allow a caller to
|
||||||
|
explicitly request that the SSH Agent is not consulted for
|
||||||
|
authentication. This is useful if your attempting explicit password
|
||||||
|
based authentication as ``paramiko``, the underlying library used for
|
||||||
|
SSH, defaults to utilizing an ssh-agent process before attempting
|
||||||
|
password authentication.
|
@ -53,7 +53,8 @@ class Client(object):
|
|||||||
|
|
||||||
def __init__(self, host, username, password=None, timeout=300, pkey=None,
|
def __init__(self, host, username, password=None, timeout=300, pkey=None,
|
||||||
channel_timeout=10, look_for_keys=False, key_filename=None,
|
channel_timeout=10, look_for_keys=False, key_filename=None,
|
||||||
port=22, proxy_client=None, ssh_key_type='rsa'):
|
port=22, proxy_client=None, ssh_key_type='rsa',
|
||||||
|
ssh_allow_agent=True):
|
||||||
"""SSH client.
|
"""SSH client.
|
||||||
|
|
||||||
Many of parameters are just passed to the underlying implementation
|
Many of parameters are just passed to the underlying implementation
|
||||||
@ -76,6 +77,9 @@ class Client(object):
|
|||||||
for ssh-over-ssh. The default is None, which means
|
for ssh-over-ssh. The default is None, which means
|
||||||
not to use ssh-over-ssh.
|
not to use ssh-over-ssh.
|
||||||
:param ssh_key_type: ssh key type (rsa, ecdsa)
|
:param ssh_key_type: ssh key type (rsa, ecdsa)
|
||||||
|
:param ssh_allow_agent: boolean, default True, if the SSH client is
|
||||||
|
allowed to also utilize the ssh-agent. Explicit use of passwords
|
||||||
|
in some tests may need this set as False.
|
||||||
:type proxy_client: ``tempest.lib.common.ssh.Client`` object
|
:type proxy_client: ``tempest.lib.common.ssh.Client`` object
|
||||||
"""
|
"""
|
||||||
self.host = host
|
self.host = host
|
||||||
@ -105,6 +109,7 @@ class Client(object):
|
|||||||
raise exceptions.SSHClientProxyClientLoop(
|
raise exceptions.SSHClientProxyClientLoop(
|
||||||
host=self.host, port=self.port, username=self.username)
|
host=self.host, port=self.port, username=self.username)
|
||||||
self._proxy_conn = None
|
self._proxy_conn = None
|
||||||
|
self.ssh_allow_agent = ssh_allow_agent
|
||||||
|
|
||||||
def _get_ssh_connection(self, sleep=1.5, backoff=1):
|
def _get_ssh_connection(self, sleep=1.5, backoff=1):
|
||||||
"""Returns an ssh connection to the specified host."""
|
"""Returns an ssh connection to the specified host."""
|
||||||
@ -133,7 +138,7 @@ class Client(object):
|
|||||||
look_for_keys=self.look_for_keys,
|
look_for_keys=self.look_for_keys,
|
||||||
key_filename=self.key_filename,
|
key_filename=self.key_filename,
|
||||||
timeout=self.channel_timeout, pkey=self.pkey,
|
timeout=self.channel_timeout, pkey=self.pkey,
|
||||||
sock=proxy_chan)
|
sock=proxy_chan, allow_agent=self.ssh_allow_agent)
|
||||||
LOG.info("ssh connection to %s@%s successfully created",
|
LOG.info("ssh connection to %s@%s successfully created",
|
||||||
self.username, self.host)
|
self.username, self.host)
|
||||||
return ssh
|
return ssh
|
||||||
|
@ -69,7 +69,8 @@ class RemoteClient(object):
|
|||||||
server=None, servers_client=None, ssh_timeout=300,
|
server=None, servers_client=None, ssh_timeout=300,
|
||||||
connect_timeout=60, console_output_enabled=True,
|
connect_timeout=60, console_output_enabled=True,
|
||||||
ssh_shell_prologue="set -eu -o pipefail; PATH=$PATH:/sbin;",
|
ssh_shell_prologue="set -eu -o pipefail; PATH=$PATH:/sbin;",
|
||||||
ping_count=1, ping_size=56, ssh_key_type='rsa'):
|
ping_count=1, ping_size=56, ssh_key_type='rsa',
|
||||||
|
ssh_allow_agent=True):
|
||||||
"""Executes commands in a VM over ssh
|
"""Executes commands in a VM over ssh
|
||||||
|
|
||||||
:param ip_address: IP address to ssh to
|
:param ip_address: IP address to ssh to
|
||||||
@ -85,6 +86,8 @@ class RemoteClient(object):
|
|||||||
:param ping_count: Number of ping packets
|
:param ping_count: Number of ping packets
|
||||||
:param ping_size: Packet size for ping packets
|
:param ping_size: Packet size for ping packets
|
||||||
:param ssh_key_type: ssh key type (rsa, ecdsa)
|
:param ssh_key_type: ssh key type (rsa, ecdsa)
|
||||||
|
:param ssh_allow_agent: Boolean if ssh agent support is permitted.
|
||||||
|
Defaults to True.
|
||||||
"""
|
"""
|
||||||
self.server = server
|
self.server = server
|
||||||
self.servers_client = servers_client
|
self.servers_client = servers_client
|
||||||
@ -94,11 +97,14 @@ class RemoteClient(object):
|
|||||||
self.ping_count = ping_count
|
self.ping_count = ping_count
|
||||||
self.ping_size = ping_size
|
self.ping_size = ping_size
|
||||||
self.ssh_key_type = ssh_key_type
|
self.ssh_key_type = ssh_key_type
|
||||||
|
self.ssh_allow_agent = ssh_allow_agent
|
||||||
|
|
||||||
self.ssh_client = ssh.Client(ip_address, username, password,
|
self.ssh_client = ssh.Client(ip_address, username, password,
|
||||||
ssh_timeout, pkey=pkey,
|
ssh_timeout, pkey=pkey,
|
||||||
channel_timeout=connect_timeout,
|
channel_timeout=connect_timeout,
|
||||||
ssh_key_type=ssh_key_type)
|
ssh_key_type=ssh_key_type,
|
||||||
|
ssh_allow_agent=ssh_allow_agent,
|
||||||
|
)
|
||||||
|
|
||||||
@debug_ssh
|
@debug_ssh
|
||||||
def exec_command(self, cmd):
|
def exec_command(self, cmd):
|
||||||
|
@ -75,7 +75,8 @@ class TestSshClient(base.TestCase):
|
|||||||
look_for_keys=False,
|
look_for_keys=False,
|
||||||
timeout=10.0,
|
timeout=10.0,
|
||||||
password=None,
|
password=None,
|
||||||
sock=None
|
sock=None,
|
||||||
|
allow_agent=True
|
||||||
)]
|
)]
|
||||||
self.assertEqual(expected_connect, client_mock.connect.mock_calls)
|
self.assertEqual(expected_connect, client_mock.connect.mock_calls)
|
||||||
self.assertEqual(0, s_mock.call_count)
|
self.assertEqual(0, s_mock.call_count)
|
||||||
@ -91,7 +92,8 @@ class TestSshClient(base.TestCase):
|
|||||||
|
|
||||||
proxy_client = ssh.Client('proxy-host', 'proxy-user', timeout=2)
|
proxy_client = ssh.Client('proxy-host', 'proxy-user', timeout=2)
|
||||||
client = ssh.Client('localhost', 'root', timeout=2,
|
client = ssh.Client('localhost', 'root', timeout=2,
|
||||||
proxy_client=proxy_client)
|
proxy_client=proxy_client,
|
||||||
|
ssh_allow_agent=False)
|
||||||
client._get_ssh_connection(sleep=1)
|
client._get_ssh_connection(sleep=1)
|
||||||
|
|
||||||
aa_mock.assert_has_calls([mock.call(), mock.call()])
|
aa_mock.assert_has_calls([mock.call(), mock.call()])
|
||||||
@ -106,7 +108,8 @@ class TestSshClient(base.TestCase):
|
|||||||
look_for_keys=False,
|
look_for_keys=False,
|
||||||
timeout=10.0,
|
timeout=10.0,
|
||||||
password=None,
|
password=None,
|
||||||
sock=None
|
sock=None,
|
||||||
|
allow_agent=True
|
||||||
)]
|
)]
|
||||||
self.assertEqual(proxy_expected_connect,
|
self.assertEqual(proxy_expected_connect,
|
||||||
proxy_client_mock.connect.mock_calls)
|
proxy_client_mock.connect.mock_calls)
|
||||||
@ -121,7 +124,8 @@ class TestSshClient(base.TestCase):
|
|||||||
look_for_keys=False,
|
look_for_keys=False,
|
||||||
timeout=10.0,
|
timeout=10.0,
|
||||||
password=None,
|
password=None,
|
||||||
sock=proxy_client_mock.get_transport().open_session()
|
sock=proxy_client_mock.get_transport().open_session(),
|
||||||
|
allow_agent=False
|
||||||
)]
|
)]
|
||||||
self.assertEqual(expected_connect, client_mock.connect.mock_calls)
|
self.assertEqual(expected_connect, client_mock.connect.mock_calls)
|
||||||
self.assertEqual(0, s_mock.call_count)
|
self.assertEqual(0, s_mock.call_count)
|
||||||
|
Loading…
Reference in New Issue
Block a user