Drop SCP

[pmeinhardt]

We should look into deprecating/dropping SCP support from SSHKit and instead implementing uploads and downloads in a different way.

The scp command is a historical protocol (called rcp) which relies upon that style of argument passing and encounters expansion problems. It has proven very difficult to add "security" to the scp model. All attempts to "detect" and "prevent" anomalous argument transfers stand a great chance of breaking existing workflows.

— https://lwn.net/Articles/835962/

The scp protocol is outdated, inflexible and not readily fixed. We recommend the use of more modern protocols like sftp and rsync for file transfer instead.

— https://www.openssh.com/txt/release-8.0

[pmeinhardt]

Mmh. 🤔 Based on what I've learned, it is at least difficult (if not impossible) to implement file transfers using SFTP if we want to allow for uploads and downloads to allow for the same convenient use of Context like we accept for running commands.

Most commonly SFTP is invoked as an SSH subsystem on a remote host (this is the case for OTP too). The subsystem command (sftp-server) may however be started in a directory different from the start directory for SSH shells/sessions. This would mean that SSHKit.run!/3 and SSHKit.upload!/4 might behave differently even though you're passing the same context. Something that I think we'd rather avoid.

On top of this issue around the base/context path, the handling of context options :umask, :user and :path would look very different from how they're handled for command execution. For SCP, we wrap the scp command in the appropriate command context. For sftp, essentially, we'd have to write file stats with adjusted permissions, figure out the numeric user and group IDs to manage these. This opens up more room for inconsistencies.

	command	file transfer
env	+	not applicable
path	+	+	for transfer, expand target relative to context path, 💥 has problem of sftp subsystem possibly starting out from a different directory which would introduce inconsistency between commands and file transfers for relative context paths
umask	+	+	mask could be applied to files by using :ssh_sftp.write_file_info/4
user	+	+	numeric user ID can be retrieved via id -u <name>, then written using :ssh_sftp.write_file_info/4
group	+	+	numeric group ID can be retrieved via id -g <name>, then written using :ssh_sftp.write_file_info/4

I thought about invoking the sftp-server command wrapped with the context execution environment. However, this would break on SFTP-only server configurations (no command shell) and servers often configure a chroot-jail via the "sftp" subsystem, which we wouldn't be able to reproduce reliably.

A bunch of the security concerns surrounding the scp command do not seem to apply to our typical use-cases and implementation. Also for many scenarios, it is assumed the SSH server has been compromised before or a man-in-the-middle can be established.

Seems like SCP will stick around for a bit. If we find solutions for the problems outlined above or we decide to not use Context for file transfers, we might revisit this. ✌️