The sftp backend could be used as the source of a multi-thread copy -
each chunk of a download is read on its own connection - but not as the
destination, because it did not implement OpenWriterAt. Uploading a
single large file was therefore limited to one connection while
downloading the same file was not, capping upload throughput well below
the link speed on high latency connections.
This implements OpenWriterAt for the sftp backend using lib/filepool. A
small pool of open write handles, each backed by its own connection,
lets the core write the chunks of a large file concurrently over several
connections. The file is created and truncated once up front so every
chunk offset is valid before the concurrent writes start.
Multi-thread uploads are off by default and turned on with the new
--sftp-multithread-upload flag, since many sftp servers only accept
sequential writes and would fail large uploads otherwise. Even when
enabled they fall back to a single connection when
--sftp-disable-concurrent-writes is set (a server that can't take
out-of-order packets on one handle won't take several handles either) or
when --sftp-connections caps the pool (the per-file fan-out would
otherwise deadlock waiting on it).
The OpenSSH test server (TestSFTPOpenssh) enables
--sftp-multithread-upload so the feature is exercised in CI against a
real server.
Tested against OpenSSH: a single large upload over a high latency link
went from ~12 MB/s to ~90 MB/s.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Splainte <r.wycke@hotmail.fr>
An audit of the shell escaping alongside the PowerShell fix found the
Windows Command Prompt path only rejected the double quote delimiter. A
remote filename containing % or ! can trigger environment variable expansion
even inside double quotes, and a newline or carriage return ends the
command, so a crafted filename could alter the command run during
server-side hashing. Reject paths containing these characters, matching
the existing treatment of the double quote.
PowerShell treats several Unicode smart-quote characters (U+2018, U+2019,
U+201A, U+201B) as single-quote delimiters in addition to the ASCII
apostrophe. The quoting helper only doubled the ASCII apostrophe, so a
remote filename containing one of these could close the quoted path and
inject statements that ran as the SSH account during server-side hashing.
Double all five delimiters when wrapping a PowerShell path so a filename
is always treated as data.
Fixes GHSA-2m8m-jhrm-w6j2
Add two new options, pin_host_key and host_keys, that
together provide a TOFU host-key validation mode for users who don't
maintain a known_hosts file. When --sftp-pin-host-key is used, rclone
records the server's host key into host_keys on the first successful
connection and verifies it on every subsequent connection.
host_keys is always validated when non-empty, so it can also be used
by hand to pin a known fingerprint without enabling TOFU writing.
known_hosts_file takes precedence if both are set. SSH host
certificates are rejected with a clear message pointing at
known_hosts_file. On-the-fly remotes log a warning since the captured
key cannot be persisted.
The connection pacer in getSftpConnection used to retry every error,
so permanent failures (host key mismatch, certificate rejection, auth
failure, etc.) were looped 10 times before reporting to the user.
Switch to using fserrors.ShouldRetry which matches the pattern other
backends use so only genuinely retriable errors (timeouts, EOF,
network blips) are retried and permanent errors are surfaced
immediately.
Previously ssh.InsecureIgnoreHostKey() was set unconditionally as the
default HostKeyCallback with no indication to the user.
This logs a warning pointing users to the documentation on how to
enable host key validation.
See: https://github.com/rclone/rclone/security/code-scanning/167
This was being done in NewFs instead of NewFsWithConnection like it
should have been which meant calls to NewFsWithConnection were not
initialising the proxy correctly.
Before this fix using --sftp-ssh with the sftp backend could leave
zombie processes.
This patch fixes the problem that sshClientExternal.session was never
assigned, so Wait() always returned nil without waiting for the SSH
process to exit. This caused zombie processes because the process was
never reaped.
It also ensures that Wait() is only called once on each process.
I gave this issue to Copilot to fix as an experiment. It went off in
the wrong direction to start with and fixed something which wasn't the
problem but still needed fixing. With a bit of a nudge it fixed the
correct problem too.
Co-authored-by: Nick Craig-Wood <nick@craig-wood.com>
Before this fix it was possible for an about call in various backends
to exceed an int64 and wrap.
This patch causes it to clip to the max int64 value instead.
All user visible Durations should be fs.Duration rather than time.Duration. Suffix is then optional and defaults to s. Additional suffices d, w, M and y are supported, in addition to ms, s, m and h - which are the only ones supported by time.Duration. Absolute times can also be specified, and will be interpreted as duration relative to now.
This commit modernizes Go usage. This was done with:
go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -fix -test ./...
Then files needed to be `go fmt`ed and a few comments needed to be
restored.
The modernizations include replacing
- if/else conditional assignment by a call to the built-in min or max functions added in go1.21
- sort.Slice(x, func(i, j int) bool) { return s[i] < s[j] } by a call to slices.Sort(s), added in go1.21
- interface{} by the 'any' type added in go1.18
- append([]T(nil), s...) by slices.Clone(s) or slices.Concat(s), added in go1.21
- loop around an m[k]=v map update by a call to one of the Collect, Copy, Clone, or Insert functions from the maps package, added in go1.21
- []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...), added in go1.19
- append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1), added in go1.21
- a 3-clause for i := 0; i < n; i++ {} loop by for i := range n {}, added in go1.22
Before this change, upgrading to v1.13.7 caused a deadlock in the tests.
This was caused by additional locking in the sftp package exposing a
bad choice by the rclone code.
See https://github.com/pkg/sftp/issues/603 and thanks to @puellanivis
for the fix suggestion.
Currently rclone allows us to specify the path to a public ssh
certificate file.
That works great for cases where we can specify key path, like local
envs.
If users are using rclone with [volsync](https://github.com/backube/volsync/tree/main/docs/usage/rclone)
there currently is a limitation that users can specify only the rclone config file.
With this change users can pass the public certificate in the same fashion
as they can with `key_file`.
The SFTP protocol (and the golang sftp package) internally uses uint32 unix
time for expressing mtime. Hence it is a waste of memory to store it as 24-byte
time.Time data structure in long-lived data structures. So despite that the
golang sftp package uses time.Time as external interface, we can re-encode the
value back to the original format and save memory.
Co-authored-by: Tomasz Melcer <tomasz@melcer.pl>
Before this change, rclone always expected --sftp-path-override to be
the absolute SSH path to remote:path/subpath which effectively made it
unusable for wrapped remotes (for example, when used with a crypt
remote, the user would need to provide the full decrypted path.)
After this change, the old behavior remains the default, but dynamic
paths are now also supported, if the user adds '@' as the first
character of --sftp-path-override. Rclone will ignore the '@' and
treat the rest of the string as the path to the SFTP remote's root.
Rclone will then add any relative subpaths automatically (including
unwrapping/decrypting remotes as necessary).
In other words, the path_override config parameter can now be used to
specify the difference between the SSH and SFTP paths. Once specified
in the config, it is no longer necessary to re-specify for each
command.
See: https://forum.rclone.org/t/sftp-path-override-breaks-on-wrapped-remotes/40025
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
Before this change we released the ssh connection back to the pool
before the upload was finished.
This meant that uploads were re-using the same ssh connection which
reduces throughput.
This releases the ssh connection back to the pool only after the
upload has finished, or on error state.
See: https://forum.rclone.org/t/sftp-backend-opens-less-connection-than-expected/40245
This introduces a new fs.Option flag, Sensitive and uses this along
with IsPassword to redact the info in the config file for support
purposes.
It adds this flag into backends where appropriate. It was necessary to
add oauthutil.SharedOptions to some backends as they were missing
them.
Fixes#5209
Implement a Partialuploads feature flag to mark backends for which
uploads are not atomic.
This is set for the following backends
- local
- ftp
- sftp
See #3770
Before this change rclone used a normal SFTP rename if present to
implement Move.
However the normal SFTP rename won't overwrite existing files.
This fixes it to either use the POSIX rename extension
("posix-rename@openssh.com") or to delete the source first before
renaming using the normal SFTP rename.
This isn't normally a problem as rclone always removes any existing
objects first, however to implement non --inplace operations we do
require overwriting an existing file.
Noah ZalevandNick Craig-Wood
Puneet DixitandGitHub
yyandGitHub
Nicolas DessartandNick Craig-Wood
albertonyandGitHub
Dimitar IvanovandGitHub
Russ BubleyandGitHub
nielashandNick Craig-Wood
Dimitri Papadopoulos OrfanosandGitHub
Bjørn SmithandGitHub
Roberto RicciandNick Craig-Wood
nielashandGitHub
ZachandGitHub
Janne HellstenandNick Craig-Wood
JoelandGitHub