Catch sshpass authentication errors and don't retry multiple times to prevent account lockout (#50776)

* Catch SSH authentication errors and don't retry multiple times to prevent account lock out

Signed-off-by: Sam Doran <sdoran@redhat.com>

* Subclass AnsibleAuthenticationFailure from AnsibleConnectionFailure

Use comparison rather than range() because it's much more efficient.

Signed-off-by: Sam Doran <sdoran@redhat.com>

* Add tests

Signed-off-by: Sam Doran <sdoran@redhat.com>

* Make paramiko_ssh connection plugin behave the same way

Signed-off-by: Sam Doran <sdoran@redhat.com>

* Add changelog

Signed-off-by: Sam Doran <sdoran@redhat.com>
This commit is contained in:
Sam Doran 2019-01-23 11:32:25 -05:00 committed by ansibot
parent 2798d5bafc
commit 9d4c0dc111
5 changed files with 114 additions and 22 deletions

View file

@ -151,8 +151,8 @@ DOCUMENTATION = '''
- section: ssh_connection
key: retries
vars:
- name: ansible_ssh_retries
version_added: '2.7'
- name: ansible_ssh_retries
version_added: '2.7'
port:
description: Remote port to connect to.
type: int
@ -280,7 +280,12 @@ import time
from functools import wraps
from ansible import constants as C
from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleFileNotFound
from ansible.errors import (
AnsibleAuthenticationFailure,
AnsibleConnectionFailure,
AnsibleError,
AnsibleFileNotFound,
)
from ansible.errors import AnsibleOptionsError
from ansible.compat import selectors
from ansible.module_utils.six import PY3, text_type, binary_type
@ -307,6 +312,55 @@ class AnsibleControlPersistBrokenPipeError(AnsibleError):
pass
def _handle_error(remaining_retries, command, return_tuple, no_log, host, display=display):
# sshpass errors
if command == b'sshpass':
# Error 5 is invalid/incorrect password. Raise an exception to prevent retries from locking the account.
if return_tuple[0] == 5:
msg = 'Invalid/incorrect username/password. Skipping remaining {0} retries to prevent account lockout:'.format(remaining_retries)
if remaining_retries <= 0:
msg = 'Invalid/incorrect password:'
if no_log:
msg = '{0} <error censored due to no log>'.format(msg)
else:
msg = '{0} {1}'.format(msg, to_native(return_tuple[2].rstrip()))
raise AnsibleAuthenticationFailure(msg)
# sshpass returns codes are 1-6. We handle 5 previously, so this catches other scenarios.
# No exception is raised, so the connection is retried.
elif return_tuple[0] in [1, 2, 3, 4, 6]:
msg = 'sshpass error:'
if no_log:
msg = '{0} <error censored due to no log>'.format(msg)
else:
msg = '{0} {1}'.format(msg, to_native(return_tuple[2].rstrip()))
if return_tuple[0] == 255:
SSH_ERROR = True
for signature in b_NOT_SSH_ERRORS:
if signature in return_tuple[1]:
SSH_ERROR = False
break
if SSH_ERROR:
msg = "Failed to connect to the host via ssh:"
if no_log:
msg = '{0} <error censored due to no log>'.format(msg)
else:
msg = '{0} {1}'.format(msg, to_native(return_tuple[2]).rstrip())
raise AnsibleConnectionFailure(msg)
# For other errors, no execption is raised so the connection is retried and we only log the messages
if 1 <= return_tuple[0] <= 254:
msg = "Failed to connect to the host via ssh:"
if no_log:
msg = '{0} <error censored due to no log>'.format(msg)
else:
msg = '{0} {1}'.format(msg, to_native(return_tuple[2]).rstrip())
display.vvv(msg, host=host)
def _ssh_retry(func):
"""
Decorator to retry ssh/scp/sftp in the case of a connection failure
@ -315,7 +369,8 @@ def _ssh_retry(func):
* an exception is caught
* ssh returns 255
Will not retry if
* remaining_tries is <2
* sshpass returns 5 (invalid password, to prevent account lockouts)
* remaining_tries is < 2
* retries limit reached
"""
@wraps(func)
@ -333,7 +388,7 @@ def _ssh_retry(func):
try:
return_tuple = func(self, *args, **kwargs)
if self._play_context.no_log:
display.vvv('rc=%s, stdout & stderr censored due to no log' % return_tuple[0], host=self.host)
display.vvv('rc=%s, stdout and stderr censored due to no log' % return_tuple[0], host=self.host)
else:
display.vvv(return_tuple, host=self.host)
# 0 = success
@ -349,24 +404,18 @@ def _ssh_retry(func):
display.vvv(u"RETRYING BECAUSE OF CONTROLPERSIST BROKEN PIPE")
return_tuple = func(self, *args, **kwargs)
if return_tuple[0] == 255:
SSH_ERROR = True
for signature in b_NOT_SSH_ERRORS:
if signature in return_tuple[1]:
SSH_ERROR = False
break
if SSH_ERROR:
msg = "Failed to connect to the host via ssh: "
if self._play_context.no_log:
msg += '<error censored due to no log>'
else:
msg += to_native(return_tuple[2])
raise AnsibleConnectionFailure(msg)
remaining_retries = remaining_tries - attempt - 1
_handle_error(remaining_retries, cmd[0], return_tuple, self._play_context.no_log, self.host)
break
# 5 = Invalid/incorrect password from sshpass
except AnsibleAuthenticationFailure as e:
# Raising this exception, which is subclassed from AnsibleConnectionFailure, prevents further retries
raise
except (AnsibleConnectionFailure, Exception) as e:
if attempt == remaining_tries - 1:
raise
else:
@ -375,9 +424,9 @@ def _ssh_retry(func):
pause = 30
if isinstance(e, AnsibleConnectionFailure):
msg = "ssh_retry: attempt: %d, ssh return code is 255. cmd (%s), pausing for %d seconds" % (attempt, cmd_summary, pause)
msg = "ssh_retry: attempt: %d, ssh return code is 255. cmd (%s), pausing for %d seconds" % (attempt + 1, cmd_summary, pause)
else:
msg = "ssh_retry: attempt: %d, caught exception(%s) from cmd (%s), pausing for %d seconds" % (attempt, e, cmd_summary, pause)
msg = "ssh_retry: attempt: %d, caught exception(%s) from cmd (%s), pausing for %d seconds" % (attempt + 1, e, cmd_summary, pause)
display.vv(msg, host=self.host)