Bug 2025486

Bruce summarized the ambiguity and general way forward in the RHEL7 bug which was closed WONTFIX: https://bugzilla.redhat.com/show_bug.cgi?id=2023135#c9

This is a low priority bug, given the workarounds and low impact.  Given the upstream maintainership challenges of nfs4-acl-tools package, it's unlikely to be taken on by engineering.  If an SME or another contributor would like to attempt a solution and present to upstream community, it's possible this bug may be addressed, otherwise it's likely to close WONTFIX.

In case anyone is unsure whether this is doable for them, or wasn't sure where to get started, I just wanted to give background on how NFS4 acls work, show how to setup and demonstrate this bug, and describe the bug in a little more depth.

the bug will require one or more patches to nfs4-acl-tools:

    browse http://git.linux-nfs.org/?p=bfields/nfs4-acl-tools.git;a=summary
    clone  git://git.linux-nfs.org/projects/bfields/nfs4-acl-tools.git

***** BACKGROUND on nfs4 acls *****

(note: in this example, Domain in /etc/idmap.conf is specified as DOMAIN, and idmapping is enabled)
    # tshark -w trace.pcap -i lo &
    # mount localhost:/ /mnt/tmp -overs=4.2,sec=sys
    # cd /mnt/tmp

    # touch 123 file123

    # ls -l 123 file123
    -rw-r--r--. 1 root root 0 Jun 16 11:19 123
    -rw-r--r--. 1 root root 8 Jun 16 11:17 file123

    # strace -fttTvyo /var/tmp/trace.out -s 10240 -etrace=file,desc nfs4_getfacl 123 file123
    # file: 123
    A::OWNER@:rwatTcCy
    A::GROUP@:rtcy
    A::EVERYONE@:rtcy

    # file: file123
    A::OWNER@:rwatTcCy
    A::GROUP@:rtcy
    A::EVERYONE@:rtcy

nfs4_getfattr makes a getxattr() syscall for 'system.nfs4_acl'; the nfs4 filesystem code translates this request into a GETATTR (ACL) request to the server, getxattr() returns the encoded ACL, and nfs4_getfattr decodes and displays the ACL


in the strace:
3586190 11:23:52.412689 getxattr("123", "system.nfs4_acl", NULL, 0) = 80 <0.000085>
3586190 11:23:52.413459 getxattr("123", "system.nfs4_acl", "\0\0\0\3\0\0\0\0\0\0\0\0\0\26\1\207\0\0\0\6OWNER@\0\0\0\0\0\0\0\0\0\0\0\22\0\201\0\0\0\6GROUP@\0\0\0\0\0\0\0\0\0\0\0\22\0\201\0\0\0\tEVERYONE@\0\0", 80) = 80 <0.000074>

3586190 11:23:52.416776 getxattr("file123", "system.nfs4_acl", NULL, 0) = 80 <0.000062>
3586190 11:23:52.416928 getxattr("file123", "system.nfs4_acl", "\0\0\0\3\0\0\0\0\0\0\0\0\0\26\1\207\0\0\0\6OWNER@\0\0\0\0\0\0\0\0\0\0\0\22\0\201\0\0\0\6GROUP@\0\0\0\0\0\0\0\0\0\0\0\22\0\201\0\0\0\tEVERYONE@\0\0", 80) = 80 <0.000035>

in the pcap:

# tshark -2n -r /var/tmp/trace.pcap -z proto,colinfo,nfs.fh.hash,nfs.fh.hash -P -Onfs nfs
  201 426.461670773          ::1 → ::1          NFS 430 V4 Call (Reply In 202) OPEN DH: 0x62d40c52/123  nfs.fh.hash == 0x62d40c52
Remote Procedure Call, Type:Call XID:0x494ae712
Network File System, Ops(6): SEQUENCE, PUTFH, OPEN, GETFH, ACCESS, GETATTR

  202 426.495155717          ::1 → ::1          NFS 446 V4 Reply (Call In 201) OPEN StateID: 0xdb48  nfs.fh.hash == 0x339c7399
Remote Procedure Call, Type:Reply XID:0x494ae712
Network File System, Ops(6): SEQUENCE PUTFH OPEN GETFH ACCESS GETATTR

  235 473.603304902          ::1 → ::1          NFS 326 V4 Call (Reply In 236) GETATTR FH: 0x339c7399  nfs.fh.hash == 0x339c7399
Remote Procedure Call, Type:Call XID:0x504ae712
Network File System, Ops(3): SEQUENCE, PUTFH, GETATTR
    Operations (count: 3): SEQUENCE, PUTFH, GETATTR
        Opcode: PUTFH (22)
                [hash (CRC-32): 0x339c7399]
        Opcode: GETATTR (9)
            Attr mask: 0x00001000 (ACL)

  236 473.603450051          ::1 → ::1          NFS 278 V4 Reply (Call In 235) GETATTR
