rsync - May 2008 - DO NOT REPLY [Bug 5448] New: rsync modifies files in place even without --inplace specified

https://bugzilla.samba.org/show_bug.cgi?id=5448

           Summary: rsync modifies files in place even without --inplace
                    specified
           Product: rsync
           Version: 3.0.2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: major
          Priority: P3
         Component: core
        AssignedTo: wayned@samba.org
        ReportedBy: sites-samba@carlthompson.net
         QAContact: rsync-qa@samba.org


rsync always modify files in place to reflect changes in permissions, ownership
or group without regard to the "--inplace" option. The
"--inplace" option is
supposed to regulate whether files to be backed up that are already found in
the destination directory are modified in place or simply used as the basis for
new files. Not being able to completely eliminate modifying files in place
greatly and negatively affects a very common use of rsync: backup systems that
create incremental backup snapshots using hard links. In such systems the
current behavior of rsync makes it a trivial exercise for a misconfigured or
compromised / malicious client to seriously damage or destroy all current and
prior backup snapshots in the set. The "--link-dest" option attempts
to solve
the problem but adds serious problems of its own. Some of the problems I see
are:

1. The hard links and permission / owner / group change problem: This is a
problem that has been well known for many years. When using hard links to
implement snapshots (as most incremental snapshot backup systems that utilize
rsync do) when files are modified in place (as rsync always does for
permission, owner and group changes without any regard for the
"--inplace"
option) those changes affect not just those files in the current backup but
also in all previous backups (since the last time the contents of the file were
changed). So if for example a client machine is compromised and the attacker
executes the command "chmod -R 666 /" the next time the rsync backup
is
performed all of the backup snapshots become unreliable. This can also happen
if the filesystem is accidentally backed up and restored locally using a medium
that does not support permissions.

2. Problems with the "--link-dest" option: The "--link-dest"
option was
apparently added to rsync to address the above problem but it does so in a way
that introduces vulnerabilities of its own.
2a. A big problem with "--link-dest" is that it requires that the
client
machine be correctly configured to work. There is no way to enforce a correct
"--link-dest" option on the server. Not only does the client need to
specify
the option but also its use of the option needs to exactly match what the
rotation / renaming scripts on the server expect or not-so-wonderful things
will happen (you can use your imagination). Configuration mistakes or malicious
users can wreak all sorts of havoc by omitting or playing with this option. Any
backup solution that requires the server to trust that the client is properly
configured in order for the server to work correctly is not reliable.
2b. Another big problem with "--link-dest" is that when used as
documented and
intended there is only one copy of the previous backup and it cannot be
protected from the forked daemon by chroot!  The daemon must have direct access
to the one and only good copy of your data so a bug, exploit or malicious
client can easily clobber it. Any backup solution that requires the server to
trust that the client is properly configured in order to protect the integrity
of previous backups is not reliable. On the other hand if the linked copy of
the backup is created after the rsync daemon has finished (i.e., not using
"--link-dest") your previous backups can be protected from the daemon
by chroot
with only a copy of the data acted upon by the daemon.
2c. Lots of other problems and confusion related to "--link-dest" .
(See
Bugzilla).


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5448





