Handle IDMAP_BOTH in posix_acls.c
abartlet at samba.org
Sun May 6 00:43:32 MDT 2012
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 http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
More information about the samba-technical