Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite auth mechanism proposal #203

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

u1735067
Copy link

@u1735067 u1735067 commented Aug 22, 2019

  • Try auth_password first (it should be the most common for network equipments and some of them don't like auth_none as first method, requiring reconnection
  • Use correct display names
  • Remove redundant code (error handling)
  • Allow some partial auth support (might not work)
  • Banner is retrieved and tested at each auth attempt, because some equipments send it only once even is the method failed
  • Some Python3 compatibility "hack" (get_banner), there's a lot to change to be fully Python3 compatible I think (unicode_literals from __future__ everywhere, as str in Py2 is basically bytes)

* Try auth_password first (it should be the most common for network equipments and some of them don't like auth_none as first method, requiring reconnection
* Use correct display names
* Remove redundant code (error handling)
* Allow some partial auth support (might not work)
* Banner is retrieved and tested at each auth attempt, because some equipments send it only once even is the method failed
* Some Python3 compatibility "hack" (get_banner), there's a lot to change to be fully Python3 compatible I think (unicode_literals from __future__ everywhere, as str in Py2 is basically bytes)
if partial_methods == [] and hasattr(e, 'allowed_types'):
allowed_methods = e.allowed_types
finally:
self.os_guesser.data_received(self.get_banner(), False) # Log banner sent DURING auth

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (102 > 79 characters)

elif type(e) is BadHostKeyException:
self._add_auth_error('{}: Bad host key: {}'.format(method_name, e))
elif type(e) is SSHException:
self._add_auth_error('{}: SSHException: {}'.format(method_name, e))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (87 > 79 characters)

elif type(e) is AuthenticationException:
self._add_auth_error('Authentication with {} failed: {}'.format(method_name, e))
elif type(e) is BadHostKeyException:
self._add_auth_error('{}: Bad host key: {}'.format(method_name, e))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (87 > 79 characters)

if type(e) is BadAuthenticationType: # Should happen only once
self._add_auth_error('{} reported as bad method, supported: {}'.format(method['ssh_method'], e.allowed_types))
elif type(e) is AuthenticationException:
self._add_auth_error('Authentication with {} failed: {}'.format(method_name, e))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (100 > 79 characters)

self._paramiko_auth(username, password, next_methods)
except (BadAuthenticationType, AuthenticationException, BadHostKeyException, SSHException) as e:
if type(e) is BadAuthenticationType: # Should happen only once
self._add_auth_error('{} reported as bad method, supported: {}'.format(method['ssh_method'], e.allowed_types))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (130 > 79 characters)

# Some OSes (e.g. JunOS ERX OS, Huawei) do not accept further login
# attempts after failing one. So in this hack, we
# re-connect after each attempt...
if self.get_driver().reconnect_between_auth_methods or not self.client.active:
# But do not reconnect in case of partial auth
if self.get_driver().reconnect_between_auth_methods or (partial_methods == [] and not self.client.active):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (118 > 79 characters)

for method in auth_types:
method_name = method.get('display_name', method['ssh_method'])
if allowed_methods is not None and method_name not in allowed_methods:
self._dbg(1, 'Skipping authentification method %s (unsupported)' % method_name)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (95 > 79 characters)

allowed_methods = partial_methods
for method in auth_types:
method_name = method.get('display_name', method['ssh_method'])
if allowed_methods is not None and method_name not in allowed_methods:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (82 > 79 characters)

{'ssh_method': 'publickey', 'function': '_paramiko_auth_agent', 'display_name': 'publickey (agent)'},
{'ssh_method': 'publickey', 'function': '_paramiko_auth_autokey', 'display_name': 'publickey (autokey)'},
# https://superuser.com/questions/894608/ssh-o-preferredauthentications-whats-the-difference-between-password-and-k
{'ssh_method': 'keyboard-interactive', 'function': '_paramiko_auth_interactive'}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (84 > 79 characters)

{'ssh_method': 'password', 'function': '_paramiko_auth_password'},
{'ssh_method': 'none', 'function': '_paramiko_auth_none'},
{'ssh_method': 'publickey', 'function': '_paramiko_auth_agent', 'display_name': 'publickey (agent)'},
{'ssh_method': 'publickey', 'function': '_paramiko_auth_autokey', 'display_name': 'publickey (autokey)'},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (109 > 79 characters)

self._paramiko_auth(username, password, next_methods)
except (BadAuthenticationType, AuthenticationException, BadHostKeyException, SSHException) as e:
if type(e) is BadAuthenticationType: # Should happen only once
self._add_auth_error('{} reported as bad method, supported: {}'.format(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (91 > 79 characters)

self._dbg(1, 'Authentication successful')
return
elif partial_methods == []: # Only attempt partial methods once (1 recursion)
self.guess_os_from_banner() # Try to get banner before trying next method

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (94 > 79 characters)

self._dbg(1, 'Authenticating with %s' % method_name)
next_methods = getattr(self, method['function'])(username, password)
if next_methods == []:
self.guess_os_from_banner(authenticated=True) # Auth done, get banner !

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (92 > 79 characters)

# This one automatically falls back to keyb-interecative with internal handler
{'ssh_method': 'password', 'function': '_paramiko_auth_password'},
{'ssh_method': 'none', 'function': '_paramiko_auth_none'},
{'ssh_method': 'publickey', 'function': '_paramiko_auth_agent', 'display_name': 'publickey (agent)'},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (105 > 79 characters)

'keyboard-interactive': ('_paramiko_auth_interactive',),
'password': ('_paramiko_auth_password',)}
auth_types = [
# This one automatically falls back to keyb-interecative with internal handler

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line too long (82 > 79 characters)

@coveralls
Copy link

coveralls commented Aug 22, 2019

Coverage Status

Coverage decreased (-0.07%) to 76.718% when pulling ef2ae01 on Alex131089:rewrite-auth-mechanism into 5c7c625 on knipknap:master.

@int3rlop3r
Copy link
Contributor

As a workaround I landed up extending the SSH protocol class.

`
class SSH2(PSSH2):

  def _paramiko_auth(self, username, password):
      """Override Exscript's default behaviour since we only use
      one auth type (password).
      """
      method = self._paramiko_auth_password
      self._dbg(1, 'Authenticating with %s' % method.__name__)
      try:
          method(username, password)
          return
      except BadHostKeyException as e:
          msg = '%s: Bad host key: %s' % (method.__name__, str(e))
          self._dbg(1, msg)
      except AuthenticationException as e:
          msg = 'Authentication with %s failed' % method.__name__
          msg += ': ' + str(e)
          self._dbg(1, msg)
      except SSHException as e:
          msg = '%s: SSHException: %s' % (method.__name__, str(e))
          self._dbg(1, msg)
      raise LoginFailure('Login failed: ' + msg)

`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants