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

Proxyjump passphrase error #413

Closed
wmegchel opened this issue Oct 21, 2021 · 4 comments
Closed

Proxyjump passphrase error #413

wmegchel opened this issue Oct 21, 2021 · 4 comments

Comments

@wmegchel
Copy link

Hi Ron,

I use asyncssh to connect through a bastion server to a server on a protected domain. I get the following error:

Traceback (most recent call last):
  File "async.py", line 49, in <module>
    conn = asyncio.get_event_loop().run_until_complete(Connect.create(host, user, passphrase))
  File "C:\Users\wmegc\miniconda3\lib\asyncio\base_events.py", line 616, in run_until_complete
    return future.result()
  File "async.py", line 32, in create
    self.conn = await asyncssh.connect(host=host, username=user , passphrase=passphrase) # , client_keys=['C:/Users/wmegc/.ssh/id_rsa_hpc'])
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\connection.py", line 6855, in connect
    return await _connect(options, loop, flags, conn_factory,
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\connection.py", line 274, in _connect
    new_tunnel = await _open_tunnel(tunnel)
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\connection.py", line 258, in _open_tunnel
    return await connect(host, port, username=username)
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\connection.py", line 6850, in connect
    options = SSHClientConnectionOptions(options, config=config, host=host,
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\connection.py", line 5596, in __init__
    super().__init__(options=options, last_config=last_config, **kwargs)
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\misc.py", line 268, in __init__
    self.prepare(**self.kwargs)
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\connection.py", line 6307, in prepare
    self.client_keys = load_keypairs(client_keys, passphrase,
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\public_key.py", line 3153, in load_keypairs
    key, certs = read_private_key_and_certs(key, passphrase)
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\public_key.py", line 2970, in read_private_key_and_certs
    key, cert = import_private_key_and_certs(read_file(filename), passphrase)
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\public_key.py", line 2860, in import_private_key_and_certs
    key, end = _decode_private(data, passphrase)
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\public_key.py", line 2520, in _decode_private
    key = _decode_pem_private(pem_name, headers, data, passphrase)
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\public_key.py", line 2434, in _decode_pem_private
    return _decode_openssh_private(data, passphrase)
  File "C:\Users\wmegc\miniconda3\lib\site-packages\asyncssh\public_key.py", line 2277, in _decode_openssh_private
    raise KeyImportError('Passphrase must be specified to import '
asyncssh.public_key.KeyImportError: Passphrase must be specified to import encrypted private keys

I am sure the password is set and correct. Instead, the error was introduced when I modified my ~/.ssh/config and changed this line:

# This works
Proxycommand ssh -xaqW%h:22 bastion_server

To this

# This does not work
ProxyJump bastion_server

From the command line, this works fine. With asyncssh I get the error above. When I modified public_key.py to print the passphrase it receives. In the first case (proxycommand), it prints:

>>>>> DEBUG: function _decode_openssh_private received password: secret_pass <<<<

using the ProxyJump it receives the password twice, and one of them is empty:

>>>>> DEBUG: function _decode_openssh_private received password: secret_pass <<<<
>>>>> DEBUG: function _decode_openssh_private received password: None <<<<

No big deal, but hope you can fix it for the next release.

Best,
Wout

@ronf
Copy link
Owner

ronf commented Oct 23, 2021

Thanks for the report! I think I see the problem. The options you pass in to the original connect() call are not being provided when AsyncSSH opens the tunnel connection. So, it would only pick up things set in your SSH config file, which by design doesn't let you specify a passphrase.

This is fixable, but it's a bit tricky given that tunnels can be used for both outbound and inbound connections, and the options passed for setting up a listener for inbound connections can not be passed directly when opening the outbound tunnel connection. I have a few thoughts around how I can work around that with a separate tunnel_options argument that could default to the original connection options in the common case where both connections are outbound but I'll need a little time to see how to most cleanly add that.

In the meantime, if you want to use AsyncSSH for both connections, you could open the connection to the bastion host explicitly and pass the SSHClientConnection which gets returned as a "tunnel" argument on a second connect() to the final destination host. That'll let you set whatever arguments you need in each of the two connect() calls. In this case, you'll want to remove the ProxyJump/ProxyCommand from the config file.

@ronf
Copy link
Owner

ronf commented Oct 23, 2021

I took a closer look at this tonight, and my original idea of sharing the existing options object won't work for a couple of reasons. First, the passphrase you're looking to share is actually not stored in the "options" object. It's only kept around long enough to decrypt keys and store those in the options, after which it is discarded for security reasons. So, it has to be passed separately for the tunnel to be able to share it. Second, there are some complications around the way config files are loaded via the options. Since the target host & port for the tunnel is different from the final server, it's important to read the config file separately for those two connections, so the right portions of the config are matched.

Given this, I've decided to just solve the passphrase issue for now, and still require that callers open the tunnel themselves if they want to use non-default options to make that connection (outside of the config file). For someone using ProxyJump, they're probably setting other options via the config file, and that case should work fine. If they need to pass in a non-default value not covered by the config, it's only one extra line of code to do open the tunnel explicitly.

Here's the patch I have so far, if you'd like to give it a try:

diff --git a/asyncssh/connection.py b/asyncssh/connection.py
index f14d594..30b954d 100644
--- a/asyncssh/connection.py
+++ b/asyncssh/connection.py
@@ -240,7 +240,7 @@ async def _open_proxy(loop, command, conn_factory):
     return tunnel.get_owner()


-async def _open_tunnel(tunnel):
+async def _open_tunnel(tunnel, passphrase):
     """Parse and open connection to tunnel over"""

     if isinstance(tunnel, str):
@@ -255,12 +255,13 @@ async def _open_tunnel(tunnel):
         else:
             port = ()

-        return await connect(host, port, username=username)
+        return await connect(host, port, username=username,
+                             passphrase=passphrase)
     else:
         return None


-async def _connect(options, loop, flags, conn_factory, msg):
+async def _connect(options, passphrase, loop, flags, conn_factory, msg):
     """Make outbound TCP or SSH tunneled connection"""

     host = options.host
@@ -271,7 +272,7 @@ async def _connect(options, loop, flags, conn_factory, msg):
     proxy_command = options.proxy_command
     free_conn = True

-    new_tunnel = await _open_tunnel(tunnel)
+    new_tunnel = await _open_tunnel(tunnel, passphrase)

     if new_tunnel:
         new_tunnel.logger.info('%s %s via %s', msg, (host, port), tunnel)
@@ -313,8 +314,8 @@ async def _connect(options, loop, flags, conn_factory, msg):
             await conn.wait_closed()


-async def _listen(options, loop, flags, backlog, reuse_address,
-                  reuse_port, conn_factory, msg):
+async def _listen(options, passphrase, loop, flags, backlog,
+                  reuse_address, reuse_port, conn_factory, msg):
     """Make inbound TCP or SSH tunneled listener"""

     def tunnel_factory(_orig_host, _orig_port):
@@ -327,7 +328,7 @@ async def _listen(options, loop, flags, backlog, reuse_address,
     tunnel = options.tunnel
     family = options.family

-    new_tunnel = await _open_tunnel(tunnel)
+    new_tunnel = await _open_tunnel(tunnel, passphrase)

     if new_tunnel:
         new_tunnel.logger.info('%s %s via %s', msg, (host, port), tunnel)
@@ -6766,7 +6767,8 @@ class SSHServerConnectionOptions(SSHConnectionOptions):

 @async_context_manager
 async def connect(host, port=(), *, tunnel=(), family=(), flags=0,
-                  local_addr=None, config=(), options=None, **kwargs):
+                  local_addr=None, config=(), options=None,
+                  passphrase=None, **kwargs):
     """Make an SSH client connection

        This function is a coroutine which can be run to create an outbound SSH
@@ -6852,13 +6854,14 @@ async def connect(host, port=(), *, tunnel=(), family=(), flags=0,
                                          family=family, local_addr=local_addr,
                                          **kwargs)

-    return await _connect(options, loop, flags, conn_factory,
+    return await _connect(options, passphrase, loop, flags, conn_factory,
                           'Opening SSH connection to')


 @async_context_manager
 async def connect_reverse(host, port=(), *, tunnel=(), family=(), flags=0,
-                          local_addr=None, config=(), options=None, **kwargs):
+                          local_addr=None, config=(), options=None,
+                          passphrase=None, **kwargs):
     """Create a reverse direction SSH connection

        This function is a coroutine which behaves similar to :func:`connect`,
@@ -6928,7 +6931,7 @@ async def connect_reverse(host, port=(), *, tunnel=(), family=(), flags=0,
                                          family=family, local_addr=local_addr,
                                          **kwargs)

-    return await _connect(options, loop, flags, conn_factory,
+    return await _connect(options, passphrase, loop, flags, conn_factory,
                           'Opening reverse SSH connection to')


@@ -6936,7 +6939,7 @@ async def connect_reverse(host, port=(), *, tunnel=(), family=(), flags=0,
 async def listen(host='', port=(), tunnel=(), family=(),
                  flags=socket.AI_PASSIVE, backlog=100, reuse_address=None,
                  reuse_port=None, acceptor=None, error_handler=None,
-                 config=(), options=None, **kwargs):
+                 config=(), options=None, passphrase=None, **kwargs):
     """Start an SSH server

        This function is a coroutine which can be run to create an SSH server
@@ -7026,8 +7029,9 @@ async def listen(host='', port=(), tunnel=(), family=(),
     # pylint: disable=attribute-defined-outside-init
     options.proxy_command = None

-    return await _listen(options, loop, flags, backlog, reuse_address,
-                         reuse_port, conn_factory, 'Creating SSH listener on')
+    return await _listen(options, passphrase, loop, flags, backlog,
+                         reuse_address, reuse_port, conn_factory,
+                         'Creating SSH listener on')


 @async_context_manager
@@ -7035,7 +7039,7 @@ async def listen_reverse(host='', port=(), *, tunnel=(), family=(),
                          flags=socket.AI_PASSIVE, backlog=100,
                          reuse_address=None, reuse_port=None,
                          acceptor=None, error_handler=None, config=(),
-                         options=None, **kwargs):
+                         options=None, passphrase=None, **kwargs):
     """Create a reverse-direction SSH listener

        This function is a coroutine which behaves similar to :func:`listen`,
@@ -7137,8 +7141,8 @@ async def listen_reverse(host='', port=(), *, tunnel=(), family=(),
     # pylint: disable=attribute-defined-outside-init
     options.proxy_command = None

-    return await _listen(options, loop, flags, backlog, reuse_address,
-                         reuse_port, conn_factory,
+    return await _listen(options, passphrase, loop, flags, backlog,
+                         reuse_address, reuse_port, conn_factory,
                          'Creating reverse direction SSH listener on')


@@ -7183,7 +7187,7 @@ async def get_server_host_key(host, port=(), *, tunnel=(), proxy_command=(),
                               family=(), flags=0, local_addr=None,
                               client_version=(), kex_algs=(),
                               server_host_key_algs=(), config=(),
-                              options=None):
+                              options=None, passphrase=None):
     """Retrieve an SSH server's host key

        This is a coroutine which can be run to connect to an SSH server and
@@ -7283,7 +7287,7 @@ async def get_server_host_key(host, port=(), *, tunnel=(), proxy_command=(),
         x509_purposes='any', gss_host=None, kex_algs=kex_algs,
         client_version=client_version)

-    conn = await _connect(options, loop, flags, conn_factory,
+    conn = await _connect(options, passphrase, loop, flags, conn_factory,
                           'Fetching server host key from')

     server_host_key = conn.get_server_host_key()

It looks big, but that's only because there are 5 different types of connections which can be made that potentially involve opening a tunneled connection. It's actually a pretty straightforward change. If you get a chance to try it out, let me know if you have any problems. I'll be looking to get this into the "develop" branch soon and from there into the next release.

@ronf
Copy link
Owner

ronf commented Oct 30, 2021

This is now available in the "develop" branch as commit 16becd8.

@ronf
Copy link
Owner

ronf commented Nov 9, 2021

There were some problems with this commit, so I ended up reworking it as commit 4976307, which is now available in release 2.8.1. If you have any further problems with this, feel free to re-open this issue or create a new one.

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

No branches or pull requests

2 participants