From e33c17efffc809548c3001d783f2cdec0132090c Mon Sep 17 00:00:00 2001 From: nicholasyang Date: Fri, 20 Sep 2024 12:01:17 +0800 Subject: [PATCH] Fix: bootstrap: check is_nologin more robustly (bsc#1228251) --- crmsh/bootstrap.py | 23 +++++++++++++---------- test/unittests/test_bootstrap.py | 20 ++++++++------------ 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/crmsh/bootstrap.py b/crmsh/bootstrap.py index aaff7ffb1..ce1c4579f 100644 --- a/crmsh/bootstrap.py +++ b/crmsh/bootstrap.py @@ -803,7 +803,7 @@ def append(fromfile, tofile, remote=None): def append_unique(fromfile, tofile, user=None, remote=None, from_local=False): """ Append unique content from fromfile to tofile - + if from_local and remote: append local fromfile to remote tofile elif remote: @@ -938,15 +938,18 @@ def is_nologin(user, remote=None): """ Check if user's shell is nologin """ - passwd_file = "/etc/passwd" - pattern = f"{user}:.*:/.*/nologin" - if remote: - cmd = f"cat {passwd_file}|grep {pattern}" - rc, _, _ = utils.run_cmd_on_remote(cmd, remote) - return rc == 0 - else: - with open(passwd_file) as f: - return re.search(pattern, f.read()) is not None + rc, _, _ = utils.get_stdout_stderr_auto_ssh_no_input( + remote, + "set -e\n" + f"shell=$(getent passwd '{user}' | awk -F: '{{ print $NF }}')\n" + '[ -n "${shell}" ] && [ -f "${shell}" ] && [ -x "${shell}" ] || exit 1\n' + 'case $(basename "$shell") in\n' + ' nologin) exit 1 ;;\n' + ' false) exit 1 ;;\n' + 'esac\n' + '"${shell}" < /dev/null &>/dev/null\n' + ) + return 0 != rc def change_user_shell(user, remote=None): diff --git a/test/unittests/test_bootstrap.py b/test/unittests/test_bootstrap.py index daa3db3e0..2886ea7a4 100644 --- a/test/unittests/test_bootstrap.py +++ b/test/unittests/test_bootstrap.py @@ -442,15 +442,18 @@ def test_start_pacemaker(self, mock_installed, mock_enabled, mock_delay_start, m mock.call(node_list, "systemctl daemon-reload"), ]) + @mock.patch('crmsh.bootstrap.change_user_shell') @mock.patch('crmsh.bootstrap.configure_ssh_key') @mock.patch('crmsh.utils.start_service') - def test_init_ssh(self, mock_start_service, mock_config_ssh): - bootstrap._context = mock.Mock(current_user="alice", user_at_node_list=[]) + def test_init_ssh(self, mock_start_service, mock_config_ssh, mock_change_user_shell): + bootstrap._context = mock.Mock(current_user="alice", user_at_node_list=[], use_ssh_agent=False) bootstrap.init_ssh() mock_start_service.assert_called_once_with("sshd.service", enable=True) mock_config_ssh.assert_has_calls([ - mock.call("alice") - ]) + mock.call("alice"), + mock.call("hacluster"), + ]) + mock_change_user_shell.assert_called_once_with("hacluster") @mock.patch('crmsh.userdir.gethomedir') def test_key_files(self, mock_gethome): @@ -459,13 +462,6 @@ def test_key_files(self, mock_gethome): self.assertEqual(bootstrap.key_files("root"), expected_res) mock_gethome.assert_called_once_with("root") - @mock.patch('builtins.open') - def test_is_nologin(self, mock_open_file): - data = "hacluster:x:90:90:heartbeat processes:/var/lib/heartbeat/cores/hacluster:/sbin/nologin" - mock_open_file.return_value = mock.mock_open(read_data=data).return_value - assert bootstrap.is_nologin("hacluster") is not None - mock_open_file.assert_called_once_with("/etc/passwd") - @mock.patch('crmsh.bootstrap.confirm') @mock.patch('logging.Logger.info') @mock.patch('crmsh.bootstrap.is_nologin') @@ -1407,7 +1403,7 @@ def test_valid_ucast_ip(self, mock_local_addr): bootstrap._context = mock.Mock(local_ip_list=["10.10.10.2", "10.10.10.3"]) bootstrap.Validation.valid_ucast_ip("10.10.10.1") mock_local_addr.assert_called_once_with(["10.10.10.2", "10.10.10.3"]) - + @mock.patch('crmsh.bootstrap.Validation._is_local_addr') def test_valid_mcast_ip(self, mock_local_addr): bootstrap._context = mock.Mock(local_ip_list=["10.10.10.2", "10.10.10.3"],