Bug 13896 - Ceph VFS module runs with ACL support disabled by default
Summary: Ceph VFS module runs with ACL support disabled by default
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: David Disseldorp
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-12 11:51 UTC by David Disseldorp
Modified: 2019-04-30 20:38 UTC (History)
2 users (show)

See Also:


Attachments
fix cherry-pick for 4.9.next and 4.10.next (2.98 KB, patch)
2019-04-14 22:05 UTC, David Disseldorp
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2019-04-12 11:51:43 UTC
vfs_ceph currently performs POSIX ACL mapping and uses xattrs for retention. libcephfs *disables* ACL support by default and returns -EOPNOTSUPP in the POSIX ACL get/setxattr paths as a result.

libcephfs ACL support should be explicitly enabled during mount via the following config:
        client acl type = posix_acl
        fuse default permissions = false
Comment 1 David Disseldorp 2019-04-14 22:05:21 UTC
Created attachment 15060 [details]
fix cherry-pick for 4.9.next and 4.10.next
Comment 2 David Disseldorp 2019-04-14 22:10:22 UTC
@Jeff: should NFS-Ganesha be doing something similar for ACLs handling, or are they not currently supported there?
Comment 3 Jeremy Allison 2019-04-15 23:25:58 UTC
Re-assigning to Karolin for inclusion in 4.10.next, 4.9.next.
Comment 4 Karolin Seeger 2019-04-16 07:40:14 UTC
(In reply to Jeremy Allison from comment #3)
Pushed to autobuild-v4-{10,9}-test.
Comment 5 Jeff Layton 2019-04-23 12:58:19 UTC
(In reply to David Disseldorp from comment #2)

They're not supported there (yet). FSAL_CEPH only supports ATTRS_POSIX (stuff in struct stat() + a few other things) and ATTR_SEC_LABEL for labeled NFS support (aka SELinux).

NFSv4 ACLs are more like Windows ACLs so we'd need to translate them to/from POSIX ACLs (which is always a lossy conversion). I think ganesha has routines for this, so we could probably enable it to use POSIX ACLs there without too much effort.

What we should probably do long-term is add richacl support to libcephfs and then have FSAL_CEPH / vfs_ceph use that directly instead. That's a larger project though.

FWIW, I also have some concerns about libceph's ACL enforcement in general. ACLs are only enforced on the client AFAICT, and I suspect there could be some ToC/ToU races in certain situations, particularly when the client doesn't hold auth caps.
Comment 6 Karolin Seeger 2019-04-25 07:06:57 UTC
(In reply to Karolin Seeger from comment #4)
Pushed to both branches.
Re-assigning to David to keep bug open for further discussion (if needed).
Comment 7 David Disseldorp 2019-04-30 20:37:56 UTC
Thanks for the feedback Jeff!

(In reply to Jeff Layton from comment #5)
> (In reply to David Disseldorp from comment #2)
> 
> They're not supported there (yet). FSAL_CEPH only supports ATTRS_POSIX
> (stuff in struct stat() + a few other things) and ATTR_SEC_LABEL for labeled
> NFS support (aka SELinux).
> 
> NFSv4 ACLs are more like Windows ACLs so we'd need to translate them to/from
> POSIX ACLs (which is always a lossy conversion). I think ganesha has
> routines for this, so we could probably enable it to use POSIX ACLs there
> without too much effort.

FSAL_CEPH POSIX ACL support would be good to have as a lingua franca until richacl support (or similar) is available.

> What we should probably do long-term is add richacl support to libcephfs and
> then have FSAL_CEPH / vfs_ceph use that directly instead. That's a larger
> project though.

Agreed, that'd be ideal and would also allow us to bypass kernel support for now (until greater adoption perhaps :).

> FWIW, I also have some concerns about libceph's ACL enforcement in general.
> ACLs are only enforced on the client AFAICT, and I suspect there could be
> some ToC/ToU races in certain situations, particularly when the client
> doesn't hold auth caps.

Ouch, okay, thanks for the heads-up. Please add me to any tickets that may exist to track this.
Comment 8 David Disseldorp 2019-04-30 20:38:14 UTC
(In reply to Karolin Seeger from comment #6)
> (In reply to Karolin Seeger from comment #4)
> Pushed to both branches.
> Re-assigning to David to keep bug open for further discussion (if needed).

Thanks Karo - closing...