diff --git a/changelogs/fragments/winrm-kinit-pexpect.yml b/changelogs/fragments/winrm-kinit-pexpect.yml new file mode 100644 index 00000000000..004987f6751 --- /dev/null +++ b/changelogs/fragments/winrm-kinit-pexpect.yml @@ -0,0 +1,5 @@ +minor_changes: + - >- + winrm - Remove need for pexpect on macOS hosts when using ``kinit`` to retrieve the Kerberos TGT. + By default the code will now only use the builtin ``subprocess`` library which should handle issues + with select and a high fd count and also simplify the code. diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index 354acce7fad..86014690540 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -117,10 +117,6 @@ DOCUMENTATION = """ - kerberos usage mode. - The managed option means Ansible will obtain kerberos ticket. - While the manual one means a ticket must already have been obtained by the user. - - If having issues with Ansible freezing when trying to obtain the - Kerberos ticket, you can either set this to V(manual) and obtain - it outside Ansible or install C(pexpect) through pip and try - again. choices: [managed, manual] vars: - name: ansible_winrm_kinit_mode @@ -223,19 +219,6 @@ except ImportError as e: HAS_XMLTODICT = False XMLTODICT_IMPORT_ERR = e -HAS_PEXPECT = False -try: - import pexpect - # echo was added in pexpect 3.3+ which is newer than the RHEL package - # we can only use pexpect for kerb auth if echo is a valid kwarg - # https://github.com/ansible/ansible/issues/43462 - if hasattr(pexpect, 'spawn'): - argspec = getfullargspec(pexpect.spawn.__init__) - if 'echo' in argspec.args: - HAS_PEXPECT = True -except ImportError as e: - pass - # used to try and parse the hostname and detect if IPv6 is being used try: import ipaddress @@ -350,6 +333,7 @@ class Connection(ConnectionBase): def _kerb_auth(self, principal: str, password: str) -> None: if password is None: password = "" + b_password = to_bytes(password, encoding='utf-8', errors='surrogate_or_strict') self._kerb_ccache = tempfile.NamedTemporaryFile() display.vvvvv("creating Kerberos CC at %s" % self._kerb_ccache.name) @@ -376,60 +360,28 @@ class Connection(ConnectionBase): kinit_cmdline.append(principal) - # pexpect runs the process in its own pty so it can correctly send - # the password as input even on MacOS which blocks subprocess from - # doing so. Unfortunately it is not available on the built in Python - # so we can only use it if someone has installed it - if HAS_PEXPECT: - proc_mechanism = "pexpect" - command = kinit_cmdline.pop(0) - password = to_text(password, encoding='utf-8', - errors='surrogate_or_strict') - - display.vvvv("calling kinit with pexpect for principal %s" - % principal) - try: - child = pexpect.spawn(command, kinit_cmdline, timeout=60, - env=krb5env, echo=False) - except pexpect.ExceptionPexpect as err: - err_msg = "Kerberos auth failure when calling kinit cmd " \ - "'%s': %s" % (command, to_native(err)) - raise AnsibleConnectionFailure(err_msg) - - try: - child.expect(".*:") - child.sendline(password) - except OSError as err: - # child exited before the pass was sent, Ansible will raise - # error based on the rc below, just display the error here - display.vvvv("kinit with pexpect raised OSError: %s" - % to_native(err)) - - # technically this is the stdout + stderr but to match the - # subprocess error checking behaviour, we will call it stderr - stderr = child.read() - child.wait() - rc = child.exitstatus - else: - proc_mechanism = "subprocess" - b_password = to_bytes(password, encoding='utf-8', - errors='surrogate_or_strict') + display.vvvv(f"calling kinit for principal {principal}") - display.vvvv("calling kinit with subprocess for principal %s" - % principal) - try: - p = subprocess.Popen(kinit_cmdline, stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - env=krb5env) + # It is important to use start_new_session which spawns the process + # with setsid() to avoid it inheriting the current tty. On macOS it + # will force it to read from stdin rather than the tty. + try: + p = subprocess.Popen( + kinit_cmdline, + start_new_session=True, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=krb5env, + ) - except OSError as err: - err_msg = "Kerberos auth failure when calling kinit cmd " \ - "'%s': %s" % (self._kinit_cmd, to_native(err)) - raise AnsibleConnectionFailure(err_msg) + except OSError as err: + err_msg = "Kerberos auth failure when calling kinit cmd " \ + "'%s': %s" % (self._kinit_cmd, to_native(err)) + raise AnsibleConnectionFailure(err_msg) - stdout, stderr = p.communicate(b_password + b'\n') - rc = p.returncode != 0 + stdout, stderr = p.communicate(b_password + b'\n') + rc = p.returncode if rc != 0: # one last attempt at making sure the password does not exist @@ -437,8 +389,7 @@ class Connection(ConnectionBase): exp_msg = to_native(stderr.strip()) exp_msg = exp_msg.replace(to_native(password), "") - err_msg = "Kerberos auth failure for principal %s with %s: %s" \ - % (principal, proc_mechanism, exp_msg) + err_msg = f"Kerberos auth failure for principal {principal}: {exp_msg}" raise AnsibleConnectionFailure(err_msg) display.vvvvv("kinit succeeded for principal %s" % principal) diff --git a/test/units/plugins/connection/test_winrm.py b/test/units/plugins/connection/test_winrm.py index d5b76ca8f26..d11d60469db 100644 --- a/test/units/plugins/connection/test_winrm.py +++ b/test/units/plugins/connection/test_winrm.py @@ -242,7 +242,6 @@ class TestWinRMKerbAuth(object): mock_popen.return_value.returncode = 0 monkeypatch.setattr("subprocess.Popen", mock_popen) - winrm.HAS_PEXPECT = False pc = PlayContext() new_stdin = StringIO() conn = connection_loader.get('winrm', pc, new_stdin) @@ -258,46 +257,6 @@ class TestWinRMKerbAuth(object): assert actual_env['KRB5CCNAME'].startswith("FILE:/") assert actual_env['PATH'] == os.environ['PATH'] - @pytest.mark.parametrize('options, expected', [ - [{"_extras": {}}, - ("kinit", ["user@domain"],)], - [{"_extras": {}, 'ansible_winrm_kinit_cmd': 'kinit2'}, - ("kinit2", ["user@domain"],)], - [{"_extras": {'ansible_winrm_kerberos_delegation': True}}, - ("kinit", ["-f", "user@domain"],)], - [{"_extras": {}, 'ansible_winrm_kinit_args': '-f -p'}, - ("kinit", ["-f", "-p", "user@domain"],)], - [{"_extras": {}, 'ansible_winrm_kerberos_delegation': True, 'ansible_winrm_kinit_args': '-p'}, - ("kinit", ["-p", "user@domain"],)] - ]) - def test_kinit_success_pexpect(self, monkeypatch, options, expected): - pytest.importorskip("pexpect") - mock_pexpect = MagicMock() - mock_pexpect.return_value.exitstatus = 0 - monkeypatch.setattr("pexpect.spawn", mock_pexpect) - - winrm.HAS_PEXPECT = True - pc = PlayContext() - new_stdin = StringIO() - conn = connection_loader.get('winrm', pc, new_stdin) - conn.set_options(var_options=options) - conn._build_winrm_kwargs() - - conn._kerb_auth("user@domain", "pass") - mock_calls = mock_pexpect.mock_calls - assert mock_calls[0][1] == expected - actual_env = mock_calls[0][2]['env'] - assert sorted(list(actual_env.keys())) == ['KRB5CCNAME', 'PATH'] - assert actual_env['KRB5CCNAME'].startswith("FILE:/") - assert actual_env['PATH'] == os.environ['PATH'] - assert mock_calls[0][2]['echo'] is False - assert mock_calls[1][0] == "().expect" - assert mock_calls[1][1] == (".*:",) - assert mock_calls[2][0] == "().sendline" - assert mock_calls[2][1] == ("pass",) - assert mock_calls[3][0] == "().read" - assert mock_calls[4][0] == "().wait" - def test_kinit_with_missing_executable_subprocess(self, monkeypatch): expected_err = "[Errno 2] No such file or directory: " \ "'/fake/kinit': '/fake/kinit'" @@ -305,30 +264,6 @@ class TestWinRMKerbAuth(object): monkeypatch.setattr("subprocess.Popen", mock_popen) - winrm.HAS_PEXPECT = False - pc = PlayContext() - new_stdin = StringIO() - conn = connection_loader.get('winrm', pc, new_stdin) - options = {"_extras": {}, "ansible_winrm_kinit_cmd": "/fake/kinit"} - conn.set_options(var_options=options) - conn._build_winrm_kwargs() - - with pytest.raises(AnsibleConnectionFailure) as err: - conn._kerb_auth("user@domain", "pass") - assert str(err.value) == "Kerberos auth failure when calling " \ - "kinit cmd '/fake/kinit': %s" % expected_err - - def test_kinit_with_missing_executable_pexpect(self, monkeypatch): - pexpect = pytest.importorskip("pexpect") - - expected_err = "The command was not found or was not " \ - "executable: /fake/kinit" - mock_pexpect = \ - MagicMock(side_effect=pexpect.ExceptionPexpect(expected_err)) - - monkeypatch.setattr("pexpect.spawn", mock_pexpect) - - winrm.HAS_PEXPECT = True pc = PlayContext() new_stdin = StringIO() conn = connection_loader.get('winrm', pc, new_stdin) @@ -353,32 +288,6 @@ class TestWinRMKerbAuth(object): mock_popen.return_value.returncode = 1 monkeypatch.setattr("subprocess.Popen", mock_popen) - winrm.HAS_PEXPECT = False - pc = PlayContext() - new_stdin = StringIO() - conn = connection_loader.get('winrm', pc, new_stdin) - conn.set_options(var_options={"_extras": {}}) - conn._build_winrm_kwargs() - - with pytest.raises(AnsibleConnectionFailure) as err: - conn._kerb_auth("invaliduser", "pass") - - assert str(err.value) == \ - "Kerberos auth failure for principal invaliduser with " \ - "subprocess: %s" % (expected_err) - - def test_kinit_error_pexpect(self, monkeypatch): - pytest.importorskip("pexpect") - - expected_err = "Configuration file does not specify default realm" - mock_pexpect = MagicMock() - mock_pexpect.return_value.expect = MagicMock(side_effect=OSError) - mock_pexpect.return_value.read.return_value = to_bytes(expected_err) - mock_pexpect.return_value.exitstatus = 1 - - monkeypatch.setattr("pexpect.spawn", mock_pexpect) - - winrm.HAS_PEXPECT = True pc = PlayContext() new_stdin = StringIO() conn = connection_loader.get('winrm', pc, new_stdin) @@ -389,8 +298,7 @@ class TestWinRMKerbAuth(object): conn._kerb_auth("invaliduser", "pass") assert str(err.value) == \ - "Kerberos auth failure for principal invaliduser with " \ - "pexpect: %s" % (expected_err) + "Kerberos auth failure for principal invaliduser: %s" % (expected_err) def test_kinit_error_pass_in_output_subprocess(self, monkeypatch): def mock_communicate(input=None, timeout=None): @@ -401,32 +309,6 @@ class TestWinRMKerbAuth(object): mock_popen.return_value.returncode = 1 monkeypatch.setattr("subprocess.Popen", mock_popen) - winrm.HAS_PEXPECT = False - pc = PlayContext() - new_stdin = StringIO() - conn = connection_loader.get('winrm', pc, new_stdin) - conn.set_options(var_options={"_extras": {}}) - conn._build_winrm_kwargs() - - with pytest.raises(AnsibleConnectionFailure) as err: - conn._kerb_auth("username", "password") - assert str(err.value) == \ - "Kerberos auth failure for principal username with subprocess: " \ - "Error with kinit\n" - - def test_kinit_error_pass_in_output_pexpect(self, monkeypatch): - pytest.importorskip("pexpect") - - mock_pexpect = MagicMock() - mock_pexpect.return_value.expect = MagicMock() - mock_pexpect.return_value.read.return_value = \ - b"Error with kinit\npassword\n" - mock_pexpect.return_value.exitstatus = 1 - - monkeypatch.setattr("pexpect.spawn", mock_pexpect) - - winrm.HAS_PEXPECT = True - pc = PlayContext() pc = PlayContext() new_stdin = StringIO() conn = connection_loader.get('winrm', pc, new_stdin) @@ -436,7 +318,7 @@ class TestWinRMKerbAuth(object): with pytest.raises(AnsibleConnectionFailure) as err: conn._kerb_auth("username", "password") assert str(err.value) == \ - "Kerberos auth failure for principal username with pexpect: " \ + "Kerberos auth failure for principal username: " \ "Error with kinit\n" def test_exec_command_with_timeout(self, monkeypatch):