Postpone ssh config parsing file when connecting to device for doing …

[FloLaco]

Related Issue

Fixes ronf/asyncssh#520

Description

When using variables in ssh config file like %h or %p, asyncssh is doing token substitution with host address and port. But since suzieq is creating an asyncssh options before trying to connect to device, asyncssh is not aware about host and port info.

Type of change

• Bug fix (non-breaking change which fixes an issue)

New Behavior

The goal is to postpone ssh config file parsing at the asyncssh.connect() time, where host and port is known.
There is another way, explained in the comments section.

...

Contrast to Current Behavior

Actually, when building the options, substitution look like this : nc -X 5 -x 127.0.0.1:2226 '' 22
Instead of nc -X 5 -x 127.0.0.1:2226 X.X.X.X 22
...

Discussion: Benefits and Drawbacks

It's very usefull for supporting multiplexing ssh session with OpenSSH options without adding such feature to suzieq

...

Proposed Release Note Entry

• Fix asyncssh OpenSSH token substitution (%h, %p) by postponing parsing ssh config file.

...

Comments

There's two way to fix this issue, the other way is to provide host and port information to asyncssh when building ssh options. I tested both and it works, but I prefer the way to postpone config parsing in the asyncssh.connect() call instead of doing parsing before and pass the result to asyncssh.connect().

Second way like this :

        if self.ssh_config_file:
            options = asyncssh.SSHClientConnectionOptions(
                host=self.address,
                port=self.port,
                options=options,
                config=[self.ssh_config_file],
            )

Example of ssh config file :

host  jumpserver
   IdentityFile   /home/suzieq/parquet/ssh_cred_conf/id_rsa
   IdentitiesOnly   yes
   user   username
   hostname   Y.Y.Y.Y
   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

Double Check

• I have read the comments and followed the CONTRIBUTING.md.
• I have explained my PR according to the information in the comments or in a linked issue.
• My PR source branch is created from the develop branch.
• My PR targets the develop branch.
• All my commits have --signoff applied

[claudiolor]

Thank you for the contribution! I left a comment which should be addressed before merging:

[claudiolor]

This won't work if no ssh config file is provided as we would pass a list containing None to the connect() function, causing it to fail.
I suggest to directly provide the string and, if empty or None, an empty tuple (as according to the docs explicitly passing None, asyncssh won't get the default configuration in .ssh/config).

Suggested change

	config=[self.ssh_config_file])
	config=self.ssh_config_file or ())

[claudiolor]

As the comment before:

Suggested change

	config=[self.ssh_config_file])
	config=self.ssh_config_file or ())

[ryanmerolle]

Is this something still useful?

[ddutt]

Is this something still useful?

Not sure, no one's ever asked for this but this user, and since the method he was pursuing was not a scalable answer, and he didn't address the comments, we haven't merged it