Re: [PATCH v5 00/14] ceph: support idmapped mounts

From: Aleksandr Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
To: Xiubo Li <xiubli@redhat.com>
Cc: Gregory Farnum <gfarnum@redhat.com>,
	Christian Brauner <brauner@kernel.org>,
	stgraber@ubuntu.com, linux-fsdevel@vger.kernel.org,
	Ilya Dryomov <idryomov@gmail.com>,
	Jeff Layton <jlayton@kernel.org>,
	ceph-devel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 00/14] ceph: support idmapped mounts
Date: Mon, 26 Jun 2023 13:49:27 +0200	[thread overview]
Message-ID: <CAEivzxeBNOeufOraU27Y+qVApVjAoLhzwPnw0HSkqSt6P3MV9w@mail.gmail.com> (raw)
In-Reply-To: <CAEivzxeQGkemxVwJ148b_+OmntUAWkdL==yMiTMN+GPyaLkFPg@mail.gmail.com>

On Mon, Jun 26, 2023 at 1:23 PM Aleksandr Mikhalitsyn
<aleksandr.mikhalitsyn@canonical.com> wrote:
>
> On Mon, Jun 26, 2023 at 4:12 AM Xiubo Li <xiubli@redhat.com> wrote:
> >
> >
> > On 6/24/23 15:11, Aleksandr Mikhalitsyn wrote:
> > > On Sat, Jun 24, 2023 at 3:37 AM Xiubo Li <xiubli@redhat.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 user to set UID/GID-based permisions caps from
> > >> ceph side.
> > >>
> > >> As I know currently there is no way to prevent users to set MDS auth
> > >> caps, IMO in ceph side at least we need one flag or option to disable
> > >> this once users want this fs cluster sever for idmap mounts use case.
> > > How this should be visible from the user side? We introducing a new
> > > kernel client mount option,
> > > like "nomdscaps", then pass flag to the MDS and MDS should check that
> > > MDS auth permissions
> > > are not applied (on the mount time) and prevent them from being
> > > applied later while session is active. Like that?
> > >
> > > At the same time I'm thinking about protocol extension that adds 2
> > > additional fields for UID/GID. This will allow to correctly
> > > handle everything. I wanted to avoid any changes to the protocol or
> > > server-side things. But if we want to change MDS side,
> > > maybe it's better then to go this way?
>
> Hi Xiubo,
>
> >
> > There is another way:
> >
> > For each client it will have a dedicated client auth caps, something like:
> >
> > client.foo
> >    key: *key*
> >    caps: [mds] allow r, allow rw path=/bar
> >    caps: [mon] allow r
> >    caps: [osd] allow rw tag cephfs data=cephfs_a
>
> Do we have any infrastructure to get this caps list on the client side
> right now?
> (I've taken a quick look through the code and can't find anything
> related to this.)

I've found your PR that looks related https://github.com/ceph/ceph/pull/48027

>
> >
> > When mounting this client with idmap enabled, then we can just check the
> > above [mds] caps, if there has any UID/GID based permissions set, then
> > fail the mounting.
>
> understood
>
> >
> > That means this kind client couldn't be mounted with idmap enabled.
> >
> > Also we need to make sure that once there is a mount with idmap enabled,
> > the corresponding client caps couldn't be append the UID/GID based
> > permissions. This need a patch in ceph anyway IMO.
>
> So, yeah we will need to effectively block cephx permission changes if
> there is a client mounted with
> an active idmapped mount. Sounds as something that require massive
> changes on the server side.
>
> At the same time this will just block users from using idmapped mounts
> along with UID/GID restrictions.
>
> If you want me to change server-side anyways, isn't it better just to
> extend cephfs protocol to properly
> handle UID/GIDs with idmapped mounts? (It was originally proposed by Christian.)
> What we need to do here is to add a separate UID/GID fields for ceph
> requests those are creating a new inodes
> (like mknod, symlink, etc).
>
> Kind regards,
> Alex
>
> >
> > Thanks
> >
> > - Xiubo
> >
> >
> >
> >
> >
> > >
> > > Thanks,
> > > Alex
> > >
> > >> Thanks
> > >>
> > >> - Xiubo
> > >>
> >