Message-ID: <CAEivzxdyLz1ZukU=1OOxcLUDidkwN0WaMn82coya0cc9V67buQ@mail.gmail.com>
Date:   Wed, 21 Jun 2023 18:55:07 +0200
From:   Aleksandr Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
To:     Xiubo Li <xiubli@...hat.com>
Cc:     Gregory Farnum <gfarnum@...hat.com>,
        Christian Brauner <brauner@...nel.org>, stgraber@...ntu.com,
        linux-fsdevel@...r.kernel.org, Ilya Dryomov <idryomov@...il.com>,
        Jeff Layton <jlayton@...nel.org>, ceph-devel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 00/14] ceph: support idmapped mounts

On Thu, Jun 15, 2023 at 2:54 PM Aleksandr Mikhalitsyn
<aleksandr.mikhalitsyn@...onical.com> wrote:
>
> On Thu, Jun 15, 2023 at 2:29 PM Xiubo Li <xiubli@...hat.com> wrote:
> >
> > [...]
> >
> >  > > >
> >  > > > I thought about this too and came to the same conclusion, that
> > UID/GID
> >  > > > based
> >  > > > restriction can be applied dynamically, so detecting it on mount-time
> >  > > > helps not so much.
> >  > > >
> >  > > For this you please raise one PR to ceph first to support this, and in
> >  > > the PR we can discuss more for the MDS auth caps. And after the PR
> >  > > getting merged then in this patch series you need to check the
> >  > > corresponding option or flag to determine whether could the idmap
> >  > > mounting succeed.
> >  >
> >  > I'm sorry but I don't understand what we want to support here. Do we
> > want to
> >  > add some new ceph request that allows to check if UID/GID-based
> >  > permissions are applied for
> >  > a particular ceph client user?
> >
> > IMO we should prevent users to set UID/GID-based MDS auth caps from ceph
> > side. And users should know what has happened.
>
> ok, we want to restrict setting of UID/GID-based permissions if there is an
> idmapped mount on the client. IMHO, idmapping mounts is truly a
> client-side feature
> and server modification looks a bit strange to me.
>
> >
> > Once users want to support the idmap mounts they should know that the
> > MDS auth caps won't work anymore.
>
> They will work, but permission rule configuration should include
> non-mapped UID/GID-s.
> As I mentioned here [1] it's already the case even without mount idmappings.
>
> It would be great to discuss this thing as a concept and synchronize
> our understanding of this
> before going into modification of a server side.

Hi everyone,

I've spent some extra time analyzing this issue with UID/GID-based
path restriction feature and idmapped mounts
one more time and am still fully sure that we have two ways here:
I. Extend Cephfs protocol (add new fields to request arguments in the
"union ceph_mds_request_args")
There should be 2 new fields for the file/directory owner's UID and
GID respectively. With the help of these
new fields, we will be able to split the permission check logic (that
is based on the caller's UID/GID and should not be affected by
the mounts idmapping at all!) and file owner concept, which involves
mounts' idmapping.
II. ignore this issue as non-critical, because:
- idmapped mounts can be created only by privileged users
(CAP_SYS_ADMIN in the superblock owner's user namespace (currently,
it's always the initial user namespace!))
- the surface of the problem is really small (combination of idmapped
mount + UID/GID path-based restriction)
- problem *can* be workarounded by appropriate permission
configuration (UID/GID permissions should be configured to
include both the mount's idmapping UIDs/GIDs and the host ones).

Before that I've highlighted some existing problems of this UID/GID
path-based restriction feature:
- [kernel client] UID/GIDs are sent to the server always from the
initial user namespace (while the caller can be from inside the
container with a non-initial user namespace)
- [fuse client] UID/GIDs are always mapped to the fuse mount's
superblock user namespace
(https://github.com/ceph/ceph-client/blob/409e873ea3c1fd3079909718bbeb06ac1ec7f38b/fs/fuse/dev.c#L138)
It means that we already have analogical inconsistency between clients
(userspace one and kernel).
- [kernel client] We always take current user credentials instead of
using (struct file)->f_cred as it has usually done for other
filesystems

Please understand me in the right way, I'm not trying to say that we
need to be lazy and ignore the issue at all, but I'm
just trying to say that this issue is not local and is not caused by
an idmapped mounts, but it there is something to do on the cephfs
side,
we need to extend protocol and it's not obvious that it is worth it.
My understanding is that we need to clarify this limitation in
cephfs kernel client documentation and explain how to configure
UID/GID path-based permissions with idmapped mounts to work around
this.
And if we get requests from our users that this is interesting to
someone to support it in the right way then we can do all of this
crazy stuff
by extending ceph protocol. Btw, I've checked when "union
ceph_mds_request_args" was extended last time. It was 6 (!) years ago
:)

Kind regards,
Alex

>
> [1] https://lore.kernel.org/lkml/CAEivzxcBBJV6DOGzy5S7=TUjrXZfVaGaJX5z7WFzYq1w4MdtiA@mail.gmail.com/
>
> Kind regards,
> Alex
>
> >
> > Thanks
> >
> > - Xiubo
> >

Powered by blists - more mailing lists