Handle IDMAP_BOTH in posix_acls.c

[Andrew Bartlett]

On Fri, 2012-05-04 at 21:07 +0200, Christian Ambach wrote:
> On 05/04/2012 04:17 AM, Andrew Bartlett wrote:
> 
> > I've looked at this, and I have three concerns:
> >   - How is this code tested
> >   - What is the purpose of the sidmap?
> 
> I think the sidmap was initially added to give users the possibility to 
> define that a certain SID should better be replaced by another one when 
> compiling the ACL. I am not aware of anyone actually using the function 
> and I have troubles remembering why this functionality was added five 
> years ago. I have crawled my mail archives without success.
> 
> sidmap was added to the code in the process of syncing Tridge's 
> samba-ctdb bzr tree to Samba, so the history is lost as the bzr repo is 
> not accessible any more. Maybe someone still has a copy around?
> 
> Maybe it was meant to be able to deal with SIDs which cannot be mapped 
> for whatever reason (e.g. somebody copies file from its local machine to 
> the share and the ACLs contain SIDs of the local machine).
> 
> >   - What is the purpose of the lookup_sid() call?
> I would guess this relates to the sidmapping functionality above: if the 
> SID cannot be found, then check if there is a replacement to use defined 
> in the sidmap database.
> 
> > On the last point, it seems we have a potentially quite inefficient
> > lookup_sid call in nfs4_acls.c:smbacl4_fill_ace4().  This is called for
> > every ACE on every ACL set.
> >
> > I'm hoping this was just added out of paranoia, but it seems to be tied
> > in to the sidmap.  In any case the posix ACL code doesn't do this, and
> > it would work best if we remove it to support IDMAP_BOTH (but, due to
> > doing this lookup, it also means that idmap both will simply be ignored
> > for nfs4 in the interim).
> 
> I would vote to remove the whole sidmap code. I do not think this has 
> seen real use yet and usecases for it seem to be very rare.
> It's also not documented in README.nfs4acls.txt

Removing the sidmap, and the lookup_sids calls would be great, and make
this much easier to patch.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org