From a6614d38dd60f24c9ae5c7f2896ff8837bea3816 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Thu, 2 Feb 2023 08:24:51 -0800 Subject: [PATCH] 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 --- .../notes/add-ssh-allow-agent-2dee6448fd250e50.yaml | 10 ++++++++++ tempest/lib/common/ssh.py | 9 +++++++-- tempest/lib/common/utils/linux/remote_client.py | 10 ++++++++-- tempest/tests/lib/test_ssh.py | 12 ++++++++---- 4 files changed, 33 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/add-ssh-allow-agent-2dee6448fd250e50.yaml diff --git a/releasenotes/notes/add-ssh-allow-agent-2dee6448fd250e50.yaml b/releasenotes/notes/add-ssh-allow-agent-2dee6448fd250e50.yaml new file mode 100644 index 0000000000..33f11ced9e --- /dev/null +++ b/releasenotes/notes/add-ssh-allow-agent-2dee6448fd250e50.yaml @@ -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. diff --git a/tempest/lib/common/ssh.py b/tempest/lib/common/ssh.py index cb59a82861..aad04b8f1b 100644 --- a/tempest/lib/common/ssh.py +++ b/tempest/lib/common/ssh.py @@ -53,7 +53,8 @@ class Client(object): def __init__(self, host, username, password=None, timeout=300, pkey=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. 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 not to use ssh-over-ssh. :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 """ self.host = host @@ -105,6 +109,7 @@ class Client(object): raise exceptions.SSHClientProxyClientLoop( host=self.host, port=self.port, username=self.username) self._proxy_conn = None + self.ssh_allow_agent = ssh_allow_agent def _get_ssh_connection(self, sleep=1.5, backoff=1): """Returns an ssh connection to the specified host.""" @@ -133,7 +138,7 @@ class Client(object): look_for_keys=self.look_for_keys, key_filename=self.key_filename, 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", self.username, self.host) return ssh diff --git a/tempest/lib/common/utils/linux/remote_client.py b/tempest/lib/common/utils/linux/remote_client.py index d0cdc25d79..662b45235f 100644 --- a/tempest/lib/common/utils/linux/remote_client.py +++ b/tempest/lib/common/utils/linux/remote_client.py @@ -69,7 +69,8 @@ class RemoteClient(object): server=None, servers_client=None, ssh_timeout=300, connect_timeout=60, console_output_enabled=True, 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 :param ip_address: IP address to ssh to @@ -85,6 +86,8 @@ class RemoteClient(object): :param ping_count: Number of ping packets :param ping_size: Packet size for ping packets :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.servers_client = servers_client @@ -94,11 +97,14 @@ class RemoteClient(object): self.ping_count = ping_count self.ping_size = ping_size self.ssh_key_type = ssh_key_type + self.ssh_allow_agent = ssh_allow_agent self.ssh_client = ssh.Client(ip_address, username, password, ssh_timeout, pkey=pkey, channel_timeout=connect_timeout, - ssh_key_type=ssh_key_type) + ssh_key_type=ssh_key_type, + ssh_allow_agent=ssh_allow_agent, + ) @debug_ssh def exec_command(self, cmd): diff --git a/tempest/tests/lib/test_ssh.py b/tempest/tests/lib/test_ssh.py index 886d99c8bd..13870baf55 100644 --- a/tempest/tests/lib/test_ssh.py +++ b/tempest/tests/lib/test_ssh.py @@ -75,7 +75,8 @@ class TestSshClient(base.TestCase): look_for_keys=False, timeout=10.0, password=None, - sock=None + sock=None, + allow_agent=True )] self.assertEqual(expected_connect, client_mock.connect.mock_calls) 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) 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) aa_mock.assert_has_calls([mock.call(), mock.call()]) @@ -106,7 +108,8 @@ class TestSshClient(base.TestCase): look_for_keys=False, timeout=10.0, password=None, - sock=None + sock=None, + allow_agent=True )] self.assertEqual(proxy_expected_connect, proxy_client_mock.connect.mock_calls) @@ -121,7 +124,8 @@ class TestSshClient(base.TestCase): look_for_keys=False, timeout=10.0, 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(0, s_mock.call_count)