Page MenuHomeFreeBSD

__acl_*_fd(2) syscalls: enable for O_PATH filedescriptors
ClosedPublic

Authored by kib on May 29 2023, 3:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 9, 5:08 PM
Unknown Object (File)
Sat, Apr 27, 4:23 AM
Unknown Object (File)
Sat, Apr 27, 4:23 AM
Unknown Object (File)
Sat, Apr 27, 4:19 AM
Unknown Object (File)
Sat, Apr 27, 3:19 AM
Unknown Object (File)
Sat, Apr 27, 3:05 AM
Unknown Object (File)
Fri, Apr 26, 8:40 PM
Unknown Object (File)
Dec 20 2023, 5:49 AM

Details

Summary
The syscalls repeat corresponding __acl_*_file(2) syscalls, so it seems
to be safe and even required to accept O_PATH fds for them.

PR:     271704

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib requested review of this revision.May 29 2023, 3:12 PM

The change makes sense to me for get and perhaps aclcheck operations, but it seems quite surprising to permit ACL modifications using O_PATH descriptors. For instance, fstat(pathfd) can be used to find the mode, but fchmod(pathfd) is forbidden. Does samba really require this?

The change makes sense to me for get and perhaps aclcheck operations, but it seems quite surprising to permit ACL modifications using O_PATH descriptors. For instance, fstat(pathfd) can be used to find the mode, but fchmod(pathfd) is forbidden. Does samba really require this?

I do not know about samba. but e.g. acl_delete_file(2) vs. acl_delete_fd(2) seems to be same. I can go with the conservative set of acl syscalls there, but having the __acl_*_file() versions looks like the limitation would be artificial. I cannot imagine a 'leak' due to the full change.

Might be fchmod/fchown should work on O_PATH too, I am not sure.

In D40318#917981, @kib wrote:

Might be fchmod/fchown should work on O_PATH too, I am not sure.

Linux documentation explicitly states that fchmod(pathfd) will return EBADF.

In D40318#917976, @kib wrote:

The change makes sense to me for get and perhaps aclcheck operations, but it seems quite surprising to permit ACL modifications using O_PATH descriptors. For instance, fstat(pathfd) can be used to find the mode, but fchmod(pathfd) is forbidden. Does samba really require this?

I do not know about samba. but e.g. acl_delete_file(2) vs. acl_delete_fd(2) seems to be same. I can go with the conservative set of acl syscalls there, but having the __acl_*_file() versions looks like the limitation would be artificial. I cannot imagine a 'leak' due to the full change.

__acl_delete_file(2) is not permitted in capability mode, so there is some difference. A sandboxed program might reasonably expect that an O_PATH fd cannot be used to modify the underlying file (though CAP_ACL_* rights exist for this as well).

I think it is preferable to try and maintain some consistency with respect to other file metadata syscalls, like the fstat/fchmod example I pointed out.

In D40318#917981, @kib wrote:

__acl_delete_file(2) is not permitted in capability mode, so there is some difference. A sandboxed program might reasonably expect that an O_PATH fd cannot be used to modify the underlying file (though CAP_ACL_* rights exist for this as well).

I do not see why would a cap-mode process be disallowed from modifying/deleting ACLs if it was granted rights by an O_PATH descriptor.

I think it is preferable to try and maintain some consistency with respect to other file metadata syscalls, like the fstat/fchmod example I pointed out.

Ok, lets change it for now.

This revision is now accepted and ready to land.May 30 2023, 1:39 AM

Did a quick test with Samba and a FreeBSD 13.2-RELEASE kernel modified with the fix for acl_get_fd and now I can create new directories and read the ACL of it. But attempting to modify the ACL from a Windows 10 client fails with:

An error occured while applying security information to:

T:\}foo

The handle is invalid.

As a test I modified the kernel a bit more and made similar changes to acl_set_fd and acl_delete_fd in sys/kern/vfs_acl.c and now it works to update the ACL from the Windows client.

So as the Samba code works today it looks like it needs to be able to update ACLs via O_PATH-opened file descriptors. I'd like to get input from the Samba folks first though if that is the way it's supposed to work. It might be that the vfs_zfsacl module in Samba should be modified since it probably never has been tested/used on Linux (since native ZFS ACLs doesn't work there yet, only via NFS4 - and there it is a bit of a hack - I wonder how that works since there the ACL is access via a special/magic extended attribute to the file/directory.... Anyway, that's not FreeBSD-related.

lib/libc/sys/open.2
347

alc_aclcheck_fd should read acl_aclcheck_fd ?