From 10449c86a45e99351786924b829493b7df886476 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Fri, 9 Jun 2023 11:02:40 +0100 Subject: [PATCH] sftp: add --sftp-ssh to specify an external ssh binary to use This allows using an external ssh binary instead of the built in ssh library for making SFTP connections. This makes another integration test target TestSFTPRcloneSSH: Fixes #7012 --- backend/sftp/sftp.go | 90 +++++---- backend/sftp/sftp_test.go | 10 + backend/sftp/ssh.go | 73 +++++++ backend/sftp/ssh_external.go | 223 +++++++++++++++++++++ backend/sftp/ssh_internal.go | 90 +++++++++ fstest/test_all/config.yaml | 3 + fstest/testserver/init.d/TestSFTPRcloneSSH | 19 ++ 7 files changed, 471 insertions(+), 37 deletions(-) create mode 100644 backend/sftp/ssh.go create mode 100644 backend/sftp/ssh_external.go create mode 100644 backend/sftp/ssh_internal.go create mode 100755 fstest/testserver/init.d/TestSFTPRcloneSSH diff --git a/backend/sftp/sftp.go b/backend/sftp/sftp.go index 62deb3d90..0f914cdfa 100644 --- a/backend/sftp/sftp.go +++ b/backend/sftp/sftp.go @@ -27,7 +27,6 @@ import ( "github.com/rclone/rclone/fs/config/configmap" "github.com/rclone/rclone/fs/config/configstruct" "github.com/rclone/rclone/fs/config/obscure" - "github.com/rclone/rclone/fs/fshttp" "github.com/rclone/rclone/fs/hash" "github.com/rclone/rclone/lib/env" "github.com/rclone/rclone/lib/pacer" @@ -386,6 +385,36 @@ Note: This can affect the outcome of key negotiation with the server even if ser Example: ssh-ed25519 ssh-rsa ssh-dss +`, + Advanced: true, + }, { + Name: "ssh", + Default: fs.SpaceSepList{}, + Help: `Path and arguments to external ssh binary. + +Normally rclone will use its internal ssh library to connect to the +SFTP server. However it does not implement all possible ssh options so +it may be desirable to use an external ssh binary. + +Rclone ignores all the internal config if you use this option and +expects you to configure the ssh binary with the user/host/port and +any other options you need. + +**Important** The ssh command must log in without asking for a +password so needs to be configured with keys or certificates. + +Rclone will run the command supplied either with the additional +arguments "-s sftp" to access the SFTP subsystem or with commands such +as "md5sum /path/to/file" appended to read checksums. + +Any arguments with spaces in should be surrounded by "double quotes". + +An example setting might be: + + ssh -o ServerAliveInterval=20 user@example.com + +Note that when using an external ssh binary rclone makes a new ssh +connection for every hash it calculates. `, Advanced: true, }}, @@ -427,6 +456,7 @@ type Options struct { KeyExchange fs.SpaceSepList `config:"key_exchange"` MACs fs.SpaceSepList `config:"macs"` HostKeyAlgorithms fs.SpaceSepList `config:"host_key_algorithms"` + SSH fs.SpaceSepList `config:"ssh"` } // Fs stores the interface to the remote SFTP files @@ -463,41 +493,16 @@ type Object struct { sha1sum *string // Cached SHA1 checksum } -// dial starts a client connection to the given SSH server. It is a -// convenience function that connects to the given network address, -// initiates the SSH handshake, and then sets up a Client. -func (f *Fs) dial(ctx context.Context, network, addr string, sshConfig *ssh.ClientConfig) (*ssh.Client, error) { - dialer := fshttp.NewDialer(ctx) - conn, err := dialer.Dial(network, addr) - if err != nil { - return nil, err - } - c, chans, reqs, err := ssh.NewClientConn(conn, addr, sshConfig) - if err != nil { - return nil, err - } - fs.Debugf(f, "New connection %s->%s to %q", c.LocalAddr(), c.RemoteAddr(), c.ServerVersion()) - return ssh.NewClient(c, chans, reqs), nil -} - // conn encapsulates an ssh client and corresponding sftp client type conn struct { - sshClient *ssh.Client + sshClient sshClient sftpClient *sftp.Client err chan error } // Wait for connection to close func (c *conn) wait() { - c.err <- c.sshClient.Conn.Wait() -} - -// Send a keepalive over the ssh connection -func (c *conn) sendKeepAlive() { - _, _, err := c.sshClient.SendRequest("keepalive@openssh.com", true, nil) - if err != nil { - fs.Debugf(nil, "Failed to send keep alive: %v", err) - } + c.err <- c.sshClient.Wait() } // Send keepalives every interval over the ssh connection until done is closed @@ -509,7 +514,7 @@ func (c *conn) sendKeepAlives(interval time.Duration) (done chan struct{}) { for { select { case <-t.C: - c.sendKeepAlive() + c.sshClient.SendKeepAlive() case <-done: return } @@ -561,7 +566,11 @@ func (f *Fs) sftpConnection(ctx context.Context) (c *conn, err error) { c = &conn{ err: make(chan error, 1), } - c.sshClient, err = f.dial(ctx, "tcp", f.opt.Host+":"+f.opt.Port, f.config) + if len(f.opt.SSH) == 0 { + c.sshClient, err = f.newSSHClientInternal(ctx, "tcp", f.opt.Host+":"+f.opt.Port, f.config) + } else { + c.sshClient, err = f.newSSHClientExternal() + } if err != nil { return nil, fmt.Errorf("couldn't connect SSH: %w", err) } @@ -575,7 +584,7 @@ func (f *Fs) sftpConnection(ctx context.Context) (c *conn, err error) { } // Set any environment variables on the ssh.Session -func (f *Fs) setEnv(s *ssh.Session) error { +func (f *Fs) setEnv(s sshSession) error { for _, env := range f.opt.SetEnv { equal := strings.IndexRune(env, '=') if equal < 0 { @@ -592,8 +601,8 @@ func (f *Fs) setEnv(s *ssh.Session) error { // Creates a new SFTP client on conn, using the specified subsystem // or sftp server, and zero or more option functions -func (f *Fs) newSftpClient(conn *ssh.Client, opts ...sftp.ClientOption) (*sftp.Client, error) { - s, err := conn.NewSession() +func (f *Fs) newSftpClient(client sshClient, opts ...sftp.ClientOption) (*sftp.Client, error) { + s, err := client.NewSession() if err != nil { return nil, err } @@ -666,6 +675,9 @@ func (f *Fs) getSftpConnection(ctx context.Context) (c *conn, err error) { // Getwd request func (f *Fs) putSftpConnection(pc **conn, err error) { c := *pc + if !c.sshClient.CanReuse() { + return + } *pc = nil if err != nil { // work out if this is an expected error @@ -744,6 +756,10 @@ func NewFs(ctx context.Context, name, root string, m configmap.Mapper) (fs.Fs, e if err != nil { return nil, err } + if len(opt.SSH) != 0 && (opt.User != "" || opt.Host != "" || opt.Port != "") { + fs.Logf(name, "--sftp-ssh is in use - ignoring user/host/port from config - set in the parameters to --sftp-ssh (remove them from the config to silence this warning)") + } + if opt.User == "" { opt.User = currentUser } @@ -1016,8 +1032,8 @@ func NewFsWithConnection(ctx context.Context, f *Fs, name string, root string, m fs.Debugf(f, "Failed to get shell session for shell type detection command: %v", err) } else { var stdout, stderr bytes.Buffer - session.Stdout = &stdout - session.Stderr = &stderr + session.SetStdout(&stdout) + session.SetStderr(&stderr) shellCmd := "echo ${ShellId}%ComSpec%" fs.Debugf(f, "Running shell type detection remote command: %s", shellCmd) err = session.Run(shellCmd) @@ -1427,8 +1443,8 @@ func (f *Fs) run(ctx context.Context, cmd string) ([]byte, error) { }() var stdout, stderr bytes.Buffer - session.Stdout = &stdout - session.Stderr = &stderr + session.SetStdout(&stdout) + session.SetStderr(&stderr) fs.Debugf(f, "Running remote command: %s", cmd) err = session.Run(cmd) diff --git a/backend/sftp/sftp_test.go b/backend/sftp/sftp_test.go index 89a267922..c5ee136a5 100644 --- a/backend/sftp/sftp_test.go +++ b/backend/sftp/sftp_test.go @@ -30,3 +30,13 @@ func TestIntegration2(t *testing.T) { NilObject: (*sftp.Object)(nil), }) } + +func TestIntegration3(t *testing.T) { + if *fstest.RemoteName != "" { + t.Skip("skipping as -remote is set") + } + fstests.Run(t, &fstests.Opt{ + RemoteName: "TestSFTPRcloneSSH:", + NilObject: (*sftp.Object)(nil), + }) +} diff --git a/backend/sftp/ssh.go b/backend/sftp/ssh.go new file mode 100644 index 000000000..4c6e65aca --- /dev/null +++ b/backend/sftp/ssh.go @@ -0,0 +1,73 @@ +//go:build !plan9 +// +build !plan9 + +package sftp + +import "io" + +// Interfaces for ssh client and session implemented in ssh_internal.go and ssh_external.go + +// An interface for an ssh client to abstract over internal ssh library and external binary +type sshClient interface { + // Wait blocks until the connection has shut down, and returns the + // error causing the shutdown. + Wait() error + + // SendKeepAlive sends a keepalive message to keep the connection open + SendKeepAlive() + + // Close the connection + Close() error + + // NewSession opens a new sshSession for this sshClient. (A + // session is a remote execution of a program.) + NewSession() (sshSession, error) + + // CanReuse indicates if this client can be reused + CanReuse() bool +} + +// An interface for an ssh session to abstract over internal ssh library and external binary +type sshSession interface { + // Setenv sets an environment variable that will be applied to any + // command executed by Shell or Run. + Setenv(name, value string) error + + // Start runs cmd on the remote host. Typically, the remote + // server passes cmd to the shell for interpretation. + // A Session only accepts one call to Run, Start or Shell. + Start(cmd string) error + + // StdinPipe returns a pipe that will be connected to the + // remote command's standard input when the command starts. + StdinPipe() (io.WriteCloser, error) + + // StdoutPipe returns a pipe that will be connected to the + // remote command's standard output when the command starts. + // There is a fixed amount of buffering that is shared between + // stdout and stderr streams. If the StdoutPipe reader is + // not serviced fast enough it may eventually cause the + // remote command to block. + StdoutPipe() (io.Reader, error) + + // RequestSubsystem requests the association of a subsystem + // with the session on the remote host. A subsystem is a + // predefined command that runs in the background when the ssh + // session is initiated + RequestSubsystem(subsystem string) error + + // Run runs cmd on the remote host. Typically, the remote + // server passes cmd to the shell for interpretation. + // A Session only accepts one call to Run, Start, Shell, Output, + // or CombinedOutput. + Run(cmd string) error + + // Close the session + Close() error + + // Set the stdout + SetStdout(io.Writer) + + // Set the stderr + SetStderr(io.Writer) +} diff --git a/backend/sftp/ssh_external.go b/backend/sftp/ssh_external.go new file mode 100644 index 000000000..9635401c0 --- /dev/null +++ b/backend/sftp/ssh_external.go @@ -0,0 +1,223 @@ +//go:build !plan9 +// +build !plan9 + +package sftp + +import ( + "context" + "errors" + "fmt" + "io" + "os/exec" + "strings" + + "github.com/rclone/rclone/fs" +) + +// Implement the sshClient interface for external ssh programs +type sshClientExternal struct { + f *Fs + session *sshSessionExternal +} + +func (f *Fs) newSSHClientExternal() (sshClient, error) { + return &sshClientExternal{f: f}, nil +} + +// Wait for connection to close +func (s *sshClientExternal) Wait() error { + if s.session == nil { + return nil + } + return s.session.Wait() +} + +// Send a keepalive over the ssh connection +func (s *sshClientExternal) SendKeepAlive() { + // Up to the user to configure -o ServerAliveInterval=20 on their ssh connections +} + +// Close the connection +func (s *sshClientExternal) Close() error { + if s.session == nil { + return nil + } + return s.session.Close() +} + +// NewSession makes a new external SSH connection +func (s *sshClientExternal) NewSession() (sshSession, error) { + session := s.f.newSshSessionExternal() + if s.session == nil { + fs.Debugf(s.f, "ssh external: creating additional session") + } + return session, nil +} + +// CanReuse indicates if this client can be reused +func (s *sshClientExternal) CanReuse() bool { + if s.session == nil { + return true + } + exited := s.session.exited() + canReuse := !exited && s.session.runningSFTP + // fs.Debugf(s.f, "ssh external: CanReuse %v, exited=%v runningSFTP=%v", canReuse, exited, s.session.runningSFTP) + return canReuse +} + +// Check interfaces +var _ sshClient = &sshClientExternal{} + +// implement the sshSession interface for external ssh binary +type sshSessionExternal struct { + f *Fs + cmd *exec.Cmd + cancel func() + startCalled bool + runningSFTP bool +} + +func (f *Fs) newSshSessionExternal() *sshSessionExternal { + s := &sshSessionExternal{ + f: f, + } + + // Make a cancellation function for this to call in Close() + ctx, cancel := context.WithCancel(context.Background()) + s.cancel = cancel + + // Connect to a remote host and request the sftp subsystem via + // the 'ssh' command. This assumes that passwordless login is + // correctly configured. + ssh := append([]string(nil), s.f.opt.SSH...) + s.cmd = exec.CommandContext(ctx, ssh[0], ssh[1:]...) + + // Allow the command a short time only to shut down + // FIXME enable when we get rid of go1.19 + // s.cmd.WaitDelay = time.Second + + return s +} + +// Setenv sets an environment variable that will be applied to any +// command executed by Shell or Run. +func (s *sshSessionExternal) Setenv(name, value string) error { + return errors.New("ssh external: can't set environment variables") +} + +const requestSubsystem = "***Subsystem***:" + +// Start runs cmd on the remote host. Typically, the remote +// server passes cmd to the shell for interpretation. +// A Session only accepts one call to Run, Start or Shell. +func (s *sshSessionExternal) Start(cmd string) error { + if s.startCalled { + return errors.New("internal error: ssh external: command already running") + } + s.startCalled = true + + // Adjust the args + if strings.HasPrefix(cmd, requestSubsystem) { + s.cmd.Args = append(s.cmd.Args, "-s", cmd[len(requestSubsystem):]) + s.runningSFTP = true + } else { + s.cmd.Args = append(s.cmd.Args, cmd) + s.runningSFTP = false + } + + fs.Debugf(s.f, "ssh external: running: %v", fs.SpaceSepList(s.cmd.Args)) + + // start the process + err := s.cmd.Start() + if err != nil { + return fmt.Errorf("ssh external: start process: %w", err) + } + + return nil +} + +// RequestSubsystem requests the association of a subsystem +// with the session on the remote host. A subsystem is a +// predefined command that runs in the background when the ssh +// session is initiated +func (s *sshSessionExternal) RequestSubsystem(subsystem string) error { + return s.Start(requestSubsystem + subsystem) +} + +// StdinPipe returns a pipe that will be connected to the +// remote command's standard input when the command starts. +func (s *sshSessionExternal) StdinPipe() (io.WriteCloser, error) { + rd, err := s.cmd.StdinPipe() + if err != nil { + return nil, fmt.Errorf("ssh external: stdin pipe: %w", err) + } + return rd, nil +} + +// StdoutPipe returns a pipe that will be connected to the +// remote command's standard output when the command starts. +// There is a fixed amount of buffering that is shared between +// stdout and stderr streams. If the StdoutPipe reader is +// not serviced fast enough it may eventually cause the +// remote command to block. +func (s *sshSessionExternal) StdoutPipe() (io.Reader, error) { + wr, err := s.cmd.StdoutPipe() + if err != nil { + return nil, fmt.Errorf("ssh external: stdout pipe: %w", err) + } + return wr, nil +} + +// Return whether the command has finished or not +func (s *sshSessionExternal) exited() bool { + return s.cmd.ProcessState != nil +} + +// Wait for the command to exit +func (s *sshSessionExternal) Wait() error { + if s.exited() { + return nil + } + err := s.cmd.Wait() + if err == nil { + fs.Debugf(s.f, "ssh external: command exited OK") + } else { + fs.Debugf(s.f, "ssh external: command exited with error: %v", err) + } + return err +} + +// Run runs cmd on the remote host. Typically, the remote +// server passes cmd to the shell for interpretation. +// A Session only accepts one call to Run, Start, Shell, Output, +// or CombinedOutput. +func (s *sshSessionExternal) Run(cmd string) error { + err := s.Start(cmd) + if err != nil { + return err + } + return s.Wait() +} + +// Close the external ssh +func (s *sshSessionExternal) Close() error { + fs.Debugf(s.f, "ssh external: close") + // Cancel the context which kills the process + s.cancel() + // Wait for it to finish + _ = s.Wait() + return nil +} + +// Set the stdout +func (s *sshSessionExternal) SetStdout(wr io.Writer) { + s.cmd.Stdout = wr +} + +// Set the stderr +func (s *sshSessionExternal) SetStderr(wr io.Writer) { + s.cmd.Stderr = wr +} + +// Check interfaces +var _ sshSession = &sshSessionExternal{} diff --git a/backend/sftp/ssh_internal.go b/backend/sftp/ssh_internal.go new file mode 100644 index 000000000..4237a8e28 --- /dev/null +++ b/backend/sftp/ssh_internal.go @@ -0,0 +1,90 @@ +//go:build !plan9 +// +build !plan9 + +package sftp + +import ( + "context" + "io" + + "github.com/rclone/rclone/fs" + "github.com/rclone/rclone/fs/fshttp" + "golang.org/x/crypto/ssh" +) + +// Internal ssh connections with "golang.org/x/crypto/ssh" + +type sshClientInternal struct { + srv *ssh.Client +} + +// newSSHClientInternal starts a client connection to the given SSH server. It is a +// convenience function that connects to the given network address, +// initiates the SSH handshake, and then sets up a Client. +func (f *Fs) newSSHClientInternal(ctx context.Context, network, addr string, sshConfig *ssh.ClientConfig) (sshClient, error) { + dialer := fshttp.NewDialer(ctx) + conn, err := dialer.Dial(network, addr) + if err != nil { + return nil, err + } + c, chans, reqs, err := ssh.NewClientConn(conn, addr, sshConfig) + if err != nil { + return nil, err + } + fs.Debugf(f, "New connection %s->%s to %q", c.LocalAddr(), c.RemoteAddr(), c.ServerVersion()) + srv := ssh.NewClient(c, chans, reqs) + return sshClientInternal{srv}, nil +} + +// Wait for connection to close +func (s sshClientInternal) Wait() error { + return s.srv.Conn.Wait() +} + +// Send a keepalive over the ssh connection +func (s sshClientInternal) SendKeepAlive() { + _, _, err := s.srv.SendRequest("keepalive@openssh.com", true, nil) + if err != nil { + fs.Debugf(nil, "Failed to send keep alive: %v", err) + } +} + +// Close the connection +func (s sshClientInternal) Close() error { + return s.srv.Close() +} + +// CanReuse indicates if this client can be reused +func (s sshClientInternal) CanReuse() bool { + return true +} + +// Check interfaces +var _ sshClient = sshClientInternal{} + +// Thin wrapper for *ssh.Session to implement sshSession interface +type sshSessionInternal struct { + *ssh.Session +} + +// Set the stdout +func (s sshSessionInternal) SetStdout(wr io.Writer) { + s.Session.Stdout = wr +} + +// Set the stderr +func (s sshSessionInternal) SetStderr(wr io.Writer) { + s.Session.Stderr = wr +} + +// NewSession makes an sshSession from an sshClient +func (s sshClientInternal) NewSession() (sshSession, error) { + session, err := s.srv.NewSession() + if err != nil { + return nil, err + } + return sshSessionInternal{Session: session}, nil +} + +// Check interfaces +var _ sshSession = sshSessionInternal{} diff --git a/fstest/test_all/config.yaml b/fstest/test_all/config.yaml index 2edaf90a3..c8a35d1b7 100644 --- a/fstest/test_all/config.yaml +++ b/fstest/test_all/config.yaml @@ -238,6 +238,9 @@ backends: - backend: "sftp" remote: "TestSFTPRclone:" fastlist: false + - backend: "sftp" + remote: "TestSFTPRcloneSSH:" + fastlist: false - backend: "sftp" remote: "TestSFTPRsyncNet:" fastlist: false diff --git a/fstest/testserver/init.d/TestSFTPRcloneSSH b/fstest/testserver/init.d/TestSFTPRcloneSSH new file mode 100755 index 000000000..71bf4076b --- /dev/null +++ b/fstest/testserver/init.d/TestSFTPRcloneSSH @@ -0,0 +1,19 @@ +#!/bin/bash + +set -e + +# No password to make working with ssh binary easy + +NAME=rclone-serve-sftp-ssh +IP=127.0.0.1 +PORT=28623 + +start() { + run rclone serve sftp --addr ${IP}:${PORT} ${DATADIR} + + echo type=sftp + echo ssh=ssh -o StrictHostKeyChecking=no -p ${PORT} user@${IP} + echo _connect=${IP}:${PORT} +} + +. $(dirname "$0")/rclone-serve.bash