Bug 5448 - Daemon parameter to prevent attribute tweaking
Summary: Daemon parameter to prevent attribute tweaking
Status: NEW
Alias: None
Product: rsync
Classification: Unclassified
Component: core (show other bugs)
Version: 3.0.2
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---
Assignee: Wayne Davison
QA Contact: Rsync QA Contact
URL:
Keywords:
Depends on: 4561
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-07 20:19 UTC by Carl E. Thompson
Modified: 2010-02-07 17:53 UTC (History)
1 user (show)

See Also:


Attachments
A patch to correct this behavior (5.49 KB, patch)
2008-05-07 20:23 UTC, Carl E. Thompson
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Carl E. Thompson 2008-05-07 20:19:10 UTC
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).
Comment 1 Carl E. Thompson 2008-05-07 20:23:13 UTC
Created attachment 3280 [details]
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.
Comment 2 Matt McCutchen 2008-05-07 22:43:38 UTC
(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
Comment 3 Carl E. Thompson 2008-05-08 08:14:14 UTC
(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.
Comment 4 Matt McCutchen 2008-05-08 12:46:44 UTC
(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).
Comment 5 Carl E. Thompson 2008-05-08 15:07:28 UTC
(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.
Comment 6 Matt McCutchen 2008-05-08 20:44:43 UTC
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