allow the usage of sftp on ssh-copy-id instead of remote-exec

[Blaimi]

this is useful for ssh-servers which don't provide shell-access but sftp

bz#3201

[jnothman]

Thanks for this draft!
This currently makes two separate connections to sftp, which for most users implies twice entering their password twice.
Can it be performed in a single connection? Sftp supports running local commands by prefixing them with ! but I've not yet tested whether it can be used here.

Is it worth considering potential concurrency issues? Even if the GET and PUT operations should be fast, we could PUT to a temporary file and check MTIME of the destination authorized_keys before overwriting.

[Blaimi]

This currently makes two separate connections to sftp, which for most users implies twice entering their password twice.

No, it does not. It's asking for the password only once: when opening the master connection which is reused several times (for the two sftp statements ATM, but there will be more because of the todos)

Is it worth considering potential concurrency issues? Even if the GET and PUT operations should be fast, we could PUT to a temporary file and check MTIME of the destination authorized_keys before overwriting.

I don't think it is worth, because you don't have the delay for entering the password (see above). Problems only occur if the file is (very) big or if the target system has SELinux enabled. In the case of SELinux, we can't do anything anyway …

[djmdjm]

This is good work, but the portable OpenSSH repository is just a mirror of ssh-copy-id from its upstream http://git.hands.com. I recommend sending your changes to the maintainer of ssh-copy-id (phil@hands.com) and we'll mirror them back once they're committed.

[Blaimi]

Thanks for giving the links, I didn't know them yet! We will adopt your recommendation when we think the commit is ready, i.e. worth for a review, i.e. when the todos are done. Until then I think this place is a nice rfc 😉

[jnothman]

Right. Now I see the shared connection. Neat! Thanks for responding.

[Jakuje]

The control path can not be this specific, since it would be very racy especially in multiuser systems or when copying keys to multiple systems "at once". It should be placed in some "random" filename/directory to avoid making it accessible by others and this race conditions.

Additionally, you are overriding this option blindly, if it was previously configured, so I would probably add some check (for example using ssh -G) to verify that this is not already set and use this one-shot value only if not.

The sftp line can be also written as sftp "$@:.ssh/authorized_keys" /tmp/ssh-copy-id_authorized_keys

And again, using the too-specific path /tmp/ssh-copy-id_authorized_keys for this critical file is also very racy -- it should be placed again in some temporary directory with random name.

[Blaimi]

both should be done with mktemp and trap to cleanup

[Blaimi]

Additionally, you are overriding this option blindly, if it was previously configured, so I would probably add some check (for example using ssh -G) to verify that this is not already set and use this one-shot value only if not.

I'm not sure if we should go for every eventuality. This works for the default-case and does not conflict with this setting otherwise. The only this is that a user opens a connection with another key and wants to reuse it here. Also: some configurations are also overridden lines above

The sftp line can be also written as sftp "$@:.ssh/authorized_keys" /tmp/ssh-copy-id_authorized_keys

this could result in sftp -p 23 example.com:.ssh/authorized_keys /tmp/ssh-copy-id_authorized_keys which will fail because sftp doesn't understand -p. To change this we would have to filter out the -p option and handle it separately for the ssh and sftp-commands. (port in sftp is -P; -pmeans “Preserve modification times”)

[phil-hands]

BTW This suggestion is being worked on here:

https://gitlab.com/phil_hands/ssh-copy-id/-/tree/bug/3201

Thanks again for the idea. Further comments are very welcome.