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

Use SFTP internally for SCP command #194

Closed
wants to merge 5 commits into from
Closed

Conversation

Jakuje
Copy link
Contributor

@Jakuje Jakuje commented Jul 15, 2020

Based on the discussion on mailing list [1], this is initial implementation of SCP mode using SFTP. Comments, suggestions, reviews, feedback welcomed.

[1] https://lists.mindrot.org/pipermail/openssh-unix-dev/2020-June/038594.html

Copy link
Contributor

@djmdjm djmdjm left a comment

Choose a reason for hiding this comment

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

This is a good start and is less intrusive than I though it would be.

My comments are mostly nitpicks because this is clearly on a start of a path and not the end. I think to properly replace the scp protocol we'd need a few more things

  1. automatic sftp-scp fallback without need to reconnect if the remote end doesn't support sftp. This is probably possible using mux (perhaps mux-proxy) mode.
  2. some degree of bug-compatibility with scp's horrid file quoting (perhaps optional?)
  3. (eventually) inter-file pipelining of transfers, though this would require a fair amount of work on sftp-client.c

@mfriedl @daztucker can you take a look too?

sftp-client.c Outdated Show resolved Hide resolved
sftp-client.h Outdated Show resolved Hide resolved
scp.c Outdated Show resolved Hide resolved
scp.c Show resolved Hide resolved
scp.c Outdated Show resolved Hide resolved
@Jakuje Jakuje force-pushed the scp-sftp branch 3 times, most recently from 1ca0e78 to 2f47dc8 Compare July 17, 2020 07:25
@Jakuje
Copy link
Contributor Author

Jakuje commented Jul 17, 2020

Thank you for the comments. I addressed your suggestions in the current version.

I will be able to follow-up no earlier than in August on the fallback to scp (thanks for suggesting the mux -- it could actually work that way).

@mfriedl
Copy link
Contributor

mfriedl commented Jul 22, 2020

I had a quick first look and I think this looks great!
It also looks like we could put this in soon and keep defaulting to the scp protocol for a while, but
get people to use it and figure out problems.
I'll have a detailed look at the code soon!
thanks!

@Jakuje
Copy link
Contributor Author

Jakuje commented Aug 4, 2020

Updated to resolve conflicts.

I tried also to play with the fallback to scp from sftp, but the current code in sftp-client is really not ready for this as it calls fatal() in send_msg() from do_init(), which is the place we should just figure out that sftp subsystem request failed so it would require some more tweaking. Putting this aside for now.

@Jakuje
Copy link
Contributor Author

Jakuje commented Oct 19, 2020

I rebased on current master and change default to SFTP to give it more testing. Any update from your side?

@mfriedl
Copy link
Contributor

mfriedl commented Oct 27, 2020

I'd like to work on this if spare time permits.

@DemiMarie
Copy link

Can we please make quoting bug-compatibility optional? I really really would like to be able to avoid it in scripts.

@djmdjm
Copy link
Contributor

djmdjm commented Aug 3, 2021

All but the switch-default change have been committed (in af5d809 and 197e29f).

My plan is to leave the protocol as scp for at least one release and then look at switching.

Thanks Jakub for writing this!

@djmdjm djmdjm closed this Aug 3, 2021
bcl added a commit to weldr/lorax that referenced this pull request Jan 18, 2022
scp in openssh 8.7 will change to use sftp protocol. See
openssh/openssh-portable#194 for details.

This enables the sshd internal-sftp implementation so that newer scp
versions will continue to work as expected. Note that the sshd service
is only running during the installation if inst.sshd is passed on the
kernel cmdline.

Resolves: rhbz#2040770
bcl added a commit to weldr/lorax that referenced this pull request Jan 18, 2022
scp in openssh 8.7 will change to use sftp protocol. See
openssh/openssh-portable#194 for details.

This enables the sshd internal-sftp implementation so that newer scp
versions will continue to work as expected. Note that the sshd service
is only running during the installation if inst.sshd is passed on the
kernel cmdline.

Resolves: rhbz#2040770
bcl added a commit to weldr/lorax that referenced this pull request Jan 18, 2022
scp in openssh 8.7 will change to use sftp protocol. See
openssh/openssh-portable#194 for details.

This enables the sshd internal-sftp implementation so that newer scp
versions will continue to work as expected. Note that the sshd service
is only running during the installation if inst.sshd is passed on the
kernel cmdline.

Resolves: rhbz#2041770
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants