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

[HPR-888] ssh: use sftp by default for transferring files #160

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lbajolet-hashicorp
Copy link
Contributor

Recently Windows's SSH server implementation has changed the way they set the MOTW on the receiving end for file transfers, which coupled with the way Packer invokes the command, causes the transfers to fail since the command exits with a non-zero error code.

While this is going to be fixed in an upcoming release, we are changing the default method for transferring data from the host to the guest from scp to sftp.

This is motivated in part by the aforementioned problem with Windows's SSH server, but also by a decision on the OpenSSH part, where with version 9, they also decided to not use scp the protocol for sending/receiving files, and even when using the `scp' client, the protocol would default to sftp.

Given both decisions, coupled with the fact that scp is insecure and obsolete, we change the default method for transferring files between endpoints to sftp by default.

Recently Windows's SSH server implementation has changed the way they
set the MOTW on the receiving end for file transfers, which coupled with
the way Packer invokes the command, causes the transfers to fail since
the command exits with a non-zero error code.

While this is going to be fixed in an upcoming release, we are changing
the default method for transferring data from the host to the guest from
scp to sftp.

This is motivated in part by the aforementioned problem with Windows's
SSH server, but also by a decision on the OpenSSH part, where with
version 9, they also decided to not use scp the protocol for
sending/receiving files, and even when using the `scp' client, the
protocol would default to sftp.

Given both decisions, coupled with the fact that scp is insecure and
obsolete, we change the default method for transferring files between
endpoints to sftp by default.
@lbajolet-hashicorp lbajolet-hashicorp requested a review from a team as a code owner February 1, 2023 20:05
@nywilken nywilken added the version/bump minor A PR that changes behavior or contains breaking changes template configuration options. label Feb 1, 2023
@nywilken
Copy link
Member

nywilken commented Feb 1, 2023

These changes look good so far but lets not merge at this time. It will take some coordination across builder components. We can import this SDK from the the branch and test locally before merging.

@nywilken nywilken changed the title ssh: use sftp by default for transferring files [HPR-888] ssh: use sftp by default for transferring files Apr 10, 2023
@lbajolet-hashicorp
Copy link
Contributor Author

lbajolet-hashicorp commented Apr 24, 2023

Coming back to this one, I ran a subset of the acceptance tests on both the amazon and the googlecompute plugins with a local replace directive in the go.mod files.

Both show the following in their logs: [DEBUG] sftp: uploading /tmp/script_<temp_number>.sh, which confirms that sftp is being used rather than scp with this modification in.

From the looks of it, it seems that the change works, I'll make sure we don't have a problem when transferring directories in addition to commands, but I think we can consider merging this PR soon.

@lbajolet-hashicorp
Copy link
Contributor Author

Regarding directories, I notice that the two protocols behave differently, and while sftp works for most use-cases, we should make sure that the two behave the same so that we don't unexpectedly break some clients.

@lbajolet-hashicorp lbajolet-hashicorp marked this pull request as draft April 25, 2023 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change version/bump minor A PR that changes behavior or contains breaking changes template configuration options.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants