There are a number of issues stemming from what feels like a simply too-naïve implementation of key handling in SSHClient and PKey.

I am making a single ticket for this because most of the existing PRs poking at it are too limited in scope; this sort of change has a high chance for bugs and breaking backwards compatibility (intentionally or no) and I feel it needs a broadly considered update.

[EDIT: See this comment for an update on the intent/scope of this issue!]

The high level details:

• First off, SSHClient._auth uses a multi-exit strategy combined with storing a single exception to raise at the end of the process. This frequently means the raised exception at auth time is flat out incorrect as to the true cause of the inability to authenticate.
  • For example, early on it attempts to check over all explicitly given key_filename values...and since it can't reliably know offhand which key types they are, it tries all known key types. In order. This will always generate/store errors unless one happens to have only one key and it's of the arbitrarily-first-in-the-list key type.
  • Thus even if all keys given were valid keys of their intended type, and they also all happened to be invalid from the target server's perspective (i.e. the user doesn't actually have an accepted key) we raise to them a nice helpful "Hey your key is fucked up" error instead of "None of your keys were accepted by the server".
• Furthermore, PKey._read_private_key - used frequently within the above, indirectly - is similarly messy, and many, many types of key loading/reading problems end up raising an error about how Private key file is encrypted - on unencrypted files.
  • Why? Simply because the logic assumes if it gets to the end of the function, and the password kwarg is blank, that a password was required and not given.
  • While I haven't debugged this as deeply as I'd like yet, it comes up frequently when trying to deal with ECDSA keys, and/or when lower level key handling (or key related crap in the test suite) is presently botched.

A number of tickets have sprung up on this, I'm sure I haven't gotten all of them, but they include #358 and #224.

Strongly related are a bevvy of tickets concerned with exception handling & exception types/subclasses, such as Fabric #85 and our own #351 and #293.