25 Jul
2019
25 Jul
'19
8:07 a.m.
Hi,
Without calling ceph_mount_perms_set(), libcephfs consumers such as
Samba can rely upon UserPerm::uid() and UserPerm::gid() to fallback to
geteuid() and setegid() respectively for things such as ACL enforcement.
However, there is no such fallback for supplementary groups, so ACL
checks for a user which is only permitted path access via a
supplementary group will result in a permission denied error.
Samba ticket: https://bugzilla.samba.org/show_bug.cgi?id=14053
I've written a patch to address this (it currently omits the get_gids()
codepath):
https://github.com/ddiss/ceph/commit/035a1785ec73d803fead42c7240df01b755a81…
Does this approach make sense, or should Samba go down the
ceph_mount_perms_set() route to avoid this bug? The latter
would likely be problematic, as user/group details for a mount will
remain static.
Cheers, David
25 Jul
25 Jul
11:25 a.m.
Hi David,
Without calling ceph_mount_perms_set(), libcephfs
consumers such as
Samba can rely upon UserPerm::uid() and UserPerm::gid() to fallback to
geteuid() and setegid() respectively for things such as ACL enforcement.
However, there is no such fallback for supplementary groups, so ACL
checks for a user which is only permitted path access via a
supplementary group will result in a permission denied error.
Samba ticket: https://bugzilla.samba.org/show_bug.cgi?id=14053
I've written a patch to address this (it currently omits the get_gids()
codepath):
https://github.com/ddiss/ceph/commit/035a1785ec73d803fead42c7240df01b755a81…
Does this approach make sense, or should Samba go down the
ceph_mount_perms_set() route to avoid this bug? The latter
would likely be problematic, as user/group details for a mount will
remain static.
Without looking at the details ceph_mount_perms_set() would be the long
term way to go. My goal is to do impersonation only where needed and not
always by default.
But the patch to ceph would also be good.
metze
11:34 a.m.
On Thu, 2019-07-25 at 17:07 +0200, David Disseldorp wrote:
Hi,
Without calling ceph_mount_perms_set(), libcephfs consumers such as
Samba can rely upon UserPerm::uid() and UserPerm::gid() to fallback to
geteuid() and setegid() respectively for things such as ACL enforcement.
However, there is no such fallback for supplementary groups, so ACL
checks for a user which is only permitted path access via a
supplementary group will result in a permission denied error.
Samba ticket: https://bugzilla.samba.org/show_bug.cgi?id=14053
I've written a patch to address this (it currently omits the get_gids()
codepath):
https://github.com/ddiss/ceph/commit/035a1785ec73d803fead42c7240df01b755a81…
Does this approach make sense, or should Samba go down the
ceph_mount_perms_set() route to avoid this bug? The latter
would likely be problematic, as user/group details for a mount will
remain static.
I think that a better approach would be to have samba just call
ceph_mount_perms_set to set the credentials soon after forking. Is there
some reason that doesn't work here?
--
Jeff Layton <jlayton(a)samba.org>
29 Jul
29 Jul
6:21 a.m.
On Thu, 25 Jul 2019 14:34:13 -0400, Jeff Layton wrote:
Samba becomes root for some privileged operations where Windows would
permit access. E.g. "acl group control", vfs_acl_xattr, etc.
We should be able to change Samba's vfs_ceph to use the ceph_ll_X
API to handle the user<->root perms switches and add corresponding
geteuid()/getegid() checks in each VFS call, but IMO this is still
something that should be fixed in libcephfs, to compliment the existing
geteuid/getegid() fallback behaviour.
Cheers, David
On Thu, 2019-07-25 at 17:07 +0200, David Disseldorp
wrote:
> Hi,
>
> Without calling ceph_mount_perms_set(), libcephfs consumers such as
> Samba can rely upon UserPerm::uid() and UserPerm::gid() to fallback to
> geteuid() and setegid() respectively for things such as ACL enforcement.
^ that should be "geteuid() and getegid() ..."
However, there
is no such fallback for supplementary groups, so ACL
checks for a user which is only permitted path access via a
supplementary group will result in a permission denied error.
Samba ticket: https://bugzilla.samba.org/show_bug.cgi?id=14053
I've written a patch to address this (it currently omits the get_gids()
codepath):
https://github.com/ddiss/ceph/commit/035a1785ec73d803fead42c7240df01b755a81…
Does this approach make sense, or should Samba go down the
ceph_mount_perms_set() route to avoid this bug? The latter
would likely be problematic, as user/group details for a mount will
remain static.
I think that a better approach would be to have samba just call
ceph_mount_perms_set to set the credentials soon after forking. Is there
some reason that doesn't work here?
3 Aug
3 Aug
3:20 a.m.
On Mon, 2019-07-29 at 15:21 +0200, David Disseldorp wrote:
On Thu, 25 Jul 2019 14:34:13 -0400, Jeff Layton
wrote:
Samba becomes root for some privileged operations where Windows would
permit access. E.g. "acl group control", vfs_acl_xattr, etc.
We should be able to change Samba's vfs_ceph to use the ceph_ll_X
API to handle the user<->root perms switches and add corresponding
geteuid()/getegid() checks in each VFS call, but IMO this is still
something that should be fixed in libcephfs, to compliment the existing
geteuid/getegid() fallback behaviour.
I'm not a fan of that fallback behavior, tbqh.
I think it's something that could be subject to race conditions,
especially in threaded programs. I know samba is mostly single-threaded,
but it does occasionally spawn threads and change their credentials to
handle certain calls. Trying to keep the library's creds in sync with
the process/thread creds seems fraught with peril.
I think it'd be best if programs explicitly set what credentials they
want to use by default, and override those creds at appropriate points
as needed.
The thing with the ceph_ll_* calls is that most of them require Inode
pointers and such. They were more designed for an NFS server usecase
where you're dealing with filehandles. You might be able to get away
with using those in a few cases, but I think samba will still want to
use the "normal" ceph_* calls since they are mostly path based.
We could look at adding UserPerm arguments to some of those calls, but
that may mean revving the client library version...which can be done,
but it's not a trivial amount of work.
--
Jeff Layton <jlayton(a)samba.org>
On Thu, 2019-07-25 at 17:07 +0200, David
Disseldorp wrote:
> Hi,
>
> Without calling ceph_mount_perms_set(), libcephfs consumers such as
> Samba can rely upon UserPerm::uid() and UserPerm::gid() to fallback to
> geteuid() and setegid() respectively for things such as ACL enforcement.
^ that should be "geteuid() and getegid() ..."
However,
there is no such fallback for supplementary groups, so ACL
checks for a user which is only permitted path access via a
supplementary group will result in a permission denied error.
Samba ticket: https://bugzilla.samba.org/show_bug.cgi?id=14053
I've written a patch to address this (it currently omits the get_gids()
codepath):
https://github.com/ddiss/ceph/commit/035a1785ec73d803fead42c7240df01b755a81…
Does this approach make sense, or should Samba go down the
ceph_mount_perms_set() route to avoid this bug? The latter
would likely be problematic, as user/group details for a mount will
remain static.
I think that a better approach would be to have samba just call
ceph_mount_perms_set to set the credentials soon after forking. Is there
some reason that doesn't work here?
1725
days inactive
1734
days old
4 comments
3 participants
participants (3)
-
David Disseldorp -
Jeff Layton -
Stefan Metzmacher