Remote Procedure Call, Type:Reply XID:0x504ae712
Network File System, Ops(3): SEQUENCE PUTFH GETATTR
    Operations (count: 3)
        Opcode: SEQUENCE (53)
        Opcode: PUTFH (22)
        Opcode: GETATTR (9)
            Status: NFS4_OK (0)
            Attr mask: 0x00001000 (ACL)
                reco_attr: ACL (12) (3 ACEs)
                    ACE count: 3
                    1. ACE Type: Access_Allowed (0)
                        ACE flags: 0x00000000
                        ACE mask: 0x00160187  (RdData/LstDir, WrData/AddFile, AppData/AddSubD, RdAttrs, WrAttrs, RdACL, WrACL, Sync)
                        Who: OWNER@
                    2. ACE Type: Access_Allowed (0)
                        ACE flags: 0x00000000
                        ACE mask: 0x00120081  (RdData/LstDir, RdAttrs, RdACL, Sync)
                        Who: GROUP@
                    3. ACE Type: Access_Allowed (0)
                        ACE flags: 0x00000000
                        ACE mask: 0x00120081  (RdData/LstDir, RdAttrs, RdACL, Sync)
                        Who: EVERYONE@


nfs4_setfattr is similar, in that it uses a setxattr() call for 'system.nfs4_acl', which the kernel translates into a SETATTR (ACL) call to the server.

***** the issue *****

the problem lies in ambiguity in syntax when setting nfs4 acls.  From the manpage:

    NFS4_SETFACL(1)         NFSv4 Access Control Lists        NFS4_SETFACL(1)

    NAME
       nfs4_setfacl,  nfs4_editfacl  -  manipulate  NFSv4  file/directory
       access control lists

    SYNOPSIS
       nfs4_setfacl [OPTIONS] COMMAND file...
       nfs4_editfacl [OPTIONS] file...
    ...
      COMMANDS
       -a acl_spec [index]
              add  the  ACEs  from  acl_spec  to  file's  ACL.   ACEs are
              inserted starting at the indexth position (DEFAULT:  1)  of
              file's ACL.

here, COMMAND '-a' can take an optional 'index' to indicate where in the ACL the new ACE is to be inserted.


if we were to add full permissions (like those for OWNER) for group1, group2, group3:

    # nfs4_setfacl -a A:g:group1@DOMAIN:rwatTcCy ./123
    # nfs4_setfacl -a A:g:group2@DOMAIN:rwatTcCy ./123

    'default=1' actually is actually not working... they're just being appended:

    # nfs4_getfacl 123
    # file: 123
    A::OWNER@:rwatTcCy
    A::GROUP@:rtcy
    A:g:group1@DOMAIN:rwatcy
    A:g:group2@DOMAIN:rwatcy
    A::EVERYONE@:rtcy


'index' is (supposed to be) used to specify insertion somewhere other than at the beginning of the list, but since it is optional for '-a' and '-x', there is an ambiguity when the first filename is all numeric:

    # nfs4_setfacl -a A:g:group3@DOMAIN:rwatTcCy 123
    No path(s) specified.

    # nfs4_setfacl -x 123
    No path(s) specified.

    # nfs4_setfacl -a A:g:group3@DOMAIN:rwatTcCy 123 file123
    Failed while inserting ACE(s) (at index 123).


If the (first) path includes non-digit characters, no ambiguity exists, as the path is interpreted correctly:

    # nfs4_setfacl -a A:g:group3@DOMAIN:rwatTcCy file123 123
    # nfs4_setfacl -a A:g:group4@DOMAIN:rwatTcCy ./123 file123


A secondary problem is that the new ACE is always appended, regardless of whether 'index' is specified (and recall that the manpage said the default was to inserted the new entry first):

    # nfs4_setfacl -a A:g:group5@DOMAIN:rwatTcCy 2 ./123

the above is expected to insert the new ACE in the ACL between the 'group1' and 'group2', but it is appended:

    # nfs4_getfacl 123
    # file: 123
    A::OWNER@:rwatTcCy
    A::GROUP@:rtcy
    A:g:group1@DOMAIN:rwatcy
    A:g:group2@DOMAIN:rwatcy
    A:g:group3@DOMAIN:rwatcy
    A:g:group4@DOMAIN:rwatcy
    A:g:group5@DOMAIN:rwatcy
    A::EVERYONE@:rtcy


So in addition to attempting to disambiguate when a number is a filename or an index, there may also need to be a discussion over whether to retain the ambiguous syntax, define a new and unambiguous syntax, remove the option to specify an index entirely (insertion would have to involve removing all ACEs and re-adding them in the desired order), etc.

Steved - it looks like you took some patches for this in upstream.  Are you ok with RHEL8.8 then and if so what is a good DTM?

(In reply to Dave Wysochanski from comment #14)
> Steved - it looks like you took some patches for this in upstream.  Are you
> ok with RHEL8.8 then and if so what is a good DTM?

The question is can we added a new flag to RHEL8.8 and then back port it to RHEL8.6 z-stream?

(In reply to Steve Dickson from comment #15)
> (In reply to Dave Wysochanski from comment #14)
> > Steved - it looks like you took some patches for this in upstream.  Are you
> > ok with RHEL8.8 then and if so what is a good DTM?
> 
> The question is can we added a new flag to RHEL8.8 and then back port it to
> RHEL8.6 z-stream?

SteveD I don't follow.  Why is backporting to 8.6.z even being mentioned?  As far as I know this is only needed in RHEL8.8 so far, there's been no request to backport or did I miss something?

This was originally a RHEL7 bug and got moved to RHEL8 because it wasn't appropriate for RHEL7.

(In reply to Dave Wysochanski from comment #16)
> (In reply to Steve Dickson from comment #15)
> > (In reply to Dave Wysochanski from comment #14)
> > > Steved - it looks like you took some patches for this in upstream.  Are you
> > > ok with RHEL8.8 then and if so what is a good DTM?
> > 
> > The question is can we added a new flag to RHEL8.8 and then back port it to
> > RHEL8.6 z-stream?
> 
> SteveD I don't follow.  Why is backporting to 8.6.z even being mentioned? 
> As far as I know this is only needed in RHEL8.8 so far, there's been no
> request to backport or did I miss something?
No... I just made the assumption that it was destine for the Z-stream.
 
> 
> This was originally a RHEL7 bug and got moved to RHEL8 because it wasn't
> appropriate for RHEL7.
So we can change the interface this late in the release....

(In reply to Steve Dickson from comment #17)
> (In reply to Dave Wysochanski from comment #16)
> > (In reply to Steve Dickson from comment #15)
> > > (In reply to Dave Wysochanski from comment #14)
> > > > Steved - it looks like you took some patches for this in upstream.  Are you
> > > > ok with RHEL8.8 then and if so what is a good DTM?
> > > 
> > > The question is can we added a new flag to RHEL8.8 and then back port it to
> > > RHEL8.6 z-stream?
> > 
> > SteveD I don't follow.  Why is backporting to 8.6.z even being mentioned? 
> > As far as I know this is only needed in RHEL8.8 so far, there's been no
> > request to backport or did I miss something?
> No... I just made the assumption that it was destine for the Z-stream.
>  
> > 
> > This was originally a RHEL7 bug and got moved to RHEL8 because it wasn't
> > appropriate for RHEL7.
> So we can change the interface this late in the release....

I'm not sure what this last sentence means.  Are you saying you do not want to fix this in RHEL8 but maybe it should be moved to RHEL9?

Talked to steved in confab.  He said he's already built into the next RHEL9 nfs4-acl-tools and should be ready for 9.2

QE - please re-ack for RHEL9.2

(In reply to Dave Wysochanski from comment #18)
> (In reply to Steve Dickson from comment #17)
> > (In reply to Dave Wysochanski from comment #16)
> > > (In reply to Steve Dickson from comment #15)
> > > > (In reply to Dave Wysochanski from comment #14)
> > > > > Steved - it looks like you took some patches for this in upstream.  Are you
> > > > > ok with RHEL8.8 then and if so what is a good DTM?
> > > > 
> > > > The question is can we added a new flag to RHEL8.8 and then back port it to
> > > > RHEL8.6 z-stream?
> > > 
> > > SteveD I don't follow.  Why is backporting to 8.6.z even being mentioned? 
> > > As far as I know this is only needed in RHEL8.8 so far, there's been no
> > > request to backport or did I miss something?
> > No... I just made the assumption that it was destine for the Z-stream.
> >  
> > > 
> > > This was originally a RHEL7 bug and got moved to RHEL8 because it wasn't
> > > appropriate for RHEL7.
> > So we can change the interface this late in the release....
> 
> I'm not sure what this last sentence means.  Are you saying you do not want
> to fix this in RHEL8 but maybe it should be moved to RHEL9?

I'm sure about changing the command line this late in the release cycle.
But it add something to the command line so maybe it would be OK....
These are the patches that would be needed

commit 1a80c0b102c1c5a4e1e0bfa0525a11b0b08564c8 (tag: nfs4-acl-tools-0.4.1-rc3)
Author: Pierguido Lambri <plambri>
Date:   Tue Sep 20 14:19:24 2022 -0400

    nfs4_setfacl: update man page about new option index

commit 9ddd3984eeb4ef94af53794213bb56be6f3f61c2
Author: Pierguido Lambri <plambri>
Date:   Tue Sep 20 14:13:53 2022 -0400

    nfs4_setfacl: add a specific option for indexes

Sorry I missed the fact this bug already had a RHEL9 clone.

Steved your call about whether to put this into RHEL8.