[PATCH] nfs4acl: Fix owner mapping with ID_TYPE_BOTH

[Andrew Bartlett]

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_Unix_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

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba