ronf / asyncssh

AsyncSSH is a Python package which provides an asynchronous client and server implementation of the SSHv2 protocol on top of the Python asyncio framework.
Eclipse Public License 2.0
1.55k stars 151 forks source link

Unable to control `known_hosts` when using ProxyJump? #698

Open bigpick opened 2 weeks ago

bigpick commented 2 weeks ago

Info

In asyncssh/connection.py's _open_tunnel, line 381-383 https://github.com/ronf/asyncssh/blob/46636d6f18b221141e659e2562c49b4a271befdb/asyncssh/connection.py#L375-L377 it doesn't seem to be able to have any user control on the known_host value?

When using host connections with known_hosts=None, trying to use a ProxyJump to access such a host where the top level connect call is ignoring keys still fails bc there's no way to force the ProxyJump tunnel connection(s) to also ignore key checking entirely (since the settings in the SSH conf are ignored)?

with a SSH conf like so:

Host jump
    User root
    Hostname IP1
    Port 22
    IdentityFile IF

Host machine
    User root
    Hostname IP2
    Port 22
    IdentityFile IF
    ProxyJump jump

and code like so

async def run_remote_cmd(
    host: str, cmd: str, ssh_conf_path: Optional[str] = None
):
    """Run a command on a remote host."""
    try:
        conn = await asyncssh.connect(host, config=ssh_conf_path, known_hosts=None)

    except Exception as exc:
        logger.error(f"Connection against '{host}' failed; skipping execution of cmds for host: {exc}")

    else:
        r = await remote_command(host, cmd, conn)

run_remote_cmd("machine", "ls")

Fails complaining that the jump machine's key isn't known - despite having known_hosts=None at the top level connection call.

Manually adding ... , known_hosts=None to the 377 line

 conn = await connect(host, port, username=username, 
                      passphrase=passphrase, tunnel=conn, 
                      config=config, known_hosts=None) 

works as expected.

Is there a way to control key checking for tunnel connections that I'm missing?

bigpick commented 2 weeks ago

Augmenting _open_tunnel to take a new arg and passing the options.known_hosts in the _connect() call probably seems what I'm asking for?

A quick test with that, and it also works as I'd expected (properly passing down a top-level's known_hosts=None as well as known_hosts=['somehost'])

IOW making

https://github.com/ronf/asyncssh/blob/46636d6f18b221141e659e2562c49b4a271befdb/asyncssh/connection.py#L401

into

 new_tunnel = await _open_tunnel(tunnel, options.passphrase, config, options.known_hosts) 

and then making the _open_tunnel take an additional Optional[list[str]]

ronf commented 2 weeks ago

You have a few different options here. If you want to go the config file route, you could add something under the "Host jump" section which sets "UserKnownHostsFile none" just for that host, though I generally don't recommend disabling known host checking like that, as it opens you up to man-in-the-middle attacks.

The other option which doesn't require any config file is to do something like:

    jump_conn = await asyncssh.connect(jump_ip, known_hosts=None)
    conn = await jump_conn.connect_ssh(machine_ip)

You can use either IPs or hostnames here, if you have a hostname which resolves to the IP you want.

You can add other arguments to set things like the client keys you want to use, and since the calls are separate, each can have its own unique parameters as needed.

Note that the second call is connect_ssh, not connect, as calling connect opens up a plain TCP connection. Using connect_ssh tells it to open a TCP tunnel but then run SSH on top of that tunnel.

One of both of the above calls can be used with "async with" instead of "await" if you want them to be treated as context managers, automatically closing the connections when the scope is exited. For instance:

   async with asyncssh.connect(jump_ip, known_hosts=None) as jump_conn:
        async with jump_conn.connect_ssh(machine_ip) as conn:
            ...do stuff on conn...

When you exit out of the block, both connections will be automatically closed.

bigpick commented 2 weeks ago

If you want to go the config file route, you could add something under the "Host jump" section which sets "UserKnownHostsFile none"

Hmm, no, I tried this before and it still did not work. IOW, something like

Host jump
    User root
    Hostname IP1
    Port 22
    IdentityFile IF
    UserKnownHostsFile none

Host machine
    User root
    Hostname IP2
    Port 22
    IdentityFile IF
    ProxyJump jump

as well as

Host jump
    User root
    Hostname IP1
    Port 22
    IdentityFile IF
    StrictHostKeyChecking no
    UserKnownHostsFile none

Host machine
    User root
    Hostname IP2
    Port 22
    IdentityFile IF
    ProxyJump jump

Both still fail with Host key is not trusted for host IP1. The only way I could get it to succeed as expected was using the setup I had above was to modify the _open_tunnel function to inherit the known_hosts

Also - any of the other methods I am not interested in, as it defeats the purpose of being able to use just the config (and a single top level call with ..., known_hosts=None); So either it'd need to pass the parent connections known_hosts or one-off respect the UserKnownHostsFile in this instance (It was my understanding that this such setting was ignored by asyncssh, e.g https://github.com/ronf/asyncssh/issues/685)

ronf commented 2 weeks ago

AsyncSSH honors UsersKnownHostsFile and GlobalKnownHostsFile, but looking at the code more closely it doesn't properly handle UserKnownHostsFile being set to 'none' in the config. It only handles these settings being string lists of alternate filenames to use. I'll let you know when I have a fix for that.

Once this is done, you'd be able to go without the top-level known_hosts=None argument as well, if your config sets UserKnownHostsFile to "none" for both (or all) hosts.

bigpick commented 2 weeks ago

Awesome, can/will pull and test updated candidate whenever you have it - thanks!

ronf commented 2 weeks ago

Try the following change:

diff --git a/asyncssh/connection.py b/asyncssh/connection.py
index cc4609c..9f6eefe 100644
--- a/asyncssh/connection.py
+++ b/asyncssh/connection.py
@@ -7911,9 +7911,16 @@ class SSHClientConnectionOptions(SSHConnectionOptions):
                         rekey_seconds, connect_timeout, login_timeout,
                         keepalive_interval, keepalive_count_max)

-        self.known_hosts = known_hosts if known_hosts != () else \
-            (cast(List[str], config.get('UserKnownHostsFile', [])) +
-             cast(List[str], config.get('GlobalKnownHostsFile', []))) or ()
+        if known_hosts != ():
+            self.known_hosts = known_hosts
+        else:
+            user_known_hosts = config.get('UserKnownHostsFile', ())
+
+            if user_known_hosts == []:
+                self.known_hosts = None
+            else:
+                self.known_hosts = cast(List[str], list(user_known_hosts)) + \
+                    cast(List[str], config.get('GlobalKnownHostsFile', []))

         self.host_key_alias = \
             cast(Optional[str], host_key_alias if host_key_alias != () else

Some additional cleanup will probably be needed from a code coverage standpoint, along with some tweaking of the types to make mypy happy, but this should be enough for you to confirm whether "UserKnownHostsFile none" in config is now being handled correctly.

ronf commented 1 week ago

Support for "UserKnownHostsFile none" is now available as commit 2fa354d in the "develop" branch.