[PATCH] nfs4acl: Fix owner mapping with ID_TYPE_BOTH
Christof Schmitt
cs at samba.org
Wed Sep 14 18:43:54 UTC 2016
On Wed, Sep 14, 2016 at 09:00:55AM -0700, Jeremy Allison wrote:
> On Tue, Sep 13, 2016 at 04:26:03PM -0700, Jeremy Allison wrote:
> > On Mon, Sep 12, 2016 at 05:34:21PM -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.
> >
> > OK, went through the logic here very carefully (w.r.t chown
> > interactions), and I think this is the correct thing to do.
> >
> > Pushed.
>
> The push failed (unrelated issue :-) and in the shower
> this morning I had a thought (something about this patch
> must have been bothering me :-).
>
> This patch adds logic to check the uid first - if the
> mapped uid matches the owner, which is correct, but
> it also removes the fallback mapping of:
>
> - } else if (sid_to_uid(&ace_nt->trustee, &uid)) {
> - ace_v4->who.uid = uid;
>
> I don't think that's right. I think the logic should be:
>
> check the uid first - if the mapped uid matches the owner
> check if mapped to group
> check if mapped to user.
>
> (i.e. keep the final fallback code). Given that, I think
> the following patch is correct. Christof, can you confirm ?
Yes, correct. This is required for id mappings that return ID_TYPE_UID
and an ACL entry that does not match the owner. I missed that, since i
was thinking along the implications of ID_TYPE_BOTH.
Thank you for finding that.
Christof
>
> Thanks,
>
> Jeremy.
> From c124b1e1ed0d2e90a0eb4c6277daaf67f17e64e6 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>
> Reviewed-by: Jeremy Allison <jra at samba.org>
> ---
> source3/modules/nfs4_acls.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c
> index 71f4d8d..6fe3b11 100644
> --- a/source3/modules/nfs4_acls.c
> +++ b/source3/modules/nfs4_acls.c
> @@ -715,7 +715,14 @@ 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)) {
> --
> 2.8.0.rc3.226.g39d4020
>
More information about the samba-technical
mailing list