Re: Unfortunate results from fake-super

2018-02-07 Thread Dave Gordon via rsync
On 06/02/18 19:03, Dave Gordon via rsync wrote:
[:snip:]
> 
> The other part of this problem is that it doesn't at present seem
> possible to satisfy all of three individually reasonable requirements of
> a backup system at the same time:
> 
> 1.The backup receiver (daemon) need not run as root.
> 2.The backup daemon should preserve all of the client's metadata.
> 3.The backup daemon should have control over modes, ownership, etc
>   of the backup files.
> 
> 1 & 2 can be satisfied today with the use of --fake-super, but even
> apart from the issue with 0777 symlinks, the modes of the backup files
> are based on those of the originals .. unless you use chmod.
> 
> 1 & 3 can be satisfied together by using --fake-super with --chmod on
> the receiving side or incoming-chmod in the daemon config. But this
> actually loses information as it affects not only the modes of the
> backup files, but also the state saved in user.rsync.%stat.
> 
> So my proposal would be for the receiving-side chmod to be applied (in
> fake mode) *after* user.rsync.%stat has been saved, and likewise after
> the conversion of symlinks (et al.) to plain files.
> 
> This would allow a daemon configuration containing both fake-super and
> an absolute setting for incoming-chmod to fulfil all three of the above.
> 
> The logical flow for a push-to-fake-super-daemon would then be:
> 1.client sends filespecs, applying any local --chmod on the way
> 2.daemon receives filespecs, does *not* tweak_mode at this stage
> 3.... data transfer as needed ...
> 4.daemon sets backup file attributes
> 4a.   in fake mode, daemon saves user.rsync.%stat, then applies
>   tweak_mode() to the local file. Daemon modes take precedence
>   as expected.
> 4b.   in non-fake mode, the daemon just applies the deferred
>   tweak_mode(). This may lose information, as at present
>   (because that's *also* useful).
> 
> The reverse flow (pull-from-fake-super-daemon) is:
> 1.daemon sends filespecs, getting modes from user.rsync.%stat
>   (daemon would apply outgoing-chmod here, but I don't think it
>   would be set in this configuration).
> 2.client receives filespecs, does *not* tweak_mode at this stage
> 3.... date transfer as needed ...
> 4.client sets local file attributes
> 4b.   (non-fake-mode) client applies any local --chmod
> 
> Any client-local --chmod is applied later in this flow than at present,
> but that should make no difference. Note that the client didn't need to
> know (and shouldn't be able to tell?) whether the daemon was super or
> just faking it :)
> 
> .Dave.

One more point for consideration: at present the set_stat_xattr()
creates the user.rsync.%stat xattr iff it would contain information
different from the state of the actual file. Thus, it's always created
for symlinks, devices, etc because they're stored as plain files; but
for plain files and directories, the xattr is created only if there's
some way in which the logical and actual objects differ (uid, gid,
permissions).

Presumably this is for reasons of economy, but it seems a little odd;
and there are (perhaps contrived) scenarios where this might lead to
confusion, information loss, and/or security breaches. For example:

Client A is backed up onto a non-root fake-super server B, which stores
all backups under its own uid/gid but leaves permissions unchanged. Then
every backup file will be tagged with a user.rsync.%stat xattr, *except*
for those owned by a user on A whose numerical uid/gid happen to match
those of the backup daemon on B. If the backups are now copied by an
administrator to server C, where the uid/gid of the backup daemon are
different from those on B, naturally the admin will adjust ownership
accordingly (c.f. rsync copy-ownership-by-username). Finally, if client
A is restored from the backups on C rather than B, one specific user's
files will be recreated with the wrong uid. This would be quite
baffling, especially as the local administrator on A might not know
anything about what the (remote) administrator of servers B & C has been
doing!

To avoid this and similar inconsistencies, I would like to completely
decouple the representation of the logical uid/gid/mode of the stored
object from the real uid/gid/mode of the filesystem object representing
it. This would simply mean removing the tests in set_stat_xattr() so
that the xattr is *always* created for every object stored in fake-super
mode. Since the purpose of fake-super mode is to preserve origin
filesystem metadata unchanged whether or not the target filesystem can
represent it, it seems odd to have the extra code that creates this
corner case. Without it, the code would be both simpler and easier to
explain :)

.Dave.


-- 
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: 

Re: Unfortunate results from fake-super

2018-02-06 Thread Dave Gordon via rsync
On 05/02/18 23:03, Dave Gordon via rsync wrote:
> On 05/02/18 05:53, Wayne Davison wrote:
>> On Sat, Feb 3, 2018 at 5:20 AM, Dave Gordon via rsync
>> > wrote:
>>
>> [...fake-super symlink saved as a file...]
>>
>> This results in the copy being world-writable.
>>
>> Indeed. The file initially gets created as a mode-600 file, but the code
>> later tweaks the permissions to match the symlink, which is (as you
>> note) a bad thing.
>>
>> My first reaction is to change the code in set_stat_xattr()
>> (in xattrs.c) from:
>>
>>        if (fst.st_mode != mode)
>>                do_chmod(fname, mode);
>>
>> to:
>>
>>        if (fst.st_mode != mode && !S_ISLNK(file->mode))
>>                do_chmod(fname, mode);
>>
>> ..wayne.. 
> 
> That's certainly an improvement; from the safety point of view, leaving
> it as 0600 is a lot better than leaving it as 0777. I'm currently
> investigating a slightly more extensive fix to allow more control over
> how fake-symlink files end up, also to make fake-super work better with
> incoming-chmod for the daemon case.
> 
> .Dave.

The other part of this problem is that it doesn't at present seem
possible to satisfy all of three individually reasonable requirements of
a backup system at the same time:

1.  The backup receiver (daemon) need not run as root.
2.  The backup daemon should preserve all of the client's metadata.
3.  The backup daemon should have control over modes, ownership, etc
of the backup files.

1 & 2 can be satisfied today with the use of --fake-super, but even
apart from the issue with 0777 symlinks, the modes of the backup files
are based on those of the originals .. unless you use chmod.

1 & 3 can be satisfied together by using --fake-super with --chmod on
the receiving side or incoming-chmod in the daemon config. But this
actually loses information as it affects not only the modes of the
backup files, but also the state saved in user.rsync.%stat.

So my proposal would be for the receiving-side chmod to be applied (in
fake mode) *after* user.rsync.%stat has been saved, and likewise after
the conversion of symlinks (et al.) to plain files.

This would allow a daemon configuration containing both fake-super and
an absolute setting for incoming-chmod to fulfil all three of the above.

