Bug 24441

Btw, Jürgen, your patch seems to be for rsync-bpc-3.0.9.8

Should I have assigned this bug to that tool instead of to backuppc?

(In reply to Marja Van Waes from comment #1)
> Btw, Jürgen, your patch seems to be for rsync-bpc-3.0.9.8
> 
> Should I have assigned this bug to that tool instead of to backuppc?

The patch itself is for ./backuppc/bpc_dirOps.c
But when you uploaded it, that comment https://bugs.mageia.org/show_bug.cgi?id=24010#c10 spoke of rync-bpc, that confused me, sorry.

> Btw, Jürgen, your patch seems to be for rsync-bpc-3.0.9.8

Right, the problem is caused in rsync-bpc-3.0.9.8 - I only became conscious that backuppc uses its own rsync when I started digging into backuppc, but did not clarify this in my comments - extremely sorry.

> The patch itself is for ./backuppc/bpc_dirOps.c
> But when you uploaded it, that comment https://bugs.mageia.org
> /show_bug.cgi?id=24010#c10 spoke of rync-bpc, that confused me, sorry.

In the context of the diff file, "backuppc" is the name a directory in the source code - confusing

> Should I have assigned this bug to that tool instead of to backuppc?

The implementation of BackupPC is organized in 3 packages (backuppc, perl-BackupPC-XS, rsync-bpc). If somebody has a problem with backuppc and files a bug, he/she normally sees "a bug of backuppc" and will file the bug on backupp without knowing which component is at fault. I think that filing all bugs on "backuppc" is clearer and makes searching more efficient (perl-BackupPC-XS and rsync-bpc are, to my knowledge, not used outside the context of backuppc).

In the meantime, I have a new problem, backuppc stopped working - I guess that came with one of the recent updates of cauldron. I will try to get evidence starting with a clean install of Beta2 - helas, expensive in time; if necessary, I will submit a new bug.

The new problem was home-made - no problem

(In reply to Juergen Harms from comment #3)
> > Btw, Jürgen, your patch seems to be for rsync-bpc-3.0.9.8
> 
> Right, the problem is caused in rsync-bpc-3.0.9.8 - I only became conscious
> that backuppc uses its own rsync when I started digging into backuppc, but
> did not clarify this in my comments - extremely sorry.
> 
> > The patch itself is for ./backuppc/bpc_dirOps.c
> > But when you uploaded it, that comment https://bugs.mageia.org
> > /show_bug.cgi?id=24010#c10 spoke of rync-bpc, that confused me, sorry.
> 
> In the context of the diff file, "backuppc" is the name a directory in the
> source code - confusing
> 
> > Should I have assigned this bug to that tool instead of to backuppc?
> 
> The implementation of BackupPC is organized in 3 packages (backuppc,
> perl-BackupPC-XS, rsync-bpc). If somebody has a problem with backuppc and
> files a bug, he/she normally sees "a bug of backuppc" and will file the bug
> on backupp without knowing which component is at fault. I think that filing
> all bugs on "backuppc" is clearer and makes searching more efficient
> (perl-BackupPC-XS and rsync-bpc are, to my knowledge, not used outside the
> context of backuppc).
> 

Thanks for the explanation :-)

rsync-bpc is maintained by guillomovitch, I'll reassign this report to him.

(The patch can be found in attachment #10721 [details] )

Keywords: (none) => PATCH
Source RPM: backuppc-4.3.0-1.mga7 => rsync-bpc-3.0.9.8-3.mga7, backuppc-4.3.0-1.mga7
Assignee: shlomif => guillomovitch
Summary: backuppc creates backups in directories with world writeable permission => backuppc creates backups in directories with world writeable permission, patching rsync-bpc fixes that.

The patch hardcodes a specific permission scheme (0770) when creating directories, instead of changing the value of ACCESSPERMS (0777) in rsync.h. It would be simpler to just build with -DACCESSPERMS defined, with better consistency with other usage elsewhere. Anyway, I don't feel comfortable with both solution, and think the issue should be discussed upstream instead.

I may eventually report the issue myself on github (https://github.com/backuppc/rsync-bpc/issues), but I don't think I have much added value as intermediate, Juergen would probably be a better advocate for the problem.

Status: NEW => ASSIGNED

> The patch hardcodes a specific permission scheme (0770) when creating
> directories, instead of changing the value of ACCESSPERMS (0777) in rsync.h
> ...

That would be the patch I had originally considered - I then preferred using a local constant since this does not risk the patch to have undesired side-effects elsewhere (and since the patch is only meant as a temporary workaround and does not aim at permanently fixing the underlying bug).

If my reading of the code is right, the original problem is a problem of handling umask: umask is set by the server as configured by $Conf{UmaskMode} (027) last thing before calling the client. This mask is then read by the client when it starts. The client remembers the initial value, but than goes ahead and modifies umask whenever need arrives. I guess that the logic in bpc_dirOps of using the value of 0777 (ACCESSPERMS) is that the permissions of directories are set under the assumption that umask has the originally set value - which it does not. This is a problem to be tackled by upstream (a promising approach could be to temporarily restore the originally memorized value of umask while the creating of a directory is launched)

For Mageia-7 I see 3 alternatives:
1. Provide backuppc-4.3.0 as it comes from upstream, arguing that backing up into world-writable directories is no problem as long as these directories are below a top-level directory which does not have world-permission for traversal.
2. Release Mageia-7 with backuppc version-3 (directory permissions are OK there).
3. Patch backuppc-4.3.0 in a way that - without fixing the underlying bug - backuppc writes properly protected directories.

Rather than shouting "bug" I had wanted to be constructive; I have no preference whatever whether "my patch" or any other approach is used (the essential effort anyhow was to clarify what goes wrong and creates the bad permissions); but I am of the strong opinion that alternative (1) is not adequate.

Guillaume, what alternative do you prefer? If we opt for (3), and seeing the upstream responsiveness I think Mageia should have some workaround available. Having watched the backuppc ML for a long time, my impression is that upstream responsiveness is strongly varying, and depends to a large degree on the time Craig Barrat has available for BackupPC.

I just created an issue at github https://github.com/backuppc/rsync-bpc/issues/9; I had already made a post in the backuppc user ML (the most active platform for communications with site administrators of backuppc) 2 weeks ago  - no reaction

Yesterday (Sunday) evening rsync-bpc-3.1.2.0-1 arrived on the mirrors. I now upgraded from rysnc-bpc-3.0.9.8-3 to this new (and unpatched) version and manually  launched a backup operation: no more  world-writable directories have been created.

Much ado about nothing, problem apparently solved. I have to run away now and will have time for more checking only later, please leave the bug still open.

Looks perfect, OK to close

Fixed, as per comment #9.

Resolution: (none) => FIXED
Status: ASSIGNED => RESOLVED