Bug 1046197

That is not so brilliant idea. We have old zlib there atm 1.2.8 which contains known CVEs and we have confirmed we forget to update this bundled one every time it gets out of sync.

From what I can see it is used in a couple of places when using the system version and thus token.c needs to be tweaked.

options.c
1869:#ifdef EXTERNAL_ZLIB

configure.ac
813:    AC_DEFINE(EXTERNAL_ZLIB, 1, [Define to 1 if using external zlib])

token.c
408:#ifndef EXTERNAL_ZLIB
582:#ifndef EXTERNAL_ZLIB

I will check out how much work would be to build the "bundled code" together with the external one for time being...

The first question you need to ask is whether these CVEs you refer to are relevant here. It's not like we are decompressing some malicious email attachment. Rather, this is compressed material coming from another instance of rsync on a remote machine. So the source of the compressed material is known.

The situation now is that compressed rsync is not possible with older versions of rsync. That situation will likely continue to exist for years to come until all the old instances have disappeared. Being able to compress content over slow lines is a VERY important feature that many people will rely on. For some it may even be mission critical. For some it may also mean additional cost if they are charged by the volume of data transferred... So there is a real impact, more or less on a day-to-day basis, from not being able to compress data.

So please consider this impact as well and keep the compression feature alive as long as is needed.

I modified the Summary to better reflect the problem and to make it easier to find.

This is not a problem of rsync version 3.1.2, it is a problem of the configuration options. You just need to change zlib to --with-included-zlib=yes, and of course not delete the included zlib in the spec file. I have done exactly that and my rsync works again, so my problem is solved at the moment, but I would prefer it if I didn't have to compile it myself every time there is an update.

To add to the motivation for enabling the compress option:
If the option -z is given to the rsync on the client side, it will ignore the option and send uncompressed data. This is not good, as in some cases the data can be compressed quite well, which can lead to additional time and cost, as mentioned before:

This rsync lacks old-style --compress due to its external zlib.  Try -zz.
Continuing without compression.

But if a client with zlib enabled passes the option -z to a new rsync on the server side, it will just abort and close the connection. That means that the transfer will not take longer of be more expensive, it will not happen at all:

rsync: This rsync lacks old-style --compress due to its external zlib.  Try -zz.
rsync error: syntax or usage error (code 1) at main.c(1582) [server=3.1.2]
rsync: connection unexpectedly closed (0 bytes received so far) [Receiver]
rsync error: error in rsync protocol data stream (code 12) at io.c(235) [Receiver=3.1.0]

So I could either modify all scripts doing rsync with my server, and some of them are not even on my computers, or I could enable the zlib again, which is of course what I did.

Silly question - why does bundled vs. external zlib influence the commandline parameters at all? I don't see why a user would have to use a different commandline parameter just because an internal detail of rsync changed.

Ideally rsync -z should use whatever zlib is there (bundled or external), and I'd expect that both will result in the same compressed bytes.

(This is something that should probably be done upstream, so feel free to forward my comment to the upstream bugtracker.)

BTW: --new-compress will become very funny if someone "invents" another compression-related parameter in a few years. Will this be --even-newer-compress then? ;-)

(In reply to Christian Boltz from comment #4)
> Silly question - why does bundled vs. external zlib influence the
> commandline parameters at all? I don't see why a user would have to use a
> different commandline parameter just because an internal detail of rsync
> changed.
According to this https://bugzilla.samba.org/show_bug.cgi?id=10372#c14 there is some problem with the system libz, so the new compression scheme was created to work with system libz. Unfortunately it is incompatible to the previous compression. There seem to be some modifications necessary to libz to support the previous compression.

On a side note, if rsync implements an new, incompatible compression, why not use something more efficient, like bz2 or xz?

OK, another silly question then. Would it be possible that rsync tries the new compression first, and if that fails automatically revert back to the old version? That would make the change transparent (and remove the need for confusing flags).

This is also clearly for upstream, so feel free to forward.

Created attachment 732218 [details]
rsync-zlib.patch

The content of the actual patch for zlib that makes rsync use the "old" compression.

Created attachment 732222 [details]
zlib-rsync.patch

Minimalized patch containing only factual changes.

Created attachment 732226 [details]
rsync-allow-both-compressions.patch

scarabeus@bugaboo: ~/tmp $ rsync -zz root@factorybreaker:~/bin/* ./
rsync: on remote machine: --new-compress: unknown option
rsync error: syntax or usage error (code 1) at main.c(1569) [server=3.1.0]
rsync: connection unexpectedly closed (0 bytes received so far) [Receiver]
rsync error: error in rsync protocol data stream (code 12) at io.c(235) [Receiver=3.1.2]
scarabeus@bugaboo: ~/tmp $ rsync -z root@factorybreaker:~/bin/* ./
scarabeus@bugaboo: ~/tmp $ echo $?
0

Looks working :)

This is an autogenerated message for OBS integration:
This bug (1046197) was mentioned in
https://build.opensuse.org/request/show/510032 Factory / rsync

(In reply to Tomáš Chvátal from comment #8)
> Created attachment 732222 [details]
> zlib-rsync.patch
> 
> Minimalized patch containing only factual changes.

Can we patch this to the system zlib during build to regenerate the bundled zlib with current fixes?

Or create a rsync-zlib-source or something.

(In reply to Michal Suchanek from comment #11)
> (In reply to Tomáš Chvátal from comment #8)
> > Created attachment 732222 [details]
> > zlib-rsync.patch
> > 
> > Minimalized patch containing only factual changes.
> 

Actually, the rename of read_buf to dread_buf is only to not conflict with rsync internal function. Making the function static would do the same I would expect.

Other than that the patch adds an option to zlib. That should not be a problem to patch into the zlib package and/or forward upstream.

Seems this has been supposedly resolved upstream:

https://bugzilla.samba.org/show_bug.cgi?id=6916
https://bugzilla.redhat.com/show_bug.cgi?id=495310

https://git.samba.org/?p=rsync.git;a=commitdiff;h=7da17144fd764a2420a8d08897475c0b7fdbf956


https://bugzilla.redhat.com/show_bug.cgi?id=490140

And broken in 3.1.1 

https://git.samba.org/?p=rsync.git;a=commitdiff;h=22a3ac0b5538ec6c1ff222570413cbdb74fef67b

In other words, you do not need a patched zlib for anything.

You need non-broken rsync.

After the last comments, I had hoped that this issue would be fixed with the next update of rsync. Yet here is rsync-3.1.2-2.2.x86_64, and the problem remains. In fact, the spec file for 2.2 is exactly the same as for 2.1, except for the Release line, so why have a new release at all? Someone did change the release number, or was this done by a script?

By the way, the supposed build date is exactly "Jun 14 14:00:00 2017" both for rsync-3.1.2-2.1 and rsync-3.1.2-2.2, so it seems that information is useless. But the Signature Date für 2.2 says Jul 17.

As mentioned before, having an rsync incapable of handling compression on the server side means that there will be no transfer at all, not merely a fallback to uncompressed.
At least is seems that "zypper up" wont replace my rsync version.

A simple solution is to just use the library that comes with rsync. That might not be the optimal solution, but it works while searching for a better solution.

--- a/rsync.spec
+++ b/rsync.spec
@@ -64,7 +64,7 @@
 
 %prep
 %setup -q -b 1
-rm -f zlib/*.h
+#rm -f zlib/*.h
 patch -p1 < patches/acls.diff
 patch -p1 < patches/xattrs.diff
 patch -p1 < patches/slp.diff
@@ -79,7 +79,7 @@
 export LDFLAGS="-Wl,-z,relro,-z,now -pie"
 %configure \
   --with-included-popt=no \
-  --with-included-zlib=no \
+  --with-included-zlib=yes \
   --disable-debug \
   --enable-slp \
   --enable-acl-support \

Or use rsync 3.1.0 (exactly - any earlier or later is broken).

(In reply to Michal Suchanek from comment #16)
> Or use rsync 3.1.0 (exactly - any earlier or later is broken).

I use rsync 3.1.2 with the patches from comment #15, and I had no problems so far.

(In reply to Ralf Friedl from comment #17)
> (In reply to Michal Suchanek from comment #16)
> > Or use rsync 3.1.0 (exactly - any earlier or later is broken).
> 
> I use rsync 3.1.2 with the patches from comment #15, and I had no problems
> so far.

But to achieve that the bundled zlib is used which brings the maintenance nightmare of trying to apply security fixes to it.

In contrast, 3.1.0 used system zlib for that.

(In reply to Michal Suchanek from comment #18)
> In contrast, 3.1.0 used system zlib for that.
Then why use 3.1.2 at all? What is the advantage?
If 3.1.0 could use system zlib, why can't 3.1.2? From the numbers I would assume just some minor changes.
What if rsync requires a security fix?

Michal is bit misleading here.

The 3.1.0 used only the "newcompress" while it broke the "oldcompress".

The package now in TW should be able to use both at once. Regardless if you use -z or -zz it should transmit. Requiring of course to have the new/old server on the other side.

(In reply to Tomáš Chvátal from comment #20)
> The 3.1.0 used only the "newcompress" while it broke the "oldcompress".
That is useless for me. I have clients with older versions that use the old compress.

> The package now in TW should be able to use both at once. Regardless if you
> use -z or -zz it should transmit. Requiring of course to have the new/old
> server on the other side.
Technically your comment is true for the client side, as it will ignore oldcompress and just make an uncompressed connection. But the server side will abort if it has no support for oldcompress.

I had hoped based on the last comments that the new rsync would support both old and new compress. But that is not true. The new release 2.2 is exactly the same as 2.1, except for the release number of course. Same spec file, same sources, same patches, same build instructions, same result.

Actually the new rsync supports both. I dunno what version and where from you grab it, but the tumbleweed version supports both compressions.

(In reply to Tomáš Chvátal from comment #22)
> Actually the new rsync supports both. I dunno what version and where from
> you grab it, but the tumbleweed version supports both compressions.
I had rsync-3.1.2-2.2 from the normal tumbleweed repositories. But now I did another update, and I got rsync-3.1.2-3.1 from the same repository, and it works as expected, thank you.

I just found out that it just seems to work, as in there are no error messages when invoking rsync with -z or -zz, but now a transfer doesn't work.

It seems that 128 Zero Bytes are inserted at the beginning. At least rsync catches the problem. The first transfer with -az deleted the file, the second with -aPz keeps the file so it can be examined. The rsync on the remote side is 3.0.7, rsync on the local side is tumbleweed rsync-3.1.2-3.1.
The reverse, getting files from the remote 3.0.7 seems to work.
See this simple test:

$ echo test > /tmp/file
$ hex /tmp/file
0000  74 65 73 74 0a                                    test.
$ rsync -az /tmp/file remote:/tmp
ERROR: file failed verification -- update discarded.
rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1178) [sender=3.1.2]
$ rsync -aPz /tmp/file remote:/tmp
sending incremental file list
file
              5 100%    0.00kB/s    0:00:00 (xfr#1, to-chk=0/1)
file
              5 100%    0.12kB/s    0:00:00 (xfr#2, to-chk=0/1)
ERROR: file failed verification -- update discarded.
rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1178) [sender=3.1.2]
$ ssh remote hex /tmp/file
0000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ........ ........
0010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ........ ........
0020  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ........ ........
0030  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ........ ........
0040  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ........ ........
0050  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ........ ........
0060  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ........ ........
0070  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  ........ ........
0080  74 65 73 74 0a                                    test.

(In reply to Ralf Friedl from comment #24)
> I just found out that it just seems to work, as in there are no error
> messages when invoking rsync with -z or -zz, but now a transfer doesn't work.
> 
> It seems that 128 Zero Bytes are inserted at the beginning. At least rsync
> catches the problem. The first transfer with -az deleted the file, the
> second with -aPz keeps the file so it can be examined. The rsync on the
> remote side is 3.0.7, rsync on the local side is tumbleweed rsync-3.1.2-3.1.
> The reverse, getting files from the remote 3.0.7 seems to work.


Thanks for debugging.

Anyway I didn't check the files results from the old->new.
You can try to download the testing package from:
https://build.opensuse.org/package/binaries/network/rsync?repository=openSUSE_Tumbleweed

If the version is higher than rsync-3.1.2-89.1.x86_64.rpm it should contain the fix.

This is an autogenerated message for OBS integration:
This bug (1046197) was mentioned in
https://build.opensuse.org/request/show/512490 Factory / rsync

(In reply to Tomáš Chvátal from comment #25)
> You can try to download the testing package from:
> https://build.opensuse.org/package/binaries/network/
> rsync?repository=openSUSE_Tumbleweed
I didn't try that, but from the normal tumbleweed repository I got rsync-3.1.2-4.1 and it works in both directions.

Ok I am closing the bug then :)
Open a new one if you find other issues.

(In reply to Tomáš Chvátal from comment #20)
> Michal is bit misleading here.
> 
> The 3.1.0 used only the "newcompress" 

It certainly does not. The compression in 3.1.0 and 3.1.1 is implemented differently.

> while it broke the "oldcompress".

Any testcase that is broken? Upstream says the intention is to implement the existing (old) compression using system zlib but the solution did not get much testing so it sure can be buggy.

This is an autogenerated message for OBS integration:
This bug (1046197) was mentioned in
https://build.opensuse.org/request/show/698102 15.1 / rsync