[PATCH] nfs4acl: Fix owner mapping with ID_TYPE_BOTH
Andrew Bartlett
abartlet at samba.org
Fri Sep 16 18:05:05 UTC 2016
On Fri, 2016-09-16 at 10:45 -0700, Christof Schmitt wrote:
> On Sat, Sep 17, 2016 at 05:28:34AM +1200, Andrew Bartlett wrote:
> >
> > On Mon, 2016-09-12 at 17:34 -0700, Christof Schmitt wrote:
> > >
> > > From f1883adf6ed027a03be2a3d4f1631d8bdb283e38 Mon Sep 17 00:00:00
> > > 2001
> > > From: Christof Schmitt <cs at samba.org>
> > > Date: Mon, 12 Sep 2016 16:22:16 -0700
> > > Subject: [PATCH] nfs4acl: Fix owner mapping with ID_TYPE_BOTH
> > >
> > > This fixes a corner case when using NFS4 ACLs with
> > > ID_TYPE_BOTH. Before
> > > this patch, the owner entry in the ACL would be mapped to a gid
> > > entry
> > > in
> > > the NFSv4 ACL, and not the expected special owner entry. This is
> > > caused
> > > by the id mapping returning a valid gid and the nfs4 mapping
> > > assumed
> > > that this was actually a group.
> > >
> > > Fix this by asking for the uid first, and explicitly checking if
> > > the
> > > mapped uid matches the owner. That creates a uid entry in the
> > > NFSv4
> > > ACL
> > > that can be changed later in smbacl4_substitute_{simple,special}
> > > to
> > > the
> > > expected special owner entry.
> > >
> > > Signed-off-by: Christof Schmitt <cs at samba.org>
> > > ---
> > > source3/modules/nfs4_acls.c | 11 ++++++++---
> > > 1 file changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/source3/modules/nfs4_acls.c
> > > b/source3/modules/nfs4_acls.c
> > > index 71f4d8d..996dbab 100644
> > > --- a/source3/modules/nfs4_acls.c
> > > +++ b/source3/modules/nfs4_acls.c
> > > @@ -715,11 +715,16 @@ static bool smbacl4_fill_ace4(
> > > uid_t uid;
> > > gid_t gid;
> > >
> > > - if (sid_to_gid(&ace_nt->trustee, &gid)) {
> > > + /*
> > > + * ID_TYPE_BOTH returns both uid and gid.
> > > Explicitly
> > > + * check for ownerUID to allow the mapping of
> > > the
> > > + * owner to a special entry in this idmap
> > > config.
> > > + */
> > > + if (sid_to_uid(&ace_nt->trustee, &uid) && uid ==
> > > ownerUID) {
> > > + ace_v4->who.uid = uid;
> > > + } else if (sid_to_gid(&ace_nt->trustee, &gid)) {
> > > ace_v4->aceFlags |=
> > > SMB_ACE4_IDENTIFIER_GROUP;
> > > ace_v4->who.gid = gid;
> > > - } else if (sid_to_uid(&ace_nt->trustee, &uid)) {
> > > - ace_v4->who.uid = uid;
> > > } else if (dom_sid_compare_domain(&ace_nt-
> > > >trustee,
> > > &global_sid_Un
> > > ix_N
> > > FS) == 0) {
> > > return false;
> >
> > Just be aware that we did want to keep the gid entry - this was
> > deliberately added.
> >
> > The translation to @owner may still be required, but the idea was
> > to
> > eventually permit sidHistory to work, and therefore not deny access
> > to
> > the user if the 'user' SID becomes an additional SID in a token at
> > some
> > later point, then only visible as a gid in NFSv4 space.
> >
> > See the commit that originally introduced this:
> >
> > commit f36e28d1316bc0bd210933bbdb77241376fe3500
> > Author: Andrew Bartlett <abartlet at samba.org>
> > Date: Mon May 7 08:48:24 2012 +1000
> >
> > s3-nfs4acls: Remove lookup_sid and sidmap from NFSv4 ACL
> > mapping
> > and check gid first
> >
> > By checking just the IDMAP, and by removing the sidmap and
> > lookup_sid calls, we support
> > IDMAP_BOTH. This is because by checking for a mapping to a GID
> > first, we can rely on
> > the fact that IDMAP_BOTH will resolve to a GID.
> >
> > If the sidmap idea is valued - it allows multiple SIDs to map
> > to a
> > single unix ID, this should
> > be done in the IDMAP layer.
> >
> > Andrew Bartlett
> >
> > Signed-off-by: Jeremy Allison <jra at samba.org>
> >
> > Autobuild-User(master): Jeremy Allison <jra at samba.org>
> > Autobuild-Date(master): Sat Aug 11 01:17:36 CEST 2012 on sn-
> > devel-
> > 104
>
> The requirement here was to keep the owner mapping to the special
> user
> entry, since that has a special meaning in the gpfs file system (the
> POSIX modebits are derived from the special entries).
>
> SID history is also a topic that keeps popping up. This would require
> having the entry stored for a gid to allow access from the old and
> new
> SID. In theory we could get there by mapping an entry for the owner
> SID
> to two entries in NFS4: special user and a group entry. I am not yet
> sure what implications that would have.
Yes.
Oddly, I actually spent quite some time on this whole area back in
2012/13 trying to sort out the simple and special mapping stuff. It
continues to be a mind-bending area, trying to both keep compatibility
and correctly map the @owner stuff is good for producing headaches.
I would say that this isn't an area to rush, and ideally we would have
some expected value tests. (I can't find any however, unlike the posix
acl tests).
Andrew Bartlett
> Christof
>
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
More information about the samba-technical
mailing list