The logical flow for a push-to-fake-super-daemon would then be:
1.  client sends filespecs, applying any local --chmod on the way
2.  daemon receives filespecs, does *not* tweak_mode at this stage
3.  ... data transfer as needed ...
4.  daemon sets backup file attributes
4a. in fake mode, daemon saves user.rsync.%stat, then applies
tweak_mode() to the local file. Daemon modes take precedence
as expected.
4b. in non-fake mode, the daemon just applies the deferred
tweak_mode(). This may lose information, as at present
(because that's *also* useful).

The reverse flow (pull-from-fake-super-daemon) is:
1.  daemon sends filespecs, getting modes from user.rsync.%stat
(daemon would apply outgoing-chmod here, but I don't think it
would be set in this configuration).
2.  client receives filespecs, does *not* tweak_mode at this stage
3.  ... date transfer as needed ...
4.  client sets local file attributes
4b. (non-fake-mode) client applies any local --chmod

Any client-local --chmod is applied later in this flow than at present,
but that should make no difference. Note that the client didn't need to
know (and shouldn't be able to tell?) whether the daemon was super or
just faking it :)

.Dave.


-- 
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html


Re: Unfortunate results from fake-super

2018-02-05 Thread Dave Gordon via rsync
On 05/02/18 05:53, Wayne Davison wrote:
> On Sat, Feb 3, 2018 at 5:20 AM, Dave Gordon via rsync
> > wrote:
> 
> [...fake-super symlink saved as a file...]
> 
> This results in the copy being world-writable.
> 
> Indeed. The file initially gets created as a mode-600 file, but the code
> later tweaks the permissions to match the symlink, which is (as you
> note) a bad thing.
> 
> My first reaction is to change the code in set_stat_xattr()
> (in xattrs.c) from:
> 
>        if (fst.st_mode != mode)
>                do_chmod(fname, mode);
> 
> to:
> 
>        if (fst.st_mode != mode && !S_ISLNK(file->mode))
>                do_chmod(fname, mode);
> 
> ..wayne.. 

That's certainly an improvement; from the safety point of view, leaving
it as 0600 is a lot better than leaving it as 0777. I'm currently
investigating a slightly more extensive fix to allow more control over
how fake-symlink files end up, also to make fake-super work better with
incoming-chmod for the daemon case.

.Dave.


-- 
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html


Re: Unfortunate results from fake-super

2018-02-04 Thread Wayne Davison via rsync
On Sat, Feb 3, 2018 at 5:20 AM, Dave Gordon via rsync  wrote:

> [...fake-super symlink saved as a file...]

This results in the copy being world-writable.
>

Indeed. The file initially gets created as a mode-600 file, but the code
later tweaks the permissions to match the symlink, which is (as you note) a
bad thing.

My first reaction is to change the code in set_stat_xattr() (in xattrs.c)
from:

   if (fst.st_mode != mode)
   do_chmod(fname, mode);

to:

   if (fst.st_mode != mode && !S_ISLNK(file->mode))
   do_chmod(fname, mode);

..wayne..
-- 
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html

Re: Unfortunate results from fake-super

2018-02-03 Thread Dave Gordon via rsync
On 03/02/18 15:52, Dave Gordon via rsync wrote:
> On 03/02/18 13:20, Dave Gordon via rsync wrote:
>> When using fake-super mode in an rsync receiver, anything that's neither a
>> file nor a directory (e.g. devices, symlinks, etc) is converted into a file,
>> and properties such as original ownership, filetype, and permissions are
>> stored in a specific extended attribute.
>>
>> In the case of a symlink, the contents of the link are stored in a plain
>> file. The original mode of the symlink is normally irrelevant, because
>> (Linux) hosts ignore a symlink's mode and use that of the target instead.
>> But in fake-super mode, the original mode of the link itself (usually
>> 0120777) is used to set the permissions on the receiver's plain-file copy.
>>
>> This results in the copy being world-writable. If this plain file is altered
>> and then transferred back to the origin, the resulting symlink can point to
>> an arbitrary path, which leads to potential security issues.
>>
>> Example:
>>
>> This was first observed in version 3.1.1 on kubuntu, but is still the same
>> in version 3.1.3 as of 28 Jan 2018.
>> See also  Storing-ownership-device-nodes-without-root
>> 
>>   
>>
>> .Dave.
>>
>> --
>> Sent from: http://samba.2283325.n4.nabble.com/Samba-rsync-f2500462.html
>>
> 
> Hmm, the forum seems to have dropped the example (which was in 'raw'
> tags to preserve the formatting). Oh well, here it is ...
> 
> Example:
> 
> As regular user 'dg':
> dg$ mkdir src
> dg$ touch src/foo
> dg$ chmod 444 src/foo
> dg$ ln -s foo src/bar
> 
> Then as trusted (but unprivileged) user 'backup':
> backup$ id -a
> uid=1000(backup) gid=1000(backup) groups=1000(backup),100(users)
> backup$ umask
> 0022
> backup$ rsync -av src/ dst-nonsuper/
> backup$ rsync -av --fake-super src/ dst-fake-super/
> 
> Now as (untrusted) user 'guest':
> guest$ id -a
> uid=(guest) gid=(guest) groups=(guest)
> guest$ echo -n '/etc/shadow' > dst-nonsuper/bar
> bash: dst-nonsuper/bar: Permission denied
> guest$ echo -n '/etc/secret' > dst-fake-super/bar # allowed!!
> 
> And finally, as user 'dg', restore from the fake-super backup:
> dg$ rsync -av --fake-super -M--super dst-fake-super/ recovered/
> dg$ ls -laR */
> ./src:
> total 8
> drwxr-xr-x 2 dg users 4096 Feb  3 11:59 .
> drwxrwxr-x 6 dg users 4096 Feb  3 13:00 ..
> lrwxrwxrwx 1 dg users3 Feb  3 11:59 bar -> foo
> -r--r--r-- 1 dg users0 Feb  3 11:59 foo
> 
> ./dst-nonsuper:
> total 8
> drwxr-xr-x 2 backup users 4096 Feb  3 11:59 .
> drwxrwxr-x 6 dg users 4096 Feb  3 13:00 ..
> lrwxrwxrwx 1 backup users3 Feb  3 11:59 bar -> foo
> -r--r--r-- 1 backup users0 Feb  3 11:59 foo
> 
> ./dst-fake-super:
> total 12
> drwxr-xr-x 2 backup backup 4096 Feb  3 11:59 .
> drwxrwxr-x 6 dg users  4096 Feb  3 13:00 ..
> -rwxrwxrwx 1 backup backup   11 Feb  3 12:51 bar
> -rw-r--r-- 1 backup backup0 Feb  3 11:59 foo
> 
> ./recovered:
> total 8
> drwxr-xr-x 2 dg users 4096 Feb  3 11:59 .
> drwxrwxr-x 6 dg users 4096 Feb  3 13:00 ..
> lrwxrwxrwx 1 dg users   11 Feb  3 12:51 bar -> /etc/secret
> -r--r--r-- 1 dg users0 Feb  3 11:59 foo
> 
> dg$ getfattr -d -R */
> 
> # file: dst-fake-super
> user.rsync.%stat="40755 0,0 32768:100"
> 
> # file: dst-fake-super/bar
> user.rsync.%stat="120777 0,0 32768:100"
> 
> # file: dst-fake-super/foo
> user.rsync.%stat="100444 0,0 32768:100"
> 
> .Dave.

In an attempt to avoid the above problem, one might try using a --chmod
option to ensure that the destination files are not world-writable (here
we use a rather silly chmod, just to make it more obvious what is -- and
isn't -- affected):


backup$ rsync -av src/ dst-nonsuper/
backup$ rsync -av --chmod=D0700,Fu=rx,g=,o=x src/ dst-chmod-501/
backup$ rsync -av --fake-super --chmod=D0700,Fu=rx,g=,o=x src/
dst-fake-chmod-501/
backup$ getfattr -d -R .
# file: dst-fake-chmod-501
user.rsync.%stat="40701 0,0 32768:100"

# file: dst-fake-chmod-501/bar
user.rsync.%stat="120777 0,0 32768:100"

# file: dst-fake-chmod-501/foo
user.rsync.%stat="100501 0,0 32768:100"

backup$ ll -R */
./src:
total 8
drwxr-xr-x 2 dg users 4096 Feb  3 11:59 ./
drwxrwxr-x 6 dg users 4096 Feb  3 17:16 ../
lrwxrwxrwx 1 dg users3 Feb  3 11:59 bar -> foo
-r--r--r-- 1 dg users0 Feb  3 11:59 foo

./dst-nonsuper:
total 8
drwxr-xr-x 2 backup users 4096 Feb  3 11:59 ./
drwxrwxr-x 6 dg users 4096 Feb  3 17:16 ../
lrwxrwxrwx 1 backup users3 Feb  3 11:59 bar -> foo
-r--r--r-- 1 backup users0 Feb  3 11:59 foo

./dst-chmod-501:
total 8
drwx-x 2 backup users 4096 Feb  3 11:59 ./
drwxrwxr-x 6 dg users 4096 Feb  3 17:16 ../
lrwxrwxrwx 1 backup users3 Feb  3 11:59 bar -> foo*
-r-x-x 1 backup users0 Feb  3 11:59 foo*

./dst-fake-chmod-501:
total 12
drwx-x 2 backup backup  4096 Feb  3 11:59 ./
drwxrwxr-x 6 dg users   4096 Feb  3 17:16 ../
-rwxrwxrwx 1 backup backup 3 Feb  3 11:59 bar*
-rwx-x 1 

Re: Unfortunate results from fake-super

2018-02-03 Thread Dave Gordon via rsync
On 03/02/18 13:20, Dave Gordon via rsync wrote:
> When using fake-super mode in an rsync receiver, anything that's neither a
> file nor a directory (e.g. devices, symlinks, etc) is converted into a file,
> and properties such as original ownership, filetype, and permissions are
> stored in a specific extended attribute.
> 
> In the case of a symlink, the contents of the link are stored in a plain
> file. The original mode of the symlink is normally irrelevant, because
> (Linux) hosts ignore a symlink's mode and use that of the target instead.
> But in fake-super mode, the original mode of the link itself (usually
> 0120777) is used to set the permissions on the receiver's plain-file copy.
> 
> This results in the copy being world-writable. If this plain file is altered
> and then transferred back to the origin, the resulting symlink can point to
> an arbitrary path, which leads to potential security issues.
> 
> Example:
> 
> This was first observed in version 3.1.1 on kubuntu, but is still the same
> in version 3.1.3 as of 28 Jan 2018.
> See also  Storing-ownership-device-nodes-without-root
> 
>   
> 
> .Dave.
> 
> --
> Sent from: http://samba.2283325.n4.nabble.com/Samba-rsync-f2500462.html
> 

Hmm, the forum seems to have dropped the example (which was in 'raw'
tags to preserve the formatting). Oh well, here it is ...

Example:

As regular user 'dg':
dg$ mkdir src
dg$ touch src/foo
dg$ chmod 444 src/foo
dg$ ln -s foo src/bar

Then as trusted (but unprivileged) user 'backup':
backup$ id -a
uid=1000(backup) gid=1000(backup) groups=1000(backup),100(users)
backup$ umask
0022
backup$ rsync -av src/ dst-nonsuper/
backup$ rsync -av --fake-super src/ dst-fake-super/

Now as (untrusted) user 'guest':
guest$ id -a
uid=(guest) gid=(guest) groups=(guest)
guest$ echo -n '/etc/shadow' > dst-nonsuper/bar
bash: dst-nonsuper/bar: Permission denied
guest$ echo -n '/etc/secret' > dst-fake-super/bar # allowed!!

And finally, as user 'dg', restore from the fake-super backup:
dg$ rsync -av --fake-super -M--super dst-fake-super/ recovered/
dg$ ls -laR */
./src:
total 8
drwxr-xr-x 2 dg users 4096 Feb  3 11:59 .
drwxrwxr-x 6 dg users 4096 Feb  3 13:00 ..
lrwxrwxrwx 1 dg users3 Feb  3 11:59 bar -> foo
-r--r--r-- 1 dg users0 Feb  3 11:59 foo

./dst-nonsuper:
total 8
drwxr-xr-x 2 backup users 4096 Feb  3 11:59 .
drwxrwxr-x 6 dg users 4096 Feb  3 13:00 ..
lrwxrwxrwx 1 backup users3 Feb  3 11:59 bar -> foo
-r--r--r-- 1 backup users0 Feb  3 11:59 foo

./dst-fake-super:
total 12
drwxr-xr-x 2 backup backup 4096 Feb  3 11:59 .
drwxrwxr-x 6 dg users  4096 Feb  3 13:00 ..
-rwxrwxrwx 1 backup backup   11 Feb  3 12:51 bar
-rw-r--r-- 1 backup backup0 Feb  3 11:59 foo

./recovered:
total 8
drwxr-xr-x 2 dg users 4096 Feb  3 11:59 .
drwxrwxr-x 6 dg users 4096 Feb  3 13:00 ..
lrwxrwxrwx 1 dg users   11 Feb  3 12:51 bar -> /etc/shadow
-r--r--r-- 1 dg users0 Feb  3 11:59 foo

dg$ getfattr -d -R */

# file: dst-fake-super
user.rsync.%stat="40755 0,0 32768:100"

# file: dst-fake-super/bar
user.rsync.%stat="120777 0,0 32768:100"

# file: dst-fake-super/foo
user.rsync.%stat="100444 0,0 32768:100"

.Dave.


-- 
Please use reply-all for most replies to avoid omitting the mailing list.
To unsubscribe or change options: https://lists.samba.org/mailman/listinfo/rsync
Before posting, read: http://www.catb.org/~esr/faqs/smart-questions.html