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. 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 > >>>> >
