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

Adapt scp source and target arguments for old openssh #386

Merged

Conversation

davidcassany
Copy link
Contributor

@davidcassany davidcassany commented Nov 5, 2021

This commit does not make use of URIs in scp command, this way lima stays compatible with legacy openssh clients (older than v8.0)

Signed-off-by: David Cassany [email protected]

@davidcassany
Copy link
Contributor Author

I found that support for target or source URIs such as scp://<user>@<host>:<port>/<path> were introduced in openSSH v8.0. I noticed it trying to make use of lima under Ubuntu Bionic (v18.04). Note that Ubuntu bionic will be supported until 2023.

@AkihiroSuda AkihiroSuda added this to the v0.7.4 milestone Nov 5, 2021
@davidcassany davidcassany force-pushed the fix_scp_command_for_old_openssh branch from a71c3a9 to 8ca3ce3 Compare November 8, 2021 09:04
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not work when copying files from multiple instances:

$ limactl --debug cp alpine:/etc/os-release default:/etc/passwd _output/
[...]
DEBU[0000] executing scp (may take a long time)): [/usr/bin/scp -F /dev/null -o IdentityFile="/Users/jan/.lima/_config/user" -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o NoHostAuthenticationForLocalhost=yes -o GSSAPIAuthentication=no -o PreferredAuthentications=publickey -o Compression=no -o BatchMode=yes -o IdentitiesOnly=yes -o Ciphers="^[email protected],[email protected]" -v -3 -P 60022 -P 63697 -- [email protected]:/etc/os-release [email protected]:/etc/passwd _output/]
[...]
Executing: program /usr/bin/ssh host 127.0.0.1, user jan, command scp -v -d -f /etc/os-release
OpenSSH_8.1p1, LibreSSL 2.7.3
debug1: Reading configuration data /dev/null
debug1: Connecting to 127.0.0.1 [127.0.0.1] port 63697.
[...]
Executing: program /usr/bin/ssh host 127.0.0.1, user jan, command scp -v -d -f /etc/passwd
[...]
debug1: Connecting to 127.0.0.1 [127.0.0.1] port 63697.

So default:/etc/passwd was actually alpine:/etc/passwd.

It is not clear to me that the legacy syntax allows using different ports for different hosts. If it doesn't, then maybe we do need to have different code branches, and limit the legacy branch to a single source and a single destination, and at least one of them must be the host.

@davidcassany
Copy link
Contributor Author

davidcassany commented Nov 9, 2021

Oh I see, this is a use case I did not consider at all. I never thought about it as I was not even aware of the client possibilities using the URI syntax.

It looks like ports per host can be specified on per host basis under ~/ssh/config, which makes me think we can probably also set that as part of the -o flag. Another option could be to replicate what scp is actually doing behind the scenes: copy from remote host1 to local host and then copy again from local host to remote host2 (we make use of the -3 flag, so everything goes through the local host). So the scp command is split into two.

Gonna investigate if I can find a way specify port per host as part of -o flags, if this can be done it should be simple. If not then we will need a slightly more elaborate alternative, then I'd vote for using the old syntax in any case and split the command into two when the copy happens between two remote hosts.

@davidcassany
Copy link
Contributor Author

davidcassany commented Nov 9, 2021

All right, I realized this a bit more complex that I had expected. First, we do not need to set the port per host, we need to set the port per file (or per URI), as in this specific setup the host is likely to be always localhost. Take the example from @jandubois above, there the host at ssh eyes is always 127.0.0.1. So trying to set ssh_config from the command line won't work in that case. In addition, I just discovered that Host directive is not allowed to be used within the command line, so we can't specify ports on per host basis either.

Also, I realized there are a wide range of cases if we consider there can be multiple source arguments and plus a target argument where any of these can relate to a different host. Now, I realize why the URI syntax was introduced 😅 The port must be defined on per argument (source or target) basis and there is no way to express it using the old syntax.

Regarding that I see three different scenarios in terms of support:

  1. Lima does not support openssh clients prior v8.0. Then only a sanity check would be needed to drop a proper message in case of error.
  2. Lima does support openssh client prior to v8.0 but it does not support commands that result in multiple host shh command lines. In this case we need a switch for v8.0 as I initially did and, eventually, code some error handling when multiple hosts are involved in the command line and openssh is pre v8.0.
  3. Lima supports pre v8.0 openssh clients. In order to do this the logic becomes tricky, I guess there are many ways to do it, but a reliable one that comes to my mind is splitting any command into a bunch of remote host and local host commands. For instance:
  • limactl cp host0:/etc/os-release host1:/etc/passwd local/path resolves into:
    • scp -P host0_port <usr>@host0:/etc/os-release local/path
    • scp -P host1_port <usr>@host1:/etc/passwd local/path
  • or limactl cp host0:/etc/os-release host0:/etc/motd host1:/etc/passwd host3:some/path resolves into:
    • scp -P host0_port <usr>@host0:/etc/os-release host0:/etc/motd local/tmp/path
    • scp -P host3_port local/tmp/path/os-release local/tmp/path/motd <usr>@host3:some/path
    • scp -P host1_port host1:/etc/passwd local/tmp/path/
    • scp -P host3_port local/tmp/path/passwd <usr>@host3:some/path

On third option I don't feel like adding such a spaghetti logic just to support some legacy clients that will become unmaintained relatively soon.

I'd say a relatively good option is to stay in option 2, with a code similar to what I did on my first attempt. So keep the current behavior for any openssh client v8.0 or higher and then use the old legacy -P syntax for old clients and run into a meaningful error if multiple hosts are used within the same command line. I adapted the PR to this option.

@jandubois @AkihiroSuda any thoughts?

This commit restricts use of URIs in scp command to openssh
clients starting from v8.0. On legacy clients the old syntax is
used.

For pre v8.0 openSSH clients only commands involving a single instance
are allowed.

Signed-off-by: David Cassany <[email protected]>
@davidcassany davidcassany force-pushed the fix_scp_command_for_old_openssh branch from 8ca3ce3 to a3a7d44 Compare November 9, 2021 15:48
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now; I've tested both the legacy and the regular codepaths.

@jandubois jandubois merged commit 2d0572d into lima-vm:master Nov 15, 2021
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 this pull request may close these issues.

3 participants