On Thu, Aug 29, 2024 at 2:08 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> On Thu, Aug 29, 2024 at 10:24:42AM GMT, Miklos Szeredi wrote:
> > On Thu, 18 Jul 2024 at 21:12, Aleksandr Mikhalitsyn
> > <aleksandr.mikhalitsyn@xxxxxxxxxxxxx> wrote:
> >
> > > This was a first Christian's idea when he originally proposed a
> > > patchset for cephfs [2]. The problem with this
> > > approach is that we don't have an idmapping provided in all
> > > inode_operations, we only have it where it is supposed to be.
> > > To workaround that, Christian suggested applying a mapping only when
> > > we have mnt_idmap, but if not just leave uid/gid as it is.
> > > This, of course, leads to inconsistencies between different
> > > inode_operations, for example ->lookup (idmapping is not applied) and
> > > ->symlink (idmapping is applied).
> > > This inconsistency, really, is not a big deal usually, but... what if
> > > a server does UID/GID-based permission checks? Then it is a problem,
> > > obviously.
> >
> > Is it even sensible to do UID/GID-based permission checks in the
> > server if idmapping is enabled?
Dear friends,
>
> It really makes no sense.
+
>
> >
> > If not, then we should just somehow disable that configuration (i.e.
> > by the server having to opt into idmapping), and then we can just use
> > the in_h.[ugi]d for creates, no?
>
> Fwiw, that's what the patchset is doing. It's only supported if the
> server sets "default_permissions".
Yeah. Thanks, Christian!
That's what we have:
+++ b/fs/fuse/inode.c
@@ -1345,6 +1345,12 @@ static void process_init_reply(struct
fuse_mount *fm, struct fuse_args *args,
fm->sb->s_export_op = &fuse_export_fid_operations;
if (flags & FUSE_OWNER_UID_GID_EXT)
fc->owner_uid_gid_ext = 1;
+ if (flags & FUSE_ALLOW_IDMAP) {
+ if (fc->owner_uid_gid_ext && fc->default_permissions)
+ fm->sb->s_iflags &= ~SB_I_NOIDMAP;
+ else
+ ok = false;
+ }
} else {
ra_pages = fc->max_read / PAGE_SIZE;
So idmapped mounts can be enabled ONLY if "default_permissions" mode
is set. At the same time,
some fuse servers (glusterfs), even when "default_permissions" is set,
still have some UID/GID-based checks.
So, they effectively duplicate permission checking logic in the
userspace. We can not do anything with that, but only
fix fuse servers to stop doing so. See also my PoC for glusterfs-fuse
idmapped mounts support:
https://github.com/mihalicyn/glusterfs/commit/ab3ec2c7cbe22618cba9cc94a52a492b1904d0b2
Kind regards,
Alex
