rsync: failed verification -- update discarded - regression from CVE fixes

[samueloph]

Hello,

We've got the following bug report on Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1093160

Then I was able to repro this regression on both Debian and Amazon Linux.

To repro, you need a host you can ssh to, in this case I'm using localhost:

cd "$HOME"
mkdir real-dir
ln -s real-dir dir
mkdir /tmp/dir
echo "foo" > /tmp/dir/file
cd /tmp
rsync -KRlptzv dir/file localhost:
touch dir/file
rsync -KRlptzv dir/file localhost:

This results in:

rsync -KRlptzv dir/file localhost:
dir/
dir/file
WARNING: dir/file failed verification -- update discarded (will try again).
dir/file
ERROR: dir/file failed verification -- update discarded.

sent 146 bytes  received 165 bytes  207.33 bytes/sec
total size is 4  speedup is 0.01
rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1179) [sender=3.1.2]

[samueloph]

cc @ncopa so they can check if Alpine is also affected.

[vinc17fr]

I would add that it is important to use ssh (or similar). Just using rsync -KRlptzv dir/file $HOME/ updates the file without any error.

[carnil]

If I have not done an error on bisecting the issue it is introduced with c35e28331f10ba6eba370611abd78bde32d54da7

git bisect start
# status: waiting for both good and bad commits
# bad: [fcfdd36054db18c12a85d9f42365f93c04247637] Update MAINTAINER_TZ_OFFSET on release.
git bisect bad fcfdd36054db18c12a85d9f42365f93c04247637
# status: waiting for good commit(s), bad commit known
# good: [9615a2492bbf96bc145e738ebff55bbb91e0bbee] added apple silicon path details
git bisect good 9615a2492bbf96bc145e738ebff55bbb91e0bbee
# good: [b4a27ca25d0abb6fcf14f41b7e11f3a6e1d8a4ff] added secure_relative_open()
git bisect good b4a27ca25d0abb6fcf14f41b7e11f3a6e1d8a4ff
# bad: [a6312e60c95e5ebb5764eaf18eb07be23420ebc6] Force rsync group when uploading files.
git bisect bad a6312e60c95e5ebb5764eaf18eb07be23420ebc6
# bad: [0590b09d9a34ae72741b91ec0708a820650198b0] fixed symlink race condition in sender
git bisect bad 0590b09d9a34ae72741b91ec0708a820650198b0
# bad: [688f5c379a433038bde36897a156d589be373a98] Refuse a duplicate dirlist.
git bisect bad 688f5c379a433038bde36897a156d589be373a98
# bad: [9f86ddc9652247233f32b241a79d5aa4fb9d4afa] disallow ../ elements in relpath for secure_relative_open
git bisect bad 9f86ddc9652247233f32b241a79d5aa4fb9d4afa
# bad: [c35e28331f10ba6eba370611abd78bde32d54da7] receiver: use secure_relative_open() for basis file
git bisect bad c35e28331f10ba6eba370611abd78bde32d54da7
# first bad commit: [c35e28331f10ba6eba370611abd78bde32d54da7] receiver: use secure_relative_open() for basis file

[yankov-pt]

@carnil Did you use the above snippet to reproduce? I've tried reproducing with bisecting and I keep triggering this behavior going back to at least several versions. With the oldest binary on a system I have - 3.1.3 still happens.

[vinc17fr]

With the oldest binary on a system I have - 3.1.3 still happens.

This is strange. In Debian 12 (bookworm), the original 3.2.7 does not have this issue, which appeared with the CVE fixes.

[vinc17fr]

BTW, when testing, do not forget --rsync-path=/path/to/tested/rsync if the version to be tested is not first in $PATH.

[carnil]

@yankov

@carnil Did you use the above snippet to reproduce? I've tried reproducing with bisecting and I keep triggering this behavior going back to at least several versions. With the oldest binary on a system I have - 3.1.3 still happens.

I'm using the following (but it does not yet work for automatic bisection, so did manual test)

#!/bin/sh

if
    rm -rf -- "$HOME/real-dir" "$HOME/dir" "/tmp/dir"
    git clean -d -x -f && ./configure --with-included-zlib=no --disable-md5-asm --with-rrsync && make
then
    # run project specific test and report its status
    cd "$HOME"
    mkdir real-dir
    ln -s real-dir dir
    mkdir /tmp/dir
    echo "foo" > /tmp/dir/file
    cd /tmp
    ${HOME}/rsync/rsync -q --rsync-path=${HOME}/rsync/rsync -KRlptzv dir/file localhost:
    sleep 1
    touch dir/file
    ${HOME}/rsync/rsync -q --rsync-path=${HOME}/rsync/rsync -KRlptzv dir/file localhost:
    status=$?
    exit ${status}
