On Tue, Jul 18, 2023 at 3:45 AM Xiubo Li <xiubli@xxxxxxxxxx> wrote: > > > On 7/14/23 20:57, Aleksandr Mikhalitsyn wrote: > > On Tue, Jul 4, 2023 at 3:09 AM Xiubo Li <xiubli@xxxxxxxxxx> wrote: > >> Sorry, not sure, why my last reply wasn't sent out. > >> > >> Do it again. > >> > >> > >> On 6/26/23 19:23, Aleksandr Mikhalitsyn wrote: > >>> On Mon, Jun 26, 2023 at 4:12 AM Xiubo Li<xiubli@xxxxxxxxxx> wrote: > >>>> On 6/24/23 15:11, Aleksandr Mikhalitsyn wrote: > >>>>> On Sat, Jun 24, 2023 at 3:37 AM Xiubo Li<xiubli@xxxxxxxxxx> 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 am afraid there is no. > >> > >> But just after the following ceph PR gets merged it will be easy to do this: > >> > >> https://github.com/ceph/ceph/pull/48027 > >> > >> This is still under testing. > >> > >>>> 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. > >> Maybe no need much, it should be simple IMO. But I am not 100% sure. > >> > >>> 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). > > Dear Xiubo, > > > > I'm sorry for delay with reply, I've missed this message accidentally. > > > >> BTW, could you explain it more ? How could this resolve the issue we are > >> discussing here ? > > This was briefly mentioned here > > https://lore.kernel.org/all/20220105141023.vrrbfhti5apdvkz7@wittgenstein/#t > > by Christian. Let me describe it in detail. > > > > In the current approach we apply mount idmapping to > > head->caller_{uid,gid} fields > > to make mkdir/mknod/symlink operations set a proper inode owner > > uid/gid in according with an idmapping. > > Sorry for late. > > I still couldn't get how this could resolve the lookup case. > > For a lookup request the caller_{uid, gid} still will be the mapped > {uid, gid}, right ? No, the idea is to stop mapping a caller_{uid, gid}. And to add a new fields like inode_owner_{uid, gid} which will be idmapped (this field will be specific only for those operations that create a new inode). > And also the same for other non-create requests. If > so this will be incorrect for the cephx perm checks IMO. Thanks, Alex > > Thanks > > - Xiubo > > > > This makes a problem with path-based UID/GID restriction mechanism, > > because it uses head->caller_{uid,gid} fields > > to check if UID/GID is permitted or not. > > > > So, the problem is that we have one field in ceph request for two > > different needs - to control permissions and to set inode owner. > > Christian pointed that the most saner way is to modify ceph protocol > > and add a separate field to store inode owner UID/GID, > > and only this fields should be idmapped, but head->caller_{uid,gid} > > will be untouched. > > > > With this approach, we will not affect UID/GID-based permission rules > > with an idmapped mounts at all. > > > > Kind regards, > > Alex > > > >> Thanks > >> > >> - Xiubo > >> > >> > >>> Kind regards, > >>> Alex > >>> > >>>> Thanks > >>>> > >>>> - Xiubo > >>>> > >>>> > >>>> > >>>> > >>>> > >>>>> Thanks, > >>>>> Alex > >>>>> > >>>>>> Thanks > >>>>>> > >>>>>> - Xiubo > >>>>>> >
