* Fix hash_search() duplicate-chain regression (#1115) and collision flood
When a hash chain contains > MAX_CHAIN_LEN identical colliding blocks
(such as 0x00 sparse regions colliding with 0xFF blocks at 64K/128K
block sizes), walking every duplicate entry exhausts MAX_CHAIN_LEN before
reaching valid matching blocks further down the chain (#1115).
Track the strong checksum of the last failed chain entry (last_false_sum2)
and unlink subsequent duplicate entries sharing the same strong checksum
(unless protected by SUMFLG_SAME_OFFSET in --inplace mode) so duplicates
are neither re-walked nor counted repeatedly against MAX_CHAIN_LEN.
Additionally, if 4 consecutive 1-byte rolling offsets experience a
weak-checksum collision where the entire chain fails to match, advance
offset by a full block rather than computing a full strong checksum on
every single byte.
* Split hash_search() collision tests and add boundary/--inplace coverage
Split testsuite/collision_test.py into two dedicated regression suites:
1. testsuite/hashsearch-dup-chain_test.py:
- Verifies forward chains (> MAX_CHAIN_LEN duplicates),
multi-group interleaved duplicate chains, and --inplace
SUMFLG_SAME_OFFSET preservation when mutating the chain after a
subsequent duplicate is overwritten on disk.
2. testsuite/hashsearch-collision-skip_test.py:
- Asserts explicit O(N/B) hash_hits CPU bounds and exact literal/matched
bandwidth bounds across skip boundaries (3, BLOCK, BLOCK+3, BLOCK+4,
2*BLOCK+3, 2*BLOCK+6, and 10*BLOCK+137) in both standard and
--inplace modes, and documents the bunched-perturbation hazard case.
* Unlink redundant SUMFLG_SAME_OFFSET duplicates in hash_search()
In --inplace mode, when > MAX_CHAIN_LEN identical basis blocks match
at their aligned offsets (such as 0x00 sparse runs), every matching
entry acquires SUMFLG_SAME_OFFSET. Skipping deduplication for all
SUMFLG_SAME_OFFSET entries left all pinned duplicates in the chain,
exhausting MAX_CHAIN_LEN, inflating false_alarms, and hiding valid
basis blocks behind them.
Track the flags of the entry that set last_false_sum2 (last_false_flags).
Once a SUMFLG_SAME_OFFSET entry has been retained as last_false_sum2,
unlink subsequent duplicate entries carrying SUMFLG_SAME_OFFSET as well,
while still preventing unpinned entries from unlinking a pinned entry.
Add Case 4 to testsuite/hashsearch-dup-chain_test.py to verify that
> MAX_CHAIN_LEN pinned duplicate blocks are unlinked and trailing valid
blocks behind them are matched with 0 extra literal bytes.
When updating_basis_file is active (--inplace), hash_search() advances
aligned_offset by s->blength and increments the 32-bit signed counter
aligned_i until aligned_offset catches up to offset.
If the source file is larger than 2 GiB and the peer supplies a small
s->blength and s->count, continuing to increment aligned_i past s->count
serves no purpose (as no basis blocks exist beyond s->count - 1) and can
overflow INT32_MAX into a negative index that bypasses the one-sided
aligned_i < s->count check.
Stop the catch-up loop once aligned_i reaches s->count so aligned_i
remains strictly within [0, s->count] and cannot overflow.
Reported-by: Mominul Islam Hemal <mominulislam-2021411216@cs.du.ac.bd>
* log: avoid recursive deletion-message overflow
A quiet daemon receiver queued one MSG_DELETED per path until the generator hit --max-alloc, so skip names the client did not request. If requested output legitimately fills the queue, keep allocator error reporting from re-entering the same buffer and exhausting the stack. Preserve explicit deletion requests across the daemon max-verbosity clamp.
* NEWS: note daemon deletion-message overflow fix
* sender: resolve --files-from entries under a module subdirectory
A --files-from pull whose source is a subdirectory of a daemon module
(rsync://host/mod/subdir/) looked up every listed file at the module
root. A missing file was reported as "file has vanished" and the
transfer ended with code 24; a same-named file at the module root was
sent in its place. With --remove-source-files, the sent files were
then kept with "Skipping sender remove for changed file".
The sender opens each file from the module root with
secure_relative_open(), rebuilding the path as F_PATHNAME + "/" +
f_name, and secure_sender_parent_fd() rebuilds the parent the same way.
A --files-from entry has no F_PATHNAME: like any relative pathname it
is relative to orig_dir, the transfer's source directory, which here is
a module subdirectory. Dropping orig_dir made both look under
mod/<entry> instead of mod/subdir/<entry>.
Build the module-relative path in one place, module_relative_path(),
and prefix a missing or relative F_PATHNAME with orig_dir's
module-relative part, as change_pathname() does when it chdirs.
orig_dir is no longer static in flist.c so sender.c can read it.
Add daemon-files-from-subdir_test.py, which fails without the fix.
* sender: keep the --files-from subdirectory fix minimal and test the module filter
* sender, testsuite: shorten the orig_subdir() comment and the test
* cleanup: don't flush the write buffer from a signal handler
When SIGUSR1 aborts an --inplace/--append transfer, _exit_cleanup()
runs inside the handler and flushes the receiver's write buffer. If
the handler interrupted flush_write_file() after write() returned but
before wf_writeBufCnt was updated, the same bytes are written a second
time, and with --append the file keeps a duplicated 256 KiB region.
Skip that flush when called from a signal handler. A SIGINT, SIGTERM
or SIGHUP that reaches the receiver itself still exits from normal code
through got_kill_signal, so its flush is unaffected.
* testsuite: abort --append/--inplace in the middle of a write
Hold the receiver right after its first write() to the destination,
with every signal pending, send it SIGUSR1 and check that the
destination is still a prefix of the source. This fails without the
previous commit. The hold is an LD_PRELOAD shim, so the test is
Linux-only and is expected to skip on macOS and Cygwin.
* Add --delay-symlinks to create symlinks after all files are in place
The generator creates a symlink as soon as it reaches it in the file
list, while regular files arrive later through the sender and receiver
(and, with --delay-updates, are only renamed into place at the end).
For the rest of the transfer a new symlink can point at a file that
isn't there yet, so anything reading the destination during the sync --
e.g. a loader resolving libfoo.so -> libfoo.so.2 -- can fail.
The new --delay-symlinks option makes the generator defer new and
changed symlinks and create them once the receiver reports that every
file, including any --delay-updates renames, is in place. Each link is
itemized and logged after it has been created, before the generator
ends the delay-updates phase, and the deletion stats are sent after any
directory that a deferred link replaces.
The option implies --no-inc-recursive on the receiving side and is
passed to a remote receiver, which must also support it. It is an
error to combine it with --remove-source-files, --hard-links,
--compare-dest, --copy-dest, or --link-dest, or to use it with a
protocol older than 29. support/rrsync accepts the option and refuses
it under -no-overwrite, as it does --delay-updates.
Add delay-symlinks_test.py, which holds a push partway through the
link's target to check that new and changed links aren't created early,
and rrsync-no-overwrite-delay-symlinks_test.py.
* Document --delay-symlinks limits and test how it is passed to the server
The man page now says the option narrows the inconsistency window rather
than making the update atomic, that an interrupted transfer leaves new
symlinks missing and changed ones at their old targets, and that
--delete-before can also remove an old target early.
Add delay-symlinks-remote_test.py. Through a remote shell that records
the server command, it checks that the option is passed to a remote
receiver on a push but not to a remote sender on a pull, and that
incremental recursion is off in both directions. It also checks that
--protocol=28 fails with the protocol error, and that a push to an older
rsync (old_versions/rsync_3.4.1) is refused before anything is
transferred while a pull from it works.
* testsuite: seed delay-symlinks fixture with rsync
* NEWS: document --delay-symlinks
---------
Co-authored-by: Zen Dodd <mail@steadytao.com>
* human-readable: group file-list and directory counts
Adds the thousand separators as file-list is computed. Useful on large
dirtrees:
Behavior before: 2523297101 files to consider
After: 2,523,297,101 files to consider (2.523.297.101 with a comma decimal point)
Also covers the file-list counter, "created N directories" and --max-delete.
* NEWS: record human-readable output changes
* human-readable: do not add trailing space in output
Co-authored-by: Zen Dodd <mail@steadytao.com>
* human-readable: use dynamic precision length
Let's lower precision for huge numbers.
The output used to be:
3.45M -> 46.73M -> 523.11M -> 1.24G -> ...
With this change the code always gives the three most significant digits:
3.45M -> 46.7M -> 523M -> 1.24G -> ...
Co-authored-by: Zen Dodd <mail@steadytao.com>
* human-readable: also handle num < mult with the same code
... just make sure no precision is added.
Co-authored-by: Zen Dodd <mail@steadytao.com>
* human-readable: add an "i" to unit to indicate binary (1024) base
Co-authored-by: Zen Dodd <mail@steadytao.com>
* human-readable: also use it to format rate in progress
Let's also simplify the code for rate in progress, and benefit from
same functionality.
Co-authored-by: Zen Dodd <mail@steadytao.com>
* human-readable: append suffix "B" for byte to ofs...
... and also fix the formatting in man page.
Co-authored-by: Zen Dodd <mail@steadytao.com>
* human-readable: unbreak for do_big_dnum() with fract
Co-authored-by: Christian Hesse <mail@eworm.de>
* testsuite: cover human-readable output
---------
Co-authored-by: Zen Dodd <mail@steadytao.com>
* Add fchmodat() as an option to set symlink Unix access permissions
syscall.c:
Add fchmodat(), if it exists on the system, as a last-resort attempt to change
the Unix access permissions for a symlink (on those systems that support it).
This is done just prior to finally abandoning such an attempt and after any
other available options for this, such as lchmod() or setattrlist().
This option is required to preserve symlink access permissions when making
copies using rsync on OpenBSD. The patch itself is not OS-specific.
testsuite:
Add a test, symlink-unix-perms_test.py, to testsuite to check on the success
of setting symlink access changes using rsync. The test is skipped on those
systems that do not allow symlink access permission changes. The test checks
at runtime whether or not the system permits symlink access changes. As with
the patch, this test is not OS-specific.
* testsuite: Modify the symlink-unix-perms test to conform to proper standards.
1) Set the proper python shebang
2) Remove the use of random test access values in favour of fixed values.
3) Skip the test only on "not implemented" or "not supported" exceptions. All
other unexpected errors will fail the test.
4) Test using symlinks to a specifically created directory or file as opposed
to the '.' directory. The latter could potentially lead to cyclical paths.
Some code tweaking has also been done for readability.
* symlink-unix-perms_test.py: Perform some useful cleanup.
1) Remove unnecessary import of "random".
2) Consolidate some unnecessarily verbose and inefficient code.
* testsuite: expect unsupported symlink permission skips
rsh_cmd() returns the lsh.sh path quoted for rsync's own --rsh tokenizer,
so a build directory containing a character shlex.quote() will not pass
bare -- the '~' in a Debian snapshot version such as
rsync-3.5.1~git20260921 -- comes back wrapped in quotes. The ssh-basic
probe passed that string straight to subprocess.run() as argv[0], where
no shell strips the quotes, and failed with FileNotFoundError. Split it
with shlex first.
Found via the rsync-latest PPA, where every snapshot build fails
`make check` on exactly this test while the release builds pass.
* Add 4 KiB logical block footprint tracking to stats
This adds a 'Number of 4 KiB logical blocks touched' metric to the --stats output to decouple network delta payload from actual local file modifications. Previously, a small amount of literal data scattered across a file (especially with --inplace) could result in a massive number of local file write operations with no visibility, and large sparse files masked their true write opreations (ignoring punch holes).
Technical details:
- Implemented a stateful block tracker in the receiver that calculates touched 4K boundaries using file offsets and lengths, including strict lseek awareness to accurately skip sparse file holes.
- Enforced strict per-file lifetime state with a reset hook inside receive_data(), successfully mitigating POSIX file descriptor (FD) recycling state leaks.
- Created MSG_BLOCK_STATS multiplex message to tunnel the block footprint safely out of the isolated receiver process and relay it over the network.
- Bumped PROTOCOL_VERSION to 33 and SUBPROTOCOL_VERSION to 8392 for safe PR testing.
- Added test suite covering contiguous, scattered, zero-byte, sparse file (hole-skipping), multi-file (FD reuse), batch mode, and maximum-I/O boundary conditions.
* io: fix block stats integration
---------
Co-authored-by: Zen Dodd <mail@steadytao.com>
* testsuite: fix fleettest example json for IDN
match CI for IDN tests
* testsuite: fix fleettest example skip extras and pin Cygwin python
#1069 replaced daemon-max-alloc-zero with max-alloc-zero, which no
longer needs the old static client, but the per-box expect_skip_extra
entries still named the old test, so runtests refused the skip list
("no such test") on every Linux target. Drop those entries.
Pin python3.9 on the cygwin target to match the workflow, which installs
only python39: the box's default python3 (3.12) has a working AF_UNIX
socketpair, so daemon-stdin-local-socket ran where cygwin.txt expects a
skip. Note that python39 must be 3.9.25+ for os.getxattr, which the
xattr tests need to run as they do in CI.
* testsuite: run bash without POSIXLY_CORRECT; tolerate Cygwin symlink copystat
runtests.py exports POSIXLY_CORRECT=1 to every test, and bash before 5.1
disables process substitution in POSIX mode, so pseudo-paths,
pseudo-paths-daemon and read-batch-pipe silently skipped on bash 4.4/5.0
hosts (Ubuntu 18.04/20.04, AlmaLinux 8). Strip the variable from bash's
environment: the tests require bash precisely for that syntax.
That was also why they skipped in the AlmaLinux 8 CI container, so its
skip list (which held only those three) goes away along with the
workflow's reference to it. The macOS list keeps them: there they skip
as Linux-only.
malicious-sender-delete-scope copies the source tree with symlinks=True.
Cygwin's python 3.9.25 gained os.listxattr, and copystat() on a fresh
symlink then fails with EACCES, although the link itself was created.
Ignore copytree errors that are only about symlink pairs; anything else
still raises.
* testsuite: update max-alloc entry
* Tighten alt-dest path resolution and partial-dir state validation
This introduces architectural best-practices to harden the receiver's state machine
and harmonize symlink handling across the delta-basis engine, addressing protocol
edge-cases reported by Fyyre (James).
- receiver.c (recv_files): Added explicit validation to ensure one_inplace is
only triggered when partial_dir is configured and the FNAMECMP_PARTIAL_DIR
token is legitimate.
- receiver.c (secure_basis_open): Enforced O_NOFOLLOW on leaf components when
resolving operator-supplied paths, aligning it on operator-path behavior.
Co-authored-by: Fyyre <fyyre@fyyre.net>
* receiver: validate partial-dir basis state
* fix: reject alt-dest leaf symlinks
* fix: preserve basis open flags
---------
Co-authored-by: Zen Dodd <mail@steadytao.com>
* Fix --files-from confinement for local and SSH transfers
The recent path confinement patch caused an ELOOP error when local or SSH users tried to use a --files-from list located outside the confined root.
This happened because the code treated the argument as if its always an operator-path peer. CLI arguments provided locally or over SSH are trusted, so the strict boundary check should only apply to untrusted clients connecting to a background daemon.
* tests: assert local files-from copy
---------
Co-authored-by: Zen Dodd <mail@steadytao.com>
Using --log-file or --files-from with /dev/stdin, /dev/stdout, or /dev/stderr
attached to a pipe previously failed in two ways:
1. On standard hosts, it failed with ENOENT because the symlink target
(pipe:[N]) was treated as a relative file path rather than a secure
kernel pseudo-path.
2. Inside user namespaces (e.g., rootless podman, unshare), it aborted
with ELOOP ("refusing to follow a symlink owned by an untrusted user").
The symlink ownership reported the kernel overflow UID (65534), which
completely blocked the walker from following the pseudo-paths.
This patch resolves the issues by refining the symlink path walker and
trust mechanisms:
* Updated fd_pin_tail() to natively recognize /dev/stdin, /dev/stdout,
and /dev/stderr, parsing them directly to their corresponding /0, /1,
and /2 descriptor tails.
* Introduced the is_anchored variable to strictly enforce absolute paths,
ensuring malformed paths cannot bypass the check using relative forms
like dev/fd/ or proc/self/.
* Removed pin_transit from the namespace_pin check and replaced it by
adding the is_anchored variable to the check. Checking for root confinement
or daemon status is unnecessary when we are already validating and restricting
traversal to known safe paths (Daemon also refuses any symlinks pointing to /).
* Added new cases to the test suite to validate standard stream pseudo-path
handling and ensure namespace overflow UID bypasses work correctly
without regression.
* Fix ona_open to safely resolve bash process substitution pseudo-paths
Bash process substitution (e.g., `<(...)` or `>(...)`) exposes file
descriptors as symlinks under `/proc/self/fd/X` pointing to kernel
pseudo-paths such as `pipe:[12345]`. Previously, `ona_open()` would read
this target and attempt to resolve it as a literal file path on disk,
causing the operation to fail with `ENOENT` and breaking legitimate local
process substitution.
This patch safely intercepts and resolves these pseudo-paths while
maintaining strict confinement boundaries and averting TOCTOU risks:
- Detects kernel pseudo-paths (`pipe:[`, `socket:[`, `anon_inode:`)
only when `fd_pin_tail` confirms the path resolves precisely to a
direct child of a valid FD directory.
- Categorically rejects pseudo-path resolution if `confine_root` is
active (yielding `ENOENT`).
- Strips `O_NOFOLLOW` for legitimate leaf pseudo-paths, allowing
`openat()` to correctly delegate resolution.
- Reverts `fd_pin_tail` to its upstream signature, as manual PID
validation is no longer required due to the secure `openat()` design.
* testsuite: expect pseudo-path skip on Alma
* syscall: reject trailing pseudo-path components
---------
Co-authored-by: Zen Dodd <mail@steadytao.com>
* main: allow --contimeout for daemon connections over --rsh
The --contimeout guard rejected the option whenever connect_timeout was
set and the connection was not being made by a socket, so a daemon
reached through a remote shell (daemon_connection == 1) was refused the
same way as a plain non-daemon remote-shell transfer. That is how
rsync-ssl talks to a daemon: it runs rsync with --rsh pointing at its
helper, so "rsync-ssl --contimeout=60 host::mod" died with "The
--contimeout option may only be used when connecting to an rsync
daemon" before it could even connect.
Only reject --contimeout when there is no daemon connection at all, so
the option is accepted for a daemon-via-rsh connection (and still works
unchanged for the socket daemon path, which returns earlier through
start_socket_client).
Add a regression test that drives a daemon-via-rsh invocation and
asserts the option is not rejected, while a plain remote-shell
destination is still refused.
* main, io: honor --contimeout for daemon-via-rsh connections
The previous commit made --contimeout legal for a daemon reached
through a remote shell, but rsync still did not act on it: the socket
path times its connect() via an alarm, while the daemon-via-rsh path
spawned the helper and then blocked in start_inband_exchange() waiting
for the daemon greeting with no connect timeout at all. A hung helper
-- e.g. rsync-ssl's openssl connect to an unreachable host -- would hang
the transfer forever.
Give the client a second wall-clock deadline, client_connect_deadline,
set from --contimeout around do_cmd() + start_inband_exchange() and
cleared once the greeting exchange finishes. handshake_poll_timeout_ms()
now also checks it and exits with RERR_CONTIMEOUT ("timeout waiting for
daemon connection"), matching the socket path's exit code, instead of the
daemon-handshake deadline's RERR_TIMEOUT.
This covers the helper spawn, its connect/TLS handshake, and the greeting
exchange as one connection-establishment phase, so it works uniformly for
openssl, stunnel, and gnutls rsync-ssl backends.
Extend the contimeout-rsh test to drive a helper that never connects and
assert rsync aborts with exit 35 within the timeout, and that a working
daemon-via-rsh connection still completes.
* options: accept --max-alloc=0 again, resolved to the parser's own ceiling
3.5.0 rejected --max-alloc=0 (CVE-2026-53794). The stated rationale was that a
zero cap "disabled the per-allocation size cap (the defense behind
CVE-2024-12084)". That was true up to 3.2.7, where the check short-circuited:
if (max_alloc && num >= max_alloc/size)
but 2f9b963a ("Make `--max-alloc=0` safer", 3.3.0) removed the short-circuit and
mapped 0 to SIZE_MAX at parse time, leaving the guard unconditional. Since the
guard admits an allocation only when num < max_alloc/size, num*size stays below
max_alloc at every setting, so the num*size overflow check was armed for 0 just
as for any other value. From 3.3.0 onward, 0 raised the magnitude ceiling and
nothing else -- and an explicit 8191P raises it exactly as far, is accepted, and
is forwarded to the peer, so rejecting 0 removed no capability.
What it did remove is the only portable spelling. The parser's ceiling is
SIZE_MAX/2, so it tracks the build's word size: on ILP32 the suffix multiplier
alone exceeds the bound, making every P and T value an error whatever the digits
and capping the option at 2047M. Because max_alloc_arg goes on the wire
un-normalized, a 0 was re-resolved by each side against its own SIZE_MAX; any
literal is resolved once on the client and shipped verbatim, so nothing above
2047M survives a 64-bit client talking to a 32-bit daemon. There is no number a
user can compute that does what 0 did, which is what #1056 ran into.
So accept 0 again, but resolve it to SIZE_ARG_MAX (SIZE_MAX/2) rather than
SIZE_MAX, so it lands exactly on the largest value that could also be typed and
is no longer a limit only the 0 spelling can reach. Keep forwarding it
verbatim: that per-side resolution is the property worth having. This leaves
the substantive half of the 3.5.0 hardening -- bounding parse_size_arg() against
the unbounded `size *= atof(size_arg)` -- untouched.
The residual concern, a 0 forwarded to a <= 3.2.7 daemon that honours it, is not
something a client-side check can address: the client is the attacker's own
code, as daemon-max-alloc-zero_test.py noted in its own docstring. Operators
who want peers kept off their cap have `refuse options = max-alloc`.
Also drops the now-unreachable rejection message, which left the min-value error
recommending a value the parser refused ("min: 1.00M or 0 for unlimited").
Tests: max-alloc-zero replaces max-alloc-zero-rejected and
daemon-max-alloc-zero, whose assertions are the behaviour being reverted. It
checks that 0 is accepted, that it reaches the peer as the literal "0" rather
than a resolved number (verified against a negative control that normalizes it),
and that the parser's upper bound still rejects an out-of-range value.
Fixes#1056
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* testsuite: refresh Cygwin expectations
* docs: note max-alloc safety
Use O_PATH on Linux for directory descriptors used only for path
traversal, fchdir(), or as *at() parents. Reopen final directory
endpoints with the caller-requested access mode and retain the existing
O_RDONLY fallback elsewhere.
Add coverage for exact sources and readable descendants beneath
search-only directories, known-file creation beneath write/search-only
destinations, and retained refusal to enumerate unreadable directories.
Fixes#1064.
* rsync-ssl, testsuite: accept --type=SSL_TYPE anywhere in the args
The --type=... option was only recognized as the first argument, so a
command such as "rsync-ssl --dry-run --type=stunnel host::mod" passed
the option through to the underlying rsync, which rejected it as
unknown. Scan the full argument list for --type=..., export
RSYNC_SSL_TYPE, and drop the option before handing the remaining args
to rsync. The manpage no longer says the option must be first.
Adds a test that runs rsync-ssl with a fake rsync in PATH and checks
that --type= is consumed in first, middle, and last positions.
* fix(rsync-ssl): stop interpreting wrapper options at --
rsync-ssl consumed a --type=... operand that appeared after a --
argument, even though -- explicitly protects the rest of the command
line from option parsing. Stop scanning for --type=... at a --
argument: preserve the -- and every subsequent argument verbatim,
passing them through to rsync unchanged. Add regression coverage for
this case and document the behavior in the manpage.
* test(rsync-ssl): assert RSYNC_SSL_TYPE and exit status in the type-option test
The rsync-ssl-type-option test only checked that --type= tokens were
removed from the rsync argv, so it would also pass if the wrapper
silently discarded the requested SSL implementation instead of
exporting RSYNC_SSL_TYPE. The fake rsync now records both the argv it
receives and the RSYNC_SSL_TYPE value it observes; each invocation
asserts the expected value (or UNSET when the wrapper must not consume
anything), and run() fails the test unless rsync-ssl exits
successfully.
* Add IDN support
rsync can now connect to IDN (internationalized domain name) hosts, and
IDN names are recognized in a daemon's hosts allow/deny.
* idn: convert host names label by label
The IDNA mapping folds some non-ASCII characters onto ASCII ones, so
running a whole hosts allow/deny token through idn2_to_ascii_8z() could
hand back a pattern the admin never wrote: a "＊" (U+FF0A FULLWIDTH
ASTERISK) entry came back as "*" and let every host in.
Convert label by label instead, keeping an ASCII label byte for byte and
using a converted label only when it comes back as a bare A-label. An
ASCII-only config now behaves as it did before there was IDN support, and
a token that cannot be converted is left alone and so matches nothing. The
client side shares the same helper, and neither side truncates a name at
its 1024-byte buffer any more. strlower() folds only ASCII now, since its
one caller is the hosts allow/deny list, which can hold UTF-8.
Adds testsuite/daemon-access-idn and extends testsuite/idn to cover
Unicode, punycode, mixed-case and invalid input on both sides.
* idn: hand libidn2 the same flags on both sides
The client path called idn2_lookup_ul() without IDN2_NFC_INPUT while the
daemon path passed it to idn2_to_ascii_8z(). Both normalize either way --
idn2_lookup_ul() ors the flag in itself, and TR46 normalizes as it maps --
but there is no reason for the two calls to read differently, so pass one set
of flags from one place. The flag asks libidn2 to normalize the label rather
than promising that it already is: it gates the u32_normalize() call, and
without it a decomposed label comes back IDN2_NOT_NFC.
Adds composed/decomposed cases to testsuite/idn, which sees the exact host
name rsync hands out, and a decomposed hosts allow token to
testsuite/daemon-access-idn.
Which symbol names this test's LD_PRELOAD hook exports is decided by the
compiler that builds it, and which names rsync imports is decided by
configure -- and the two do not have to agree.
Where off_t is not already 64 bits, configure's AC_SYS_LARGEFILE adds
-D_FILE_OFFSET_BITS=64 (i386 and alpha in Debian), so glibc redirects every
open()/openat()/fstatat() call in rsync to open64(), openat64() and
fstatat64(). The hook is compiled by a bare "cc", so on those architectures
it defines only the unsuffixed names: the receiver's opens never reach it, no
EACCES is injected, the marker the positive control looks for is never
written, and the test fails with
positive control failed: receiver did not open the existing partial file
with O_CREAT (rc=0, output='')
It happens to work on Debian's 64-bit time_t ports (armhf, hppa, powerpc,
...) only by luck: their gcc predefines -D_FILE_OFFSET_BITS=64 -D_TIME_BITS=64,
so glibc's __REDIRECT renames the hook's own DEFINITIONS as well and it ends
up exporting exactly the *64 names rsync imports.
Define both spellings explicitly so the hook interposes whichever set the
rsync under test was linked against, and #undef the two macros at the top of
the hook so that renaming cannot happen -- otherwise, on precisely those ports
whose compiler predefines them, open() would be emitted as open64() and
collide with the explicit wrapper ("symbol `open64' is already defined"),
leaving the hook unbuildable and the test skipped.
The new pointers are resolved through hook_resolve(), so the existing
nested-dlsym recursion guard covers them as well.
Failing build logs:
i386 https://buildd.debian.org/status/fetch.php?pkg=rsync&arch=i386&ver=3.5.0%2Bds1-1&stamp=1786938290&raw=0
alpha https://buildd.debian.org/status/fetch.php?pkg=rsync&arch=alpha&ver=3.5.0%2Bds1-1&stamp=1786973432&raw=0
The last section of preallocate_test.py writes 256 blocks of
[4 KiB data][24 KiB zeros][4 KiB data] and requires --inplace --sparse to
deallocate at least half of the file. A punch only frees whole allocation
units, and that interior run sits 4 KiB into each 32 KiB block: with a 4 KiB
unit it covers six whole units (24 KiB freed per block), but with a 16 KiB
unit it covers none at all -- [0,16K) holds live data at [0,4K) and [16K,32K)
holds live data at [28K,32K) -- so st_blocks cannot move whatever rsync does.
That fails the build on Debian loong64, whose kernel uses 16 KiB pages:
--inplace --sparse left matching interior zero runs allocated: 8388608 of
8388608 bytes remain allocated after a 8388608-byte matched-block update
i.e. exactly "nothing was freed". fs_can_punch_holes() does not catch it
because it punches a whole 64 KiB file, which frees blocks at any granularity.
Generalise that helper into punch_frees(offset, length, size) so each
assertion can probe the shape it actually depends on, and gate the interior
one on the exact layout written just above it. Where the filesystem can free
that run the assertion runs exactly as before; where it cannot, the test says
so rather than reporting a regression. The content check is unconditional
either way.
While here, call fallocate64() instead of fallocate(). ctypes declares the
offset and length as c_longlong, but glibc's fallocate() takes 32-bit off_t
where off_t is 32 bits, so the callee reads the high half of `offset` as its
`length`, the call fails with EINVAL, and can_punch comes back False -- which
is why none of this file's hole-punching assertions have ever run on a 32-bit
port. On i386:
fs_can_punch_holes() as written : before=128 ret=-1 errno=22 -> False
the same probe via fallocate64 : before=128 ret=0 -> True
fallocate64() takes off64_t on every architecture and is a plain alias of
fallocate() where off_t is already 64 bits. The probe also writes urandom
rather than a repeated byte, so a filesystem that compresses does not answer
"nothing was freed" merely because nothing was allocated.
Also note that backport CI, once it exists, has to consume these lists the
same way fleettest does -- read backport.txt from the branch being built and
pass it as RSYNC_EXCLUDE -- or it will fail on every entry and get switched
off.
A test named in a backport's backport.txt never runs, so it cannot skip either.
If the suite's expected-skip list also names it -- the two --compress-threads
tests are declared as expected skips because they need --use-tcp -- the oracle
waits for a skip that can no longer happen and every pipe cell reports a skip
mismatch.
Emit a '-name' removal for those, as the per-target expect_skip_omit already
does. Only for names the spec actually contains: runtests rejects a '-name'
that removes a name nothing added, and most of a backport's exclusions (a test
for a feature it lacks) are not expected skips at all. Deciding that needs the
@FILE references expanded locally, which is what _expand_spec_names does.
v3.4.1 with this: 5/5 cells OK on ubuntu-2404, from 3 OK / 36 not OK across the
fleet before the mechanism existed.
Running the 3.5.0 suite against an older branch (--repo BACKPORT
--testsuite-repo .) reports a wall of failures that are not regressions: tests
for fixes the branch does not carry, and tests whose unit-test helpers its
Makefile cannot build. Both backport branches came back 3 OK / 36 not OK with
every distinct failure explained that way, which makes the run useless as an
oracle -- a real regression would not stand out.
A backport now declares those in its own testsuite/skiplist/backport.txt, read
from the tree being BUILT rather than the one providing the suite: only the
built tree knows what it lacks. The names go to runtests.py as RSYNC_EXCLUDE
rather than as an expected-skip declaration, because some of them fail rather
than skip and an expected-skip list cannot describe a failure.
The overlay that puts a newer testsuite/ onto an older tree is a merge with no
delete, so a file that exists only on the backport survives it. skiplist-spec
exempts the name from its every-list-must-be-referenced rule, since nothing
references this one by design.
Under inc_recurse the first flist (ndx_start == 1) has no parent entry of its
own, so recv_file_list() trusts the peer's "." entry to be the transfer root and
leaves parent_ndx at the flist_new() default of 0 -- dir_flist->files[0]. Only
S_ISDIR entries are appended to dir_flist, so a peer that sends "." with a
NON-directory mode keeps dir_flist->used at 0 while the basename strcmp still
passes: parent_ndx stays 0 and the consumers index a never-written slot.
Drives a real daemon with the pure-Python sender: an inc_recurse push whose only
flist is a regular file "." plus a regular file "a" (no directory anywhere, and
"." sorts lowest; "a" keeps file_total != 1 so the receiver doesn't divert into
recv_additional_file_list). The file list alone is what does it -- the
generator crashes in generate_files() before any transfer phase -- reproduced on
released 3.2.7, 3.4.0 and 3.4.1.
The oracle needs both halves: a positive control that the daemon logged
"receiving file list", and the condition-specific refusal. Accepting any
"rsync error:" line is not enough -- with "." sent as a valid directory and a
bogus file index, that form passes on "File-list index 1000000 not in 0 - 2"
without the crafted transfer root ever reaching the parser.
A fixed daemon has already refused the list and exited by the time the ndx-0
token is sent, so that send and the drain can hit a closed socket; Linux and
FreeBSD swallow it, Solaris, the other BSDs, macOS x86 and Cygwin raise
EPIPE/ECONNRESET. Treat it as an expected outcome, not a result.
The header records what this does not prove: it gates the attack shape rather
than the parent_ndx clause (only the parse-time transfer-root check fires on a
current build), it does not exercise the receiver-side consumer, and the
dereferenced slot is not guaranteed NULL since dir_flist->files[] comes from
realloc(), not calloc().
The daemon sets its deadline with time(NULL) (set_daemon_handshake_timeout,
io.c), and this test measured the elapsed time with CLOCK_MONOTONIC. Those
agree on a quiet machine and diverge on a stalled one: a virtualised guest
resyncs its wall clock after the host deschedules it, while monotonic keeps
its own count. The daemon then closes exactly when it meant to and the test
reports it closed early.
That is what a NetBSD CI run showed -- "closed after 39.55s, before the
expected timeout window (58.75s)" -- and it is the same shape as the macOS
failure that turned out to be the machine sleeping mid-test.
Measure the bound on the clock the daemon decides with. Monotonic still
drives the poll budget, where the job is only "do not hang forever".
On its own that would trade a false failure for a false pass, which is worse:
a refusal or a crash arriving just as the guest's wall clock caught up would
read as a clean timeout, and no diagnostic would fire because the test would
be green. So when the two clocks disagree -- wall says on time, monotonic
says early -- neither settles it, and the daemon has to have recorded its own
deadline firing. The offset of its log is taken before each observation, so a
timeout it logged earlier cannot vouch for this one.
Failures now carry both clocks and that window of the daemon's log, bounded
and with control bytes escaped. The clock note states the discrepancy without
concluding from it: a stalled host produces it, but so does an NTP step, and
either can accompany a real failure.
Nothing in CI or the fleet has ever set --enable-roll-simd, --enable-roll-asm
or --enable-md5-asm, which is why the over-read above sat behind a "fixed"
label for two months, and why the fix applied for it went to the wrong
assembly file.
mac-x86-asm is the same host and OS as mac-x86 with all three on. Mach-O is
the interesting part -- both problems reported against these flags were
macOS-x86-64 -- and it is the only machine in the fleet that can build the
x86-64 assembly at all.
It needs MacPorts clang 19 through CC/CXX, because Apple clang 10 (the ceiling
on macOS 10.13) rejects configure's target("default") multiversioning probe.
mac-x86 keeps the stock Apple compiler, which is what caught #161, so the two
cover different ground rather than one replacing the other.
simd-checksum is a macOS-wide expected skip, since simdtest is only built when
SIMD is enabled; this target subtracts it, because running it is the point.
Also corrects mac-x86's comment, which claimed the probe "cannot compile here
with any clang". It is a compiler-version limit: clang 19 on that same box
compiles, links and runs it.
An LD_PRELOAD hook refuses linkat() for a symlink source only, so the arm is
reachable on a filesystem that hard-links symlinks perfectly well. Three
controls keep it from proving less than it looks:
- the regular file in the same transfer must still be hard-linked, or "it fell
back" would also be satisfied by --link-dest having been abandoned;
- the itemised run must emit exactly one "cL... sym -> some-target" line. A
plain -a run cannot see a duplicated itemisation, which is how that defect
reached an HFS+ target before this was added;
- EPERM and ENOSYS must fall back too, since errno does not separate "cannot"
from "may not".
itemize picked its expected change-type letter from the build capability, which
is the wrong question -- the link happens on whichever filesystem holds the test
data. Ask that one too, and drop the XFAIL the old mismatch needed.
The hook is Linux-only, so the test joins the macOS and Cygwin skip lists,
which are required to be sorted.
Uses the exclude-only merge form, which leaves no diagnostic to assert on: the
escape shows up as a file silently missing from the transfer, so the test reads
the oracle the same way an attacker would. Pull mode, so no --delete is
involved. Each case requires the transfer to have succeeded as well, since
refusing outright would hand the peer a denial of service.
A second escape reaches the source through a symlink, which is what makes
rsync's tracked cwd and the real one disagree -- the shape that catches a
lexical seed. That one drives --confine-root directly: rrsync rejects the
argument spellings that would carry it, so routing it through the wrapper would
pass either way and prove nothing.
Both controls repeat their escape with an in-tree merge target and require it
to be read AND obeyed, since "the transfer failed" and "every merge file is
refused" would otherwise satisfy the escape assertions on their own.
The exclude-self rule that a ":e" merge synthesizes is built by hand with
new0(), so it inherited no flags. While the merge file was still being
parsed the global parse state masked that, but once parsing finished the
stored rule looked argument-origin, and report_filter_result() printed its
pattern -- a merge file's own text -- verbatim:
[sender] hiding file PAT-x9 because of pattern PAT-[x]9 [per-dir ...]
Plain -vv reaches this on a stock client; no --debug is involved. That is
the fifth site of this shape, and the first to get there by constructing a
rule rather than by printing one, so the redaction helper could not catch it.
Also fix the location a per-directory merge reports. Its fname points into
dirbuf, which is cut back to the directory before the name was saved, so the
error said "<rule from .../src/ line 1>" instead of naming .rsync-filter --
no leak, but it breaks the "redact what, keep where" bargain the rest of this
work depends on. Save the name before the truncation.
The rrsync test's claim to close "the rest of the FILTER trace family" was
too strong and is corrected: options.c maps verbosity onto the debug flags,
so -vvv still raises a restricted server to FILTER2 and its trace metadata
comes back. Rule text stays redacted at every verbosity, which is the
property that matters; -vvv is added to the unaffected-transfer cases.
The --debug=FILTER traces print rule text and merge-file names that came
out of a file's contents -- and a word-split per-dir merge (":w- FILE")
turns every word of a file into a merge-file name, so the trace echoes
what the syntax errors no longer do, with nothing failing to parse.
Redacting every trace would mean carrying provenance on each rule, which
a deferred ":" merge does not currently keep. For a restricted account
the cheaper answer is to deny the peer the switch: server_options() only
ever forwards --info, so no stock client sends --debug to a server and
the only way it arrives is a deliberate -M--debug=. An operator
debugging their own server is unaffected.
Disabled rather than deleted from the table, because that table is
generated by the cull-options script and a regeneration would put the
line back; the test would then catch it.
A filter rule that fails to parse was printed back verbatim. When the
rule came from a file rather than an argument, that text is file
CONTENT, and the peer picks which file gets merged: a per-directory
merge rule travels over the protocol, so no argument of ours ever names
it and nothing a wrapper can see mentions it either. Any line that is
not valid filter syntax therefore came straight back to the peer -- a
read-any-line oracle over an rrsync restricted account or a daemon
module, neither of which confines the merge open.
The syntax errors turned out to be the smaller half. The MATCH trace
names the pattern that acted, and report_filter_result() logs at level 1
for a sender or generator, so plain -vv -- no --debug, nothing a stock
client cannot send -- returns a server-side merge file's rules:
[generator] protecting file X because of pattern <the file's text>
So provenance is carried on the rule itself (FILTRULE_FROM_FILE), not
just in the parser: a deferred ":" merge is processed long after the
file that named it was read, and its own name is file content too.
TEXT_FROM_FILE() consults the parse-time context and the rule, so both
the immediate and the deferred paths redact.
Rather than test the provenance at each message -- which is how the last
few of these were found, one at a time, after the ones before them were
fixed -- every string that is or is built from a rule's own text goes
through rule_text(). It returns the text for an argument-supplied rule
and a description of where it came from otherwise, so a message added
later cannot reintroduce the leak by forgetting to check, and there is
one place to audit. rule_detail() does the same for the extra detail a
message adds ABOUT the text: a character of it, an offset into it, the
[not found] bit.
Thirteen sites now route through them: the syntax errors; the modifier
character (one byte of the file, a slower oracle but still one); the
failed-open and merge-depth messages, whose pathname is file content
whenever a rule named it -- and errno with them, since it answers "does
this path exist"; both over-long messages, the deferred one of which
needed no verbosity at all; both merge-name overflows; the long-named
directory error; the [not found] openability bit; the match trace; the
add_rule, parse_filter_file and daemon-hidden traces; and the per-dir
mergelist label, which had the name baked in.
rule_detail() covers more than it first looks: the trailing-whitespace
CAUTION is computed from the rule's last byte, and "hidden by daemon
filter" distinguishes a daemon-filter rejection from an ordinary open,
so both would answer questions about text the peer cannot see.
The regression proves the chokepoint rather than the sites: making
rule_text() return its input unconditionally fails the test. It also
pins what must NOT change for the user's own rules -- the whitespace
warning still fires, and an over-long argument rule is still reported at
full length (the helper buffers at BIGPATHBUFLEN, as rprintf does, so
redaction does not quietly truncate what the user typed).
Bounded and left alone: the numeric rflags in the FILTER2 trace and the
in/exclude wording still describe a file-derived rule without quoting
it, and the daemon's own FLOG line records the name it filtered -- that
one goes to the operator's log, not the peer.
Rules given AS arguments are still echoed in full -- that text is the
user's own, and hiding it would only make ordinary typos harder to fix.
Where a rule did come from a file, the diagnostic names the file and
line instead, which is more useful anyway.
Two things the location itself needed: fname can point into
parse_merge_name()'s static buffer, which a merge rule inside the same
file overwrites while we are still reading it, so a rule after a nested
merge was blamed on the nested file -- keep our own copy. And a CRLF
pair was counted as two line endings while word-split mode counted
tokens rather than lines, so the number pointed at nothing; consume the
LF of a CRLF (preserving the byte for the next rule if pushback ever
fails), and report word-split sources without a line number.
Not covered, deliberately: a rule's provenance is not serialized by
send_filter_list(), so it does not survive to the far side. That is
right -- only the client sends that list, and the server already knows
the patterns the peer gave it.
A daemon test could leave an orphaned rsyncd squatting its port even when
every test PASSED, so nothing in the results pointed at it. On Cygwin the
orphan then wedged the whole fleet: it kept the ssh session from closing,
so fleettest's run_on() blocked until its 2400s timeout and unrelated
tests failed with 300s timeouts as collateral. One such wedge cost a fleet
run 21 minutes.
Cause: rsyncd forks a child per connection, but _stop_rsyncd only killed
the parent -- the one pid the Popen handle knows. A child still winding up
or down when the test ended survived, inherited the listening socket, and
was reparented to init. Cygwin turned that from untidy into unrecoverable:
its signals are cooperative, delivered by a helper thread inside the
target, so a process sitting in a Windows call ignores even SIGKILL. kill,
killpg and pkill all failed against it, which also defeated the orphan
reapers and fleettest --cleanup.
Snapshot the daemon's children before killing it (once the parent is gone
they are reparented and no longer identifiable as ours) and kill them too,
re-checking with _pid_is_rsync before each signal so a pid recycled in the
meantime is never signalled. Where signals cannot win, fall back to
terminating the winpid via taskkill; fleettest --cleanup gets the same
fallback, so it can no longer report SURVIVED and leave the port squatted.
_reap_group() reports success only once the daemon is confirmed gone
rather than when a signal was merely accepted -- on Cygwin a signal is
routinely accepted by a process that then ignores it -- and confirms with
a bounded poll, because SIGKILL is asynchronous and calling a
still-terminating process "alive" would make _probe_bindable() skip its
retry and fail a test for a port that was about to free itself.
_cleanup_rsyncd() keeps the port's pid record only while it still names a
live rsync. Keying that on the port being busy instead looks safer but is
worse: a port sits in TIME_WAIT after a passing test, so a record naming
an already-dead pid would be retained forever, and nothing clears such a
record -- yet no reaper can use it either, since they all reject it at the
_pid_is_rsync guard, leaving only the hazard that its pid is recycled onto
an unrelated rsync.
The daemon stays in the TEST's process group on purpose: runtests.py
killpg's that group on a per-test timeout, and that is what keeps a
timed-out test from stranding its daemon. An earlier version of this fix
gave the daemon its own group so one killpg would catch the children --
which silently broke that sweep, and a full Cygwin pass then stranded two
parent daemons when variety hit its timeout.
Two residual limitations are documented in the code rather than left to be
rediscovered: _kill_pid's check-then-signal is inherently a TOCTOU
(narrowed to microseconds, not closed; closing it needs pidfd or retained
Windows handles across seven platforms), and _stop_rsyncd cannot collect
children when the parent has already exited on its own, because the
parent-child link it relies on is gone by then.
Measured on a Cygwin VM, 4 proxy/daemon tests x 8 runs at -j4: before 5/8
runs left an orphan (one left two), after 0/10. All tests passed in every
run, before and after -- which is the point: the leak was invisible to the
suite. A test killed by the runner's timeout still leaves no daemon
behind.
Zen Dodd