From fb9729fdb9bf1e15dc6c90383df85a8968f6ddb0 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sat, 23 Sep 2017 11:21:27 +0200 Subject: [PATCH] sftp: Allow password entry 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. --- cmd/restic/global.go | 4 ++-- internal/backend/sftp/layout_test.go | 2 +- internal/backend/sftp/sftp.go | 31 ++++++++++++++++----------- internal/backend/sftp/sftp_test.go | 4 ++-- internal/backend/sftp/sftp_unix.go | 13 ----------- internal/backend/sftp/sftp_windows.go | 11 ---------- 6 files changed, 24 insertions(+), 41 deletions(-) delete mode 100644 internal/backend/sftp/sftp_unix.go delete mode 100644 internal/backend/sftp/sftp_windows.go diff --git a/cmd/restic/global.go b/cmd/restic/global.go index ccfe1b9c0..3b9f2853a 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -470,7 +470,7 @@ func open(s string, opts options.Options) (restic.Backend, error) { case "local": be, err = local.Open(cfg.(local.Config)) case "sftp": - be, err = sftp.Open(cfg.(sftp.Config)) + be, err = sftp.Open(cfg.(sftp.Config), SuspendSignalHandler, InstallSignalHandler) case "s3": be, err = s3.Open(cfg.(s3.Config)) case "gs": @@ -522,7 +522,7 @@ func create(s string, opts options.Options) (restic.Backend, error) { case "local": return local.Create(cfg.(local.Config)) case "sftp": - return sftp.Create(cfg.(sftp.Config)) + return sftp.Create(cfg.(sftp.Config), SuspendSignalHandler, InstallSignalHandler) case "s3": return s3.Create(cfg.(s3.Config)) case "gs": diff --git a/internal/backend/sftp/layout_test.go b/internal/backend/sftp/layout_test.go index 1aa4da5b9..9a5878e0d 100644 --- a/internal/backend/sftp/layout_test.go +++ b/internal/backend/sftp/layout_test.go @@ -46,7 +46,7 @@ func TestLayout(t *testing.T) { Command: fmt.Sprintf("%q -e", sftpServer), Path: repo, Layout: test.layout, - }) + }, nil, nil) if err != nil { t.Fatal(err) } diff --git a/internal/backend/sftp/sftp.go b/internal/backend/sftp/sftp.go index 0014f7549..27ecabd81 100644 --- a/internal/backend/sftp/sftp.go +++ b/internal/backend/sftp/sftp.go @@ -36,7 +36,7 @@ var _ restic.Backend = &SFTP{} const defaultLayout = "default" -func startClient(program string, args ...string) (*SFTP, error) { +func startClient(preExec, postExec func(), program string, args ...string) (*SFTP, error) { debug.Log("start client %v %v", program, args) // Connect to a remote host and request the sftp subsystem via the 'ssh' // command. This assumes that passwordless login is correctly configured. @@ -55,9 +55,6 @@ func startClient(program string, args ...string) (*SFTP, error) { } }() - // ignore signals sent to the parent (e.g. SIGINT) - cmd.SysProcAttr = ignoreSigIntProcAttr() - // get stdin and stdout wr, err := cmd.StdinPipe() if err != nil { @@ -68,11 +65,19 @@ func startClient(program string, args ...string) (*SFTP, error) { return nil, errors.Wrap(err, "cmd.StdoutPipe") } + if preExec != nil { + preExec() + } + // start the process if err := cmd.Start(); err != nil { return nil, errors.Wrap(err, "cmd.Start") } + if postExec != nil { + postExec() + } + // wait in a different goroutine ch := make(chan error, 1) go func() { @@ -104,8 +109,9 @@ func (r *SFTP) clientError() error { } // Open opens an sftp backend as described by the config by running -// "ssh" with the appropriate arguments (or cfg.Command, if set). -func Open(cfg Config) (*SFTP, error) { +// "ssh" with the appropriate arguments (or cfg.Command, if set). The function +// preExec is run just before, postExec just after starting a program. +func Open(cfg Config, preExec, postExec func()) (*SFTP, error) { debug.Log("open backend with config %#v", cfg) cmd, args, err := buildSSHCommand(cfg) @@ -113,7 +119,7 @@ func Open(cfg Config) (*SFTP, error) { return nil, err } - sftp, err := startClient(cmd, args...) + sftp, err := startClient(preExec, postExec, cmd, args...) if err != nil { debug.Log("unable to start program: %v", err) return nil, err @@ -225,15 +231,16 @@ func buildSSHCommand(cfg Config) (cmd string, args []string, err error) { return cmd, args, nil } -// Create creates an sftp backend as described by the config by running -// "ssh" with the appropriate arguments (or cfg.Command, if set). -func Create(cfg Config) (*SFTP, error) { +// Create creates an sftp backend as described by the config by running "ssh" +// with the appropriate arguments (or cfg.Command, if set). The function +// preExec is run just before, postExec just after starting a program. +func Create(cfg Config, preExec, postExec func()) (*SFTP, error) { cmd, args, err := buildSSHCommand(cfg) if err != nil { return nil, err } - sftp, err := startClient(cmd, args...) + sftp, err := startClient(preExec, postExec, cmd, args...) if err != nil { debug.Log("unable to start program: %v", err) return nil, err @@ -261,7 +268,7 @@ func Create(cfg Config) (*SFTP, error) { } // open backend - return Open(cfg) + return Open(cfg, preExec, postExec) } // Location returns this backend's location (the directory name). diff --git a/internal/backend/sftp/sftp_test.go b/internal/backend/sftp/sftp_test.go index 8449c6daa..56d9f9172 100644 --- a/internal/backend/sftp/sftp_test.go +++ b/internal/backend/sftp/sftp_test.go @@ -51,13 +51,13 @@ func newTestSuite(t testing.TB) *test.Suite { // CreateFn is a function that creates a temporary repository for the tests. Create: func(config interface{}) (restic.Backend, error) { cfg := config.(sftp.Config) - return sftp.Create(cfg) + return sftp.Create(cfg, nil, nil) }, // OpenFn is a function that opens a previously created temporary repository. Open: func(config interface{}) (restic.Backend, error) { cfg := config.(sftp.Config) - return sftp.Open(cfg) + return sftp.Open(cfg, nil, nil) }, // CleanupFn removes data created during the tests. diff --git a/internal/backend/sftp/sftp_unix.go b/internal/backend/sftp/sftp_unix.go deleted file mode 100644 index f924f0d05..000000000 --- a/internal/backend/sftp/sftp_unix.go +++ /dev/null @@ -1,13 +0,0 @@ -// +build !windows - -package sftp - -import ( - "syscall" -) - -// ignoreSigIntProcAttr returns a syscall.SysProcAttr that -// disables SIGINT on parent. -func ignoreSigIntProcAttr() *syscall.SysProcAttr { - return &syscall.SysProcAttr{Setsid: true} -} diff --git a/internal/backend/sftp/sftp_windows.go b/internal/backend/sftp/sftp_windows.go deleted file mode 100644 index 62f748c6d..000000000 --- a/internal/backend/sftp/sftp_windows.go +++ /dev/null @@ -1,11 +0,0 @@ -package sftp - -import ( - "syscall" -) - -// ignoreSigIntProcAttr returns a default syscall.SysProcAttr -// on Windows. -func ignoreSigIntProcAttr() *syscall.SysProcAttr { - return &syscall.SysProcAttr{} -}