else
    # tell the caller this is untestable
    status=125
    exit ${status}
fi

(and it's surely not state-of-the-art script bisection, sorry, but more quick-and-dirty to get an idea on the problem)

Edit: Fixed with providing correct existstatus and so possible to bisect automatically.

[yankov-pt]

@vinc17fr Thank you! I thought I was going crazy, had a bad version rsync version in path, so ALL my tests were redundant. Nevermind my last comment in this case!

[samueloph]

Here's a working bisect script, based on the work of @carnil:

#!/bin/bash

untestable() {
    git_clean
    exit 125
}

fail() {
    git_clean
    exit 1
}

git_clean() {
    git reset --hard
    git clean -d -x -f
    set +x
}

set -x

RSYNC_BINARY="${HOME}/rsync/rsync"

rm -rf -- "$HOME/real-dir" "$HOME/dir" "/tmp/dir"
./configure --with-included-zlib=no --disable-md5-asm --with-rrsync --disable-md2man --disable-xxhash || untestable
make || untestable

# run project specific test and report its status
pushd "$HOME" || fail
mkdir real-dir
ln -s real-dir dir
mkdir /tmp/dir
echo "foo" > /tmp/dir/file
pushd /tmp || fail
${RSYNC_BINARY} -q --rsync-path="${RSYNC_BINARY}" -KRlptzv dir/file localhost: || fail
sleep 1
touch dir/file
${RSYNC_BINARY} -q --rsync-path="${RSYNC_BINARY}" -KRlptzv dir/file localhost: || fail

# Go back to the git folder
popd || :
popd || :

# If we got here, success.
set +x
git reset --hard
exit 0

Beware that you need to install some build dependencies, otherwise the configure step will always fail.

Place the script in the same folder where you have the git repo folder, eg:

$HOME/rsync_bisect.sh 
$HOME/rsync/ <- This is the git repo

Then run:

chmod +x rsync_bisect.sh
cd "$HOME"/rsync
git bisect start v3.4.1 v3.2.7
git bisect run ../rsync_bisect.sh

The automated run against v3.2.7->v3.4.1 identified the same commit as @carnil as the cause: https://github.com/RsyncProject/rsync/commit/c35e28331f10ba6eba370611abd78bde32d54da7

[samueloph]

I have modified receiver.c to make the rsync server hang right before printing the error, allowing me to attach gdb to it.

diff --git a/receiver.c b/receiver.c
index edfbb210..d91ce64a 100644
--- a/receiver.c
+++ b/receiver.c
@@ -962,6 +962,9 @@ int recv_files(int f_in, int f_out, char *local_name)
                                        redostr = read_batch ? " (may try again)"
                                                             : " (will try again)";
                                }
+                               sigset_t myset;
+                               (void) sigemptyset(&myset);
+                               sigsuspend(&myset);
                                rprintf(msgtype,
                                        "%s: %s failed verification -- update %s%s.\n",
                                        errstr, local_name ? f_name(file, NULL) : fname,

Stacktrace on the rsync server/receiver:

(gdb) bt 25
#0  0x00007f8c5e44a025 in __GI___sigsuspend (set=0x7ffff0510b00) at ../sysdeps/unix/sysv/linux/sigsuspend.c:26
#1  0x0000560bc6b3490b in recv_files (f_in=0, f_out=4, local_name=0x0) at receiver.c:967
#2  0x0000560bc6b424f3 in do_recv (f_in=0, f_out=4, local_name=0x0) at main.c:1056
#3  0x0000560bc6b42b19 in do_server_recv (f_in=0, f_out=1, argc=1, argv=0x560befad97d8) at main.c:1227
#4  0x0000560bc6b42c5b in start_server (f_in=0, f_out=1, argc=2, argv=0x560befad97d0) at main.c:1261
#5  0x0000560bc6b44197 in main (argc=2, argv=0x560befad97d0) at main.c:1845

Note that you need to compile without optimizations so the variables are not redacted.

[samueloph]

Debugging this with @sergiodj.

It appears that the issue was caused by changing a function used to open files, from do_open to secure_relative_open. This happened at: https://github.com/RsyncProject/rsync/commit/c35e28331f10ba6eba370611abd78bde32d54da7?diff=unified#diff-d683462ec90349ecc1059c14be8fe3e48d6dbd0d1980e81914c5bad5b670e5c1L768-R772 And: https://github.com/RsyncProject/rsync/commit/c35e28331f10ba6eba370611abd78bde32d54da7?diff=unified#diff-d683462ec90349ecc1059c14be8fe3e48d6dbd0d1980e81914c5bad5b670e5c1L783-R786

The new function secure_relative_open was created at: https://github.com/RsyncProject/rsync/commit/b4a27ca25d0abb6fcf14f41b7e11f3a6e1d8a4ff

And here you see O_NOFOLLOW being used: https://github.com/RsyncProject/rsync/blob/23d9ead5af0249babf241c5917ea2b22c2754bcb/syscall.c#L764-L772

secure_relative_open will make use of O_NOFOLLOW, which breaks the usecase of the reproducer.

[samueloph]

To summarize what I believe it's going on at a high level:

recv_files was modified to open files with O_NOFOLLOW, the comment in the new function secure_relative_open hints at this being to avoid vulnerabilities related to path traversal.

The problem is that this breaks usecases where rsync needs to do that, such as when there's a symlink which points to a folder outside of the destdir.

Now, this only breaks when there's a transfer trying to update an existing file, it will still traverse the path on the first transfer.

[vinc17fr]

Note that (at least in my case) the path traversal is set up on the server side. So it is not due to an "attack" from the source (client).

[sudhackar]

Can confirm this fails on ubuntu as well

sudhackar@sec-noble-server-amd64:~$ cd "$HOME"
mkdir real-dir
ln -s real-dir dir
mkdir /tmp/dir
echo "foo" > /tmp/dir/file
cd /tmp
rsync -KRlptzv dir/file localhost:
touch dir/file
rsync -KRlptzv dir/file localhost:
sudhackar@localhost's password:
dir/file

sent 109 bytes  received 31 bytes  56.00 bytes/sec
total size is 4  speedup is 0.03
sudhackar@localhost's password:
dir/
dir/file
WARNING: dir/file failed verification -- update discarded (will try again).
dir/file
ERROR: dir/file failed verification -- update discarded.

sent 146 bytes  received 85 bytes  66.00 bytes/sec
total size is 4  speedup is 0.02
rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1338) [sender=3.2.7]

sudhackar@sec-noble-server-amd64:/tmp$ apt-cache policy rsync
rsync:
  Installed: 3.2.7-1ubuntu1.2
  Candidate: 3.2.7-1ubuntu1.2
  Version table:
 *** 3.2.7-1ubuntu1.2 500
        500 http://archive.ubuntu.com/ubuntu noble-updates/main amd64 Packages
        500 http://security.ubuntu.com/ubuntu noble-security/main amd64 Packages
        500 https://ppa.launchpadcontent.net/ubuntu-security-proposed/ppa/ubuntu noble/main amd64 Packages
        100 /var/lib/dpkg/status
     3.2.7-1ubuntu1 500
        500 http://archive.ubuntu.com/ubuntu noble/main amd64 Packages

[tridge]

the secure_relative_open() change was to address CVE-2024-12086. That vulnerability involved a way for a malicious server to manipulate which file is used as the "basis file" for checksum comparison. If an attacker can manipulated to point at a sensitive file (think for of example of pointing at /etc/shadow) then the attacker can determine the contents of the sensitive file. In this particular case the call looks like this:

  secure_relative_open(NULL, "dir/file", O_RDONLY, 0);

dir is a symlink to real-dir which is inside the target tree, so it is legitimate, but there is no way I know of to have a call which says "only follow symlinks if they stay within a subtree" without a race condition. still thinking about this ...

[vinc17fr]

Please ignore the "only follow symlinks if they stay within a subtree". In my real case, the symlink on the server is

public_html -> ../../www/www.vinc17.net/htdocs

and I'm using a symlink precisely because the directory to update is not inside the target tree.

But the escape of the subtree is done on the server, and this is a symbolic link that will not be modified by the rsync. So I don't see any security issue (a possible attack by the client) in such a case.

There could also be the notion of trusted user (e.g. via configuration), such as users who have full access to both machines anyway (I think, something like --trust-sender, but that would be set up on the server side).

[carenas]

but there is no way I know of to have a call which says "only follow symlinks if they stay within a subtree" without a race condition.

there is no portable way, but for linux there is openat2()

[samueloph]

What I was looking to achieve for a solution was having the receiver be able to distinguish what is a symlink from the source and what's a symlink from dest, then it should only refuse to follow symlinks outside of the target tree if the symlink is in source, not in dest.

But a change of this scope probably requires more time than I can allocate, one has to be very careful to not re-introduce the path traversal CVE when distinguishing between source and dest symlinks.

[DemiMarie]

Please ignore the "only follow symlinks if they stay within a subtree". In my real case, the symlink on the server is

public_html -> ../../www/www.vinc17.net/htdocs

and I'm using a symlink precisely because the directory to update is not inside the target tree.

But the escape of the subtree is done on the server, and this is a symbolic link that will not be modified by the rsync. So I don't see any security issue (a possible attack by the client) in such a case.

What prevents the client from making the symlink point to somewhere else?

I recommend modifying the rsync call so that the symlink is included in the (trusted) path included on the command line, rather than the (untrusted) path provided by the client. Alternatively, you can use a bind mount, but that requires root privileges or a user namespace. Bind mounts cannot be created by the client, so they are not spoofable.

[vinc17fr]

What prevents the client from making the symlink point to somewhere else?

Thanks to the -K option, this is regarded as a directory. Or do you mean that rsync can replace a directory (thus removing it with all its files) by something else without --delete?

I recommend modifying the rsync call so that the symlink is included in the (trusted) path included on the command line, rather than the (untrusted) path provided by the client.

Other files under the home directory need to be transferred, not just files under public_html. Moreover, for some machines, public_html is a real directory. So any change would complicate things even more.

Alternatively, you can use a bind mount, but that requires root privileges or a user namespace. Bind mounts cannot be created by the client, so they are not spoofable.

I prefer to avoid the bind mount, which yields other problems.

Anyway, there are probably other use cases where symlinks to arbitrary locations need to be followed, for users who want to synchronize their own machines with specific settings, from a single source. So there should be a notion of trusted directory on the server, meaning that any change is under the control of a trusted user. For instance, a user could typically declare his home directory as trusted (if an arbitrary user could modify the home directory, he could modify the init scripts, which is much more problematic than issues with symlinks).

[DemiMarie]

-K is already documented as unsafe with untrusted clients. Does this regression affect setups that do not use -K?

[vinc17fr]

-K is already documented as unsafe with untrusted clients.

Note that the remote side is a personal server, and only a trusted client can be used: rsync is remotely usable only via an unrestricted ssh access (once authentication has succeeded), i.e. being allowed to use rsync is equivalent to have a remote shell on the server. And the use with this -K option on the client is done via a script from a specific directory where I control all the files (which is really important, even if -K were not used).

Does this regression affect setups that do not use -K?

It is the -K option that makes this regression occur, in the sense that errors occur. Dropping -K does not yield errors, but it has the effect to overwrite the symlink on the server, meaning that the files are stored in an incorrect place.

So, attempting to fix the regression only for -K would be OK. And since -K should not be enabled with untrusted clients, there is no need to be strict on security about the files in such a case, IMHO.

[DemiMarie]

-K is already documented as unsafe with untrusted clients.

Note that the remote side is a personal server, and only a trusted client can be used: rsync is remotely usable only via an unrestricted ssh access (once authentication has succeeded), i.e. being allowed to use rsync is equivalent to have a remote shell on the server. And the use with this -K option on the client is done via a script from a specific directory where I control all the files (which is really important, even if -K were not used).

That’s what I expected. rsync can also be used in restricted setups that do not allow shell access or to receive files from a not-fully-trusted server, and those are the cases where the receiver-side vulnerabilities matter. In your case they do not.

Does this regression affect setups that do not use -K?

It is the -K option that makes this regression occur, in the sense that errors occur. Dropping -K does not yield errors, but it has the effect to overwrite the symlink on the server, meaning that the files are stored in an incorrect place.

So, attempting to fix the regression only for -K would be OK. And since -K should not be enabled with untrusted clients, there is no need to be strict on security about the files in such a case, IMHO.

I agree. -K is already unsafe unless both the sender and all other users with write access to the directory tree are trusted, so it is okay to not use secure_relative_open() on the receiver in this case.

[GuillaumeHullin]

Hi there, Jumping into a conversation (which is out of my depth) but I think I have a similar regression from latest CVE fixes. It's not the same but maybe very related.

My problem is concerning interrupted file transfer (via SSH) with the --partial and --partial-dir.

When restarting the transfer, I get an error AFTER (what seems like) a full retransmission

WARNING: path/file.ext failed verification -- update put into partial-dir (will try again).
ERROR: path/file.ext failed verification -- update put into partial-dir.

Running an md5sum on each file shows that the files are actually different. Also what is "weird" is that the size of the partial file (after interruption of the transfer) is bigger by a few bytes (about 40). Which is not logical (at least for me).

If I delete the partial dir (and temp dir if left-over), the transfer does not complete and have the same warning and error.

Edit: I run Debian 12 and BTRFS as filesystem.

[sudhackar]

Hello @GuillaumeHullin

Can you maybe do a short reproducer? Also #722 has some --partial-dir issue as well.

[GuillaumeHullin]

@sudhackar

Here is my reproducer (2 working exemple and 1 failing one). The problem seems to be actually --partial-dir as you said.

## Rsync without --partial: working.
root@debian:~# rsync --archive --progress --human-readable --rsh 'ssh -oPort=12345' root@remote-server:/root/rsync/somefile.ext /root/rsync/
receiving incremental file list
somefile.ext
        123.52M  21%    2.94MB/s    0:02:29  ^C
rsync error: received SIGINT, SIGTERM, or SIGHUP (code 20) at io.c(519) [receiver=3.2.7]
rsync error: unexplained error (code 255) at rsync.c(716) [generator=3.2.7]
rsync: [generator] write error: Broken pipe (32)
###CTRL+C###
root@debian:~# rsync --archive --progress --human-readable --rsh 'ssh -oPort=12345' root@remote-server:/root/rsync/somefile.ext /root/rsync/
receiving incremental file list
somefile.ext
        574.04M 100%   19.10MB/s    0:00:28 (xfr#1, to-chk=0/1)
root@debian:~#


## Rsync with --partial: working.
root@debian:~# rsync --archive --progress --partial --human-readable --rsh 'ssh -oPort=12345' root@remote-server:/root/rsync/somefile.ext /root/rsync/
receiving incremental file list
somefile.ext
        101.71M  17%   20.41MB/s    0:00:22  ^C
rsync error: received SIGINT, SIGTERM, or SIGHUP (code 20) at io.c(519) [receiver=3.2.7]
rsync error: unexplained error (code 255) at rsync.c(716) [generator=3.2.7]
rsync: [generator] write error: Broken pipe (32)
###CTRL+C###
root@debian:~# rsync --archive --progress --partial --human-readable --rsh 'ssh -oPort=12345' root@remote-server:/root/rsync/somefile.ext /root/rsync/
receiving incremental file list
somefile.ext
        574.04M 100%    7.53MB/s    0:01:12 (xfr#1, to-chk=0/1)
root@debian:~#

## Rsync with --partial and --partial-dir: NOT working.
root@debian:~# rsync --archive --progress --partial --partial-dir /root/rsync-partial --human-readable --rsh 'ssh -oPort=12345' root@remote-server:/root/rsync/somefile.ext /root/rsync/
receiving incremental file list
somefile.ext
        149.45M  26%   19.40MB/s    0:00:21  ^C
rsync error: unexplained error (code 255) at rsync.c(716) [generator=3.2.7]
rsync: [generator] write error: Broken pipe (32)
rsync error: received SIGINT, SIGTERM, or SIGHUP (code 20) at io.c(519) [receiver=3.2.7]
###CTRL+C###
root@debian:~# rsync --archive --progress --partial --partial-dir /root/rsync-partial --human-readable --rsh 'ssh -oPort=12345' root@remote-server:/root/rsync/somefile.ext /root/rsync/
receiving incremental file list
somefile.ext
        574.04M 100%    7.60MB/s    0:01:12 (xfr#1, to-chk=0/1)
WARNING: somefile.ext failed verification -- update discarded (will try again).
somefile.ext
        574.04M 100%   13.31MB/s    0:00:41 (xfr#2, to-chk=0/1)
ERROR: somefile.ext failed verification -- update discarded.
rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1865) [generator=3.2.7]
root@debian:~#

[sudhackar]

Hello @GuillaumeHullin

I have root-caused your issue to something similar in #722

This only happens when the --partial-dir is actually an absolute path - this issue won't happen when you do something like --partial-dir ./rsync-partial. Let's continue in that thread.

[kirik]

Got same error recently with rsync v3.1.3 (both sender and receiver) on Ubuntu 20.04. It was working fine before for years but then something happened :) I'm 99% sure that environment hasn't been changed, I'm using it as automatic web-project deployment tool and haven't touch anything for several months. Can confirm that without --keep-dirlinks (-K) it working. And also if I replace symlink with real directory on receiver side - it also going ok. Also can confirm that it fails only with existed files, first sync is completed but second one fails.

[yankov-pt]

@kirik You probably got a security update in the last weeks. The CVE fixes that introduced this were ported to pretty much any rsync 3.x version lately. Checking the changelog, ubuntu20.04 rysnc contains the fixes for those issues.

[moximoxini]

The code seems to be a change caused by secure_relative_open(), so do you want to optimize this function further?