rsync 3.1.3 performance regression

The attachment "add-trusted-sender-arg.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

Thank you for your bug report and for providing the relevant commit. A backport of the --trust-sender flag might not be accepted since it involves adding a new feature. It may be worth a try though since it helps reduce performance issues.

To get this started I created a PPA that patches in the flag here: https://launchpad.net/~lvoytek/+archive/ubuntu/rsync-focal-add-trust-sender

If you would like to test it you can run the following:

$ sudo add-apt-repository ppa:lvoytek/rsync-focal-add-trust-sender
$ sudo apt update
$ sudo apt upgrade

Hi Lena, thanks for getting back to this and appreciate your effort in putting up the PPA!
I think the major concerns here is that the security patch changed the behavior on how arguments from the remote side were handled. And adding the new flag a) doesn't break any existing usage of the tool, and b) will give users the option to get back the old behavior and performance before we introduced the security path series. What do you think?

As one member of the SRU team, I think that adding the --trust-sender feature in an SRU is acceptable in principle, provided that the patch is trivial and low risk, in order to mitigate the (required) security performance regression. So it looks like this should be fine, but I defer a final decision to a deeper review which we can do once the fix is tested in Lena's PPA.

Note however that this would requiring adding the option to every subsequent still-supported stable release that does not already have it to ensure that users don't face a regression when the upgrade to a newer release.

SRU team opinion seems to be that this should be fine in principle, subject to full review of the final upload.

@Ye Lu

Please could you test Lena's PPA above and report the result?

Hello, that's alright, I created a test case to show the argument works and that there is a speedup:

$ lxc launch ubuntu:focal test-rsync-receiver
$ lxc exec test-rsync-receiver bash
# apt update && apt dist-upgrade -y
# apt install openssh-server rsync -y
# passwd ubuntu
- set password for user
# exit

- Check ip of receiver with lxc list
$ lxc list

$ lxc launch ubuntu:focal test-rsync-sender
$ lxc exec test-rsync-sender bash
# apt update && apt dist-upgrade -y

# add-apt-repository ppa:lvoytek/rsync-focal-add-trust-sender
# apt install rsync -y

- Create a random file to send over
# dd if=/dev/urandom of=randomfile.bin bs=1M count=1000

- Send without --trust-sender
# rsync -av randomfile.bin ubuntu@<receiver ip>:~/file1.bin

- Send with --trust-sender
# rsync -av --trust-sender randomfile.bin ubuntu@<receiver ip>:~/file2.bin

I'll get this ready for 20.04 on Monday since the SRU team will likely approve.
Thanks!

Hi Robie and Lena,

Thanks for accepting the patch and taking the effort to test it!

Although I wasn't able to test your PPA on the workload we are having issue with, I did some additional testing by extracting your binary and install it manually on a test machine. I tested it with some local rsync workloads before and after using --trust-sender. And I put the result here for your reference in case it would be helpful:

```
$ dpkg -l rsync
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name Version Architecture Description
+++-==============-=====================-============-===============================>
ii rsync 3.1.3-8ubuntu0.6~ppa2 amd64 fast, versatile, remote (and lo>

$ sudo rsync -av $(local-src) $(local-dest)
...
sent 957,005,746 bytes received 1,478,644 bytes 112,762,869.41 bytes/sec

$ sudo rsync -av --trust-sender $(local-src) $(local-dest2)
sent 957,005,754 bytes received 1,478,704 bytes 147,459,147.38 bytes/sec
```

We are seeing ~30% speedup for this specific workload.

Hi,
My team has run into this problem too. We have a backup job that runs via rsync. Prior to this security fix, the job completed in about 1.5 hours, but with the security fix it's now running at about 10 hours instead. (About 6.6x slower). We've tested the proposed fix here and found that it restores our expected backup times, as long as we use the --trust-sender argument. Is this something that you'd consider making generally available for focal and the other releases where this fix was backported without --trust-sender being available? Thanks!

Hi Krister,
Yes, this should be available soon for Focal generally. The update is currently under review by the SRU team. Once that is complete and the package is tested it will be uploaded to the archive. This page will then be updated marking focal as fix-released.
Thanks!

Hello Ye, or anyone else affected,

Accepted rsync into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/rsync/3.1.3-8ubuntu0.6 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

All autopkgtests for the newly accepted rsync (3.1.3-8ubuntu0.6) for focal have finished running.
The following regressions have been reported in tests triggered by the package:

mariadb-10.3/1:10.3.38-0ubuntu0.20.04.1 (armhf)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/focal/update_excuses.html#rsync

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

Verified:

$ lxc launch ubuntu:focal test-rsync-receiver
$ lxc exec test-rsync-receiver bash
# apt update && apt dist-upgrade -y
# apt install openssh-server rsync -y

# passwd ubuntu
New password:
Retype new password:
passwd: password updated successfully

# sed -i 's/PasswordAuthentication no/PasswordAuthentication yes/' /etc/ssh/sshd_config
# systemctl restart sshd

# exit

- Check ip of receiver with lxc list
$ lxc list

+---------------------+---------+----------------------+-----------------------------------------------+-----------+-----------+
| NAME | STATE | IPV4 | IPV6 | TYPE | SNAPSHOTS |
+---------------------+---------+----------------------+-----------------------------------------------+-----------+-----------+
| test-rsync-receiver | RUNNING | 10.190.23.229 (eth0) | -------------------------------------- (eth0) | CONTAINER | 0 |
+---------------------+---------+----------------------+-----------------------------------------------+-----------+-----------+

$ lxc launch ubuntu:focal test-rsync-sender
$ lxc exec test-rsync-sender bash
# apt update && apt dist-upgrade -y

# cat <<EOF >/etc/apt/sources.list.d/ubuntu-$(lsb_release -cs)-proposed.list
# Enable Ubuntu proposed archive
deb http://archive.ubuntu.com/ubuntu/ $(lsb_release -cs)-proposed restricted main multiverse universe
EOF

# apt install rsync -y

# dd if=/dev/urandom of=randomfile.bin bs=1M count=1000

# rsync -av randomfile.bin ubuntu@10.190.23.229:~/file1.bin
ubuntu@10.190.23.229's password:
sending incremental file list
randomfile.bin

sent 1,048,832,093 bytes received 35 bytes 123,392,015.06 bytes/sec
total size is 1,048,576,000 speedup is 1.00

# rsync -av --trust-sender randomfile.bin ubuntu@10.190.23.229:~/file2.bin
ubuntu@10.190.23.229's password:
sending incremental file list
randomfile.bin

sent 1,048,832,093 bytes received 35 bytes 139,844,283.73 bytes/sec
total size is 1,048,576,000 speedup is 1.00

I'm a bit reluctant to release this because the update is not documenting the new option, and I can't find out why. The original MP[1] review implies that documentation was present, both in -h output, and in the manpage, but I don't see it in the package in proposed. This makes it very hard for users to discover the jammy package would now have this new option.

Do we know what happened here? Was it a conscious decision to not include the doc changes? Or did something fell through the cracks between review and upload to unapproved? Note the --help output comes from a section of the manpage.

1. https://code.launchpad.net/~lvoytek/ubuntu/+source/rsync/+git/rsync/+merge/448637/comments/1197139

Added an additional merge request containing man and help docs
https://code.launchpad.net/~lvoytek/ubuntu/+source/rsync/+git/rsync/+merge/450497

3.1.3-8ubuntu0.7 was just uploaded to focal unapproved with the documentation additions and an appropriate .changes file:

Changes:
 rsync (3.1.3-8ubuntu0.7) focal; urgency=medium
 .
   * d/p/add-trust-sender-option-docs.patch: Add manpage and help documentation
     for the --trust-sender option (LP: #2028810)
 .
 rsync (3.1.3-8ubuntu0.6) focal; urgency=medium
 .
   * d/p/add-trust-sender-option.patch: Add --trust-sender argument to decrease
     overhead when transferring files (LP: #2028810)
     In order to mitigate the performance decrease experienced by the security
     update blocking arbitrary file writes by remote servers, this update allows
     users the option to inherently trust the remote server instead. The
     --trust-sender argument tells the local server to trust the remote server's
     file list, leading to a speedup in transfer speed since the extra checks
     are no longer needed. The argument should only be used when transferring
     between two controlled servers though, to avoid arbitrary file access from
     a malicious server.

Hello Ye, or anyone else affected,

Accepted rsync into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/rsync/3.1.3-8ubuntu0.7 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Re-verified with documentation:

$ lxc launch ubuntu:focal test-rsync-receiver
$ lxc exec test-rsync-receiver bash
# apt update && apt dist-upgrade -y
# apt install openssh-server rsync -y

# passwd ubuntu
New password:
Retype new password:
passwd: password updated successfully

# sed -i 's/PasswordAuthentication no/PasswordAuthentication yes/' /etc/ssh/sshd_config
# systemctl restart sshd

# exit

- Check ip of receiver with lxc list
$ lxc list

+---------------------+---------+----------------------+-----------------------------------------------+-----------+-----------+
| NAME | STATE | IPV4 | IPV6 | TYPE | SNAPSHOTS |
+---------------------+---------+----------------------+-----------------------------------------------+-----------+-----------+
| test-rsync-receiver | RUNNING | 10.190.23.243 (eth0) | -------------------------------------- (eth0) | CONTAINER | 0 |
+---------------------+---------+----------------------+-----------------------------------------------+-----------+-----------+

$ lxc launch ubuntu:focal test-rsync-sender
$ lxc exec test-rsync-sender bash

# cat <<EOF >/etc/apt/sources.list.d/ubuntu-$(lsb_release -cs)-proposed.list
# Enable Ubuntu proposed archive
deb http://archive.ubuntu.com/ubuntu/ $(lsb_release -cs)-proposed restricted main multiverse universe
EOF

# apt update && apt dist-upgrade -y

# apt install rsync -y

# rsync --help

...
 -s, --protect-args no space-splitting; only wildcard special-chars
     --trust-sender trust the remote sender's file list
     --address=ADDRESS bind address for outgoing socket to daemon
...

# man rsync

...
       --trust-sender
              Disable the extra validation of the file list from a remote sender (this safety feature was added to address the performance downgrade after
              fixing CVE 2022-29154). This should only be done if you trust the sender to not try to do something malicious, which should be the case if
              they're running a stock rsync.

              Normally when pulling files from a remote rsync, the client runs 2 extra validation checks:

              o Verify that additional arg items didn't get added at the top of the transfer.

              o Verify that none of the items in the file list should have been excluded.

              Note that various options can turn off one or both of these checks if the option interferes with the validation. For instance:

              o Using a per-directory filter file reads filter rules that only the server knows about, so the filter checking is disabled.

              o Using the --old-args option allows the sender to manipulate the requested args, so the arg checking is disabled.

              o Reading the files-from list from the server side means that the client doesn't know the arg list, so the arg checking is disabled.

              o Using --read-batch disables both checks since the batch file's contents will have been verified when it was created.

              This option may help an under-powered client server if the extra pattern matching is slowing things down on a huge transfer. It can also be
              used to work...

This bug was fixed in the package rsync - 3.1.3-8ubuntu0.7

---------------
rsync (3.1.3-8ubuntu0.7) focal; urgency=medium

  * d/p/add-trust-sender-option-docs.patch: Add manpage and help documentation
    for the --trust-sender option (LP: #2028810)

rsync (3.1.3-8ubuntu0.6) focal; urgency=medium

  * d/p/add-trust-sender-option.patch: Add --trust-sender argument to decrease
    overhead when transferring files (LP: #2028810)
    In order to mitigate the performance decrease experienced by the security
    update blocking arbitrary file writes by remote servers, this update allows
    users the option to inherently trust the remote server instead. The
    --trust-sender argument tells the local server to trust the remote server's
    file list, leading to a speedup in transfer speed since the extra checks
    are no longer needed. The argument should only be used when transferring
    between two controlled servers though, to avoid arbitrary file access from
    a malicious server.

 -- Lena Voytek <email address hidden> Fri, 01 Sep 2023 11:38:04 -0700

The verification of the Stable Release Update for rsync has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.