diff options
| -rw-r--r-- | changelogs/fragments/winrm_kinit-remove-pass-log.yml | 2 | ||||
| -rw-r--r-- | lib/ansible/plugins/connection/winrm.py | 9 | ||||
| -rw-r--r-- | test/units/plugins/connection/test_winrm.py | 46 |
3 files changed, 55 insertions, 2 deletions
diff --git a/changelogs/fragments/winrm_kinit-remove-pass-log.yml b/changelogs/fragments/winrm_kinit-remove-pass-log.yml new file mode 100644 index 0000000000..250809afd8 --- /dev/null +++ b/changelogs/fragments/winrm_kinit-remove-pass-log.yml @@ -0,0 +1,2 @@ +bugfixes: +- winrm - ensure pexpect is set to not echo the input on a failure and have a manual sanity check afterwards https://github.com/ansible/ansible/issues/41865 diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index f358cc57f2..28aa5a62d7 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -294,7 +294,7 @@ class Connection(ConnectionBase): % principal) try: child = pexpect.spawn(command, kinit_cmdline, timeout=60, - env=krb5env) + env=krb5env, echo=False) except pexpect.ExceptionPexpect as err: err_msg = "Kerberos auth failure when calling kinit cmd " \ "'%s': %s" % (command, to_native(err)) @@ -336,8 +336,13 @@ class Connection(ConnectionBase): rc = p.returncode != 0 if rc != 0: + # one last attempt at making sure the password does not exist + # in the output + exp_msg = to_native(stderr.strip()) + exp_msg = exp_msg.replace(to_native(password), "<redacted>") + err_msg = "Kerberos auth failure for principal %s with %s: %s" \ - % (principal, proc_mechanism, to_native(stderr.strip())) + % (principal, proc_mechanism, 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 92f0ceadee..bfd27743b8 100644 --- a/test/units/plugins/connection/test_winrm.py +++ b/test/units/plugins/connection/test_winrm.py @@ -271,6 +271,7 @@ class TestWinRMKerbAuth(object): actual_env = mock_calls[0][2]['env'] assert list(actual_env.keys()) == ['KRB5CCNAME'] assert actual_env['KRB5CCNAME'].startswith("FILE:/") + 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" @@ -367,3 +368,48 @@ class TestWinRMKerbAuth(object): assert str(err.value) == \ "Kerberos auth failure for principal invaliduser with " \ "pexpect: %s" % (expected_err) + + def test_kinit_error_pass_in_output_subprocess(self, monkeypatch): + def mock_communicate(input=None, timeout=None): + return b"", b"Error with kinit\n" + input + + mock_popen = MagicMock() + mock_popen.return_value.communicate = mock_communicate + 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": {}}) + + 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<redacted>" + + 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) + conn.set_options(var_options={"_extras": {}}) + + with pytest.raises(AnsibleConnectionFailure) as err: + conn._kerb_auth("username", "password") + assert str(err.value) == \ + "Kerberos auth failure for principal username with pexpect: " \ + "Error with kinit\n<redacted>" |