Support enforcement of POSIX ACLs - #50
ThomasWaldmann wants to merge 4 commits into
Conversation
| # Values for the binary ACL format documented in acl(5) and defined in <linux/posix_acl_xattr.h>. | ||
| POSIX_ACL_XATTR_VERSION = 2 | ||
| ACL_UNDEFINED_ID = 0xFFFF_FFFF | ||
| ACL_USER_OBJ = 0x01 | ||
| ACL_USER = 0x02 | ||
| ACL_GROUP_OBJ = 0x04 | ||
| ACL_GROUP = 0x08 | ||
| ACL_MASK = 0x10 | ||
| ACL_OTHER = 0x20 | ||
| ACL_READ = 4 | ||
| ACL_WRITE = 2 | ||
| ACL_EXECUTE = 1 | ||
|
|
||
| ACL_ACCESS_XATTR = 'system.posix_acl_access' | ||
| FILE_CONTENTS = b'Hello ACL!\n' | ||
|
|
||
|
|
||
| def pack_acl(entries) -> bytes: | ||
| 'Pack (tag, permissions, id) triples into the binary format expected by the kernel.' | ||
| return struct.pack('<I', POSIX_ACL_XATTR_VERSION) + b''.join( | ||
| struct.pack('<HHI', tag, permissions, entry_id) for tag, permissions, entry_id in entries | ||
| ) |
There was a problem hiding this comment.
Maybe this could be moved to a global place in mfusepy, so that users can just import it from there.
Additionally to pack_acl, there could be also some text_to_acl function.
There was a problem hiding this comment.
pack_acl, and also the inverse unpack_acl, seems helpful, yes. How standardized/cross-platform is this struct format? Then again, even if it were not, it would be nice for mfusepy to handle the cross-platform compatibility.
There was a problem hiding this comment.
Done in f34f496: the constants, a PosixACLEntry(tag, permissions, qualifier) named tuple, pack_posix_acl, and the inverse unpack_posix_acl now live in mfusepy and the test imports them from there.
How standardized/cross-platform is this struct format?
It is Linux UAPI and very stable, but it is only Linux:
<linux/posix_acl_xattr.h>definesa_version(POSIX_ACL_XATTR_VERSION= 2, the only version that ever existed, unchanged since 2002) plus{__le16 e_tag; __le16 e_perm; __le32 e_id;}per entry, and<linux/posix_acl.h>the tag and permission values. The__letypes mean the layout is fixed little-endian and does not depend on the host byte order, so the same bytes are correct on s390x as on x86.- FreeBSD stores POSIX.1e ACLs as the
posix1e.acl_access/posix1e.acl_defaultextattrs in the system namespace using its ownstruct oldaclin native layout, and it is normally accessed viaacl_get_file(3), not via the xattr API. - macOS has NFSv4-style ACLs which are not exposed as an extended attribute at all (
ls -leshows them,xattr -ldoes not).
Since FUSE_CAP_POSIX_ACL is a Linux FUSE protocol flag and no other FUSE implementation does ACL passthrough, there is no second format for mfusepy to abstract over, so the helpers are documented as the Linux format rather than pretending to be portable. I named them pack_posix_acl/unpack_posix_acl instead of pack_acl/unpack_acl to leave room for NFSv4 or Darwin ACLs later, but happy to rename.
To make sure the packing is not just self-consistent, the new test_pack_posix_acl_is_understood_by_the_kernel sets a packed ACL as system.posix_acl_access on a real file and checks both what the kernel hands back and which mode bits it derives from the ACL.
Additionally to
pack_acl, there could be also sometext_to_aclfunction.
I left that out for now because it is a fair bit more than a struct conversion: the getfacl/setfacl text grammar (default: prefixes, comment header lines, the #effective: comments) plus resolving names to uids/gids via pwd/grp, and a decision about what to do when a name does not resolve. Should I add it to this PR, or would you rather have it as a follow-up once the packing part is settled?
| # Values for the binary ACL format documented in acl(5) and defined in <linux/posix_acl_xattr.h>. | ||
| POSIX_ACL_XATTR_VERSION = 2 | ||
| ACL_UNDEFINED_ID = 0xFFFF_FFFF | ||
| ACL_USER_OBJ = 0x01 | ||
| ACL_USER = 0x02 | ||
| ACL_GROUP_OBJ = 0x04 | ||
| ACL_GROUP = 0x08 | ||
| ACL_MASK = 0x10 | ||
| ACL_OTHER = 0x20 | ||
| ACL_READ = 4 | ||
| ACL_WRITE = 2 | ||
| ACL_EXECUTE = 1 | ||
|
|
||
| ACL_ACCESS_XATTR = 'system.posix_acl_access' | ||
| FILE_CONTENTS = b'Hello ACL!\n' | ||
|
|
||
|
|
||
| def pack_acl(entries) -> bytes: | ||
| 'Pack (tag, permissions, id) triples into the binary format expected by the kernel.' | ||
| return struct.pack('<I', POSIX_ACL_XATTR_VERSION) + b''.join( | ||
| struct.pack('<HHI', tag, permissions, entry_id) for tag, permissions, entry_id in entries | ||
| ) |
There was a problem hiding this comment.
pack_acl, and also the inverse unpack_acl, seems helpful, yes. How standardized/cross-platform is this struct format? Then again, even if it were not, it would be nice for mfusepy to handle the cross-platform compatibility.
| def is_capable(self, flags: int) -> bool: | ||
| 'Return whether the kernel supports all of the given FUSE_CAP_* feature flags.' | ||
| capable = self.capable_ext if self._has_extended_features else self.capable | ||
| return bool(flags) and capable & flags == flags |
There was a problem hiding this comment.
| return bool(flags) and capable & flags == flags | |
| return (capable & flags) == flags |
Maybe this is better? I don't understand why no flags given should be specially handled and set to False, even though no capability flags were required. Same for is_wanted.
There was a problem hiding this comment.
Applied in f34f496 for both is_capable and is_wanted.
The bool(flags) guard was me being overly literal about mirroring libfuse: fuse_set_feature_flag does if (conn->capable_ext & flag), which is false for 0 (and, for a multi-bit argument, means any instead of all). Your version is the better semantics: no requested feature is trivially supported, so is_capable(0) is now True and set_feature_flag(0) trivially succeeds without changing anything. There is a test for that now.
The added parentheses do not change the parse, by the way: unlike in C, & binds tighter than == in Python, so capable & flags == flags already meant (capable & flags) == flags. They make it obvious without having to know that, though.
…failure
perfuse is part of the NetBSD base system, there is no such package. pkgin
now fails when asked to install a package that does not exist:
perfuse is not available in the repository
fail-fast: false, so one can see which platforms a failure affects.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
8ebb337 to
f34f496
Compare
mfusepy looked up the macfuse_version symbol in libfuse to detect the
legacy MacFuse (and then used an old 32 bit inode struct stat layout).
macFUSE does not export that symbol, so this was a lookup of a missing
symbol on every import.
With macFUSE 5.3.x, libfuse depends on a lot of system frameworks (dyld
loads about 500 images for it) and on macOS 14, that lookup makes dyld abort:
dyld[29263]: Assertion failed: (_usedCount < _allocCount), function push_back, file Array.h, line 64.
So "import mfusepy" crashed the Python interpreter there (macOS 15 is fine).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Add the FUSE_CAP_* feature flags and a way to request them, so that a file system can enable optional kernel features during the init handshake: - fuse_conn_info.is_capable / is_wanted / set_feature_flag / unset_feature_flag, mirroring the libfuse helpers, including the 'want' vs. 'want_ext' handling for libfuse >= 3.17. - Operations.wanted_features as declarative alternative that does not require overriding 'init_with_config'. Unsupported features are skipped with a warning because requesting a feature that the kernel is not capable of makes libfuse abort the mount with EPROTO. The motivation is FUSE_CAP_POSIX_ACL (Linux): without it, the ACLs returned by getxattr for system.posix_acl_access / _default are only visible, e.g. to getfacl, but the kernel does not check access against them. Inside a user namespace, e.g. a rootless container, getxattr for these attributes is even refused with EOPNOTSUPP. The new test mounts a file system whose files can only be read (or not read) as expected if the kernel does enforce the returned ACLs. Fixes mxmlnkn#49 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review feedback: - Move the POSIX ACL constants and the conversion to the binary xattr format from the test into mfusepy, so that file systems can simply import them, and add the inverse 'unpack_posix_acl'. The format is Linux UAPI (<linux/posix_acl_xattr.h>, version 2 since 2002) and is fixed little-endian, i.e., it does not vary with the host byte order. Other platforms neither use it nor support FUSE_CAP_POSIX_ACL. - Drop the special case for no flags in 'is_capable' and 'is_wanted'. Requesting no features now trivially succeeds instead of failing. The new test checks the packed representation against the kernel by setting it as 'system.posix_acl_access' on a real file and comparing what the kernel returns and which mode bits it derives from the ACL. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
f34f496 to
3fd3fae
Compare
|
@mxmlnkn Can you restart the netbsd CI job please? |
Fixes #49.
The kernel only enforces POSIX ACLs when the file system requests
FUSE_CAP_POSIX_ACLduring the init handshake. Without it, the ACLs a file system returns viagetxattrforsystem.posix_acl_access/system.posix_acl_defaultare merely visible, e.g. togetfacl, but access is never checked against them. Inside a user namespace, e.g. a rootless container, the kernel even refusesgetxattrfor these attributes withEOPNOTSUPPunless the feature was negotiated.Since mfusepy had no way at all to request optional kernel features, this adds the general mechanism and uses POSIX ACLs as its first application:
All
FUSE_CAP_*constants fromfuse_common.hplusfeature_flag_names()for log messages.fuse_conn_info.is_capable,is_wanted,set_feature_flag, andunset_feature_flag, mirroring the libfuse helper functions. They setwantandwant_extbecause libfuse >= 3.17 only convertswantintowant_extwhen exactly one of both was changed (fuse_convert_to_conn_want_ext), and they refuse flags missing fromcapable/capable_extbecause libfuse aborts the mount withEPROTOfor unknownwantflags.Operations.wanted_featuresas a declarative alternative that does not require overridinginit_with_config:It is applied before the file system's
init_with_configis called, so it can still check the outcome withconn_info.is_wantedor adjust it. Features that the kernel or the loaded libfuse version is not capable of, e.g. everything beyondFUSE_CAP_IOCTL_DIRwith libfuse 2, are skipped with a warning instead of breaking the mount.Testing
tests/test_posix_acl.pyunit-tests the flag helpers everywhere and mounts a small file system for an end-to-end check. Its two root-owned files can only be read (or not read) as expected if the kernel does enforce the returned ACLs:alloweduser:<uid>:r--denieduser:<uid>:---EACCES, although the mode bits allow itThe end-to-end test skips on non-Linux, when running as root (which is not subject to ACL checks), and when the kernel or libfuse is not capable of
FUSE_CAP_POSIX_ACL, e.g. for libfuse 2.Verified on Linux 6.18 with libfuse 3.14 in a rootless container: before the change, both files were readable and
getxattrforsystem.posix_acl_accessfailed withEOPNOTSUPP; with the change,getfaclshows the ACLs through the mount and access is enforced in both directions.black,ruff,flake8,pylint,codespell,mypy, andpytypeare clean and the existing test suite still passes.I did not touch
CHANGELOG.mdsince its entries seem to be written at version-bump time.🤖 Generated with Claude Code