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

sftp: Allow password entry #1270

Merged
merged 2 commits into from
Sep 23, 2017
Merged

sftp: Allow password entry #1270

merged 2 commits into from
Sep 23, 2017

Conversation

fd0
Copy link
Member

@fd0 fd0 commented Sep 23, 2017

This was a bit tricky: We start the ssh binary, but we want it to ignore SIGINT. In contrast, restic itself should process SIGINT and clean up properly. Before, we used setsid() to give the ssh process its own process group, but that means it cannot prompt the user for a password because the tty is gone.

So, now we're passing in two functions that ignore SIGINT just before the ssh process is started and re-install it after start.

Closes #448

This was a bit tricky: We start the ssh binary, but we want it to ignore
SIGINT. In contrast, restic itself should process SIGINT and clean up
properly. Before, we used `setsid()` to give the ssh process its own
process group, but that means it cannot prompt the user for a password
because the tty is gone.

So, now we're passing in two functions that ignore SIGINT just before
the ssh process is started and re-install it after start.
@codecov-io
Copy link

codecov-io commented Sep 23, 2017

Codecov Report

Merging #1270 into master will increase coverage by 0.07%.
The diff coverage is 52.38%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1270      +/-   ##
=========================================
+ Coverage   52.82%   52.9%   +0.07%     
=========================================
  Files         131     130       -1     
  Lines       12437   12441       +4     
=========================================
+ Hits         6570    6582      +12     
+ Misses       5106    5090      -16     
- Partials      761     769       +8
Impacted Files Coverage Δ
cmd/restic/global.go 20.22% <0%> (ø) ⬆️
internal/backend/sftp/sftp.go 59.04% <50%> (-1.53%) ⬇️
cmd/restic/cleanup.go 37.5% <71.42%> (-0.34%) ⬇️
internal/backend/s3/s3.go 59.65% <0%> (-1.38%) ⬇️
internal/archiver/archiver.go 65.2% <0%> (-0.17%) ⬇️
internal/checker/checker.go 72.02% <0%> (+3.83%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f9bf19...fb9729f. Read the comment docs.

@fd0
Copy link
Member Author

fd0 commented Sep 23, 2017

@Mebus @rawtaz can maybe one of you test this PR?

@rawtaz
Copy link
Contributor

rawtaz commented Sep 23, 2017

Works great here, for all I can tell it now asks me for the password of my key when the ssh-agent isn't loaded. Great!

@fd0
Copy link
Member Author

fd0 commented Sep 23, 2017

Nice, thanks for the feedback!

@fd0 fd0 merged commit fb9729f into master Sep 23, 2017
fd0 added a commit that referenced this pull request Sep 23, 2017
@fd0 fd0 deleted the sftp-allow-password-prompt branch October 3, 2017 10:43
@aadrian
Copy link

aadrian commented Jul 21, 2020

@fd0 how to use this feature? The documentation still mentions only the key based config: https://restic.readthedocs.io/en/latest/030_preparing_a_new_repo.html#sftp

Thanks in advance.

@fd0
Copy link
Member Author

fd0 commented Jul 21, 2020

Hm, you should have been asked for the password interactively. If that doesn't work, please open a new issue so we can triage it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

request: SFTP with password prompt
4 participants