Skip to content

[BUG FIX] Fix an issue with propagating invalid exception when using RSA key but authentication failure occurs#351

Closed
Kami wants to merge 3 commits intoparamiko:mainfrom
Kami:fix_invalid_exception_propagation_on_valid_rsa_key_and_authentication_failure
Closed

[BUG FIX] Fix an issue with propagating invalid exception when using RSA key but authentication failure occurs#351
Kami wants to merge 3 commits intoparamiko:mainfrom
Kami:fix_invalid_exception_propagation_on_valid_rsa_key_and_authentication_failure

Conversation

@Kami
Copy link
Copy Markdown

@Kami Kami commented Jul 2, 2014

Problem description

Currently if you use a valid RSA key, but authentication failure occurs, paramiko will propagate an invalid not a valid DSA private key file exception. The exception which should be propagated is Authentication failed.

The reason why this occurs is because paramiko tries to load / parse private key in the following order:

  1. RSA
  2. DSA

If parsing of the key as RSA succeeds, but authentication failure occurs, paramiko won't stop, but it will also try to parse the same key which is already to be determined to be an RSA key as DSA. Obviously, parsing RSA key as DSA won't work, so not a valid DSA private key file exception will be stored in saved_exception and propagated to the end user later in the code.

Note 1: I discovered this bug while debugging some Libcloud deployment issues. Sadly a work-around for us (and others) until this has been merged into master is to catch Invalid DSS key exception exception and treat it in the same way as authentication failure which is far from ideal.

Note 2: I wonder how more people didn't spot this issue. RSA keys are far more common than DSA keys and I would imagine that most people only use / try one key for authentication.

… a valid

RSA key, but authentication failure occurs.
@dtgay
Copy link
Copy Markdown

dtgay commented Jul 2, 2014

+1, thanks for writing this up :)

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.03%) when pulling 2552d75 on Kami:fix_invalid_exception_propagation_on_valid_rsa_key_and_authentication_failure into e811e71 on paramiko:master.

@Kami
Copy link
Copy Markdown
Author

Kami commented Jul 3, 2014

Added a test case for previous invalid behavior and for this fix in e8c7f85

If you revert my fix, the test will fail because an invalid SSHException: not a valid DSA private key file exception is thrown.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.03%) when pulling e8c7f85 on Kami:fix_invalid_exception_propagation_on_valid_rsa_key_and_authentication_failure into e811e71 on paramiko:master.

asfgit pushed a commit to apache/libcloud that referenced this pull request Jul 7, 2014
@bitprophet bitprophet closed this Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants