Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add native NFSv4 style ZFS ACL support for Linux #206

Open
wants to merge 6 commits into
base: tn_master
Choose a base branch
from

Conversation

usaleem-ix
Copy link

Motivation and Context

So far, ZFS on Linux does not support NFSv4 style native ZFS ACLs. ZFS on Linux has implemented POSIX ACL type. The ACL types are not interchangeable, so existing pools cannot be used across different platforms without loss of ACLs.

See also: openzfs#13186 and openzfs#9709

The purpose of this PR is to get all the changes reviewed internally once before posting a PR upstream.

Description

This PR adds support for NFSv4.1 style native ZFS ACLs for ZFS on Linux through xattr.

A new xattr sysem.nfs4_acl_xdr is added, that is used to store NFSv4.1 ACL structures. Anew inode operations endpoint is added as zpl_permssions(), that is used by VFS in Linux to determine whether to allow/deny an operation. There are certain situations where ACL may need to be overridden based on capabilities. This is handled in secpolicy_vnode_access2() and the logic is almost directly copied from Linux VFS.

The PR contains all the improvements and fixes after initial implementation for NFSv4.1 ACLs for TrueNAS SCALE:

  • xattr handler can strip NFSv4.1 ACL
  • Non-RFC flags that are useful to user-space applications, i.e. ACL_IS_TRIVIAL and ACL_IS_DIR are exposed.
  • BSD semantics for group ownership are forced for NFSV4.1 ACLs
  • Get / Set operation for NFS ACLs are simplified and extra memory allocations are removed. This simplifies conversion between internal NFSv4.1 ACLs and external xattr representation.
  • Improved performance for zpl_permission().

This PR also adds a common library, libzfsacl, for Linux and FreeBSD for accessing and manipulating NFSv4 style ACLs. libpyzfsacl provides python bindings for libzfsacl. Python bindings are used to write Get (zfs_getnfs4facl) / Set (zfs_setnfs4facl) tools for NFSv4.1 ACLs.

libsunacl provides an interface for Samba to access acl() and facl() for vfs_zfsacl.c in Samba.

Since, Linux kernel does not support NFSv4 style ACLs, there are some limitations:

  • PERM_READ_ATTRIBUTES is currently not implemented for Linux. It does not have any equivalent in POSIX ACLs as well.
  • PERM_WRITE_OWNER is not supported without patching the Linux kernel.

For RPM/DEB packaging, zfs_getnfs4facl and zfs_setnfs4facl are packaged in zfs package. For libzfsacl, libsunacl, python bindings and test suite, a new package python3-libzfsacl is created.

For native Debian packaging, zfs_getnfs4facl and zfs_setnfs4facl are packaged in openzfs-zfsutils package. For libzfsacl, libsunacl, python bindings and test suite, a new package openzfs-python3-libzfsacl is created.

Further details can be found in individual commit messages.

How Has This Been Tested?

The test suite zfsacltests uses python bindings for libzfsacl for verifying the behavior of NFSv4.1 ACLs. The test suite tries to cover almost all aspects of the NFSv4.1 ACLs.

  • The very basic behavior of ACLs is verified.
  • The behavior of flags is tested and verify that ACLs are inherited correctly.
  • The behavior of each individual permission is verified for ALLOW and DENY.
  • There are tests where a specific permission is needed to perform an operation, but the user has all the permissions except the one that is required. These tests should fail.
  • Allowing a permission only allows to perform that operation is also tested. These tests should fail as well.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

}

cr = CRED();
crhold(cr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I can see, in most (all?) cases where ZFS on Linux calls crhold() before calling some ZFS functions it also calls spl_fstrans_mark() to prevent recursion on memory allocation, leading to deadlock, like this: openzfs#15786 . I seems related to MAY_NOT_BLOCK check above, but I wonder if one covers it completely, or there are cases here when recursion is still not desired.

Comment on lines 1602 to 1605
#define ACES_TO_XDRSIZE(x) (XDRBASE + (x * ACE4SIZE))
#define XDRSIZE_TO_ACES(x) ((x - XDRBASE) / ACE4SIZE)
#define XDRSIZE_IS_VALID(x) ((x >= XDRBASE) && \
(((x - XDRBASE) % ACE4SIZE) == 0))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good practice is to take "x" on the right side into parentheses in case there sill be a complex expression.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I have updated.

Copy link
Collaborator

@amotin amotin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/This also commit fixes/This commit also fixes/ in the commit message.

Copy link
Collaborator

@amotin amotin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand it right that libzfsacl and libsunacl are packaged into python3-libzfsacl-*? Wouldn't it have sense for them to be independent for use by Samba and possibly other apps, and second package depending on one providing python bindings for those who need them?

anodos325 and others added 6 commits January 31, 2024 15:51
This implements NFSv41 (RFC 5661) ACLs in a manner compatible with
vfs_nfs4acl_xattr in Samba.

There are three key areas of change in this commit:

1) NFSv4 ACL management through system.nfs4_acl_xdr xattr. Install
  an xattr handler for "system.nfs4_acl_xdr" that presents an xattr
  containing full NFSv41 ACL structures generated through rpcgen
  using specification from the Samba project. This xattr is used by
  userspace programs to read and set permissions.

2) add an i_op->permissions endpoint: zpl_permissions(). This is used
  by the VFS in Linux to determine whether to allow / deny an
  operation. Wherever possible, we try to avoid having to call
  zfs_access(). If kernel has NFSv4 patch for VFS, then perform more
  complete check of avaiable access mask.

3) add capability-based overrides to secpolicy_vnode_access2(). There
  are various situations in which ACL may need to be overridden based
  on capabilities. This logic is almost directly copied from Linux
  VFS. Switch to using ns-aware checks rather than capable(). Expand
  optimization allow bypass of zfs_zaccess() in case of trivial ACL if
  MAY_OPEN is included in requested mask.

This is commit was initially inspired by work from Paul B. Henson to
implement NFSv4.0 (RFC3530) ACLs in ZFS on Linux. Key areas of
divergence are as follows:

- ACL specification, xattr format, xattr name
- Addition of handling for NFSv4 masks from Linux VFS
- Addition of ACL overrides based on capabilities

Authored-by: Andrew Walker <[email protected]>
Signed-off-by: Umer Saleem <[email protected]>
Adds ability for xattr handler to "strip" NFSv4.1 ACLs. Since there
is no libc equivalent of strip operation in Linux for NFSv4 ACLs, as
there are in POSIX ACLs and on FreeBSD, this commit handles the
operation entirely in ZFS.

Expose ACL_IS_TRIVIAL and ACL_IS_DIR flags as ACL-wide flags in the
system.nfs4_acl_xdr generated on getxattr requests. This are non-RFC
flags that are useful for userspace applications. ACL_IS_TRIVIAL helps
to avoid relatively expensive ACL-related operations.

Advertise support for large xattrs. SB_LARGEXATTR is used to indicate
to the kernel that the filesystem supports large-size xattrs greater
than 64KiB. This flag is used to evaluate whether to allow large xattr
read or write requests (up to 2 MiB).

Force BSD semantics for group ownership if NFSV4ACL. Since there is no
hard-and-fast rule about creation semantics for NFSv4 ACLs on Linux,
opt for what is least likely to break users permissions on change from
FreeBSD to Linux.

Improves zpl_permission performance. This function can be frequently
called with MAY_EXEC|MAY_NOT_BLOCK during RCU path walk.

Authored-by: Andrew Walker <[email protected]>
Signed-off-by: Umer Saleem <[email protected]>
MS-FSCC 2.6 specifies that for a file, applications can read the file
but cannot write to it or delete it. For a directory, applications
cannot delete it, but applications can create and delete files from
the directory.

This commit also fixes a bug whereby owner@ ACL that limits WRITE_DATA
access for the owner of a file was not being properly enforced. The
owner of a file should be prevented from write access in this case,
but being owner of file should still allow the file owner to chmod,
chown, and setacl.

Authored-by: Andrew Walker <[email protected]>
Signed-off-by: Umer Saleem <[email protected]>
This commit adds common ACL libraries, libzfsacl for Linux and FreeBSD
to provide helper functions to access ACLs. On Linux, libsunacl
provides acl() and facl() to be consumed by vfs_zfsacl.c in Samba.

libpyzfsacl.c provides python bindings for libzfsacl. Python bindings
are packaged in python3-libzfsacl. A new package is added for libzfsacl
and libsunacl.

Authored-by: Andrew Walker <[email protected]>
Signed-off-by: Umer Saleem <[email protected]>
This commit adds zfs_getnfs4facl and zfs_setnfs4facl.

zfs_getnfs4facl will display the NFSv4 ACLs for a file or directory
on a ZFS filesystem with acltype set to nfsv4 that exposes NFSv4 ACLs
as a system.nfs4_acl_xdr xattr.

zfs_setnfs4facl manipulates the NFSv4 ACLs of one or more files or
directories, on a ZFS filesystem with acltype set to nfsv4.

Both scripts provide output compatible with getfacl and setfacl on
FreeBSD, and provides support for viewing and managing ACL features
present in the NFSv4.1.

Signed-off-by: Umer Saleem <[email protected]>
This commit adds test suite for NFSv4.1 ACLS. The test suite uses
libzfsacl python bindings to validate functionality of NFS ACLs.

The test suite validates the basic behavior of ACLs by verifying
default ACEs and then moves to testing all the flags and
permissions for deny and allow permissions.

Test suite also verifies that allow ACEs don't work without
setting the specific permission flag, i.e. to perform an operation,
it's permission is required. Similarly, test suite also verifies
that allow ACE for a specific permission only allows that
perticular permission and user does not have access to other
permissions.

Signed-off-by: Umer Saleem <[email protected]>
@usaleem-ix usaleem-ix changed the base branch from master to tn_master January 31, 2024 14:19
@usaleem-ix
Copy link
Author

s/This also commit fixes/This commit also fixes/ in the commit message.

Thanks, Updated.

@usaleem-ix
Copy link
Author

Do I understand it right that libzfsacl and libsunacl are packaged into python3-libzfsacl-*? Wouldn't it have sense for them to be independent for use by Samba and possibly other apps, and second package depending on one providing python bindings for those who need them?

This is a better approach. I have created a separate package for libzfsacl and libsunacl as libzfsacl1. Python bindings remain in python3-libzfsacl package.

@amotin
Copy link
Collaborator

amotin commented Jan 31, 2024

libzfsacl1 is an odd name.

@usaleem-ix
Copy link
Author

@anodos325 ping.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants