[crmsh-4.6] Fix: bootstrap: ssh public key should be copied to qnetd node when ssh-agent feature is not enabled (bsc#1228950)

crmsh/bootstrap.py

@@ -885,14 +885,15 @@ def init_ssh_impl(local_user: str, ssh_public_keys: typing.List[ssh_key.Key], us
                 logger.info("Adding public keys to authorized_keys on %s@%s", user, node)
                 for key in ssh_public_keys:
                     authorized_key_manager.add(node, local_user, key)
-                if user != 'root' and 0 != shell.subprocess_run_without_input(
-                        node, user, 'sudo true',
-                        stdout=subprocess.DEVNULL,
-                        stderr=subprocess.DEVNULL,
-                ).returncode:
-                    raise ValueError(f'Failed to sudo on {user}@{node}')
         else:
             _init_ssh_on_remote_nodes(local_user, user_node_list)
+        for user, node in user_node_list:
+            if user != 'root' and 0 != shell.subprocess_run_without_input(
+                    node, user, 'sudo true',
+                    stdout=subprocess.DEVNULL,
+                    stderr=subprocess.DEVNULL,
+            ).returncode:
+                raise ValueError(f'Failed to sudo on {user}@{node}')
         for user, node in user_node_list:
             user_by_host.add(user, node)
         user_by_host.add(local_user, utils.this_node())
@@ -1624,19 +1625,21 @@ def _setup_passwordless_ssh_for_qnetd(cluster_node_list: typing.List[str]):
                 'root',
             )).add(qnetd_addr, qnetd_user, key)
     else:
-        if utils.check_ssh_passwd_need(local_user, qnetd_user, qnetd_addr):
-            if 0 != utils.ssh_copy_id_no_raise(local_user, qnetd_user, qnetd_addr):
-                msg = f"Failed to login to {qnetd_user}@{qnetd_addr}. Please check the credentials."
-                sudoer = userdir.get_sudoer()
-                if sudoer and qnetd_user != sudoer:
-                    args = ['sudo crm']
-                    args += [x for x in sys.argv[1:]]
-                    for i, arg in enumerate(args):
-                        if arg == '--qnetd-hostname' and i + 1 < len(args):
-                            if '@' not in args[i + 1]:
-                                args[i + 1] = f'{sudoer}@{qnetd_addr}'
-                                msg += '\nOr, run "{}".'.format(' '.join(args))
-                raise ValueError(msg)
+        if 0 != utils.ssh_copy_id_no_raise(
+                local_user, qnetd_user, qnetd_addr,
+                sh.LocalShell(additional_environ={'SSH_AUTH_SOCK': ''}),
+        ):
+            msg = f"Failed to login to {qnetd_user}@{qnetd_addr}. Please check the credentials."
+            sudoer = userdir.get_sudoer()
+            if sudoer and qnetd_user != sudoer:
+                args = ['sudo crm']
+                args += [x for x in sys.argv[1:]]
+                for i, arg in enumerate(args):
+                    if arg == '--qnetd-hostname' and i + 1 < len(args):
+                        if '@' not in args[i + 1]:
+                            args[i + 1] = f'{sudoer}@{qnetd_addr}'
+                            msg += '\nOr, run "{}".'.format(' '.join(args))
+            raise ValueError(msg)
 
         cluster_shell = sh.cluster_shell()
         # Add other nodes' public keys to qnetd's authorized_keys
@@ -1710,9 +1713,9 @@ def join_ssh_impl(local_user, seed_host, seed_user, ssh_public_keys: typing.List
         local_shell = sh.LocalShell(additional_environ={'SSH_AUTH_SOCK': os.environ.get('SSH_AUTH_SOCK')})
         join_ssh_with_ssh_agent(local_shell, local_user, seed_host, seed_user, ssh_public_keys)
     else:
-        local_shell = sh.LocalShell()
+        local_shell = sh.LocalShell(additional_environ={'SSH_AUTH_SOCK': ''})
         configure_ssh_key(local_user)
-        if 0 != utils.ssh_copy_id_no_raise(local_user, seed_user, seed_host):
+        if 0 != utils.ssh_copy_id_no_raise(local_user, seed_user, seed_host, local_shell):
             msg = f"Failed to login to {seed_user}@{seed_host}. Please check the credentials."
             sudoer = userdir.get_sudoer()
             if sudoer and seed_user != sudoer:
@@ -1726,6 +1729,13 @@ def join_ssh_impl(local_user, seed_host, seed_user, ssh_public_keys: typing.List
             raise ValueError(msg)
         # After this, login to remote_node is passwordless
         swap_public_ssh_key(seed_host, local_user, seed_user, local_user, seed_user, add=True)
+    ssh_shell = sh.SSHShell(local_shell, local_user)
+    if seed_user != 'root' and 0 != ssh_shell.subprocess_run_without_input(
+            seed_host, seed_user, 'sudo true',
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+    ).returncode:
+        raise ValueError(f'Failed to sudo on {seed_user}@{seed_host}')
 
     user_by_host = utils.HostUserConfig()
     user_by_host.clear()
@@ -1756,12 +1766,6 @@ def join_ssh_with_ssh_agent(
     if not shell.can_run_as(seed_host, seed_user):
         for key in ssh_public_keys:
             authorized_key_manager.add(seed_host, seed_user, key)
-    if seed_user != 'root' and 0 != shell.subprocess_run_without_input(
-            seed_host, seed_user, 'sudo true',
-            stdout=subprocess.DEVNULL,
-            stderr=subprocess.DEVNULL,
-    ).returncode:
-        raise ValueError(f'Failed to sudo on {seed_user}@{seed_host}')
     for key in ssh_public_keys:
         authorized_key_manager.add(None, local_user, key)
 
@@ -2911,7 +2915,10 @@ def bootstrap_join_geo(context):
             join_ssh_with_ssh_agent(local_shell, local_user, node, remote_user, keys)
         else:
             configure_ssh_key(local_user)
-            if 0 != utils.ssh_copy_id_no_raise(local_user, remote_user, node):
+            if 0 != utils.ssh_copy_id_no_raise(
+                    local_user, remote_user, node,
+                    sh.LocalShell(additional_environ={'SSH_AUTH_SOCK': ''}),
+            ):
                 raise ValueError(f"Failed to login to {remote_user}@{node}. Please check the credentials.")
             swap_public_ssh_key(node, local_user, remote_user, local_user, remote_user, add=True)
         user_by_host = utils.HostUserConfig()
@@ -2947,7 +2954,10 @@ def bootstrap_arbitrator(context):
             join_ssh_with_ssh_agent(local_shell, local_user, node, remote_user, keys)
         else:
             configure_ssh_key(local_user)
-            if 0 != utils.ssh_copy_id_no_raise(local_user, remote_user, node):
+            if 0 != utils.ssh_copy_id_no_raise(
+                    local_user, remote_user, node,
+                    sh.LocalShell(additional_environ={'SSH_AUTH_SOCK': ''}),
+            ):
                 raise ValueError(f"Failed to login to {remote_user}@{node}. Please check the credentials.")
             swap_public_ssh_key(node, local_user, remote_user, local_user, remote_user, add=True)
         user_by_host.add(local_user, utils.this_node())

crmsh/utils.py

@@ -126,11 +126,13 @@ def user_pair_for_ssh(host):
         raise ValueError('Can not create ssh session from {} to {}.'.format(this_node(), host))
 
 
-def ssh_copy_id_no_raise(local_user, remote_user, remote_node):
-    if check_ssh_passwd_need(local_user, remote_user, remote_node):
+def ssh_copy_id_no_raise(local_user, remote_user, remote_node, shell: sh.LocalShell = None):
+    if shell is None:
+        shell = sh.LocalShell()
+    if check_ssh_passwd_need(local_user, remote_user, remote_node, shell):
         logger.info("Configuring SSH passwordless with {}@{}".format(remote_user, remote_node))
         cmd = "ssh-copy-id -i ~/.ssh/id_rsa.pub '{}@{}' &> /dev/null".format(remote_user, remote_node)
-        result = sh.LocalShell().su_subprocess_run(local_user, cmd, tty=True)
+        result = shell.su_subprocess_run(local_user, cmd, tty=True)
         return result.returncode
     else:
         return 0
@@ -2138,23 +2140,18 @@ def debug_timestamp():
     return datetime.datetime.now().strftime('%Y/%m/%d %H:%M:%S')
 
 
-def check_ssh_passwd_need(local_user, remote_user, host):
+def check_ssh_passwd_need(local_user, remote_user, host, shell: sh.LocalShell = None):
     """
     Check whether access to host need password
     """
     ssh_options = "-o StrictHostKeyChecking=no -o EscapeChar=none -o ConnectTimeout=15"
-    ssh_cmd = "{} ssh {} -T -o Batchmode=yes {}@{} true".format(get_ssh_agent_str(), ssh_options, remote_user, host)
-    rc, _ = sh.LocalShell().get_rc_and_error(local_user, ssh_cmd)
+    ssh_cmd = "ssh {} -T -o Batchmode=yes {}@{} true".format(ssh_options, remote_user, host)
+    if shell is None:
+        shell = sh.LocalShell()
+    rc, _ = shell.get_rc_and_error(local_user, ssh_cmd)
     return rc != 0
 
 
-def get_ssh_agent_str():
-    ssh_agent_str = ""
-    if crmsh.user_of_host.instance().use_ssh_agent():
-        ssh_agent_str = f"SSH_AUTH_SOCK={os.environ.get('SSH_AUTH_SOCK')}"
-    return ssh_agent_str
-
-
 def check_port_open(ip, port):
     import socket
 

test/unittests/test_bootstrap.py

@@ -554,29 +554,44 @@ def test_join_ssh_no_seed_host(self, mock_error):
     @mock.patch('crmsh.bootstrap.get_node_canonical_hostname')
     @mock.patch('crmsh.bootstrap.swap_public_ssh_key_for_secondary_user')
     @mock.patch('crmsh.bootstrap.change_user_shell')
+    @mock.patch('crmsh.sh.SSHShell')
     @mock.patch('crmsh.bootstrap.swap_public_ssh_key')
     @mock.patch('crmsh.utils.ssh_copy_id_no_raise')
     @mock.patch('crmsh.bootstrap.configure_ssh_key')
+    @mock.patch('crmsh.sh.LocalShell')
     @mock.patch('crmsh.service_manager.ServiceManager.start_service')
     def test_join_ssh(
             self,
-            mock_start_service, mock_config_ssh, mock_ssh_copy_id, mock_swap, mock_change, mock_swap_2,
+            mock_start_service, mock_local_shell, mock_config_ssh, mock_ssh_copy_id, mock_swap,
+            mock_ssh_shell,
+            mock_change, mock_swap_2,
             mock_get_node_cononical_hostname,
             mock_detect_cluster_service_on_node
     ):
         bootstrap._context = mock.Mock(current_user="bob", default_nic="eth1", use_ssh_agent=False, stage=None)
         mock_swap.return_value = None
         mock_ssh_copy_id.return_value = 0
+        mock_subprocess_run_without_input = mock_ssh_shell.return_value.subprocess_run_without_input
+        mock_subprocess_run_without_input.return_value = mock.Mock(returncode=0)
         mock_get_node_cononical_hostname.return_value='node1'
 
         bootstrap.join_ssh("node1", "alice")
 
         mock_start_service.assert_called_once_with("sshd.service", enable=True)
+        mock_local_shell: mock.MagicMock
+        mock_local_shell.assert_has_calls([
+            mock.call(additional_environ={'SSH_AUTH_SOCK': ''}),
+        ])
         mock_config_ssh.assert_has_calls([
                 mock.call("bob"),
                 mock.call("hacluster"),
             ])
-        mock_ssh_copy_id.assert_called_once_with("bob", "alice", "node1")
+        mock_ssh_copy_id.assert_called_once_with("bob", "alice", "node1", mock_local_shell.return_value)
+        mock_subprocess_run_without_input.assert_called_once_with(
+            'node1', 'alice', 'sudo true',
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+        )
         mock_swap.assert_called_once_with("node1", "bob", "alice", "bob", "alice", add=True)
         mock_swap_2.assert_called_once()
         args, kwargs = mock_swap_2.call_args
@@ -622,8 +637,9 @@ def test_swap_public_ssh_key_for_secondary_user(
     @mock.patch('crmsh.bootstrap.swap_public_ssh_key')
     @mock.patch('crmsh.utils.ssh_copy_id_no_raise')
     @mock.patch('crmsh.bootstrap.configure_ssh_key')
+    @mock.patch('crmsh.sh.LocalShell')
     @mock.patch('crmsh.service_manager.ServiceManager.start_service')
-    def test_join_ssh_bad_credential(self, mock_start_service, mock_config_ssh, mock_ssh_copy_id, mock_swap, mock_invoke, mock_change):
+    def test_join_ssh_bad_credential(self, mock_start_service, mock_local_shell, mock_config_ssh, mock_ssh_copy_id, mock_swap, mock_invoke, mock_change):
         bootstrap._context = mock.Mock(current_user="bob", default_nic_list=["eth1"], use_ssh_agent=False)
         mock_invoke.return_value = ''
         mock_swap.return_value = None
@@ -633,10 +649,13 @@ def test_join_ssh_bad_credential(self, mock_start_service, mock_config_ssh, mock
             bootstrap.join_ssh("node1", "alice")
 
         mock_start_service.assert_called_once_with("sshd.service", enable=True)
+        mock_local_shell.assert_has_calls([
+            mock.call(additional_environ={'SSH_AUTH_SOCK': ''}),
+        ])
         mock_config_ssh.assert_has_calls([
             mock.call("bob"),
         ])
-        mock_ssh_copy_id.assert_called_once_with("bob", "alice", "node1")
+        mock_ssh_copy_id.assert_called_once_with("bob", "alice", "node1", mock_local_shell.return_value)
         mock_swap.assert_not_called()
         mock_invoke.assert_not_called()
 
@@ -1001,10 +1020,11 @@ def test_init_qdevice_no_config(self, mock_status, mock_disable):
     @mock.patch('crmsh.utils.is_qdevice_configured')
     @mock.patch('crmsh.bootstrap.configure_ssh_key')
     @mock.patch('crmsh.utils.check_ssh_passwd_need')
+    @mock.patch('crmsh.sh.LocalShell')
     @mock.patch('logging.Logger.info')
     def test_init_qdevice_already_configured(
             self,
-            mock_status, mock_ssh, mock_configure_ssh_key,
+            mock_status, mock_local_shell, mock_ssh, mock_configure_ssh_key,
             mock_qdevice_configured, mock_confirm, mock_list_nodes, mock_user_of_host,
             mock_host_user_config_class,
             mock_select_user_pair_for_ssh,
@@ -1022,7 +1042,11 @@ def test_init_qdevice_already_configured(
         bootstrap.init_qdevice()
 
         mock_status.assert_called_once_with("Configure Qdevice/Qnetd:")
-        mock_ssh.assert_called_once_with("bob", "bob", "qnetd-node")
+        mock_local_shell.assert_has_calls([
+            mock.call(additional_environ={'SSH_AUTH_SOCK': ''}),
+            mock.call(),
+        ])
+        mock_ssh.assert_called_once_with("bob", "bob", "qnetd-node", mock_local_shell.return_value)
         mock_configure_ssh_key.assert_not_called()
         mock_host_user_config_class.return_value.save_remote.assert_called_once_with(mock_list_nodes.return_value)
         mock_qdevice_configured.assert_called_once_with()
@@ -1039,8 +1063,9 @@ def test_init_qdevice_already_configured(
     @mock.patch('crmsh.utils.is_qdevice_configured')
     @mock.patch('crmsh.bootstrap.configure_ssh_key')
     @mock.patch('crmsh.utils.check_ssh_passwd_need')
+    @mock.patch('crmsh.sh.LocalShell')
     @mock.patch('logging.Logger.info')
-    def test_init_qdevice(self, mock_info, mock_ssh, mock_configure_ssh_key, mock_qdevice_configured,
+    def test_init_qdevice(self, mock_info, mock_local_shell, mock_ssh, mock_configure_ssh_key, mock_qdevice_configured,
                           mock_this_node, mock_list_nodes, mock_adjust_priority, mock_adjust_fence_delay,
                           mock_user_of_host, mock_host_user_config_class, mock_select_user_pair_for_ssh):
         bootstrap._context = mock.Mock(qdevice_inst=self.qdevice_with_ip, current_user="bob")
@@ -1058,7 +1083,11 @@ def test_init_qdevice(self, mock_info, mock_ssh, mock_configure_ssh_key, mock_qd
         bootstrap.init_qdevice()
 
         mock_info.assert_called_once_with("Configure Qdevice/Qnetd:")
-        mock_ssh.assert_called_once_with("bob", "bob", "qnetd-node")
+        mock_local_shell.assert_has_calls([
+            mock.call(additional_environ={'SSH_AUTH_SOCK': ''}),
+            mock.call(),
+        ])
+        mock_ssh.assert_called_once_with("bob", "bob", "qnetd-node", mock_local_shell.return_value)
         mock_host_user_config_class.return_value.add.assert_has_calls([
             mock.call('bob', '192.0.2.100'),
             mock.call('bob', 'qnetd-node'),

test/unittests/test_utils.py

@@ -79,7 +79,7 @@ def test_check_ssh_passwd_need(mock_run):
     assert res is True
     mock_run.assert_called_once_with(
         "bob",
-        " ssh -o StrictHostKeyChecking=no -o EscapeChar=none -o ConnectTimeout=15 -T -o Batchmode=yes alice@node1 true",
+        "ssh -o StrictHostKeyChecking=no -o EscapeChar=none -o ConnectTimeout=15 -T -o Batchmode=yes alice@node1 true",
     )