ACL issue w/Vista backup

[Joe Meadows]

Hi,

One of our QA guys found a problem using Windows Vista's built in backup 
utility with Samba as the backup destination.  I've got a solution that 
I am posting for sanity checking and to submit if it is correct and 
useful.  Here's the issue:

My setup:
Windows Vista and Samba 3.0.23d (running on FC1, RH9 and SUSE 10 systems 
all have the same symptom, but FC6 did not show this problem)

To reproduce problem:
Use Vista backup and specify a Samba share as the destination.  Backup 
attempts to create a directory (eg. 
{653AA951-BE0F-436B-B5D5-FEF6AD150E6E}) on the target share and you will 
get an error message telling you that it already exists.

Analysis:
What is happening is that Samba successfully creates the directory but 
fails when trying to set the ACL specified in the create request, 
returning an NT_STATUS_ACCESS_DENIED error.  Backup retries the request 
with the create flags set to 'fail if exists', which explains the 
somewhat misleading error message.

What is happening is that backup is trying to set an ACL on this 
directory and some of the ACEs map to the same user.  On my setup the 
owner gets represented at least twice, once by it's own SID and again by 
the SID S-1-3-0 (well known SID 'Creator Owner').  This will lead to 
illegal multiple entries for of type SMB_ACL_USER_OBJ or SMB_ACL_USER 
causing the ACL to be rejected by the kernel when the call is made to 
set it.

EXAMPLE from log.smbd:
[2007/01/30 16:43:06, 10] smbd/posix_acls.c:set_canon_ace_list(3456)
  canon_ace index 0. Type = allow SID = S-1-1-0 other SMB_ACL_OTHER 
perms ------
  canon_ace index 1. Type = allow SID = S-1-5-32-549 gid 100 
(AllLocalUsers) SMB_ACL_GROUP_OBJ perms ------
  canon_ace index 2. Type = allow SID = S-1-3-0 uid 1 (admin) 
SMB_ACL_USER_OBJ perms rwxdpo
  canon_ace index 3. Type = allow SID = 
S-1-5-21-3748822599-1304825643-849436012-1002 uid 1 (admin) 
SMB_ACL_USER_OBJ perms rwxdpo


The solution that I found was to modify slightly the function merge_aces 
which originally merged ACEs based on a SID match.  I expanded the 
matching to also match the unix id as follows:

Original code:
    if (sid_equal(&curr_ace->trustee, &curr_ace_outer->trustee) &&
                (curr_ace->attr == curr_ace_outer->attr)) {

Modified code:
    if ( (sid_equal(&curr_ace->trustee, &curr_ace_outer->trustee) || 
(curr_ace->unix_ug.uid == curr_ace_outer->unix_ug.uid)) &&
                (curr_ace->attr == curr_ace_outer->attr)) {


Note the additional test that checks if the unix id is a match.

Results:
With this change in place I am able to backup successfully.  I hope this 
fix is correct, but if not maybe the information above will give someone 
else a head start in solving this correctly.  I reproduced this failure 
on three different linux distros but not on FC6 (I suspect that FC6 may 
be more forgiving when it comes to setting the ACL) so if you are not 
able to reproduce this problem it may be because the system you are 
running Samba on is also willing to overlook this.

I think I've got all the important details here, but if there is some 
other information needed please let me know.

Cheers!
Joe