ScrapliAuthenticationFailed not raised during _authenticate_password

[drod0991]

Hi, Issues found while testing wrong creds, specifically when trying to auth using wrong password. Setup: nornir==3.0.0 nornir-scrapli==2020.9.16.post1 scrapli==2020.8.28 scrapli-ssh2==2020.6.6 connection_options: scrapli: extras: {auth_strict_key: false, ssh_config_file: false, transport: ssh2} port: 22

Aren't we missing to raise an exception if password authentication fails? Like below: (scrapli_ssh2/transport/cssh2.py)

        self._authenticate_password()
        if self._isauthenticated():
            LOG.debug(f"Authenticated to host {self.host} with password")
            return
        **if not self._authenticate_password():
            msg =(
                f"Failed to authenticate to host {self.host} during _authenticate_password."
                "Unable to continue authentication."
            )
            LOG.critical(msg)
            raise ScrapliAuthenticationFailed(msg)**
        self._authenticate_keyboard_interactive()

Thanks!

[carlmontanari]

Hey @drod0991

I'm not sure what issue you are seeing (or if there is an exception or something?), but I dont think there is a problem here. ScrapliAuthenticationFailed is raised if auth does not succeed. We check that here.

There is an explicit exception raised after key auth fails (and if there is no password) just to be very clear about key auth failing and there not being any other available mechanisms to try. After that we just raise an exception if auth fails w/ the generic "auth tho host blah failed".

Im going to close this as I dont think there is an issue here. If you feel there is still something wrong please open an issue in the ssh2 repo not here in the nornir_scrapli one and provide an exception or what you think should be happening. For sure there are things that can get improved around this but keep in mind that if we do anything to ssh2 transport we need to be at least close to parity on the others as well.

Carl

[carlmontanari]

Hey again @drod0991

I'm unclear what happened here, but I got a comment notification in email about this thread but I dont see any comment here! Weird! In any case, for posterity, here is the comment from the email:

---

Sorry I was probably not clear enough, the problem is that when password-based auth fails, there is no exception raised, so I am not able to react to the issue. Following the logic from the _authenticate_password (line 280 of scrapli_ssh2/transport/cssh2.py) function if auth fails it simply logs an error but the exception is been already captured and won't bubble:

try:
  self.session.userauth_password(self.auth_username, self.auth_password)
except AuthenticationError as exc:
  LOG.critical(f"Password authentication with host {self.host} failed. Exception: {exc}.")
except Exception as exc:
  LOG.critical(
      "Unknown error occurred during password authentication with host "
      f"{self.host}; Exception: {exc}"
   )
  raise exc

So only when an unknown error is raised, the exception is allowed to bubble up.

Then in the _authenticate (line 216 of scrapli_ssh2/transport/cssh2.py) function it simply attempts to continue to the next available auth option (_authenticate_keyboard_interactive) since is not authenticated:

        if self.auth_private_key:
            self._authenticate_public_key()
            if self._isauthenticated():
                LOG.debug(f"Authenticated to host {self.host} with public key auth")
                return
            if not self.auth_password or not self.auth_username:
                msg = (
                    f"Failed to authenticate to host {self.host} with private key "
                    f"`{self.auth_private_key}`. Unable to continue authentication, "
                    "missing username, password, or both."
                )
                LOG.critical(msg)
                raise ScrapliAuthenticationFailed(msg)

        self._authenticate_password()
        if self._isauthenticated():
            LOG.debug(f"Authenticated to host {self.host} with password")
            return
        self._authenticate_keyboard_interactive()
        if self._isauthenticated():
            LOG.debug(f"Authenticated to host {self.host} with keyboard interactive")

Are we checking for authentication upstream in the logic? Am I missing something?

Also could you please explain why auth_private_key has specific checks for authentication that raise exceptions while _authenticate_password and _authenticate_keyboard_interactive don't?

Thanks in advance, I really appreciate your help :)

---

Let me try to clear things up! Again, totally open to changes/improvements, but we just gotta make sure we do things consistently across the transports!

"the problem is that when password-based auth fails, there is no exception raised" -- this is sorta true but mostly not true :) If password auth fails we automagically try keyboard interactive auth. If both of those fail, then we raise ScrapliAuthenticationFailed. The general process (for all the transport plugins) is to try key based auth first, and if that fails and there is no password set we raise an exception. If there is a password set we carry on to try password/keyboard interactive.

If password or keyboard interactive fail we will eventually raise that ScrapliAuthenticationFailed message.

One thing I've tried to make happen (and have not compeltely succeded in) is to have any/all common exceptions that would occur be caught and raised as scrapli exceptions -- the thinking is that you can always (in theory at least... this is def not universally true) catch the base scrapli exception and not have to concern yourself w/ catching more explicit exceptions unless you want to, but even in that case you would be catching things like ScrapliAuthenticationFailed instead of the underlying libraries exception... the thinking here is that the transports should be totally abstracted so you dont actually care which one you are using.

"Are we checking for authentication upstream in the logic? Am I missing something?" -- mostly yes, see above. Also again, I'm treating password and keyboard interactive as one "group" of auth type (password based) -- I don't know anyone that really cares about the difference between the two, hence why I treat them like one "group".

"Also could you please explain why auth_private_key has specific checks for authentication that raise exceptions while _authenticate_password and _authenticate_keyboard_interactive don't?" -- basically see above -- these two auth methods are attempted, then if we are still not authenticated we raise the ScrapliAuthenticationFailed message. Perhaps either the password "group" should have its own exception or the key based auth shoudl just not have an exception, instead waiting to raise the same exception the password group rasies for consistency.

Hopefully that all makes sense, but holler if it doesnt!

Also, still not sure wtf happend to the github comment so... I dunno... hope you see this message and there is nothing weird going on there!

Carl