------- Comment #1 from sites-samba@carlthompson.net  2008-05-07 20:23 CST
-------
Created an attachment (id=3280)
 --> (https://bugzilla.samba.org/attachment.cgi?id=3280&action=view)
A patch to correct this behavior

Attached is a patch to version 3.0.2 to correct this behavior. This patch
causes rsync to honor the absence of the "--inplace" option for
permission,
owner and group changes. This patch is a greatly simplified version of Matt
McCutchen's excellent "tweak-opts" patch found in his git
repository
(http://mattmccutchen.net/rsync/rsync.git/?a=shortlog;h=refs/heads/tweak-opts).
However this patch does not add any new options.

It's my opinion that this simple patch makes any rsync-based backup system
that
uses hard links (as so many do) more reliable and robust and significantly
helps maintain the integrity of the backups. It does not require any changes to
the configuration or use of the clients, server or backup system to achieve
this benefit.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5448


matt@mattmccutchen.net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|major                       |enhancement
            Summary|rsync modifies files in     |Daemon parameter to prevent
                   |place even without --inplace|attribute tweaking
                   |specified                   |




------- Comment #2 from matt@mattmccutchen.net  2008-05-07 22:43 CST -------
(In reply to comment #0)
> rsync always modify files in place to reflect changes in permissions,
ownership
> or group without regard to the "--inplace" option.
This behavior is not a bug per se: it is documented in the man page in the
second paragraph under DESCRIPTION.
> Not being able to completely eliminate modifying files in place
> greatly and negatively affects a very common use of rsync: backup systems
that
> create incremental backup snapshots using hard links.
I'm not sure it would be wise to change the default behavior to --no-tweak,
but
it would certainly be useful to add a daemon configuration parameter to force
--no-tweak mode; I'll convert this bug to an enhancement request for such a
parameter.
> The "--link-dest" option attempts to solve
> the problem but adds serious problems of its own. Some of the problems I
see
> are:
> 
> 1. The hard links and permission / owner / group change problem:
Right; this would be solved by --no-tweak or a corresponding daemon parameter.
> 2a. A big problem with "--link-dest" is that it requires that the
client
> machine be correctly configured to work.
I have entered bug 5449 for the ability to specify a link-dest dir on a daemon
in a way that is transparent to the client.
> 2b. Another big problem with "--link-dest" is that when used as
documented and
> intended there is only one copy of the previous backup and it cannot be
> protected from the forked daemon by chroot!  The daemon must have direct
access
> to the one and only good copy of your data so a bug, exploit or malicious
> client can easily clobber it.
If you want to keep only one copy of the previous backup, clearly there's no
way the daemon can --link-dest from it unless it is inside the chroot; nothing
can be done about this.  Thus, if you are worried about bugs in the daemon, you
will have to keep a second copy just for the daemon, as you mention.

However, a bug-free daemon with the enhancements that we're discussing could
be
configured so it would not harm the previous backup no matter what the client
does.  Specifically: the daemon link-dest parameter I'm envisioning would
let
you locate the previous backup outside the module (but of course inside the
chroot).  With that arrangement and symlink munging, the client can't write
to
the previous backup directly, and with a no-tweak parameter, the client
can't
write via existing hard links in the module in the event that a backup is
interrupted and restarted.
> 2c. Lots of other problems and confusion related to "--link-dest"
. (See
> Bugzilla).
I'm not sure why you mention this here, because if the bugs are in Bugzilla,
that means that we already know about them (or they are fixed).  By the way,
the link in your email matched a lot of bugs that have nothing to do with
--link-dest.  Here's mine:

https://bugzilla.samba.org/buglist.cgi?query_format=advanced&long_desc_type=substring&long_desc=--link-dest


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5448


sites-samba@carlthompson.net changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|4561                        |




------- Comment #3 from sites-samba@carlthompson.net  2008-05-08 08:14 CST
-------
(In reply to comment #2)
> (In reply to comment #0)
> ...
> > Not being able to completely eliminate modifying files in place
> > greatly and negatively affects a very common use of rsync: backup
systems that
> > create incremental backup snapshots using hard links.
> 
> I'm not sure it would be wise to change the default behavior to
--no-tweak, but
> it would certainly be useful to add a daemon configuration parameter to
force
> --no-tweak mode; I'll convert this bug to an enhancement request for
such a
> parameter.
The reason why I think it's a good idea to change the default behavior is
because doing so would "fix" current backup solutions that are out
there
without any modifications. I also think it is a much more sane default and
can't think of any cases where it would cause undesired results. (But of
just
because I can't think of any doesn't mean they don't exist!)
> > The "--link-dest" option attempts to solve
> > the problem but adds serious problems of its own. Some of the problems
I see
> > are:
> > 
> > 1. The hard links and permission / owner / group change problem:
> 
> Right; this would be solved by --no-tweak or a corresponding daemon
parameter.
> 
> > 2a. A big problem with "--link-dest" is that it requires
that the client
> > machine be correctly configured to work.
> 
> I have entered bug 5449 for the ability to specify a link-dest dir on a
daemon
> in a way that is transparent to the client.
But of course these require changes to the use of the client and daemon and I
think it's a good idea to avoid that. It also is still vulnerable to the
chroot
problem below.
> > 2b. Another big problem with "--link-dest" is that when used
as documented and
> > intended there is only one copy of the previous backup and it cannot
be
> > protected from the forked daemon by chroot!  The daemon must have
direct access
> > to the one and only good copy of your data so a bug, exploit or
malicious
> > client can easily clobber it.
> 
> If you want to keep only one copy of the previous backup, clearly
there's no
> way the daemon can --link-dest from it unless it is inside the chroot;
nothing
> can be done about this.  Thus, if you are worried about bugs in the daemon,
you
> will have to keep a second copy just for the daemon, as you mention.
Exactly. But if you are keeping a second copy there is no point in using
"--link-dest" because you are already doing a "cp -al" to
make the copy.
> However, a bug-free daemon with the enhancements that we're discussing
could be
> configured so it would not harm the previous backup no matter what the
client
> does.  Specifically: the daemon link-dest parameter I'm envisioning
would let
> you locate the previous backup outside the module (but of course inside the
> chroot).  With that arrangement and symlink munging, the client can't
write to
> the previous backup directly, and with a no-tweak parameter, the client
can't
> write via existing hard links in the module in the event that a backup is
> interrupted and restarted.
The client may not not be able to write to the previous backup but a buggy or
exploited forked daemon could. So I don't think this is a good a solution
and
is more complex.
> ...
Thanks, Carl.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5448





------- Comment #4 from matt@mattmccutchen.net  2008-05-08 12:46 CST -------
(In reply to comment #3)
> The reason why I think it's a good idea to change the default behavior
is
> because doing so would "fix" current backup solutions that are
out there
> without any modifications.
I am very wary of trying to second-guess users and administrators like this. 
(Here, let me "fix" your computer...)  A user who changes the
permissions on a
collection of large files and then mirrors it somewhere is not going to be
happy about the job taking several times as long as usual.  We can certainly
try to raise awareness about the tweaking problem (perhaps Wayne could be
persuaded to send something to rsync-announce), but I think forcing a change to
a long-established default on people is not called for.
> > > 2a. A big problem with "--link-dest" is that it
requires that the client
> > > machine be correctly configured to work.
> > 
> > I have entered bug 5449 for the ability to specify a link-dest dir on
a daemon
> > in a way that is transparent to the client.
> 
> But of course these require changes to the use of the client and daemon and
I
> think it's a good idea to avoid that.
No, only the daemon configuration needs to be changed; the client is not
involved at all.  Bug 5449 simply requests the "way to enforce a correct
"--link-dest" option on the server" that you originally
mentioned.
> > If you want to keep only one copy of the previous backup, clearly
there's no
> > way the daemon can --link-dest from it unless it is inside the chroot;
nothing
> > can be done about this.  Thus, if you are worried about bugs in the
daemon, you
> > will have to keep a second copy just for the daemon, as you mention.
> 
> Exactly. But if you are keeping a second copy there is no point in using
> "--link-dest" because you are already doing a "cp -al"
to make the copy.
I don't know why one would use "cp -al".  I was thinking that the
client would
upload to the module and then the post-xfer script would copy the module
contents to a backup set elsewhere using --link-dest, just as if the module
were the original source.
> The client may not not be able to write to the previous backup but a buggy
or
> exploited forked daemon could. So I don't think this is a good a
solution and
> is more complex.
As I said, if you do not want to trust a properly configured daemon with direct
access to the previous backup, your alternative is to use a second copy.  If
you have a better idea, I would like to hear it (preferably on the list or in a
separate enhancement request).


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5448





------- Comment #5 from sites-samba@carlthompson.net  2008-05-08 15:07 CST
-------
(In reply to comment #4)
> (In reply to comment #3)
> > The reason why I think it's a good idea to change the default
behavior is
> > because doing so would "fix" current backup solutions that
are out there
> > without any modifications.
> 
> I am very wary of trying to second-guess users and administrators like
this.
> (Here, let me "fix" your computer...)
I understand that but I think the reality is that the current default is
behavior that is unwanted in real-life and causes serious security
vulnerabilities.
> A user who changes the permissions on a
> collection of large files and then mirrors it somewhere is not going to be
> happy about the job taking several times as long as usual.
This is an edge case where things would be a little slower. But this is
actually an argument _for_ my idea because this user's previous backups
would
be broken by the current behavior. If the user really wants the files modified
in place regardless of the risk it makes sense to me for the user to explicitly
request that by using the "--inplace" option.
> We can certainly
> try to raise awareness about the tweaking problem (perhaps Wayne could be
> persuaded to send something to rsync-announce), but I think forcing a
change to
> a long-established default on people is not called for.
Normally I would agree with you but I have pointed out multiple cases where the
current behavior causes problems and can compromise the integrity of every
backup in the set making the entire set unusable. To me, the ability of a
malicious user on a compromised host being able to easily make all of the
backups on the server unusable is a pretty strong argument for changing the
default. Further, I can't think of a use case where the change would produce
an
undesired end result. If you can think of such a real-world case then of course
it would be different.

I think it's best to look at it as a bug / security fix.
> ...
> > Exactly. But if you are keeping a second copy there is no point in
using
> > "--link-dest" because you are already doing a "cp
-al" to make the copy.
> 
> I don't know why one would use "cp -al".  I was thinking that
the client would
> upload to the module and then the post-xfer script would copy the module
> contents to a backup set elsewhere using --link-dest, just as if the module
> were the original source.
Because I think that one "rsync" run and one "cp -al" copy
would be faster than
two "rsync" runs.
> > The client may not not be able to write to the previous backup but a
buggy or
> > exploited forked daemon could. So I don't think this is a good a
solution and
> > is more complex.
> 
> As I said, if you do not want to trust a properly configured daemon with
direct
> access to the previous backup, your alternative is to use a second copy. 
If
> you have a better idea, I would like to hear it (preferably on the list or
in a
> separate enhancement request).
_This is_ my better idea! Fix rsync to not modify files in place without
"--inplace" and never use "--link-dest" but use server side
scripts intead.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=5448





------- Comment #6 from matt@mattmccutchen.net  2008-05-08 20:44 CST -------
Let's continue the discussion on the list:

http://lists.samba.org/archive/rsync/2008-May/020829.html
http://lists.samba.org/archive/rsync/2008-May/020830.html


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.