Re: [PATCH] Basic support for checking NFSv4 ACLs in Linux

[Andreas Grünbacher]

Ondrej,

this doesn't belong on the acl-devel mailing list; may I suggest the
coreutils@gnu.org list instead?

But see below anyway ...

Am Mi., 9. Nov. 2022 um 16:31 Uhr schrieb Ondrej Valousek
<ondrej.valousek.xm@renesas.com>:
> As per suggestions from Andreas, I am attaching a simplified version of the 
> patch implementing basic
> checks for non-trivial NFSv4 acls
>
> ---
>  lib/file-has-acl.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
> index e02f0626a..37b3aaa30 100644
> --- a/lib/file-has-acl.c
> +++ b/lib/file-has-acl.c
> @@ -32,6 +32,11 @@
>  #if GETXATTR_WITH_POSIX_ACLS
>  # include <sys/xattr.h>
>  # include <linux/xattr.h>
> +# include <arpa/inet.h>
> +#ifndef XATTR_NAME_NFSV4_ACL
> +#define XATTR_NAME_NFSV4_ACL "system.nfs4_acl"
> +#endif
> +#define TRIVIAL_NFS4_ACL_LENGTH 80
>  #endif
>
>  /* Return 1 if NAME has a nontrivial access control list,
> @@ -67,6 +72,14 @@ file_has_acl (char const *name, struct stat const *sb)
>              return 1;
>          }
>
> +      if (ret < 0) { /* we might be on NFS, so try to check NFSv4 ACLs too */
> +         char xattr[TRIVIAL_NFS4_ACL_LENGTH];
> +
> +         ret = getxattr (name, XATTR_NAME_NFSV4_ACL, xattr, 
> TRIVIAL_NFS4_ACL_LENGTH);
> +         if (ret < 0 && errno == ENODATA)
> +              ret = 0;
> +         else ret = errno == ERANGE;  /* we won't fit into the buffer, so 
> non-trivial ACL is presented */
> +      }

Something similar to that probably, yes. Now the remaining loophole is
that what you're getting here is the over-the-wire value of an NVSv4
ACL. Servers can implement any kind of ACL scheme, and so having less
than three ACL entries doesn't guarantee that the resulting ACL is
already truthfully represented by the file mode. For example, the ACL
can grant permissions that go beyond what the file mode can grant,
like the permission to change permissions.

Beyond that, the file mode permission bits could also be set to
something like r---w-rw-. In that case, a matching NFSv4 ACL could
contain something the following entries:

1 - Grant the owner Read access.
2 - Deny the owning group read access.
3 - Grant the owning group Write access.
4 - Grant everyone Read and Write access.

In that example, the third entry is redundant because it is implied by
the fourth entry, and so it could be missing. But the Deny entry here
would be perfectly reasonable to represent the file mode. The entries
could also be presented in a slightly different order than shown here.

So reasonable and straight-forward representations of the file mode
can have up to three Allow and two Deny entries, for the owner, the
owning group, and for everyone else, the permissions in the Allow
entries shouldn't go beyond what the file mode permission bits can
express (Read, Write, Execute), and the CL entries don't contain any
unexpected inheritance flags.

Once that is implemented, it would make sense to test against the most
common NFSv4 servers to make sure that the trivial ACL detection
algorithm implemented here recognizes the servers' file mode
permission bit equivalents, and behaves properly for non-trivial ACLs
as well.


>        if (ret < 0)
>          return - acl_errno_valid (errno);
>        return ret;
> --
> 2.37.3

Thanks,
Andreas