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

SSHException should define _str_ method #1440

Closed
fabianbuechler opened this issue May 7, 2019 · 3 comments
Closed

SSHException should define _str_ method #1440

fabianbuechler opened this issue May 7, 2019 · 3 comments

Comments

@fabianbuechler
Copy link

@fabianbuechler fabianbuechler commented May 7, 2019

Most paramiko.ssh_exception.SSHException subclasses call SSHException.__init__(self, message) with some nicely crafted message.
However, since they overwrite self.args = (...) just a few lines later, the Exception.args used in Exception.__str__ don't return the message but basically str(self.args).

class BadHostKeyException(SSHException):
    def __init__(self, hostname, got_key, expected_key):
        message = (
            "Host key for server {} does not match: got {}, expected {}"
        )  # noqa
        message = message.format(
            hostname, got_key.get_base64(), expected_key.get_base64()
        )
        SSHException.__init__(self, message)
        self.hostname = hostname
        self.key = got_key
        self.expected_key = expected_key
        # for unpickling
        self.args = (hostname, got_key, expected_key)

For example:

In [1]: from paramiko.ssh_exception import BadHostKeyException                                                                                                            

In [2]: from paramiko.ed25519key import Ed25519Key                                                                                                                        

# Assume this key exists
In [3]: key = Ed25519Key.from_private_key_file('ssh_host_ed25519_key')                                                                            

In [4]: exc = BadHostKeyException('somehost', key, key)                                                                                                                   

In [5]: exc.args                                                                                                                                                          
Out[5]: 
('somehost',
 <paramiko.ed25519key.Ed25519Key at 0x7f6dcc122b00>,
 <paramiko.ed25519key.Ed25519Key at 0x7f6dcc122b00>)

In [6]: str(exc)                                                                                                                                                          
Out[6]: "('somehost', <paramiko.ed25519key.Ed25519Key object at 0x7f6dcc122b00>, <paramiko.ed25519key.Ed25519Key object at 0x7f6dcc122b00>)"
@d3dave
Copy link

@d3dave d3dave commented Jun 15, 2019

Note that this change was introduced in #314, originally for the purpose of supporting pickle-ing. But since #394, pickle-ing hasn't worked since the various key classes hold cryptography CompiledFFI objects:

import pickle
from paramiko.rsakey import RSAKey 
key = RSAKey.from_private_key_file('rsa_key') 
pickle.dumps(key)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can't pickle CompiledFFI objects

So removing all the overwrites won't have any negative effects. (The pickle-ing issue can be solved by making the key classes pickle-able, but that's a different issue.)

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 21, 2019

See #1460!

@bitprophet bitprophet closed this Jun 21, 2019
bitprophet added a commit that referenced this issue Jun 21, 2019
@fabianbuechler
Copy link
Author

@fabianbuechler fabianbuechler commented Jul 15, 2019

Thank's a lot for the fix! I can now confirm it works just fine!

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

Successfully merging a pull request may close this issue.

3 participants