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 rsync instead of scp for file copying #1323

Merged
merged 1 commit into from May 1, 2020
Merged

Conversation

@adisbladis
Copy link
Member

adisbladis commented Apr 24, 2020

This will allow us to pass options to openssh and escalate privileges (see #1270) more easily.

Closes #1322

@adisbladis adisbladis requested a review from grahamc Apr 24, 2020
@adisbladis adisbladis force-pushed the adisbladis:scp-to-rsync branch from b626820 to 1e43587 Apr 24, 2020
@lordcirth
Copy link

lordcirth commented Apr 24, 2020

I cannot find a reference now, but I distinctly recall the openssh devs mentioning that scp was supported for legacy reasons, and not really recommended. It was during the discussion of CVE-2018-20685.

default.nix Show resolved Hide resolved
This will allow us to pass options to openssh and escalate
privileges (see #1270) more easily.

Closes #1322
@adisbladis adisbladis force-pushed the adisbladis:scp-to-rsync branch from 1e43587 to a07bdf6 May 1, 2020
@@ -419,27 +419,35 @@ def copy_closure_to(self, path):
env=env,
)

def get_scp_name(self):
def _get_scp_name(self) -> str:

This comment has been minimized.

Copy link
@grahamc

grahamc May 1, 2020

Member

double-checked that this wasn't used in plugins. I probably wasn't conclusive, but I checked at least GCE, AWS, and nix-community's plugins.

@grahamc
Copy link
Member

grahamc commented May 1, 2020

Just looking at the semantics of the trailing slash in rsync vs. scp.

rsync's manpage:

You use rsync in the same way you use rcp. You must specify a source and a destination, one of which may be remote.

scp's manpage:

scp is based on the rcp program in Bx source code from the Regents of the University of California.

I think we're good on this front.

@grahamc
Copy link
Member

grahamc commented May 1, 2020

I gave this a try on my network and it worked well. I added new keys and did a send-keys, which also worked. Notes:

  1. send-keys with new keys requires a deploy first as the list of keys is cached
  2. deployment.keyFile actually is a file: directories are not (and have never been, afaik) supported.
@grahamc grahamc merged commit d3c127c into NixOS:master May 1, 2020
6 checks passed
6 checks passed
parsing
Details
build
Details
black
Details
mypy
Details
mypy-ratchet
Details
coverage
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.