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

ProxyCommand %h not replaced #520

Closed
FloLaco opened this issue Oct 27, 2022 · 9 comments · May be fixed by netenglabs/suzieq#812
Closed

ProxyCommand %h not replaced #520

FloLaco opened this issue Oct 27, 2022 · 9 comments · May be fixed by netenglabs/suzieq#812

Comments

@FloLaco
Copy link

FloLaco commented Oct 27, 2022

I'm using another library (suzieq) which use asyncssh library.
I'm providing ssh config file with ProxyCommand option for trying to multiplex ssh session with netcat.
It's working with openssh client, but it seems that asyncssh does not replace the token %h from the ProxyCommand option :

[WORKER 0]: 2022-10-27 13:03:59,303 - asyncssh - INFO - [conn=252] Connected to SSH server at 10.38.119.40, port 22
[WORKER 0]: 2022-10-27 13:03:59,303 - asyncssh - INFO - [conn=252]   Proxy command: nc -X 5 -x 127.0.0.1:2226 '' 22
[...]
[WORKER 0]: 2022-10-27 13:04:14,275 - asyncssh - INFO - [conn=252] Aborting connection
[...]
[WORKER 0]: 2022-10-27 13:04:14,275 - asyncssh - INFO - [conn=252] Connection closed

As you can see, we see '' instead of 10.38.119.40.

Here's my ssh config file :

host  jumpserver
   IdentityFile   /home/suzieq/parquet/ssh_cred_conf/id_rsa
   IdentitiesOnly   yes
   user   xxxxxx
   hostname   x.x.x.x
   Protocol  2
   Port  22
   StrictHostKeyChecking   no
   DynamicForward 127.0.0.1:2226

host * !jumpserver
   Protocol  2
   StrictHostKeyChecking  no
   ProxyCommand nc -X 5 -x 127.0.0.1:2226 %h %p
   KexAlgorithms  +diffie-hellman-group-exchange-sha1,diffie-hellman-group1-sha1

Just for information, I'm starting the ssh connection to jumpserver before running Python with this command : ssh jumpserver -F /home/suzieq/parquet/ssh_cred_conf/config -N &

@ronf
Copy link
Owner

ronf commented Oct 27, 2022

It looks like the '%h' substitution during config file parsing isn't getting the hostname, but in a quick test I was not able to reproduce that here.

Can you show the call you are making to AsyncSSH to make the connection? Are you passing in that IP of 10.38.119.40 directly as the host to connect to, or is there something more involved going on there?

@FloLaco
Copy link
Author

FloLaco commented Oct 27, 2022

suzieq is passing the IP directly :

                        self._conn = await asyncssh.connect(
                            self.address,
                            username=self.username,
                            port=self.port,
                            options=options)

@ronf
Copy link
Owner

ronf commented Oct 29, 2022

I set up a configuration here with a jump server connection using DynamicForward and a ProxyCommand matching what you have here, but I'm still not able to reproduce the problem. The substituted hostname properly shows up in the debug output, and the proper host is contacted by the jump server.

I'm not sure what to try next.

@ronf
Copy link
Owner

ronf commented Oct 29, 2022

I'm not sure if it's an option for you to use ProxyJump instead of ProxyCommand, letting AsyncSSH open the connection to the jump server instead of running it yourself in the background and using SOCKS to connect through it, but AsyncSSH does support that if you wanted to try it. It can be done either via ProxyJump in the config file or by passing the jump host info as the "tunnel" argument on an asyncssh.connect(), either referencing a previously opened SSHClientConnection or as a string containing a hostname and optional port of the jump server.

That said, I would like to get the bottom of why the substitutions aren't working for you, as they should. I just don't see anything obvious in the code which would cause this, and can't get it to happen here. If you're up for it, perhaps some debugging could be added into AsyncSSH on your system to try and narrow it down.

@FloLaco
Copy link
Author

FloLaco commented Nov 4, 2022

I cannot use Jump Server because I'm using this option to multiplex session, which is not supported by asyncssh. I'm trying to find workaround :)
It works well manually, but not with asyncssh with suzieq.

I can try to add more debug to asyncssh (or suzieq if needed).
Please let me know which debug I can add

EDIT :
Just for info, here's my ps aux output :

USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
suzieq       1  0.1  0.0   5480  3040 ?        Ss   14:41   0:00 /bin/bash ./parquet/entrypoint.sh parquet/inventory.yml
suzieq       8  0.2  0.0  11008  5632 ?        S    14:41   0:00 ssh jumpserver -F /home/suzieq/parquet/ssh_cred_conf/config -N
suzieq       9  6.1  1.5 505424 123676 ?       Sl   14:41   0:01 /usr/local/bin/python3 /usr/local/bin/sq-poller --no-coalescer -I parquet/inventory.yml -c parquet/suzieq.cfg.yml --ssh-config-file /home/suzieq/parquet/ssh_cred_conf/config
suzieq      16 21.3  1.9 1357700 162136 ?      Sl   14:41   0:05 python3 /usr/local/lib/python3.8/site-packages/suzieq/poller/worker/sq_worker.py --config parquet/suzieq.cfg.yml --outputs parquet --ssh-config-file /home/suzieq/parquet/ssh_cred_conf/config --worker-id 0
suzieq      24  0.0  0.0   3276  2004 ?        S    14:41   0:00 nc -X 5 -x 127.0.0.1:2226  22
suzieq      25  0.0  0.0   3276  1896 ?        S    14:41   0:00 nc -X 5 -x 127.0.0.1:2226  22
suzieq      37  0.6  0.0   5744  3544 pts/0    Ss   14:42   0:00 bash
suzieq      44  0.0  0.0   9384  3016 pts/0    R+   14:42   0:00 ps aux```

@FloLaco
Copy link
Author

FloLaco commented Nov 4, 2022

Found out that host argument is empty here :

self._orig_host = host

That's why substitution does not work

@ronf
Copy link
Owner

ronf commented Nov 5, 2022

The line you identified in the SSHClientConfig constructor is called from the load() class method of SSHConfig, which gets the value from the prepare() method in SSHClientConfigOptions, and that gets the value from the asyncssh.connect() call (among other calls). So, if a "host" argument is being passed into the call to connect(), it should end up being filled into the a config object when connect() creates the "new_options" object. Based on the code you provided above, the host argument to connect() should be self.address in suzieq.

Can you confirm self.address is being passed into connect(), and that it has a proper value? Assuming it is, can you see what this value ends up being in SSHClientConnectionOptions.prepare() and SSHClientConfig.load()? This path is working correctly for me, but somehow in your environment it seems like the host value is getting lost somewhere along the way.

@FloLaco
Copy link
Author

FloLaco commented Nov 7, 2022

I put some logger message on suzieqand asyncssh :

asyncssh :

    loop = asyncio.get_event_loop()

    logger.info("FLA connect BEGIN %s", host)
    new_options = cast(SSHClientConnectionOptions, await _run_in_executor(
        loop, SSHClientConnectionOptions, options, config=config,
        host=host, port=port, tunnel=tunnel, family=family,
        local_addr=local_addr, **kwargs))

suzieq :

                        self.logger.info(
                            f"FLAFLAFLAFLA {self.address}:{self.port} at "
                            f"{time.time()}")
                        self._conn = await asyncssh.connect(
                            self.address,
                            username=self.username,
                            port=self.port,
                            options=options)

Here's the output log :

[...]
[WORKER 0]: 2022-11-04 16:28:58,736 - suzieq.poller.worker.nodes.node - INFO - FLAFLAFLAFLA 10.34.87.5:22 at 1667579338.7368286
[WORKER 0]: 2022-11-04 16:28:58,736 - asyncssh - INFO - FLA connect BEGIN 10.34.87.5
[...]

So yes the self.adress is okay.

But the problem is in SSHConfig.load(). config returned is generated by config = cls(last_config, reload, *args), which is the call of SSHClientConfig.__init__() which itself call super().__init__(last_config, reload) --> SSHConfig.__init__(). As last_options is defined self._options use last_config.

So, I put a log in the connect() function :
logger.info('FLA connect options PROXY COMMAND: %s', options.config.get('ProxyCommand'))
which print me :
[WORKER 0]: 2022-11-07 11:10:03,369 - asyncssh - INFO - FLA connect options PROXY COMMAND: nc,-X,5,-x,127.0.0.1:2226,,22

I supposed this is because, when suzieq call the asyncssh.connect() it provide the option argument, which is constructed here : https://github.com/netenglabs/suzieq/blob/2267cbb8e8a8dd6045b9df819849959c39dc0953/suzieq/poller/worker/nodes/node.py#L594

When substitution is done, it's done before the connect(), so it has no context info about host and port.

@ronf
Copy link
Owner

ronf commented Nov 7, 2022

If you specify "config" explicitly as an argument when constructing an SSHClientConnectionOptions object, I believe that causes the config file parsing to happen at the time of that call. In this case, the "host" argument is not being specified until later, but by then the substitutions done when reading the config file have already happened.

If you want this to work, I think you are going to need to defer passing in the "config" option until the connect() call if you want the host substitution to work. Alternately, the host is know at the point where suzieq's init_ssh_options() is called, it may be sufficient to pass in the "host" argument within that.

The intent behind this behavior is to allow you to pay the cost of parsing the config file and any other operations related to constructing the SSHClientConnectionOptions only once, and any future options are built up incrementally from that. You'd only parse the config file again if the "config" argument is passed in again.

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

Successfully merging a pull request may close this issue.

2 participants